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

Resource Modifier should support json merge patch and strategic merge patch #6728

Closed
27149chen opened this issue Aug 31, 2023 · 5 comments · Fixed by #6797 or #6917
Closed

Resource Modifier should support json merge patch and strategic merge patch #6728

27149chen opened this issue Aug 31, 2023 · 5 comments · Fixed by #6797 or #6917
Assignees
Milestone

Comments

@27149chen
Copy link
Contributor

Describe the problem/challenge you have
Resource Modifier uses json patch to patch object, but json patch is not easy to use and has limitations.
For example:

  1. if I want to remove an annotation not matter it exists or not, a json merge patch is better ({"metadata":{"annotations":{"foo":null}}})
  2. if I want to update a container image by container name, a strategic merge patch should be used

Describe the solution you'd like
support json merge patch and strategic merge patch in Resource Modifier

Anything else you would like to add:

Environment:

  • Velero version (use velero version):
  • Kubernetes version (use kubectl version):
  • Kubernetes installer & version:
  • Cloud provider or hardware configuration:
  • OS (e.g. from /etc/os-release):

Vote on this issue!

This is an invitation to the Velero community to vote on issues, you can see the project's top voted issues listed here.
Use the "reaction smiley face" up to the right of this comment to vote.

  • 👍 for "The project would be better with this feature added"
  • 👎 for "This feature will not enhance the project in a meaningful way"
@anshulahuja98
Copy link
Collaborator

#6344
#6344 (comment) what are your thoughts on leveraging jq instead to give such capabilities, while possibly keeping the jsonpatch setup intact.

CC @qiuming-best

@anshulahuja98 anshulahuja98 added the 1.13-candidate issue/pr that should be considered to target v1.13 minor release label Aug 31, 2023
@27149chen
Copy link
Contributor Author

I think they are different things, jq can not cover the capabilities which json merge patch or strategic merge patch can provide

@27149chen
Copy link
Contributor Author

@reasonerjt I want to implement this feature, can you assign it to me?

@anshulahuja98
Copy link
Collaborator

@27149chen I think it would be great if you could pick impl. I can assign it to you.
Can we meanwhile further close on the approach over slack?
https://kubernetes.slack.com/archives/C021GPR1L3S/p1693808146936179

Had put further points on this, I am trying to see if through whatever enhancements we do to resource modifiers, we are also able to support scenarios in #6344

@ywk253100
Copy link
Contributor

Reopen it as the implementation isn't merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment