Fix revive linter issues for all production code, and exclude revive linter issues for test code

.golangci.yaml

@@ -53,6 +53,10 @@ issues:
       linters:
         - funlen
         - gochecknoglobals
+        - revive
+    - path: internal/testutil/
+      linters:
+      - revive
 
 linters-settings:
   funlen:

cmd/pinniped/cmd/kubeconfig.go

@@ -1,4 +1,4 @@
-// Copyright 2020-2023 the Pinniped contributors. All Rights Reserved.
+// Copyright 2020-2024 the Pinniped contributors. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 
 package cmd
@@ -163,7 +163,7 @@ func kubeconfigCommand(deps kubeconfigDeps) *cobra.Command {
 
 	mustMarkDeprecated(cmd, "concierge-namespace", "not needed anymore")
 
-	cmd.RunE = func(cmd *cobra.Command, args []string) error {
+	cmd.RunE = func(cmd *cobra.Command, _args []string) error {
 		if flags.outputPath != "" {
 			out, err := os.Create(flags.outputPath)
 			if err != nil {

cmd/pinniped/cmd/login_oidc.go

@@ -147,7 +147,7 @@ func oidcLoginCommand(deps oidcLoginCommandDeps) *cobra.Command {
 	mustMarkHidden(cmd, "skip-listen")
 	mustMarkHidden(cmd, "debug-session-cache")
 	mustMarkRequired(cmd, "issuer")
-	cmd.RunE = func(cmd *cobra.Command, args []string) error { return runOIDCLogin(cmd, deps, flags) }
+	cmd.RunE = func(cmd *cobra.Command, _args []string) error { return runOIDCLogin(cmd, deps, flags) }
 
 	mustMarkDeprecated(cmd, "concierge-namespace", "not needed anymore")
 	mustMarkHidden(cmd, "concierge-namespace")

cmd/pinniped/cmd/login_static.go

@@ -86,7 +86,7 @@ func staticLoginCommand(deps staticLoginDeps) *cobra.Command {
 	cmd.Flags().StringVar(&flags.conciergeAPIGroupSuffix, "concierge-api-group-suffix", groupsuffix.PinnipedDefaultSuffix, "Concierge API group suffix")
 	cmd.Flags().StringVar(&flags.credentialCachePath, "credential-cache", filepath.Join(mustGetConfigDir(), "credentials.yaml"), "Path to cluster-specific credentials cache (\"\" disables the cache)")
 
-	cmd.RunE = func(cmd *cobra.Command, args []string) error { return runStaticLogin(cmd, deps, flags) }
+	cmd.RunE = func(cmd *cobra.Command, _args []string) error { return runStaticLogin(cmd, deps, flags) }
 
 	mustMarkDeprecated(cmd, "concierge-namespace", "not needed anymore")
 	mustMarkHidden(cmd, "concierge-namespace")

internal/concierge/apiserver/apiserver.go

@@ -1,4 +1,4 @@
-// Copyright 2020-2023 the Pinniped contributors. All Rights Reserved.
+// Copyright 2020-2024 the Pinniped contributors. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 
 package apiserver
@@ -113,7 +113,7 @@ func (c completedConfig) New() (*PinnipedServer, error) {
 	controllersCtx, cancelControllerCtx := context.WithCancel(context.Background())
 
 	s.GenericAPIServer.AddPostStartHookOrDie("start-controllers",
-		func(postStartContext genericapiserver.PostStartHookContext) error {
+		func(_ genericapiserver.PostStartHookContext) error {
 			plog.Debug("start-controllers post start hook starting")
 			defer plog.Debug("start-controllers post start hook completed")
 
@@ -137,7 +137,7 @@ func (c completedConfig) New() (*PinnipedServer, error) {
 	)
 
 	s.GenericAPIServer.AddPostStartHookOrDie("fetch-impersonation-proxy-tokens",
-		func(postStartContext genericapiserver.PostStartHookContext) error {
+		func(_ genericapiserver.PostStartHookContext) error {
 			plog.Debug("fetch-impersonation-proxy-tokens start hook starting")
 			defer plog.Debug("fetch-impersonation-proxy-tokens start hook completed")
 

internal/concierge/server/server.go

@@ -73,7 +73,7 @@ func (a *App) addServerCommand(ctx context.Context, args []string, stdout, stder
 			pinniped-concierge provides a generic API for mapping an external
 			credential from somewhere to an internal credential to be used for
 			authenticating to the Kubernetes API.`),
-		RunE: func(cmd *cobra.Command, args []string) error { return a.runServer(ctx) },
+		RunE: func(_ *cobra.Command, _args []string) error { return a.runServer(ctx) },
 		Args: cobra.NoArgs,
 	}
 

internal/controller/supervisorconfig/generator/secret_helper.go

@@ -1,4 +1,4 @@
-// Copyright 2020-2021 the Pinniped contributors. All Rights Reserved.
+// Copyright 2020-2024 the Pinniped contributors. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 
 package generator
@@ -60,7 +60,7 @@ const (
 	SecretUsageStateEncryptionKey
 )
 
-// New returns a SecretHelper that has been parameterized with common symmetric secret generation
+// NewSymmetricSecretHelper returns a SecretHelper that has been parameterized with common symmetric secret generation
 // knobs.
 func NewSymmetricSecretHelper(
 	namePrefix string,

internal/controller/supervisorstorage/garbage_collector.go

@@ -79,7 +79,7 @@ func GarbageCollectorController(
 				UpdateFunc: func(oldObj, newObj metav1.Object) bool {
 					return isSecretWithGCAnnotation(oldObj) || isSecretWithGCAnnotation(newObj)
 				},
-				DeleteFunc: func(obj metav1.Object) bool { return false }, // ignore all deletes
+				DeleteFunc: func(_ metav1.Object) bool { return false }, // ignore all deletes
 				ParentFunc: pinnipedcontroller.SingletonQueue(),
 			},
 			controllerlib.InformerOption{},

internal/controller/utils.go

@@ -1,4 +1,4 @@
-// Copyright 2020-2023 the Pinniped contributors. All Rights Reserved.
+// Copyright 2020-2024 the Pinniped contributors. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 
 package controller
@@ -21,16 +21,16 @@ func NameAndNamespaceExactMatchFilterFactory(name, namespace string) controllerl
 // MatchAnythingIgnoringUpdatesFilter returns a controllerlib.Filter that allows all objects but ignores updates.
 func MatchAnythingIgnoringUpdatesFilter(parentFunc controllerlib.ParentFunc) controllerlib.Filter {
 	return controllerlib.FilterFuncs{
-		AddFunc:    func(object metav1.Object) bool { return true },
-		UpdateFunc: func(oldObj, newObj metav1.Object) bool { return false },
-		DeleteFunc: func(object metav1.Object) bool { return true },
+		AddFunc:    func(_ metav1.Object) bool { return true },
+		UpdateFunc: func(_oldObj, _newObj metav1.Object) bool { return false },
+		DeleteFunc: func(_ metav1.Object) bool { return true },
 		ParentFunc: parentFunc,
 	}
 }
 
 // MatchAnythingFilter returns a controllerlib.Filter that allows all objects.
 func MatchAnythingFilter(parentFunc controllerlib.ParentFunc) controllerlib.Filter {
-	return SimpleFilter(func(object metav1.Object) bool { return true }, parentFunc)
+	return SimpleFilter(func(_ metav1.Object) bool { return true }, parentFunc)
 }
 
 // SimpleFilter takes a single boolean match function on a metav1.Object and wraps it into a proper controllerlib.Filter.

internal/controllerlib/filter.go

@@ -1,4 +1,4 @@
-// Copyright 2020 the Pinniped contributors. All Rights Reserved.
+// Copyright 2020-2024 the Pinniped contributors. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 
 package controllerlib
@@ -66,7 +66,7 @@ func FilterByNames(parentFunc ParentFunc, names ...string) Filter {
 	return FilterFuncs{
 		ParentFunc: parentFunc,
 		AddFunc:    has,
-		UpdateFunc: func(oldObj, newObj metav1.Object) bool {
+		UpdateFunc: func(_oldObj, newObj metav1.Object) bool {
 			return has(newObj)
 		},
 		DeleteFunc: has,

internal/federationdomain/oidc/oidc.go

@@ -173,7 +173,7 @@ func DefaultOIDCTimeoutsConfiguration() timeouts.Configuration {
 		AuthorizeCodeLifespan: authorizationCodeLifespan,
 
 		AccessTokenLifespan: accessTokenLifespan,
-		OverrideDefaultAccessTokenLifespan: func(accessRequest fosite.AccessRequester) (time.Duration, bool) {
+		OverrideDefaultAccessTokenLifespan: func(_ fosite.AccessRequester) (time.Duration, bool) {
 			// Not currently overriding the defaults.
 			return 0, false
 		},
@@ -202,23 +202,23 @@ func DefaultOIDCTimeoutsConfiguration() timeouts.Configuration {
 
 		RefreshTokenLifespan: refreshTokenLifespan,
 
-		AuthorizationCodeSessionStorageLifetime: func(requester fosite.Requester) time.Duration {
+		AuthorizationCodeSessionStorageLifetime: func(_ fosite.Requester) time.Duration {
 			return authorizationCodeLifespan + refreshTokenLifespan
 		},
 
-		PKCESessionStorageLifetime: func(_requester fosite.Requester) time.Duration {
+		PKCESessionStorageLifetime: func(_ fosite.Requester) time.Duration {
 			return authorizationCodeLifespan + storageExtraLifetime
 		},
 
-		OIDCSessionStorageLifetime: func(_requester fosite.Requester) time.Duration {
+		OIDCSessionStorageLifetime: func(_ fosite.Requester) time.Duration {
 			return authorizationCodeLifespan + storageExtraLifetime
 		},
 
-		AccessTokenSessionStorageLifetime: func(requester fosite.Requester) time.Duration {
+		AccessTokenSessionStorageLifetime: func(_ fosite.Requester) time.Duration {
 			return refreshTokenLifespan + accessTokenLifespan
 		},
 
-		RefreshTokenSessionStorageLifetime: func(requester fosite.Requester) time.Duration {
+		RefreshTokenSessionStorageLifetime: func(_ fosite.Requester) time.Duration {
 			return refreshTokenLifespan + accessTokenLifespan
 		},
 	}

internal/supervisor/apiserver/apiserver.go

@@ -1,4 +1,4 @@
-// Copyright 2022-2023 the Pinniped contributors. All Rights Reserved.
+// Copyright 2022-2024 the Pinniped contributors. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 
 package apiserver
@@ -117,7 +117,7 @@ func (c completedConfig) New() (*PinnipedServer, error) {
 	controllersCtx, cancelControllerCtx := context.WithCancel(context.Background())
 
 	s.GenericAPIServer.AddPostStartHookOrDie("start-controllers",
-		func(postStartContext genericapiserver.PostStartHookContext) error {
+		func(_ genericapiserver.PostStartHookContext) error {
 			plog.Debug("start-controllers post start hook starting")
 			defer plog.Debug("start-controllers post start hook completed")
 

internal/supervisor/server/server.go

@@ -441,8 +441,8 @@ func runSupervisor(ctx context.Context, podInfo *downward.PodInfo, cfg *supervis
 
 	// Serve the /healthz endpoint and make all other paths result in 404.
 	healthMux := http.NewServeMux()
-	healthMux.Handle("/healthz", http.HandlerFunc(func(writer http.ResponseWriter, request *http.Request) {
-		_, _ = writer.Write([]byte("ok"))
+	healthMux.Handle("/healthz", http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		_, _ = w.Write([]byte("ok"))
 	}))
 
 	dynamicServingCertProvider := dynamiccert.NewServingCert("supervisor-serving-cert")

internal/upstreamldap/upstreamldap.go

@@ -414,12 +414,12 @@ func (p *Provider) TestConnection(ctx context.Context) error {
 	return nil
 }
 
-// DryRunAuthenticateUser provides a method for testing all of the Provider settings in a kind of dry run of
+// DryRunAuthenticateUser provides a method for testing all the Provider settings in a kind of dry run of
 // authentication for a given end user's username. It runs the same logic as AuthenticateUser except it does
 // not bind as that user, so it does not test their password. It returns the same values that a real call to
 // AuthenticateUser with the correct password would return.
 func (p *Provider) DryRunAuthenticateUser(ctx context.Context, username string) (*authenticators.Response, bool, error) {
-	endUserBindFunc := func(conn Conn, foundUserDN string) error {
+	endUserBindFunc := func(_ Conn, _foundUserDN string) error {
 		// Act as if the end user bind always succeeds.
 		return nil
 	}

pkg/oidcclient/login.go

@@ -477,7 +477,7 @@ func (h *handlerState) cliBasedAuth(authorizeOptions *[]oauth2.AuthCodeOption) (
 
 	// Don't follow redirects automatically because we want to handle redirects here.
 	var sawRedirect bool
-	h.httpClient.CheckRedirect = func(req *http.Request, via []*http.Request) error {
+	h.httpClient.CheckRedirect = func(_ *http.Request, _via []*http.Request) error {
 		sawRedirect = true
 		return http.ErrUseLastResponse
 	}

test/testlib/assertions.go

@@ -1,4 +1,4 @@
-// Copyright 2021-2023 the Pinniped contributors. All Rights Reserved.
+// Copyright 2021-2024 the Pinniped contributors. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 
 package testlib
@@ -87,7 +87,7 @@ func RequireEventually(
 	)
 
 	// Run the check until it completes with no assertion failures.
-	waitErr := wait.PollUntilContextTimeout(context.Background(), tick, waitFor, true, func(ctx context.Context) (bool, error) {
+	waitErr := wait.PollUntilContextTimeout(context.Background(), tick, waitFor, true, func(_ context.Context) (bool, error) {
 		t.Helper()
 		attempts++
 
@@ -134,7 +134,7 @@ func RequireEventuallyWithoutError(
 	t.Helper()
 	// This previously used wait.PollImmediate (now deprecated), which did not take a ctx arg in the func.
 	// Hide this detail from the callers for now to keep the old signature.
-	fWithCtx := func(ctx context.Context) (bool, error) { return f() }
+	fWithCtx := func(_ context.Context) (bool, error) { return f() }
 	require.NoError(t, wait.PollUntilContextTimeout(context.Background(), tick, waitFor, true, fWithCtx), msgAndArgs...)
 }
 
@@ -151,7 +151,7 @@ func RequireNeverWithoutError(
 	t.Helper()
 	// This previously used wait.PollImmediate (now deprecated), which did not take a ctx arg in the func.
 	// Hide this detail from the callers for now to keep the old signature.
-	fWithCtx := func(ctx context.Context) (bool, error) { return f() }
+	fWithCtx := func(_ context.Context) (bool, error) { return f() }
 	err := wait.PollUntilContextTimeout(context.Background(), tick, waitFor, true, fWithCtx)
 	if err != nil && !wait.Interrupted(err) {
 		require.NoError(t, err, msgAndArgs...) // this will fail and throw the right error message