From b4935d910dcf256288694391ef675acfbdb8e7a3 Mon Sep 17 00:00:00 2001 From: Monis Khan Date: Wed, 10 Jan 2024 12:36:55 -0500 Subject: [PATCH] Add dynamic reload support for authentication configuration Signed-off-by: Monis Khan --- pkg/controlplane/apiserver/config.go | 6 +- pkg/kubeapiserver/authenticator/config.go | 148 +++++-- pkg/kubeapiserver/options/authentication.go | 93 +++- .../options/authentication_test.go | 68 ++- .../pkg/authenticator/token/oidc/metrics.go | 12 +- .../authenticator/token/oidc/metrics_test.go | 6 +- .../pkg/authenticator/token/oidc/oidc.go | 122 +++-- .../pkg/authenticator/token/oidc/oidc_test.go | 87 +++- test/integration/apiserver/oidc/oidc_test.go | 418 +++++++++++++++++- 9 files changed, 855 insertions(+), 105 deletions(-) diff --git a/pkg/controlplane/apiserver/config.go b/pkg/controlplane/apiserver/config.go index 35ec3db5936..a0134b16f4a 100644 --- a/pkg/controlplane/apiserver/config.go +++ b/pkg/controlplane/apiserver/config.go @@ -144,14 +144,16 @@ func BuildGenericConfig( return } + ctx := wait.ContextForChannel(genericConfig.DrainedNotify()) + // Authentication.ApplyTo requires already applied OpenAPIConfig and EgressSelector if present - if lastErr = s.Authentication.ApplyTo(&genericConfig.Authentication, genericConfig.SecureServing, genericConfig.EgressSelector, genericConfig.OpenAPIConfig, genericConfig.OpenAPIV3Config, clientgoExternalClient, versionedInformers); lastErr != nil { + if lastErr = s.Authentication.ApplyTo(ctx, &genericConfig.Authentication, genericConfig.SecureServing, genericConfig.EgressSelector, genericConfig.OpenAPIConfig, genericConfig.OpenAPIV3Config, clientgoExternalClient, versionedInformers); lastErr != nil { return } var enablesRBAC bool genericConfig.Authorization.Authorizer, genericConfig.RuleResolver, enablesRBAC, err = BuildAuthorizer( - wait.ContextForChannel(genericConfig.ShutdownInitiatedNotify()), + ctx, s, genericConfig.EgressSelector, genericConfig.APIServerID, diff --git a/pkg/kubeapiserver/authenticator/config.go b/pkg/kubeapiserver/authenticator/config.go index c931b47c70c..bd7ed0e1496 100644 --- a/pkg/kubeapiserver/authenticator/config.go +++ b/pkg/kubeapiserver/authenticator/config.go @@ -17,10 +17,13 @@ limitations under the License. package authenticator import ( + "context" "errors" "fmt" + "sync/atomic" "time" + utilerrors "k8s.io/apimachinery/pkg/util/errors" utilnet "k8s.io/apimachinery/pkg/util/net" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/apiserver/pkg/apis/apiserver" @@ -57,6 +60,7 @@ type Config struct { TokenAuthFile string AuthenticationConfig *apiserver.AuthenticationConfiguration + AuthenticationConfigData string OIDCSigningAlgs []string ServiceAccountKeyFiles []string ServiceAccountLookup bool @@ -90,7 +94,7 @@ type Config struct { // New returns an authenticator.Request or an error that supports the standard // Kubernetes authentication mechanisms. -func (config Config) New() (authenticator.Request, *spec.SecurityDefinitions, spec3.SecuritySchemes, error) { +func (config Config) New(serverLifecycle context.Context) (authenticator.Request, func(context.Context, *apiserver.AuthenticationConfiguration) error, *spec.SecurityDefinitions, spec3.SecuritySchemes, error) { var authenticators []authenticator.Request var tokenAuthenticators []authenticator.Token securityDefinitionsV2 := spec.SecurityDefinitions{} @@ -119,21 +123,21 @@ func (config Config) New() (authenticator.Request, *spec.SecurityDefinitions, sp if len(config.TokenAuthFile) > 0 { tokenAuth, err := newAuthenticatorFromTokenFile(config.TokenAuthFile) if err != nil { - return nil, nil, nil, err + return nil, nil, nil, nil, err } tokenAuthenticators = append(tokenAuthenticators, authenticator.WrapAudienceAgnosticToken(config.APIAudiences, tokenAuth)) } if len(config.ServiceAccountKeyFiles) > 0 { serviceAccountAuth, err := newLegacyServiceAccountAuthenticator(config.ServiceAccountKeyFiles, config.ServiceAccountLookup, config.APIAudiences, config.ServiceAccountTokenGetter, config.SecretsWriter) if err != nil { - return nil, nil, nil, err + return nil, nil, nil, nil, err } tokenAuthenticators = append(tokenAuthenticators, serviceAccountAuth) } if len(config.ServiceAccountIssuers) > 0 { serviceAccountAuth, err := newServiceAccountAuthenticator(config.ServiceAccountIssuers, config.ServiceAccountKeyFiles, config.APIAudiences, config.ServiceAccountTokenGetter) if err != nil { - return nil, nil, nil, err + return nil, nil, nil, nil, err } tokenAuthenticators = append(tokenAuthenticators, serviceAccountAuth) } @@ -148,33 +152,33 @@ func (config Config) New() (authenticator.Request, *spec.SecurityDefinitions, sp // cache misses for all requests using the other. While the service account plugin // simply returns an error, the OpenID Connect plugin may query the provider to // update the keys, causing performance hits. + var updateAuthenticationConfig func(context.Context, *apiserver.AuthenticationConfiguration) error if config.AuthenticationConfig != nil { - for _, jwtAuthenticator := range config.AuthenticationConfig.JWT { - var oidcCAContent oidc.CAContentProvider - if len(jwtAuthenticator.Issuer.CertificateAuthority) > 0 { - var oidcCAError error - oidcCAContent, oidcCAError = dynamiccertificates.NewStaticCAContent("oidc-authenticator", []byte(jwtAuthenticator.Issuer.CertificateAuthority)) - if oidcCAError != nil { - return nil, nil, nil, oidcCAError - } - } - oidcAuth, err := oidc.New(oidc.Options{ - JWTAuthenticator: jwtAuthenticator, - CAContentProvider: oidcCAContent, - SupportedSigningAlgs: config.OIDCSigningAlgs, - DisallowedIssuers: config.ServiceAccountIssuers, - }) - if err != nil { - return nil, nil, nil, err - } - tokenAuthenticators = append(tokenAuthenticators, authenticator.WrapAudienceAgnosticToken(config.APIAudiences, oidcAuth)) + initialJWTAuthenticator, err := newJWTAuthenticator(serverLifecycle, config.AuthenticationConfig, config.OIDCSigningAlgs, config.APIAudiences, config.ServiceAccountIssuers) + if err != nil { + return nil, nil, nil, nil, err } + + jwtAuthenticatorPtr := &atomic.Pointer[jwtAuthenticatorWithCancel]{} + jwtAuthenticatorPtr.Store(initialJWTAuthenticator) + + updateAuthenticationConfig = (&authenticationConfigUpdater{ + serverLifecycle: serverLifecycle, + config: config, + jwtAuthenticatorPtr: jwtAuthenticatorPtr, + }).updateAuthenticationConfig + + tokenAuthenticators = append(tokenAuthenticators, + authenticator.TokenFunc(func(ctx context.Context, token string) (*authenticator.Response, bool, error) { + return jwtAuthenticatorPtr.Load().jwtAuthenticator.AuthenticateToken(ctx, token) + }), + ) } if len(config.WebhookTokenAuthnConfigFile) > 0 { webhookTokenAuth, err := newWebhookTokenAuthenticator(config) if err != nil { - return nil, nil, nil, err + return nil, nil, nil, nil, err } tokenAuthenticators = append(tokenAuthenticators, webhookTokenAuth) @@ -209,9 +213,9 @@ func (config Config) New() (authenticator.Request, *spec.SecurityDefinitions, sp if len(authenticators) == 0 { if config.Anonymous { - return anonymous.NewAuthenticator(), &securityDefinitionsV2, securitySchemesV3, nil + return anonymous.NewAuthenticator(), nil, &securityDefinitionsV2, securitySchemesV3, nil } - return nil, &securityDefinitionsV2, securitySchemesV3, nil + return nil, nil, &securityDefinitionsV2, securitySchemesV3, nil } authenticator := union.New(authenticators...) @@ -224,7 +228,97 @@ func (config Config) New() (authenticator.Request, *spec.SecurityDefinitions, sp authenticator = union.NewFailOnError(authenticator, anonymous.NewAuthenticator()) } - return authenticator, &securityDefinitionsV2, securitySchemesV3, nil + return authenticator, updateAuthenticationConfig, &securityDefinitionsV2, securitySchemesV3, nil +} + +type jwtAuthenticatorWithCancel struct { + jwtAuthenticator authenticator.Token + healthCheck func() error + cancel func() +} + +func newJWTAuthenticator(serverLifecycle context.Context, config *apiserver.AuthenticationConfiguration, oidcSigningAlgs []string, apiAudiences authenticator.Audiences, disallowedIssuers []string) (_ *jwtAuthenticatorWithCancel, buildErr error) { + ctx, cancel := context.WithCancel(serverLifecycle) + + defer func() { + if buildErr != nil { + cancel() + } + }() + var jwtAuthenticators []authenticator.Token + var healthChecks []func() error + for _, jwtAuthenticator := range config.JWT { + // TODO remove this CAContentProvider indirection + var oidcCAContent oidc.CAContentProvider + if len(jwtAuthenticator.Issuer.CertificateAuthority) > 0 { + var oidcCAError error + oidcCAContent, oidcCAError = dynamiccertificates.NewStaticCAContent("oidc-authenticator", []byte(jwtAuthenticator.Issuer.CertificateAuthority)) + if oidcCAError != nil { + return nil, oidcCAError + } + } + oidcAuth, err := oidc.New(ctx, oidc.Options{ + JWTAuthenticator: jwtAuthenticator, + CAContentProvider: oidcCAContent, + SupportedSigningAlgs: oidcSigningAlgs, + DisallowedIssuers: disallowedIssuers, + }) + if err != nil { + return nil, err + } + jwtAuthenticators = append(jwtAuthenticators, oidcAuth) + healthChecks = append(healthChecks, oidcAuth.HealthCheck) + } + return &jwtAuthenticatorWithCancel{ + jwtAuthenticator: authenticator.WrapAudienceAgnosticToken(apiAudiences, tokenunion.NewFailOnError(jwtAuthenticators...)), // this handles the empty jwtAuthenticators slice case correctly + healthCheck: func() error { + var errs []error + for _, check := range healthChecks { + if err := check(); err != nil { + errs = append(errs, err) + } + } + return utilerrors.NewAggregate(errs) + }, + cancel: cancel, + }, nil +} + +type authenticationConfigUpdater struct { + serverLifecycle context.Context + config Config + jwtAuthenticatorPtr *atomic.Pointer[jwtAuthenticatorWithCancel] +} + +// the input ctx controls the timeout for updateAuthenticationConfig to return, not the lifetime of the constructed authenticators. +func (c *authenticationConfigUpdater) updateAuthenticationConfig(ctx context.Context, authConfig *apiserver.AuthenticationConfiguration) error { + updatedJWTAuthenticator, err := newJWTAuthenticator(c.serverLifecycle, authConfig, c.config.OIDCSigningAlgs, c.config.APIAudiences, c.config.ServiceAccountIssuers) + if err != nil { + return err + } + + var lastErr error + if waitErr := wait.PollUntilContextCancel(ctx, 10*time.Second, true, func(_ context.Context) (done bool, err error) { + lastErr = updatedJWTAuthenticator.healthCheck() + return lastErr == nil, nil + }); lastErr != nil || waitErr != nil { + updatedJWTAuthenticator.cancel() + return utilerrors.NewAggregate([]error{lastErr, waitErr}) // filters out nil errors + } + + oldJWTAuthenticator := c.jwtAuthenticatorPtr.Swap(updatedJWTAuthenticator) + go func() { + t := time.NewTimer(time.Minute) + defer t.Stop() + select { + case <-c.serverLifecycle.Done(): + case <-t.C: + } + // TODO maybe track requests so we know when this is safe to do + oldJWTAuthenticator.cancel() + }() + + return nil } // IsValidServiceAccountKeyFile returns true if a valid public RSA key can be read from the given file diff --git a/pkg/kubeapiserver/options/authentication.go b/pkg/kubeapiserver/options/authentication.go index cb431ee4df9..6d5e684660c 100644 --- a/pkg/kubeapiserver/options/authentication.go +++ b/pkg/kubeapiserver/options/authentication.go @@ -17,11 +17,13 @@ limitations under the License. package options import ( + "context" "errors" "fmt" "net/url" "os" "strings" + "sync" "time" "github.com/spf13/pflag" @@ -51,6 +53,7 @@ import ( "k8s.io/kubernetes/pkg/features" kubeauthenticator "k8s.io/kubernetes/pkg/kubeapiserver/authenticator" authzmodes "k8s.io/kubernetes/pkg/kubeapiserver/authorizer/modes" + "k8s.io/kubernetes/pkg/util/filesystem" "k8s.io/kubernetes/plugin/pkg/auth/authenticator/token/bootstrap" "k8s.io/utils/pointer" ) @@ -67,6 +70,10 @@ const ( oidcRequiredClaimFlag = "oidc-required-claim" ) +// UpdateAuthenticationConfigTimeout controls how long we wait for calls to updateAuthenticationConfig to succeed. +// Exported as a variable so that it can be overridden in integration tests. +var UpdateAuthenticationConfigTimeout = time.Minute + // BuiltInAuthenticationOptions contains all build-in authentication options for API Server type BuiltInAuthenticationOptions struct { APIAudiences []string @@ -464,7 +471,7 @@ func (o *BuiltInAuthenticationOptions) ToAuthenticationConfig() (kubeauthenticat // load the authentication config from the file. if len(o.AuthenticationConfigFile) > 0 { var err error - if ret.AuthenticationConfig, err = loadAuthenticationConfig(o.AuthenticationConfigFile); err != nil { + if ret.AuthenticationConfig, ret.AuthenticationConfigData, err = loadAuthenticationConfig(o.AuthenticationConfigFile); err != nil { return kubeauthenticator.Config{}, err } // all known signing algs are allowed when using authentication config @@ -580,7 +587,8 @@ func (o *BuiltInAuthenticationOptions) ToAuthenticationConfig() (kubeauthenticat } // ApplyTo requires already applied OpenAPIConfig and EgressSelector if present. -func (o *BuiltInAuthenticationOptions) ApplyTo(authInfo *genericapiserver.AuthenticationInfo, secureServing *genericapiserver.SecureServingInfo, egressSelector *egressselector.EgressSelector, openAPIConfig *openapicommon.Config, openAPIV3Config *openapicommon.OpenAPIV3Config, extclient kubernetes.Interface, versionedInformer informers.SharedInformerFactory) error { +// The input context controls the lifecycle of background goroutines started to reload the authentication config file. +func (o *BuiltInAuthenticationOptions) ApplyTo(ctx context.Context, authInfo *genericapiserver.AuthenticationInfo, secureServing *genericapiserver.SecureServingInfo, egressSelector *egressselector.EgressSelector, openAPIConfig *openapicommon.Config, openAPIV3Config *openapicommon.OpenAPIV3Config, extclient kubernetes.Interface, versionedInformer informers.SharedInformerFactory) error { if o == nil { return nil } @@ -639,11 +647,68 @@ func (o *BuiltInAuthenticationOptions) ApplyTo(authInfo *genericapiserver.Authen } // var openAPIV3SecuritySchemes spec3.SecuritySchemes - authenticator, openAPIV2SecurityDefinitions, openAPIV3SecuritySchemes, err := authenticatorConfig.New() + authenticator, updateAuthenticationConfig, openAPIV2SecurityDefinitions, openAPIV3SecuritySchemes, err := authenticatorConfig.New(ctx) if err != nil { return err } authInfo.Authenticator = authenticator + + if len(o.AuthenticationConfigFile) > 0 { + trackedAuthenticationConfigData := authenticatorConfig.AuthenticationConfigData + var mu sync.Mutex + go filesystem.WatchUntil( + ctx, + time.Minute, + o.AuthenticationConfigFile, + func() { + // TODO add metrics + // TODO collapse onto shared logic with DynamicEncryptionConfigContent controller + + mu.Lock() + defer mu.Unlock() + + authConfigBytes, err := os.ReadFile(o.AuthenticationConfigFile) + if err != nil { + klog.ErrorS(err, "failed to read authentication config file") + // we do not update the tracker here because this error could eventually resolve as we keep retrying + return + } + + authConfigData := string(authConfigBytes) + + if authConfigData == trackedAuthenticationConfigData { + return + } + + authConfig, err := loadAuthenticationConfigFromData(authConfigBytes) + if err != nil { + klog.ErrorS(err, "failed to load authentication config") + // this config is not structurally valid and never will be, update the tracker so we stop retrying + trackedAuthenticationConfigData = authConfigData + return + } + + if err := apiservervalidation.ValidateAuthenticationConfiguration(authConfig, authenticatorConfig.ServiceAccountIssuers).ToAggregate(); err != nil { + klog.ErrorS(err, "failed to validate authentication config") + // this config is not semantically valid and never will be, update the tracker so we stop retrying + trackedAuthenticationConfigData = authConfigData + return + } + + timeoutCtx, timeoutCancel := context.WithTimeout(ctx, UpdateAuthenticationConfigTimeout) + defer timeoutCancel() + if err := updateAuthenticationConfig(timeoutCtx, authConfig); err != nil { + klog.ErrorS(err, "failed to update authentication config") + // we do not update the tracker here because this error could eventually resolve as we keep retrying + return + } + + trackedAuthenticationConfigData = authConfigData + }, + func(err error) { klog.ErrorS(err, "watching authentication config file") }, + ) + } + openAPIConfig.SecurityDefinitions = openAPIV2SecurityDefinitions if openAPIV3Config != nil { openAPIV3Config.SecuritySchemes = openAPIV3SecuritySchemes @@ -701,15 +766,24 @@ func init() { install.Install(cfgScheme) } -// loadAuthenticationConfig parses the authentication configuration from the given file and returns it. -func loadAuthenticationConfig(configFilePath string) (*apiserver.AuthenticationConfiguration, error) { - // read from file +// loadAuthenticationConfig parses the authentication configuration from the given file and returns it and the file's contents. +func loadAuthenticationConfig(configFilePath string) (*apiserver.AuthenticationConfiguration, string, error) { data, err := os.ReadFile(configFilePath) if err != nil { - return nil, err + return nil, "", err } + + configuration, err := loadAuthenticationConfigFromData(data) + if err != nil { + return nil, "", err + } + + return configuration, string(data), nil +} + +func loadAuthenticationConfigFromData(data []byte) (*apiserver.AuthenticationConfiguration, error) { if len(data) == 0 { - return nil, fmt.Errorf("empty config file %q", configFilePath) + return nil, fmt.Errorf("empty config data") } decodedObj, err := runtime.Decode(codecs.UniversalDecoder(), data) @@ -720,6 +794,9 @@ func loadAuthenticationConfig(configFilePath string) (*apiserver.AuthenticationC if !ok { return nil, fmt.Errorf("expected AuthenticationConfiguration, got %T", decodedObj) } + if configuration == nil { // sanity check, this should never happen but check just in case since we rely on it + return nil, fmt.Errorf("expected non-nil AuthenticationConfiguration") + } return configuration, nil } diff --git a/pkg/kubeapiserver/options/authentication_test.go b/pkg/kubeapiserver/options/authentication_test.go index b2f938b92bb..687a502d7b1 100644 --- a/pkg/kubeapiserver/options/authentication_test.go +++ b/pkg/kubeapiserver/options/authentication_test.go @@ -691,6 +691,18 @@ jwt: }, }, }, + AuthenticationConfigData: ` +apiVersion: apiserver.config.k8s.io/v1alpha1 +kind: AuthenticationConfiguration +jwt: +- issuer: + url: https://test-issuer + audiences: [ "🐼" ] + claimMappings: + username: + claim: sub + prefix: "" +`, OIDCSigningAlgs: []string{"ES256", "ES384", "ES512", "PS256", "PS384", "PS512", "RS256", "RS384", "RS512"}, }, }, @@ -888,15 +900,16 @@ func TestValidateOIDCOptions(t *testing.T) { func TestLoadAuthenticationConfig(t *testing.T) { testCases := []struct { - name string - file func() string - expectErr string - expectedConfig *apiserver.AuthenticationConfiguration + name string + file func() string + expectErr string + expectedConfig *apiserver.AuthenticationConfiguration + expectedContentData string }{ { name: "empty file", file: func() string { return writeTempFile(t, ``) }, - expectErr: "empty config file", + expectErr: "empty config data", expectedConfig: nil, }, { @@ -916,6 +929,10 @@ func TestLoadAuthenticationConfig(t *testing.T) { }, }, }, + expectedContentData: `{ + "apiVersion":"apiserver.config.k8s.io/v1alpha1", + "kind":"AuthenticationConfiguration", + "jwt":[{"issuer":{"url": "https://test-issuer"}}]}`, }, { name: "missing file", @@ -989,6 +1006,10 @@ func TestLoadAuthenticationConfig(t *testing.T) { }, }, }, + expectedContentData: `{ + "apiVersion":"apiserver.config.k8s.io/v1alpha1", + "kind":"AuthenticationConfiguration", + "jwt":[{"issuer":{"url": "https://test-issuer"}}]}`, }, { name: "v1alpha1 - yaml", @@ -1020,6 +1041,17 @@ jwt: }, }, }, + expectedContentData: ` +apiVersion: apiserver.config.k8s.io/v1alpha1 +kind: AuthenticationConfiguration +jwt: +- issuer: + url: https://test-issuer + claimMappings: + username: + claim: sub + prefix: "" +`, }, { name: "v1alpha1 - no jwt", @@ -1029,6 +1061,9 @@ jwt: "kind":"AuthenticationConfiguration"}`) }, expectedConfig: &apiserver.AuthenticationConfiguration{}, + expectedContentData: `{ + "apiVersion":"apiserver.config.k8s.io/v1alpha1", + "kind":"AuthenticationConfiguration"}`, }, { name: "v1beta1 - json", @@ -1047,6 +1082,10 @@ jwt: }, }, }, + expectedContentData: `{ + "apiVersion":"apiserver.config.k8s.io/v1beta1", + "kind":"AuthenticationConfiguration", + "jwt":[{"issuer":{"url": "https://test-issuer"}}]}`, }, { name: "v1beta1 - yaml", @@ -1078,6 +1117,17 @@ jwt: }, }, }, + expectedContentData: ` +apiVersion: apiserver.config.k8s.io/v1beta1 +kind: AuthenticationConfiguration +jwt: +- issuer: + url: https://test-issuer + claimMappings: + username: + claim: sub + prefix: "" +`, }, { name: "v1beta1 - no jwt", @@ -1087,18 +1137,24 @@ jwt: "kind":"AuthenticationConfiguration"}`) }, expectedConfig: &apiserver.AuthenticationConfiguration{}, + expectedContentData: `{ + "apiVersion":"apiserver.config.k8s.io/v1beta1", + "kind":"AuthenticationConfiguration"}`, }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - config, err := loadAuthenticationConfig(tc.file()) + config, contentData, err := loadAuthenticationConfig(tc.file()) if !strings.Contains(errString(err), tc.expectErr) { t.Fatalf("expected error %q, got %v", tc.expectErr, err) } if !reflect.DeepEqual(config, tc.expectedConfig) { t.Fatalf("unexpected config:\n%s", cmp.Diff(tc.expectedConfig, config)) } + if contentData != tc.expectedContentData { + t.Errorf("unexpected content data: want=%q, got=%q", tc.expectedContentData, contentData) + } }) } } diff --git a/staging/src/k8s.io/apiserver/plugin/pkg/authenticator/token/oidc/metrics.go b/staging/src/k8s.io/apiserver/plugin/pkg/authenticator/token/oidc/metrics.go index 66c6254ec81..109cde254ed 100644 --- a/staging/src/k8s.io/apiserver/plugin/pkg/authenticator/token/oidc/metrics.go +++ b/staging/src/k8s.io/apiserver/plugin/pkg/authenticator/token/oidc/metrics.go @@ -20,13 +20,13 @@ import ( "context" "crypto/sha256" "fmt" - "k8s.io/utils/clock" "sync" "time" "k8s.io/apiserver/pkg/authentication/authenticator" "k8s.io/component-base/metrics" "k8s.io/component-base/metrics/legacyregistry" + "k8s.io/utils/clock" ) const ( @@ -68,11 +68,11 @@ func getHash(data string) string { return "" } -func newInstrumentedAuthenticator(jwtIssuer string, delegate authenticator.Token) authenticator.Token { +func newInstrumentedAuthenticator(jwtIssuer string, delegate AuthenticatorTokenWithHealthCheck) AuthenticatorTokenWithHealthCheck { return newInstrumentedAuthenticatorWithClock(jwtIssuer, delegate, clock.RealClock{}) } -func newInstrumentedAuthenticatorWithClock(jwtIssuer string, delegate authenticator.Token, clock clock.PassiveClock) *instrumentedAuthenticator { +func newInstrumentedAuthenticatorWithClock(jwtIssuer string, delegate AuthenticatorTokenWithHealthCheck, clock clock.PassiveClock) *instrumentedAuthenticator { RegisterMetrics() return &instrumentedAuthenticator{ jwtIssuerHash: getHash(jwtIssuer), @@ -83,7 +83,7 @@ func newInstrumentedAuthenticatorWithClock(jwtIssuer string, delegate authentica type instrumentedAuthenticator struct { jwtIssuerHash string - delegate authenticator.Token + delegate AuthenticatorTokenWithHealthCheck clock clock.PassiveClock } @@ -104,3 +104,7 @@ func (a *instrumentedAuthenticator) AuthenticateToken(ctx context.Context, token } return response, ok, err } + +func (a *instrumentedAuthenticator) HealthCheck() error { + return a.delegate.HealthCheck() +} diff --git a/staging/src/k8s.io/apiserver/plugin/pkg/authenticator/token/oidc/metrics_test.go b/staging/src/k8s.io/apiserver/plugin/pkg/authenticator/token/oidc/metrics_test.go index 998b1846693..cdcad1d05ef 100644 --- a/staging/src/k8s.io/apiserver/plugin/pkg/authenticator/token/oidc/metrics_test.go +++ b/staging/src/k8s.io/apiserver/plugin/pkg/authenticator/token/oidc/metrics_test.go @@ -35,7 +35,7 @@ const ( func TestRecordAuthenticationLatency(t *testing.T) { tests := []struct { name string - authenticator authenticator.Token + authenticator AuthenticatorTokenWithHealthCheck generateMetrics func() expectedValue string }{ @@ -117,6 +117,10 @@ func (a *dummyAuthenticator) AuthenticateToken(ctx context.Context, token string return a.response, a.ok, a.err } +func (a *dummyAuthenticator) HealthCheck() error { + panic("should not be called") +} + type dummyClock struct { } diff --git a/staging/src/k8s.io/apiserver/plugin/pkg/authenticator/token/oidc/oidc.go b/staging/src/k8s.io/apiserver/plugin/pkg/authenticator/token/oidc/oidc.go index 0ce213d9f0e..bdf50d89c3e 100644 --- a/staging/src/k8s.io/apiserver/plugin/pkg/authenticator/token/oidc/oidc.go +++ b/staging/src/k8s.io/apiserver/plugin/pkg/authenticator/token/oidc/oidc.go @@ -74,7 +74,9 @@ const ( type Options struct { // JWTAuthenticator is the authenticator that will be used to verify the JWT. JWTAuthenticator apiserver.JWTAuthenticator + // Optional KeySet to allow for synchronous initialization instead of fetching from the remote issuer. + // Mutually exclusive with JWTAuthenticator.Issuer.DiscoveryURL. KeySet oidc.KeySet // PEM encoded root certificate contents of the provider. Mutually exclusive with Client. @@ -135,7 +137,7 @@ func newAsyncIDTokenVerifier(ctx context.Context, c *oidc.Config, iss string, au sync := make(chan struct{}) // Polls indefinitely in an attempt to initialize the distributed claims // verifier, or until context canceled. - initFn := func() (done bool, err error) { + initFn := func(ctx context.Context) (done bool, err error) { klog.V(4).Infof("oidc authenticator: attempting init: iss=%v", iss) v, err := initVerifier(ctx, c, iss, audiences) if err != nil { @@ -150,13 +152,14 @@ func newAsyncIDTokenVerifier(ctx context.Context, c *oidc.Config, iss string, au } go func() { - if done, _ := initFn(); !done { - go wait.PollUntil(time.Second*10, initFn, ctx.Done()) - } + _ = wait.PollUntilContextCancel(ctx, 10*time.Second, true, initFn) }() if synchronizeTokenIDVerifierForTest { - <-sync + select { + case <-sync: + case <-ctx.Done(): + } } return t @@ -169,15 +172,13 @@ func (a *asyncIDTokenVerifier) verifier() *idTokenVerifier { return a.v } -type Authenticator struct { +type jwtAuthenticator struct { jwtAuthenticator apiserver.JWTAuthenticator // Contains an *oidc.IDTokenVerifier. Do not access directly use the // idTokenVerifier method. verifier atomic.Value - cancel context.CancelFunc - // resolver is used to resolve distributed claims. resolver *claimResolver @@ -187,6 +188,8 @@ type Authenticator struct { // requiredClaims contains the list of claims that must be present in the token. requiredClaims map[string]string + + healthCheck atomic.Pointer[errorHolder] } // idTokenVerifier is a wrapper around oidc.IDTokenVerifier. It uses the oidc.IDTokenVerifier @@ -196,21 +199,22 @@ type idTokenVerifier struct { audiences sets.Set[string] } -func (a *Authenticator) setVerifier(v *idTokenVerifier) { +func (a *jwtAuthenticator) setVerifier(v *idTokenVerifier) { a.verifier.Store(v) + if v != nil { + // this must be done after the verifier has been stored so that a nil error + // from HealthCheck always means that the authenticator is ready for use. + a.healthCheck.Store(&errorHolder{}) + } } -func (a *Authenticator) idTokenVerifier() (*idTokenVerifier, bool) { +func (a *jwtAuthenticator) idTokenVerifier() (*idTokenVerifier, bool) { if v := a.verifier.Load(); v != nil { return v.(*idTokenVerifier), true } return nil, false } -func (a *Authenticator) Close() { - a.cancel() -} - func AllValidSigningAlgorithms() []string { return sets.List(sets.KeySet(allowedSigningAlgs)) } @@ -228,7 +232,18 @@ var allowedSigningAlgs = map[string]bool{ oidc.PS512: true, } -func New(opts Options) (authenticator.Token, error) { +type AuthenticatorTokenWithHealthCheck interface { + authenticator.Token + HealthCheck() error +} + +// New returns an authenticator that is asynchronously initialized when opts.KeySet is not set. +// The input lifecycleCtx is used to: +// - terminate background goroutines that are needed for asynchronous initialization +// - as the base context for any requests that are made (i.e. for key fetching) +// Thus, once the lifecycleCtx is canceled, the authenticator must not be used. +// A caller may check if the authenticator is healthy by calling the HealthCheck method. +func New(lifecycleCtx context.Context, opts Options) (AuthenticatorTokenWithHealthCheck, error) { celMapper, fieldErr := apiservervalidation.CompileAndValidateJWTAuthenticator(opts.JWTAuthenticator, opts.DisallowedIssuers) if err := fieldErr.ToAggregate(); err != nil { return nil, err @@ -280,6 +295,10 @@ func New(opts Options) (authenticator.Token, error) { // the discovery URL. This is useful for self-hosted providers, for example, // providers that run on top of Kubernetes itself. if len(opts.JWTAuthenticator.Issuer.DiscoveryURL) > 0 { + if opts.KeySet != nil { + return nil, fmt.Errorf("oidc: KeySet and DiscoveryURL are mutually exclusive") + } + discoveryURL, err := url.Parse(opts.JWTAuthenticator.Issuer.DiscoveryURL) if err != nil { return nil, fmt.Errorf("oidc: invalid discovery URL: %w", err) @@ -297,8 +316,7 @@ func New(opts Options) (authenticator.Token, error) { client = &clientWithDiscoveryURL } - ctx, cancel := context.WithCancel(context.Background()) - ctx = oidc.ClientContext(ctx, client) + lifecycleCtx = oidc.ClientContext(lifecycleCtx, client) now := opts.now if now == nil { @@ -324,7 +342,7 @@ func New(opts Options) (authenticator.Token, error) { var resolver *claimResolver groupsClaim := opts.JWTAuthenticator.ClaimMappings.Groups.Claim if groupsClaim != "" { - resolver = newClaimResolver(groupsClaim, client, verifierConfig, audiences) + resolver = newClaimResolver(lifecycleCtx, groupsClaim, client, verifierConfig, audiences) } requiredClaims := make(map[string]string) @@ -334,38 +352,51 @@ func New(opts Options) (authenticator.Token, error) { } } - authenticator := &Authenticator{ + authn := &jwtAuthenticator{ jwtAuthenticator: opts.JWTAuthenticator, - cancel: cancel, resolver: resolver, celMapper: celMapper, requiredClaims: requiredClaims, } + authn.healthCheck.Store(&errorHolder{ + err: fmt.Errorf("oidc: authenticator for issuer %q is not initialized", authn.jwtAuthenticator.Issuer.URL), + }) issuerURL := opts.JWTAuthenticator.Issuer.URL if opts.KeySet != nil { // We already have a key set, synchronously initialize the verifier. - authenticator.setVerifier(&idTokenVerifier{ + authn.setVerifier(&idTokenVerifier{ oidc.NewVerifier(issuerURL, opts.KeySet, verifierConfig), audiences, }) } else { // Asynchronously attempt to initialize the authenticator. This enables // self-hosted providers, providers that run on top of Kubernetes itself. - go wait.PollImmediateUntil(10*time.Second, func() (done bool, err error) { - provider, err := oidc.NewProvider(ctx, issuerURL) - if err != nil { - klog.Errorf("oidc authenticator: initializing plugin: %v", err) - return false, nil - } + go func() { + // we ignore any errors from polling because they can only come from the context being canceled + _ = wait.PollUntilContextCancel(lifecycleCtx, 10*time.Second, true, func(_ context.Context) (done bool, err error) { + // this must always use lifecycleCtx because NewProvider uses that context for future key set fetching. + // this also means that there is no correct way to control the timeout of the discovery request made by NewProvider. + // the global timeout of the http.Client is still honored. + provider, err := oidc.NewProvider(lifecycleCtx, issuerURL) + if err != nil { + klog.Errorf("oidc authenticator: initializing plugin: %v", err) + authn.healthCheck.Store(&errorHolder{err: err}) + return false, nil + } - verifier := provider.Verifier(verifierConfig) - authenticator.setVerifier(&idTokenVerifier{verifier, audiences}) - return true, nil - }, ctx.Done()) + verifier := provider.Verifier(verifierConfig) + authn.setVerifier(&idTokenVerifier{verifier, audiences}) + return true, nil + }) + }() } - return newInstrumentedAuthenticator(issuerURL, authenticator), nil + return newInstrumentedAuthenticator(issuerURL, authn), nil +} + +type errorHolder struct { + err error } // discoveryURLRoundTripper is a http.RoundTripper that rewrites the @@ -448,6 +479,8 @@ type endpoint struct { // claimResolver expands distributed claims by calling respective claim source // endpoints. type claimResolver struct { + ctx context.Context + // claim is the distributed claim that may be resolved. claim string @@ -471,8 +504,10 @@ type claimResolver struct { } // newClaimResolver creates a new resolver for distributed claims. -func newClaimResolver(claim string, client *http.Client, config *oidc.Config, audiences sets.Set[string]) *claimResolver { +// the input ctx is retained and is used as the base context for background requests such as key fetching. +func newClaimResolver(ctx context.Context, claim string, client *http.Client, config *oidc.Config, audiences sets.Set[string]) *claimResolver { return &claimResolver{ + ctx: ctx, claim: claim, audiences: audiences, client: client, @@ -487,8 +522,7 @@ func (r *claimResolver) Verifier(iss string) (*idTokenVerifier, error) { av := r.verifierPerIssuer[iss] if av == nil { // This lazy init should normally be very quick. - // TODO: Make this context cancelable. - ctx := oidc.ClientContext(context.Background(), r.client) + ctx := oidc.ClientContext(r.ctx, r.client) av = newAsyncIDTokenVerifier(ctx, r.config, iss, r.audiences) r.verifierPerIssuer[iss] = av } @@ -638,7 +672,7 @@ func (v *idTokenVerifier) verifyAudience(t *oidc.IDToken) error { return fmt.Errorf("oidc: expected audience in %q got %q", sets.List(v.audiences), t.Audience) } -func (a *Authenticator) AuthenticateToken(ctx context.Context, token string) (*authenticator.Response, bool, error) { +func (a *jwtAuthenticator) AuthenticateToken(ctx context.Context, token string) (*authenticator.Response, bool, error) { if !hasCorrectIssuer(a.jwtAuthenticator.Issuer.URL, token) { return nil, false, nil } @@ -759,7 +793,15 @@ func (a *Authenticator) AuthenticateToken(ctx context.Context, token string) (*a return &authenticator.Response{User: info}, true, nil } -func (a *Authenticator) getUsername(ctx context.Context, c claims, claimsUnstructured *unstructured.Unstructured) (string, error) { +func (a *jwtAuthenticator) HealthCheck() error { + if holder := *a.healthCheck.Load(); holder.err != nil { + return fmt.Errorf("oidc: authenticator for issuer %q is not healthy: %w", a.jwtAuthenticator.Issuer.URL, holder.err) + } + + return nil +} + +func (a *jwtAuthenticator) getUsername(ctx context.Context, c claims, claimsUnstructured *unstructured.Unstructured) (string, error) { if a.celMapper.Username != nil { evalResult, err := a.celMapper.Username.EvalClaimMapping(ctx, claimsUnstructured) if err != nil { @@ -807,7 +849,7 @@ func (a *Authenticator) getUsername(ctx context.Context, c claims, claimsUnstruc return username, nil } -func (a *Authenticator) getGroups(ctx context.Context, c claims, claimsUnstructured *unstructured.Unstructured) ([]string, error) { +func (a *jwtAuthenticator) getGroups(ctx context.Context, c claims, claimsUnstructured *unstructured.Unstructured) ([]string, error) { groupsClaim := a.jwtAuthenticator.ClaimMappings.Groups.Claim if len(groupsClaim) > 0 { if _, ok := c[groupsClaim]; ok { @@ -847,7 +889,7 @@ func (a *Authenticator) getGroups(ctx context.Context, c claims, claimsUnstructu return groups, nil } -func (a *Authenticator) getUID(ctx context.Context, c claims, claimsUnstructured *unstructured.Unstructured) (string, error) { +func (a *jwtAuthenticator) getUID(ctx context.Context, c claims, claimsUnstructured *unstructured.Unstructured) (string, error) { uidClaim := a.jwtAuthenticator.ClaimMappings.UID.Claim if len(uidClaim) > 0 { var uid string @@ -872,7 +914,7 @@ func (a *Authenticator) getUID(ctx context.Context, c claims, claimsUnstructured return evalResult.EvalResult.Value().(string), nil } -func (a *Authenticator) getExtra(ctx context.Context, claimsUnstructured *unstructured.Unstructured) (map[string][]string, error) { +func (a *jwtAuthenticator) getExtra(ctx context.Context, claimsUnstructured *unstructured.Unstructured) (map[string][]string, error) { if a.celMapper.Extra == nil { return nil, nil } diff --git a/staging/src/k8s.io/apiserver/plugin/pkg/authenticator/token/oidc/oidc_test.go b/staging/src/k8s.io/apiserver/plugin/pkg/authenticator/token/oidc/oidc_test.go index 3b246df4d08..d4db3faa3f7 100644 --- a/staging/src/k8s.io/apiserver/plugin/pkg/authenticator/token/oidc/oidc_test.go +++ b/staging/src/k8s.io/apiserver/plugin/pkg/authenticator/token/oidc/oidc_test.go @@ -145,6 +145,7 @@ type claimsTest struct { wantSkip bool wantErr string wantInitErr string + wantHealthErrPrefix string claimToResponseMap map[string]string openIDConfig string fetchKeysFromRemote bool @@ -283,8 +284,10 @@ func (c *claimsTest) run(t *testing.T) { expectInitErr := len(c.wantInitErr) > 0 + ctx := testContext(t) + // Initialize the authenticator. - a, err := New(c.options) + a, err := New(ctx, c.options) if err != nil { if !expectInitErr { t.Fatalf("initialize authenticator: %v", err) @@ -298,6 +301,25 @@ func (c *claimsTest) run(t *testing.T) { t.Fatalf("wanted initialization error %q but got none", c.wantInitErr) } + if len(c.wantHealthErrPrefix) > 0 { + if err := wait.PollUntilContextTimeout(ctx, time.Second, time.Minute, true, func(context.Context) (bool, error) { + healthErr := a.HealthCheck() + if healthErr == nil { + return false, fmt.Errorf("authenticator reported healthy when it should not") + } + + if strings.HasPrefix(healthErr.Error(), c.wantHealthErrPrefix) { + return true, nil + } + + t.Logf("saw health error prefix that did not match: want=%q got=%q", c.wantHealthErrPrefix, healthErr.Error()) + return false, nil + }); err != nil { + t.Fatalf("authenticator did not match wanted health error: %v", err) + } + return + } + claims := struct{}{} if err := json.Unmarshal([]byte(c.claims), &claims); err != nil { t.Fatalf("failed to unmarshal claims: %v", err) @@ -313,21 +335,9 @@ func (c *claimsTest) run(t *testing.T) { t.Fatalf("serialize token: %v", err) } - ia, ok := a.(*instrumentedAuthenticator) - if !ok { - t.Fatalf("expected authenticator to be instrumented") - } - authenticator, ok := ia.delegate.(*Authenticator) - if !ok { - t.Fatalf("expected delegate to be Authenticator") - } - ctx := testContext(t) - // wait for the authenticator to be initialized + // wait for the authenticator to be healthy err = wait.PollUntilContextCancel(ctx, time.Millisecond, true, func(context.Context) (bool, error) { - if v, _ := authenticator.idTokenVerifier(); v == nil { - return false, nil - } - return true, nil + return a.HealthCheck() == nil, nil }) if err != nil { t.Fatalf("failed to initialize the authenticator: %v", err) @@ -2060,6 +2070,51 @@ func TestToken(t *testing.T) { }, wantInitErr: "oidc: Client and CAContentProvider are mutually exclusive", }, + { + name: "keyset and discovery URL mutually exclusive", + options: Options{ + JWTAuthenticator: apiserver.JWTAuthenticator{ + Issuer: apiserver.Issuer{ + URL: "https://auth.example.com", + DiscoveryURL: "https://auth.example.com/foo", + Audiences: []string{"my-client"}, + }, + ClaimMappings: apiserver.ClaimMappings{ + Username: apiserver.PrefixedClaimOrExpression{ + Claim: "username", + Prefix: pointer.String("prefix:"), + }, + }, + }, + SupportedSigningAlgs: []string{"RS256"}, + now: func() time.Time { return now }, + KeySet: &staticKeySet{}, + }, + pubKeys: []*jose.JSONWebKey{ + loadRSAKey(t, "testdata/rsa_1.pem", jose.RS256), + }, + wantInitErr: "oidc: KeySet and DiscoveryURL are mutually exclusive", + }, + { + name: "health check failure", + options: Options{ + JWTAuthenticator: apiserver.JWTAuthenticator{ + Issuer: apiserver.Issuer{ + URL: "https://this-will-not-work.notatld", + Audiences: []string{"my-client"}, + }, + ClaimMappings: apiserver.ClaimMappings{ + Username: apiserver.PrefixedClaimOrExpression{ + Claim: "username", + Prefix: pointer.String("prefix:"), + }, + }, + }, + SupportedSigningAlgs: []string{"RS256"}, + }, + fetchKeysFromRemote: true, + wantHealthErrPrefix: `oidc: authenticator for issuer "https://this-will-not-work.notatld" is not healthy: Get "https://this-will-not-work.notatld/.well-known/openid-configuration": dial tcp: lookup this-will-not-work.notatld`, + }, { name: "accounts.google.com issuer", options: Options{ @@ -3306,7 +3361,7 @@ func TestToken(t *testing.T) { var successTestCount, failureTestCount int for _, test := range tests { t.Run(test.name, test.run) - if test.wantSkip || test.wantInitErr != "" { + if test.wantSkip || len(test.wantInitErr) > 0 || len(test.wantHealthErrPrefix) > 0 { continue } // check metrics for success and failure diff --git a/test/integration/apiserver/oidc/oidc_test.go b/test/integration/apiserver/oidc/oidc_test.go index 24262e65b58..97381fb3731 100644 --- a/test/integration/apiserver/oidc/oidc_test.go +++ b/test/integration/apiserver/oidc/oidc_test.go @@ -35,6 +35,7 @@ import ( "testing" "time" + "github.com/google/go-cmp/cmp" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "gopkg.in/square/go-jose.v2" @@ -44,6 +45,7 @@ import ( apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" utilrand "k8s.io/apimachinery/pkg/util/rand" + "k8s.io/apimachinery/pkg/util/wait" "k8s.io/apiserver/pkg/features" utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/client-go/kubernetes" @@ -54,6 +56,7 @@ import ( featuregatetesting "k8s.io/component-base/featuregate/testing" kubeapiserverapptesting "k8s.io/kubernetes/cmd/kube-apiserver/app/testing" "k8s.io/kubernetes/pkg/apis/rbac" + "k8s.io/kubernetes/pkg/kubeapiserver/options" "k8s.io/kubernetes/test/integration/framework" utilsoidc "k8s.io/kubernetes/test/utils/oidc" utilsnet "k8s.io/utils/net" @@ -959,6 +962,419 @@ jwt: } } +func TestStructuredAuthenticationConfigReload(t *testing.T) { + const hardCodedTokenCacheTTLAndPollInterval = 10 * time.Second + + origUpdateAuthenticationConfigTimeout := options.UpdateAuthenticationConfigTimeout + t.Cleanup(func() { options.UpdateAuthenticationConfigTimeout = origUpdateAuthenticationConfigTimeout }) + options.UpdateAuthenticationConfigTimeout = 2 * hardCodedTokenCacheTTLAndPollInterval // needs to be large enough for polling to run multiple times + + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.StructuredAuthenticationConfiguration, true)() + + tests := []struct { + name string + authConfigFn, newAuthConfigFn authenticationConfigFunc + assertErrFn, newAssertErrFn func(t *testing.T, errorToCheck error) + wantUser, newWantUser *authenticationv1.UserInfo + ignoreTransitionErrFn func(error) bool + waitAfterConfigSwap bool + }{ + { + name: "old valid config to new valid config", + authConfigFn: func(t *testing.T, issuerURL, caCert string) string { + return fmt.Sprintf(` +apiVersion: apiserver.config.k8s.io/v1alpha1 +kind: AuthenticationConfiguration +jwt: +- issuer: + url: %s + audiences: + - %s + - another-audience + audienceMatchPolicy: MatchAny + certificateAuthority: | + %s + claimMappings: + username: + expression: "'k8s-' + claims.sub" +`, issuerURL, defaultOIDCClientID, indentCertificateAuthority(caCert)) + }, + newAuthConfigFn: func(t *testing.T, issuerURL, caCert string) string { + return fmt.Sprintf(` +apiVersion: apiserver.config.k8s.io/v1alpha1 +kind: AuthenticationConfiguration +jwt: +- issuer: + url: %s + audiences: + - %s + - another-audience + audienceMatchPolicy: MatchAny + certificateAuthority: | + %s + claimMappings: + username: + expression: "'panda-' + claims.sub" # this is the only new part of the config +`, issuerURL, defaultOIDCClientID, indentCertificateAuthority(caCert)) + }, + assertErrFn: func(t *testing.T, errorToCheck error) { + assert.NoError(t, errorToCheck) + }, + wantUser: &authenticationv1.UserInfo{ + Username: "k8s-john_doe", + Groups: []string{"system:authenticated"}, + }, + newAssertErrFn: func(t *testing.T, errorToCheck error) { + _ = assert.True(t, apierrors.IsForbidden(errorToCheck)) && + assert.Equal( + t, + `pods is forbidden: User "panda-john_doe" cannot list resource "pods" in API group "" in the namespace "default"`, + errorToCheck.Error(), + ) + }, + newWantUser: &authenticationv1.UserInfo{ + Username: "panda-john_doe", + Groups: []string{"system:authenticated"}, + }, + }, + { + name: "old empty config to new valid config", + authConfigFn: func(t *testing.T, _, _ string) string { + return ` +apiVersion: apiserver.config.k8s.io/v1alpha1 +kind: AuthenticationConfiguration +` + }, + newAuthConfigFn: func(t *testing.T, issuerURL, caCert string) string { + return fmt.Sprintf(` +apiVersion: apiserver.config.k8s.io/v1alpha1 +kind: AuthenticationConfiguration +jwt: +- issuer: + url: %s + audiences: + - %s + - another-audience + audienceMatchPolicy: MatchAny + certificateAuthority: | + %s + claimMappings: + username: + expression: "'snorlax-' + claims.sub" +`, issuerURL, defaultOIDCClientID, indentCertificateAuthority(caCert)) + }, + assertErrFn: func(t *testing.T, errorToCheck error) { + assert.True(t, apierrors.IsUnauthorized(errorToCheck)) + }, + wantUser: nil, + ignoreTransitionErrFn: apierrors.IsUnauthorized, + newAssertErrFn: func(t *testing.T, errorToCheck error) { + _ = assert.True(t, apierrors.IsForbidden(errorToCheck)) && + assert.Equal( + t, + `pods is forbidden: User "snorlax-john_doe" cannot list resource "pods" in API group "" in the namespace "default"`, + errorToCheck.Error(), + ) + }, + newWantUser: &authenticationv1.UserInfo{ + Username: "snorlax-john_doe", + Groups: []string{"system:authenticated"}, + }, + }, + { + name: "old invalid config to new valid config", + authConfigFn: func(t *testing.T, issuerURL, _ string) string { + return fmt.Sprintf(` +apiVersion: apiserver.config.k8s.io/v1alpha1 +kind: AuthenticationConfiguration +jwt: +- issuer: + url: %s + audiences: + - %s + - another-audience + audienceMatchPolicy: MatchAny + certificateAuthority: "" # missing CA + claimMappings: + username: + expression: "'k8s-' + claims.sub" +`, issuerURL, defaultOIDCClientID) + }, + newAuthConfigFn: func(t *testing.T, issuerURL, caCert string) string { + return fmt.Sprintf(` +apiVersion: apiserver.config.k8s.io/v1alpha1 +kind: AuthenticationConfiguration +jwt: +- issuer: + url: %s + audiences: + - %s + - another-audience + audienceMatchPolicy: MatchAny + # this is the only new part of the config + certificateAuthority: | + %s + claimMappings: + username: + expression: "'k8s-' + claims.sub" +`, issuerURL, defaultOIDCClientID, indentCertificateAuthority(caCert)) + }, + assertErrFn: func(t *testing.T, errorToCheck error) { + assert.True(t, apierrors.IsUnauthorized(errorToCheck)) + }, + wantUser: nil, + ignoreTransitionErrFn: apierrors.IsUnauthorized, + newAssertErrFn: func(t *testing.T, errorToCheck error) { + assert.NoError(t, errorToCheck) + }, + newWantUser: &authenticationv1.UserInfo{ + Username: "k8s-john_doe", + Groups: []string{"system:authenticated"}, + }, + }, + { + name: "old valid config to new structurally invalid config (should be ignored)", + authConfigFn: func(t *testing.T, issuerURL, caCert string) string { + return fmt.Sprintf(` +apiVersion: apiserver.config.k8s.io/v1alpha1 +kind: AuthenticationConfiguration +jwt: +- issuer: + url: %s + audiences: + - %s + - another-audience + audienceMatchPolicy: MatchAny + certificateAuthority: | + %s + claimMappings: + username: + expression: "'k8s-' + claims.sub" +`, issuerURL, defaultOIDCClientID, indentCertificateAuthority(caCert)) + }, + newAuthConfigFn: func(t *testing.T, issuerURL, caCert string) string { + return fmt.Sprintf(` +apiVersion: apiserver.config.k8s.io/v1alpha1 +kind: AuthenticationConfiguration +jwt: +- issuer: + url: %s + audiences: + - %s + - another-audience + audienceMatchPolicy: MatchAny + certificateAuthority: | + %s + claimMappings: + username: + expression: "'k8s-' + claimss.sub" # has typo +`, issuerURL, defaultOIDCClientID, indentCertificateAuthority(caCert)) + }, + assertErrFn: func(t *testing.T, errorToCheck error) { + assert.NoError(t, errorToCheck) + }, + wantUser: &authenticationv1.UserInfo{ + Username: "k8s-john_doe", + Groups: []string{"system:authenticated"}, + }, + newAssertErrFn: func(t *testing.T, errorToCheck error) { + assert.NoError(t, errorToCheck) + }, + newWantUser: &authenticationv1.UserInfo{ + Username: "k8s-john_doe", + Groups: []string{"system:authenticated"}, + }, + waitAfterConfigSwap: true, + }, + { + name: "old valid config to new valid empty config (should cause tokens to stop working)", + authConfigFn: func(t *testing.T, issuerURL, caCert string) string { + return fmt.Sprintf(` +apiVersion: apiserver.config.k8s.io/v1alpha1 +kind: AuthenticationConfiguration +jwt: +- issuer: + url: %s + audiences: + - %s + - another-audience + audienceMatchPolicy: MatchAny + certificateAuthority: | + %s + claimMappings: + username: + expression: "'k8s-' + claims.sub" +`, issuerURL, defaultOIDCClientID, indentCertificateAuthority(caCert)) + }, + newAuthConfigFn: func(t *testing.T, _, _ string) string { + return ` +apiVersion: apiserver.config.k8s.io/v1alpha1 +kind: AuthenticationConfiguration +` + }, + assertErrFn: func(t *testing.T, errorToCheck error) { + assert.NoError(t, errorToCheck) + }, + wantUser: &authenticationv1.UserInfo{ + Username: "k8s-john_doe", + Groups: []string{"system:authenticated"}, + }, + newAssertErrFn: func(t *testing.T, errorToCheck error) { + assert.True(t, apierrors.IsUnauthorized(errorToCheck)) + }, + newWantUser: nil, + waitAfterConfigSwap: true, + }, + { + name: "old valid config to new valid config with typo (should be ignored)", + authConfigFn: func(t *testing.T, issuerURL, caCert string) string { + return fmt.Sprintf(` +apiVersion: apiserver.config.k8s.io/v1alpha1 +kind: AuthenticationConfiguration +jwt: +- issuer: + url: %s + audiences: + - %s + - another-audience + audienceMatchPolicy: MatchAny + certificateAuthority: | + %s + claimMappings: + username: + expression: "'k8s-' + claims.sub" +`, issuerURL, defaultOIDCClientID, indentCertificateAuthority(caCert)) + }, + newAuthConfigFn: func(t *testing.T, issuerURL, _ string) string { + return fmt.Sprintf(` +apiVersion: apiserver.config.k8s.io/v1alpha1 +kind: AuthenticationConfiguration +jwt: +- issuer: + url: %s + audiences: + - %s + - another-audience + audienceMatchPolicy: MatchAny + certificateAuthority: "" # missing CA + claimMappings: + username: + expression: "'k8s-' + claims.sub" +`, issuerURL, defaultOIDCClientID) + }, + assertErrFn: func(t *testing.T, errorToCheck error) { + assert.NoError(t, errorToCheck) + }, + wantUser: &authenticationv1.UserInfo{ + Username: "k8s-john_doe", + Groups: []string{"system:authenticated"}, + }, + newAssertErrFn: func(t *testing.T, errorToCheck error) { + assert.NoError(t, errorToCheck) + }, + newWantUser: &authenticationv1.UserInfo{ + Username: "k8s-john_doe", + Groups: []string{"system:authenticated"}, + }, + waitAfterConfigSwap: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx := testContext(t) + + oidcServer, apiServer, caCert, certPath := configureBasicTestInfrastructureWithRandomKeyType(t, tt.authConfigFn) + + tokenURL, err := oidcServer.TokenURL() + require.NoError(t, err) + + client := configureClientFetchingOIDCCredentials(t, apiServer.ClientConfig, caCert, certPath, oidcServer.URL(), tokenURL) + + if tt.wantUser != nil { + res, err := client.AuthenticationV1().SelfSubjectReviews().Create(ctx, &authenticationv1.SelfSubjectReview{}, metav1.CreateOptions{}) + require.NoError(t, err) + assert.Equal(t, *tt.wantUser, res.Status.UserInfo) + } + + _, err = client.CoreV1().Pods(defaultNamespace).List(ctx, metav1.ListOptions{}) + tt.assertErrFn(t, err) + + err = os.WriteFile(apiServer.ServerOpts.Authentication.AuthenticationConfigFile, []byte(tt.newAuthConfigFn(t, oidcServer.URL(), string(caCert))), 0600) + require.NoError(t, err) + + if tt.waitAfterConfigSwap { + time.Sleep(options.UpdateAuthenticationConfigTimeout + hardCodedTokenCacheTTLAndPollInterval) // has to be longer than UpdateAuthenticationConfigTimeout + } + + if tt.newWantUser != nil { + start := time.Now() + err = wait.PollUntilContextTimeout(ctx, time.Second, 3*hardCodedTokenCacheTTLAndPollInterval, true, func(ctx context.Context) (done bool, err error) { + res, err := client.AuthenticationV1().SelfSubjectReviews().Create(ctx, &authenticationv1.SelfSubjectReview{}, metav1.CreateOptions{}) + if err != nil { + if tt.ignoreTransitionErrFn != nil && tt.ignoreTransitionErrFn(err) { + return false, nil + } + return false, err + } + + diff := cmp.Diff(*tt.newWantUser, res.Status.UserInfo) + if len(diff) > 0 && time.Since(start) > 2*hardCodedTokenCacheTTLAndPollInterval { + t.Logf("%s saw new user diff:\n%s", t.Name(), diff) + } + + return len(diff) == 0, nil + }) + require.NoError(t, err, "new authentication config not loaded") + } + + _, err = client.CoreV1().Pods(defaultNamespace).List(ctx, metav1.ListOptions{}) + tt.newAssertErrFn(t, err) + }) + } +} + +func configureBasicTestInfrastructureWithRandomKeyType(t *testing.T, fn authenticationConfigFunc) ( + oidcServer *utilsoidc.TestServer, + apiServer *kubeapiserverapptesting.TestServer, + caCertContent []byte, + caFilePath string, +) { + t.Helper() + + if randomBool() { + return configureBasicTestInfrastructure(t, fn, rsaGenerateKey) + } + + return configureBasicTestInfrastructure(t, fn, ecdsaGenerateKey) +} + +func configureBasicTestInfrastructure[K utilsoidc.JosePrivateKey, L utilsoidc.JosePublicKey](t *testing.T, fn authenticationConfigFunc, keyFunc func(t *testing.T) (K, L)) ( + oidcServer *utilsoidc.TestServer, + apiServer *kubeapiserverapptesting.TestServer, + caCertContent []byte, + caFilePath string, +) { + t.Helper() + + oidcServer, apiServer, signingPrivateKey, caCertContent, caFilePath := configureTestInfrastructure(t, fn, keyFunc) + + oidcServer.TokenHandler().EXPECT().Token().Times(1).DoAndReturn(utilsoidc.TokenHandlerBehaviorReturningPredefinedJWT( + t, + signingPrivateKey, + map[string]interface{}{ + "iss": oidcServer.URL(), + "sub": defaultOIDCClaimedUsername, + "aud": defaultOIDCClientID, + "exp": time.Now().Add(10 * time.Minute).Unix(), + }, + defaultStubAccessToken, + defaultStubRefreshToken, + )) + + return oidcServer, apiServer, caCertContent, caFilePath +} + // TestStructuredAuthenticationDiscoveryURL tests that the discovery URL configured in jwt.issuer.discoveryURL is used to // fetch the discovery document and the issuer in jwt.issuer.url is used to validate the ID token. func TestStructuredAuthenticationDiscoveryURL(t *testing.T) { @@ -1398,7 +1814,7 @@ func indentCertificateAuthority(caCert string) string { } func testContext(t *testing.T) context.Context { - ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute) t.Cleanup(cancel) return ctx }