From 4acedb5132b2c3a7d61bd9e088c964af3fcfee3d Mon Sep 17 00:00:00 2001 From: Richa Banker Date: Tue, 23 Jul 2024 22:19:02 -0700 Subject: [PATCH] init a common apiserver for TestAuthorizationDecisionCaching testcases --- .../plugin/policy/generic/policy_source.go | 20 +- .../cel/validatingadmissionpolicy_test.go | 364 ++++++++++-------- 2 files changed, 209 insertions(+), 175 deletions(-) diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/generic/policy_source.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/generic/policy_source.go index 7d47c6f9728..ca6cdc884fc 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/generic/policy_source.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/policy/generic/policy_source.go @@ -41,9 +41,12 @@ import ( "k8s.io/klog/v2" ) -var ( - PolicyRefreshInterval = 1 * time.Second -) +// Interval for refreshing policies. +// TODO: Consider reducing this to a shorter duration or replacing this entirely +// with checks that detect when a policy change took effect. +const policyRefreshIntervalDefault = 1 * time.Second + +var policyRefreshInterval = policyRefreshIntervalDefault type policySource[P runtime.Object, B runtime.Object, E Evaluator] struct { ctx context.Context @@ -126,6 +129,15 @@ func NewPolicySource[P runtime.Object, B runtime.Object, E Evaluator]( return res } +// SetPolicyRefreshIntervalForTests allows the refresh interval to be overridden during tests. +// This should only be called from tests. +func SetPolicyRefreshIntervalForTests(interval time.Duration) func() { + policyRefreshInterval = interval + return func() { + policyRefreshInterval = policyRefreshIntervalDefault + } +} + func (s *policySource[P, B, E]) Run(ctx context.Context) error { if s.ctx != nil { return fmt.Errorf("policy source already running") @@ -182,7 +194,7 @@ func (s *policySource[P, B, E]) Run(ctx context.Context) error { // and needs to be recompiled go func() { // Loop every 1 second until context is cancelled, refreshing policies - wait.Until(s.refreshPolicies, PolicyRefreshInterval, ctx.Done()) + wait.Until(s.refreshPolicies, policyRefreshInterval, ctx.Done()) }() <-ctx.Done() diff --git a/test/integration/apiserver/cel/validatingadmissionpolicy_test.go b/test/integration/apiserver/cel/validatingadmissionpolicy_test.go index d58a8dc2b16..2dd810dd9da 100644 --- a/test/integration/apiserver/cel/validatingadmissionpolicy_test.go +++ b/test/integration/apiserver/cel/validatingadmissionpolicy_test.go @@ -67,10 +67,15 @@ import ( rbacv1 "k8s.io/api/rbac/v1" ) +// Short term fix to refresh the policy source cache faster for tests +// until we reduce the polling interval by default. +const policyRefreshInterval = 10 * time.Millisecond + // Test_ValidateNamespace_NoParams_Success tests a ValidatingAdmissionPolicy that validates creation of a Namespace // with no params via happy path. func Test_ValidateNamespace_NoParams_Success(t *testing.T) { - generic.PolicyRefreshInterval = 10 * time.Millisecond + resetPolicyRefreshInterval := generic.SetPolicyRefreshIntervalForTests(policyRefreshInterval) + defer resetPolicyRefreshInterval() featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, genericfeatures.ValidatingAdmissionPolicy, true) server, err := apiservertesting.StartTestServer(t, nil, []string{ "--enable-admission-plugins", "ValidatingAdmissionPolicy", @@ -184,7 +189,8 @@ func Test_ValidateNamespace_NoParams_Success(t *testing.T) { // Test_ValidateNamespace_NoParams_Failures tests a ValidatingAdmissionPolicy that fails creation of a Namespace with no params. func Test_ValidateNamespace_NoParams_Failures(t *testing.T) { - generic.PolicyRefreshInterval = 10 * time.Millisecond + resetPolicyRefreshInterval := generic.SetPolicyRefreshIntervalForTests(policyRefreshInterval) + defer resetPolicyRefreshInterval() featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, genericfeatures.ValidatingAdmissionPolicy, true) server, err := apiservertesting.StartTestServer(t, nil, []string{ "--enable-admission-plugins", "ValidatingAdmissionPolicy", @@ -516,7 +522,8 @@ func Test_ValidateAnnotationsAndWarnings(t *testing.T) { // Test_ValidateNamespace_WithConfigMapParams tests a ValidatingAdmissionPolicy that validates creation of a Namespace, // using ConfigMap as a param reference. func Test_ValidateNamespace_WithConfigMapParams(t *testing.T) { - generic.PolicyRefreshInterval = 10 * time.Millisecond + resetPolicyRefreshInterval := generic.SetPolicyRefreshIntervalForTests(policyRefreshInterval) + defer resetPolicyRefreshInterval() featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, genericfeatures.ValidatingAdmissionPolicy, true) server, err := apiservertesting.StartTestServer(t, nil, []string{ "--enable-admission-plugins", "ValidatingAdmissionPolicy", @@ -2135,7 +2142,8 @@ func Test_ValidatingAdmissionPolicy_ParamResourceDeletedThenRecreated(t *testing // Test_CostLimitForValidation tests the cost limit set for a ValidatingAdmissionPolicy // with StrictCostEnforcementForVAP feature enabled. func Test_CostLimitForValidation(t *testing.T) { - generic.PolicyRefreshInterval = 10 * time.Millisecond + resetPolicyRefreshInterval := generic.SetPolicyRefreshIntervalForTests(policyRefreshInterval) + defer resetPolicyRefreshInterval() featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, genericfeatures.StrictCostEnforcementForVAP, true) server, err := apiservertesting.StartTestServer(t, nil, []string{ "--enable-admission-plugins", "ValidatingAdmissionPolicy", @@ -2244,7 +2252,8 @@ func Test_CostLimitForValidation(t *testing.T) { // Test_CostLimitForValidationWithFeatureDisabled tests the cost limit set for a ValidatingAdmissionPolicy // with StrictCostEnforcementForVAP feature disabled. func Test_CostLimitForValidationWithFeatureDisabled(t *testing.T) { - generic.PolicyRefreshInterval = 10 * time.Millisecond + resetPolicyRefreshInterval := generic.SetPolicyRefreshIntervalForTests(policyRefreshInterval) + defer resetPolicyRefreshInterval() server, err := apiservertesting.StartTestServer(t, nil, []string{ "--enable-admission-plugins", "ValidatingAdmissionPolicy", }, framework.SharedEtcd()) @@ -2341,7 +2350,8 @@ func generateValidationsWithAuthzCheck(num int, exp string) []admissionregistrat // TestCRDParams tests that a CustomResource can be used as a param resource for a ValidatingAdmissionPolicy. func TestCRDParams(t *testing.T) { - generic.PolicyRefreshInterval = 10 * time.Millisecond + resetPolicyRefreshInterval := generic.SetPolicyRefreshIntervalForTests(policyRefreshInterval) + defer resetPolicyRefreshInterval() featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, genericfeatures.ValidatingAdmissionPolicy, true) server, err := apiservertesting.StartTestServer(t, nil, []string{ "--enable-admission-plugins", "ValidatingAdmissionPolicy", @@ -2543,7 +2553,8 @@ func TestBindingRemoval(t *testing.T) { // Test_ValidateSecondaryAuthorization tests a ValidatingAdmissionPolicy that performs secondary authorization checks // for both users and service accounts. func Test_ValidateSecondaryAuthorization(t *testing.T) { - generic.PolicyRefreshInterval = 10 * time.Millisecond + resetPolicyRefreshInterval := generic.SetPolicyRefreshIntervalForTests(policyRefreshInterval) + defer resetPolicyRefreshInterval() featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, genericfeatures.ValidatingAdmissionPolicy, true) server, err := apiservertesting.StartTestServer(t, nil, []string{ "--enable-admission-plugins", "ValidatingAdmissionPolicy", @@ -2821,6 +2832,181 @@ func TestCRDsOnStartup(t *testing.T) { } +func TestAuthorizationDecisionCaching(t *testing.T) { + resetPolicyRefreshInterval := generic.SetPolicyRefreshIntervalForTests(policyRefreshInterval) + defer resetPolicyRefreshInterval() + ctx, cancel := context.WithCancel(context.TODO()) + defer cancel() + featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, genericfeatures.ValidatingAdmissionPolicy, true) + var nChecks int + webhook := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + var review authorizationv1.SubjectAccessReview + if err := json.NewDecoder(r.Body).Decode(&review); err != nil { + http.Error(w, err.Error(), http.StatusBadRequest) + } + + review.Status.Allowed = true + if review.Spec.ResourceAttributes.Verb == "test" { + nChecks++ + review.Status.Reason = fmt.Sprintf("%d", nChecks) + } + + w.Header().Set("Content-Type", "application/json") + if err := json.NewEncoder(w).Encode(review); err != nil { + http.Error(w, err.Error(), http.StatusInternalServerError) + } + })) + defer webhook.Close() + kcfd, err := os.CreateTemp("", "kubeconfig-") + if err != nil { + t.Fatal(err) + } + func() { + defer func() { + if err := kcfd.Close(); err != nil { + t.Fatal(err) + } + }() + tmpl, err := template.New("kubeconfig").Parse(` +apiVersion: v1 +kind: Config +clusters: + - name: test-authz-service + cluster: + server: {{ .Server }} +users: + - name: test-api-server +current-context: webhook +contexts: +- context: + cluster: test-authz-service + user: test-api-server + name: webhook +`) + if err != nil { + t.Fatal(err) + } + err = tmpl.Execute(kcfd, struct { + Server string + }{ + Server: webhook.URL, + }) + if err != nil { + t.Fatal(err) + } + }() + _, config, teardown := framework.StartTestServer(ctx, t, framework.TestServerSetup{ + ModifyServerRunOptions: func(options *options.ServerRunOptions) { + options.Admission.GenericAdmission.EnablePlugins = append(options.Admission.GenericAdmission.EnablePlugins, "ValidatingAdmissionPolicy") + if err = options.APIEnablement.RuntimeConfig.Set("api/all=true"); err != nil { + t.Fatal(err) + } + + options.Authorization.Modes = []string{authzmodes.ModeWebhook} + options.Authorization.WebhookConfigFile = kcfd.Name() + options.Authorization.WebhookVersion = "v1" + // Bypass webhook cache to observe the policy plugin's cache behavior. + options.Authorization.WebhookCacheAuthorizedTTL = 0 + options.Authorization.WebhookCacheUnauthorizedTTL = 0 + }, + }) + defer teardown() + + config = rest.CopyConfig(config) + config.Impersonate = rest.ImpersonationConfig{ + UserName: "alice", + UID: "1234", + } + client, err := clientset.NewForConfig(config) + if err != nil { + t.Fatal(err) + } + + for i, tc := range []struct { + name string + validations []admissionregistrationv1.Validation + }{ + { + name: "hit", + validations: []admissionregistrationv1.Validation{ + { + Expression: "authorizer.requestResource.check('test').reason() == authorizer.requestResource.check('test').reason()", + }, + }, + }, + { + name: "miss", + validations: []admissionregistrationv1.Validation{ + { + Expression: "authorizer.requestResource.subresource('a').check('test').reason() == '1'", + }, + { + Expression: "authorizer.requestResource.subresource('b').check('test').reason() == '2'", + }, + { + Expression: "authorizer.requestResource.subresource('c').check('test').reason() == '3'", + }, + }, + }, + } { + t.Run(tc.name, func(t *testing.T) { + policy := &admissionregistrationv1.ValidatingAdmissionPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-authorization-decision-caching-policy", + }, + Spec: admissionregistrationv1.ValidatingAdmissionPolicySpec{ + MatchConstraints: &admissionregistrationv1.MatchResources{ + ResourceRules: []admissionregistrationv1.NamedRuleWithOperations{ + { + ResourceNames: []string{"test-authorization-decision-caching-namespace"}, + RuleWithOperations: admissionregistrationv1.RuleWithOperations{ + Operations: []admissionregistrationv1.OperationType{ + admissionregistrationv1.Create, + }, + Rule: admissionregistrationv1.Rule{ + APIGroups: []string{""}, + APIVersions: []string{"v1"}, + Resources: []string{"namespaces"}, + }, + }, + }, + }, + }, + Validations: tc.validations, + }, + } + + policy, err = client.AdmissionregistrationV1().ValidatingAdmissionPolicies().Create(ctx, withWaitReadyConstraintAndExpression(policy), metav1.CreateOptions{}) + if err != nil { + t.Fatal(err) + } + + policyBinding := makeBinding(policy.Name+"-binding", policy.Name, "") + if err := createAndWaitReady(t, client, policyBinding, nil); err != nil { + t.Fatal(err) + } + + if _, err := client.CoreV1().Namespaces().Create( + ctx, + &v1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: fmt.Sprintf("test-authorization-decision-caching-namespace-%d", i), + }, + }, + metav1.CreateOptions{}, + ); err != nil { + if !apierrors.IsAlreadyExists(err) { + t.Fatal(err) + } + } + + if err := cleanupPolicy(t, client, policy, policyBinding); err != nil { + t.Errorf("error while cleaning up policy and its bindings: %v", err) + } + }) + } +} + type clientFn func(t *testing.T, adminClient *clientset.Clientset, clientConfig *rest.Config, rules []rbacv1.PolicyRule) *clientset.Clientset func secondaryAuthorizationUserClient(t *testing.T, adminClient *clientset.Clientset, clientConfig *rest.Config, rules []rbacv1.PolicyRule) *clientset.Clientset { @@ -3382,167 +3568,3 @@ rules: resources: ["configmaps"] ` ) - -func TestAuthorizationDecisionCaching(t *testing.T) { - for _, tc := range []struct { - name string - validations []admissionregistrationv1.Validation - }{ - { - name: "hit", - validations: []admissionregistrationv1.Validation{ - { - Expression: "authorizer.requestResource.check('test').reason() == authorizer.requestResource.check('test').reason()", - }, - }, - }, - { - name: "miss", - validations: []admissionregistrationv1.Validation{ - { - Expression: "authorizer.requestResource.subresource('a').check('test').reason() == '1'", - }, - { - Expression: "authorizer.requestResource.subresource('b').check('test').reason() == '2'", - }, - { - Expression: "authorizer.requestResource.subresource('c').check('test').reason() == '3'", - }, - }, - }, - } { - t.Run(tc.name, func(t *testing.T) { - featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, genericfeatures.ValidatingAdmissionPolicy, true) - - ctx, cancel := context.WithCancel(context.TODO()) - defer cancel() - - var nChecks int - webhook := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - var review authorizationv1.SubjectAccessReview - if err := json.NewDecoder(r.Body).Decode(&review); err != nil { - http.Error(w, err.Error(), http.StatusBadRequest) - } - - review.Status.Allowed = true - if review.Spec.ResourceAttributes.Verb == "test" { - nChecks++ - review.Status.Reason = fmt.Sprintf("%d", nChecks) - } - - w.Header().Set("Content-Type", "application/json") - if err := json.NewEncoder(w).Encode(review); err != nil { - http.Error(w, err.Error(), http.StatusInternalServerError) - } - })) - defer webhook.Close() - - kcfd, err := os.CreateTemp("", "kubeconfig-") - if err != nil { - t.Fatal(err) - } - func() { - defer kcfd.Close() - tmpl, err := template.New("kubeconfig").Parse(` -apiVersion: v1 -kind: Config -clusters: - - name: test-authz-service - cluster: - server: {{ .Server }} -users: - - name: test-api-server -current-context: webhook -contexts: -- context: - cluster: test-authz-service - user: test-api-server - name: webhook -`) - if err != nil { - t.Fatal(err) - } - err = tmpl.Execute(kcfd, struct { - Server string - }{ - Server: webhook.URL, - }) - if err != nil { - t.Fatal(err) - } - }() - - client, config, teardown := framework.StartTestServer(ctx, t, framework.TestServerSetup{ - ModifyServerRunOptions: func(options *options.ServerRunOptions) { - options.Admission.GenericAdmission.EnablePlugins = append(options.Admission.GenericAdmission.EnablePlugins, "ValidatingAdmissionPolicy") - options.APIEnablement.RuntimeConfig.Set("api/all=true") - - options.Authorization.Modes = []string{authzmodes.ModeWebhook} - options.Authorization.WebhookConfigFile = kcfd.Name() - options.Authorization.WebhookVersion = "v1" - // Bypass webhook cache to observe the policy plugin's cache behavior. - options.Authorization.WebhookCacheAuthorizedTTL = 0 - options.Authorization.WebhookCacheUnauthorizedTTL = 0 - }, - }) - defer teardown() - - policy := &admissionregistrationv1.ValidatingAdmissionPolicy{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-authorization-decision-caching-policy", - }, - Spec: admissionregistrationv1.ValidatingAdmissionPolicySpec{ - MatchConstraints: &admissionregistrationv1.MatchResources{ - ResourceRules: []admissionregistrationv1.NamedRuleWithOperations{ - { - ResourceNames: []string{"test-authorization-decision-caching-namespace"}, - RuleWithOperations: admissionregistrationv1.RuleWithOperations{ - Operations: []admissionregistrationv1.OperationType{ - admissionregistrationv1.Create, - }, - Rule: admissionregistrationv1.Rule{ - APIGroups: []string{""}, - APIVersions: []string{"v1"}, - Resources: []string{"namespaces"}, - }, - }, - }, - }, - }, - Validations: tc.validations, - }, - } - - policy, err = client.AdmissionregistrationV1().ValidatingAdmissionPolicies().Create(ctx, withWaitReadyConstraintAndExpression(policy), metav1.CreateOptions{}) - if err != nil { - t.Fatal(err) - } - - if err := createAndWaitReady(t, client, makeBinding(policy.Name+"-binding", policy.Name, ""), nil); err != nil { - t.Fatal(err) - } - - config = rest.CopyConfig(config) - config.Impersonate = rest.ImpersonationConfig{ - UserName: "alice", - UID: "1234", - } - client, err = clientset.NewForConfig(config) - if err != nil { - t.Fatal(err) - } - - if _, err := client.CoreV1().Namespaces().Create( - ctx, - &v1.Namespace{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-authorization-decision-caching-namespace", - }, - }, - metav1.CreateOptions{}, - ); err != nil { - t.Fatal(err) - } - }) - } -}