build: add more linters to golangci-lint configuration (#3274)

.golangci.yaml

@@ -11,8 +11,10 @@ run:
   - istio_tests
 linters:
   enable:
+  - asasalint
   - asciicheck
   - bodyclose
+  - contextcheck
   - depguard
   - dogsled
   - durationcheck
@@ -31,6 +33,7 @@ linters:
   - govet
   - importas
   - ineffassign
+  - loggercheck
   - megacheck
   - misspell
   - nakedret
@@ -39,6 +42,7 @@ linters:
   - predeclared
   - revive
   - staticcheck
+  - tenv
   - typecheck
   - unconvert
   - unparam
@@ -89,6 +93,13 @@ linters-settings:
     include-go-root: false
     packages-with-error-message:
       - k8s.io/utils/pointer: "Use github.com/samber/lo ToPtr instead"
+  tenv:
+    all: true
+  loggercheck:
+    kitlog: false
+    klog: true
+    logr: true
+    zap: false
 issues:
   fix: true
   max-same-issues: 0

internal/admission/handler.go

@@ -132,8 +132,8 @@ func (h RequestHandler) handleKongConsumer(
 	if err != nil {
 		return nil, err
 	}
-	//nolint:exhaustive
-	switch request.Operation {
+
+	switch request.Operation { //nolint:exhaustive
 	case admissionv1.Create:
 		ok, msg, err := h.Validator.ValidateConsumer(ctx, consumer)
 		if err != nil {
@@ -217,8 +217,8 @@ func (h RequestHandler) handleSecret(
 	// managed consumer in order for us to validate them, and because credentials
 	// validation also happens at the consumer side of the reference so a
 	// credentials secret can not be referenced without being validated.
-	//nolint:exhaustive
-	switch request.Operation {
+
+	switch request.Operation { //nolint:exhaustive
 	case admissionv1.Update:
 		ok, message, err := h.Validator.ValidateCredential(ctx, secret)
 		if err != nil {

internal/cmd/rootcmd/env_test.go

@@ -3,7 +3,6 @@ package rootcmd
 import (
 	"fmt"
 	"io"
-	"os"
 	"strings"
 	"testing"
 
@@ -34,12 +33,8 @@ func TestBindEnvVars(t *testing.T) {
 	cmd.Flags().String("flag-3", "default3", "Set by args only")
 	cmd.Flags().String("flag-4", "default4", "Set by both env and args")
 
-	_ = os.Setenv("CONTROLLER_FLAG_2", "env2")
-	_ = os.Setenv("CONTROLLER_FLAG_4", "env4")
-	defer func() {
-		_ = os.Unsetenv("CONTROLLER_FLAG_2")
-		_ = os.Unsetenv("CONTROLLER_FLAG_4")
-	}()
+	t.Setenv("CONTROLLER_FLAG_2", "env2")
+	t.Setenv("CONTROLLER_FLAG_4", "env4")
 
 	cmd.SetArgs([]string{
 		"--flag-3=args3",
@@ -63,8 +58,7 @@ func TestBindEnvVarsValidation(t *testing.T) {
 	cmd.SetOut(io.Discard)
 	cmd.SetErr(io.Discard)
 
-	_ = os.Setenv("CONTROLLER_VALIDATION_TEST", "intentionally_fail")
-	defer os.Unsetenv("CONTROLLER_VALIDATION_TEST")
+	t.Setenv("CONTROLLER_VALIDATION_TEST", "intentionally_fail")
 
 	err := cmd.Execute()
 	assert.Error(t, err)

internal/controllers/gateway/gateway_controller.go

@@ -145,7 +145,11 @@ func (r *GatewayReconciler) SetupWithManager(mgr ctrl.Manager) error {
 func (r *GatewayReconciler) gatewayHasMatchingGatewayClass(obj client.Object) bool {
 	gateway, ok := obj.(*gatewayv1beta1.Gateway)
 	if !ok {
-		r.Log.Error(fmt.Errorf("unexpected object type in gateway watch predicates"), "expected", "*gatewayv1beta1.Gateway", "found", reflect.TypeOf(obj))
+		r.Log.Error(
+			fmt.Errorf("unexpected object type"),
+			"gateway watch predicate received unexpected object type",
+			"expected", "*gatewayv1beta1.Gateway", "found", reflect.TypeOf(obj),
+		)
 		return false
 	}
 	gatewayClass := &gatewayv1beta1.GatewayClass{}
@@ -161,7 +165,11 @@ func (r *GatewayReconciler) gatewayHasMatchingGatewayClass(obj client.Object) bo
 func (r *GatewayReconciler) gatewayClassMatchesController(obj client.Object) bool {
 	gatewayClass, ok := obj.(*gatewayv1beta1.GatewayClass)
 	if !ok {
-		r.Log.Error(fmt.Errorf("unexpected object type in gatewayclass watch predicates"), "expected", "*gatewayv1beta1.GatewayClass", "found", reflect.TypeOf(obj))
+		r.Log.Error(
+			fmt.Errorf("unexpected object type"),
+			"gatewayclass watch predicate received unexpected object type",
+			"expected", "*gatewayv1beta1.GatewayClass", "found", reflect.TypeOf(obj),
+		)
 		return false
 	}
 	return isGatewayClassControlledAndUnmanaged(gatewayClass)
@@ -184,8 +192,11 @@ func (r *GatewayReconciler) listGatewaysForGatewayClass(gatewayClass client.Obje
 func (r *GatewayReconciler) listReferenceGrantsForGateway(obj client.Object) []reconcile.Request {
 	grant, ok := obj.(*gatewayv1alpha2.ReferenceGrant)
 	if !ok {
-		r.Log.Error(fmt.Errorf("unexpected object type in referencegrant watch predicates"), "expected",
-			"*gatewayv1alpha2.ReferenceGrant", "found", reflect.TypeOf(obj))
+		r.Log.Error(
+			fmt.Errorf("unexpected object type"),
+			"referencegrant watch predicate received unexpected object type",
+			"expected", "*gatewayv1alpha2.ReferenceGrant", "found", reflect.TypeOf(obj),
+		)
 		return nil
 	}
 	gateways := &gatewayv1beta1.GatewayList{}
@@ -218,7 +229,7 @@ func (r *GatewayReconciler) listReferenceGrantsForGateway(obj client.Object) []r
 func (r *GatewayReconciler) listGatewaysForService(svc client.Object) (recs []reconcile.Request) {
 	gateways := &gatewayv1beta1.GatewayList{}
 	if err := r.Client.List(context.Background(), gateways); err != nil {
-		r.Log.Error(err, "failed to list gateways for service in watch predicates", "service")
+		r.Log.Error(err, "failed to list gateways for service in watch predicates", "service", svc)
 		return
 	}
 	for _, gateway := range gateways.Items {

internal/controllers/gateway/gatewayclass_controller.go

@@ -71,7 +71,11 @@ func (r *GatewayClassReconciler) SetupWithManager(mgr ctrl.Manager) error {
 func (r *GatewayClassReconciler) GatewayClassIsUnmanaged(obj client.Object) bool {
 	gatewayClass, ok := obj.(*gatewayv1beta1.GatewayClass)
 	if !ok {
-		r.Log.Error(fmt.Errorf("unexpected object type in gateway watch predicates"), "expected", "*gatewayv1beta1.GatewayClass", "found", reflect.TypeOf(obj))
+		r.Log.Error(
+			fmt.Errorf("unexpected object type"),
+			"gatewayclass watch predicate received unexpected object type",
+			"expected", "*gatewayv1beta1.GatewayClass", "found", reflect.TypeOf(obj),
+		)
 		return false
 	}
 

internal/controllers/gateway/route_utils.go

@@ -372,7 +372,6 @@ func routeMatchesListenerAllowedRoutes[T types.RouteT](
 	}
 
 	switch *listener.AllowedRoutes.Namespaces.From {
-
 	case gatewayv1beta1.NamespacesFromAll:
 		return true, nil
 

internal/controllers/knative/knative.go

@@ -202,7 +202,7 @@ func (r *Knativev1alpha1IngressReconciler) Reconcile(ctx context.Context, req ct
 	if r.DataplaneClient.AreKubernetesObjectReportsEnabled() {
 		log.V(util.DebugLevel).Info("determining whether data-plane configuration has succeeded", "namespace", req.Namespace, "name", req.Name)
 		if !r.DataplaneClient.KubernetesObjectIsConfigured(obj) {
-			log.V(util.DebugLevel).Error(fmt.Errorf("resource not yet configured in the data-plane"), "namespace", req.Namespace, "name", req.Name)
+			log.V(util.DebugLevel).Error(fmt.Errorf("resource not yet configured"), "resource not yet configured in the data-plane", "namespace", req.Namespace, "name", req.Name)
 			return ctrl.Result{Requeue: true}, nil // requeue until the object has been properly configured
 		}
 

internal/controllers/reference/reference.go

@@ -26,7 +26,6 @@ func UpdateReferencesToSecret(
 	referrer client.Object, referencedSecretNameMap map[types.NamespacedName]struct{},
 ) error {
 	for nsName := range referencedSecretNameMap {
-
 		secret := &corev1.Secret{
 			ObjectMeta: metav1.ObjectMeta{
 				Namespace: nsName.Namespace,
@@ -107,7 +106,6 @@ func removeOutdatedReferencesToSecret(
 			if err := indexers.DeleteObjectIfNotReferred(obj, dataplaneClient); err != nil {
 				return err
 			}
-
 		}
 	}
 	return nil

internal/dataplane/kong_client.go

@@ -124,6 +124,7 @@ type KongClient struct {
 // NewKongClient provides a new KongClient object after connecting to the
 // data-plane API and verifying integrity.
 func NewKongClient(
+	ctx context.Context,
 	logger logrus.FieldLogger,
 	timeout time.Duration,
 	ingressClass string,
@@ -149,7 +150,7 @@ func NewKongClient(
 	}
 
 	// download the kong root configuration (and validate connectivity to the proxy API)
-	root, err := c.RootWithTimeout()
+	root, err := c.RootWithTimeout(ctx)
 	if err != nil {
 		return nil, err
 	}
@@ -227,8 +228,8 @@ func (c *KongClient) Listeners(ctx context.Context) ([]kong.ProxyListener, []kon
 
 // RootWithTimeout provides the root configuration from Kong, but uses a configurable timeout to avoid long waits if the Admin API
 // is not yet ready to respond. If a timeout error occurs, the caller is responsible for providing a retry mechanism.
-func (c *KongClient) RootWithTimeout() (map[string]interface{}, error) {
-	ctx, cancel := context.WithTimeout(context.Background(), c.requestTimeout)
+func (c *KongClient) RootWithTimeout(ctx context.Context) (map[string]interface{}, error) {
+	ctx, cancel := context.WithTimeout(ctx, c.requestTimeout)
 	defer cancel()
 	return c.kongConfig.Client.Root(ctx)
 }

internal/dataplane/kongstate/route.go

@@ -61,7 +61,6 @@ func (r *Route) useSSLProtocol() {
 	var prots []*string
 
 	for _, val := range r.Protocols {
-
 		if strings.Contains(*val, "grpc") {
 			grpc = true
 		}

internal/dataplane/parser/parser.go

@@ -682,7 +682,6 @@ func getEndpoints(
 
 	for _, ss := range ep.Subsets {
 		for _, epPort := range ss.Ports {
-
 			if !reflect.DeepEqual(epPort.Protocol, proto) {
 				continue
 			}

internal/dataplane/parser/translate_httproute.go

@@ -108,7 +108,6 @@ func (p *Parser) ingressRulesFromHTTPRouteLegacyFallback(httproute *gatewayv1bet
 	// each rule may represent a different set of backend services that will be accepting
 	// traffic, so we make separate routes and Kong services for every present rule.
 	for ruleNumber, rule := range httproute.Spec.Rules {
-
 		// determine the routes needed to route traffic to services for this rule
 		routes, err := generateKongRoutesFromHTTPRouteRule(httproute, ruleNumber, rule, p.flagEnabledRegexPathPrefix)
 		if err != nil {

internal/dataplane/parser/translate_knative.go

@@ -90,7 +90,6 @@ func (p *Parser) ingressRulesFromKnativeIngress() ingressRules {
 					knativeBackend.ServicePort.String())
 				service, ok := services[serviceName]
 				if !ok {
-
 					var headers []string
 					for key, value := range knativeBackend.AppendHeaders {
 						headers = append(headers, key+":"+value)

internal/dataplane/parser/wrappers_backendref.go

@@ -104,7 +104,6 @@ func backendRefsToKongStateBackends[T types.BackendRefT](
 			brw.Group(),
 			brw.Kind(),
 		) && newRefChecker(backendRef).IsRefAllowedByGrant(allowed) {
-
 			backend := kongstate.ServiceBackend{
 				Name: brw.Name(),
 				PortDef: kongstate.PortDef{

internal/diagnostics/server.go

@@ -69,7 +69,7 @@ func (s *Server) Listen(ctx context.Context, port int) error {
 	select {
 	case <-ctx.Done():
 		s.Logger.Info("shutting down diagnostics server")
-		return httpServer.Shutdown(context.Background())
+		return httpServer.Shutdown(context.Background()) //nolint:contextcheck
 	case err := <-errChan:
 		return err
 	}

internal/manager/run.go

@@ -144,6 +144,7 @@ func Run(ctx context.Context, c *Config, diagnostic util.ConfigDumpDiagnostic, d
 
 	eventRecorder := mgr.GetEventRecorderFor(KongClientEventRecorderComponentName)
 	dataplaneClient, err := dataplane.NewKongClient(
+		ctx,
 		deprecatedLogger,
 		timeoutDuration,
 		c.IngressClassName,

internal/manager/utils/reports.go

@@ -92,7 +92,7 @@ func RunReport(ctx context.Context, kubeCfg *rest.Config, kongCfg sendconfig.Kon
 		reporter.MeshDetector = detector
 	}
 
-	go reporter.Run(ctx.Done())
+	go reporter.Run(ctx)
 
 	return nil
 }

internal/meshdetect/detector.go

@@ -152,7 +152,6 @@ func (d *Detector) DetectRunUnder(ctx context.Context) map[MeshKind]*RunUnderRes
 
 		// detect if pod has a init container.
 		runUnderResults[meshKind].InitContainerInjected = isPodInitContainerInjected(meshKind, pod)
-
 	}
 
 	return runUnderResults
@@ -350,7 +349,6 @@ func (d *Detector) DetectServiceDistribution(ctx context.Context) (*ServiceDistr
 	}
 
 	for _, svc := range serviceList {
-
 		key := client.ObjectKeyFromObject(svc)
 		endpointsResource := endpoints[key]
 		if endpointsResource == nil {

internal/meshdetect/detector_test.go

@@ -317,7 +317,6 @@ func TestDetectRunUnder(t *testing.T) {
 					"test case %s: detection result should be same for mesh %s", tc.caseName, meshKind)
 			}
 		})
-
 	}
 }
 

internal/util/k8s_test.go

@@ -18,7 +18,6 @@ package util
 
 import (
 	"context"
-	"os"
 	"testing"
 
 	corev1 "k8s.io/api/core/v1"
@@ -176,32 +175,32 @@ func TestGetNodeIP(t *testing.T) {
 func TestGetPodDetails(t *testing.T) {
 	ctx := context.Background()
 	// POD_NAME & POD_NAMESPACE not exist
-	os.Setenv("POD_NAME", "")
-	os.Setenv("POD_NAMESPACE", "")
+	t.Setenv("POD_NAME", "")
+	t.Setenv("POD_NAMESPACE", "")
 	_, err1 := GetPodDetails(ctx, testclient.NewSimpleClientset())
 	if err1 == nil {
 		t.Errorf("expected an error but returned nil")
 	}
 
 	// POD_NAME not exist
-	os.Setenv("POD_NAME", "")
-	os.Setenv("POD_NAMESPACE", corev1.NamespaceDefault)
+	t.Setenv("POD_NAME", "")
+	t.Setenv("POD_NAMESPACE", corev1.NamespaceDefault)
 	_, err2 := GetPodDetails(ctx, testclient.NewSimpleClientset())
 	if err2 == nil {
 		t.Errorf("expected an error but returned nil")
 	}
 
 	// POD_NAMESPACE not exist
-	os.Setenv("POD_NAME", "testpod")
-	os.Setenv("POD_NAMESPACE", "")
+	t.Setenv("POD_NAME", "testpod")
+	t.Setenv("POD_NAMESPACE", "")
 	_, err3 := GetPodDetails(ctx, testclient.NewSimpleClientset())
 	if err3 == nil {
 		t.Errorf("expected an error but returned nil")
 	}
 
 	// POD not exist
-	os.Setenv("POD_NAME", "testpod")
-	os.Setenv("POD_NAMESPACE", corev1.NamespaceDefault)
+	t.Setenv("POD_NAME", "testpod")
+	t.Setenv("POD_NAMESPACE", corev1.NamespaceDefault)
 	_, err4 := GetPodDetails(ctx, testclient.NewSimpleClientset())
 	if err4 == nil {
 		t.Errorf("expected an error but returned nil")