From e04e5e01854a89db92f6e8257e8754e139bcba9e Mon Sep 17 00:00:00 2001 From: Joshua Casey Date: Tue, 7 May 2024 22:34:32 -0500 Subject: [PATCH] Fix revive linter issues for all production code, and exclude revive linter issues for test code --- .golangci.yaml | 4 ++++ cmd/pinniped/cmd/kubeconfig.go | 4 ++-- cmd/pinniped/cmd/login_oidc.go | 2 +- cmd/pinniped/cmd/login_static.go | 2 +- ...issionpluginconfg.go => admissionpluginconfig.go} | 0 ...inconfg_test.go => admissionpluginconfig_test.go} | 0 internal/concierge/apiserver/apiserver.go | 6 +++--- internal/concierge/server/server.go | 2 +- .../supervisorconfig/generator/secret_helper.go | 4 ++-- .../supervisorstorage/garbage_collector.go | 2 +- internal/controller/utils.go | 10 +++++----- internal/controllerlib/filter.go | 4 ++-- internal/federationdomain/oidc/oidc.go | 12 ++++++------ internal/supervisor/apiserver/apiserver.go | 4 ++-- internal/supervisor/server/server.go | 4 ++-- internal/upstreamldap/upstreamldap.go | 4 ++-- pkg/oidcclient/login.go | 2 +- test/testlib/assertions.go | 8 ++++---- 18 files changed, 39 insertions(+), 35 deletions(-) rename internal/admissionpluginconfig/{admissionpluginconfg.go => admissionpluginconfig.go} (100%) rename internal/admissionpluginconfig/{admissionpluginconfg_test.go => admissionpluginconfig_test.go} (100%) diff --git a/.golangci.yaml b/.golangci.yaml index 1caff55d6..2c872f55e 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -53,6 +53,10 @@ issues: linters: - funlen - gochecknoglobals + - revive + - path: internal/testutil/ + linters: + - revive linters-settings: funlen: diff --git a/cmd/pinniped/cmd/kubeconfig.go b/cmd/pinniped/cmd/kubeconfig.go index 1674c1bd8..0265fab9f 100644 --- a/cmd/pinniped/cmd/kubeconfig.go +++ b/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 { diff --git a/cmd/pinniped/cmd/login_oidc.go b/cmd/pinniped/cmd/login_oidc.go index 5bfc50db2..3efa88130 100644 --- a/cmd/pinniped/cmd/login_oidc.go +++ b/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") diff --git a/cmd/pinniped/cmd/login_static.go b/cmd/pinniped/cmd/login_static.go index 283b4a0fa..9f82a3f12 100644 --- a/cmd/pinniped/cmd/login_static.go +++ b/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") diff --git a/internal/admissionpluginconfig/admissionpluginconfg.go b/internal/admissionpluginconfig/admissionpluginconfig.go similarity index 100% rename from internal/admissionpluginconfig/admissionpluginconfg.go rename to internal/admissionpluginconfig/admissionpluginconfig.go diff --git a/internal/admissionpluginconfig/admissionpluginconfg_test.go b/internal/admissionpluginconfig/admissionpluginconfig_test.go similarity index 100% rename from internal/admissionpluginconfig/admissionpluginconfg_test.go rename to internal/admissionpluginconfig/admissionpluginconfig_test.go diff --git a/internal/concierge/apiserver/apiserver.go b/internal/concierge/apiserver/apiserver.go index 37aafff74..a43c82261 100644 --- a/internal/concierge/apiserver/apiserver.go +++ b/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") diff --git a/internal/concierge/server/server.go b/internal/concierge/server/server.go index 2f64775c3..728acb353 100644 --- a/internal/concierge/server/server.go +++ b/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, } diff --git a/internal/controller/supervisorconfig/generator/secret_helper.go b/internal/controller/supervisorconfig/generator/secret_helper.go index 9ae700c9e..3b5a32d86 100644 --- a/internal/controller/supervisorconfig/generator/secret_helper.go +++ b/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, diff --git a/internal/controller/supervisorstorage/garbage_collector.go b/internal/controller/supervisorstorage/garbage_collector.go index 2bab1bcd2..bd791f8ba 100644 --- a/internal/controller/supervisorstorage/garbage_collector.go +++ b/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{}, diff --git a/internal/controller/utils.go b/internal/controller/utils.go index 1b938e83b..f63fa3dae 100644 --- a/internal/controller/utils.go +++ b/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. diff --git a/internal/controllerlib/filter.go b/internal/controllerlib/filter.go index aeda6d4d2..3e9a8ec1d 100644 --- a/internal/controllerlib/filter.go +++ b/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, diff --git a/internal/federationdomain/oidc/oidc.go b/internal/federationdomain/oidc/oidc.go index ba839ca7c..2e07a65f6 100644 --- a/internal/federationdomain/oidc/oidc.go +++ b/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 }, } diff --git a/internal/supervisor/apiserver/apiserver.go b/internal/supervisor/apiserver/apiserver.go index 6fd040be7..703e61fb4 100644 --- a/internal/supervisor/apiserver/apiserver.go +++ b/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") diff --git a/internal/supervisor/server/server.go b/internal/supervisor/server/server.go index 99709d57d..7f3925c11 100644 --- a/internal/supervisor/server/server.go +++ b/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") diff --git a/internal/upstreamldap/upstreamldap.go b/internal/upstreamldap/upstreamldap.go index b49333608..7735b231b 100644 --- a/internal/upstreamldap/upstreamldap.go +++ b/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 } diff --git a/pkg/oidcclient/login.go b/pkg/oidcclient/login.go index ad0e8e532..d82849c2a 100644 --- a/pkg/oidcclient/login.go +++ b/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 } diff --git a/test/testlib/assertions.go b/test/testlib/assertions.go index 1178ed0e5..21acf56b7 100644 --- a/test/testlib/assertions.go +++ b/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