Merge pull request #97820 from tkashem/webhook-error

prevent panic on webhook authenticator and authorizer timeout before response

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

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

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 {