From a000af25ff3bcc79fe7d8da299225ad252c9894a Mon Sep 17 00:00:00 2001 From: Jordan Liggitt Date: Thu, 2 Nov 2023 13:54:39 -0400 Subject: [PATCH 1/4] Require match condition version only if matchConditions are specified --- .../apiserver/pkg/apis/apiserver/validation/validation.go | 4 +++- .../pkg/apis/apiserver/validation/validation_test.go | 1 + 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/staging/src/k8s.io/apiserver/pkg/apis/apiserver/validation/validation.go b/staging/src/k8s.io/apiserver/pkg/apis/apiserver/validation/validation.go index 7b22a200f96..843324085cf 100644 --- a/staging/src/k8s.io/apiserver/pkg/apis/apiserver/validation/validation.go +++ b/staging/src/k8s.io/apiserver/pkg/apis/apiserver/validation/validation.go @@ -512,7 +512,9 @@ func ValidateWebhookConfiguration(fldPath *field.Path, c *api.WebhookConfigurati switch c.MatchConditionSubjectAccessReviewVersion { case "": - allErrs = append(allErrs, field.Required(fldPath.Child("matchConditionSubjectAccessReviewVersion"), "")) + if len(c.MatchConditions) > 0 { + allErrs = append(allErrs, field.Required(fldPath.Child("matchConditionSubjectAccessReviewVersion"), "required if match conditions are specified")) + } case "v1": _ = &v1.SubjectAccessReview{} default: diff --git a/staging/src/k8s.io/apiserver/pkg/apis/apiserver/validation/validation_test.go b/staging/src/k8s.io/apiserver/pkg/apis/apiserver/validation/validation_test.go index ca688a0bee5..4be785d6a2d 100644 --- a/staging/src/k8s.io/apiserver/pkg/apis/apiserver/validation/validation_test.go +++ b/staging/src/k8s.io/apiserver/pkg/apis/apiserver/validation/validation_test.go @@ -1438,6 +1438,7 @@ func TestValidateAuthorizationConfiguration(t *testing.T) { ConnectionInfo: api.WebhookConnectionInfo{ Type: "InClusterConfig", }, + MatchConditions: []api.WebhookMatchCondition{{Expression: "true"}}, }, }, }, From 2e2f51a4417d93b5505091d28b319365dc95e137 Mon Sep 17 00:00:00 2001 From: Jordan Liggitt Date: Thu, 2 Nov 2023 13:55:35 -0400 Subject: [PATCH 2/4] Plumb failure policy from config to webhook construction --- pkg/kubeapiserver/authorizer/config.go | 10 ++++++++++ .../authorization/authorizerfactory/delegating.go | 1 + .../plugin/pkg/authorizer/webhook/webhook.go | 12 ++++++------ .../plugin/pkg/authorizer/webhook/webhook_v1_test.go | 5 +++-- .../pkg/authorizer/webhook/webhook_v1beta1_test.go | 5 +++-- 5 files changed, 23 insertions(+), 10 deletions(-) diff --git a/pkg/kubeapiserver/authorizer/config.go b/pkg/kubeapiserver/authorizer/config.go index dee62636220..b2bc689e420 100644 --- a/pkg/kubeapiserver/authorizer/config.go +++ b/pkg/kubeapiserver/authorizer/config.go @@ -118,11 +118,21 @@ func (config Config) New() (authorizer.Authorizer, authorizer.RuleResolver, erro if err != nil { return nil, nil, err } + var decisionOnError authorizer.Decision + switch configuredAuthorizer.Webhook.FailurePolicy { + case authzconfig.FailurePolicyNoOpinion: + decisionOnError = authorizer.DecisionNoOpinion + case authzconfig.FailurePolicyDeny: + decisionOnError = authorizer.DecisionDeny + default: + return nil, nil, fmt.Errorf("unknown failurePolicy %q", configuredAuthorizer.Webhook.FailurePolicy) + } webhookAuthorizer, err := webhook.New(clientConfig, configuredAuthorizer.Webhook.SubjectAccessReviewVersion, configuredAuthorizer.Webhook.AuthorizedTTL.Duration, configuredAuthorizer.Webhook.UnauthorizedTTL.Duration, *config.WebhookRetryBackoff, + decisionOnError, configuredAuthorizer.Webhook.MatchConditions, ) if err != nil { diff --git a/staging/src/k8s.io/apiserver/pkg/authorization/authorizerfactory/delegating.go b/staging/src/k8s.io/apiserver/pkg/authorization/authorizerfactory/delegating.go index d1ead25dbb2..a8355ee6191 100644 --- a/staging/src/k8s.io/apiserver/pkg/authorization/authorizerfactory/delegating.go +++ b/staging/src/k8s.io/apiserver/pkg/authorization/authorizerfactory/delegating.go @@ -54,6 +54,7 @@ func (c DelegatingAuthorizerConfig) New() (authorizer.Authorizer, error) { c.AllowCacheTTL, c.DenyCacheTTL, *c.WebhookRetryBackoff, + authorizer.DecisionNoOpinion, webhook.AuthorizerMetrics{ RecordRequestTotal: RecordRequestTotal, RecordRequestLatency: RecordRequestLatency, 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 594fdca19d8..29ee0e84d15 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 @@ -75,8 +75,8 @@ type WebhookAuthorizer struct { } // NewFromInterface creates a WebhookAuthorizer using the given subjectAccessReview client -func NewFromInterface(subjectAccessReview authorizationv1client.AuthorizationV1Interface, authorizedTTL, unauthorizedTTL time.Duration, retryBackoff wait.Backoff, metrics AuthorizerMetrics) (*WebhookAuthorizer, error) { - return newWithBackoff(&subjectAccessReviewV1Client{subjectAccessReview.RESTClient()}, authorizedTTL, unauthorizedTTL, retryBackoff, nil, metrics) +func NewFromInterface(subjectAccessReview authorizationv1client.AuthorizationV1Interface, authorizedTTL, unauthorizedTTL time.Duration, retryBackoff wait.Backoff, decisionOnError authorizer.Decision, metrics AuthorizerMetrics) (*WebhookAuthorizer, error) { + return newWithBackoff(&subjectAccessReviewV1Client{subjectAccessReview.RESTClient()}, authorizedTTL, unauthorizedTTL, retryBackoff, decisionOnError, nil, metrics) } // New creates a new WebhookAuthorizer from the provided kubeconfig file. @@ -98,19 +98,19 @@ func NewFromInterface(subjectAccessReview authorizationv1client.AuthorizationV1I // // For additional HTTP configuration, refer to the kubeconfig documentation // https://kubernetes.io/docs/user-guide/kubeconfig-file/. -func New(config *rest.Config, version string, authorizedTTL, unauthorizedTTL time.Duration, retryBackoff wait.Backoff, matchConditions []apiserver.WebhookMatchCondition) (*WebhookAuthorizer, error) { +func New(config *rest.Config, version string, authorizedTTL, unauthorizedTTL time.Duration, retryBackoff wait.Backoff, decisionOnError authorizer.Decision, matchConditions []apiserver.WebhookMatchCondition) (*WebhookAuthorizer, error) { subjectAccessReview, err := subjectAccessReviewInterfaceFromConfig(config, version, retryBackoff) if err != nil { return nil, err } - return newWithBackoff(subjectAccessReview, authorizedTTL, unauthorizedTTL, retryBackoff, matchConditions, AuthorizerMetrics{ + return newWithBackoff(subjectAccessReview, authorizedTTL, unauthorizedTTL, retryBackoff, decisionOnError, matchConditions, AuthorizerMetrics{ RecordRequestTotal: noopMetrics{}.RecordRequestTotal, RecordRequestLatency: noopMetrics{}.RecordRequestLatency, }) } // newWithBackoff allows tests to skip the sleep. -func newWithBackoff(subjectAccessReview subjectAccessReviewer, authorizedTTL, unauthorizedTTL time.Duration, retryBackoff wait.Backoff, matchConditions []apiserver.WebhookMatchCondition, metrics AuthorizerMetrics) (*WebhookAuthorizer, error) { +func newWithBackoff(subjectAccessReview subjectAccessReviewer, authorizedTTL, unauthorizedTTL time.Duration, retryBackoff wait.Backoff, decisionOnError authorizer.Decision, matchConditions []apiserver.WebhookMatchCondition, metrics AuthorizerMetrics) (*WebhookAuthorizer, error) { // compile all expressions once in validation and save the results to be used for eval later cm, fieldErr := apiservervalidation.ValidateAndCompileMatchConditions(matchConditions) if err := fieldErr.ToAggregate(); err != nil { @@ -122,7 +122,7 @@ func newWithBackoff(subjectAccessReview subjectAccessReviewer, authorizedTTL, un authorizedTTL: authorizedTTL, unauthorizedTTL: unauthorizedTTL, retryBackoff: retryBackoff, - decisionOnError: authorizer.DecisionNoOpinion, + decisionOnError: decisionOnError, metrics: metrics, celMatcher: cm, }, nil diff --git a/staging/src/k8s.io/apiserver/plugin/pkg/authorizer/webhook/webhook_v1_test.go b/staging/src/k8s.io/apiserver/plugin/pkg/authorizer/webhook/webhook_v1_test.go index ab3d6f3296b..a72169fc2ac 100644 --- a/staging/src/k8s.io/apiserver/plugin/pkg/authorizer/webhook/webhook_v1_test.go +++ b/staging/src/k8s.io/apiserver/plugin/pkg/authorizer/webhook/webhook_v1_test.go @@ -37,6 +37,7 @@ import ( utiltesting "k8s.io/client-go/util/testing" "github.com/google/go-cmp/cmp" + authorizationv1 "k8s.io/api/authorization/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/wait" @@ -209,7 +210,7 @@ current-context: default if err != nil { return fmt.Errorf("error building sar client: %v", err) } - _, err = newWithBackoff(sarClient, 0, 0, testRetryBackoff, []apiserver.WebhookMatchCondition{}, noopAuthorizerMetrics()) + _, err = newWithBackoff(sarClient, 0, 0, testRetryBackoff, authorizer.DecisionNoOpinion, []apiserver.WebhookMatchCondition{}, noopAuthorizerMetrics()) return err }() if err != nil && !tt.wantErr { @@ -352,7 +353,7 @@ func newV1Authorizer(callbackURL string, clientCert, clientKey, ca []byte, cache if err != nil { return nil, fmt.Errorf("error building sar client: %v", err) } - return newWithBackoff(sarClient, cacheTime, cacheTime, testRetryBackoff, expressions, metrics) + return newWithBackoff(sarClient, cacheTime, cacheTime, testRetryBackoff, authorizer.DecisionNoOpinion, expressions, metrics) } func TestV1TLSConfig(t *testing.T) { diff --git a/staging/src/k8s.io/apiserver/plugin/pkg/authorizer/webhook/webhook_v1beta1_test.go b/staging/src/k8s.io/apiserver/plugin/pkg/authorizer/webhook/webhook_v1beta1_test.go index 8cb6e6716f4..e2621c3ca75 100644 --- a/staging/src/k8s.io/apiserver/plugin/pkg/authorizer/webhook/webhook_v1beta1_test.go +++ b/staging/src/k8s.io/apiserver/plugin/pkg/authorizer/webhook/webhook_v1beta1_test.go @@ -35,6 +35,7 @@ import ( "time" "github.com/google/go-cmp/cmp" + authorizationv1beta1 "k8s.io/api/authorization/v1beta1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" authzconfig "k8s.io/apiserver/pkg/apis/apiserver" @@ -196,7 +197,7 @@ current-context: default if err != nil { return fmt.Errorf("error building sar client: %v", err) } - _, err = newWithBackoff(sarClient, 0, 0, testRetryBackoff, []authzconfig.WebhookMatchCondition{}, noopAuthorizerMetrics()) + _, err = newWithBackoff(sarClient, 0, 0, testRetryBackoff, authorizer.DecisionNoOpinion, []authzconfig.WebhookMatchCondition{}, noopAuthorizerMetrics()) return err }() if err != nil && !tt.wantErr { @@ -339,7 +340,7 @@ func newV1beta1Authorizer(callbackURL string, clientCert, clientKey, ca []byte, if err != nil { return nil, fmt.Errorf("error building sar client: %v", err) } - return newWithBackoff(sarClient, cacheTime, cacheTime, testRetryBackoff, []authzconfig.WebhookMatchCondition{}, noopAuthorizerMetrics()) + return newWithBackoff(sarClient, cacheTime, cacheTime, testRetryBackoff, authorizer.DecisionNoOpinion, []authzconfig.WebhookMatchCondition{}, noopAuthorizerMetrics()) } func TestV1beta1TLSConfig(t *testing.T) { From 44d89c8cf8c1ba883029e1244492a523d6b50b92 Mon Sep 17 00:00:00 2001 From: Jordan Liggitt Date: Thu, 2 Nov 2023 15:14:06 -0400 Subject: [PATCH 3/4] Include empty string attributes for CEL authz evaluation --- .../pkg/authorization/cel/compile.go | 36 +++++++++++++++++++ .../pkg/authorization/cel/matcher.go | 19 ++-------- .../pkg/authorizer/webhook/webhook_v1_test.go | 21 ++++++++--- 3 files changed, 54 insertions(+), 22 deletions(-) diff --git a/staging/src/k8s.io/apiserver/pkg/authorization/cel/compile.go b/staging/src/k8s.io/apiserver/pkg/authorization/cel/compile.go index e441c347d07..0d9293dd704 100644 --- a/staging/src/k8s.io/apiserver/pkg/authorization/cel/compile.go +++ b/staging/src/k8s.io/apiserver/pkg/authorization/cel/compile.go @@ -22,6 +22,7 @@ import ( "github.com/google/cel-go/cel" "github.com/google/cel-go/common/types/ref" + authorizationv1 "k8s.io/api/authorization/v1" "k8s.io/apimachinery/pkg/util/version" apiservercel "k8s.io/apiserver/pkg/cel" "k8s.io/apiserver/pkg/cel/environment" @@ -143,6 +144,7 @@ func mustBuildEnv(baseEnv *environment.EnvSet) *environment.EnvSet { } // buildRequestType generates a DeclType for SubjectAccessReviewSpec. +// if attributes are added here, also add to convertObjectToUnstructured. func buildRequestType(field func(name string, declType *apiservercel.DeclType, required bool) *apiservercel.DeclField, fields func(fields ...*apiservercel.DeclField) map[string]*apiservercel.DeclField) *apiservercel.DeclType { resourceAttributesType := buildResourceAttributesType(field, fields) nonResourceAttributesType := buildNonResourceAttributesType(field, fields) @@ -157,6 +159,7 @@ func buildRequestType(field func(name string, declType *apiservercel.DeclType, r } // buildResourceAttributesType generates a DeclType for ResourceAttributes. +// if attributes are added here, also add to convertObjectToUnstructured. func buildResourceAttributesType(field func(name string, declType *apiservercel.DeclType, required bool) *apiservercel.DeclField, fields func(fields ...*apiservercel.DeclField) map[string]*apiservercel.DeclField) *apiservercel.DeclType { return apiservercel.NewObjectType("kubernetes.ResourceAttributes", fields( field("namespace", apiservercel.StringType, false), @@ -170,9 +173,42 @@ func buildResourceAttributesType(field func(name string, declType *apiservercel. } // buildNonResourceAttributesType generates a DeclType for NonResourceAttributes. +// if attributes are added here, also add to convertObjectToUnstructured. func buildNonResourceAttributesType(field func(name string, declType *apiservercel.DeclType, required bool) *apiservercel.DeclField, fields func(fields ...*apiservercel.DeclField) map[string]*apiservercel.DeclField) *apiservercel.DeclType { return apiservercel.NewObjectType("kubernetes.NonResourceAttributes", fields( field("path", apiservercel.StringType, false), field("verb", apiservercel.StringType, false), )) } + +func convertObjectToUnstructured(obj *authorizationv1.SubjectAccessReviewSpec) map[string]interface{} { + // Construct version containing every SubjectAccessReview user and string attribute field, even omitempty ones, for evaluation by CEL + extra := obj.Extra + if extra == nil { + extra = map[string]authorizationv1.ExtraValue{} + } + ret := map[string]interface{}{ + "user": obj.User, + "groups": obj.Groups, + "uid": string(obj.UID), + "extra": extra, + } + if obj.ResourceAttributes != nil { + ret["resourceAttributes"] = map[string]string{ + "namespace": obj.ResourceAttributes.Namespace, + "verb": obj.ResourceAttributes.Verb, + "group": obj.ResourceAttributes.Group, + "version": obj.ResourceAttributes.Version, + "resource": obj.ResourceAttributes.Resource, + "subresource": obj.ResourceAttributes.Subresource, + "name": obj.ResourceAttributes.Name, + } + } + if obj.NonResourceAttributes != nil { + ret["nonResourceAttributes"] = map[string]string{ + "verb": obj.NonResourceAttributes.Verb, + "path": obj.NonResourceAttributes.Path, + } + } + return ret +} diff --git a/staging/src/k8s.io/apiserver/pkg/authorization/cel/matcher.go b/staging/src/k8s.io/apiserver/pkg/authorization/cel/matcher.go index 09a462f68d7..30ce5b69c99 100644 --- a/staging/src/k8s.io/apiserver/pkg/authorization/cel/matcher.go +++ b/staging/src/k8s.io/apiserver/pkg/authorization/cel/matcher.go @@ -21,8 +21,8 @@ import ( "fmt" celgo "github.com/google/cel-go/cel" + authorizationv1 "k8s.io/api/authorization/v1" - "k8s.io/apimachinery/pkg/runtime" utilerrors "k8s.io/apimachinery/pkg/util/errors" ) @@ -33,12 +33,8 @@ type CELMatcher struct { // eval evaluates the given SubjectAccessReview against all cel matchCondition expression func (c *CELMatcher) Eval(ctx context.Context, r *authorizationv1.SubjectAccessReview) (bool, error) { var evalErrors []error - specValObject, err := convertObjectToUnstructured(&r.Spec) - if err != nil { - return false, fmt.Errorf("authz celMatcher eval error: convert SubjectAccessReviewSpec object to unstructured failed: %w", err) - } va := map[string]interface{}{ - "request": specValObject, + "request": convertObjectToUnstructured(&r.Spec), } for _, compilationResult := range c.CompilationResults { evalResult, _, err := compilationResult.Program.ContextEval(ctx, va) @@ -68,14 +64,3 @@ func (c *CELMatcher) Eval(ctx context.Context, r *authorizationv1.SubjectAccessR // return ALL matchConditions evaluate to TRUE successfully without error return true, nil } - -func convertObjectToUnstructured(obj *authorizationv1.SubjectAccessReviewSpec) (map[string]interface{}, error) { - if obj == nil { - return nil, nil - } - ret, err := runtime.DefaultUnstructuredConverter.ToUnstructured(obj) - if err != nil { - return nil, err - } - return ret, nil -} diff --git a/staging/src/k8s.io/apiserver/plugin/pkg/authorizer/webhook/webhook_v1_test.go b/staging/src/k8s.io/apiserver/plugin/pkg/authorizer/webhook/webhook_v1_test.go index a72169fc2ac..4cdfd1643dc 100644 --- a/staging/src/k8s.io/apiserver/plugin/pkg/authorizer/webhook/webhook_v1_test.go +++ b/staging/src/k8s.io/apiserver/plugin/pkg/authorizer/webhook/webhook_v1_test.go @@ -704,6 +704,7 @@ func TestStructuredAuthzConfigFeatureEnablement(t *testing.T) { Name: "alice", UID: "1", Groups: []string{"group1", "group2"}, + Extra: map[string][]string{"key1": {"a", "b", "c"}}, }, ResourceRequest: true, Namespace: "kittensandponies", @@ -798,6 +799,7 @@ func TestV1WebhookMatchConditions(t *testing.T) { Name: "alice", UID: "1", Groups: []string{"group1", "group2"}, + Extra: map[string][]string{"key1": {"a", "b", "c"}}, }, ResourceRequest: true, Namespace: "kittensandponies", @@ -848,6 +850,18 @@ func TestV1WebhookMatchConditions(t *testing.T) { { Expression: "('group1' in request.groups)", }, + { + Expression: "('key1' in request.extra)", + }, + { + Expression: "!('key2' in request.extra)", + }, + { + Expression: "('a' in request.extra['key1'])", + }, + { + Expression: "!('z' in request.extra['key1'])", + }, { Expression: "has(request.resourceAttributes) && request.resourceAttributes.namespace == 'kittensandponies'", }, @@ -1028,13 +1042,10 @@ func TestV1WebhookMatchConditions(t *testing.T) { expectedCompileErr: "", // default decisionOnError in newWithBackoff to skip expectedDecision: authorizer.DecisionNoOpinion, - expectedEvalErr: "[cel evaluation error: expression 'request.user == 'bob'' resulted in error: no such key: user, cel evaluation error: expression 'has(request.nonResourceAttributes) && request.nonResourceAttributes.verb == 'get'' resulted in error: no such key: verb]", + expectedEvalErr: "cel evaluation error: expression 'request.resourceAttributes.verb == 'get'' resulted in error: no such key: resourceAttributes", expressions: []apiserver.WebhookMatchCondition{ { - Expression: "request.user == 'bob'", - }, - { - Expression: "has(request.nonResourceAttributes) && request.nonResourceAttributes.verb == 'get'", + Expression: "request.resourceAttributes.verb == 'get'", }, }, }, From 0112d91a056ac4f84e2e9a6a6695cc8a3db4e8b4 Mon Sep 17 00:00:00 2001 From: Jordan Liggitt Date: Thu, 2 Nov 2023 14:39:32 -0400 Subject: [PATCH 4/4] Add multi-webhook integration test --- test/integration/auth/authz_config_test.go | 323 +++++++++++++++++++++ 1 file changed, 323 insertions(+) diff --git a/test/integration/auth/authz_config_test.go b/test/integration/auth/authz_config_test.go index f5be54e3540..3f5128aa6e4 100644 --- a/test/integration/auth/authz_config_test.go +++ b/test/integration/auth/authz_config_test.go @@ -18,9 +18,15 @@ package auth import ( "context" + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" "os" "path/filepath" + "sync/atomic" "testing" + "time" authorizationv1 "k8s.io/api/authorization/v1" rbacv1 "k8s.io/api/rbac/v1" @@ -93,3 +99,320 @@ authorizers: t.Fatal("expected allowed, got denied") } } + +func TestMultiWebhookAuthzConfig(t *testing.T) { + defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.StructuredAuthorizationConfiguration, true)() + + dir := t.TempDir() + + kubeconfigTemplate := ` +apiVersion: v1 +kind: Config +clusters: +- name: integration + cluster: + server: %q + insecure-skip-tls-verify: true +contexts: +- name: default-context + context: + cluster: integration + user: test +current-context: default-context +users: +- name: test +` + + // returns malformed responses when called + serverErrorCalled := atomic.Int32{} + serverError := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + serverErrorCalled.Add(1) + sar := &authorizationv1.SubjectAccessReview{} + if err := json.NewDecoder(req.Body).Decode(sar); err != nil { + t.Error(err) + } + t.Log("serverError", sar) + if _, err := w.Write([]byte(`error response`)); err != nil { + t.Error(err) + } + })) + defer serverError.Close() + serverErrorKubeconfigName := filepath.Join(dir, "serverError.yaml") + if err := os.WriteFile(serverErrorKubeconfigName, []byte(fmt.Sprintf(kubeconfigTemplate, serverError.URL)), os.FileMode(0644)); err != nil { + t.Fatal(err) + } + + // hangs for 2 seconds when called + serverTimeoutCalled := atomic.Int32{} + serverTimeout := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + serverTimeoutCalled.Add(1) + sar := &authorizationv1.SubjectAccessReview{} + if err := json.NewDecoder(req.Body).Decode(sar); err != nil { + t.Error(err) + } + t.Log("serverTimeout", sar) + time.Sleep(2 * time.Second) + })) + defer serverTimeout.Close() + serverTimeoutKubeconfigName := filepath.Join(dir, "serverTimeout.yaml") + if err := os.WriteFile(serverTimeoutKubeconfigName, []byte(fmt.Sprintf(kubeconfigTemplate, serverTimeout.URL)), os.FileMode(0644)); err != nil { + t.Fatal(err) + } + + // returns a deny response when called + serverDenyCalled := atomic.Int32{} + serverDeny := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + serverDenyCalled.Add(1) + sar := &authorizationv1.SubjectAccessReview{} + if err := json.NewDecoder(req.Body).Decode(sar); err != nil { + t.Error(err) + } + t.Log("serverDeny", sar) + sar.Status.Allowed = false + sar.Status.Denied = true + sar.Status.Reason = "denied by webhook" + if err := json.NewEncoder(w).Encode(sar); err != nil { + t.Error(err) + } + })) + defer serverDeny.Close() + serverDenyKubeconfigName := filepath.Join(dir, "serverDeny.yaml") + if err := os.WriteFile(serverDenyKubeconfigName, []byte(fmt.Sprintf(kubeconfigTemplate, serverDeny.URL)), os.FileMode(0644)); err != nil { + t.Fatal(err) + } + + // returns a no opinion response when called + serverNoOpinionCalled := atomic.Int32{} + serverNoOpinion := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + serverNoOpinionCalled.Add(1) + sar := &authorizationv1.SubjectAccessReview{} + if err := json.NewDecoder(req.Body).Decode(sar); err != nil { + t.Error(err) + } + t.Log("serverNoOpinion", sar) + sar.Status.Allowed = false + sar.Status.Denied = false + if err := json.NewEncoder(w).Encode(sar); err != nil { + t.Error(err) + } + })) + defer serverNoOpinion.Close() + serverNoOpinionKubeconfigName := filepath.Join(dir, "serverNoOpinion.yaml") + if err := os.WriteFile(serverNoOpinionKubeconfigName, []byte(fmt.Sprintf(kubeconfigTemplate, serverNoOpinion.URL)), os.FileMode(0644)); err != nil { + t.Fatal(err) + } + + // returns an allow response when called + serverAllowCalled := atomic.Int32{} + serverAllow := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + serverAllowCalled.Add(1) + sar := &authorizationv1.SubjectAccessReview{} + if err := json.NewDecoder(req.Body).Decode(sar); err != nil { + t.Error(err) + } + t.Log("serverAllow", sar) + sar.Status.Allowed = true + sar.Status.Reason = "allowed by webhook" + if err := json.NewEncoder(w).Encode(sar); err != nil { + t.Error(err) + } + })) + defer serverAllow.Close() + serverAllowKubeconfigName := filepath.Join(dir, "serverAllow.yaml") + if err := os.WriteFile(serverAllowKubeconfigName, []byte(fmt.Sprintf(kubeconfigTemplate, serverAllow.URL)), os.FileMode(0644)); err != nil { + t.Fatal(err) + } + + resetCounts := func() { + serverErrorCalled.Store(0) + serverTimeoutCalled.Store(0) + serverDenyCalled.Store(0) + serverNoOpinionCalled.Store(0) + serverAllowCalled.Store(0) + } + assertCounts := func(errorCount, timeoutCount, denyCount, noOpinionCount, allowCount int32) { + t.Helper() + if e, a := errorCount, serverErrorCalled.Load(); e != a { + t.Errorf("expected fail webhook calls: %d, got %d", e, a) + } + if e, a := timeoutCount, serverTimeoutCalled.Load(); e != a { + t.Errorf("expected timeout webhook calls: %d, got %d", e, a) + } + if e, a := denyCount, serverDenyCalled.Load(); e != a { + t.Errorf("expected deny webhook calls: %d, got %d", e, a) + } + if e, a := noOpinionCount, serverNoOpinionCalled.Load(); e != a { + t.Errorf("expected noOpinion webhook calls: %d, got %d", e, a) + } + if e, a := allowCount, serverAllowCalled.Load(); e != a { + t.Errorf("expected allow webhook calls: %d, got %d", e, a) + } + resetCounts() + } + + configFileName := filepath.Join(dir, "config.yaml") + if err := os.WriteFile(configFileName, []byte(` +apiVersion: apiserver.config.k8s.io/v1alpha1 +kind: AuthorizationConfiguration +authorizers: +- type: Webhook + name: error.example.com + webhook: + timeout: 5s + failurePolicy: Deny + subjectAccessReviewVersion: v1 + matchConditionSubjectAccessReviewVersion: v1 + connectionInfo: + type: KubeConfigFile + kubeConfigFile: `+serverErrorKubeconfigName+` + matchConditions: + - expression: has(request.resourceAttributes) + - expression: 'request.resourceAttributes.namespace == "fail"' + - expression: 'request.resourceAttributes.name == "error"' + +- type: Webhook + name: timeout.example.com + webhook: + timeout: 1s + failurePolicy: Deny + subjectAccessReviewVersion: v1 + matchConditionSubjectAccessReviewVersion: v1 + connectionInfo: + type: KubeConfigFile + kubeConfigFile: `+serverTimeoutKubeconfigName+` + matchConditions: + - expression: has(request.resourceAttributes) + - expression: 'request.resourceAttributes.namespace == "fail"' + - expression: 'request.resourceAttributes.name == "timeout"' + +- type: Webhook + name: deny.example.com + webhook: + timeout: 5s + failurePolicy: NoOpinion + subjectAccessReviewVersion: v1 + matchConditionSubjectAccessReviewVersion: v1 + connectionInfo: + type: KubeConfigFile + kubeConfigFile: `+serverDenyKubeconfigName+` + matchConditions: + - expression: has(request.resourceAttributes) + - expression: 'request.resourceAttributes.namespace == "fail"' + +- type: Webhook + name: noopinion.example.com + webhook: + timeout: 5s + failurePolicy: Deny + subjectAccessReviewVersion: v1 + connectionInfo: + type: KubeConfigFile + kubeConfigFile: `+serverNoOpinionKubeconfigName+` + +- type: Webhook + name: allow.example.com + webhook: + timeout: 5s + failurePolicy: Deny + subjectAccessReviewVersion: v1 + connectionInfo: + type: KubeConfigFile + kubeConfigFile: `+serverAllowKubeconfigName+` +`), os.FileMode(0644)); err != nil { + t.Fatal(err) + } + + server := kubeapiservertesting.StartTestServerOrDie( + t, + nil, + []string{"--authorization-config=" + configFileName}, + framework.SharedEtcd(), + ) + t.Cleanup(server.TearDownFn) + + adminClient := clientset.NewForConfigOrDie(server.ClientConfig) + + // malformed webhook short circuits + t.Log("checking error") + if result, err := adminClient.AuthorizationV1().SubjectAccessReviews().Create(context.TODO(), &authorizationv1.SubjectAccessReview{Spec: authorizationv1.SubjectAccessReviewSpec{ + User: "alice", + ResourceAttributes: &authorizationv1.ResourceAttributes{ + Verb: "get", + Group: "", + Version: "v1", + Resource: "configmaps", + Namespace: "fail", + Name: "error", + }, + }}, metav1.CreateOptions{}); err != nil { + t.Fatal(err) + } else if result.Status.Allowed { + t.Fatal("expected denied, got allowed") + } else { + t.Log(result.Status.Reason) + assertCounts(1, 0, 0, 0, 0) + } + + // timeout webhook short circuits + t.Log("checking timeout") + if result, err := adminClient.AuthorizationV1().SubjectAccessReviews().Create(context.TODO(), &authorizationv1.SubjectAccessReview{Spec: authorizationv1.SubjectAccessReviewSpec{ + User: "alice", + ResourceAttributes: &authorizationv1.ResourceAttributes{ + Verb: "get", + Group: "", + Version: "v1", + Resource: "configmaps", + Namespace: "fail", + Name: "timeout", + }, + }}, metav1.CreateOptions{}); err != nil { + t.Fatal(err) + } else if result.Status.Allowed { + t.Fatal("expected denied, got allowed") + } else { + t.Log(result.Status.Reason) + assertCounts(0, 1, 0, 0, 0) + } + + // deny webhook short circuits + t.Log("checking deny") + if result, err := adminClient.AuthorizationV1().SubjectAccessReviews().Create(context.TODO(), &authorizationv1.SubjectAccessReview{Spec: authorizationv1.SubjectAccessReviewSpec{ + User: "alice", + ResourceAttributes: &authorizationv1.ResourceAttributes{ + Verb: "list", + Group: "", + Version: "v1", + Resource: "configmaps", + Namespace: "fail", + Name: "", + }, + }}, metav1.CreateOptions{}); err != nil { + t.Fatal(err) + } else if result.Status.Allowed { + t.Fatal("expected denied, got allowed") + } else { + t.Log(result.Status.Reason) + assertCounts(0, 0, 1, 0, 0) + } + + // no-opinion webhook passes through, allow webhook allows + t.Log("checking allow") + if result, err := adminClient.AuthorizationV1().SubjectAccessReviews().Create(context.TODO(), &authorizationv1.SubjectAccessReview{Spec: authorizationv1.SubjectAccessReviewSpec{ + User: "alice", + ResourceAttributes: &authorizationv1.ResourceAttributes{ + Verb: "list", + Group: "", + Version: "v1", + Resource: "configmaps", + Namespace: "allow", + Name: "", + }, + }}, metav1.CreateOptions{}); err != nil { + t.Fatal(err) + } else if !result.Status.Allowed { + t.Fatal("expected allowed, got denied") + } else { + t.Log(result.Status.Reason) + assertCounts(0, 0, 0, 1, 1) + } +}