diff --git a/cmd/kube-apiserver/app/options/options_test.go b/cmd/kube-apiserver/app/options/options_test.go index ad148e10434..8b22349924d 100644 --- a/cmd/kube-apiserver/app/options/options_test.go +++ b/cmd/kube-apiserver/app/options/options_test.go @@ -44,6 +44,7 @@ import ( "k8s.io/kubernetes/pkg/controlplane/reconcilers" kubeoptions "k8s.io/kubernetes/pkg/kubeapiserver/options" kubeletclient "k8s.io/kubernetes/pkg/kubelet/client" + "k8s.io/kubernetes/pkg/serviceaccount" netutils "k8s.io/utils/net" ) @@ -272,8 +273,9 @@ func TestAddFlags(t *testing.T) { OIDC: s.Authentication.OIDC, RequestHeader: &apiserveroptions.RequestHeaderAuthenticationOptions{}, ServiceAccounts: &kubeoptions.ServiceAccountAuthenticationOptions{ - Lookup: true, - ExtendExpiration: true, + Lookup: true, + ExtendExpiration: true, + MaxExtendedExpiration: serviceaccount.ExpirationExtensionSeconds * time.Second, }, TokenFile: &kubeoptions.TokenFileAuthenticationOptions{}, TokenSuccessCacheTTL: 10 * time.Second, @@ -348,7 +350,7 @@ func TestAddFlags(t *testing.T) { s.Authorization.AreLegacyFlagsSet = nil if !reflect.DeepEqual(expected, s) { - t.Errorf("Got different run options than expected.\nDifference detected on:\n%s", cmp.Diff(expected, s, cmpopts.IgnoreUnexported(admission.Plugins{}, kubeoptions.OIDCAuthenticationOptions{}, kubeoptions.AnonymousAuthenticationOptions{}))) + t.Errorf("Got different run options than expected.\nDifference detected on:\n%s", cmp.Diff(expected, s, cmpopts.IgnoreFields(apiserveroptions.ServerRunOptions{}, "ComponentGlobalsRegistry"), cmpopts.IgnoreUnexported(admission.Plugins{}, kubeoptions.OIDCAuthenticationOptions{}, kubeoptions.AnonymousAuthenticationOptions{}))) } testEffectiveVersion := s.GenericServerRunOptions.ComponentGlobalsRegistry.EffectiveVersionFor("test") if testEffectiveVersion.EmulationVersion().String() != "1.31" { diff --git a/pkg/controlplane/apiserver/apis.go b/pkg/controlplane/apiserver/apis.go index 7ed3a58489b..c129ba8849b 100644 --- a/pkg/controlplane/apiserver/apis.go +++ b/pkg/controlplane/apiserver/apis.go @@ -52,8 +52,8 @@ func (c *CompletedConfig) NewCoreGenericConfig() *corerest.GenericConfig { LoopbackClientConfig: c.Generic.LoopbackClientConfig, ServiceAccountIssuer: c.Extra.ServiceAccountIssuer, ExtendExpiration: c.Extra.ExtendExpiration, - IsTokenSignerExternal: c.Extra.IsTokenSignerExternal, ServiceAccountMaxExpiration: c.Extra.ServiceAccountMaxExpiration, + MaxExtendedExpiration: c.Extra.ServiceAccountExtendedMaxExpiration, APIAudiences: c.Generic.Authentication.APIAudiences, Informers: c.Extra.VersionedInformers, } diff --git a/pkg/controlplane/apiserver/config.go b/pkg/controlplane/apiserver/config.go index aed7e6866ec..1dab17042fa 100644 --- a/pkg/controlplane/apiserver/config.go +++ b/pkg/controlplane/apiserver/config.go @@ -89,10 +89,10 @@ type Extra struct { // version skew. If unset, AdvertiseAddress/BindAddress will be used. PeerAdvertiseAddress peerreconcilers.PeerAdvertiseAddress - ServiceAccountIssuer serviceaccount.TokenGenerator - ServiceAccountMaxExpiration time.Duration - ExtendExpiration bool - IsTokenSignerExternal bool + ServiceAccountIssuer serviceaccount.TokenGenerator + ServiceAccountMaxExpiration time.Duration + ServiceAccountExtendedMaxExpiration time.Duration + ExtendExpiration bool // ServiceAccountIssuerDiscovery ServiceAccountIssuerURL string @@ -299,10 +299,10 @@ func CreateConfig( ProxyTransport: proxyTransport, SystemNamespaces: opts.SystemNamespaces, - ServiceAccountIssuer: opts.ServiceAccountIssuer, - ServiceAccountMaxExpiration: opts.ServiceAccountTokenMaxExpiration, - ExtendExpiration: opts.Authentication.ServiceAccounts.ExtendExpiration, - IsTokenSignerExternal: opts.Authentication.ServiceAccounts.IsTokenSignerExternal, + ServiceAccountIssuer: opts.ServiceAccountIssuer, + ServiceAccountMaxExpiration: opts.ServiceAccountTokenMaxExpiration, + ServiceAccountExtendedMaxExpiration: opts.Authentication.ServiceAccounts.MaxExtendedExpiration, + ExtendExpiration: opts.Authentication.ServiceAccounts.ExtendExpiration, VersionedInformers: versionedInformers, }, diff --git a/pkg/controlplane/apiserver/options/options.go b/pkg/controlplane/apiserver/options/options.go index 026ffc59caf..a6a81ee137d 100644 --- a/pkg/controlplane/apiserver/options/options.go +++ b/pkg/controlplane/apiserver/options/options.go @@ -316,15 +316,19 @@ func (o *Options) completeServiceAccountOptions(ctx context.Context, completed * if metadata.MaxTokenExpirationSeconds < validation.MinTokenAgeSec { return fmt.Errorf("max token life supported by external-jwt-signer (%ds) is less than acceptable (min %ds)", metadata.MaxTokenExpirationSeconds, validation.MinTokenAgeSec) } - if completed.Authentication.ServiceAccounts.MaxExpiration != 0 { - return fmt.Errorf("service-account-max-token-expiration and service-account-signing-endpoint are mutually exclusive and cannot be set at the same time") + maxExternalExpiration := time.Duration(metadata.MaxTokenExpirationSeconds) * time.Second + switch { + case completed.Authentication.ServiceAccounts.MaxExpiration == 0: + completed.Authentication.ServiceAccounts.MaxExpiration = maxExternalExpiration + case completed.Authentication.ServiceAccounts.MaxExpiration > maxExternalExpiration: + return fmt.Errorf("service-account-max-token-expiration cannot be set longer than the token expiration supported by service-account-signing-endpoint: %s > %s", completed.Authentication.ServiceAccounts.MaxExpiration, maxExternalExpiration) } transitionWarningFmt = "service-account-extend-token-expiration is true, in order to correctly trigger safe transition logic, token lifetime supported by external-jwt-signer must be longer than %d seconds (currently %s)" expExtensionWarningFmt = "service-account-extend-token-expiration is true, tokens validity will be caped at the smaller of %d seconds and maximum token lifetime supported by external-jwt-signer (%s)" completed.ServiceAccountIssuer = plugin completed.Authentication.ServiceAccounts.ExternalPublicKeysGetter = cache - completed.Authentication.ServiceAccounts.MaxExpiration = time.Duration(metadata.MaxTokenExpirationSeconds) * time.Second - completed.Authentication.ServiceAccounts.IsTokenSignerExternal = true + // shorten ExtendedExpiration, if needed, to fit within the external signer's max expiration + completed.Authentication.ServiceAccounts.MaxExtendedExpiration = min(maxExternalExpiration, completed.Authentication.ServiceAccounts.MaxExtendedExpiration) } } diff --git a/pkg/controlplane/apiserver/options/options_test.go b/pkg/controlplane/apiserver/options/options_test.go index 9fa3f0abfa3..55e30fafcce 100644 --- a/pkg/controlplane/apiserver/options/options_test.go +++ b/pkg/controlplane/apiserver/options/options_test.go @@ -32,7 +32,6 @@ import ( "github.com/google/go-cmp/cmp/cmpopts" "github.com/spf13/pflag" noopoteltrace "go.opentelemetry.io/otel/trace/noop" - utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/apiserver/pkg/admission" apiserveroptions "k8s.io/apiserver/pkg/server/options" @@ -46,6 +45,7 @@ import ( "k8s.io/component-base/metrics" utilversion "k8s.io/component-base/version" kubeoptions "k8s.io/kubernetes/pkg/kubeapiserver/options" + "k8s.io/kubernetes/pkg/serviceaccount" v1alpha1testing "k8s.io/kubernetes/pkg/serviceaccount/externaljwt/plugin/testing/v1alpha1" netutils "k8s.io/utils/net" ) @@ -264,8 +264,9 @@ func TestAddFlags(t *testing.T) { OIDC: s.Authentication.OIDC, RequestHeader: &apiserveroptions.RequestHeaderAuthenticationOptions{}, ServiceAccounts: &kubeoptions.ServiceAccountAuthenticationOptions{ - Lookup: true, - ExtendExpiration: true, + Lookup: true, + ExtendExpiration: true, + MaxExtendedExpiration: serviceaccount.ExpirationExtensionSeconds * time.Second, }, TokenFile: &kubeoptions.TokenFileAuthenticationOptions{}, TokenSuccessCacheTTL: 10 * time.Second, @@ -309,7 +310,7 @@ func TestAddFlags(t *testing.T) { s.Authorization.AreLegacyFlagsSet = nil if !reflect.DeepEqual(expected, s) { - t.Errorf("Got different run options than expected.\nDifference detected on:\n%s", cmp.Diff(expected, s, cmpopts.IgnoreUnexported(admission.Plugins{}, kubeoptions.OIDCAuthenticationOptions{}, kubeoptions.AnonymousAuthenticationOptions{}))) + t.Errorf("Got different run options than expected.\nDifference detected on:\n%s", cmp.Diff(expected, s, cmpopts.IgnoreFields(apiserveroptions.ServerRunOptions{}, "ComponentGlobalsRegistry"), cmpopts.IgnoreUnexported(admission.Plugins{}, kubeoptions.OIDCAuthenticationOptions{}, kubeoptions.AnonymousAuthenticationOptions{}))) } testEffectiveVersion := s.GenericServerRunOptions.ComponentGlobalsRegistry.EffectiveVersionFor("test") @@ -352,13 +353,14 @@ func TestCompleteForServiceAccount(t *testing.T) { externalSigner bool signingKeyFiles string maxExpiration time.Duration + maxExtendedExpiration time.Duration externalMaxExpirationSec int64 fetchError error metadataError error wantError error expectedMaxtokenExp time.Duration - expectedIsExternalSigner bool + expectedExtendedMaxTokenExp time.Duration externalPublicKeyGetterPresent bool }{ { @@ -373,7 +375,7 @@ func TestCompleteForServiceAccount(t *testing.T) { wantError: fmt.Errorf("service-account-signing-key-file and service-account-signing-endpoint are mutually exclusive and cannot be set at the same time"), }, { - desc: "max token expiration breaching accepteable values", + desc: "max token expiration breaching acceptable values", issuers: []string{ "iss", }, @@ -392,35 +394,50 @@ func TestCompleteForServiceAccount(t *testing.T) { signingKeyFiles: "private_key.pem", maxExpiration: time.Second * 3600, - expectedIsExternalSigner: false, externalPublicKeyGetterPresent: false, expectedMaxtokenExp: time.Second * 3600, }, { - desc: "signing endpoint provided", + desc: "signing endpoint provided, use endpoint expiration", issuers: []string{ "iss", }, externalSigner: true, signingKeyFiles: "", maxExpiration: 0, + maxExtendedExpiration: 365 * 24 * time.Hour, externalMaxExpirationSec: 600, // 10m - expectedIsExternalSigner: true, + expectedMaxtokenExp: 10 * time.Minute, + expectedExtendedMaxTokenExp: 10 * time.Minute, externalPublicKeyGetterPresent: true, - expectedMaxtokenExp: time.Second * 600, // 10m }, { - desc: "signing endpoint provided and max token expiration set", + desc: "signing endpoint provided, use local smaller expirations", issuers: []string{ "iss", }, externalSigner: true, signingKeyFiles: "", - maxExpiration: time.Second * 3600, - externalMaxExpirationSec: 600, // 10m + maxExpiration: 1 * time.Hour, + maxExtendedExpiration: 24 * time.Hour, + externalMaxExpirationSec: 31556952, // 1 year - wantError: fmt.Errorf("service-account-max-token-expiration and service-account-signing-endpoint are mutually exclusive and cannot be set at the same time"), + expectedMaxtokenExp: 1 * time.Hour, + expectedExtendedMaxTokenExp: 24 * time.Hour, + externalPublicKeyGetterPresent: true, + }, + { + desc: "signing endpoint provided and want larger than signer can provide", + issuers: []string{ + "iss", + }, + externalSigner: true, + signingKeyFiles: "", + maxExpiration: 1 * time.Hour, // want 1hr + externalMaxExpirationSec: 600, // signer can only sign 10m + + wantError: fmt.Errorf("service-account-max-token-expiration cannot be set longer than the token expiration supported by service-account-signing-endpoint: 1h0m0s > 10m0s"), }, { desc: "signing endpoint provided but return smaller than accaptable max token exp", @@ -481,8 +498,9 @@ func TestCompleteForServiceAccount(t *testing.T) { options.ServiceAccountSigningKeyFile = tc.signingKeyFiles options.Authentication = &kubeoptions.BuiltInAuthenticationOptions{ ServiceAccounts: &kubeoptions.ServiceAccountAuthenticationOptions{ - Issuers: tc.issuers, - MaxExpiration: tc.maxExpiration, + Issuers: tc.issuers, + MaxExpiration: tc.maxExpiration, + MaxExtendedExpiration: tc.maxExtendedExpiration, }, } @@ -507,8 +525,8 @@ func TestCompleteForServiceAccount(t *testing.T) { if tc.externalPublicKeyGetterPresent != (co.Authentication.ServiceAccounts.ExternalPublicKeysGetter != nil) { t.Errorf("Unexpected value of ExternalPublicKeysGetter: %v", co.Authentication.ServiceAccounts.ExternalPublicKeysGetter) } - if tc.expectedIsExternalSigner != co.Authentication.ServiceAccounts.IsTokenSignerExternal { - t.Errorf("Expected IsTokenSignerExternal %v, found %v", tc.expectedIsExternalSigner, co.Authentication.ServiceAccounts.IsTokenSignerExternal) + if tc.expectedExtendedMaxTokenExp != co.Authentication.ServiceAccounts.MaxExtendedExpiration { + t.Errorf("Expected MaxExtendedExpiration %v, found %v", tc.expectedExtendedMaxTokenExp, co.Authentication.ServiceAccounts.MaxExtendedExpiration) } if tc.expectedMaxtokenExp.Seconds() != co.Authentication.ServiceAccounts.MaxExpiration.Seconds() { t.Errorf("Expected MaxExpiration to be %v, found %v", tc.expectedMaxtokenExp, co.Authentication.ServiceAccounts.MaxExpiration) diff --git a/pkg/kubeapiserver/options/authentication.go b/pkg/kubeapiserver/options/authentication.go index e77cd5526b0..6b5c9f5718a 100644 --- a/pkg/kubeapiserver/options/authentication.go +++ b/pkg/kubeapiserver/options/authentication.go @@ -131,9 +131,9 @@ type ServiceAccountAuthenticationOptions struct { Lookup bool Issuers []string JWKSURI string - MaxExpiration time.Duration ExtendExpiration bool - IsTokenSignerExternal bool + MaxExpiration time.Duration + MaxExtendedExpiration time.Duration // OptionalTokenGetter is a function that returns a service account token getter. // If not set, the default token getter will be used. OptionalTokenGetter func(factory informers.SharedInformerFactory) serviceaccount.ServiceAccountTokenGetter @@ -224,6 +224,7 @@ func (o *BuiltInAuthenticationOptions) WithServiceAccounts() *BuiltInAuthenticat } o.ServiceAccounts.Lookup = true o.ServiceAccounts.ExtendExpiration = true + o.ServiceAccounts.MaxExtendedExpiration = serviceaccount.ExpirationExtensionSeconds * time.Second return o } diff --git a/pkg/kubeapiserver/options/authentication_test.go b/pkg/kubeapiserver/options/authentication_test.go index 4842104ef00..0b377214ea1 100644 --- a/pkg/kubeapiserver/options/authentication_test.go +++ b/pkg/kubeapiserver/options/authentication_test.go @@ -437,11 +437,12 @@ func TestBuiltInAuthenticationOptionsAddFlags(t *testing.T) { AllowedNames: []string{"kube-aggregator"}, }, ServiceAccounts: &ServiceAccountAuthenticationOptions{ - KeyFiles: []string{"cert", "key"}, - Lookup: true, - Issuers: []string{"http://foo.bar.com"}, - JWKSURI: "https://qux.com", - ExtendExpiration: true, + KeyFiles: []string{"cert", "key"}, + Lookup: true, + Issuers: []string{"http://foo.bar.com"}, + JWKSURI: "https://qux.com", + ExtendExpiration: true, + MaxExtendedExpiration: serviceaccount.ExpirationExtensionSeconds * time.Second, }, TokenFile: &TokenFileAuthenticationOptions{ TokenFile: "tokenfile", diff --git a/pkg/registry/core/rest/storage_core.go b/pkg/registry/core/rest/storage_core.go index 0da3e488e39..da2384aae05 100644 --- a/pkg/registry/core/rest/storage_core.go +++ b/pkg/registry/core/rest/storage_core.go @@ -223,7 +223,7 @@ func (p *legacyProvider) NewRESTStorage(apiResourceConfigSource serverstorage.AP utilfeature.DefaultFeatureGate.Enabled(features.ServiceAccountTokenPodNodeInfo) { nodeGetter = nodeStorage.Node.Store } - serviceAccountStorage, err = serviceaccountstore.NewREST(restOptionsGetter, p.ServiceAccountIssuer, p.APIAudiences, p.ServiceAccountMaxExpiration, podStorage.Pod.Store, storage["secrets"].(rest.Getter), nodeGetter, p.ExtendExpiration, p.IsTokenSignerExternal) + serviceAccountStorage, err = serviceaccountstore.NewREST(restOptionsGetter, p.ServiceAccountIssuer, p.APIAudiences, p.ServiceAccountMaxExpiration, podStorage.Pod.Store, storage["secrets"].(rest.Getter), nodeGetter, p.ExtendExpiration, p.MaxExtendedExpiration) if err != nil { return genericapiserver.APIGroupInfo{}, err } diff --git a/pkg/registry/core/rest/storage_core_generic.go b/pkg/registry/core/rest/storage_core_generic.go index 664724f3bc0..26a9f293186 100644 --- a/pkg/registry/core/rest/storage_core_generic.go +++ b/pkg/registry/core/rest/storage_core_generic.go @@ -57,7 +57,7 @@ type GenericConfig struct { ServiceAccountIssuer serviceaccount.TokenGenerator ServiceAccountMaxExpiration time.Duration ExtendExpiration bool - IsTokenSignerExternal bool + MaxExtendedExpiration time.Duration APIAudiences authenticator.Audiences @@ -103,9 +103,9 @@ func (c *GenericConfig) NewRESTStorage(apiResourceConfigSource serverstorage.API var serviceAccountStorage *serviceaccountstore.REST if c.ServiceAccountIssuer != nil { - serviceAccountStorage, err = serviceaccountstore.NewREST(restOptionsGetter, c.ServiceAccountIssuer, c.APIAudiences, c.ServiceAccountMaxExpiration, newNotFoundGetter(schema.GroupResource{Resource: "pods"}), secretStorage.Store, newNotFoundGetter(schema.GroupResource{Resource: "nodes"}), c.ExtendExpiration, c.IsTokenSignerExternal) + serviceAccountStorage, err = serviceaccountstore.NewREST(restOptionsGetter, c.ServiceAccountIssuer, c.APIAudiences, c.ServiceAccountMaxExpiration, newNotFoundGetter(schema.GroupResource{Resource: "pods"}), secretStorage.Store, newNotFoundGetter(schema.GroupResource{Resource: "nodes"}), c.ExtendExpiration, c.MaxExtendedExpiration) } else { - serviceAccountStorage, err = serviceaccountstore.NewREST(restOptionsGetter, nil, nil, 0, newNotFoundGetter(schema.GroupResource{Resource: "pods"}), newNotFoundGetter(schema.GroupResource{Resource: "secrets"}), newNotFoundGetter(schema.GroupResource{Resource: "nodes"}), false, c.IsTokenSignerExternal) + serviceAccountStorage, err = serviceaccountstore.NewREST(restOptionsGetter, nil, nil, 0, newNotFoundGetter(schema.GroupResource{Resource: "pods"}), newNotFoundGetter(schema.GroupResource{Resource: "secrets"}), newNotFoundGetter(schema.GroupResource{Resource: "nodes"}), false, c.MaxExtendedExpiration) } if err != nil { return genericapiserver.APIGroupInfo{}, err diff --git a/pkg/registry/core/serviceaccount/storage/storage.go b/pkg/registry/core/serviceaccount/storage/storage.go index d6bbaf5c33d..c1a4d250d71 100644 --- a/pkg/registry/core/serviceaccount/storage/storage.go +++ b/pkg/registry/core/serviceaccount/storage/storage.go @@ -39,7 +39,7 @@ type REST struct { } // NewREST returns a RESTStorage object that will work against service accounts. -func NewREST(optsGetter generic.RESTOptionsGetter, issuer token.TokenGenerator, auds authenticator.Audiences, max time.Duration, podStorage, secretStorage, nodeStorage rest.Getter, extendExpiration bool, isTokenSignerExternal bool) (*REST, error) { +func NewREST(optsGetter generic.RESTOptionsGetter, issuer token.TokenGenerator, auds authenticator.Audiences, max time.Duration, podStorage, secretStorage, nodeStorage rest.Getter, extendExpiration bool, maxExtendedExpiration time.Duration) (*REST, error) { store := &genericregistry.Store{ NewFunc: func() runtime.Object { return &api.ServiceAccount{} }, NewListFunc: func() runtime.Object { return &api.ServiceAccountList{} }, @@ -61,16 +61,16 @@ func NewREST(optsGetter generic.RESTOptionsGetter, issuer token.TokenGenerator, var trest *TokenREST if issuer != nil && podStorage != nil && secretStorage != nil { trest = &TokenREST{ - svcaccts: store, - pods: podStorage, - secrets: secretStorage, - nodes: nodeStorage, - issuer: issuer, - auds: auds, - audsSet: sets.NewString(auds...), - maxExpirationSeconds: int64(max.Seconds()), - extendExpiration: extendExpiration, - isTokenSignerExternal: isTokenSignerExternal, + svcaccts: store, + pods: podStorage, + secrets: secretStorage, + nodes: nodeStorage, + issuer: issuer, + auds: auds, + audsSet: sets.NewString(auds...), + maxExpirationSeconds: int64(max.Seconds()), + maxExtendedExpirationSeconds: int64(maxExtendedExpiration.Seconds()), + extendExpiration: extendExpiration, } } diff --git a/pkg/registry/core/serviceaccount/storage/storage_test.go b/pkg/registry/core/serviceaccount/storage/storage_test.go index 1e0d6ec9d44..c77602f2ad7 100644 --- a/pkg/registry/core/serviceaccount/storage/storage_test.go +++ b/pkg/registry/core/serviceaccount/storage/storage_test.go @@ -19,6 +19,7 @@ package storage import ( "context" "testing" + "time" "gopkg.in/go-jose/go-jose.v2/jwt" @@ -55,7 +56,7 @@ func newTokenStorage(t *testing.T, issuer token.TokenGenerator, auds authenticat ResourcePrefix: "serviceaccounts", } // set issuer, podStore and secretStore to allow the token endpoint to be initialised - rest, err := NewREST(restOptions, issuer, auds, 0, podStorage, secretStorage, nodeStorage, false, false) + rest, err := NewREST(restOptions, issuer, auds, 0, podStorage, secretStorage, nodeStorage, false, time.Hour*9999) if err != nil { t.Fatalf("unexpected error from REST storage: %v", err) } diff --git a/pkg/registry/core/serviceaccount/storage/token.go b/pkg/registry/core/serviceaccount/storage/token.go index 2cf899203db..c3e46bf796e 100644 --- a/pkg/registry/core/serviceaccount/storage/token.go +++ b/pkg/registry/core/serviceaccount/storage/token.go @@ -56,16 +56,16 @@ func (r *TokenREST) Destroy() { } type TokenREST struct { - svcaccts rest.Getter - pods rest.Getter - secrets rest.Getter - nodes rest.Getter - issuer token.TokenGenerator - auds authenticator.Audiences - audsSet sets.String - maxExpirationSeconds int64 - extendExpiration bool - isTokenSignerExternal bool + svcaccts rest.Getter + pods rest.Getter + secrets rest.Getter + nodes rest.Getter + issuer token.TokenGenerator + auds authenticator.Audiences + audsSet sets.String + maxExpirationSeconds int64 + extendExpiration bool + maxExtendedExpirationSeconds int64 } var _ = rest.NamedCreater(&TokenREST{}) @@ -218,13 +218,7 @@ func (r *TokenREST) Create(ctx context.Context, name string, obj runtime.Object, exp := req.Spec.ExpirationSeconds if r.extendExpiration && pod != nil && req.Spec.ExpirationSeconds == token.WarnOnlyBoundTokenExpirationSeconds && r.isKubeAudiences(req.Spec.Audiences) { warnAfter = exp - // If token issuer is external-jwt-signer, then choose the smaller of - // ExpirationExtensionSeconds and max token lifetime supported by external signer. - if r.isTokenSignerExternal { - exp = min(r.maxExpirationSeconds, token.ExpirationExtensionSeconds) - } else { - exp = token.ExpirationExtensionSeconds - } + exp = r.maxExtendedExpirationSeconds } sc, pc, err := token.Claims(*svcacct, pod, secret, node, exp, warnAfter, req.Spec.Audiences) diff --git a/pkg/registry/core/serviceaccount/storage/token_test.go b/pkg/registry/core/serviceaccount/storage/token_test.go index 355af79dd42..89b13fe34b1 100644 --- a/pkg/registry/core/serviceaccount/storage/token_test.go +++ b/pkg/registry/core/serviceaccount/storage/token_test.go @@ -42,46 +42,53 @@ import ( func TestCreate_Token_WithExpiryCap(t *testing.T) { testcases := []struct { - desc string - extendExpiration bool - maxExpirationSeconds int - expectedTokenAgeSec int - isExternal bool + desc string + extendExpiration bool + maxExpirationSeconds int + maxExtendedExpirationSeconds int + expectedTokenAgeSec int }{ { - desc: "maxExpirationSeconds honoured", - extendExpiration: true, - maxExpirationSeconds: 5 * 60 * 60, // 5h - expectedTokenAgeSec: 5 * 60 * 60, // 5h - isExternal: true, + desc: "passed expiration respected if less than max", + extendExpiration: false, + maxExpirationSeconds: 5 * 60 * 60, // 5h + maxExtendedExpirationSeconds: token.ExpirationExtensionSeconds, // 1y + expectedTokenAgeSec: token.WarnOnlyBoundTokenExpirationSeconds, // 1h 7s }, { - desc: "ExpirationExtensionSeconds used for exp", - extendExpiration: true, - maxExpirationSeconds: 2 * 365 * 24 * 60 * 60, // 2 years - expectedTokenAgeSec: token.ExpirationExtensionSeconds, // 1y - isExternal: true, + desc: "maxExtendedExpirationSeconds honoured", + extendExpiration: true, + maxExpirationSeconds: 2 * 60 * 60, // 2h + maxExtendedExpirationSeconds: 5 * 60 * 60, // 5h + expectedTokenAgeSec: 5 * 60 * 60, // 5h }, { - desc: "ExpirationExtensionSeconds used for exp", - extendExpiration: true, - maxExpirationSeconds: 5 * 60 * 60, // 5h - expectedTokenAgeSec: token.ExpirationExtensionSeconds, // 1y - isExternal: false, + desc: "ExpirationExtensionSeconds used for exp", + extendExpiration: true, + maxExpirationSeconds: 2 * 365 * 24 * 60 * 60, // 2y + maxExtendedExpirationSeconds: token.ExpirationExtensionSeconds, // 1y + expectedTokenAgeSec: token.ExpirationExtensionSeconds, // 1y }, { - desc: "requested time use with extension disabled", - extendExpiration: false, - maxExpirationSeconds: 5 * 60 * 60, // 5h - expectedTokenAgeSec: 3607, // 1h - isExternal: true, + desc: "ExpirationSeconds used for exp", + extendExpiration: true, + maxExpirationSeconds: 5 * 60 * 60, // 5h + maxExtendedExpirationSeconds: token.ExpirationExtensionSeconds, // 1y + expectedTokenAgeSec: token.ExpirationExtensionSeconds, // 1y }, { - desc: "maxExpirationSeconds honoured with extension disabled", - extendExpiration: false, - maxExpirationSeconds: 30 * 60, // 30m - expectedTokenAgeSec: 30 * 60, // 30m - isExternal: true, + desc: "requested time use with extension disabled", + extendExpiration: false, + maxExpirationSeconds: 5 * 60 * 60, // 5h + expectedTokenAgeSec: 3607, // 1h + maxExtendedExpirationSeconds: token.ExpirationExtensionSeconds, + }, + { + desc: "maxExpirationSeconds honoured with extension disabled", + extendExpiration: false, + maxExpirationSeconds: 30 * 60, // 30m + expectedTokenAgeSec: 30 * 60, // 30m + maxExtendedExpirationSeconds: token.ExpirationExtensionSeconds, }, } @@ -127,7 +134,7 @@ func TestCreate_Token_WithExpiryCap(t *testing.T) { ctx = request.WithNamespace(ctx, serviceAccount.Namespace) storage.Token.extendExpiration = tc.extendExpiration storage.Token.maxExpirationSeconds = int64(tc.maxExpirationSeconds) - storage.Token.isTokenSignerExternal = tc.isExternal + storage.Token.maxExtendedExpirationSeconds = int64(tc.maxExtendedExpirationSeconds) tokenReqTimeStamp := time.Now() out, err := storage.Token.Create(ctx, serviceAccount.Name, &authenticationapi.TokenRequest{ @@ -161,13 +168,15 @@ func TestCreate_Token_WithExpiryCap(t *testing.T) { t.Fatalf("Error unmarshalling Claims: %v", err) } structuredClaim.Expiry.Time() - upperBound := tokenReqTimeStamp.Add(time.Duration(tc.expectedTokenAgeSec+10) * time.Second) - lowerBound := tokenReqTimeStamp.Add(time.Duration(tc.expectedTokenAgeSec-10) * time.Second) + confidenceInterval := 10 // seconds + upperBound := tokenReqTimeStamp.Add(time.Duration(tc.expectedTokenAgeSec+confidenceInterval) * time.Second) + lowerBound := tokenReqTimeStamp.Add(time.Duration(tc.expectedTokenAgeSec-confidenceInterval) * time.Second) // check for token expiration with a toleration of +/-10s after tokenReqTimeStamp to make for latencies. if structuredClaim.Expiry.Time().After(upperBound) || structuredClaim.Expiry.Time().Before(lowerBound) { - t.Fatalf("expected token expiration to be between %v to %v\n was %v", upperBound, lowerBound, structuredClaim.Expiry.Time()) + expiryDiff := structuredClaim.Expiry.Time().Sub(tokenReqTimeStamp) + t.Fatalf("expected token expiration to be %v (±%ds) in the future, was %v", time.Duration(tc.expectedTokenAgeSec)*time.Second, confidenceInterval, expiryDiff) } })