Merge pull request #101102 from p0lyn0mial/automated-cherry-pick-of-#…

…100959-upstream-release-1.21

Automated cherry pick of #100959: DelegatingAuthenticationOptions TokenReview request timeout

Kubernetes-commit: 4982ce134034e917708b3ea6038a8096113d749c

go.mod

@@ -42,8 +42,8 @@ require (
 	gopkg.in/natefinch/lumberjack.v2 v2.0.0
 	gopkg.in/square/go-jose.v2 v2.2.2
 	gopkg.in/yaml.v2 v2.4.0
-	k8s.io/api v0.0.0-20210329112114-1f635bfa4973
-	k8s.io/apimachinery v0.0.0-20210329111815-e337f44144a6
+	k8s.io/api v0.0.0-20210518102353-e379db4b881d
+	k8s.io/apimachinery v0.0.0-20210421190351-be0c81341c37
 	k8s.io/client-go v0.0.0-20210401152202-307e3a38a167
 	k8s.io/component-base v0.0.0-20210329113144-5feaa80744bd
 	k8s.io/klog/v2 v2.8.0
@@ -55,8 +55,8 @@ require (
 )
 
 replace (
-	k8s.io/api => k8s.io/api v0.0.0-20210329112114-1f635bfa4973
-	k8s.io/apimachinery => k8s.io/apimachinery v0.0.0-20210329111815-e337f44144a6
+	k8s.io/api => k8s.io/api v0.0.0-20210518102353-e379db4b881d
+	k8s.io/apimachinery => k8s.io/apimachinery v0.0.0-20210421190351-be0c81341c37
 	k8s.io/client-go => k8s.io/client-go v0.0.0-20210401152202-307e3a38a167
 	k8s.io/component-base => k8s.io/component-base v0.0.0-20210329113144-5feaa80744bd
 )

go.sum

@@ -589,13 +589,13 @@ honnef.co/go/tools v0.0.0-20190418001031-e561f6794a2a/go.mod h1:rf3lG4BRIbNafJWh
 honnef.co/go/tools v0.0.0-20190523083050-ea95bdfd59fc/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
 honnef.co/go/tools v0.0.1-2019.2.3/go.mod h1:a3bituU0lyd329TUQxRnasdCoJDkEUEAqEt0JzvZhAg=
 honnef.co/go/tools v0.0.1-2020.1.3/go.mod h1:X/FiERA/W4tHapMX5mGpAtMSVEeEUOyHaw9vFzvIQ3k=
-k8s.io/api v0.0.0-20210329112114-1f635bfa4973 h1:lpbSfpknIINyTQKly4k/FjqxmODVfgQHgmVCqbL/1qA=
-k8s.io/api v0.0.0-20210329112114-1f635bfa4973/go.mod h1:bbunxWRHC0NymRWrp/wsI0V/Zs/w4UBMliggHDfC1xw=
-k8s.io/apimachinery v0.0.0-20210329111815-e337f44144a6 h1:rF9UWPFMeCzTx61QJd1PO4vZmpe8eNmJRuFGfZBG1r4=
-k8s.io/apimachinery v0.0.0-20210329111815-e337f44144a6/go.mod h1:jbreFvJo3ov9rj7eWT7+sYiRx+qZuCYXwWT1bcDswPY=
-k8s.io/client-go v0.0.0-20210401152202-307e3a38a167 h1:6dfdaZ0/RfxMbyUuuKL4xdk4+DVjkO76nmb6z4S+bn0=
+k8s.io/api v0.0.0-20210518102353-e379db4b881d h1:DHzIPPwk8Y05yWpj33Kz8KaRnosOODhmDwLAaJjA8Rs=
+k8s.io/api v0.0.0-20210518102353-e379db4b881d/go.mod h1:80FDcYLi/0zAklIq9x0fk3F7zTkqxDN4WUvnc7KD0pI=
+k8s.io/apimachinery v0.0.0-20210421190351-be0c81341c37 h1:TunYeninmgd1K1OI/QQUDmg0HMhq71kfraPk9EbGlRc=
+k8s.io/apimachinery v0.0.0-20210421190351-be0c81341c37/go.mod h1:jbreFvJo3ov9rj7eWT7+sYiRx+qZuCYXwWT1bcDswPY=
+k8s.io/client-go v0.0.0-20210401152202-307e3a38a167 h1:opqPJ8l/hqiKIseiNVxcUiYF0vc8gAcyy/rZZrxseTs=
 k8s.io/client-go v0.0.0-20210401152202-307e3a38a167/go.mod h1:0NnunnaJsArENJKmzcOD5u2QcfS7Nf3fIzcyhNQI5GI=
-k8s.io/component-base v0.0.0-20210329113144-5feaa80744bd h1:PuPyv7L2fj1h4jyFAgFMhRgGZgoR3yqTuiVNGzskEW0=
+k8s.io/component-base v0.0.0-20210329113144-5feaa80744bd h1:tLLDsfLUt36KzVGAhprbwRc2lbhpKjcQbynsk+d+uAU=
 k8s.io/component-base v0.0.0-20210329113144-5feaa80744bd/go.mod h1:IEys5YfGZC2nKiSTFY0147SzFCqtO/jiMVVCR8FcTKQ=
 k8s.io/gengo v0.0.0-20200413195148-3a45101e95ac/go.mod h1:ezvh/TsK7cY6rbqRK0oQQ8IAqLxYwwyPxAX1Pzy0ii0=
 k8s.io/klog/v2 v2.0.0/go.mod h1:PBfzABfn139FHAV07az/IF9Wp1bkk3vpT2XSJ76fSDE=

pkg/authentication/authenticatorfactory/delegating.go

@@ -44,6 +44,9 @@ type DelegatingAuthenticatorConfig struct {
 	// TokenAccessReviewClient is a client to do token review. It can be nil. Then every token is ignored.
 	TokenAccessReviewClient authenticationclient.TokenReviewInterface
 
+	// TokenAccessReviewTimeout specifies a time limit for requests made by the authorization webhook client.
+	TokenAccessReviewTimeout time.Duration
+
 	// WebhookRetryBackoff specifies the backoff parameters for the authentication webhook retry logic.
 	// This allows us to configure the sleep time at each iteration and the maximum number of retries allowed
 	// before we fail the webhook call in order to limit the fan out that ensues when the system is degraded.
@@ -88,7 +91,7 @@ func (c DelegatingAuthenticatorConfig) New() (authenticator.Request, *spec.Secur
 		if c.WebhookRetryBackoff == nil {
 			return nil, nil, errors.New("retry backoff parameters for delegating authentication webhook has not been specified")
 		}
-		tokenAuth, err := webhooktoken.NewFromInterface(c.TokenAccessReviewClient, c.APIAudiences, *c.WebhookRetryBackoff)
+		tokenAuth, err := webhooktoken.NewFromInterface(c.TokenAccessReviewClient, c.APIAudiences, *c.WebhookRetryBackoff, c.TokenAccessReviewTimeout)
 		if err != nil {
 			return nil, nil, err
 		}

pkg/server/options/authentication.go

@@ -195,9 +195,9 @@ type DelegatingAuthenticationOptions struct {
 	// before we fail the webhook call in order to limit the fan out that ensues when the system is degraded.
 	WebhookRetryBackoff *wait.Backoff
 
-	// ClientTimeout specifies a time limit for requests made by the authorization webhook client.
+	// TokenRequestTimeout specifies a time limit for requests made by the authorization webhook client.
 	// The default value is set to 10 seconds.
-	ClientTimeout time.Duration
+	TokenRequestTimeout time.Duration
 }
 
 func NewDelegatingAuthenticationOptions() *DelegatingAuthenticationOptions {
@@ -211,7 +211,7 @@ func NewDelegatingAuthenticationOptions() *DelegatingAuthenticationOptions {
 			ExtraHeaderPrefixes: []string{"x-remote-extra-"},
 		},
 		WebhookRetryBackoff: DefaultAuthWebhookRetryBackoff(),
-		ClientTimeout:       10 * time.Second,
+		TokenRequestTimeout: 10 * time.Second,
 	}
 }
 
@@ -220,9 +220,9 @@ func (s *DelegatingAuthenticationOptions) WithCustomRetryBackoff(backoff wait.Ba
 	s.WebhookRetryBackoff = &backoff
 }
 
-// WithClientTimeout sets the given timeout for the authentication webhook client.
-func (s *DelegatingAuthenticationOptions) WithClientTimeout(timeout time.Duration) {
-	s.ClientTimeout = timeout
+// WithRequestTimeout sets the given timeout for requests made by the authentication webhook client.
+func (s *DelegatingAuthenticationOptions) WithRequestTimeout(timeout time.Duration) {
+	s.TokenRequestTimeout = timeout
 }
 
 func (s *DelegatingAuthenticationOptions) Validate() []error {
@@ -274,9 +274,10 @@ func (s *DelegatingAuthenticationOptions) ApplyTo(authenticationInfo *server.Aut
 	}
 
 	cfg := authenticatorfactory.DelegatingAuthenticatorConfig{
-		Anonymous:           true,
-		CacheTTL:            s.CacheTTL,
-		WebhookRetryBackoff: s.WebhookRetryBackoff,
+		Anonymous:                true,
+		CacheTTL:                 s.CacheTTL,
+		WebhookRetryBackoff:      s.WebhookRetryBackoff,
+		TokenAccessReviewTimeout: s.TokenRequestTimeout,
 	}
 
 	client, err := s.getClient()
@@ -419,7 +420,10 @@ func (s *DelegatingAuthenticationOptions) getClient() (kubernetes.Interface, err
 	// set high qps/burst limits since this will effectively limit API server responsiveness
 	clientConfig.QPS = 200
 	clientConfig.Burst = 400
-	clientConfig.Timeout = s.ClientTimeout
+	// do not set a timeout on the http client, instead use context for cancellation
+	// if multiple timeouts were set, the request will pick the smaller timeout to be applied, leaving other useless.
+	//
+	// see https://github.com/golang/go/blob/a937729c2c2f6950a32bc5cd0f5b88700882f078/src/net/http/client.go#L364
 
 	return kubernetes.NewForConfig(clientConfig)
 }

plugin/pkg/authenticator/token/webhook/webhook.go

@@ -52,17 +52,18 @@ type tokenReviewer interface {
 }
 
 type WebhookTokenAuthenticator struct {
-	tokenReview  tokenReviewer
-	retryBackoff wait.Backoff
-	implicitAuds authenticator.Audiences
+	tokenReview    tokenReviewer
+	retryBackoff   wait.Backoff
+	implicitAuds   authenticator.Audiences
+	requestTimeout time.Duration
 }
 
 // NewFromInterface creates a webhook authenticator using the given tokenReview
 // client. It is recommend to wrap this authenticator with the token cache
 // authenticator implemented in
 // k8s.io/apiserver/pkg/authentication/token/cache.
-func NewFromInterface(tokenReview authenticationv1client.TokenReviewInterface, implicitAuds authenticator.Audiences, retryBackoff wait.Backoff) (*WebhookTokenAuthenticator, error) {
-	return newWithBackoff(tokenReview, retryBackoff, implicitAuds)
+func NewFromInterface(tokenReview authenticationv1client.TokenReviewInterface, implicitAuds authenticator.Audiences, retryBackoff wait.Backoff, requestTimeout time.Duration) (*WebhookTokenAuthenticator, error) {
+	return newWithBackoff(tokenReview, retryBackoff, implicitAuds, requestTimeout)
 }
 
 // New creates a new WebhookTokenAuthenticator from the provided kubeconfig
@@ -74,12 +75,12 @@ func New(kubeConfigFile string, version string, implicitAuds authenticator.Audie
 	if err != nil {
 		return nil, err
 	}
-	return newWithBackoff(tokenReview, retryBackoff, implicitAuds)
+	return newWithBackoff(tokenReview, retryBackoff, implicitAuds, time.Duration(0))
 }
 
 // newWithBackoff allows tests to skip the sleep.
-func newWithBackoff(tokenReview tokenReviewer, retryBackoff wait.Backoff, implicitAuds authenticator.Audiences) (*WebhookTokenAuthenticator, error) {
-	return &WebhookTokenAuthenticator{tokenReview, retryBackoff, implicitAuds}, nil
+func newWithBackoff(tokenReview tokenReviewer, retryBackoff wait.Backoff, implicitAuds authenticator.Audiences, requestTimeout time.Duration) (*WebhookTokenAuthenticator, error) {
+	return &WebhookTokenAuthenticator{tokenReview, retryBackoff, implicitAuds, requestTimeout}, nil
 }
 
 // AuthenticateToken implements the authenticator.Token interface.
@@ -105,7 +106,17 @@ func (w *WebhookTokenAuthenticator) AuthenticateToken(ctx context.Context, token
 	var (
 		result *authenticationv1.TokenReview
 		auds   authenticator.Audiences
+		cancel context.CancelFunc
 	)
+
+	// set a hard timeout if it was defined
+	// if the child has a shorter deadline then it will expire first,
+	// otherwise if the parent has a shorter deadline then the parent will expire and it will be propagate to the child
+	if w.requestTimeout > 0 {
+		ctx, cancel = context.WithTimeout(ctx, w.requestTimeout)
+		defer cancel()
+	}
+
 	// WithExponentialBackoff will return tokenreview create error (tokenReviewErr) if any.
 	if err := webhook.WithExponentialBackoff(ctx, w.retryBackoff, func() error {
 		var tokenReviewErr error

plugin/pkg/authenticator/token/webhook/webhook_v1_test.go

@@ -206,7 +206,7 @@ func newV1TokenAuthenticator(serverURL string, clientCert, clientKey, ca []byte,
 		return nil, err
 	}
 
-	authn, err := newWithBackoff(c, testRetryBackoff, implicitAuds)
+	authn, err := newWithBackoff(c, testRetryBackoff, implicitAuds, 10*time.Second)
 	if err != nil {
 		return nil, err
 	}

plugin/pkg/authenticator/token/webhook/webhook_v1beta1_test.go

@@ -200,7 +200,7 @@ func newV1beta1TokenAuthenticator(serverURL string, clientCert, clientKey, ca []
 		return nil, err
 	}
 
-	authn, err := newWithBackoff(c, testRetryBackoff, implicitAuds)
+	authn, err := newWithBackoff(c, testRetryBackoff, implicitAuds, 10*time.Second)
 	if err != nil {
 		return nil, err
 	}