From c50f68d6eef33079e44f5cd8f658e8d08d09708d Mon Sep 17 00:00:00 2001 From: Jordan Liggitt Date: Mon, 17 Jun 2024 11:08:30 -0400 Subject: [PATCH] Fix structured authorization webhook timeout wiring --- pkg/kubeapiserver/authorizer/reload.go | 3 +++ .../plugin/pkg/authorizer/webhook/webhook.go | 3 ++- test/integration/auth/authz_config_test.go | 16 ++++++++++++++++ 3 files changed, 21 insertions(+), 1 deletion(-) diff --git a/pkg/kubeapiserver/authorizer/reload.go b/pkg/kubeapiserver/authorizer/reload.go index b7cb17a94a3..afd3ca76be6 100644 --- a/pkg/kubeapiserver/authorizer/reload.go +++ b/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: diff --git a/staging/src/k8s.io/apiserver/plugin/pkg/authorizer/webhook/webhook.go b/staging/src/k8s.io/apiserver/plugin/pkg/authorizer/webhook/webhook.go index d97b121453a..589899d7234 100644 --- a/staging/src/k8s.io/apiserver/plugin/pkg/authorizer/webhook/webhook.go +++ b/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" diff --git a/test/integration/auth/authz_config_test.go b/test/integration/auth/authz_config_test.go index 7b0af962b66..fdb967bb104 100644 --- a/test/integration/auth/authz_config_test.go +++ b/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)