-
Notifications
You must be signed in to change notification settings - Fork 85
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
apis: Kustomize target selector must be a pointer #517
Conversation
@@ -93,7 +93,7 @@ type Patch struct { | |||
|
|||
// Target points to the resources that the patch document should be applied to. | |||
// +optional | |||
Target Selector `json:"target,omitempty"` | |||
Target *Selector `json:"target,omitempty"` |
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.
This is an API breaking change, right?
Just curious if we'll be providing warnings about it in KC and HC release notes for this.
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.
It is not breaking except for consumption from the Go-side as the YAML behavior is leading for the APIs.
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.
@darkowlzz this will be released as apis/kustomize v1 as for KC and HC release notes there is no need for that, this has no impact on users expect for the bug fix.
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.
okay. I was just thinking of anyone using the Go API that will be affected by this.
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.
Sure they will, Kustomization v1 API will come with other breaking changes besides this. I'll make sure this is mentioned in the API changelog.
Signed-off-by: Stefan Prodan <stefan.prodan@gmail.com>
a0a4914
to
5ea22ed
Compare
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.
LGTM!
Setting the
Target
to be a pointer as in the upstream API. This fixes a bug in helm-controller where patches were applied to all objects asTarget
comes with empty fields https://github.com/fluxcd/helm-controller/blob/88a3beabd896be448e9c966b1f9aace6ec02c6c5/internal/runner/post_renderer_kustomize.go#L81