Merge pull request kubernetes#125552 from liggitt/fix-timeout-wiring

KEP-3221: Fix structured authorization webhook timeout wiring and detection

Author: k8s-ci-robot

pkg/kubeapiserver/authorizer/reload.go

@@ -128,6 +128,9 @@ func (r *reloadableAuthorizerResolver) newForConfig(authzConfig *authzconfig.Aut
 			if err != nil {
 				return nil, nil, err
 			}
+			if configuredAuthorizer.Webhook.Timeout.Duration != 0 {
+				clientConfig.Timeout = configuredAuthorizer.Webhook.Timeout.Duration
+			}
 			var decisionOnError authorizer.Decision
 			switch configuredAuthorizer.Webhook.FailurePolicy {
 			case authzconfig.FailurePolicyNoOpinion:

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

@@ -33,6 +33,7 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/util/cache"
+	utilnet "k8s.io/apimachinery/pkg/util/net"
 	"k8s.io/apimachinery/pkg/util/wait"
 	"k8s.io/apiserver/pkg/apis/apiserver"
 	apiservervalidation "k8s.io/apiserver/pkg/apis/apiserver/validation"
@@ -251,7 +252,7 @@ func (w *WebhookAuthorizer) Authorize(ctx context.Context, attr authorizer.Attri
 				metricsResult = "success"
 			case ctx.Err() != nil:
 				metricsResult = "canceled"
-			case errors.Is(sarErr, context.DeadlineExceeded) || apierrors.IsTimeout(sarErr) || statusCode == http.StatusGatewayTimeout:
+			case utilnet.IsTimeout(sarErr) || errors.Is(sarErr, context.DeadlineExceeded) || apierrors.IsTimeout(sarErr) || statusCode == http.StatusGatewayTimeout:
 				metricsResult = "timeout"
 			default:
 				metricsResult = "error"

test/integration/auth/authz_config_test.go

@@ -177,6 +177,10 @@ users:
 		}
 		t.Log("serverTimeout", sar)
 		time.Sleep(2 * time.Second)
+		sar.Status.Reason = "no opinion"
+		if err := json.NewEncoder(w).Encode(sar); err != nil {
+			t.Error(err)
+		}
 	}))
 	defer serverTimeout.Close()
 	serverTimeoutKubeconfigName := filepath.Join(dir, "serverTimeout.yaml")
@@ -331,6 +335,13 @@ users:
 
 		assertCount(errorName, c.errorCount, &serverErrorCalled)
 		assertCount(timeoutName, c.timeoutCount, &serverTimeoutCalled)
+		expectedTimeoutCounts := map[string]int{}
+		if c.timeoutCount > 0 {
+			expectedTimeoutCounts[timeoutName] = int(c.timeoutCount)
+		}
+		if !reflect.DeepEqual(expectedTimeoutCounts, metrics.whTimeoutTotal) {
+			t.Fatalf("expected timeouts %#v, got %#v", expectedTimeoutCounts, metrics.whTimeoutTotal)
+		}
 		assertCount(denyName, c.denyCount, &serverDenyCalled)
 		if e, a := c.denyCount, metrics.decisions[authorizerKey{authorizerType: "Webhook", authorizerName: denyName}]["denied"]; e != int32(a) {
 			t.Fatalf("expected deny webhook denied metrics calls: %d, got %d", e, a)
@@ -770,6 +781,7 @@ type metrics struct {
 	evalErrors    int
 
 	whTotal         map[string]int
+	whTimeoutTotal  map[string]int
 	whFailOpenTotal map[string]int
 	whDurationCount map[string]int
 }
@@ -803,6 +815,7 @@ func getMetrics(t *testing.T, client *clientset.Clientset) (*metrics, error) {
 	var m metrics
 
 	m.whTotal = map[string]int{}
+	m.whTimeoutTotal = map[string]int{}
 	m.whFailOpenTotal = map[string]int{}
 	m.whDurationCount = map[string]int{}
 	m.exclusions = 0
@@ -849,6 +862,9 @@ func getMetrics(t *testing.T, client *clientset.Clientset) (*metrics, error) {
 			}
 			t.Log(count)
 			m.whTotal[matches[1]] += count
+			if matches[2] == "timeout" {
+				m.whTimeoutTotal[matches[1]] += count
+			}
 		}
 		if matches := webhookDurationMetric.FindStringSubmatch(line); matches != nil {
 			t.Log(matches)