Upgrade the linter to golangci-lint@v1.55.1

The unused-parameter linter became stricter, so we adjust it to
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.

.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"`