UPSTREAM: 97820: 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.

staging/src/k8s.io/apiserver/pkg/util/webhook/webhook_test.go

@@ -712,6 +712,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

staging/src/k8s.io/apiserver/plugin/pkg/authenticator/token/webhook/webhook.go

@@ -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

staging/src/k8s.io/apiserver/plugin/pkg/authorizer/webhook/webhook.go

@@ -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 {