Support kustomize "extended" patches. #2909#3663
Support kustomize "extended" patches. #2909#3663tejal29 merged 1 commit intoGoogleContainerTools:masterfrom
Conversation
|
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here with What to do if you already signed the CLAIndividual signers
Corporate signers
ℹ️ Googlers: Go here for more info. |
Codecov Report
|
|
@googlebot I signed it! |
|
CLAs look good, thanks! ℹ️ Googlers: Go here for more info. |
273a133 to
641c871
Compare
|
Restarted the failed window unit test. |
|
@tejal29 looks like the windows test passed this time! |
nkubala
left a comment
There was a problem hiding this comment.
shouldn't we try and stay backwards compatible with older kustomization.yamls? this will break existing users using the older format.
pkg/skaffold/deploy/kustomize.go
Outdated
| // UnmarshalYAML implements JSON unmarshalling by reading an inline yaml fragment. | ||
| func (p *patchWrapper) UnmarshalYAML(unmarshal func(interface{}) error) (err error) { | ||
| pp := &patchPath{} | ||
| _ = unmarshal(&pp) |
There was a problem hiding this comment.
should we be checking an error here?
There was a problem hiding this comment.
@nkubala, I'm relatively new to Golang so .... I didn't do the err check here because it will error if its a string and is part of the old API. Now that I'm thinking about it now maybe I could do something like (psuedo code)
err = unmarshall
if pp.path and pp.patch is nil and if err:
err2 := unmarshal_to_string()
if err and err2:
log error
Thoughts?
There was a problem hiding this comment.
i think this change is backward compatible.
if pp.Path == "" && pp.Patch == "" {
should imply
if err != nil {
to make it explicitly you could
pp := &patchPath{}
if err := unmarshal(&pp); err != nil{
var oldPathString string
if err := unmarshal(&oldPathString); err != nil {
return err
}
warnings.Printf("list of file paths deprecated: see https://github.com/kubernetes-sigs/kustomize/blob/master/docs/plugins/builtins.md#patchtransformer")
pp.Path = oldPathString
}
There was a problem hiding this comment.
Implemented "explicit".
641c871 to
8a9b4a7
Compare
Relates to in case of new feature, this should point to issue/(s) which describes the feature
Fixes #2909
Should merge before :n/a
Should merge after : n/a
Description
implement #2909: support extended kustomize patches
User facing changes
n/a
Before
n/a
After
n/a
Next PRs.
n/a
-->
Submitter Checklist
These are the criteria that every PR should meet, please check them off as you
review them:
Reviewer Notes
Release Notes