chore: stricter golangci-lint (#298)

**Commit Message**

This makes the lint settings stricter and
starts enforcing more rules.

Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com>

.golangci.yml

@@ -71,13 +71,7 @@ linters-settings:
   govet:
     enable-all: true
     disable:
-      - shadow
       - fieldalignment
-  revive:
-    rules:
-      # TODO: enable if-return check
-      - name: if-return
-        disabled: true
   testifylint:
     disable:
       - float-compare

internal/apischema/awsbedrock/awsbedrock.go

@@ -337,7 +337,9 @@ type ConverseMetrics struct {
 	LatencyMs *int64 `json:"latencyMs"`
 }
 
-type ConverseOutput struct {
+// ConverseResponse is the response from a call to Converse.
+// https://docs.aws.amazon.com/bedrock/latest/APIReference/API_runtime_Converse.html
+type ConverseResponse struct {
 	// Metrics for the call to Converse.
 	//
 	// Metrics is a required field
@@ -346,7 +348,7 @@ type ConverseOutput struct {
 	// The result from the call to Converse.
 	//
 	// Output is a required field
-	Output *ConverseOutput_ `json:"output"`
+	Output *ConverseOutput `json:"output"`
 
 	// The reason why the model stopped generating output.
 	//
@@ -362,9 +364,9 @@ type ConverseOutput struct {
 	Usage *TokenUsage `json:"usage"`
 }
 
-// ConverseOutput_ ConverseResponseOutput is defined in the AWS Bedrock API:
+// ConverseOutput is defined in the AWS Bedrock API:
 // https://docs.aws.amazon.com/bedrock/latest/APIReference/API_runtime_ConverseOutput.html
-type ConverseOutput_ struct {
+type ConverseOutput struct {
 	Message Message `json:"message,omitempty"`
 }
 

internal/controller/ai_gateway_route.go

@@ -153,7 +153,7 @@ func (c *aiGatewayRouteController) ensuresExtProcConfigMapExists(ctx context.Con
 			},
 			Data: map[string]string{expProcConfigFileName: filterapi.DefaultConfig},
 		}
-		if err := ctrlutil.SetControllerReference(aiGatewayRoute, configMap, c.client.Scheme()); err != nil {
+		if err = ctrlutil.SetControllerReference(aiGatewayRoute, configMap, c.client.Scheme()); err != nil {
 			panic(fmt.Errorf("BUG: failed to set controller reference for extproc configmap: %w", err))
 		}
 		_, err = c.kube.CoreV1().ConfigMaps(aiGatewayRoute.Namespace).Create(ctx, configMap, metav1.CreateOptions{})

internal/controller/ai_gateway_route_test.go

@@ -157,7 +157,7 @@ func Test_applyExtProcDeploymentConfigUpdate(t *testing.T) {
 			},
 		},
 	}
-	t.Run("not panic", func(t *testing.T) {
+	t.Run("not panic", func(_ *testing.T) {
 		applyExtProcDeploymentConfigUpdate(dep, nil)
 		applyExtProcDeploymentConfigUpdate(dep, &aigv1a1.AIGatewayFilterConfig{})
 		applyExtProcDeploymentConfigUpdate(dep, &aigv1a1.AIGatewayFilterConfig{

internal/controller/ai_service_backend_test.go

@@ -6,7 +6,6 @@ import (
 
 	"github.com/stretchr/testify/require"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
 	fake2 "k8s.io/client-go/kubernetes/fake"
 	"k8s.io/utils/ptr"
@@ -42,9 +41,6 @@ func TestAIServiceBackendController_Reconcile(t *testing.T) {
 }
 
 func Test_AiServiceBackendIndexFunc(t *testing.T) {
-	scheme := runtime.NewScheme()
-	require.NoError(t, aigv1a1.AddToScheme(scheme))
-
 	c := fake.NewClientBuilder().
 		WithScheme(scheme).
 		WithIndex(&aigv1a1.AIServiceBackend{}, k8sClientIndexBackendSecurityPolicyToReferencingAIServiceBackend, aiServiceBackendIndexFunc).

internal/controller/controller_test.go

@@ -7,7 +7,6 @@ import (
 	"github.com/stretchr/testify/require"
 	"go.uber.org/goleak"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/utils/ptr"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/client/fake"
@@ -22,9 +21,6 @@ func TestMain(m *testing.M) {
 }
 
 func Test_aiGatewayRouteIndexFunc(t *testing.T) {
-	scheme := runtime.NewScheme()
-	require.NoError(t, aigv1a1.AddToScheme(scheme))
-
 	c := fake.NewClientBuilder().
 		WithScheme(scheme).
 		WithIndex(&aigv1a1.AIGatewayRoute{}, k8sClientIndexBackendToReferencingAIGatewayRoute, aiGatewayRouteIndexFunc).
@@ -137,9 +133,6 @@ func Test_backendSecurityPolicyIndexFunc(t *testing.T) {
 		},
 	} {
 		t.Run(bsp.name, func(t *testing.T) {
-			scheme := runtime.NewScheme()
-			require.NoError(t, aigv1a1.AddToScheme(scheme))
-
 			c := fake.NewClientBuilder().
 				WithScheme(scheme).
 				WithIndex(&aigv1a1.BackendSecurityPolicy{}, k8sClientIndexSecretToReferencingBackendSecurityPolicy, backendSecurityPolicyIndexFunc).

internal/controller/sink.go

@@ -149,7 +149,7 @@ func (c *configSink) syncAIGatewayRoute(ctx context.Context, aiGatewayRoute *aig
 				},
 			},
 		}
-		if err := c.client.Create(ctx, &httpRouteFilter); err != nil {
+		if err = c.client.Create(ctx, &httpRouteFilter); err != nil {
 			c.logger.Error(err, "failed to create HTTPRouteFilter", "namespace", aiGatewayRoute.Namespace, "name", hostRewriteHTTPFilterName)
 			return
 		}
@@ -181,28 +181,28 @@ func (c *configSink) syncAIGatewayRoute(ctx context.Context, aiGatewayRoute *aig
 	}
 
 	// Update the HTTPRoute with the new AIGatewayRoute.
-	if err := c.newHTTPRoute(ctx, &httpRoute, aiGatewayRoute); err != nil {
+	if err = c.newHTTPRoute(ctx, &httpRoute, aiGatewayRoute); err != nil {
 		c.logger.Error(err, "failed to update HTTPRoute with AIGatewayRoute", "namespace", aiGatewayRoute.Namespace, "name", aiGatewayRoute.Name)
 		return
 	}
 
 	if existingRoute {
 		c.logger.Info("updating HTTPRoute", "namespace", httpRoute.Namespace, "name", httpRoute.Name)
-		if err := c.client.Update(ctx, &httpRoute); err != nil {
+		if err = c.client.Update(ctx, &httpRoute); err != nil {
 			c.logger.Error(err, "failed to update HTTPRoute", "namespace", httpRoute.Namespace, "name", httpRoute.Name)
 			return
 		}
 	} else {
 		c.logger.Info("creating HTTPRoute", "namespace", httpRoute.Namespace, "name", httpRoute.Name)
-		if err := c.client.Create(ctx, &httpRoute); err != nil {
+		if err = c.client.Create(ctx, &httpRoute); err != nil {
 			c.logger.Error(err, "failed to create HTTPRoute", "namespace", httpRoute.Namespace, "name", httpRoute.Name)
 			return
 		}
 	}
 
 	// Update the extproc configmap.
 	uuid := string(uuid2.NewUUID())
-	if err := c.updateExtProcConfigMap(ctx, aiGatewayRoute, uuid); err != nil {
+	if err = c.updateExtProcConfigMap(ctx, aiGatewayRoute, uuid); err != nil {
 		c.logger.Error(err, "failed to update extproc configmap", "namespace", aiGatewayRoute.Namespace, "name", aiGatewayRoute.Name)
 		return
 	}
@@ -277,19 +277,20 @@ func (c *configSink) updateExtProcConfigMap(ctx context.Context, aiGatewayRoute
 			key := fmt.Sprintf("%s.%s", backend.Name, aiGatewayRoute.Namespace)
 			ec.Rules[i].Backends[j].Name = key
 			ec.Rules[i].Backends[j].Weight = backend.Weight
-			backendObj, err := c.backend(ctx, aiGatewayRoute.Namespace, backend.Name)
+			var backendObj *aigv1a1.AIServiceBackend
+			backendObj, err = c.backend(ctx, aiGatewayRoute.Namespace, backend.Name)
 			if err != nil {
 				return fmt.Errorf("failed to get AIServiceBackend %s: %w", key, err)
-			} else {
-				ec.Rules[i].Backends[j].Schema.Name = filterapi.APISchemaName(backendObj.Spec.APISchema.Name)
-				ec.Rules[i].Backends[j].Schema.Version = backendObj.Spec.APISchema.Version
 			}
+			ec.Rules[i].Backends[j].Schema.Name = filterapi.APISchemaName(backendObj.Spec.APISchema.Name)
+			ec.Rules[i].Backends[j].Schema.Version = backendObj.Spec.APISchema.Version
 
 			if bspRef := backendObj.Spec.BackendSecurityPolicyRef; bspRef != nil {
 				volumeName := backendSecurityPolicyVolumeName(
 					i, j, string(backendObj.Spec.BackendSecurityPolicyRef.Name),
 				)
-				backendSecurityPolicy, err := c.backendSecurityPolicy(ctx, aiGatewayRoute.Namespace, string(bspRef.Name))
+				var backendSecurityPolicy *aigv1a1.BackendSecurityPolicy
+				backendSecurityPolicy, err = c.backendSecurityPolicy(ctx, aiGatewayRoute.Namespace, string(bspRef.Name))
 				if err != nil {
 					return fmt.Errorf("failed to get BackendSecurityPolicy %s: %w", bspRef.Name, err)
 				}
@@ -338,7 +339,7 @@ func (c *configSink) updateExtProcConfigMap(ctx context.Context, aiGatewayRoute
 			fc.Type = filterapi.LLMRequestCostTypeCELExpression
 			expr := *cost.CELExpression
 			// Sanity check the CEL expression.
-			_, err := llmcostcel.NewProgram(expr)
+			_, err = llmcostcel.NewProgram(expr)
 			if err != nil {
 				return fmt.Errorf("invalid CEL expression: %w", err)
 			}
@@ -513,10 +514,11 @@ func (c *configSink) syncExtProcDeployment(ctx context.Context, aiGatewayRoute *
 					},
 				},
 			}
-			if err := ctrlutil.SetControllerReference(aiGatewayRoute, deployment, c.client.Scheme()); err != nil {
+			if err = ctrlutil.SetControllerReference(aiGatewayRoute, deployment, c.client.Scheme()); err != nil {
 				panic(fmt.Errorf("BUG: failed to set controller reference for deployment: %w", err))
 			}
-			updatedSpec, err := c.mountBackendSecurityPolicySecrets(ctx, &deployment.Spec.Template.Spec, aiGatewayRoute)
+			var updatedSpec *corev1.PodSpec
+			updatedSpec, err = c.mountBackendSecurityPolicySecrets(ctx, &deployment.Spec.Template.Spec, aiGatewayRoute)
 			if err == nil {
 				deployment.Spec.Template.Spec = *updatedSpec
 			}
@@ -530,7 +532,8 @@ func (c *configSink) syncExtProcDeployment(ctx context.Context, aiGatewayRoute *
 			return fmt.Errorf("failed to get deployment: %w", err)
 		}
 	} else {
-		updatedSpec, err := c.mountBackendSecurityPolicySecrets(ctx, &deployment.Spec.Template.Spec, aiGatewayRoute)
+		var updatedSpec *corev1.PodSpec
+		updatedSpec, err = c.mountBackendSecurityPolicySecrets(ctx, &deployment.Spec.Template.Spec, aiGatewayRoute)
 		if err == nil {
 			deployment.Spec.Template.Spec = *updatedSpec
 		}

internal/controller/sink_test.go

@@ -30,7 +30,7 @@ import (
 
 func requireNewFakeClientWithIndexes(t *testing.T) client.Client {
 	builder := fake.NewClientBuilder().WithScheme(scheme)
-	err := applyIndexing(context.Background(), func(ctx context.Context, obj client.Object, field string, extractValue client.IndexerFunc) error {
+	err := applyIndexing(context.Background(), func(_ context.Context, obj client.Object, field string, extractValue client.IndexerFunc) error {
 		builder = builder.WithIndex(obj, field, extractValue)
 		return nil
 	})
@@ -501,8 +501,7 @@ func TestConfigSink_SyncExtprocDeployment(t *testing.T) {
 			},
 		},
 	} {
-		err := fakeClient.Create(context.Background(), bsp, &client.CreateOptions{})
-		require.NoError(t, err)
+		require.NoError(t, fakeClient.Create(context.Background(), bsp, &client.CreateOptions{}))
 	}
 
 	for _, b := range []*aigv1a1.AIServiceBackend{
@@ -530,8 +529,7 @@ func TestConfigSink_SyncExtprocDeployment(t *testing.T) {
 			},
 		},
 	} {
-		err := fakeClient.Create(context.Background(), b, &client.CreateOptions{})
-		require.NoError(t, err)
+		require.NoError(t, fakeClient.Create(context.Background(), b, &client.CreateOptions{}))
 	}
 	require.NotNil(t, s)
 

internal/extproc/mocks_test.go

@@ -97,7 +97,7 @@ func (m mockTranslator) ResponseError(_ map[string]string, body io.Reader) (head
 }
 
 // ResponseBody implements [translator.Translator.ResponseBody].
-func (m mockTranslator) ResponseBody(respHeader map[string]string, body io.Reader, _ bool) (headerMutation *extprocv3.HeaderMutation, bodyMutation *extprocv3.BodyMutation, tokenUsage translator.LLMTokenUsage, err error) {
+func (m mockTranslator) ResponseBody(_ map[string]string, body io.Reader, _ bool) (headerMutation *extprocv3.HeaderMutation, bodyMutation *extprocv3.BodyMutation, tokenUsage translator.LLMTokenUsage, err error) {
 	if m.expResponseBody != nil {
 		buf, err := io.ReadAll(body)
 		require.NoError(m.t, err)
@@ -179,7 +179,7 @@ func (m mockExternalProcessingStream) Recv() (*extprocv3.ProcessingRequest, erro
 }
 
 // SetHeader implements [extprocv3.ExternalProcessor_ProcessServer].
-func (m mockExternalProcessingStream) SetHeader(md metadata.MD) error { panic("TODO") }
+func (m mockExternalProcessingStream) SetHeader(_ metadata.MD) error { panic("TODO") }
 
 // SendHeader implements [extprocv3.ExternalProcessor_ProcessServer].
 func (m mockExternalProcessingStream) SendHeader(metadata.MD) error { panic("TODO") }

internal/extproc/router/request_body.go

@@ -32,7 +32,6 @@ func openAIParseBody(path string, body *extprocv3.HttpBody) (modelName string, r
 			return "", nil, fmt.Errorf("failed to unmarshal body: %w", err)
 		}
 		return openAIReq.Model, &openAIReq, nil
-	} else {
-		return "", nil, fmt.Errorf("unsupported path: %s", path)
 	}
+	return "", nil, fmt.Errorf("unsupported path: %s", path)
 }

internal/extproc/router/router_test.go

@@ -18,7 +18,7 @@ func (c *dummyCustomRouter) Calculate(map[string]string) (*filterapi.Backend, er
 }
 
 func TestRouter_NewRouter_Custom(t *testing.T) {
-	r, err := NewRouter(&filterapi.Config{}, func(defaultRouter x.Router, config *filterapi.Config) x.Router {
+	r, err := NewRouter(&filterapi.Config{}, func(defaultRouter x.Router, _ *filterapi.Config) x.Router {
 		require.NotNil(t, defaultRouter)
 		_, ok := defaultRouter.(*router)
 		require.True(t, ok) // Checking if the default router is correctly passed.

internal/extproc/server.go

@@ -66,11 +66,10 @@ func (s *Server[P]) LoadConfig(ctx context.Context, config *filterapi.Config) er
 			}
 
 			if b.Auth != nil {
-				h, err := backendauth.NewHandler(ctx, b.Auth)
+				backendAuthHandlers[b.Name], err = backendauth.NewHandler(ctx, b.Auth)
 				if err != nil {
 					return fmt.Errorf("cannot create backend auth handler: %w", err)
 				}
-				backendAuthHandlers[b.Name] = h
 			}
 		}
 	}