Merge pull request #1752 from vmware-tanzu/upgrade-linter

Upgrade the linter to golangci-lint@v1.55.1

.golangci.yaml

@@ -7,52 +7,52 @@ run:
 linters:
   disable-all: true
   enable:
-  # default linters
-  - errcheck
-  - gosimple
-  - govet
-  - ineffassign
-  - staticcheck
-  - typecheck
-  - unused
+    # default linters
+    - errcheck
+    - gosimple
+    - govet
+    - ineffassign
+    - staticcheck
+    - typecheck
+    - unused
 
-  # additional linters for this project (we should disable these if they get annoying).
-  - asciicheck
-  - bodyclose
-  - depguard
-  - dogsled
-  - exhaustive
-  - exportloopref
-  - funlen
-  - gochecknoglobals
-  - gochecknoinits
-  - gocritic
-  - gocyclo
-  - godot
-  - goheader
-  - goimports
-  - revive
-  - goprintffuncname
-  - gosec
-  - misspell
-  - nakedret
-  - nestif
-  - noctx
-  - nolintlint
-  - prealloc
-  - rowserrcheck
-  - exportloopref
-  - sqlclosecheck
-  - unconvert
-  - whitespace
+    # additional linters for this project (we should disable these if they get annoying).
+    - asciicheck
+    - bodyclose
+    # - depguard
+    - dogsled
+    - exhaustive
+    - exportloopref
+    - funlen
+    - gochecknoglobals
+    - gochecknoinits
+    - gocritic
+    - gocyclo
+    - godot
+    - goheader
+    - goimports
+    - revive
+    - goprintffuncname
+    - gosec
+    - misspell
+    - nakedret
+    - nestif
+    - noctx
+    - nolintlint
+    - prealloc
+    - rowserrcheck
+    - exportloopref
+    - sqlclosecheck
+    - unconvert
+    - whitespace
 
 issues:
   exclude-rules:
     # exclude tests from some rules for things that are useful in a testing context.
-  - path: _test\.go
-    linters:
-    - funlen
-    - gochecknoglobals
+    - path: _test\.go
+      linters:
+        - funlen
+        - gochecknoglobals
 
 linters-settings:
   funlen:
@@ -64,7 +64,15 @@ linters-settings:
         # YYYY or YYYY-YYYY
         YEARS: \d\d\d\d(-\d\d\d\d)?
     template: |-
-        Copyright {{YEARS}} the Pinniped contributors. All Rights Reserved.
-        SPDX-License-Identifier: Apache-2.0
+      Copyright {{YEARS}} the Pinniped contributors. All Rights Reserved.
+      SPDX-License-Identifier: Apache-2.0
   goimports:
     local-prefixes: go.pinniped.dev
+  revive:
+    max-open-files: 2048
+    rules:
+      - name: unused-parameter
+        arguments:
+          # Allow unused params that start with underscore. It can be nice to keep unused param names when implementing
+          # an interface sometimes, to help readers understand why it is unused in that particular implementation.
+          - allowRegex: "^_"

hack/install-linter.sh

@@ -15,7 +15,7 @@ go version
 # so you can get the same results when running the linter locally.
 # Whenever the linter is updated in the CI pipelines, it should also be
 # updated here to make local development more convenient.
-go install -v github.com/golangci/golangci-lint/cmd/golangci-lint@v1.51.2
+go install -v github.com/golangci/golangci-lint/cmd/golangci-lint@v1.55.1
 golangci-lint --version
 
 echo "Finished. You may need to run 'rehash' in your current shell before using the new version (e.g. if you are using gvm)."

internal/dynamiccert/provider.go

@@ -1,4 +1,4 @@
-// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved.
+// Copyright 2020-2023 the Pinniped contributors. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 
 package dynamiccert
@@ -153,6 +153,6 @@ func (p *provider) RunOnce(_ context.Context) error {
 	return nil // no-op, but we want to make sure to stay in sync with dynamiccertificates.ControllerRunner
 }
 
-func (p *provider) Run(_ context.Context, workers int) {
+func (p *provider) Run(_ context.Context, _workers int) {
 	// no-op, but we want to make sure to stay in sync with dynamiccertificates.ControllerRunner
 }

internal/federationdomain/clientregistry/clientregistry.go

@@ -118,7 +118,7 @@ func (m *ClientManager) GetClient(ctx context.Context, id string) (fosite.Client
 // known or the DB check failed and nil if the JTI is not known.
 //
 // This functionality is not supported by the ClientManager.
-func (*ClientManager) ClientAssertionJWTValid(ctx context.Context, jti string) error {
+func (*ClientManager) ClientAssertionJWTValid(_ctx context.Context, _jti string) error {
 	return fmt.Errorf("not implemented")
 }
 
@@ -128,7 +128,7 @@ func (*ClientManager) ClientAssertionJWTValid(ctx context.Context, jti string) e
 // not be replayed due to the expiry.
 //
 // This functionality is not supported by the ClientManager.
-func (*ClientManager) SetClientAssertionJWT(ctx context.Context, jti string, exp time.Time) error {
+func (*ClientManager) SetClientAssertionJWT(_ctx context.Context, _jti string, _exp time.Time) error {
 	return fmt.Errorf("not implemented")
 }
 

internal/federationdomain/storage/dynamic_global_secret_config.go

@@ -51,13 +51,13 @@ func (d *DynamicGlobalSecretConfig) GetHMACHasher(ctx context.Context) func() ha
 	return d.fositeConfig.GetHMACHasher(ctx)
 }
 
-func (d *DynamicGlobalSecretConfig) GetGlobalSecret(ctx context.Context) ([]byte, error) {
+func (d *DynamicGlobalSecretConfig) GetGlobalSecret(_ctx context.Context) ([]byte, error) {
 	// Always call keyFunc() without ever caching its value, because that is the whole point
 	// of this type. We want the global secret to be dynamic.
 	return d.keyFunc(), nil
 }
 
-func (d *DynamicGlobalSecretConfig) GetRotatedGlobalSecrets(ctx context.Context) ([][]byte, error) {
+func (d *DynamicGlobalSecretConfig) GetRotatedGlobalSecrets(_ctx context.Context) ([][]byte, error) {
 	// We don't support having multiple global secrets yet, but when we do we will need to implement this.
 	return nil, nil
 }

internal/fositestorage/refreshtoken/refreshtoken.go

@@ -77,7 +77,7 @@ func (a *refreshTokenStorage) RevokeRefreshToken(ctx context.Context, requestID
 	return a.storage.DeleteByLabel(ctx, fositestorage.StorageRequestIDLabelName, requestID)
 }
 
-func (a *refreshTokenStorage) RevokeRefreshTokenMaybeGracePeriod(ctx context.Context, requestID string, signature string) error {
+func (a *refreshTokenStorage) RevokeRefreshTokenMaybeGracePeriod(ctx context.Context, requestID string, _signature string) error {
 	// We don't support a grace period, so always call the regular RevokeRefreshToken().
 	return a.RevokeRefreshToken(ctx, requestID)
 }

internal/idtransform/identity_transformations_test.go

@@ -13,7 +13,7 @@ import (
 
 type fakeNoopTransformer struct{}
 
-func (a fakeNoopTransformer) Evaluate(ctx context.Context, username string, groups []string) (*TransformationResult, error) {
+func (a fakeNoopTransformer) Evaluate(_ctx context.Context, username string, groups []string) (*TransformationResult, error) {
 	return &TransformationResult{
 		Username:                      username,
 		Groups:                        groups,
@@ -28,7 +28,7 @@ func (a fakeNoopTransformer) Source() interface{} {
 
 type fakeNilGroupTransformer struct{}
 
-func (a fakeNilGroupTransformer) Evaluate(ctx context.Context, username string, groups []string) (*TransformationResult, error) {
+func (a fakeNilGroupTransformer) Evaluate(_ctx context.Context, username string, _groups []string) (*TransformationResult, error) {
 	return &TransformationResult{
 		Username:                      username,
 		Groups:                        nil,
@@ -43,7 +43,7 @@ func (a fakeNilGroupTransformer) Source() interface{} {
 
 type fakeAppendStringTransformer struct{}
 
-func (a fakeAppendStringTransformer) Evaluate(ctx context.Context, username string, groups []string) (*TransformationResult, error) {
+func (a fakeAppendStringTransformer) Evaluate(_ctx context.Context, username string, groups []string) (*TransformationResult, error) {
 	newGroups := []string{}
 	for _, group := range groups {
 		newGroups = append(newGroups, group+":transformed")
@@ -62,7 +62,7 @@ func (a fakeAppendStringTransformer) Source() interface{} {
 
 type fakeDeleteUsernameAndGroupsTransformer struct{}
 
-func (a fakeDeleteUsernameAndGroupsTransformer) Evaluate(ctx context.Context, username string, groups []string) (*TransformationResult, error) {
+func (a fakeDeleteUsernameAndGroupsTransformer) Evaluate(_ctx context.Context, _username string, _groups []string) (*TransformationResult, error) {
 	return &TransformationResult{
 		Username:                      "",
 		Groups:                        []string{},
@@ -77,7 +77,7 @@ func (a fakeDeleteUsernameAndGroupsTransformer) Source() interface{} {
 
 type fakeAuthenticationDisallowedTransformer struct{}
 
-func (a fakeAuthenticationDisallowedTransformer) Evaluate(ctx context.Context, username string, groups []string) (*TransformationResult, error) {
+func (a fakeAuthenticationDisallowedTransformer) Evaluate(_ctx context.Context, username string, groups []string) (*TransformationResult, error) {
 	newGroups := []string{}
 	for _, group := range groups {
 		newGroups = append(newGroups, group+":disallowed")
@@ -96,7 +96,7 @@ func (a fakeAuthenticationDisallowedTransformer) Source() interface{} {
 
 type fakeErrorTransformer struct{}
 
-func (a fakeErrorTransformer) Evaluate(ctx context.Context, username string, groups []string) (*TransformationResult, error) {
+func (a fakeErrorTransformer) Evaluate(_ctx context.Context, _username string, _groups []string) (*TransformationResult, error) {
 	return &TransformationResult{}, errors.New("unexpected catastrophic error")
 }
 
@@ -108,7 +108,7 @@ type fakeTransformerWithSource struct {
 	source string
 }
 
-func (a fakeTransformerWithSource) Evaluate(ctx context.Context, username string, groups []string) (*TransformationResult, error) {
+func (a fakeTransformerWithSource) Evaluate(_ctx context.Context, _username string, _groups []string) (*TransformationResult, error) {
 	return nil, nil // not needed for this test
 }
 

internal/kubeclient/copied.go

@@ -1,4 +1,4 @@
-// Copyright 2021-2022 the Pinniped contributors. All Rights Reserved.
+// Copyright 2021-2023 the Pinniped contributors. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 
 package kubeclient
@@ -15,7 +15,7 @@ import (
 )
 
 // defaultServerUrlFor was copied from k8s.io/client-go/rest/url_utils.go.
-func defaultServerUrlFor(config *restclient.Config) (*url.URL, string, error) { //nolint:revive
+func defaultServerUrlFor(config *restclient.Config) (*url.URL, string, error) {
 	hasCA := len(config.CAFile) != 0 || len(config.CAData) != 0
 	hasCert := len(config.CertFile) != 0 || len(config.CertData) != 0
 	defaultTLS := hasCA || hasCert || config.Insecure

internal/testutil/oidctestutil/oidctestutil.go

@@ -212,7 +212,7 @@ func (u *TestUpstreamLDAPIdentityProvider) GetName() string {
 	return u.Name
 }
 
-func (u *TestUpstreamLDAPIdentityProvider) AuthenticateUser(ctx context.Context, username, password string, grantedScopes []string) (*authenticators.Response, bool, error) {
+func (u *TestUpstreamLDAPIdentityProvider) AuthenticateUser(ctx context.Context, username, password string, _grantedScopes []string) (*authenticators.Response, bool, error) {
 	return u.AuthenticateFunc(ctx, username, password)
 }
 
@@ -220,7 +220,7 @@ func (u *TestUpstreamLDAPIdentityProvider) GetURL() *url.URL {
 	return u.URL
 }
 
-func (u *TestUpstreamLDAPIdentityProvider) PerformRefresh(ctx context.Context, storedRefreshAttributes upstreamprovider.RefreshAttributes, idpDisplayName string) ([]string, error) {
+func (u *TestUpstreamLDAPIdentityProvider) PerformRefresh(ctx context.Context, storedRefreshAttributes upstreamprovider.RefreshAttributes, _idpDisplayName string) ([]string, error) {
 	if u.performRefreshArgs == nil {
 		u.performRefreshArgs = make([]*PerformRefreshArgs, 0)
 	}

internal/testutil/testlogger/stdr_copied.go

@@ -105,7 +105,7 @@ func (l logger) Info(level int, msg string, kvList ...interface{}) {
 	}
 }
 
-func (l logger) Enabled(level int) bool {
+func (l logger) Enabled(_level int) bool {
 	return true
 }
 
@@ -134,7 +134,7 @@ func (l logger) output(calldepth int, s string) {
 	}
 }
 
-func (l logger) V(level int) logr.LogSink {
+func (l logger) V(_level int) logr.LogSink {
 	return l.clone()
 }
 
@@ -142,27 +142,27 @@ func (l logger) V(level int) logr.LogSink {
 // uses '/' characters to separate name elements.  Callers should not pass '/'
 // in the provided name string, but this library does not actually enforce that.
 func (l logger) WithName(name string) logr.LogSink {
-	new := l.clone()
+	lgr := l.clone()
 	if len(l.prefix) > 0 {
-		new.prefix = l.prefix + "/"
+		lgr.prefix = l.prefix + "/"
 	}
-	new.prefix += name
-	return new
+	lgr.prefix += name
+	return lgr
 }
 
 // WithValues returns a new logr.Logger with the specified key-and-values
 // saved.
 func (l logger) WithValues(kvList ...interface{}) logr.LogSink {
-	new := l.clone()
-	new.values = append(new.values, kvList...)
-	return new
+	lgr := l.clone()
+	lgr.values = append(lgr.values, kvList...)
+	return lgr
 }
 
-func (l logger) WithCallDepth(depth int) logr.LogSink {
+func (l logger) WithCallDepth(_depth int) logr.LogSink {
 	return l.clone()
 }
 
 var _ logr.LogSink = logger{}
 var _ logr.CallDepthLogSink = logger{}
 
-func (l logger) Init(info logr.RuntimeInfo) {}
+func (l logger) Init(_info logr.RuntimeInfo) {}

internal/testutil/transcript_logger.go

@@ -1,4 +1,4 @@
-// Copyright 2020-2022 the Pinniped contributors. All Rights Reserved.
+// Copyright 2020-2023 the Pinniped contributors. All Rights Reserved.
 // SPDX-License-Identifier: Apache-2.0
 
 package testutil
@@ -37,7 +37,7 @@ func (log *TranscriptLogger) Transcript() []TranscriptLogMessage {
 	return result
 }
 
-func (log *TranscriptLogger) Info(level int, msg string, keysAndValues ...interface{}) {
+func (log *TranscriptLogger) Info(_level int, msg string, keysAndValues ...interface{}) {
 	log.lock.Lock()
 	defer log.lock.Unlock()
 	log.transcript = append(log.transcript, TranscriptLogMessage{
@@ -55,7 +55,7 @@ func (log *TranscriptLogger) Error(_ error, msg string, _ ...interface{}) {
 	})
 }
 
-func (log *TranscriptLogger) Enabled(level int) bool {
+func (log *TranscriptLogger) Enabled(_level int) bool {
 	return true
 }
 
@@ -71,4 +71,4 @@ func (log *TranscriptLogger) WithValues(_ ...interface{}) logr.LogSink {
 	return log
 }
 
-func (log *TranscriptLogger) Init(info logr.RuntimeInfo) {}
+func (log *TranscriptLogger) Init(_info logr.RuntimeInfo) {}

test/testlib/env.go

@@ -101,7 +101,7 @@ type TestLDAPUpstream struct {
 	TestUserUniqueIDAttributeValue                  string   `json:"testUserUniqueIDAttributeValue"`
 	TestUserDirectGroupsCNs                         []string `json:"testUserDirectGroupsCNs"`
 	TestUserDirectPosixGroupsCNs                    []string `json:"testUserDirectPosixGroupsCNs"`
-	TestUserDirectGroupsDNs                         []string `json:"testUserDirectGroupsDNs"` //nolint:revive // this is "distinguished names", not "DNS"
+	TestUserDirectGroupsDNs                         []string `json:"testUserDirectGroupsDNs"`
 	TestUserSAMAccountNameValue                     string   `json:"testUserSAMAccountNameValue"`
 	TestUserPrincipalNameValue                      string   `json:"testUserPrincipalNameValue"`
 	TestUserIndirectGroupsSAMAccountNames           []string `json:"TestUserIndirectGroupsSAMAccountNames"`