chore(ci): fix dir exclusion and add gocritic to golangci-lint, fix r…

…eported issues (#101)

.golangci.yaml

@@ -1,9 +1,5 @@
 run:
   timeout: 5m
-  skip-dirs:
-  - pkg/clientset
-  - config/
-  - third_party/
 linters:
   enable:
   - asciicheck
@@ -17,6 +13,7 @@ linters:
   - forbidigo
   - gci
   - gofmt
+  - gocritic
   - goimports
   - gomodguard
   - gosec
@@ -113,6 +110,10 @@ linters-settings:
 issues:
   max-same-issues: 0
   fix: true
+  exclude-dirs:
+    - pkg/clientset
+    - config/
+    - third_party/
   include:
     - EXC0012
   exclude-rules:

controller/controlplane/controller_reconciler_utils.go

@@ -116,24 +116,19 @@ func setControlPlaneEnvOnDataPlaneChange(
 	namespace string,
 	dataplaneServiceName string,
 ) bool {
-	var changed bool
-
-	dataplaneIsSet := spec.DataPlane != nil && *spec.DataPlane != ""
 	container := k8sutils.GetPodContainerByName(&spec.Deployment.PodTemplateSpec.Spec, consts.ControlPlaneControllerContainerName)
-	if dataplaneIsSet {
+	if dataplaneIsSet := spec.DataPlane != nil && *spec.DataPlane != ""; dataplaneIsSet {
 		newPublishServiceValue := k8stypes.NamespacedName{Namespace: namespace, Name: dataplaneServiceName}.String()
 		if k8sutils.EnvValueByName(container.Env, "CONTROLLER_PUBLISH_SERVICE") != newPublishServiceValue {
 			container.Env = k8sutils.UpdateEnv(container.Env, "CONTROLLER_PUBLISH_SERVICE", newPublishServiceValue)
-			changed = true
-		}
-	} else {
-		if k8sutils.EnvValueByName(container.Env, "CONTROLLER_PUBLISH_SERVICE") != "" {
-			container.Env = k8sutils.RejectEnvByName(container.Env, "CONTROLLER_PUBLISH_SERVICE")
-			changed = true
+			return true
 		}
+	} else if k8sutils.EnvValueByName(container.Env, "CONTROLLER_PUBLISH_SERVICE") != "" {
+		container.Env = k8sutils.RejectEnvByName(container.Env, "CONTROLLER_PUBLISH_SERVICE")
+		return true
 	}
 
-	return changed
+	return false
 }
 
 // -----------------------------------------------------------------------------
@@ -208,7 +203,7 @@ func (r *Reconciler) ensureDeployment(
 		// some custom comparison rules are needed for some PodTemplateSpec sub-attributes, in particular
 		// resources and affinity.
 		opts := []cmp.Option{
-			cmp.Comparer(func(a, b corev1.ResourceRequirements) bool { return k8sresources.ResourceRequirementsEqual(a, b) }),
+			cmp.Comparer(k8sresources.ResourceRequirementsEqual),
 		}
 
 		// ensure that PodTemplateSpec is up to date

controller/controlplane/status_conditions.go

@@ -10,17 +10,17 @@ import (
 // markAsProvisioned marks the provided resource as ready by the means of Provisioned
 // Status Condition.
 func markAsProvisioned[T *operatorv1beta1.ControlPlane](resource T) {
-	switch resource := any(resource).(type) {
-	case *operatorv1beta1.ControlPlane:
+	cp, ok := any(resource).(*operatorv1beta1.ControlPlane)
+	if ok {
 		k8sutils.SetCondition(
 			k8sutils.NewConditionWithGeneration(
 				ConditionTypeProvisioned,
 				metav1.ConditionTrue,
 				ConditionReasonPodsReady,
 				"pods for all Deployments are ready",
-				resource.Generation,
+				cp.Generation,
 			),
-			resource,
+			cp,
 		)
 	}
 }

controller/dataplane/owned_deployment.go

@@ -307,7 +307,7 @@ func reconcileDataPlaneDeployment(
 		// some custom comparison rules are needed for some PodTemplateSpec sub-attributes, in particular
 		// resources and affinity.
 		opts := []cmp.Option{
-			cmp.Comparer(func(a, b corev1.ResourceRequirements) bool { return k8sresources.ResourceRequirementsEqual(a, b) }),
+			cmp.Comparer(k8sresources.ResourceRequirementsEqual),
 		}
 
 		// ensure that PodTemplateSpec is up to date

controller/gateway/controller.go

@@ -668,9 +668,7 @@ func deploymentOptionsDeepEqual(o1, o2 *operatorv1beta1.DeploymentOptions, envVa
 	}
 
 	opts := []cmp.Option{
-		cmp.Comparer(func(a, b corev1.ResourceRequirements) bool {
-			return k8sresources.ResourceRequirementsEqual(a, b)
-		}),
+		cmp.Comparer(k8sresources.ResourceRequirementsEqual),
 		cmp.Comparer(func(a, b []corev1.EnvVar) bool {
 			// Throw out env vars that we ignore.
 			a = lo.Filter(a, func(e corev1.EnvVar, _ int) bool {

modules/admission/validator.go

@@ -22,21 +22,18 @@ func (v *validator) ValidateControlPlane(ctx context.Context, controlPlane opera
 
 // ValidateDataPlane validates the DataPlane resource.
 func (v *validator) ValidateDataPlane(ctx context.Context, dataPlane, old operatorv1beta1.DataPlane, operation admissionv1.Operation) error {
-	//nolint:exhaustive
 	switch operation {
 	case admissionv1.Update, admissionv1.Create:
 		if err := v.dataplaneValidator.Validate(&dataPlane); err != nil {
 			return err
 		}
-	}
-
-	//nolint:exhaustive
-	switch operation {
-	case admissionv1.Update:
-		if err := v.dataplaneValidator.ValidateUpdate(&dataPlane, &old); err != nil {
-			return err
+		if operation == admissionv1.Update {
+			if err := v.dataplaneValidator.ValidateUpdate(&dataPlane, &old); err != nil {
+				return err
+			}
 		}
+		return nil
+	default:
+		return nil
 	}
-
-	return nil
 }

pkg/utils/kubernetes/compare/comparators.go

@@ -26,9 +26,7 @@ func ControlPlaneDeploymentOptionsDeepEqual(o1, o2 *operatorv1beta1.ControlPlane
 	}
 
 	opts := []cmp.Option{
-		cmp.Comparer(func(a, b corev1.ResourceRequirements) bool {
-			return resources.ResourceRequirementsEqual(a, b)
-		}),
+		cmp.Comparer(resources.ResourceRequirementsEqual),
 		cmp.Comparer(func(a, b []corev1.EnvVar) bool {
 			// Throw out env vars that we ignore.
 			a = lo.Filter(a, func(e corev1.EnvVar, _ int) bool {

pkg/utils/kubernetes/metadata.go

@@ -72,7 +72,7 @@ func TrimGenerateName(name string) string {
 		name = name[:62]
 	}
 	if !strings.HasSuffix(name, "-") {
-		name = name + "-"
+		name += "-"
 	}
 	return name
 }

test/conformance/conformance_test.go

@@ -28,7 +28,7 @@ var skippedTests = []string{
 	tests.GatewayModifyListeners.ShortName,
 	tests.GatewayWithAttachedRoutes.ShortName,
 
-	//httproute
+	// httproute
 	tests.HTTPRouteHeaderMatching.ShortName,
 }
 
@@ -96,7 +96,7 @@ func TestGatewayConformance(t *testing.T) {
 	// To work with individual tests only, you can disable the normal Run call and construct a slice containing a
 	// single test only, e.g.:
 	//
-	//cSuite.Run(t, []suite.ConformanceTest{tests.HTTPRouteRedirectPortAndScheme})
+	// cSuite.Run(t, []suite.ConformanceTest{tests.HTTPRouteRedirectPortAndScheme})
 	require.NoError(t, cSuite.Run(t, tests.ConformanceTests))
 
 	// In the future we'll likely change the file name as https://github.com/kubernetes-sigs/gateway-api/issues/2740 will be implemented.

test/conformance/suite_test.go

@@ -6,6 +6,7 @@ import (
 	"net/http"
 	"os"
 	"path"
+	"runtime/debug"
 	"testing"
 	"time"
 
@@ -54,6 +55,14 @@ var (
 // -----------------------------------------------------------------------------
 
 func TestMain(m *testing.M) {
+	var code int
+	defer func() {
+		if r := recover(); r != nil {
+			fmt.Printf("%v stack trace:\n%s\n", r, debug.Stack())
+			code = 1
+		}
+		os.Exit(code)
+	}()
 	ctx, cancel = context.WithCancel(context.Background())
 	defer cancel()
 
@@ -103,7 +112,7 @@ func TestMain(m *testing.M) {
 	exitOnErr(testutils.BuildMTLSCredentials(ctx, clients.K8sClient, &httpc))
 
 	fmt.Println("INFO: environment is ready, starting tests")
-	code := m.Run()
+	code = m.Run()
 	if code != 0 {
 		output, err := env.Cluster().DumpDiagnostics(ctx, "gateway_api_conformance")
 		if err != nil {
@@ -125,8 +134,6 @@ func TestMain(m *testing.M) {
 		fmt.Println("INFO: cleaning up testing cluster and environment")
 		exitOnErr(env.Cleanup(ctx))
 	}
-
-	os.Exit(code)
 }
 
 // -----------------------------------------------------------------------------

test/integration/suite.go

@@ -8,6 +8,7 @@ import (
 	"net/http"
 	"os"
 	"path"
+	"runtime/debug"
 	"strings"
 	"testing"
 	"time"
@@ -108,6 +109,14 @@ func TestMain(
 	m *testing.M,
 	setUpAndRunManager SetUpAndRunManagerFunc,
 ) {
+	var code int
+	defer func() {
+		if r := recover(); r != nil {
+			fmt.Printf("%v stack trace:\n%s\n", r, debug.Stack())
+			code = 1
+		}
+		os.Exit(code)
+	}()
 	var cancel context.CancelFunc
 	ctx, cancel = context.WithCancel(context.Background())
 	defer cancel()
@@ -179,14 +188,12 @@ func TestMain(
 	}
 
 	fmt.Println("INFO: environment is ready, starting tests")
-	code := m.Run()
+	code = m.Run()
 
 	if !skipClusterCleanup && existingCluster == "" {
 		fmt.Println("INFO: cleaning up testing cluster and environment")
 		exitOnErr(GetEnv().Cleanup(GetCtx()))
 	}
-
-	os.Exit(code)
 }
 
 // -----------------------------------------------------------------------------