handle webhook authenticator and authorizer error

webhook.WithExponentialBackoff returns an error, and the priority is:
- A: if the last invocation of the webhook function returned an error
  that error should be returned, otherwise
- B: the error associated with the context if it has been canceled or
  it has expired, or the ErrWaitTimeout returned by the wait package
  once all retries have been exhausted.

caller should check the error returned by webhook.WithExponentialBackoff
to handle both A and B. Currently, we only handle A.
This commit is contained in:
Abu Kashem 2021-01-07 16:14:18 -05:00
parent 84b4569390
commit ae2b353fbf
No known key found for this signature in database
GPG Key ID: 76146D1A14E658ED
3 changed files with 41 additions and 16 deletions

View File

@ -711,6 +711,33 @@ func TestWithExponentialBackoffWebhookErrorIsMostImportant(t *testing.T) {
}
}
func TestWithExponentialBackoffWithRetryExhaustedWhileContextIsNotCanceled(t *testing.T) {
alwaysRetry := func(e error) bool {
return true
}
ctx, cancel := context.WithCancel(context.TODO())
defer cancel()
attemptsGot := 0
errExpected := errors.New("webhook not available")
webhookFunc := func() error {
attemptsGot++
return errExpected
}
// webhook err has higher priority than ctx error. we expect the webhook error to be returned.
retryBackoff := wait.Backoff{Steps: 5}
err := WithExponentialBackoff(ctx, retryBackoff, webhookFunc, alwaysRetry)
if attemptsGot != 5 {
t.Errorf("expected %d webhook attempts, but got: %d", 1, attemptsGot)
}
if errExpected != err {
t.Errorf("expected error: %v, but got: %v", errExpected, err)
}
}
func TestWithExponentialBackoffParametersNotSet(t *testing.T) {
alwaysRetry := func(e error) bool {
return true

View File

@ -104,14 +104,14 @@ func (w *WebhookTokenAuthenticator) AuthenticateToken(ctx context.Context, token
}
var (
result *authenticationv1.TokenReview
err error
auds authenticator.Audiences
)
webhook.WithExponentialBackoff(ctx, w.retryBackoff, func() error {
result, err = w.tokenReview.Create(ctx, r, metav1.CreateOptions{})
return err
}, webhook.DefaultShouldRetry)
if err != nil {
// WithExponentialBackoff will return tokenreview create error (tokenReviewErr) if any.
if err := webhook.WithExponentialBackoff(ctx, w.retryBackoff, func() error {
var tokenReviewErr error
result, tokenReviewErr = w.tokenReview.Create(ctx, r, metav1.CreateOptions{})
return tokenReviewErr
}, webhook.DefaultShouldRetry); err != nil {
// An error here indicates bad configuration or an outage. Log for debugging.
klog.Errorf("Failed to make webhook authenticator request: %v", err)
return nil, false, err

View File

@ -192,19 +192,17 @@ func (w *WebhookAuthorizer) Authorize(ctx context.Context, attr authorizer.Attri
if entry, ok := w.responseCache.Get(string(key)); ok {
r.Status = entry.(authorizationv1.SubjectAccessReviewStatus)
} else {
var (
result *authorizationv1.SubjectAccessReview
err error
)
webhook.WithExponentialBackoff(ctx, w.retryBackoff, func() error {
result, err = w.subjectAccessReview.Create(ctx, r, metav1.CreateOptions{})
return err
}, webhook.DefaultShouldRetry)
if err != nil {
// An error here indicates bad configuration or an outage. Log for debugging.
var result *authorizationv1.SubjectAccessReview
// WithExponentialBackoff will return SAR create error (sarErr) if any.
if err := webhook.WithExponentialBackoff(ctx, w.retryBackoff, func() error {
var sarErr error
result, sarErr = w.subjectAccessReview.Create(ctx, r, metav1.CreateOptions{})
return sarErr
}, webhook.DefaultShouldRetry); err != nil {
klog.Errorf("Failed to make webhook authorizer request: %v", err)
return w.decisionOnError, "", err
}
r.Status = result.Status
if shouldCache(attr) {
if r.Status.Allowed {