Skip to content
This repository has been archived by the owner on Aug 12, 2024. It is now read-only.

Fix: Overriding user defined labels #666

Conversation

varshaprasad96
Copy link
Member

This PR fixes an issue where helm's postrenderer
used to override the user defined object labels instead of merging them.

Issue:
Previously, if any manifest in the bundle had labels provided in them, they were overwritten when bundle deployment was created.

@varshaprasad96 varshaprasad96 requested a review from a team as a code owner August 1, 2023 16:58
@ncdc
Copy link
Member

ncdc commented Aug 1, 2023

Could you please add a test that fails without the fix, and passes with it?

@codecov
Copy link

codecov bot commented Aug 1, 2023

Codecov Report

Merging #666 (7802031) into main (2411f47) will decrease coverage by 10.70%.
Report is 2 commits behind head on main.
The diff coverage is 100.00%.

❗ Current head 7802031 differs from pull request most recent head f173729. Consider uploading reports for the commit f173729 to get more accurate results

@@             Coverage Diff             @@
##             main     #666       +/-   ##
===========================================
- Coverage   31.94%   21.24%   -10.70%     
===========================================
  Files          10       12        +2     
  Lines         479      772      +293     
===========================================
+ Hits          153      164       +11     
- Misses        304      583      +279     
- Partials       22       25        +3     
Files Changed Coverage Δ
...l/controllers/bundledeployment/bundledeployment.go 3.76% <100.00%> (ø)

... and 1 file with indirect coverage changes

@varshaprasad96
Copy link
Member Author

varshaprasad96 commented Aug 1, 2023

Could you please add a test that fails without the fix, and passes with it?

Done. This package did not have tests for bundleDeployment controller, I've added one just to cover this case for now.
Will create issue to add more add coverage to the controller.

@varshaprasad96 varshaprasad96 force-pushed the sre/cloud-ingress branch 2 times, most recently from 3aa0dc6 to 7802031 Compare August 1, 2023 19:11
This PR fixes an issue where helm's postrenderer
used to override the user defined object labels instead
of merging them.

Signed-off-by: Varsha Prasad Narsing <[email protected]>
@varshaprasad96
Copy link
Member Author

varshaprasad96 commented Aug 1, 2023

Looks like manifests-diff maybe failing because my branch name, as it has a slash, where its trying to copy rukpak.yaml and is not able to find the path. Will close this and re-open the PR with a different branch name :/

@varshaprasad96 varshaprasad96 deleted the sre/cloud-ingress branch August 1, 2023 21:18
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants