Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: deepcopy patched resource in foreach mutate #10252

Merged
merged 6 commits into from
May 20, 2024

Conversation

vishal-chdhry
Copy link
Member

@vishal-chdhry vishal-chdhry commented May 17, 2024

Explanation

After commit 46f02a8, when the order is set to Descending in foreach mutate, the order of items in the trigger resource also gets reverted, causing the wrong item to get patched.

This PR:

  1. copies and reverses elements instead of swapping.
  2. deepcopies the resource at the start of foreach

Related issue

Closes #10224
Closes #10199

Milestone of this PR

Documentation (required for features)

My PR contains new or altered behavior to Kyverno.

What type of PR is this

Proposed Changes

Proof Manifests

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 .
  • My PR contains new or altered behavior to Kyverno and
    • CLI support should be added and my PR doesn't contain that functionality.

Further Comments

…ents

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

codecov bot commented May 17, 2024

Codecov Report

Attention: Patch coverage is 71.42857% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 10.17%. Comparing base (1302004) to head (44adfa3).

Files Patch % Lines
pkg/engine/handlers/mutation/common.go 0.00% 2 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main   #10252   +/-   ##
=======================================
  Coverage   10.16%   10.17%           
=======================================
  Files        1030     1030           
  Lines       91833    91836    +3     
=======================================
+ Hits         9338     9343    +5     
+ Misses      81473    81471    -2     
  Partials     1022     1022           

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

@vishal-chdhry vishal-chdhry changed the title fix: deepcopy patched resource to avoid indirect reversal of its elements fix: deepcopy patched resource in foreach mutate May 17, 2024
Signed-off-by: Jim Bugwadia <jim@nirmata.com>
@JimBugwadia JimBugwadia added the release-critical Critical issues which MUST be addressed in the specified milestone. These cannot get bumped. label May 18, 2024
Signed-off-by: Jim Bugwadia <jim@nirmata.com>
@realshuting realshuting merged commit 3af0e46 into kyverno:main May 20, 2024
249 of 250 checks passed
@realshuting
Copy link
Member

/cherry-pick release-1.12

gcp-cherry-pick-bot bot pushed a commit that referenced this pull request May 20, 2024
* fix: deepcopy patched resource to avoid indirect reversal of its elements

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

* fix: copy elements while reversing

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

* fix: copy resources inside foreach

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

* add test

Signed-off-by: Jim Bugwadia <jim@nirmata.com>

* add test

Signed-off-by: Jim Bugwadia <jim@nirmata.com>

---------

Signed-off-by: Vishal Choudhary <vishal.choudhary@nirmata.com>
Signed-off-by: Jim Bugwadia <jim@nirmata.com>
Co-authored-by: Jim Bugwadia <jim@nirmata.com>
@realshuting realshuting added the cherry-pick-completed The PR was cherry-picked (or merged) to required release branches label May 20, 2024
realshuting pushed a commit that referenced this pull request May 20, 2024
* fix: deepcopy patched resource to avoid indirect reversal of its elements



* fix: copy elements while reversing



* fix: copy resources inside foreach



* add test



* add test



---------

Signed-off-by: Vishal Choudhary <vishal.choudhary@nirmata.com>
Signed-off-by: Jim Bugwadia <jim@nirmata.com>
Co-authored-by: Vishal Choudhary <vishal.choudhary@nirmata.com>
Co-authored-by: Jim Bugwadia <jim@nirmata.com>
anushkamittal2001 pushed a commit to nirmata/kyverno that referenced this pull request May 24, 2024
…erno#10258)

* fix: deepcopy patched resource to avoid indirect reversal of its elements



* fix: copy elements while reversing



* fix: copy resources inside foreach



* add test



* add test



---------

Signed-off-by: Vishal Choudhary <vishal.choudhary@nirmata.com>
Signed-off-by: Jim Bugwadia <jim@nirmata.com>
Co-authored-by: Vishal Choudhary <vishal.choudhary@nirmata.com>
Co-authored-by: Jim Bugwadia <jim@nirmata.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherry-pick-completed The PR was cherry-picked (or merged) to required release branches cherry-pick-required milestone 1.12.2 milestone 1.13.0 release-critical Critical issues which MUST be addressed in the specified milestone. These cannot get bumped.
Projects
None yet
3 participants