From c630f17ec424eef45060ed66ba37d9fbd83f98da Mon Sep 17 00:00:00 2001 From: Vishal Choudhary Date: Wed, 22 Nov 2023 22:31:46 +0530 Subject: [PATCH] fix: block mutation only when failurePolicy is set to fail (#8952) * fix: only block mutation when failurePolicy is set to fail Signed-off-by: Vishal Choudhary * feat: kuttl test Signed-off-by: Vishal Choudhary * fix: add else check Signed-off-by: Vishal Choudhary * fix: update defaulting ns label policy's failure policy to be fail based on readme, this test has nothing to do with failurePolicy and resource should not be blocked in case of ignore failurePolicy Signed-off-by: Vishal Choudhary * fix: there is another Signed-off-by: Vishal Choudhary * fix: update policy Signed-off-by: Vishal Choudhary * nit Signed-off-by: Vishal Choudhary * feat: add logs Signed-off-by: Vishal Choudhary * Update pkg/webhooks/resource/mutation/mutation.go Signed-off-by: shuting --------- Signed-off-by: Vishal Choudhary Signed-off-by: shuting Co-authored-by: shuting Co-authored-by: shuting --- pkg/webhooks/resource/mutation/mutation.go | 17 +++++++++++--- .../01-manifests.yaml | 2 +- .../04-manifests.yaml | 2 +- .../mutate-with-404-api-call/01-policy.yaml | 6 +++++ .../mutate-with-404-api-call/02-pod.yaml | 6 +++++ .../mutate-with-404-api-call/README.md | 11 +++++++++ .../mutate-with-404-api-call/pod-assert.yaml | 5 ++++ .../mutate-with-404-api-call/pod.yaml | 10 ++++++++ .../policy-assert.yaml | 4 ++++ .../mutate-with-404-api-call/policy.yaml | 23 +++++++++++++++++++ 10 files changed, 81 insertions(+), 5 deletions(-) create mode 100644 test/conformance/kuttl/mutate/clusterpolicy/cornercases/mutate-with-404-api-call/01-policy.yaml create mode 100644 test/conformance/kuttl/mutate/clusterpolicy/cornercases/mutate-with-404-api-call/02-pod.yaml create mode 100644 test/conformance/kuttl/mutate/clusterpolicy/cornercases/mutate-with-404-api-call/README.md create mode 100644 test/conformance/kuttl/mutate/clusterpolicy/cornercases/mutate-with-404-api-call/pod-assert.yaml create mode 100644 test/conformance/kuttl/mutate/clusterpolicy/cornercases/mutate-with-404-api-call/pod.yaml create mode 100644 test/conformance/kuttl/mutate/clusterpolicy/cornercases/mutate-with-404-api-call/policy-assert.yaml create mode 100644 test/conformance/kuttl/mutate/clusterpolicy/cornercases/mutate-with-404-api-call/policy.yaml diff --git a/pkg/webhooks/resource/mutation/mutation.go b/pkg/webhooks/resource/mutation/mutation.go index c9698fe21a..8fd35cebb6 100644 --- a/pkg/webhooks/resource/mutation/mutation.go +++ b/pkg/webhooks/resource/mutation/mutation.go @@ -82,6 +82,7 @@ func (v *mutationHandler) applyMutations( var patches []jsonpatch.JsonPatchOperation var engineResponses []engineapi.EngineResponse + failurePolicy := kyvernov1.Ignore for _, policy := range policies { spec := policy.GetSpec() @@ -96,7 +97,11 @@ func (v *mutationHandler) applyMutations( func(ctx context.Context, span trace.Span) error { v.log.V(3).Info("applying policy mutate rules", "policy", policy.GetName()) currentContext := policyContext.WithPolicy(policy) - engineResponse, policyPatches, err := v.applyMutation(ctx, request, currentContext) + if policy.GetSpec().GetFailurePolicy(ctx) == kyvernov1.Fail { + failurePolicy = kyvernov1.Fail + } + + engineResponse, policyPatches, err := v.applyMutation(ctx, request, currentContext, failurePolicy) if err != nil { return fmt.Errorf("mutation policy %s error: %v", policy.GetName(), err) } @@ -131,7 +136,7 @@ func (v *mutationHandler) applyMutations( return jsonutils.JoinPatches(patch.ConvertPatches(patches...)...), engineResponses, nil } -func (h *mutationHandler) applyMutation(ctx context.Context, request admissionv1.AdmissionRequest, policyContext *engine.PolicyContext) (*engineapi.EngineResponse, []jsonpatch.JsonPatchOperation, error) { +func (h *mutationHandler) applyMutation(ctx context.Context, request admissionv1.AdmissionRequest, policyContext *engine.PolicyContext, failurePolicy kyvernov1.FailurePolicyType) (*engineapi.EngineResponse, []jsonpatch.JsonPatchOperation, error) { if request.Kind.Kind != "Namespace" && request.Namespace != "" { policyContext = policyContext.WithNamespaceLabels(engineutils.GetNamespaceSelectorsFromNamespaceLister(request.Kind.Kind, request.Namespace, h.nsLister, h.log)) } @@ -140,7 +145,13 @@ func (h *mutationHandler) applyMutation(ctx context.Context, request admissionv1 policyPatches := engineResponse.GetPatches() if !engineResponse.IsSuccessful() { - return nil, nil, fmt.Errorf("failed to apply policy %s rules %v", policyContext.Policy().GetName(), engineResponse.GetFailedRulesWithErrors()) + if webhookutils.BlockRequest([]engineapi.EngineResponse{engineResponse}, failurePolicy, h.log) { + h.log.Info("failed to apply policy, blocking request", "policy", policyContext.Policy().GetName(), "rules", engineResponse.GetFailedRulesWithErrors()) + return nil, nil, fmt.Errorf("failed to apply policy %s rules %v", policyContext.Policy().GetName(), engineResponse.GetFailedRulesWithErrors()) + } else { + h.log.Info("ignoring unsuccessful engine responses", "policy", policyContext.Policy().GetName(), "rules", engineResponse.GetFailedRulesWithErrors()) + return &engineResponse, nil, nil + } } return &engineResponse, policyPatches, nil diff --git a/test/conformance/kuttl/mutate/clusterpolicy/cornercases/defaulting-namespace-labels/01-manifests.yaml b/test/conformance/kuttl/mutate/clusterpolicy/cornercases/defaulting-namespace-labels/01-manifests.yaml index ec2377eb57..01f9296295 100644 --- a/test/conformance/kuttl/mutate/clusterpolicy/cornercases/defaulting-namespace-labels/01-manifests.yaml +++ b/test/conformance/kuttl/mutate/clusterpolicy/cornercases/defaulting-namespace-labels/01-manifests.yaml @@ -3,7 +3,7 @@ kind: ClusterPolicy metadata: name: propagate-cost-labels-from-namespace spec: - failurePolicy: Ignore + failurePolicy: Fail rules: - name: add-cost-labels context: diff --git a/test/conformance/kuttl/mutate/clusterpolicy/cornercases/defaulting-namespace-labels/04-manifests.yaml b/test/conformance/kuttl/mutate/clusterpolicy/cornercases/defaulting-namespace-labels/04-manifests.yaml index 80983ca4e3..11013d6d91 100644 --- a/test/conformance/kuttl/mutate/clusterpolicy/cornercases/defaulting-namespace-labels/04-manifests.yaml +++ b/test/conformance/kuttl/mutate/clusterpolicy/cornercases/defaulting-namespace-labels/04-manifests.yaml @@ -3,7 +3,7 @@ kind: ClusterPolicy metadata: name: propagate-cost-labels-from-namespace spec: - failurePolicy: Ignore + failurePolicy: Fail rules: - name: add-cost-labels context: diff --git a/test/conformance/kuttl/mutate/clusterpolicy/cornercases/mutate-with-404-api-call/01-policy.yaml b/test/conformance/kuttl/mutate/clusterpolicy/cornercases/mutate-with-404-api-call/01-policy.yaml new file mode 100644 index 0000000000..b088ed7601 --- /dev/null +++ b/test/conformance/kuttl/mutate/clusterpolicy/cornercases/mutate-with-404-api-call/01-policy.yaml @@ -0,0 +1,6 @@ +apiVersion: kuttl.dev/v1beta1 +kind: TestStep +apply: +- policy.yaml +assert: +- policy-assert.yaml diff --git a/test/conformance/kuttl/mutate/clusterpolicy/cornercases/mutate-with-404-api-call/02-pod.yaml b/test/conformance/kuttl/mutate/clusterpolicy/cornercases/mutate-with-404-api-call/02-pod.yaml new file mode 100644 index 0000000000..3e1752d840 --- /dev/null +++ b/test/conformance/kuttl/mutate/clusterpolicy/cornercases/mutate-with-404-api-call/02-pod.yaml @@ -0,0 +1,6 @@ +apiVersion: kuttl.dev/v1beta1 +kind: TestStep +apply: +- pod.yaml +assert: +- pod-assert.yaml diff --git a/test/conformance/kuttl/mutate/clusterpolicy/cornercases/mutate-with-404-api-call/README.md b/test/conformance/kuttl/mutate/clusterpolicy/cornercases/mutate-with-404-api-call/README.md new file mode 100644 index 0000000000..6220bb7e84 --- /dev/null +++ b/test/conformance/kuttl/mutate/clusterpolicy/cornercases/mutate-with-404-api-call/README.md @@ -0,0 +1,11 @@ +## Description + +This test checks that the mutate policy does not fail because of 404 in API Call when failure policy is set to `Ignore`. + +## Expected Behavior + +The failure policy in the policy is set to Ignore and the API Call refers to a non existent URL. Mutation should not happen and error should not be thrown. + +## Reference Issue(s) + +https://github.com/kyverno/kyverno/issues/8936 diff --git a/test/conformance/kuttl/mutate/clusterpolicy/cornercases/mutate-with-404-api-call/pod-assert.yaml b/test/conformance/kuttl/mutate/clusterpolicy/cornercases/mutate-with-404-api-call/pod-assert.yaml new file mode 100644 index 0000000000..ccb73f4a08 --- /dev/null +++ b/test/conformance/kuttl/mutate/clusterpolicy/cornercases/mutate-with-404-api-call/pod-assert.yaml @@ -0,0 +1,5 @@ +apiVersion: v1 +kind: Pod +metadata: + name: mutate-404-api-call-example + namespace: default diff --git a/test/conformance/kuttl/mutate/clusterpolicy/cornercases/mutate-with-404-api-call/pod.yaml b/test/conformance/kuttl/mutate/clusterpolicy/cornercases/mutate-with-404-api-call/pod.yaml new file mode 100644 index 0000000000..521fbfaa95 --- /dev/null +++ b/test/conformance/kuttl/mutate/clusterpolicy/cornercases/mutate-with-404-api-call/pod.yaml @@ -0,0 +1,10 @@ +apiVersion: v1 +kind: Pod +metadata: + name: mutate-404-api-call-example + namespace: default +spec: + containers: + - name: example + image: busybox + args: ["sleep", "infinity"] diff --git a/test/conformance/kuttl/mutate/clusterpolicy/cornercases/mutate-with-404-api-call/policy-assert.yaml b/test/conformance/kuttl/mutate/clusterpolicy/cornercases/mutate-with-404-api-call/policy-assert.yaml new file mode 100644 index 0000000000..3a5cb6bb42 --- /dev/null +++ b/test/conformance/kuttl/mutate/clusterpolicy/cornercases/mutate-with-404-api-call/policy-assert.yaml @@ -0,0 +1,4 @@ +apiVersion: kyverno.io/v1 +kind: ClusterPolicy +metadata: + name: mutate-404-api-call diff --git a/test/conformance/kuttl/mutate/clusterpolicy/cornercases/mutate-with-404-api-call/policy.yaml b/test/conformance/kuttl/mutate/clusterpolicy/cornercases/mutate-with-404-api-call/policy.yaml new file mode 100644 index 0000000000..1d85202026 --- /dev/null +++ b/test/conformance/kuttl/mutate/clusterpolicy/cornercases/mutate-with-404-api-call/policy.yaml @@ -0,0 +1,23 @@ +apiVersion: kyverno.io/v1 +kind: ClusterPolicy +metadata: + name: mutate-404-api-call +spec: + failurePolicy: Ignore + rules: + - name: mutate-404-api-call + context: + - name: val + apiCall: + service: + url: "https://www.google.com/404" + match: + any: + - resources: + kinds: + - Pod + mutate: + patchStrategicMerge: + metadata: + labels: + foo: "{{ val }}"