diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/validatingadmissionpolicy/admission_test.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/validatingadmissionpolicy/admission_test.go index 2ba862136cb..3e41a9684c3 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/validatingadmissionpolicy/admission_test.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/validatingadmissionpolicy/admission_test.go @@ -20,6 +20,7 @@ import ( "context" "fmt" "sync" + "sync/atomic" "testing" "time" @@ -633,6 +634,12 @@ func TestBasicPolicyDefinitionFailure(t *testing.T) { require.ErrorContains(t, err, `Denied`) } +type validatorFunc func(a admission.Attributes, o admission.ObjectInterfaces, params runtime.Object, matchKind schema.GroupVersionKind) ([]policyDecision, error) + +func (f validatorFunc) Validate(a admission.Attributes, o admission.ObjectInterfaces, params runtime.Object, matchKind schema.GroupVersionKind) ([]policyDecision, error) { + return f(a, o, params, matchKind) +} + type testValidator struct { } @@ -1075,3 +1082,130 @@ func TestEmptyParamSource(t *testing.T) { require.ErrorContains(t, err, `Denied`) require.Equal(t, 1, numCompiles) } + +// Shows what happens when multiple policies share one param type, then +// one policy stops using the param. The expectation is the second policy +// keeps behaving normally +func TestMultiplePoliciesSharedParamType(t *testing.T) { + testContext, testContextCancel := context.WithCancel(context.Background()) + defer testContextCancel() + + compiler := &fakeCompiler{ + // Match everything by default + DefaultMatch: true, + } + handler, paramTracker, tracker, controller := setupFakeTest(t, compiler) + + // Use ConfigMap native-typed param + policy1 := *denyPolicy + policy1.Name = "denypolicy1.example.com" + + policy2 := *denyPolicy + policy2.Name = "denypolicy2.example.com" + + binding1 := *denyBinding + binding2 := *denyBinding + + binding1.Name = "denybinding1.example.com" + binding1.Spec.PolicyName = policy1.Name + binding2.Name = "denybinding2.example.com" + binding2.Spec.PolicyName = policy2.Name + + compiles1 := atomic.Int64{} + evaluations1 := atomic.Int64{} + + compiles2 := atomic.Int64{} + evaluations2 := atomic.Int64{} + + compiler.RegisterDefinition(&policy1, func(vap *v1alpha1.ValidatingAdmissionPolicy) Validator { + compiles1.Add(1) + + return validatorFunc(func(a admission.Attributes, o admission.ObjectInterfaces, params runtime.Object, matchKind schema.GroupVersionKind) ([]policyDecision, error) { + evaluations1.Add(1) + return []policyDecision{ + { + action: actionAdmit, + }, + }, nil + }) + }, nil) + + compiler.RegisterDefinition(&policy2, func(vap *v1alpha1.ValidatingAdmissionPolicy) Validator { + compiles2.Add(1) + + return validatorFunc(func(a admission.Attributes, o admission.ObjectInterfaces, params runtime.Object, matchKind schema.GroupVersionKind) ([]policyDecision, error) { + evaluations2.Add(1) + return []policyDecision{ + { + action: actionDeny, + message: "Policy2Denied", + }, + }, nil + }) + }, nil) + + require.NoError(t, tracker.Create(definitionsGVR, &policy1, policy1.Namespace)) + require.NoError(t, tracker.Create(bindingsGVR, &binding1, binding1.Namespace)) + require.NoError(t, paramTracker.Add(fakeParams)) + + // Wait for controller to reconcile given objects + require.NoError(t, + waitForReconcile( + testContext, controller, + &binding1, &policy1, fakeParams)) + + // Make sure policy 1 is created and bound to the params type first + require.NoError(t, tracker.Create(definitionsGVR, &policy2, policy2.Namespace)) + require.NoError(t, tracker.Create(bindingsGVR, &binding2, binding2.Namespace)) + + // Wait for controller to reconcile given objects + require.NoError(t, + waitForReconcile( + testContext, controller, + &binding1, &binding2, &policy1, &policy2, fakeParams)) + + err := handler.Validate( + testContext, + // Object is irrelevant/unchecked for this test. Just test that + // the evaluator is executed, and returns admit meaning the params + // passed was a configmap + attributeRecord(nil, fakeParams, admission.Create), + &admission.RuntimeObjectInterfaces{}, + ) + + require.ErrorContains(t, err, `Denied`) + require.EqualValues(t, 1, compiles1.Load()) + require.EqualValues(t, 1, evaluations1.Load()) + require.EqualValues(t, 1, compiles2.Load()) + require.EqualValues(t, 1, evaluations2.Load()) + + // Remove param type from policy1 + // Show that policy2 evaluator is still being passed the configmaps + policy1.Spec.ParamKind = nil + policy1.ResourceVersion = "2" + + binding1.Spec.ParamRef = nil + binding1.ResourceVersion = "2" + + require.NoError(t, tracker.Update(definitionsGVR, &policy1, policy1.Namespace)) + require.NoError(t, tracker.Update(bindingsGVR, &binding1, binding1.Namespace)) + require.NoError(t, + waitForReconcile( + testContext, controller, + &binding1, &policy1)) + + err = handler.Validate( + testContext, + // Object is irrelevant/unchecked for this test. Just test that + // the evaluator is executed, and returns admit meaning the params + // passed was a configmap + attributeRecord(nil, fakeParams, admission.Create), + &admission.RuntimeObjectInterfaces{}, + ) + + require.ErrorContains(t, err, `Policy2Denied`) + require.EqualValues(t, 2, compiles1.Load()) + require.EqualValues(t, 2, evaluations1.Load()) + require.EqualValues(t, 1, compiles2.Load()) + require.EqualValues(t, 2, evaluations2.Load()) +} diff --git a/staging/src/k8s.io/apiserver/pkg/admission/plugin/validatingadmissionpolicy/controller_reconcile.go b/staging/src/k8s.io/apiserver/pkg/admission/plugin/validatingadmissionpolicy/controller_reconcile.go index b3fdcaa9ee3..bb2289f1b63 100644 --- a/staging/src/k8s.io/apiserver/pkg/admission/plugin/validatingadmissionpolicy/controller_reconcile.go +++ b/staging/src/k8s.io/apiserver/pkg/admission/plugin/validatingadmissionpolicy/controller_reconcile.go @@ -229,8 +229,12 @@ func (c *policyController) reconcilePolicyDefinition(namespace, name string, def return info.configurationError } - // Start watching the param CRD - if _, ok := c.paramsCRDControllers[*paramSource]; !ok { + if info, ok := c.paramsCRDControllers[*paramSource]; ok { + // If a param controller is already active for this paramsource, make + // sure it is tracking this policy's dependency upon it + info.dependentDefinitions.Insert(nn) + + } else { instanceContext, instanceCancel := context.WithCancel(c.context) // Watch for new instances of this policy