diff --git a/.golangci.yml b/.golangci.yml index 29bee73ab6..0277e67195 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -53,14 +53,19 @@ linters: enable: - asasalint - asciicheck + - bidichk - copyloopvar - dupword + - durationcheck - errcheck + - errchkjson - errname - errorlint - fatcontext - ginkgolinter - gocheckcompilerdirectives + - gochecksumtype + - gocritic - gocyclo - godot - gofmt @@ -76,6 +81,7 @@ linters: - loggercheck - makezero - misspell + - musttag - nilerr - noctx - nolintlint @@ -86,6 +92,7 @@ linters: - spancheck - staticcheck - stylecheck + - tagalign - tenv - thelper - tparallel diff --git a/internal/mode/provisioner/manager.go b/internal/mode/provisioner/manager.go index 1052120d11..7651b05a2e 100644 --- a/internal/mode/provisioner/manager.go +++ b/internal/mode/provisioner/manager.go @@ -133,7 +133,7 @@ func StartManager(cfg Config) error { statusUpdater, mgr.GetClient(), embeddedfiles.StaticModeDeploymentYAML, - func() metav1.Time { return metav1.Now() }, + metav1.Now, ) eventLoop := events.NewEventLoop( diff --git a/internal/mode/static/handler.go b/internal/mode/static/handler.go index cf4410f013..d8cd558fa0 100644 --- a/internal/mode/static/handler.go +++ b/internal/mode/static/handler.go @@ -471,8 +471,7 @@ func getGatewayAddresses( } var addresses, hostnames []string - switch gwSvc.Spec.Type { - case v1.ServiceTypeLoadBalancer: + if gwSvc.Spec.Type == v1.ServiceTypeLoadBalancer { for _, ingress := range gwSvc.Status.LoadBalancer.Ingress { if ingress.IP != "" { addresses = append(addresses, ingress.IP) diff --git a/internal/mode/static/manager.go b/internal/mode/static/manager.go index 9663f215cf..5b0cd4c8b0 100644 --- a/internal/mode/static/manager.go +++ b/internal/mode/static/manager.go @@ -107,7 +107,8 @@ func StartManager(cfg config.Config) error { Namespace: cfg.GatewayPodConfig.Namespace, Name: cfg.ConfigName, } - if err := registerControllers(ctx, cfg, mgr, recorder, logLevelSetter, eventCh, controlConfigNSName); err != nil { + err = registerControllers(ctx, cfg, mgr, recorder, logLevelSetter, eventCh, controlConfigNSName) + if err != nil { return err } @@ -149,9 +150,11 @@ func StartManager(cfg config.Config) error { processHandler := ngxruntime.NewProcessHandlerImpl(os.ReadFile, os.Stat) // Ensure NGINX is running before registering metrics & starting the manager. - if _, err := processHandler.FindMainProcess(ctx, ngxruntime.PidFileTimeout); err != nil { + p, err := processHandler.FindMainProcess(ctx, ngxruntime.PidFileTimeout) + if err != nil { return fmt.Errorf("NGINX is not running: %w", err) } + cfg.Logger.Info("NGINX is running with PID", "pid", p) var ( ngxruntimeCollector ngxruntime.MetricsCollector = collectors.NewManagerNoopCollector() @@ -162,11 +165,6 @@ func StartManager(cfg config.Config) error { var usageSecret *usage.Secret if cfg.Plus { - ngxPlusClient, err = ngxruntime.CreatePlusClient() - if err != nil { - return fmt.Errorf("error creating NGINX plus client: %w", err) - } - if cfg.UsageReportConfig != nil { usageSecret = usage.NewUsageSecret() reporter, err := createUsageReporterJob(mgr.GetAPIReader(), cfg, usageSecret, nginxChecker.getReadyCh()) @@ -188,6 +186,10 @@ func StartManager(cfg config.Config) error { constLabels := map[string]string{"class": cfg.GatewayClassName} var ngxCollector prometheus.Collector if cfg.Plus { + ngxPlusClient, err = ngxruntime.CreatePlusClient() + if err != nil { + return fmt.Errorf("error creating NGINX plus client: %w", err) + } ngxCollector, err = collectors.NewNginxPlusMetricsCollector(ngxPlusClient, constLabels, promLogger) } else { ngxCollector = collectors.NewNginxMetricsCollector(constLabels, promLogger) diff --git a/internal/mode/static/nginx/config/validation/common_test.go b/internal/mode/static/nginx/config/validation/common_test.go index da3059e4c4..33cdd0ee5e 100644 --- a/internal/mode/static/nginx/config/validation/common_test.go +++ b/internal/mode/static/nginx/config/validation/common_test.go @@ -48,7 +48,7 @@ func TestValidateEscapedStringNoVarExpansion(t *testing.T) { func TestValidateValidHeaderName(t *testing.T) { t.Parallel() - validator := func(value string) error { return validateHeaderName(value) } + validator := validateHeaderName testValidValuesForSimpleValidator( t, diff --git a/internal/mode/static/nginx/file/folders.go b/internal/mode/static/nginx/file/folders.go index ad0669a395..d3a3eebbd5 100644 --- a/internal/mode/static/nginx/file/folders.go +++ b/internal/mode/static/nginx/file/folders.go @@ -28,12 +28,12 @@ func ClearFolders(fileMgr ClearFoldersOSFileManager, paths []string) (removedFil } for _, entry := range entries { - path := filepath.Join(path, entry.Name()) - if err := fileMgr.Remove(path); err != nil { - return removedFiles, fmt.Errorf("failed to remove %q: %w", path, err) + entryPath := filepath.Join(path, entry.Name()) + if err := fileMgr.Remove(entryPath); err != nil { + return removedFiles, fmt.Errorf("failed to remove %q: %w", entryPath, err) } - removedFiles = append(removedFiles, path) + removedFiles = append(removedFiles, entryPath) } } diff --git a/internal/mode/static/nginx/runtime/manager.go b/internal/mode/static/nginx/runtime/manager.go index fd7bf9e3fc..45d24dbb75 100644 --- a/internal/mode/static/nginx/runtime/manager.go +++ b/internal/mode/static/nginx/runtime/manager.go @@ -121,9 +121,9 @@ func (m *ManagerImpl) Reload(ctx context.Context, configVersion int) error { // send HUP signal to the NGINX main process reload configuration // See https://nginx.org/en/docs/control.html - if err := m.processHandler.Kill(pid); err != nil { + if errP := m.processHandler.Kill(pid); errP != nil { m.metricsCollector.IncReloadErrors() - return fmt.Errorf("failed to send the HUP signal to NGINX main: %w", err) + return fmt.Errorf("failed to send the HUP signal to NGINX main: %w", errP) } if err = m.verifyClient.WaitForCorrectVersion( diff --git a/internal/mode/static/state/dataplane/configuration.go b/internal/mode/static/state/dataplane/configuration.go index eefe5e4bb4..a313aef6da 100644 --- a/internal/mode/static/state/dataplane/configuration.go +++ b/internal/mode/static/state/dataplane/configuration.go @@ -279,12 +279,12 @@ func buildBackendGroups(servers []VirtualServer) []BackendGroup { for _, mr := range pr.MatchRules { group := mr.BackendGroup - key := key{ + k := key{ nsname: group.Source, ruleIdx: group.RuleIdx, } - uniqueGroups[key] = group + uniqueGroups[k] = group } } } diff --git a/internal/mode/static/state/graph/backend_refs.go b/internal/mode/static/state/graph/backend_refs.go index 2d441fca38..524032de17 100644 --- a/internal/mode/static/state/graph/backend_refs.go +++ b/internal/mode/static/state/graph/backend_refs.go @@ -238,12 +238,11 @@ func validateBackendTLSPolicyMatchingAllBackends(backendRefs []BackendRef) *cond if referencePolicy == nil { // First reference, store the policy as reference referencePolicy = backendRef.BackendTLSPolicy - } else { - // Check if the policies match - if checkPoliciesEqual(backendRef.BackendTLSPolicy.Source, referencePolicy.Source) { - mismatch = true - break - } + } else + // Check if the policies match + if checkPoliciesEqual(backendRef.BackendTLSPolicy.Source, referencePolicy.Source) { + mismatch = true + break } } if mismatch { diff --git a/internal/mode/static/state/graph/backend_tls_policy.go b/internal/mode/static/state/graph/backend_tls_policy.go index 5e2169cfbb..7ef8570331 100644 --- a/internal/mode/static/state/graph/backend_tls_policy.go +++ b/internal/mode/static/state/graph/backend_tls_policy.go @@ -86,23 +86,27 @@ func validateBackendTLSPolicy( caCertRefs := backendTLSPolicy.Spec.Validation.CACertificateRefs wellKnownCerts := backendTLSPolicy.Spec.Validation.WellKnownCACertificates - if len(caCertRefs) > 0 && wellKnownCerts != nil { + switch { + case len(caCertRefs) > 0 && wellKnownCerts != nil: valid = false msg := "CACertificateRefs and WellKnownCACertificates are mutually exclusive" conds = append(conds, staticConds.NewPolicyInvalid(msg)) - } else if len(caCertRefs) > 0 { + + case len(caCertRefs) > 0: if err := validateBackendTLSCACertRef(backendTLSPolicy, configMapResolver); err != nil { valid = false conds = append(conds, staticConds.NewPolicyInvalid( fmt.Sprintf("invalid CACertificateRef: %s", err.Error()))) } - } else if wellKnownCerts != nil { + + case wellKnownCerts != nil: if err := validateBackendTLSWellKnownCACerts(backendTLSPolicy); err != nil { valid = false conds = append(conds, staticConds.NewPolicyInvalid( fmt.Sprintf("invalid WellKnownCACertificates: %s", err.Error()))) } - } else { + + default: valid = false conds = append(conds, staticConds.NewPolicyInvalid("CACertRefs and WellKnownCACerts are both nil")) } diff --git a/internal/mode/static/state/graph/backend_tls_policy_test.go b/internal/mode/static/state/graph/backend_tls_policy_test.go index b4ded8f21d..3d89fa399e 100644 --- a/internal/mode/static/state/graph/backend_tls_policy_test.go +++ b/internal/mode/static/state/graph/backend_tls_policy_test.go @@ -122,7 +122,18 @@ func TestValidateBackendTLSPolicy(t *testing.T) { }, } - localObjectRefTooManyCerts := append(localObjectRefNormalCase, localObjectRefInvalidName...) + localObjectRefTooManyCerts := []gatewayv1.LocalObjectReference{ + { + Kind: "ConfigMap", + Name: "configmap", + Group: "", + }, + { + Kind: "ConfigMap", + Name: "invalid", + Group: "", + }, + } getAncestorRef := func(ctlrName, parentName string) v1alpha2.PolicyAncestorStatus { return v1alpha2.PolicyAncestorStatus{ diff --git a/internal/mode/static/state/graph/gateway_listener.go b/internal/mode/static/state/graph/gateway_listener.go index d8e7a60ff5..b5dadce8bf 100644 --- a/internal/mode/static/state/graph/gateway_listener.go +++ b/internal/mode/static/state/graph/gateway_listener.go @@ -563,7 +563,7 @@ func GetAllowedRouteLabelSelector(l v1.Listener) *metav1.LabelSelector { // matchesWildcard checks if hostname2 matches the wildcard pattern of hostname1. func matchesWildcard(hostname1, hostname2 string) bool { - matchesWildcard := func(h1, h2 string) bool { + mw := func(h1, h2 string) bool { if strings.HasPrefix(h1, "*.") { // Remove the "*." from h1 h1 = h1[2:] @@ -572,7 +572,7 @@ func matchesWildcard(hostname1, hostname2 string) bool { } return false } - return matchesWildcard(hostname1, hostname2) || matchesWildcard(hostname2, hostname1) + return mw(hostname1, hostname2) || mw(hostname2, hostname1) } // haveOverlap checks for overlap between two hostnames. diff --git a/internal/mode/static/state/graph/secret.go b/internal/mode/static/state/graph/secret.go index 9cbbd84663..c7d339aae4 100644 --- a/internal/mode/static/state/graph/secret.go +++ b/internal/mode/static/state/graph/secret.go @@ -44,11 +44,14 @@ func (r *secretResolver) resolve(nsname types.NamespacedName) error { var validationErr error - if !exist { + switch { + case !exist: validationErr = errors.New("secret does not exist") - } else if secret.Type != apiv1.SecretTypeTLS { + + case secret.Type != apiv1.SecretTypeTLS: validationErr = fmt.Errorf("secret type must be %q not %q", apiv1.SecretTypeTLS, secret.Type) - } else { + + default: // A TLS Secret is guaranteed to have these data fields. _, err := tls.X509KeyPair(secret.Data[apiv1.TLSCertKey], secret.Data[apiv1.TLSPrivateKeyKey]) if err != nil { diff --git a/internal/mode/static/state/resolver/resolver.go b/internal/mode/static/state/resolver/resolver.go index 6ceb9637cd..440a245975 100644 --- a/internal/mode/static/state/resolver/resolver.go +++ b/internal/mode/static/state/resolver/resolver.go @@ -155,11 +155,8 @@ func resolveEndpoints( // If the ServicePort has a non-zero integer TargetPort, the TargetPort integer value is returned. // Otherwise, the ServicePort port value is returned. func getDefaultPort(svcPort v1.ServicePort) int32 { - switch svcPort.TargetPort.Type { - case intstr.Int: - if svcPort.TargetPort.IntVal != 0 { - return svcPort.TargetPort.IntVal - } + if svcPort.TargetPort.Type == intstr.Int && svcPort.TargetPort.IntVal != 0 { + return svcPort.TargetPort.IntVal } return svcPort.Port diff --git a/internal/mode/static/status/prepare_requests.go b/internal/mode/static/status/prepare_requests.go index b7b13b4261..b8d39a0c1c 100644 --- a/internal/mode/static/status/prepare_requests.go +++ b/internal/mode/static/status/prepare_requests.go @@ -68,7 +68,8 @@ func PrepareRouteRequests( r.Source.GetGeneration(), ) - if r.RouteType == graph.RouteTypeHTTP { + switch r.RouteType { + case graph.RouteTypeHTTP: status := v1.HTTPRouteStatus{ RouteStatus: routeStatus, } @@ -80,7 +81,8 @@ func PrepareRouteRequests( } reqs = append(reqs, req) - } else if r.RouteType == graph.RouteTypeGRPC { + + case graph.RouteTypeGRPC: status := v1.GRPCRouteStatus{ RouteStatus: routeStatus, } @@ -92,7 +94,8 @@ func PrepareRouteRequests( } reqs = append(reqs, req) - } else { + + default: panic(fmt.Sprintf("Unknown route type: %s", r.RouteType)) } } diff --git a/internal/mode/static/telemetry/collector.go b/internal/mode/static/telemetry/collector.go index 64973da90e..bcfd6fe283 100644 --- a/internal/mode/static/telemetry/collector.go +++ b/internal/mode/static/telemetry/collector.go @@ -245,10 +245,10 @@ func computeRouteCount( for _, r := range routes { if r.RouteType == graph.RouteTypeHTTP { - httpRouteCount = httpRouteCount + 1 + httpRouteCount++ } if r.RouteType == graph.RouteTypeGRPC { - grpcRouteCount = grpcRouteCount + 1 + grpcRouteCount++ } } diff --git a/internal/mode/static/usage/job_worker_test.go b/internal/mode/static/usage/job_worker_test.go index 034a2dade2..ac7676e853 100644 --- a/internal/mode/static/usage/job_worker_test.go +++ b/internal/mode/static/usage/job_worker_test.go @@ -58,8 +58,7 @@ func TestCreateUsageJobWorker(t *testing.T) { return nil }, getCalls: func(_ context.Context, _ types.NamespacedName, object client.Object, _ ...client.GetOption) error { - switch typedObject := object.(type) { - case *v1.Namespace: + if typedObject, ok := object.(*v1.Namespace); ok { typedObject.Name = metav1.NamespaceSystem typedObject.UID = "1234abcd" return nil @@ -83,8 +82,7 @@ func TestCreateUsageJobWorker(t *testing.T) { { name: "collect node count fails", listCalls: func(_ context.Context, object client.ObjectList, _ ...client.ListOption) error { - switch object.(type) { - case *v1.NodeList: + if _, ok := object.(*v1.NodeList); ok { return errors.New("failed to collect node list") } return nil @@ -127,8 +125,7 @@ func TestCreateUsageJobWorker(t *testing.T) { return nil }, getCalls: func(_ context.Context, _ types.NamespacedName, object client.Object, _ ...client.GetOption) error { - switch object.(type) { - case *v1.Namespace: + if _, ok := object.(*v1.Namespace); ok { return errors.New("failed to collect namespace") } return nil diff --git a/tests/framework/ngf.go b/tests/framework/ngf.go index 64eb23be25..67b1257464 100644 --- a/tests/framework/ngf.go +++ b/tests/framework/ngf.go @@ -73,7 +73,7 @@ func InstallNGF(cfg InstallationConfig, extraArgs ...string) ([]byte, error) { } args = append(args, setImageArgs(cfg)...) - fullArgs := append(args, extraArgs...) + fullArgs := append(args, extraArgs...) //nolint:gocritic GinkgoWriter.Printf("Installing NGF with command: helm %v\n", strings.Join(fullArgs, " ")) @@ -102,7 +102,7 @@ func UpgradeNGF(cfg InstallationConfig, extraArgs ...string) ([]byte, error) { } args = append(args, setImageArgs(cfg)...) - fullArgs := append(args, extraArgs...) + fullArgs := append(args, extraArgs...) //nolint:gocritic GinkgoWriter.Printf("Upgrading NGF with command: helm %v\n", strings.Join(fullArgs, " ")) diff --git a/tests/suite/system_suite_test.go b/tests/suite/system_suite_test.go index 524898ffd0..d99d4b7f63 100644 --- a/tests/suite/system_suite_test.go +++ b/tests/suite/system_suite_test.go @@ -148,11 +148,12 @@ func setup(cfg setupConfig, extraInstallArgs ...string) { Skip("Graceful Recovery test must be run on Kind") } - if *versionUnderTest != "" { + switch { + case *versionUnderTest != "": version = *versionUnderTest - } else if *imageTag != "" { + case *imageTag != "": version = *imageTag - } else { + default: version = "edge" } @@ -203,11 +204,9 @@ func createNGFInstallConfig(cfg setupConfig, extraInstallArgs ...string) framewo } installCfg.ImageTag = *imageTag installCfg.ImagePullPolicy = *imagePullPolicy - } else { - if version == "edge" { - chartVersion = "0.0.0-edge" - installCfg.ChartVersion = chartVersion - } + } else if version == "edge" { + chartVersion = "0.0.0-edge" + installCfg.ChartVersion = chartVersion } output, err := framework.InstallGatewayAPI(cfg.gwAPIVersion) diff --git a/tests/suite/tracing_test.go b/tests/suite/tracing_test.go index 4e4519e3c5..354c1137d0 100644 --- a/tests/suite/tracing_test.go +++ b/tests/suite/tracing_test.go @@ -142,17 +142,13 @@ var _ = Describe("Tracing", FlakeAttempts(2), Label("functional", "tracing"), fu checkStatusAndTraces := func() { Eventually( - func() error { - return verifyGatewayClassResolvedRefs() - }). + verifyGatewayClassResolvedRefs). WithTimeout(timeoutConfig.GetTimeout). WithPolling(500 * time.Millisecond). Should(Succeed()) Eventually( - func() error { - return verifyPolicyStatus() - }). + verifyPolicyStatus). WithTimeout(timeoutConfig.GetTimeout). WithPolling(500 * time.Millisecond). Should(Succeed())