diff --git a/pkg/engine/context/deferred_test.go b/pkg/engine/context/deferred_test.go index 67e8f234d7..709d6ad08d 100644 --- a/pkg/engine/context/deferred_test.go +++ b/pkg/engine/context/deferred_test.go @@ -10,7 +10,7 @@ import ( func TestDeferredLoaderMatch(t *testing.T) { ctx := newContext() - mockLoader, _ := addDeferred(ctx, "one", "1") + mockLoader, _ := AddMockDeferredLoader(ctx, "one", "1") assert.Equal(t, 0, mockLoader.invocations) val, err := ctx.Query("one") @@ -22,28 +22,28 @@ func TestDeferredLoaderMatch(t *testing.T) { assert.Equal(t, 1, mockLoader.invocations) ctx = newContext() - ml, _ := addDeferred(ctx, "one", "1") + ml, _ := AddMockDeferredLoader(ctx, "one", "1") testCheckMatch(t, ctx, "onetwo", "one", "1", ml) - ml, _ = addDeferred(ctx, "one", "1") + ml, _ = AddMockDeferredLoader(ctx, "one", "1") testCheckMatch(t, ctx, "one, two, three", "one", "1", ml) - ml, _ = addDeferred(ctx, "one1", "11") + ml, _ = AddMockDeferredLoader(ctx, "one1", "11") testCheckMatch(t, ctx, "one1", "one1", "11", ml) } @@ -64,7 +64,7 @@ func testCheckMatch(t *testing.T, ctx *context, query, name, value string, ml *m func TestDeferredLoaderMismatch(t *testing.T) { ctx := newContext() - addDeferred(ctx, "one", "1") + AddMockDeferredLoader(ctx, "one", "1") _, err := ctx.Query("oneTwoThree") assert.ErrorContains(t, err, `Unknown key "oneTwoThree" in path`) @@ -101,92 +101,9 @@ func newContext() *context { } } -type mockLoader struct { - name string - level int - value interface{} - query string - hasLoaded bool - invocations int - eventHandler func(event string) - ctx *context -} - -func (ml *mockLoader) Name() string { - return ml.name -} - -func (ml *mockLoader) SetLevel(level int) { - ml.level = level -} - -func (ml *mockLoader) GetLevel() int { - return ml.level -} - -func (ml *mockLoader) HasLoaded() bool { - return ml.hasLoaded -} - -func (ml *mockLoader) LoadData() error { - ml.invocations++ - ml.ctx.AddVariable(ml.name, ml.value) - - // simulate a JMESPath evaluation after loading - if err := ml.executeQuery(); err != nil { - return err - } - - ml.hasLoaded = true - if ml.eventHandler != nil { - event := fmt.Sprintf("%s=%v", ml.name, ml.value) - ml.eventHandler(event) - } - - return nil -} - -func (ml *mockLoader) executeQuery() error { - if ml.query == "" { - return nil - } - - results, err := ml.ctx.Query(ml.query) - if err != nil { - return err - } - - return ml.ctx.AddVariable(ml.name, results) -} - -func (ml *mockLoader) setEventHandler(eventHandler func(string)) { - ml.eventHandler = eventHandler -} - -func addDeferred(ctx *context, name string, value interface{}) (*mockLoader, error) { - return addDeferredWithQuery(ctx, name, value, "") -} - -func addDeferredWithQuery(ctx *context, name string, value interface{}, query string) (*mockLoader, error) { - loader := &mockLoader{ - name: name, - value: value, - ctx: ctx, - query: query, - } - - d, err := NewDeferredLoader(name, loader, logger) - if err != nil { - return loader, err - } - - ctx.AddDeferredLoader(d) - return loader, nil -} - func TestDeferredReset(t *testing.T) { ctx := newContext() - addDeferred(ctx, "value", "0") + AddMockDeferredLoader(ctx, "value", "0") ctx.Checkpoint() val, err := ctx.Query("value") @@ -203,8 +120,8 @@ func TestDeferredCheckpointRestore(t *testing.T) { ctx := newContext() ctx.Checkpoint() - unused, _ := addDeferred(ctx, "unused", "unused") - mock, _ := addDeferred(ctx, "one", "1") + unused, _ := AddMockDeferredLoader(ctx, "unused", "unused") + mock, _ := AddMockDeferredLoader(ctx, "one", "1") ctx.Restore() assert.Equal(t, 0, mock.invocations) assert.Equal(t, 0, unused.invocations) @@ -219,7 +136,7 @@ func TestDeferredCheckpointRestore(t *testing.T) { _, err = ctx.Query("one") assert.ErrorContains(t, err, "Unknown key \"one\" in path") - _, _ = addDeferred(ctx, "one", "1") + _, _ = AddMockDeferredLoader(ctx, "one", "1") ctx.Checkpoint() one, err := ctx.Query("one") assert.NilError(t, err) @@ -235,14 +152,14 @@ func TestDeferredCheckpointRestore(t *testing.T) { assert.NilError(t, err) assert.Equal(t, "1", one) - mock, _ = addDeferred(ctx, "one", "1") + mock, _ = AddMockDeferredLoader(ctx, "one", "1") ctx.Checkpoint() val, err := ctx.Query("one") assert.NilError(t, err) assert.Equal(t, "1", val) assert.Equal(t, 1, mock.invocations) - mock2, _ := addDeferred(ctx, "two", "2") + mock2, _ := AddMockDeferredLoader(ctx, "two", "2") val, err = ctx.Query("two") assert.NilError(t, err) assert.Equal(t, "2", val) @@ -269,7 +186,7 @@ func TestDeferredCheckpointRestore(t *testing.T) { _, err = ctx.Query("two") assert.ErrorContains(t, err, `Unknown key "two" in path`) - mock3, _ := addDeferred(ctx, "three", "3") + mock3, _ := AddMockDeferredLoader(ctx, "three", "3") val, err = ctx.Query("three") assert.NilError(t, err) assert.Equal(t, "3", val) @@ -290,7 +207,7 @@ func TestDeferredCheckpointRestore(t *testing.T) { func TestDeferredForloop(t *testing.T) { ctx := newContext() - addDeferred(ctx, "value", float64(-1)) + AddMockDeferredLoader(ctx, "value", float64(-1)) ctx.Checkpoint() for i := 0; i < 5; i++ { @@ -299,7 +216,7 @@ func TestDeferredForloop(t *testing.T) { assert.Equal(t, float64(i-1), val) ctx.Reset() - mock, _ := addDeferred(ctx, "value", float64(i)) + mock, _ := AddMockDeferredLoader(ctx, "value", float64(i)) val, err = ctx.Query("value") assert.NilError(t, err) assert.Equal(t, float64(i), val) @@ -315,13 +232,13 @@ func TestDeferredForloop(t *testing.T) { func TestDeferredInvalidReset(t *testing.T) { ctx := newContext() - addDeferred(ctx, "value", "0") + AddMockDeferredLoader(ctx, "value", "0") ctx.Reset() // no checkpoint val, err := ctx.Query("value") assert.NilError(t, err) assert.Equal(t, "0", val) - addDeferred(ctx, "value", "0") + AddMockDeferredLoader(ctx, "value", "0") ctx.Restore() // no checkpoint val, err = ctx.Query("value") assert.NilError(t, err) @@ -330,18 +247,18 @@ func TestDeferredInvalidReset(t *testing.T) { func TestDeferredValidResetRestore(t *testing.T) { ctx := newContext() - addDeferred(ctx, "value", "0") + AddMockDeferredLoader(ctx, "value", "0") ctx.Checkpoint() - addDeferred(ctx, "leak", "leak") + AddMockDeferredLoader(ctx, "leak", "leak") ctx.Reset() _, err := ctx.Query("leak") assert.ErrorContains(t, err, `Unknown key "leak" in path`) - addDeferred(ctx, "value", "0") + AddMockDeferredLoader(ctx, "value", "0") ctx.Checkpoint() - addDeferred(ctx, "leak", "leak") + AddMockDeferredLoader(ctx, "leak", "leak") ctx.Restore() _, err = ctx.Query("leak") @@ -355,10 +272,10 @@ func TestDeferredSameName(t *testing.T) { sequence = append(sequence, name) } - mock1, _ := addDeferred(ctx, "value", "0") + mock1, _ := AddMockDeferredLoader(ctx, "value", "0") mock1.setEventHandler(hdlr) - mock2, _ := addDeferred(ctx, "value", "1") + mock2, _ := AddMockDeferredLoader(ctx, "value", "1") mock2.setEventHandler(hdlr) val, err := ctx.Query("value") @@ -383,7 +300,7 @@ func TestDeferredRecursive(t *testing.T) { func TestJMESPathDependency(t *testing.T) { ctx := newContext() - addDeferred(ctx, "foo", "foo") + AddMockDeferredLoader(ctx, "foo", "foo") addDeferredWithQuery(ctx, "one", "1", "foo") val, err := ctx.Query("one") @@ -393,10 +310,10 @@ func TestJMESPathDependency(t *testing.T) { func TestDeferredHiddenEval(t *testing.T) { ctx := newContext() - addDeferred(ctx, "foo", "foo") + AddMockDeferredLoader(ctx, "foo", "foo") ctx.Checkpoint() - addDeferred(ctx, "foo", "bar") + AddMockDeferredLoader(ctx, "foo", "bar") val, err := ctx.Query("foo") assert.NilError(t, err) @@ -405,11 +322,11 @@ func TestDeferredHiddenEval(t *testing.T) { func TestDeferredNotHidden(t *testing.T) { ctx := newContext() - addDeferred(ctx, "foo", "foo") + AddMockDeferredLoader(ctx, "foo", "foo") addDeferredWithQuery(ctx, "one", "1", "foo") ctx.Checkpoint() - addDeferred(ctx, "foo", "bar") + AddMockDeferredLoader(ctx, "foo", "bar") val, err := ctx.Query("one") assert.NilError(t, err) @@ -418,12 +335,12 @@ func TestDeferredNotHidden(t *testing.T) { func TestDeferredNotHiddenOrdered(t *testing.T) { ctx := newContext() - addDeferred(ctx, "foo", "foo") + AddMockDeferredLoader(ctx, "foo", "foo") addDeferredWithQuery(ctx, "one", "1", "foo") - addDeferred(ctx, "foo", "baz") + AddMockDeferredLoader(ctx, "foo", "baz") ctx.Checkpoint() - addDeferred(ctx, "foo", "bar") + AddMockDeferredLoader(ctx, "foo", "bar") val, err := ctx.Query("one") assert.NilError(t, err) assert.Equal(t, "foo", val) diff --git a/pkg/engine/context/mock_deferred.go b/pkg/engine/context/mock_deferred.go new file mode 100644 index 0000000000..dde4859de1 --- /dev/null +++ b/pkg/engine/context/mock_deferred.go @@ -0,0 +1,92 @@ +package context + +import "fmt" + +type mockLoader struct { + name string + level int + value interface{} + query string + hasLoaded bool + invocations int + eventHandler func(event string) + ctx Interface +} + +func (ml *mockLoader) Name() string { + return ml.name +} + +func (ml *mockLoader) SetLevel(level int) { + ml.level = level +} + +func (ml *mockLoader) GetLevel() int { + return ml.level +} + +func (ml *mockLoader) HasLoaded() bool { + return ml.hasLoaded +} + +func (ml *mockLoader) LoadData() error { + ml.invocations++ + err := ml.ctx.AddVariable(ml.name, ml.value) + if err != nil { + return err + } + + // simulate a JMESPath evaluation after loading + if err := ml.executeQuery(); err != nil { + return err + } + + ml.hasLoaded = true + if ml.eventHandler != nil { + event := fmt.Sprintf("%s=%v", ml.name, ml.value) + ml.eventHandler(event) + } + + return nil +} + +func (ml *mockLoader) executeQuery() error { + if ml.query == "" { + return nil + } + + results, err := ml.ctx.Query(ml.query) + if err != nil { + return err + } + + return ml.ctx.AddVariable(ml.name, results) +} + +func (ml *mockLoader) setEventHandler(eventHandler func(string)) { + ml.eventHandler = eventHandler +} + +func AddMockDeferredLoader(ctx Interface, name string, value interface{}) (*mockLoader, error) { + return addDeferredWithQuery(ctx, name, value, "") +} + +func addDeferredWithQuery(ctx Interface, name string, value interface{}, query string) (*mockLoader, error) { + loader := &mockLoader{ + name: name, + value: value, + ctx: ctx, + query: query, + } + + d, err := NewDeferredLoader(name, loader, logger) + if err != nil { + return loader, err + } + + err = ctx.AddDeferredLoader(d) + if err != nil { + return nil, err + } + return loader, nil +} diff --git a/pkg/webhooks/resource/fake.go b/pkg/webhooks/resource/fake.go index 67569def92..a3c0deb84b 100644 --- a/pkg/webhooks/resource/fake.go +++ b/pkg/webhooks/resource/fake.go @@ -18,14 +18,13 @@ import ( "github.com/kyverno/kyverno/pkg/metrics" "github.com/kyverno/kyverno/pkg/policycache" "github.com/kyverno/kyverno/pkg/registryclient" - "github.com/kyverno/kyverno/pkg/webhooks" "github.com/kyverno/kyverno/pkg/webhooks/updaterequest" webhookutils "github.com/kyverno/kyverno/pkg/webhooks/utils" kubeinformers "k8s.io/client-go/informers" "k8s.io/client-go/kubernetes/fake" ) -func NewFakeHandlers(ctx context.Context, policyCache policycache.Cache) webhooks.ResourceHandlers { +func NewFakeHandlers(ctx context.Context, policyCache policycache.Cache) *resourceHandlers { client := fake.NewSimpleClientset() metricsConfig := metrics.NewFakeMetricsConfig() diff --git a/pkg/webhooks/resource/handlers.go b/pkg/webhooks/resource/handlers.go index a6eb8f22d1..ab0d6eae7c 100644 --- a/pkg/webhooks/resource/handlers.go +++ b/pkg/webhooks/resource/handlers.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "strings" + "sync" "time" "github.com/go-logr/logr" @@ -17,6 +18,7 @@ import ( "github.com/kyverno/kyverno/pkg/config" engineapi "github.com/kyverno/kyverno/pkg/engine/api" "github.com/kyverno/kyverno/pkg/engine/jmespath" + "github.com/kyverno/kyverno/pkg/engine/policycontext" "github.com/kyverno/kyverno/pkg/event" "github.com/kyverno/kyverno/pkg/metrics" "github.com/kyverno/kyverno/pkg/policycache" @@ -35,6 +37,8 @@ import ( ) type resourceHandlers struct { + wg sync.WaitGroup + // clients client dclient.Interface kyvernoClient versioned.Interface @@ -113,16 +117,11 @@ func (h *resourceHandlers) Validate(ctx context.Context, logger logr.Logger, req logger.V(4).Info("processing policies for validate admission request", "validate", len(policies), "mutate", len(mutatePolicies), "generate", len(generatePolicies)) - policyContext, err := h.pcBuilder.Build(request.AdmissionRequest, request.Roles, request.ClusterRoles, request.GroupVersionKind) + policyContext, err := h.buildPolicyContextFromAdmissionRequest(logger, request) if err != nil { return errorResponse(logger, request.UID, err, "failed create policy context") } - namespaceLabels := make(map[string]string) - if request.Kind.Kind != "Namespace" && request.Namespace != "" { - namespaceLabels = engineutils.GetNamespaceSelectorsFromNamespaceLister(request.Kind.Kind, request.Namespace, h.nsLister, logger) - } - policyContext = policyContext.WithNamespaceLabels(namespaceLabels) vh := validation.NewValidationHandler(logger, h.kyvernoClient, h.engine, h.pCache, h.pcBuilder, h.eventGen, h.admissionReports, h.metricsConfig, h.configuration) ok, msg, warnings := vh.HandleValidation(ctx, request, policies, policyContext, startTime) @@ -131,7 +130,9 @@ func (h *resourceHandlers) Validate(ctx context.Context, logger logr.Logger, req return admissionutils.Response(request.UID, errors.New(msg), warnings...) } if !admissionutils.IsDryRun(request.AdmissionRequest) { - go h.handleBackgroundApplies(ctx, logger, request.AdmissionRequest, policyContext, generatePolicies, mutatePolicies, startTime) + h.wg.Add(1) + go h.handleBackgroundApplies(ctx, logger, request, generatePolicies, mutatePolicies, startTime) + h.wg.Wait() } return admissionutils.ResponseSuccess(request.UID, warnings...) } @@ -241,6 +242,19 @@ func (h *resourceHandlers) retrieveAndCategorizePolicies( return policies, mutatePolicies, generatePolicies, imageVerifyValidatePolicies, nil } +func (h *resourceHandlers) buildPolicyContextFromAdmissionRequest(logger logr.Logger, request handlers.AdmissionRequest) (*policycontext.PolicyContext, error) { + policyContext, err := h.pcBuilder.Build(request.AdmissionRequest, request.Roles, request.ClusterRoles, request.GroupVersionKind) + if err != nil { + return nil, err + } + namespaceLabels := make(map[string]string) + if request.Kind.Kind != "Namespace" && request.Namespace != "" { + namespaceLabels = engineutils.GetNamespaceSelectorsFromNamespaceLister(request.Kind.Kind, request.Namespace, h.nsLister, logger) + } + policyContext = policyContext.WithNamespaceLabels(namespaceLabels) + return policyContext, nil +} + func filterPolicies(ctx context.Context, failurePolicy string, policies ...kyvernov1.PolicyInterface) []kyvernov1.PolicyInterface { var results []kyvernov1.PolicyInterface for _, policy := range policies { diff --git a/pkg/webhooks/resource/handlers_test.go b/pkg/webhooks/resource/handlers_test.go index 9b802b29e0..f419350dc1 100644 --- a/pkg/webhooks/resource/handlers_test.go +++ b/pkg/webhooks/resource/handlers_test.go @@ -3,17 +3,27 @@ package resource import ( "context" "encoding/json" + "fmt" "testing" "time" kyverno "github.com/kyverno/kyverno/api/kyverno/v1" + kyvernov1beta1 "github.com/kyverno/kyverno/api/kyverno/v1beta1" + "github.com/kyverno/kyverno/pkg/config" + "github.com/kyverno/kyverno/pkg/engine" + enginecontext "github.com/kyverno/kyverno/pkg/engine/context" + "github.com/kyverno/kyverno/pkg/engine/jmespath" + "github.com/kyverno/kyverno/pkg/engine/policycontext" log "github.com/kyverno/kyverno/pkg/logging" "github.com/kyverno/kyverno/pkg/policycache" "github.com/kyverno/kyverno/pkg/webhooks/handlers" "gotest.tools/assert" + admissionv1 "k8s.io/api/admission/v1" v1 "k8s.io/api/admission/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" + apiruntime "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/utils/pointer" ) var policyCheckLabel = `{ @@ -257,6 +267,121 @@ var pod = `{ } ` +var mutateAndGenerateMutatePolicy = `{ + "apiVersion": "kyverno.io/v1", + "kind": "ClusterPolicy", + "metadata": { + "name": "test-mutate" + }, + "spec": { + "rules": [ + { + "name": "test-mutate", + "match": { + "any": [ + { + "resources": { + "kinds": [ + "Pod" + ], + "operations": [ + "CREATE" + ] + } + } + ] + }, + "mutate": { + "foreach": [ + { + "list": "request.object.spec.containers", + "patchStrategicMerge": { + "spec": { + "containers": [ + { + "name": "{{ element.name }}", + "image": "{{ regex_replace_all('^([^/]+\\.[^/]+/)?(.*)$', '{{element.image}}', 'ghcr.io/kyverno/$2' )}}" + } + ] + } + } + } + ] + } + } + ] + } +}` + +var mutateAndGenerateGeneratePolicy = `{ + "apiVersion": "kyverno.io/v1", + "kind": "ClusterPolicy", + "metadata": { + "name": "test-generate" + }, + "spec": { + "rules": [ + { + "name": "test-generate", + "match": { + "any": [ + { + "resources": { + "kinds": [ + "Pod" + ], + "operations": [ + "CREATE" + ] + } + } + ] + }, + "generate": { + "synchronize": true, + "apiVersion": "v1", + "kind": "Pod", + "name": "pod1-{{request.name}}", + "namespace": "shared-dp", + "data": { + "spec": { + "containers": [ + { + "name": "container", + "image": "nginx", + "volumeMounts": [ + { + "name": "shared-volume", + "mountPath": "/data" + } + ] + } + ] + } + } + } + } + ] + } +}` + +var resourceMutateandGenerate = `{ + "apiVersion": "v1", + "kind": "Pod", + "metadata": { + "name": "pod-test-1", + "namespace": "shared-dp" + }, + "spec": { + "containers": [ + { + "name": "container", + "image": "nginx" + } + ] + } +}` + func Test_AdmissionResponseValid(t *testing.T) { policyCache := policycache.NewCache() logger := log.WithName("Test_AdmissionResponseValid") @@ -278,7 +403,7 @@ func Test_AdmissionResponseValid(t *testing.T) { Operation: v1.Create, Kind: metav1.GroupVersionKind{Group: "", Version: "v1", Kind: "Pod"}, Resource: metav1.GroupVersionResource{Group: "", Version: "v1", Resource: "pods"}, - Object: runtime.RawExtension{ + Object: apiruntime.RawExtension{ Raw: []byte(pod), }, RequestResource: &metav1.GroupVersionResource{Group: "", Version: "v1", Resource: "pods"}, @@ -320,7 +445,7 @@ func Test_AdmissionResponseInvalid(t *testing.T) { Operation: v1.Create, Kind: metav1.GroupVersionKind{Group: "", Version: "v1", Kind: "Pod"}, Resource: metav1.GroupVersionResource{Group: "", Version: "v1", Resource: "pods"}, - Object: runtime.RawExtension{ + Object: apiruntime.RawExtension{ Raw: []byte(pod), }, RequestResource: &metav1.GroupVersionResource{Group: "", Version: "v1", Resource: "pods"}, @@ -365,7 +490,7 @@ func Test_ImageVerify(t *testing.T) { Operation: v1.Create, Kind: metav1.GroupVersionKind{Group: "", Version: "v1", Kind: "Pod"}, Resource: metav1.GroupVersionResource{Group: "", Version: "v1", Resource: "pods"}, - Object: runtime.RawExtension{ + Object: apiruntime.RawExtension{ Raw: []byte(pod), }, RequestResource: &metav1.GroupVersionResource{Group: "", Version: "v1", Resource: "pods"}, @@ -409,7 +534,7 @@ func Test_MutateAndVerify(t *testing.T) { Operation: v1.Create, Kind: metav1.GroupVersionKind{Group: "", Version: "v1", Kind: "Pod"}, Resource: metav1.GroupVersionResource{Group: "", Version: "v1", Resource: "Pod"}, - Object: runtime.RawExtension{ + Object: apiruntime.RawExtension{ Raw: []byte(resourceMutateAndVerify), }, RequestResource: &metav1.GroupVersionResource{Group: "", Version: "v1", Resource: "pods"}, @@ -421,6 +546,81 @@ func Test_MutateAndVerify(t *testing.T) { assert.Equal(t, len(response.Warnings), 0) } +func Test_MutateAndGenerate(t *testing.T) { + policyCache := policycache.NewCache() + logger := log.WithName("Test_MutateAndGenerate") + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + resourceHandlers := NewFakeHandlers(ctx, policyCache) + + cfg := config.NewDefaultConfiguration(false) + jp := jmespath.New(cfg) + mockPcBuilder := newMockPolicyContextBuilder(cfg, jp) + resourceHandlers.pcBuilder = mockPcBuilder + + var generatePolicy kyverno.ClusterPolicy + err := json.Unmarshal([]byte(mutateAndGenerateGeneratePolicy), &generatePolicy) + assert.NilError(t, err) + + key := makeKey(&generatePolicy) + policyCache.Set(key, &generatePolicy, policycache.TestResourceFinder{}) + + var mutatePolicy kyverno.ClusterPolicy + err = json.Unmarshal([]byte(mutateAndGenerateMutatePolicy), &mutatePolicy) + assert.NilError(t, err) + + key = makeKey(&mutatePolicy) + policyCache.Set(key, &mutatePolicy, policycache.TestResourceFinder{}) + + request := handlers.AdmissionRequest{ + AdmissionRequest: v1.AdmissionRequest{ + Operation: v1.Create, + Kind: metav1.GroupVersionKind{Group: "", Version: "v1", Kind: "Pod"}, + Resource: metav1.GroupVersionResource{Group: "", Version: "v1", Resource: "Pod"}, + Object: apiruntime.RawExtension{ + Raw: []byte(resourceMutateandGenerate), + }, + RequestResource: &metav1.GroupVersionResource{Group: "", Version: "v1", Resource: "pods"}, + DryRun: pointer.Bool(false), + }, + } + + response := resourceHandlers.Validate(ctx, logger, request, "", time.Now()) + + assert.Assert(t, len(mockPcBuilder.contexts) >= 3, fmt.Sprint("expected no of context ", 3, " received ", len(mockPcBuilder.contexts))) + + validateJSONContext := mockPcBuilder.contexts[0].JSONContext() + mutateJSONContext := mockPcBuilder.contexts[1].JSONContext() + generateJSONContext := mockPcBuilder.contexts[2].JSONContext() + + _, err = enginecontext.AddMockDeferredLoader(validateJSONContext, "key1", "value1") + assert.NilError(t, err) + _, err = enginecontext.AddMockDeferredLoader(mutateJSONContext, "key2", "value2") + assert.NilError(t, err) + _, err = enginecontext.AddMockDeferredLoader(generateJSONContext, "key3", "value3") + assert.NilError(t, err) + + _, err = mutateJSONContext.Query("key1") + assert.ErrorContains(t, err, `Unknown key "key1" in path`) + _, err = generateJSONContext.Query("key1") + assert.ErrorContains(t, err, `Unknown key "key1" in path`) + + _, err = validateJSONContext.Query("key2") + assert.ErrorContains(t, err, `Unknown key "key2" in path`) + _, err = generateJSONContext.Query("key2") + assert.ErrorContains(t, err, `Unknown key "key2" in path`) + + _, err = validateJSONContext.Query("key3") + assert.ErrorContains(t, err, `Unknown key "key3" in path`) + _, err = mutateJSONContext.Query("key3") + assert.ErrorContains(t, err, `Unknown key "key3" in path`) + + assert.Equal(t, response.Allowed, true) + assert.Equal(t, len(response.Warnings), 0) +} + func makeKey(policy kyverno.PolicyInterface) string { name := policy.GetName() namespace := policy.GetNamespace() @@ -430,3 +630,37 @@ func makeKey(policy kyverno.PolicyInterface) string { return namespace + "/" + name } + +type mockPolicyContextBuilder struct { + configuration config.Configuration + jp jmespath.Interface + contexts []*engine.PolicyContext + count int +} + +func newMockPolicyContextBuilder( + configuration config.Configuration, + jp jmespath.Interface, +) *mockPolicyContextBuilder { + return &mockPolicyContextBuilder{ + configuration: configuration, + jp: jp, + contexts: make([]*policycontext.PolicyContext, 0), + count: 0, + } +} + +func (b *mockPolicyContextBuilder) Build(request admissionv1.AdmissionRequest, roles, clusterRoles []string, gvk schema.GroupVersionKind) (*engine.PolicyContext, error) { + userRequestInfo := kyvernov1beta1.RequestInfo{ + AdmissionUserInfo: *request.UserInfo.DeepCopy(), + Roles: roles, + ClusterRoles: clusterRoles, + } + pc, err := engine.NewPolicyContextFromAdmissionRequest(b.jp, request, userRequestInfo, gvk, b.configuration) + if err != nil { + return nil, err + } + b.count += 1 + b.contexts = append(b.contexts, pc) + return pc, err +} diff --git a/pkg/webhooks/resource/updaterequest.go b/pkg/webhooks/resource/updaterequest.go index 2dcbc70791..eeb6cb162a 100644 --- a/pkg/webhooks/resource/updaterequest.go +++ b/pkg/webhooks/resource/updaterequest.go @@ -9,25 +9,31 @@ import ( kyvernov1 "github.com/kyverno/kyverno/api/kyverno/v1" kyvernov1beta1 "github.com/kyverno/kyverno/api/kyverno/v1beta1" "github.com/kyverno/kyverno/pkg/autogen" - "github.com/kyverno/kyverno/pkg/engine" engineapi "github.com/kyverno/kyverno/pkg/engine/api" "github.com/kyverno/kyverno/pkg/event" datautils "github.com/kyverno/kyverno/pkg/utils/data" + "github.com/kyverno/kyverno/pkg/webhooks/handlers" "github.com/kyverno/kyverno/pkg/webhooks/resource/generation" webhookutils "github.com/kyverno/kyverno/pkg/webhooks/utils" admissionv1 "k8s.io/api/admission/v1" ) // handleBackgroundApplies applies generate and mutateExisting policies, and creates update requests for background reconcile -func (h *resourceHandlers) handleBackgroundApplies(ctx context.Context, logger logr.Logger, request admissionv1.AdmissionRequest, policyContext *engine.PolicyContext, generatePolicies, mutatePolicies []kyvernov1.PolicyInterface, ts time.Time) { - go h.handleMutateExisting(ctx, logger, request, mutatePolicies, policyContext, ts) - h.handleGenerate(ctx, logger, request, generatePolicies, policyContext, ts) +func (h *resourceHandlers) handleBackgroundApplies(ctx context.Context, logger logr.Logger, request handlers.AdmissionRequest, generatePolicies, mutatePolicies []kyvernov1.PolicyInterface, ts time.Time) { + h.wg.Add(1) + go h.handleMutateExisting(ctx, logger, request, mutatePolicies, ts) + h.handleGenerate(ctx, logger, request, generatePolicies, ts) } -func (h *resourceHandlers) handleMutateExisting(ctx context.Context, logger logr.Logger, request admissionv1.AdmissionRequest, policies []kyvernov1.PolicyInterface, polCtx *engine.PolicyContext, admissionRequestTimestamp time.Time) { - policyContext := &engine.PolicyContext{} - *policyContext = *polCtx - if request.Operation == admissionv1.Delete { +func (h *resourceHandlers) handleMutateExisting(ctx context.Context, logger logr.Logger, request handlers.AdmissionRequest, policies []kyvernov1.PolicyInterface, admissionRequestTimestamp time.Time) { + policyContext, err := h.buildPolicyContextFromAdmissionRequest(logger, request) + if err != nil { + logger.Error(err, "failed to create policy context") + return + } + h.wg.Done() + + if request.AdmissionRequest.Operation == admissionv1.Delete { policyContext = policyContext.WithNewResource(policyContext.OldResource()) } @@ -46,7 +52,7 @@ func (h *resourceHandlers) handleMutateExisting(ctx context.Context, logger logr // skip rules that don't specify the DELETE operation in case the admission request is of type DELETE var skipped []string for _, rule := range autogen.ComputeRules(policy) { - if request.Operation == admissionv1.Delete && !webhookutils.MatchDeleteOperation(rule) { + if request.AdmissionRequest.Operation == admissionv1.Delete && !webhookutils.MatchDeleteOperation(rule) { skipped = append(skipped, rule.Name) } } @@ -67,7 +73,7 @@ func (h *resourceHandlers) handleMutateExisting(ctx context.Context, logger logr } } - if failedResponse := applyUpdateRequest(ctx, request, kyvernov1beta1.Mutate, h.urGenerator, policyContext.AdmissionInfo(), request.Operation, engineResponses...); failedResponse != nil { + if failedResponse := applyUpdateRequest(ctx, request.AdmissionRequest, kyvernov1beta1.Mutate, h.urGenerator, policyContext.AdmissionInfo(), request.Operation, engineResponses...); failedResponse != nil { for _, failedUR := range failedResponse { err := fmt.Errorf("failed to create update request: %v", failedUR.err) @@ -86,7 +92,14 @@ func (h *resourceHandlers) handleMutateExisting(ctx context.Context, logger logr } } -func (h *resourceHandlers) handleGenerate(ctx context.Context, logger logr.Logger, request admissionv1.AdmissionRequest, generatePolicies []kyvernov1.PolicyInterface, policyContext *engine.PolicyContext, ts time.Time) { +func (h *resourceHandlers) handleGenerate(ctx context.Context, logger logr.Logger, request handlers.AdmissionRequest, generatePolicies []kyvernov1.PolicyInterface, ts time.Time) { + policyContext, err := h.buildPolicyContextFromAdmissionRequest(logger, request) + if err != nil { + logger.Error(err, "failed to create policy context") + return + } + h.wg.Done() + gh := generation.NewGenerationHandler(logger, h.engine, h.client, h.kyvernoClient, h.nsLister, h.urLister, h.cpolLister, h.polLister, h.urGenerator, h.eventGen, h.metricsConfig, h.backgroundServiceAccountName) var policies []kyvernov1.PolicyInterface for _, p := range generatePolicies { @@ -95,5 +108,5 @@ func (h *resourceHandlers) handleGenerate(ctx context.Context, logger logr.Logge policies = append(policies, new) } } - go gh.Handle(ctx, request, policies, policyContext) + go gh.Handle(ctx, request.AdmissionRequest, policies, policyContext) } diff --git a/test/conformance/chainsaw/generate/clusterpolicy/standard/data/sync/cpol-data-sync-mutate-and-generate/README.md b/test/conformance/chainsaw/generate/clusterpolicy/standard/data/sync/cpol-data-sync-mutate-and-generate/README.md new file mode 100644 index 0000000000..fb6e4a0c93 --- /dev/null +++ b/test/conformance/chainsaw/generate/clusterpolicy/standard/data/sync/cpol-data-sync-mutate-and-generate/README.md @@ -0,0 +1,6 @@ +# Title + +This is a generate test to ensure that when there is a generate and a mutate policy present, the deferred loader does not panic because of concurrency issues in the policy context. + +# Related Issue +https://github.com/kyverno/kyverno/issues/9413 diff --git a/test/conformance/chainsaw/generate/clusterpolicy/standard/data/sync/cpol-data-sync-mutate-and-generate/chainsaw-test.yaml b/test/conformance/chainsaw/generate/clusterpolicy/standard/data/sync/cpol-data-sync-mutate-and-generate/chainsaw-test.yaml new file mode 100755 index 0000000000..4c27bb9b6b --- /dev/null +++ b/test/conformance/chainsaw/generate/clusterpolicy/standard/data/sync/cpol-data-sync-mutate-and-generate/chainsaw-test.yaml @@ -0,0 +1,70 @@ +apiVersion: chainsaw.kyverno.io/v1alpha1 +kind: Test +metadata: + creationTimestamp: null + name: cpol-data-sync-create +spec: + steps: + - name: step-01 + try: + - apply: + file: ns.yaml + - name: step-02 + try: + - apply: + file: first-pod.yaml + - name: step-03 + try: + - apply: + file: policies.yaml + - name: step-04 + try: + - apply: + file: pod1.yaml + - name: step-05 # checks if admission controller was restarted + try: + - script: + content: kubectl get po -A | awk '$5>0' | grep -q 'kyverno-admission-controller' + check: + # there should not be any matching value thus error != null is true + ($error != null): true + - name: step-06 + try: + - apply: + file: pod2.yaml + - name: step-07 + try: + - script: + content: kubectl get po -A | awk '$5>0' | grep -q 'kyverno-admission-controller' + check: + ($error != null): true + - name: step-08 + try: + - apply: + file: pod3.yaml + - name: step-09 + try: + - script: + content: kubectl get po -A | awk '$5>0' | grep -q 'kyverno-admission-controller' + check: + ($error != null): true + - name: step-10 + try: + - apply: + file: pod4.yaml + - name: step-11 + try: + - script: + content: kubectl get po -A | awk '$5>0' | grep -q 'kyverno-admission-controller' + check: + ($error != null): true + - name: step-12 + try: + - apply: + file: pod5.yaml + - name: step-13 + try: + - script: + content: kubectl get po -A | awk '$5>0' | grep -q 'kyverno-admission-controller' + check: + ($error != null): true diff --git a/test/conformance/chainsaw/generate/clusterpolicy/standard/data/sync/cpol-data-sync-mutate-and-generate/first-pod.yaml b/test/conformance/chainsaw/generate/clusterpolicy/standard/data/sync/cpol-data-sync-mutate-and-generate/first-pod.yaml new file mode 100644 index 0000000000..1d3b1b7238 --- /dev/null +++ b/test/conformance/chainsaw/generate/clusterpolicy/standard/data/sync/cpol-data-sync-mutate-and-generate/first-pod.yaml @@ -0,0 +1,9 @@ +apiVersion: v1 +kind: Pod +metadata: + name: first-pod + namespace: shared-dp +spec: + containers: + - name: container + image: nginx diff --git a/test/conformance/chainsaw/generate/clusterpolicy/standard/data/sync/cpol-data-sync-mutate-and-generate/ns.yaml b/test/conformance/chainsaw/generate/clusterpolicy/standard/data/sync/cpol-data-sync-mutate-and-generate/ns.yaml new file mode 100644 index 0000000000..e7303f56b6 --- /dev/null +++ b/test/conformance/chainsaw/generate/clusterpolicy/standard/data/sync/cpol-data-sync-mutate-and-generate/ns.yaml @@ -0,0 +1,4 @@ +apiVersion: v1 +kind: Namespace +metadata: + name: shared-dp diff --git a/test/conformance/chainsaw/generate/clusterpolicy/standard/data/sync/cpol-data-sync-mutate-and-generate/pod1.yaml b/test/conformance/chainsaw/generate/clusterpolicy/standard/data/sync/cpol-data-sync-mutate-and-generate/pod1.yaml new file mode 100644 index 0000000000..781fb00c42 --- /dev/null +++ b/test/conformance/chainsaw/generate/clusterpolicy/standard/data/sync/cpol-data-sync-mutate-and-generate/pod1.yaml @@ -0,0 +1,9 @@ +apiVersion: v1 +kind: Pod +metadata: + name: pod-test-1 + namespace: shared-dp +spec: + containers: + - name: container + image: nginx diff --git a/test/conformance/chainsaw/generate/clusterpolicy/standard/data/sync/cpol-data-sync-mutate-and-generate/pod2.yaml b/test/conformance/chainsaw/generate/clusterpolicy/standard/data/sync/cpol-data-sync-mutate-and-generate/pod2.yaml new file mode 100644 index 0000000000..5e36578bf1 --- /dev/null +++ b/test/conformance/chainsaw/generate/clusterpolicy/standard/data/sync/cpol-data-sync-mutate-and-generate/pod2.yaml @@ -0,0 +1,11 @@ +apiVersion: v1 +kind: Pod +metadata: + name: pod-test-2 + namespace: shared-dp +spec: + containers: + - name: container + image: nginx + + diff --git a/test/conformance/chainsaw/generate/clusterpolicy/standard/data/sync/cpol-data-sync-mutate-and-generate/pod3.yaml b/test/conformance/chainsaw/generate/clusterpolicy/standard/data/sync/cpol-data-sync-mutate-and-generate/pod3.yaml new file mode 100644 index 0000000000..c617944a31 --- /dev/null +++ b/test/conformance/chainsaw/generate/clusterpolicy/standard/data/sync/cpol-data-sync-mutate-and-generate/pod3.yaml @@ -0,0 +1,9 @@ +apiVersion: v1 +kind: Pod +metadata: + name: pod-test-3 + namespace: shared-dp +spec: + containers: + - name: container + image: nginx diff --git a/test/conformance/chainsaw/generate/clusterpolicy/standard/data/sync/cpol-data-sync-mutate-and-generate/pod4.yaml b/test/conformance/chainsaw/generate/clusterpolicy/standard/data/sync/cpol-data-sync-mutate-and-generate/pod4.yaml new file mode 100644 index 0000000000..9d1e6fff2e --- /dev/null +++ b/test/conformance/chainsaw/generate/clusterpolicy/standard/data/sync/cpol-data-sync-mutate-and-generate/pod4.yaml @@ -0,0 +1,9 @@ +apiVersion: v1 +kind: Pod +metadata: + name: pod-test-4 + namespace: shared-dp +spec: + containers: + - name: container + image: nginx diff --git a/test/conformance/chainsaw/generate/clusterpolicy/standard/data/sync/cpol-data-sync-mutate-and-generate/pod5.yaml b/test/conformance/chainsaw/generate/clusterpolicy/standard/data/sync/cpol-data-sync-mutate-and-generate/pod5.yaml new file mode 100644 index 0000000000..711d700954 --- /dev/null +++ b/test/conformance/chainsaw/generate/clusterpolicy/standard/data/sync/cpol-data-sync-mutate-and-generate/pod5.yaml @@ -0,0 +1,9 @@ +apiVersion: v1 +kind: Pod +metadata: + name: pod-test-5 + namespace: shared-dp +spec: + containers: + - name: container + image: nginx diff --git a/test/conformance/chainsaw/generate/clusterpolicy/standard/data/sync/cpol-data-sync-mutate-and-generate/policies.yaml b/test/conformance/chainsaw/generate/clusterpolicy/standard/data/sync/cpol-data-sync-mutate-and-generate/policies.yaml new file mode 100644 index 0000000000..93ad43fed5 --- /dev/null +++ b/test/conformance/chainsaw/generate/clusterpolicy/standard/data/sync/cpol-data-sync-mutate-and-generate/policies.yaml @@ -0,0 +1,72 @@ +apiVersion: kyverno.io/v1 +kind: ClusterPolicy +metadata: + name: test-mutate +spec: + rules: + - name: test-mutate + match: + any: + - resources: + kinds: + - Pod + operations: + - CREATE + context: + - name: list_pods + apiCall: + urlPath: "/api/v1/namespaces/{{request.namespace}}/pods" + jmesPath: "items[]" + preconditions: + all: + - key: "{{ length(list_pods) }}" + operator: GreaterThan + value: 0 + mutate: + targets: + - apiVersion: v1 + kind: Pod + name: "{{ list_pods[0].metadata.name }}" + patchesJson6902: |- + - path: "/spec/priority" + op: add + value: 1000 +--- +apiVersion: kyverno.io/v1 +kind: ClusterPolicy +metadata: + name: test-generate +spec: + rules: + - name: test-generate + match: + any: + - resources: + kinds: + - Pod + operations: + - CREATE + context: + - name: list_pods + apiCall: + urlPath: "/api/v1/namespaces/{{request.namespace}}/pods" + jmesPath: "items[]" + preconditions: + all: + - key: "{{ length(list_pods) }}" + operator: GreaterThan + value: 0 + generate: + synchronize: true + apiVersion: v1 + kind: Pod + name: pod1-{{request.name}} + namespace: shared-dp + data: + spec: + containers: + - name: container + image: nginx + volumeMounts: + - name: shared-volume + mountPath: /data