Merge pull request #115147 from alexzielenski/apiserver/policy/multiple-paramkind-bug
ValidatingAdmissionPolicy: fix bug preventing multiple policies from using same paramKind Kubernetes-commit: 06ad4258096621190b23c6e41818715e86291989
This commit is contained in:
Kubernetes Publisher
commit
f4be1be367
12
go.mod
12
go.mod
|
|
@ -43,9 +43,9 @@ require (
|
|||
gopkg.in/natefinch/lumberjack.v2 v2.0.0
|
||||
gopkg.in/square/go-jose.v2 v2.2.2
|
||||
k8s.io/api v0.0.0-20230112183318-59fcd23597fd
|
||||
k8s.io/apimachinery v0.0.0-20230117181045-35ae53239086
|
||||
k8s.io/client-go v0.0.0-20230118115900-e6998df99e84
|
||||
k8s.io/component-base v0.0.0-20230112192324-33f62c7b2818
|
||||
k8s.io/apimachinery v0.0.0-20230119000403-b39d3a97b9a6
|
||||
k8s.io/client-go v0.0.0-20230119002741-c33df1cccbfb
|
||||
k8s.io/component-base v0.0.0-20230119004332-38b99c2c9e9a
|
||||
k8s.io/klog/v2 v2.80.1
|
||||
k8s.io/kms v0.0.0-20230117203143-407acb40b4f8
|
||||
k8s.io/kube-openapi v0.0.0-20230109183929-3758b55a6596
|
||||
|
|
@ -123,8 +123,8 @@ require (
|
|||
|
||||
replace (
|
||||
k8s.io/api => k8s.io/api v0.0.0-20230112183318-59fcd23597fd
|
||||
k8s.io/apimachinery => k8s.io/apimachinery v0.0.0-20230117181045-35ae53239086
|
||||
k8s.io/client-go => k8s.io/client-go v0.0.0-20230118115900-e6998df99e84
|
||||
k8s.io/component-base => k8s.io/component-base v0.0.0-20230112192324-33f62c7b2818
|
||||
k8s.io/apimachinery => k8s.io/apimachinery v0.0.0-20230119000403-b39d3a97b9a6
|
||||
k8s.io/client-go => k8s.io/client-go v0.0.0-20230119002741-c33df1cccbfb
|
||||
k8s.io/component-base => k8s.io/component-base v0.0.0-20230119004332-38b99c2c9e9a
|
||||
k8s.io/kms => k8s.io/kms v0.0.0-20230117203143-407acb40b4f8
|
||||
)
|
||||
|
|
|
|||
12
go.sum
12
go.sum
|
|
@ -992,12 +992,12 @@ honnef.co/go/tools v0.0.1-2020.1.3/go.mod h1:X/FiERA/W4tHapMX5mGpAtMSVEeEUOyHaw9
|
|||
honnef.co/go/tools v0.0.1-2020.1.4/go.mod h1:X/FiERA/W4tHapMX5mGpAtMSVEeEUOyHaw9vFzvIQ3k=
|
||||
k8s.io/api v0.0.0-20230112183318-59fcd23597fd h1:Y/vljv797SrqMUZ4KwQaIDcz+lecu074ACcke2d8D9s=
|
||||
k8s.io/api v0.0.0-20230112183318-59fcd23597fd/go.mod h1:K1fbar68K91an+8RbH9eJWJRnoXQ5rugf6J5wsGMMWg=
|
||||
k8s.io/apimachinery v0.0.0-20230117181045-35ae53239086 h1:7yhcU6Liv6A9zpgeLsSASfmgpBHz/4WT7WK0Y+szTsM=
|
||||
k8s.io/apimachinery v0.0.0-20230117181045-35ae53239086/go.mod h1:Kq932aOItR+rI/aiAVDXxxdRFNf0eLnI0gZmXJ6hl3o=
|
||||
k8s.io/client-go v0.0.0-20230118115900-e6998df99e84 h1:YcxNPmKn8JNmwxPqAbONbH0vCCM0G/IhBp4+KCd9LNw=
|
||||
k8s.io/client-go v0.0.0-20230118115900-e6998df99e84/go.mod h1:q8YXsL0M5VV5aF6DprurfY/VjRi6ByqRrR22EmP4SUM=
|
||||
k8s.io/component-base v0.0.0-20230112192324-33f62c7b2818 h1:sZ5awJx2PpXaOwNSEu8Pq6zovP1+YG/nWZpagmpDRcs=
|
||||
k8s.io/component-base v0.0.0-20230112192324-33f62c7b2818/go.mod h1:6IeVfLNdhjJkJFQHYMuwk7dgkOLA3Pt+Y7j7Md1d+CA=
|
||||
k8s.io/apimachinery v0.0.0-20230119000403-b39d3a97b9a6 h1:nxc7EioumBwPm/wfQ2OfTyKq7sSt7RSThMTheCqe7Gg=
|
||||
k8s.io/apimachinery v0.0.0-20230119000403-b39d3a97b9a6/go.mod h1:mb1AP2xs7Ajs+OvXRynwIgKVID9/rOtI7lFxIixvFp0=
|
||||
k8s.io/client-go v0.0.0-20230119002741-c33df1cccbfb h1:b86Po/QFXB7JQOSAb/fc/hBa33vGyhFChy7NyGZl31s=
|
||||
k8s.io/client-go v0.0.0-20230119002741-c33df1cccbfb/go.mod h1:4LcNfL89QHAE9BINU66MM61I4GJ0BLuE8xSYu+OIEXg=
|
||||
k8s.io/component-base v0.0.0-20230119004332-38b99c2c9e9a h1:zYjKaAlDW2q7W1t4W8ds+wn+AB09ZlA68rtcz+9EMyg=
|
||||
k8s.io/component-base v0.0.0-20230119004332-38b99c2c9e9a/go.mod h1:ljIiNP+BVFJrmiMy7ELMvnp161IE9EPA7XL3ZkniZ7U=
|
||||
k8s.io/klog/v2 v2.80.1 h1:atnLQ121W371wYYFawwYx1aEY2eUfs4l3J72wtgAwV4=
|
||||
k8s.io/klog/v2 v2.80.1/go.mod h1:y1WjHnz7Dj687irZUWR/WLkLc5N1YHtjLdmgWjndZn0=
|
||||
k8s.io/kms v0.0.0-20230117203143-407acb40b4f8 h1:oYOLi+NfW2BoplN1w5lO0NdBaQpV5Oe8C2XmFkjFi8g=
|
||||
|
|
|
|||
|
|
@ -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())
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Reference in New Issue