diff --git a/.golangci.yaml b/.golangci.yaml index ebb6b2576..13665b474 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -1,24 +1,14 @@ # https://golangci-lint.run/usage/configuration/ -run: - timeout: 1m +version: "2" linters: - disable-all: true + default: none enable: - # 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 + - copyloopvar - dogsled + - errcheck - exhaustive - funlen - gochecknoglobals @@ -27,136 +17,149 @@ linters: - gocyclo - godot - goheader - - goimports - - revive - goprintffuncname - gosec + - govet + - importas + - ineffassign + - intrange + - makezero - misspell - nakedret - nestif - noctx - nolintlint - prealloc + - revive - rowserrcheck - - sqlclosecheck - - unconvert - - whitespace - - copyloopvar - - intrange - # - fatcontext Starting in go@1.23.1 and golangci-lint@1.61.0 this gave a lot of false positives - # - canonicalheader Can't do this one since it alerts on valid headers such as X-XSS-Protection - spancheck - - importas - - makezero - - prealloc - - gofmt - -issues: - exclude-dirs: - - generated - exclude-rules: - # exclude tests from some rules for things that are useful in a testing context. - - path: _test\.go - linters: - - funlen - - gochecknoglobals - - revive - - path: internal/testutil/ - linters: - - revive - -linters-settings: - funlen: - lines: 150 - statements: 50 - goheader: - values: - regexp: - # 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 - goimports: - local-prefixes: go.pinniped.dev - revive: - max-open-files: 2048 + - sqlclosecheck + - staticcheck + - unconvert + - unused + - whitespace + settings: + funlen: + lines: 150 + statements: 50 + goheader: + values: + regexp: + # 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 + importas: + alias: + - pkg: k8s.io/apimachinery/pkg/util/errors + alias: utilerrors + - pkg: k8s.io/apimachinery/pkg/api/errors + alias: apierrors + - pkg: k8s.io/apimachinery/pkg/apis/meta/v1 + alias: metav1 + - pkg: k8s.io/api/core/v1 + alias: corev1 + - pkg: github.com/coreos/go-oidc/v3/oidc + alias: coreosoidc + - pkg: github.com/ory/fosite/handler/oauth2 + alias: fositeoauth2 + - pkg: github.com/ory/fosite/token/jwt + alias: fositejwt + - pkg: github.com/go-jose/go-jose/v4/jwt + alias: josejwt + - pkg: github.com/go-jose/go-jose/v3 + alias: oldjosev3 + - pkg: go.pinniped.dev/generated/latest/apis/concierge/authentication/v1alpha1 + alias: authenticationv1alpha1 + - pkg: go.pinniped.dev/generated/latest/apis/supervisor/clientsecret/v1alpha1 + alias: clientsecretv1alpha1 + - pkg: go.pinniped.dev/generated/latest/apis/supervisor/config/v1alpha1 + alias: supervisorconfigv1alpha1 + - pkg: go.pinniped.dev/generated/latest/apis/concierge/config/v1alpha1 + alias: conciergeconfigv1alpha1 + - pkg: go.pinniped.dev/generated/latest/client/concierge/clientset/versioned + alias: conciergeclientset + - pkg: go.pinniped.dev/generated/latest/client/concierge/clientset/versioned/scheme + alias: conciergeclientsetscheme + - pkg: go.pinniped.dev/generated/latest/client/concierge/clientset/versioned/fake + alias: conciergefake + - pkg: go.pinniped.dev/generated/latest/client/supervisor/clientset/versioned + alias: supervisorclientset + - pkg: go.pinniped.dev/generated/latest/client/supervisor/clientset/versioned/scheme + alias: supervisorclientsetscheme + - pkg: go.pinniped.dev/generated/latest/client/supervisor/clientset/versioned/fake + alias: supervisorfake + - pkg: go.pinniped.dev/generated/latest/apis/supervisor/idp/v1alpha1 + alias: idpv1alpha1 + - pkg: go.pinniped.dev/generated/latest/client/concierge/informers/externalversions + alias: conciergeinformers + - pkg: go.pinniped.dev/generated/latest/client/supervisor/informers/externalversions + alias: supervisorinformers + - pkg: go.pinniped.dev/internal/concierge/scheme + alias: conciergescheme + no-unaliased: true # All packages explicitly listed above must be aliased + no-extra-aliases: false # Allow other aliases than the ones explicitly listed above + revive: + max-open-files: 2048 + rules: + # 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. + - name: unused-parameter + arguments: + - allowRegex: ^_ + spancheck: + # https://golangci-lint.run/usage/linters/#spancheck + checks: + - end + - record-error + - set-status + exclusions: + generated: lax + presets: + - comments + - common-false-positives + - legacy + - std-error-handling 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: "^_" - spancheck: - # https://golangci-lint.run/usage/linters/#spancheck - checks: - - end - - record-error - - set-status - importas: - no-unaliased: true # All packages explicitly listed below must be aliased - no-extra-aliases: false # Allow other aliases than the ones explicitly listed below - alias: - # k8s.io/apimachinery - - pkg: k8s.io/apimachinery/pkg/util/errors - alias: utilerrors - - pkg: k8s.io/apimachinery/pkg/api/errors - alias: apierrors - - pkg: k8s.io/apimachinery/pkg/apis/meta/v1 - alias: metav1 - # k8s.io - - pkg: k8s.io/api/core/v1 - alias: corev1 - # OAuth2/OIDC/Fosite/JOSE - - pkg: github.com/coreos/go-oidc/v3/oidc - alias: coreosoidc - - pkg: github.com/ory/fosite/handler/oauth2 - alias: fositeoauth2 - - pkg: github.com/ory/fosite/token/jwt - alias: fositejwt - - pkg: github.com/go-jose/go-jose/v4/jwt - alias: josejwt - - pkg: github.com/go-jose/go-jose/v3 - alias: oldjosev3 - # Generated Pinniped - - pkg: go.pinniped.dev/generated/latest/apis/concierge/authentication/v1alpha1 - alias: authenticationv1alpha1 - - pkg: go.pinniped.dev/generated/latest/apis/supervisor/clientsecret/v1alpha1 - alias: clientsecretv1alpha1 - - pkg: go.pinniped.dev/generated/latest/apis/supervisor/config/v1alpha1 - alias: supervisorconfigv1alpha1 - - pkg: go.pinniped.dev/generated/latest/apis/concierge/config/v1alpha1 - alias: conciergeconfigv1alpha1 - - pkg: go.pinniped.dev/generated/latest/client/concierge/clientset/versioned - alias: conciergeclientset - - pkg: go.pinniped.dev/generated/latest/client/concierge/clientset/versioned/scheme - alias: conciergeclientsetscheme - - pkg: go.pinniped.dev/generated/latest/client/concierge/clientset/versioned/fake - alias: conciergefake - - pkg: go.pinniped.dev/generated/latest/client/supervisor/clientset/versioned - alias: supervisorclientset - - pkg: go.pinniped.dev/generated/latest/client/supervisor/clientset/versioned/scheme - alias: supervisorclientsetscheme - - pkg: go.pinniped.dev/generated/latest/client/supervisor/clientset/versioned/fake - alias: supervisorfake - - pkg: go.pinniped.dev/generated/latest/apis/supervisor/idp/v1alpha1 - alias: idpv1alpha1 - - pkg: go.pinniped.dev/generated/latest/client/concierge/informers/externalversions - alias: conciergeinformers - - pkg: go.pinniped.dev/generated/latest/client/supervisor/informers/externalversions - alias: supervisorinformers - # Pinniped internal - - pkg: go.pinniped.dev/internal/concierge/scheme - alias: conciergescheme - gofmt: - # Simplify code: gofmt with `-s` option. - # Default: true - simplify: false - # Apply the rewrite rules to the source before reformatting. - # https://pkg.go.dev/cmd/gofmt - # Default: [] - rewrite-rules: - - pattern: 'interface{}' - replacement: 'any' - - pattern: 'a[b:len(a)]' - replacement: 'a[b:]' + # exclude tests from some rules for things that are useful in a testing context. + - linters: + - funlen + - gochecknoglobals + - revive + path: _test\.go + - linters: + - revive + path: internal/testutil/ + paths: + - generated + - third_party$ + - builtin$ + - examples$ +formatters: + enable: + - gofmt + - goimports + settings: + gofmt: + # Simplify code: gofmt with `-s` option. + # Default: true + simplify: false + # Apply the rewrite rules to the source before reformatting. + # https://pkg.go.dev/cmd/gofmt + # Default: [] + rewrite-rules: + - pattern: interface{} + replacement: any + - pattern: a[b:len(a)] + replacement: a[b:] + goimports: + local-prefixes: + - go.pinniped.dev + exclusions: + generated: lax + paths: + - generated + - third_party$ + - builtin$ + - examples$ diff --git a/cmd/pinniped/cmd/kubeconfig.go b/cmd/pinniped/cmd/kubeconfig.go index 7a8fd8937..bceccafa6 100644 --- a/cmd/pinniped/cmd/kubeconfig.go +++ b/cmd/pinniped/cmd/kubeconfig.go @@ -416,10 +416,7 @@ func waitForCredentialIssuer(ctx context.Context, clientset conciergeclientset.I deadline, _ := ctx.Deadline() attempts := 1 - for { - if !hasPendingStrategy(credentialIssuer) { - break - } + for hasPendingStrategy(credentialIssuer) { logStrategies(credentialIssuer, deps.log) deps.log.Info("waiting for CredentialIssuer pending strategies to finish", "attempts", attempts, diff --git a/hack/install-linter.sh b/hack/install-linter.sh index ba30e81aa..3e7aebaa9 100755 --- a/hack/install-linter.sh +++ b/hack/install-linter.sh @@ -21,7 +21,7 @@ echo "Installing golangci-lint@${lint_version} using toolchain ${GOTOOLCHAIN}" # Install the same version of the linter that the pipelines will use # so you can get the same results when running the linter locally. -go install -v "github.com/golangci/golangci-lint/cmd/golangci-lint@${lint_version}" +go install -v "github.com/golangci/golangci-lint/v2/cmd/golangci-lint@${lint_version}" 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)." diff --git a/hack/lib/lint-version.txt b/hack/lib/lint-version.txt index d1dc303a1..399088bf4 100644 --- a/hack/lib/lint-version.txt +++ b/hack/lib/lint-version.txt @@ -1 +1 @@ -1.64.8 +2.1.6 diff --git a/internal/admissionpluginconfig/admissionpluginconfig_test.go b/internal/admissionpluginconfig/admissionpluginconfig_test.go index 2b6e96a47..5bfd40b87 100644 --- a/internal/admissionpluginconfig/admissionpluginconfig_test.go +++ b/internal/admissionpluginconfig/admissionpluginconfig_test.go @@ -18,7 +18,6 @@ import ( "k8s.io/client-go/discovery" kubernetesfake "k8s.io/client-go/kubernetes/fake" k8stesting "k8s.io/client-go/testing" - kubetesting "k8s.io/client-go/testing" ) func TestValidateAdmissionPluginNames(t *testing.T) { @@ -237,7 +236,7 @@ func TestConfigureAdmissionPlugins(t *testing.T) { t.Parallel() kubeClient := kubernetesfake.NewSimpleClientset() - kubeClient.Fake.Resources = tt.availableAPIResources + kubeClient.Resources = tt.availableAPIResources // Unfortunately, kubernetesfake.NewSimpleClientset() does not support using reactors to // cause discovery to return errors. Instead, we will make our own fake implementation of the @@ -248,7 +247,7 @@ func TestConfigureAdmissionPlugins(t *testing.T) { kubeClient.PrependReactor( "get", "resource", - func(a kubetesting.Action) (bool, runtime.Object, error) { + func(a k8stesting.Action) (bool, runtime.Object, error) { return true, nil, tt.discoveryErr }, ) diff --git a/internal/concierge/impersonator/impersonator.go b/internal/concierge/impersonator/impersonator.go index 29c0210bd..6cfac6dcd 100644 --- a/internal/concierge/impersonator/impersonator.go +++ b/internal/concierge/impersonator/impersonator.go @@ -39,7 +39,6 @@ import ( "k8s.io/apiserver/pkg/authorization/authorizer" "k8s.io/apiserver/pkg/endpoints/filterlatency" "k8s.io/apiserver/pkg/endpoints/handlers/responsewriters" - "k8s.io/apiserver/pkg/endpoints/request" genericapirequest "k8s.io/apiserver/pkg/endpoints/request" genericapiserver "k8s.io/apiserver/pkg/server" "k8s.io/apiserver/pkg/server/dynamiccertificates" @@ -509,7 +508,7 @@ func newImpersonationReverseProxyFunc(restConfig *rest.Config) (func(*genericapi return } - userInfo, ok := request.UserFrom(r.Context()) + userInfo, ok := genericapirequest.UserFrom(r.Context()) if !ok { plog.Warning("aggregated API server logic did not set user info but it is always supposed to do so", "url", r.URL.String(), diff --git a/internal/concierge/impersonator/impersonator_test.go b/internal/concierge/impersonator/impersonator_test.go index aa49eaed2..49a8734d0 100644 --- a/internal/concierge/impersonator/impersonator_test.go +++ b/internal/concierge/impersonator/impersonator_test.go @@ -1,4 +1,4 @@ -// Copyright 2020-2024 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2025 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package impersonator @@ -1042,8 +1042,8 @@ func TestImpersonator(t *testing.T) { badCertConfig := kubeclient.SecureAnonymousClientConfig(clientKubeconfig) badCert := newClientCert(t, unrelatedCA, "bad-user", []string{"bad-group"}) - badCertConfig.TLSClientConfig.CertData = badCert.certPEM - badCertConfig.TLSClientConfig.KeyData = badCert.keyPEM + badCertConfig.CertData = badCert.certPEM + badCertConfig.KeyData = badCert.keyPEM tcrBadCert, err := kubeclient.New(kubeclient.WithConfig(badCertConfig)) require.NoError(t, err) diff --git a/internal/controller/authenticator/jwtcachefiller/jwtcachefiller_test.go b/internal/controller/authenticator/jwtcachefiller/jwtcachefiller_test.go index d76a03d79..9662ed0ba 100644 --- a/internal/controller/authenticator/jwtcachefiller/jwtcachefiller_test.go +++ b/internal/controller/authenticator/jwtcachefiller/jwtcachefiller_test.go @@ -1,4 +1,4 @@ -// Copyright 2020-2024 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2025 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package jwtcachefiller @@ -35,7 +35,6 @@ import ( "k8s.io/apiserver/pkg/authentication/authenticator" "k8s.io/apiserver/pkg/authentication/user" k8sinformers "k8s.io/client-go/informers" - kubeinformers "k8s.io/client-go/informers" kubernetesfake "k8s.io/client-go/kubernetes/fake" coretesting "k8s.io/client-go/testing" clocktesting "k8s.io/utils/clock/testing" @@ -2410,7 +2409,7 @@ func TestController(t *testing.T) { tt.configClient(pinnipedAPIClient) } pinnipedInformers := conciergeinformers.NewSharedInformerFactory(pinnipedAPIClient, 0) - kubeInformers := kubeinformers.NewSharedInformerFactory(kubernetesfake.NewSimpleClientset(tt.secretsAndConfigMaps...), 0) + kubeInformers := k8sinformers.NewSharedInformerFactory(kubernetesfake.NewSimpleClientset(tt.secretsAndConfigMaps...), 0) cache := authncache.New() logger, log := plog.TestLogger(t) diff --git a/internal/controller/authenticator/webhookcachefiller/webhookcachefiller_test.go b/internal/controller/authenticator/webhookcachefiller/webhookcachefiller_test.go index 89ce4cdaa..d0104ec81 100644 --- a/internal/controller/authenticator/webhookcachefiller/webhookcachefiller_test.go +++ b/internal/controller/authenticator/webhookcachefiller/webhookcachefiller_test.go @@ -1,4 +1,4 @@ -// Copyright 2020-2024 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2025 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package webhookcachefiller @@ -27,7 +27,6 @@ import ( "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" k8sinformers "k8s.io/client-go/informers" - kubeinformers "k8s.io/client-go/informers" kubernetesfake "k8s.io/client-go/kubernetes/fake" coretesting "k8s.io/client-go/testing" clocktesting "k8s.io/utils/clock/testing" @@ -2124,7 +2123,7 @@ func TestController(t *testing.T) { tt.configClient(pinnipedAPIClient) } pinnipedInformers := conciergeinformers.NewSharedInformerFactory(pinnipedAPIClient, 0) - kubeInformers := kubeinformers.NewSharedInformerFactory(kubernetesfake.NewSimpleClientset(tt.secretsAndConfigMaps...), 0) + kubeInformers := k8sinformers.NewSharedInformerFactory(kubernetesfake.NewSimpleClientset(tt.secretsAndConfigMaps...), 0) cache := authncache.New() logger, log := plog.TestLogger(t) diff --git a/internal/controller/impersonatorconfig/impersonator_config.go b/internal/controller/impersonatorconfig/impersonator_config.go index 9b215fd55..d1d56b944 100644 --- a/internal/controller/impersonatorconfig/impersonator_config.go +++ b/internal/controller/impersonatorconfig/impersonator_config.go @@ -1,4 +1,4 @@ -// Copyright 2021-2024 the Pinniped contributors. All Rights Reserved. +// Copyright 2021-2025 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package impersonatorconfig @@ -662,7 +662,7 @@ func (c *impersonatorConfigController) createOrUpdateService(ctx context.Context // The Service already exists, so update only the specific fields that are meaningfully part of our desired state. updatedService := existingService.DeepCopy() - updatedService.ObjectMeta.Labels = desiredService.ObjectMeta.Labels + updatedService.Labels = desiredService.Labels updatedService.Spec.LoadBalancerIP = desiredService.Spec.LoadBalancerIP updatedService.Spec.Type = desiredService.Spec.Type updatedService.Spec.Selector = desiredService.Spec.Selector diff --git a/internal/controller/kubecertagent/kubecertagent_test.go b/internal/controller/kubecertagent/kubecertagent_test.go index 817f77440..c6b715300 100644 --- a/internal/controller/kubecertagent/kubecertagent_test.go +++ b/internal/controller/kubecertagent/kubecertagent_test.go @@ -1,4 +1,4 @@ -// Copyright 2021-2024 the Pinniped contributors. All Rights Reserved. +// Copyright 2021-2025 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package kubecertagent @@ -176,7 +176,7 @@ func TestAgentController(t *testing.T) { } healthyAgentDeploymentWithOldStyleSelector := healthyAgentDeployment.DeepCopy() healthyAgentDeploymentWithOldStyleSelector.Spec.Selector = metav1.SetAsLabelSelector(oldStyleLabels) - healthyAgentDeploymentWithOldStyleSelector.Spec.Template.ObjectMeta.Labels = oldStyleLabels + healthyAgentDeploymentWithOldStyleSelector.Spec.Template.Labels = oldStyleLabels healthyAgentDeploymentWithOldStyleSelector.UID = "fake-uid-abc123" // needs UID to test delete options healthyAgentDeploymentWithOldStyleSelector.ResourceVersion = "fake-resource-version-1234" // needs ResourceVersion to test delete options diff --git a/internal/controller/supervisorstorage/garbage_collector.go b/internal/controller/supervisorstorage/garbage_collector.go index 394ddf371..d8b2aeced 100644 --- a/internal/controller/supervisorstorage/garbage_collector.go +++ b/internal/controller/supervisorstorage/garbage_collector.go @@ -1,4 +1,4 @@ -// Copyright 2020-2024 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2025 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package supervisorstorage @@ -118,7 +118,7 @@ func (c *garbageCollectorController) Sync(ctx controllerlib.Context) error { // Sort secrets by name so that audit log tests are deterministic slices.SortStableFunc(listOfSecrets, func(a, b *corev1.Secret) int { - return strings.Compare(a.ObjectMeta.Name, b.ObjectMeta.Name) + return strings.Compare(a.Name, b.Name) }) for i := range listOfSecrets { diff --git a/internal/execcredcache/cachefile.go b/internal/execcredcache/cachefile.go index 12aa6d986..50d785826 100644 --- a/internal/execcredcache/cachefile.go +++ b/internal/execcredcache/cachefile.go @@ -1,4 +1,4 @@ -// Copyright 2021-2022 the Pinniped contributors. All Rights Reserved. +// Copyright 2021-2025 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package execcredcache @@ -67,7 +67,7 @@ func readCache(path string) (*credCache, error) { } // Validate that we're reading a version of the config we understand how to parse. - if !(cache.TypeMeta.APIVersion == apiVersion && cache.TypeMeta.Kind == apiKind) { + if !(cache.TypeMeta.APIVersion == apiVersion && cache.TypeMeta.Kind == apiKind) { //nolint:staticcheck // De Morgan's doesn't make this more readable return nil, fmt.Errorf("%w: %#v", errUnsupportedVersion, cache.TypeMeta) } return &cache, nil diff --git a/internal/federationdomain/endpoints/auth/auth_handler.go b/internal/federationdomain/endpoints/auth/auth_handler.go index 158138985..f55b783c8 100644 --- a/internal/federationdomain/endpoints/auth/auth_handler.go +++ b/internal/federationdomain/endpoints/auth/auth_handler.go @@ -311,7 +311,7 @@ func shouldShowIDPChooser( } func requireStaticClientForUsernameAndPasswordHeaders(authorizeRequester fosite.AuthorizeRequester) error { - if !(authorizeRequester.GetClient().GetID() == oidcapi.ClientIDPinnipedCLI) { + if !(authorizeRequester.GetClient().GetID() == oidcapi.ClientIDPinnipedCLI) { //nolint:staticcheck // De Morgan's doesn't make this more readable return fosite.ErrAccessDenied.WithHint("This client is not allowed to submit username or password headers to this endpoint.") } return nil diff --git a/internal/federationdomain/endpoints/chooseidp/choose_idp_handler.go b/internal/federationdomain/endpoints/chooseidp/choose_idp_handler.go index 6b83f7601..1d9529e55 100644 --- a/internal/federationdomain/endpoints/chooseidp/choose_idp_handler.go +++ b/internal/federationdomain/endpoints/chooseidp/choose_idp_handler.go @@ -1,4 +1,4 @@ -// Copyright 2023-2024 the Pinniped contributors. All Rights Reserved. +// Copyright 2023-2025 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package chooseidp @@ -30,6 +30,7 @@ func NewHandler(authURL string, upstreamIDPs federationdomainproviders.Federatio // This is just a sanity check that it appears to be an authorize request. // Actual enforcement of parameters will happen at the authorization endpoint. query := r.URL.Query() + //nolint:staticcheck // De Morgan's doesn't make this more readable if !(query.Has("client_id") && query.Has("redirect_uri") && query.Has("scope") && query.Has("response_type")) { return httperr.New(http.StatusBadRequest, "missing required query params (must include client_id, redirect_uri, scope, and response_type)") } diff --git a/internal/federationdomain/resolvedprovider/resolvedoidc/resolved_oidc_provider.go b/internal/federationdomain/resolvedprovider/resolvedoidc/resolved_oidc_provider.go index be6ad5836..9cdff65a5 100644 --- a/internal/federationdomain/resolvedprovider/resolvedoidc/resolved_oidc_provider.go +++ b/internal/federationdomain/resolvedprovider/resolvedoidc/resolved_oidc_provider.go @@ -1,4 +1,4 @@ -// Copyright 2024 the Pinniped contributors. All Rights Reserved. +// Copyright 2024-2025 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package resolvedoidc @@ -17,7 +17,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "go.pinniped.dev/generated/latest/apis/supervisor/idpdiscovery/v1alpha1" - "go.pinniped.dev/generated/latest/apis/supervisor/oidc" oidcapi "go.pinniped.dev/generated/latest/apis/supervisor/oidc" "go.pinniped.dev/internal/constable" "go.pinniped.dev/internal/federationdomain/downstreamsubject" @@ -34,7 +33,7 @@ import ( const ( // The name of the email claim from https://openid.net/specs/openid-connect-core-1_0.html#StandardClaims - emailClaimName = oidc.ScopeEmail + emailClaimName = oidcapi.ScopeEmail // The name of the email_verified claim from https://openid.net/specs/openid-connect-core-1_0.html#StandardClaims emailVerifiedClaimName = "email_verified" @@ -234,6 +233,7 @@ func (p *FederationDomainResolvedOIDCIdentityProvider) UpstreamRefresh( accessTokenStored := sessionData.UpstreamAccessToken != "" refreshTokenStored := sessionData.UpstreamRefreshToken != "" + //nolint:staticcheck // De Morgan's doesn't make this more readable exactlyOneTokenStored := (accessTokenStored || refreshTokenStored) && !(accessTokenStored && refreshTokenStored) if !exactlyOneTokenStored { return nil, errorsx.WithStack(resolvedprovider.ErrMissingUpstreamSessionInternalError()) @@ -373,11 +373,11 @@ func makeDownstreamOIDCSessionData( oidcUpstream upstreamprovider.UpstreamOIDCIdentityProviderI, token *oidctypes.Token, ) (*psession.OIDCSessionData, []string, error) { - upstreamSubject, err := extractStringClaimValue(oidc.IDTokenClaimSubject, oidcUpstream.GetResourceName(), token.IDToken.Claims) + upstreamSubject, err := extractStringClaimValue(oidcapi.IDTokenClaimSubject, oidcUpstream.GetResourceName(), token.IDToken.Claims) if err != nil { return nil, nil, err } - upstreamIssuer, err := extractStringClaimValue(oidc.IDTokenClaimIssuer, oidcUpstream.GetResourceName(), token.IDToken.Claims) + upstreamIssuer, err := extractStringClaimValue(oidcapi.IDTokenClaimIssuer, oidcUpstream.GetResourceName(), token.IDToken.Claims) if err != nil { return nil, nil, err } @@ -472,11 +472,11 @@ func getDownstreamSubjectAndUpstreamUsernameFromUpstreamIDToken( ) (string, string, error) { // The spec says the "sub" claim is only unique per issuer, // so we will prepend the issuer string to make it globally unique. - upstreamIssuer, err := extractStringClaimValue(oidc.IDTokenClaimIssuer, upstreamIDPConfig.GetResourceName(), idTokenClaims) + upstreamIssuer, err := extractStringClaimValue(oidcapi.IDTokenClaimIssuer, upstreamIDPConfig.GetResourceName(), idTokenClaims) if err != nil { return "", "", err } - upstreamSubject, err := extractStringClaimValue(oidc.IDTokenClaimSubject, upstreamIDPConfig.GetResourceName(), idTokenClaims) + upstreamSubject, err := extractStringClaimValue(oidcapi.IDTokenClaimSubject, upstreamIDPConfig.GetResourceName(), idTokenClaims) if err != nil { return "", "", err } @@ -554,7 +554,7 @@ func extractStringClaimValue(claimName string, upstreamIDPName string, idTokenCl func mappedUsernameFromUpstreamOIDCSubject(upstreamIssuerAsString string, upstreamSubject string) string { return fmt.Sprintf("%s?%s=%s", upstreamIssuerAsString, - oidc.IDTokenClaimSubject, url.QueryEscape(upstreamSubject), + oidcapi.IDTokenClaimSubject, url.QueryEscape(upstreamSubject), ) } diff --git a/internal/kubeclient/kubeclient_test.go b/internal/kubeclient/kubeclient_test.go index 6207f34be..bc3d65ea1 100644 --- a/internal/kubeclient/kubeclient_test.go +++ b/internal/kubeclient/kubeclient_test.go @@ -1,4 +1,4 @@ -// Copyright 2021-2024 the Pinniped contributors. All Rights Reserved. +// Copyright 2021-2025 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package kubeclient @@ -827,7 +827,7 @@ func wantCloseReqWrapper(t *testing.T) transport.WrapperFunc { wc := &wantCloser{_rc: req.Body} t.Cleanup(func() { require.Eventuallyf(t, func() bool { - return 1 == len(wc.calls()) + return len(wc.calls()) == 1 }, 5*time.Second, 100*time.Millisecond, "did not close req body expected number of times at %s for req %#v; actual calls = %s", caller, req, wc.calls()) }) @@ -843,7 +843,7 @@ func wantCloseReqWrapper(t *testing.T) transport.WrapperFunc { wc := &wantCloser{_rc: originalBodyCopy} t.Cleanup(func() { require.Eventuallyf(t, func() bool { - return 1 == len(wc.calls()) + return len(wc.calls()) == 1 }, 5*time.Second, 100*time.Millisecond, "did not close req body copy expected number of times at %s for req %#v; actual calls = %s", caller, req, wc.calls()) }) @@ -872,7 +872,7 @@ func wantCloseRespWrapper(t *testing.T) transport.WrapperFunc { t.Cleanup(func() { require.Eventuallyf(t, func() bool { return wc.couldRead() == false && - 1 == len(wc.calls()) + len(wc.calls()) == 1 }, 5*time.Second, 10*time.Millisecond, `did not close resp body expected number of times at %s for req %#v; actual calls = %s did not consume all response body bytes before closing %s, couldRead=%v`, caller, req, wc.calls(), caller, wc.couldRead()) diff --git a/internal/kubeclient/watch.go b/internal/kubeclient/watch.go index c4007b9b3..9b9d70b7a 100644 --- a/internal/kubeclient/watch.go +++ b/internal/kubeclient/watch.go @@ -1,4 +1,4 @@ -// Copyright 2021-2022 the Pinniped contributors. All Rights Reserved. +// Copyright 2021-2025 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package kubeclient @@ -68,12 +68,12 @@ func handleWatchResponseNewGVK( }() defer newBodyWriter.Close() - frameReader := serializerInfo.StreamSerializer.Framer.NewFrameReader(resp.Body) + frameReader := serializerInfo.StreamSerializer.NewFrameReader(resp.Body) watchEventDecoder := streaming.NewDecoder(frameReader, serializerInfo.StreamSerializer.Serializer) sourceDecoder = restclientwatch.NewDecoder(watchEventDecoder, &passthroughDecoder{}) defer sourceDecoder.Close() - frameWriter := serializerInfo.StreamSerializer.Framer.NewFrameWriter(newBodyWriter) + frameWriter := serializerInfo.StreamSerializer.NewFrameWriter(newBodyWriter) watchEventEncoder := streaming.NewEncoder(frameWriter, serializerInfo.StreamSerializer.Serializer) for { diff --git a/internal/localuserauthenticator/localuserauthenticator.go b/internal/localuserauthenticator/localuserauthenticator.go index 33ee42b25..fdf2da4b0 100644 --- a/internal/localuserauthenticator/localuserauthenticator.go +++ b/internal/localuserauthenticator/localuserauthenticator.go @@ -1,4 +1,4 @@ -// Copyright 2020-2024 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2025 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 // Package localuserauthenticator provides a authentication webhook program. @@ -151,7 +151,7 @@ func (w *webhook) ServeHTTP(rsp http.ResponseWriter, req *http.Request) { } plog.Debug("successful authentication") - respondWithAuthenticated(rsp, secret.ObjectMeta.Name, groups) + respondWithAuthenticated(rsp, secret.Name, groups) } func getUsernameAndPasswordFromRequest(rsp http.ResponseWriter, req *http.Request) (string, string, error) { diff --git a/internal/plog/config_test.go b/internal/plog/config_test.go index 55208e29f..69bd55140 100644 --- a/internal/plog/config_test.go +++ b/internal/plog/config_test.go @@ -135,8 +135,7 @@ func TestFormat(t *testing.T) { %s/config_test.go:%d testing.tRunner %s/src/testing/testing.go:%d`, - //nolint:staticcheck // runtime.GOROOT() is deprecated but good enough for this unit test. - wd, getLineNumberOfCaller()-20, runtime.GOROOT(), getLineNumberOfCaller(2), + wd, getLineNumberOfCaller()-19, runtime.GOROOT(), getLineNumberOfCaller(2), //nolint:staticcheck // calling a deprecated function is good enough for this unit test ), ), ), scanner.Text()) diff --git a/internal/testutil/oidcclient_test.go b/internal/testutil/oidcclient_test.go index c47a129a0..01caa1bd5 100644 --- a/internal/testutil/oidcclient_test.go +++ b/internal/testutil/oidcclient_test.go @@ -1,4 +1,4 @@ -// Copyright 2022-2023 the Pinniped contributors. All Rights Reserved. +// Copyright 2022-2025 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package testutil @@ -48,7 +48,7 @@ func TestBcryptHashedPassword2TestHelpers(t *testing.T) { requireCost(t, oidcclientvalidator.DefaultMinBcryptCost, HashedPassword2AtSupervisorMinCost) } -func generateHash(t *testing.T, password string, cost int) string { //nolint:unused,deadcode // used in comments above +func generateHash(t *testing.T, password string, cost int) string { //nolint:unused // used in comments above hash, err := bcrypt.GenerateFromPassword([]byte(password), cost) require.NoError(t, err) return string(hash) diff --git a/internal/upstreamldap/upstreamldap.go b/internal/upstreamldap/upstreamldap.go index 9c83044ab..0d599b290 100644 --- a/internal/upstreamldap/upstreamldap.go +++ b/internal/upstreamldap/upstreamldap.go @@ -1,4 +1,4 @@ -// Copyright 2021-2024 the Pinniped contributors. All Rights Reserved. +// Copyright 2021-2025 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 // Package upstreamldap implements an abstraction of upstream LDAP IDP interactions. @@ -291,11 +291,11 @@ func (p *Provider) dial(ctx context.Context) (Conn, error) { // Choose how and where to dial based on TLS vs. StartTLS config option. var dialFunc LDAPDialerFunc var addr endpointaddr.HostPort - switch { - case p.c.ConnectionProtocol == TLS: + switch p.c.ConnectionProtocol { + case TLS: dialFunc = p.dialTLS addr = tlsAddr - case p.c.ConnectionProtocol == StartTLS: + case StartTLS: dialFunc = p.dialStartTLS addr = startTLSAddr default: diff --git a/pkg/oidcclient/filesession/cachefile.go b/pkg/oidcclient/filesession/cachefile.go index 6c9bc7c6e..017175236 100644 --- a/pkg/oidcclient/filesession/cachefile.go +++ b/pkg/oidcclient/filesession/cachefile.go @@ -1,4 +1,4 @@ -// Copyright 2020-2024 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2025 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 // Package filesession implements the file format for session caches. @@ -72,7 +72,7 @@ func readSessionCache(path string) (*sessionCache, error) { } // Validate that we're reading a version of the config we understand how to parse. - if !(cache.TypeMeta.APIVersion == apiVersion && cache.TypeMeta.Kind == apiKind) { + if !(cache.TypeMeta.APIVersion == apiVersion && cache.TypeMeta.Kind == apiKind) { //nolint:staticcheck // De Morgan's doesn't make this more readable return nil, fmt.Errorf("%w: %#v", errUnsupportedVersion, cache.TypeMeta) } return &cache, nil diff --git a/test/integration/concierge_credentialissuer_test.go b/test/integration/concierge_credentialissuer_test.go index a7b401dad..704f7a2e2 100644 --- a/test/integration/concierge_credentialissuer_test.go +++ b/test/integration/concierge_credentialissuer_test.go @@ -1,4 +1,4 @@ -// Copyright 2020-2024 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2025 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package integration @@ -73,7 +73,7 @@ func TestCredentialIssuer(t *testing.T) { require.Equal(t, conciergeconfigv1alpha1.TokenCredentialRequestAPIFrontendType, actualStatusStrategy.Frontend.Type) expectedTokenRequestAPIInfo := conciergeconfigv1alpha1.TokenCredentialRequestAPIInfo{ Server: config.Host, - CertificateAuthorityData: base64.StdEncoding.EncodeToString(config.TLSClientConfig.CAData), + CertificateAuthorityData: base64.StdEncoding.EncodeToString(config.CAData), } require.Equal(t, &expectedTokenRequestAPIInfo, actualStatusStrategy.Frontend.TokenCredentialRequestAPIInfo) } else { diff --git a/test/integration/concierge_impersonation_proxy_test.go b/test/integration/concierge_impersonation_proxy_test.go index 4ea8525fc..6c1d65749 100644 --- a/test/integration/concierge_impersonation_proxy_test.go +++ b/test/integration/concierge_impersonation_proxy_test.go @@ -1,4 +1,4 @@ -// Copyright 2020-2024 the Pinniped contributors. All Rights Reserved. +// Copyright 2020-2025 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package integration @@ -1587,6 +1587,7 @@ func TestImpersonationProxy(t *testing.T) { //nolint:gocyclo // yeah, it's compl }) t.Run("adding an annotation reconciles the LoadBalancer service", func(t *testing.T) { + //nolint:staticcheck // De Morgan's doesn't make this more readable if !(impersonatorShouldHaveStartedAutomaticallyByDefault && clusterSupportsLoadBalancers) { t.Skip("only running when the cluster is meant to be using LoadBalancer services") } @@ -2124,9 +2125,9 @@ func performImpersonatorDiscovery(ctx context.Context, t *testing.T, env *testli config := newImpersonationProxyConfigWithCredentials(t, credentials, impersonationProxyURL, impersonationProxyCACertPEM, nil) config = rest.CopyConfig(config) - config.Proxy = kubeconfigProxyFunc(t, env.Proxy) // always use the proxy since we are talking directly to a pod IP - config.Host = "https://" + pod.Status.PodIP + ":8444" // hardcode the internal port - it should not change - config.TLSClientConfig.ServerName = impersonationProxyParsedURL.Hostname() // make SNI hostname TLS verification work even when using IP + config.Proxy = kubeconfigProxyFunc(t, env.Proxy) // always use the proxy since we are talking directly to a pod IP + config.Host = "https://" + pod.Status.PodIP + ":8444" // hardcode the internal port - it should not change + config.ServerName = impersonationProxyParsedURL.Hostname() // make SNI hostname TLS verification work even when using IP whoAmI, err := testlib.NewKubeclient(t, config).PinnipedConcierge.IdentityV1alpha1().WhoAmIRequests(). Create(ctx, &identityv1alpha1.WhoAmIRequest{}, metav1.CreateOptions{}) diff --git a/test/integration/supervisor_oidcclientsecret_test.go b/test/integration/supervisor_oidcclientsecret_test.go index 34ff16f5c..47f22a39e 100644 --- a/test/integration/supervisor_oidcclientsecret_test.go +++ b/test/integration/supervisor_oidcclientsecret_test.go @@ -1,4 +1,4 @@ -// Copyright 2022-2024 the Pinniped contributors. All Rights Reserved. +// Copyright 2022-2025 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package integration @@ -937,11 +937,11 @@ func TestCreateOIDCClientSecretRequest_Parallel(t *testing.T) { } else { require.NoError(t, err) - require.Equal(t, ttt.secretRequest.ObjectMeta.Name, clientSecretRequestResponse.ObjectMeta.Name, + require.Equal(t, ttt.secretRequest.Name, clientSecretRequestResponse.Name, "name in response should match name in sent request") - require.Equal(t, ttt.secretRequest.ObjectMeta.Namespace, clientSecretRequestResponse.ObjectMeta.Namespace, + require.Equal(t, ttt.secretRequest.Namespace, clientSecretRequestResponse.Namespace, "namespace in response should match namespace in sent request") - testutil.RequireTimeInDelta(t, clientSecretRequestResponse.ObjectMeta.CreationTimestamp.Time, time.Now(), 1*time.Minute) + testutil.RequireTimeInDelta(t, clientSecretRequestResponse.CreationTimestamp.Time, time.Now(), 1*time.Minute) require.Equalf(t, ttt.secretRequest.TypeMeta, clientSecretRequestResponse.TypeMeta, "type meta of response should match the sent request") diff --git a/test/testlib/assertions.go b/test/testlib/assertions.go index f6bae9bff..eab1849fa 100644 --- a/test/testlib/assertions.go +++ b/test/testlib/assertions.go @@ -1,4 +1,4 @@ -// Copyright 2021-2024 the Pinniped contributors. All Rights Reserved. +// Copyright 2021-2025 the Pinniped contributors. All Rights Reserved. // SPDX-License-Identifier: Apache-2.0 package testlib @@ -227,7 +227,7 @@ func getRestartCounts(ctx context.Context, t *testing.T, kubeClient kubernetes.I // Ignore pods that are already terminating at the start of the test. The app may have been redeployed // just before the tests were invoked, so it would be normal for some pods to still be terminating // in that situation. Note that terminating pods in this situation do not count as a "restart" anyway. - if pod.ObjectMeta.DeletionTimestamp != nil { + if pod.DeletionTimestamp != nil { continue } diff --git a/test/testlib/browsertest/browsertest.go b/test/testlib/browsertest/browsertest.go index 955b7ef61..2a97adb30 100644 --- a/test/testlib/browsertest/browsertest.go +++ b/test/testlib/browsertest/browsertest.go @@ -115,7 +115,7 @@ func OpenBrowser(t *testing.T) *Browser { args := make([]string, len(ev.Args)) for i, arg := range ev.Args { // Could also pay attention to arg.Type here, but choosing to keep it simple for now. - args[i] = fmt.Sprintf("%s", arg.Value) //nolint:gosimple // this is an acceptable way to get a string + args[i] = arg.Value.String() } b.lock.Lock() defer b.lock.Unlock()