Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Automated cherry pick of #100959: DelegatingAuthenticationOptions TokenReview request timeout #101103

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion cmd/kube-controller-manager/app/options/options_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -412,7 +412,7 @@ func TestAddFlags(t *testing.T) {
}).WithLoopback(),
Authentication: &apiserveroptions.DelegatingAuthenticationOptions{
CacheTTL: 10 * time.Second,
ClientTimeout: 10 * time.Second,
TokenRequestTimeout: 10 * time.Second,
WebhookRetryBackoff: apiserveroptions.DefaultAuthWebhookRetryBackoff(),
ClientCert: apiserveroptions.ClientCertAuthenticationOptions{},
RequestHeader: apiserveroptions.RequestHeaderAuthenticationOptions{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
}
Expand Down
24 changes: 14 additions & 10 deletions staging/src/k8s.io/apiserver/pkg/server/options/authentication.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -211,7 +211,7 @@ func NewDelegatingAuthenticationOptions() *DelegatingAuthenticationOptions {
ExtraHeaderPrefixes: []string{"x-remote-extra-"},
},
WebhookRetryBackoff: DefaultAuthWebhookRetryBackoff(),
ClientTimeout: 10 * time.Second,
TokenRequestTimeout: 10 * time.Second,
}
}

Expand All @@ -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 {
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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)
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
4 changes: 2 additions & 2 deletions staging/src/k8s.io/cloud-provider/options/options_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ func TestDefaultFlags(t *testing.T) {
}).WithLoopback(),
Authentication: &apiserveroptions.DelegatingAuthenticationOptions{
CacheTTL: 10 * time.Second,
ClientTimeout: 10 * time.Second,
TokenRequestTimeout: 10 * time.Second,
WebhookRetryBackoff: apiserveroptions.DefaultAuthWebhookRetryBackoff(),
ClientCert: apiserveroptions.ClientCertAuthenticationOptions{},
RequestHeader: apiserveroptions.RequestHeaderAuthenticationOptions{
Expand Down Expand Up @@ -240,7 +240,7 @@ func TestAddFlags(t *testing.T) {
}).WithLoopback(),
Authentication: &apiserveroptions.DelegatingAuthenticationOptions{
CacheTTL: 10 * time.Second,
ClientTimeout: 10 * time.Second,
TokenRequestTimeout: 10 * time.Second,
WebhookRetryBackoff: apiserveroptions.DefaultAuthWebhookRetryBackoff(),
ClientCert: apiserveroptions.ClientCertAuthenticationOptions{},
RequestHeader: apiserveroptions.RequestHeaderAuthenticationOptions{
Expand Down