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

feat: add fieldoverrider #5591

Merged

Conversation

sophiefeifeifeiya
Copy link
Contributor

What type of PR is this?

/kind feature

What this PR does / why we need it:

The proposal introduces a new feature that allows users to partially override values inside JSON and YAML fields. This is achieved using JSON patch operation. This design enables users to to override the values within JSON/YAML fields partially, rather than replacing a whole JSON/YAML fields with PlaintextOverrider. Currently, PlaintextOverrider applies JSON patch operations to whole fields, rather than specific values within fields, making it unsuitable for cases where users need to override individual values within those fields.

Which issue(s) this PR fixes:
Fixes #5330

Special notes for your reviewer:
Proposal: #5449
Related PR: #5329, #5581

Does this PR introduce a user-facing change?:

Introduced `FieldOverrider`  for overriding values in JSON and YAML.

@karmada-bot karmada-bot added the kind/feature Categorizes issue or PR as related to a new feature. label Sep 23, 2024
@karmada-bot karmada-bot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Sep 23, 2024
@codecov-commenter
Copy link

codecov-commenter commented Sep 23, 2024

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 41.90476% with 61 lines in your changes missing coverage. Please review.

Project coverage is 35.25%. Comparing base (f656d9a) to head (6bf6f30).

Files with missing lines Patch % Lines
pkg/util/overridemanager/overridemanager.go 55.00% 22 Missing and 14 partials ⚠️
pkg/util/validation/validation.go 0.00% 24 Missing and 1 partial ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5591      +/-   ##
==========================================
+ Coverage   35.23%   35.25%   +0.01%     
==========================================
  Files         645      645              
  Lines       44891    44994     +103     
==========================================
+ Hits        15817    15861      +44     
- Misses      27841    27884      +43     
- Partials     1233     1249      +16     
Flag Coverage Δ
unittests 35.25% <41.90%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

@karmada-bot karmada-bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Sep 23, 2024
@chaunceyjiang
Copy link
Member

/assign

Copy link

@Patrick0308 Patrick0308 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@sophiefeifeifeiya sophiefeifeifeiya force-pushed the feat-fieldoverrider branch 2 times, most recently from cd7f9eb to 283c732 Compare September 24, 2024 21:19
@RainbowMango
Copy link
Member

@sophiefeifeifeiya Please rebase this PR to resolve the conflict. I just merged the API part at #5581.

@karmada-bot karmada-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Sep 25, 2024
@karmada-bot karmada-bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Sep 25, 2024
@sophiefeifeifeiya sophiefeifeifeiya force-pushed the feat-fieldoverrider branch 2 times, most recently from 1cf5323 to 6beb11c Compare September 25, 2024 08:35
@sophiefeifeifeiya sophiefeifeifeiya force-pushed the feat-fieldoverrider branch 7 times, most recently from ccc2364 to c2486b6 Compare September 26, 2024 11:27
@sophiefeifeifeiya sophiefeifeifeiya force-pushed the feat-fieldoverrider branch 2 times, most recently from d79631c to 6bf7fbc Compare September 26, 2024 22:41
@chaunceyjiang
Copy link
Member

/cc @RainbowMango @XiShanYongYe-Chang PTAL.

@sophiefeifeifeiya sophiefeifeifeiya force-pushed the feat-fieldoverrider branch 2 times, most recently from ff126f5 to 22e4db5 Compare September 30, 2024 13:51
Copy link
Member

@RainbowMango RainbowMango left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks awesome! Thank you @sophiefeifeifeiya for your excellent job!

test/e2e/overridepolicy_test.go Show resolved Hide resolved
Signed-off-by: sophie <yl5357@columbia.edu>
Copy link
Member

@RainbowMango RainbowMango left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm
/approve
Thanks.

@karmada-bot karmada-bot added the lgtm Indicates that a PR is ready to be merged. label Oct 9, 2024
@karmada-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Patrick0308, RainbowMango

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@karmada-bot karmada-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 9, 2024
@karmada-bot karmada-bot merged commit 2a78e2b into karmada-io:master Oct 9, 2024
12 checks passed
@RainbowMango RainbowMango added this to the v1.12 milestone Oct 9, 2024
@sophiefeifeifeiya sophiefeifeifeiya deleted the feat-fieldoverrider branch October 12, 2024 13:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. kind/feature Categorizes issue or PR as related to a new feature. lgtm Indicates that a PR is ready to be merged. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support json or yaml plaintext field overriding
6 participants