From a43866d5aedc7d1111a8e2afd00aa35f3d88423f Mon Sep 17 00:00:00 2001 From: Jakub Warczarek Date: Fri, 19 Apr 2024 16:54:20 +0200 Subject: [PATCH] chore(ci): fix dir exclusion and add gocritic to golangci-lint, fix reported issues (#101) --- .golangci.yaml | 9 +++++---- .../controller_reconciler_utils.go | 19 +++++++------------ controller/controlplane/status_conditions.go | 8 ++++---- controller/dataplane/owned_deployment.go | 2 +- controller/gateway/controller.go | 4 +--- modules/admission/validator.go | 17 +++++++---------- pkg/utils/kubernetes/compare/comparators.go | 4 +--- pkg/utils/kubernetes/metadata.go | 2 +- test/conformance/conformance_test.go | 4 ++-- test/conformance/suite_test.go | 13 ++++++++++--- test/integration/suite.go | 13 ++++++++++--- 11 files changed, 49 insertions(+), 46 deletions(-) diff --git a/.golangci.yaml b/.golangci.yaml index 6ddad4207..edbba90b8 100644 --- a/.golangci.yaml +++ b/.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: diff --git a/controller/controlplane/controller_reconciler_utils.go b/controller/controlplane/controller_reconciler_utils.go index dc7280bd1..9fd56466f 100644 --- a/controller/controlplane/controller_reconciler_utils.go +++ b/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 diff --git a/controller/controlplane/status_conditions.go b/controller/controlplane/status_conditions.go index 1b498f783..991615a2b 100644 --- a/controller/controlplane/status_conditions.go +++ b/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, ) } } diff --git a/controller/dataplane/owned_deployment.go b/controller/dataplane/owned_deployment.go index 82eb7d3e9..17c0ed6e2 100644 --- a/controller/dataplane/owned_deployment.go +++ b/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 diff --git a/controller/gateway/controller.go b/controller/gateway/controller.go index 0d41060b7..5e009abcc 100644 --- a/controller/gateway/controller.go +++ b/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 { diff --git a/modules/admission/validator.go b/modules/admission/validator.go index f17097574..a89265223 100644 --- a/modules/admission/validator.go +++ b/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 } diff --git a/pkg/utils/kubernetes/compare/comparators.go b/pkg/utils/kubernetes/compare/comparators.go index d68b8b459..0de36d88c 100644 --- a/pkg/utils/kubernetes/compare/comparators.go +++ b/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 { diff --git a/pkg/utils/kubernetes/metadata.go b/pkg/utils/kubernetes/metadata.go index 98c55d9cf..afabc2a6a 100644 --- a/pkg/utils/kubernetes/metadata.go +++ b/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 } diff --git a/test/conformance/conformance_test.go b/test/conformance/conformance_test.go index d4b307d01..b8dddec54 100644 --- a/test/conformance/conformance_test.go +++ b/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. diff --git a/test/conformance/suite_test.go b/test/conformance/suite_test.go index 7b2e3f60f..ec11f9b34 100644 --- a/test/conformance/suite_test.go +++ b/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) } // ----------------------------------------------------------------------------- diff --git a/test/integration/suite.go b/test/integration/suite.go index 3b60938f8..86e97ec36 100644 --- a/test/integration/suite.go +++ b/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) } // -----------------------------------------------------------------------------