-
Notifications
You must be signed in to change notification settings - Fork 115
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 diff logic for CRD kinds with the same name as a built-in #1779
Conversation
The diff logic was incorrectly treating CRD kinds with a name matching a built-in kind as known (e.g., Service). In these cases, the patch behavior was erroneously set to strategic merge rather than JSON merge.
@lblackstone thanks for the quick fix! |
Does the PR have any schema changes?Looking good! No breaking changes found. |
2 similar comments
Does the PR have any schema changes?Looking good! No breaking changes found. |
Does the PR have any schema changes?Looking good! No breaking changes found. |
Does the PR have any schema changes?Looking good! No breaking changes found. |
"storage.k8s.io/v1", | ||
"storage.k8s.io/v1alpha1", | ||
"storage.k8s.io/v1beta1", | ||
"v1", // alias for "core/v1" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a way to know that we are not missing any other aliases?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"core/v1" is the only special case I'm aware of, and is a relic from early versions of k8s. Anything else should include a group and a version.
Proposed changes
The diff logic was incorrectly treating CRD kinds with a name
matching a built-in kind as known (e.g., Service). In these cases,
the patch behavior was erroneously set to strategic merge rather
than JSON merge.
Related issues (optional)
Fix #1774