fix: block mutation only when failurePolicy is set to fail #8952

vishal-chdhry · 2023-11-19T09:02:31Z

Explanation

Mutation policies should not fail when failurePolicy is set to Ignore. This behaviour was working in 1.10.x because of this bug that was fixed in 1.11.0 (The issue there was that nil caused API Calls to not fail in any case and errors from API Call is not returned).

In 1.11, failurePolicy: Ignore still causes failure as we are not checking for its value in mutation webhooks

Related issue

Closes: #8936

Milestone of this PR

/milestone 1.11.1

What type of PR is this

/kind bug

Proposed Changes

Proof Manifests

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:
        urlPath: "https://www.google.com/404"
    match:
      any:
      - resources:
          kinds:
          - Pod
    mutate:
      patchStrategicMerge:
        metadata:
          labels:
            foo: "{{ val }}"
---
apiVersion: v1
kind: Pod
metadata:
  name: mutate-404-api-call-example
  namespace: default
spec:
  containers:
  - name: example
    image: busybox
    args: ["sleep", "infinity"]

Checklist

I have read the contributing guidelines.
I have read the PR documentation guide and followed the process including adding proof manifests to this PR.
This is a bug fix and I have added unit tests that prove my fix is effective.
This is a feature and I have added CLI tests that are applicable.
My PR needs to be cherry picked to a specific release branch which is release 1.11.1.
My PR contains new or altered behavior to Kyverno and
- CLI support should be added and my PR doesn't contain that functionality.
- I have added or changed the documentation myself in an existing PR and the link is:
- I have raised an issue in kyverno/website to track the documentation update and the link is:

Further Comments

to fail Signed-off-by: Vishal Choudhary <vishal.choudhary@nirmata.com>

Signed-off-by: Vishal Choudhary <vishal.choudhary@nirmata.com>

codecov · 2023-11-19T09:07:00Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (058c162) 33.60% compared to head (6aeca12) 33.60%.
Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #8952   +/-   ##
=======================================
  Coverage   33.60%   33.60%           
=======================================
  Files         315      315           
  Lines       24953    24953           
=======================================
  Hits         8386     8386           
  Misses      15769    15769           
  Partials      798      798

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Signed-off-by: Vishal Choudhary <vishal.choudhary@nirmata.com>

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 <vishal.choudhary@nirmata.com>

vishal-chdhry · 2023-11-19T10:10:00Z

Reason for unrelated change 3b7bd0d:
Based on the readme, this test has nothing to do with failurePolicy and the resource should not be blocked in case of Ignore failurePolicy

Signed-off-by: Vishal Choudhary <vishal.choudhary@nirmata.com>

pkg/webhooks/resource/mutation/mutation.go

Signed-off-by: Vishal Choudhary <vishal.choudhary@nirmata.com>

pkg/webhooks/resource/mutation/mutation.go

Signed-off-by: shuting <shuting@nirmata.com>

realshuting · 2023-11-22T16:23:33Z

/cherry-pick release-1.11

* fix: only block mutation when failurePolicy is set to fail Signed-off-by: Vishal Choudhary <vishal.choudhary@nirmata.com> * feat: kuttl test Signed-off-by: Vishal Choudhary <vishal.choudhary@nirmata.com> * fix: add else check Signed-off-by: Vishal Choudhary <vishal.choudhary@nirmata.com> * 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 <vishal.choudhary@nirmata.com> * fix: there is another Signed-off-by: Vishal Choudhary <vishal.choudhary@nirmata.com> * fix: update policy Signed-off-by: Vishal Choudhary <vishal.choudhary@nirmata.com> * nit Signed-off-by: Vishal Choudhary <vishal.choudhary@nirmata.com> * feat: add logs Signed-off-by: Vishal Choudhary <vishal.choudhary@nirmata.com> * Update pkg/webhooks/resource/mutation/mutation.go Signed-off-by: shuting <shuting@nirmata.com> --------- Signed-off-by: Vishal Choudhary <vishal.choudhary@nirmata.com> Signed-off-by: shuting <shuting@nirmata.com> Co-authored-by: shuting <shuting@nirmata.com> Co-authored-by: shuting <shutting06@gmail.com>

…8986) * fix: only block mutation when failurePolicy is set to fail * feat: kuttl test * fix: add else check * 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 * fix: there is another * fix: update policy * nit * feat: add logs * Update pkg/webhooks/resource/mutation/mutation.go --------- Signed-off-by: Vishal Choudhary <vishal.choudhary@nirmata.com> Signed-off-by: shuting <shuting@nirmata.com> Co-authored-by: Vishal Choudhary <sendtovishalchoudhary@gmail.com> Co-authored-by: shuting <shuting@nirmata.com> Co-authored-by: shuting <shutting06@gmail.com>

) * fix: only block mutation when failurePolicy is set to fail Signed-off-by: Vishal Choudhary <vishal.choudhary@nirmata.com> * feat: kuttl test Signed-off-by: Vishal Choudhary <vishal.choudhary@nirmata.com> * fix: add else check Signed-off-by: Vishal Choudhary <vishal.choudhary@nirmata.com> * 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 <vishal.choudhary@nirmata.com> * fix: there is another Signed-off-by: Vishal Choudhary <vishal.choudhary@nirmata.com> * fix: update policy Signed-off-by: Vishal Choudhary <vishal.choudhary@nirmata.com> * nit Signed-off-by: Vishal Choudhary <vishal.choudhary@nirmata.com> * feat: add logs Signed-off-by: Vishal Choudhary <vishal.choudhary@nirmata.com> * Update pkg/webhooks/resource/mutation/mutation.go Signed-off-by: shuting <shuting@nirmata.com> --------- Signed-off-by: Vishal Choudhary <vishal.choudhary@nirmata.com> Signed-off-by: shuting <shuting@nirmata.com> Co-authored-by: shuting <shuting@nirmata.com> Co-authored-by: shuting <shutting06@gmail.com>

vishal-chdhry added 2 commits November 19, 2023 14:19

fix: only block mutation when failurePolicy is set

69136e5

to fail Signed-off-by: Vishal Choudhary <vishal.choudhary@nirmata.com>

feat: kuttl test

7e74e78

Signed-off-by: Vishal Choudhary <vishal.choudhary@nirmata.com>

vishal-chdhry added the milestone 1.11.1 label Nov 19, 2023

vishal-chdhry added this to the Kyverno Release 1.11.1 milestone Nov 19, 2023

vishal-chdhry requested review from chipzoller, eddycharly, realshuting, MariamFahmy98 and MarcelMue as code owners November 19, 2023 09:02

vishal-chdhry added 2 commits November 19, 2023 14:51

fix: add else check

74001b8

Signed-off-by: Vishal Choudhary <vishal.choudhary@nirmata.com>

fix: update defaulting ns label policy's failure policy to be fail

3b7bd0d

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 <vishal.choudhary@nirmata.com>

vishal-chdhry and others added 4 commits November 19, 2023 16:45

fix: there is another

0948130

Signed-off-by: Vishal Choudhary <vishal.choudhary@nirmata.com>

fix: update policy

664122d

Signed-off-by: Vishal Choudhary <vishal.choudhary@nirmata.com>

nit

513024f

Signed-off-by: Vishal Choudhary <vishal.choudhary@nirmata.com>

Merge branch 'main' into mutation-failure-policies

1d2cd5f

realshuting reviewed Nov 20, 2023

View reviewed changes

pkg/webhooks/resource/mutation/mutation.go Show resolved Hide resolved

vishal-chdhry and others added 2 commits November 22, 2023 19:42

feat: add logs

508ee6c

Signed-off-by: Vishal Choudhary <vishal.choudhary@nirmata.com>

Merge branch 'main' into mutation-failure-policies

14ae4b7

realshuting reviewed Nov 22, 2023

View reviewed changes

pkg/webhooks/resource/mutation/mutation.go Outdated Show resolved Hide resolved

Update pkg/webhooks/resource/mutation/mutation.go

6aeca12

Signed-off-by: shuting <shuting@nirmata.com>

realshuting approved these changes Nov 22, 2023

View reviewed changes

realshuting added cherry-pick-required milestone 1.12.0 labels Nov 22, 2023

realshuting enabled auto-merge (squash) November 22, 2023 16:23

realshuting merged commit c630f17 into kyverno:main Nov 22, 2023

gcp-cherry-pick-bot bot mentioned this pull request Nov 22, 2023

fix: block mutation only when failurePolicy is set to fail (cherry-pick #8952) #8986

Merged

realshuting added the cherry-pick-completed The PR was cherry-picked (or merged) to required release branches label Nov 22, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: block mutation only when failurePolicy is set to fail #8952

fix: block mutation only when failurePolicy is set to fail #8952

Uh oh!

vishal-chdhry commented Nov 19, 2023 •

edited

Loading

Uh oh!

codecov bot commented Nov 19, 2023 •

edited

Loading

Uh oh!

vishal-chdhry commented Nov 19, 2023

Uh oh!

Uh oh!

Uh oh!

realshuting commented Nov 22, 2023

Uh oh!

Uh oh!

fix: block mutation only when failurePolicy is set to fail #8952

fix: block mutation only when failurePolicy is set to fail #8952

Uh oh!

Conversation

vishal-chdhry commented Nov 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Explanation

Related issue

Milestone of this PR

What type of PR is this

Proposed Changes

Proof Manifests

Checklist

Further Comments

Uh oh!

codecov bot commented Nov 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

vishal-chdhry commented Nov 19, 2023

Uh oh!

Uh oh!

Uh oh!

realshuting commented Nov 22, 2023

Uh oh!

Uh oh!

vishal-chdhry commented Nov 19, 2023 •

edited

Loading

codecov bot commented Nov 19, 2023 •

edited

Loading