Add more linters

Problem: We want to catch errors and styling issues as early as
possible.

Solution: Enable new linters.

Author: lucacome

.github/workflows/ci.yml

@@ -43,6 +43,9 @@ jobs:
         uses: actions/setup-go@cdcb36043654635271a94b9a6d1392de5bb323a7 # v5.0.1
         with:
           go-version: stable
+          cache-dependency-path: |
+            go.sum
+            .github/.cache/buster-for-vars
 
       - name: Check for changes
         uses: dorny/paths-filter@de90cc6fb38fc0963ad72b210f1f284cd68cea36 # v3.0.2
@@ -81,6 +84,9 @@ jobs:
         uses: actions/setup-go@cdcb36043654635271a94b9a6d1392de5bb323a7 # v5.0.1
         with:
           go-version: stable
+          cache-dependency-path: |
+            go.sum
+            .github/.cache/buster-for-unit-tests
 
       - name: Run Tests
         run: make unit-test

.golangci.yml

@@ -37,33 +37,57 @@ linters-settings:
       - fieldalignment
   lll:
     line-length: 120
+  dupword:
+    ignore:
+      - "test"
 linters:
   enable:
+    - asasalint
     - asciicheck
+    - dupword
     - errcheck
+    - errname
     - errorlint
+    - exportloopref
+    - fatcontext
+    - forcetypeassert
     - ginkgolinter
+    - gocheckcompilerdirectives
     - gocyclo
+    - godot
     - gofmt
     - gofumpt
     - goimports
     - gosec
     - gosimple
+    - gosmopolitan
     - govet
     - ineffassign
+    - intrange
     - lll
+    - loggercheck
     - makezero
     - misspell
     - nilerr
     - noctx
+    - nolintlint
+    - prealloc
     - predeclared
+    - promlinter
+    - reassign
     - revive
+    - spancheck
     - staticcheck
+    - stylecheck
+    - tenv
+    - thelper
     - typecheck
     - unconvert
     - unparam
     - unused
+    - usestdlibvars
     - wastedassign
+    - whitespace
   disable-all: true
 issues:
   max-issues-per-linter: 0

Makefile

@@ -188,7 +188,7 @@ check-golangci-lint:
 
 .PHONY: lint
 lint: check-golangci-lint ## Run golangci-lint against code
-	golangci-lint run
+	golangci-lint run --fix
 
 .PHONY: unit-test
 unit-test: ## Run unit tests for the go code

apis/v1alpha1/clientsettingspolicy_types.go

@@ -108,7 +108,7 @@ type ClientKeepAlive struct {
 }
 
 // ClientKeepAliveTimeout defines the timeouts related to keep-alive client connections.
-// Default: Default: https://nginx.org/en/docs/http/ngx_http_core_module.html#keepalive_timeout.
+// Default: https://nginx.org/en/docs/http/ngx_http_core_module.html#keepalive_timeout.
 type ClientKeepAliveTimeout struct {
 	// Server sets the timeout during which a keep-alive client connection will stay open on the server side.
 	// Setting this value to 0 disables keep-alive client connections.

apis/v1alpha1/register.go

@@ -9,10 +9,10 @@ import (
 // GroupName specifies the group name used to register the objects.
 const GroupName = "gateway.nginx.org"
 
-// SchemeGroupVersion is group version used to register these objects
+// SchemeGroupVersion is group version used to register these objects.
 var SchemeGroupVersion = schema.GroupVersion{Group: GroupName, Version: "v1alpha1"}
 
-// Resource takes an unqualified resource and returns a Group qualified GroupResource
+// Resource takes an unqualified resource and returns a Group qualified GroupResource.
 func Resource(resource string) schema.GroupResource {
 	return SchemeGroupVersion.WithResource(resource).GroupResource()
 }

cmd/gateway/commands.go

@@ -444,8 +444,9 @@ func createProvisionerModeCommand() *cobra.Command {
 
 // FIXME(pleshakov): Remove this command once NGF min supported Kubernetes version supports sleep action in
 // preStop hook.
-// nolint:lll
 // See https://github.com/kubernetes/enhancements/tree/4ec371d92dcd4f56a2ab18c8ba20bb85d8d20efe/keps/sig-node/3960-pod-lifecycle-sleep-action
+//
+//nolint:lll
 func createSleepCommand() *cobra.Command {
 	// flag names
 	const durationFlag = "duration"

cmd/gateway/commands_test.go

@@ -18,6 +18,7 @@ type flagTestCase struct {
 }
 
 func testFlag(t *testing.T, cmd *cobra.Command, test flagTestCase) {
+	t.Helper()
 	g := NewWithT(t)
 	// discard any output generated by cobra
 	cmd.SetOut(io.Discard)

cmd/gateway/main.go

@@ -5,7 +5,7 @@ import (
 	"os"
 )
 
-// Set during go build
+// Set during go build.
 var (
 	version string
 	commit  string

cmd/gateway/validation.go

@@ -14,7 +14,6 @@ import (
 )
 
 const (
-	// nolint:lll
 	// Regex from: https://github.com/kubernetes-sigs/gateway-api/blob/v1.1.0/apis/v1/shared_types.go#L647
 	controllerNameRegex = `^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*\/[A-Za-z0-9\/\-._~%!$&'()*+,;=:]+$` //nolint:lll
 )
@@ -163,15 +162,15 @@ func validateEndpoint(endpoint string) error {
 	return fmt.Errorf("%q must be in the format <host>:<port>", endpoint)
 }
 
-// validatePort makes sure a given port is inside the valid port range for its usage
+// validatePort makes sure a given port is inside the valid port range for its usage.
 func validatePort(port int) error {
 	if port < 1024 || port > 65535 {
 		return fmt.Errorf("port outside of valid port range [1024 - 65535]: %v", port)
 	}
 	return nil
 }
 
-// ensureNoPortCollisions checks if the same port has been defined multiple times
+// ensureNoPortCollisions checks if the same port has been defined multiple times.
 func ensureNoPortCollisions(ports ...int) error {
 	seen := make(map[int]struct{})
 

internal/framework/controller/predicate/service.go

@@ -50,7 +50,7 @@ func (ServicePortsChangedPredicate) Update(e event.UpdateEvent) bool {
 	oldPortSet := make(map[ports]struct{})
 	newPortSet := make(map[ports]struct{})
 
-	for i := 0; i < len(oldSvc.Spec.Ports); i++ {
+	for i := range len(oldSvc.Spec.Ports) {
 		oldPortSet[ports{servicePort: oldPorts[i].Port, targetPort: oldPorts[i].TargetPort}] = struct{}{}
 		newPortSet[ports{servicePort: newPorts[i].Port, targetPort: newPorts[i].TargetPort}] = struct{}{}
 	}

internal/framework/controller/reconciler.go

@@ -65,7 +65,11 @@ func (r *Reconciler) newObject(objectType ngftypes.ObjectType) ngftypes.ObjectTy
 	t := reflect.TypeOf(objectType).Elem()
 
 	// We could've used objectType.DeepCopyObject() here, but it's a bit slower confirmed by benchmarks.
-	return reflect.New(t).Interface().(client.Object)
+	obj, ok := reflect.New(t).Interface().(client.Object)
+	if !ok {
+		panic("failed to create a new object")
+	}
+	return obj
 }
 
 // Reconcile implements the reconcile.Reconciler Reconcile method.

internal/framework/kinds/kinds.go

@@ -9,9 +9,9 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/client/apiutil"
 )
 
-// Gateway API Kinds
+// Gateway API Kinds.
 const (
-	// Gateway is the Gateway Kind
+	// Gateway is the Gateway Kind.
 	Gateway = "Gateway"
 	// GatewayClass is the GatewayClass Kind.
 	GatewayClass = "GatewayClass"

internal/framework/runnables/cronjob.go

@@ -37,7 +37,7 @@ func NewCronJob(cfg CronJobConfig) *CronJob {
 }
 
 // Start starts the cronjob.
-// Implements controller-runtime manager.Runnable
+// Implements controller-runtime manager.Runnable.
 func (j *CronJob) Start(ctx context.Context) error {
 	select {
 	case <-j.cfg.ReadyCh:

internal/framework/status/updater.go

@@ -121,8 +121,6 @@ func (u *Updater) writeStatuses(
 //
 // Note: this function is public because fake dependencies require us to test this function from the test package
 // to avoid import cycles.
-//
-//nolint:nilerr
 func NewRetryUpdateFunc(
 	getter controller.Getter,
 	updater K8sUpdater,

internal/mode/static/handler.go

@@ -8,7 +8,6 @@ import (
 
 	"github.com/go-logr/logr"
 	ngxclient "github.com/nginxinc/nginx-plus-go-client/client"
-	apiv1 "k8s.io/api/core/v1"
 	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
@@ -20,7 +19,7 @@ import (
 	"github.com/nginxinc/nginx-gateway-fabric/internal/framework/events"
 	"github.com/nginxinc/nginx-gateway-fabric/internal/framework/helpers"
 	frameworkStatus "github.com/nginxinc/nginx-gateway-fabric/internal/framework/status"
-	"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/config"
+
 	ngfConfig "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/config"
 	ngxConfig "github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/config"
 	"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/nginx/file"
@@ -75,7 +74,7 @@ type eventHandlerConfig struct {
 	// eventRecorder records events for Kubernetes resources.
 	eventRecorder record.EventRecorder
 	// usageReportConfig contains the configuration for NGINX Plus usage reporting.
-	usageReportConfig *config.UsageReportConfig
+	usageReportConfig *ngfConfig.UsageReportConfig
 	// nginxConfiguredOnStartChecker sets the health of the Pod to Ready once we've written out our initial config.
 	nginxConfiguredOnStartChecker *nginxConfiguredOnStartChecker
 	// gatewayPodConfig contains information about this Pod.
@@ -89,7 +88,7 @@ type eventHandlerConfig struct {
 }
 
 const (
-	// groups for GroupStatusUpdater
+	// groups for GroupStatusUpdater.
 	groupAllExceptGateways = "all-graphs-except-gateways"
 	groupGateways          = "gateways"
 	groupControlPlane      = "control-plane"
@@ -308,7 +307,7 @@ func (h *eventHandlerImpl) parseAndCaptureEvent(ctx context.Context, logger logr
 	}
 }
 
-// updateNginxConf updates nginx conf files and reloads nginx
+// updateNginxConf updates nginx conf files and reloads nginx.
 func (h *eventHandlerImpl) updateNginxConf(ctx context.Context, conf dataplane.Configuration) error {
 	files := h.cfg.generator.Generate(conf)
 	if err := h.cfg.nginxFileMgr.ReplaceFiles(files); err != nil {
@@ -324,7 +323,7 @@ func (h *eventHandlerImpl) updateNginxConf(ctx context.Context, conf dataplane.C
 
 // updateUpstreamServers is called only when endpoints have changed. It updates nginx conf files and then:
 // - if using NGINX Plus, determines which servers have changed and uses the N+ API to update them;
-// - otherwise if not using NGINX Plus, or an error was returned from the API, reloads nginx
+// - otherwise if not using NGINX Plus, or an error was returned from the API, reloads nginx.
 func (h *eventHandlerImpl) updateUpstreamServers(
 	ctx context.Context,
 	logger logr.Logger,
@@ -410,7 +409,7 @@ func serversEqual(newServers []ngxclient.UpstreamServer, oldServers []ngxclient.
 }
 
 // updateControlPlaneAndSetStatus updates the control plane configuration and then sets the status
-// based on the outcome
+// based on the outcome.
 func (h *eventHandlerImpl) updateControlPlaneAndSetStatus(
 	ctx context.Context,
 	logger logr.Logger,
@@ -429,7 +428,7 @@ func (h *eventHandlerImpl) updateControlPlaneAndSetStatus(
 		logger.Error(err, msg)
 		h.cfg.eventRecorder.Eventf(
 			cfg,
-			apiv1.EventTypeWarning,
+			v1.EventTypeWarning,
 			"UpdateFailed",
 			msg+": %s",
 			err.Error(),
@@ -454,7 +453,7 @@ func getGatewayAddresses(
 	ctx context.Context,
 	k8sClient client.Client,
 	svc *v1.Service,
-	podConfig config.GatewayPodConfig,
+	podConfig ngfConfig.GatewayPodConfig,
 ) ([]gatewayv1.GatewayStatusAddress, error) {
 	podAddress := []gatewayv1.GatewayStatusAddress{
 		{

internal/mode/static/manager.go

@@ -63,7 +63,7 @@ import (
 )
 
 const (
-	// clusterTimeout is a timeout for connections to the Kubernetes API
+	// clusterTimeout is a timeout for connections to the Kubernetes API.
 	clusterTimeout = 10 * time.Second
 )
 
@@ -80,7 +80,7 @@ func init() {
 	utilruntime.Must(appsv1.AddToScheme(scheme))
 }
 
-// nolint:gocyclo
+//nolint:gocyclo
 func StartManager(cfg config.Config) error {
 	nginxChecker := newNginxConfiguredOnStartChecker()
 	mgr, err := createManager(cfg, nginxChecker)
@@ -530,7 +530,7 @@ func registerControllers(
 }
 
 // 10 min jitter is enough per telemetry destination recommendation
-// For the default period of 24 hours, jitter will be 10min /(24*60)min  = 0.0069
+// For the default period of 24 hours, jitter will be 10min /(24*60)min  = 0.0069.
 const telemetryJitterFactor = 10.0 / (24 * 60) // added jitter is bound by jitterFactor * period
 
 func createTelemetryJob(
@@ -566,7 +566,6 @@ func createTelemetryJob(
 		if err != nil {
 			return nil, fmt.Errorf("cannot create telemetry exporter: %w", err)
 		}
-
 	} else {
 		exporter = telemetry.NewLoggingExporter(cfg.Logger.WithName("telemetryExporter").V(1 /* debug */))
 	}

internal/mode/static/metrics/collectors/controller.go

@@ -15,7 +15,7 @@ type ControllerCollector struct {
 	eventBatchProcessDuration prometheus.Histogram
 }
 
-// NewControllerCollector creates a new ControllerCollector
+// NewControllerCollector creates a new ControllerCollector.
 func NewControllerCollector(constLabels map[string]string) *ControllerCollector {
 	nc := &ControllerCollector{
 		eventBatchProcessDuration: prometheus.NewHistogram(

internal/mode/static/metrics/collectors/nginx.go

@@ -16,15 +16,15 @@ const (
 	nginxStatusURI  = "http://config-status/stub_status"
 )
 
-// NewNginxMetricsCollector creates an NginxCollector which fetches stats from NGINX over a unix socket
+// NewNginxMetricsCollector creates an NginxCollector which fetches stats from NGINX over a unix socket.
 func NewNginxMetricsCollector(constLabels map[string]string, logger log.Logger) prometheus.Collector {
 	httpClient := runtime.GetSocketClient(nginxStatusSock)
 	ngxClient := prometheusClient.NewNginxClient(&httpClient, nginxStatusURI)
 
 	return nginxCollector.NewNginxCollector(ngxClient, metrics.Namespace, constLabels, logger)
 }
 
-// NewNginxPlusMetricsCollector creates an NginxCollector which fetches stats from NGINX Plus API over a unix socket
+// NewNginxPlusMetricsCollector creates an NginxCollector which fetches stats from NGINX Plus API over a unix socket.
 func NewNginxPlusMetricsCollector(
 	plusClient *client.NginxClient,
 	constLabels map[string]string,

internal/mode/static/metrics/collectors/nginx_runtime.go

@@ -8,7 +8,7 @@ import (
 	"github.com/nginxinc/nginx-gateway-fabric/internal/mode/static/metrics"
 )
 
-// NginxRuntimeCollector implements runtime.Collector interface and prometheus.Collector interface
+// NginxRuntimeCollector implements runtime.Collector interface and prometheus.Collector interface.
 type NginxRuntimeCollector struct {
 	// Metrics
 	reloadsTotal    prometheus.Counter
@@ -17,7 +17,7 @@ type NginxRuntimeCollector struct {
 	reloadsDuration prometheus.Histogram
 }
 
-// NewManagerMetricsCollector creates a new NginxRuntimeCollector
+// NewManagerMetricsCollector creates a new NginxRuntimeCollector.
 func NewManagerMetricsCollector(constLabels map[string]string) *NginxRuntimeCollector {
 	nc := &NginxRuntimeCollector{
 		reloadsTotal: prometheus.NewCounter(

internal/mode/static/metrics/metrics.go

@@ -1,4 +1,3 @@
 package metrics
 
-// nolint:gosec // flagged as potential hardcoded credentials, but is not sensitive
 const Namespace = "nginx_gateway_fabric"

internal/mode/static/nginx/config/base_http_config_test.go

@@ -42,7 +42,6 @@ func TestExecuteBaseHttp(t *testing.T) {
 	}
 
 	for _, test := range tests {
-
 		g := NewWithT(t)
 
 		res := executeBaseHTTPConfig(test.conf)

internal/mode/static/nginx/config/maps.go

@@ -48,7 +48,7 @@ func buildAddHeaderMaps(servers []dataplane.VirtualServer) []http.Map {
 
 const (
 	// In order to prepend any passed client header values to values specified in the add headers field of request
-	// header modifiers, we need to create a map parameter regex for any string value
+	// header modifiers, we need to create a map parameter regex for any string value.
 	anyStringFmt = `~.*`
 )