🐛 Fix kustomize syntax during conversion to patches

[mjlshen]

This commit fixes the syntax error introduced in #3374 where pathcesStrategicMerge was replaced with patches, which now requires that every patch additionally have a "path" key when multiple patches are specified in a file.

Fixes #3425

[k8s-ci-robot]

Hi @mjlshen. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[lauchokyip]

/lgtm

[k8s-ci-robot]

@lauchokyip: changing LGTM is restricted to collaborators

In response to this:

/lgtm

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[camilamacedo86]

It is great thank you for add this fix.

Would be amazing if we could get it in the e2e tests.
See that we generate a project with webhooks and uncommenting all options here: https://github.com/kubernetes-sigs/kubebuilder/blob/master/test/e2e/v4/generate_test.go we have an e2e test using webhooks.

I think it is missing we add here: https://github.com/kubernetes-sigs/kubebuilder/blob/master/test/e2e/v4/plugin_cluster_test.go#L106-L108 a line to call make manifests and make generate and see if it does not finish with error.

WDYT? If you be enable to add in this PR could you please give a hand for we have the tests in place in a follow up? Or for we have tracked an issue to do so at least?

/Otherwise

All shows fine for me.

/approved
/ok-to-test

required to fix the testdata issue first. If possible would be great we also have the check in the e2e in place to avoid this scenario in the future.

[mjlshen]

A test for this definitely makes sense, I'll get the test in this PR as well, thanks for the feedback and pointers!

[mjlshen]

I believe I have now also added a relevant test to guard against bugs like these in the future!

[rbroggi]

Nice, I also faced this issue yesterday!

@camilamacedo86 : Should we create a ticket to followup this change with the proper updates in the kubebuilder book?

What is the adopted practice on the book updates (since it is not versioned along with kubebuilder)?

[mjlshen]

The book updates should be in this PR too (the changes underneath docs/), unless you are referring to something else - but the process is described here: https://github.com/kubernetes-sigs/kubebuilder/blob/master/CONTRIBUTING.md#how-to-contribute-to-docs

[camilamacedo86]

HI @mjlshen,

Could you please rebase this one with the master branch to get the CI fixes?
Also, please ensure that we have only 1 commit for we are able to move forward with this one.

By last would be amazing if we could add the e2e tests in this one

[camilamacedo86]

I am unable to reproduce the issue scenario, see: #3425 (comment)
Also, by looking at the kustomize docs the current code implementation shows right: https://kubectl.docs.kubernetes.io/references/kustomize/kustomization/patches/#patch-using-inline-strategic-merge

By last, since I am unable to reproduce, we test this config in the e2e tests where we ensure that all works fine and by looking in the issue seems that the problem is that not all values that should be uncommented were not then, I do not think that we should move with this one.

So, /hold

Let's discuss this scenario in the issue.
Thank you a lot for your collaboration.

[camilamacedo86]

Hi mjlshen,

Can we add here the e2e test changes as well so that we can ensure that the same error is no longer faced with the changes performed?

[mjlshen]

I've moved the e2e test fixes into this PR - thank you a lot for your collaboration as well, working on this PR has been a great experience!

[camilamacedo86]

/test pull-kubebuilder-e2e-k8s-1-26-0

[camilamacedo86]

/hold cancel

It has a test either to show that it fixes the issue

/approved
/lgtm

Thank you a lot for your contribution 🥇

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: camilamacedo86, lauchokyip, mjlshen

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:

• OWNERS [camilamacedo86]

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