Remove use of deprecated patchesStrategicMerge

lentzi90 commented 1 year ago

What broke? What's expected?

Most of the deprecated kustomize syntax has already been removed, but I came across one instance of patchesStrategicMerge still lingering: https://github.com/kubernetes-sigs/kubebuilder/blob/68abac1dfb5550f7159dec4ce600cd9b6db925bb/pkg/plugins/common/kustomize/v2/scaffolds/internal/templates/config/kdefault/kustomization.go#L75 This should be changed to patches.

Reproducing this issue

mkdir -p ~/projects/guestbook
cd ~/projects/guestbook
kubebuilder init --domain my.domain --repo my.domain/guestbook
kubebuilder create api --group webapp --version v1 --kind Guestbook --resource --controller

Now check config/default/kustomization.yaml:

grep "patchesStrategicMerge" config/default/kustomization.yaml

KubeBuilder (CLI) Version

Version: main.version{KubeBuilderVersion:"3.11.1", KubernetesVendor:"1.27.1", GitCommit:"1dc8ed95f7cc55fef3151f749d3d541bec3423c9", BuildDate:"2023-07-03T13:10:56Z", GoOs:"linux", GoArch:"amd64"}

PROJECT version

3

Plugin versions

layout:
- go.kubebuilder.io/v4

Other versions

No response

Extra Labels

/kind deprecation

typeid commented 1 year ago

There seem to be a few more occurrences:

./docs/book/src/component-config-tutorial/api-changes.md:121:Update the file `default/kustomization.yaml` by adding under the [`patchesStrategicMerge:` key](https://kubectl.docs.kubernetes.io/references/kustomize/builtins/#_patchesstrategicmerge_) the following patch:
./docs/book/src/component-config-tutorial/api-changes.md:124:patchesStrategicMerge:
./docs/book/src/component-config-tutorial/testdata/project/config/default/kustomization.yaml:29:patchesStrategicMerge:
./docs/book/src/cronjob-tutorial/testdata/project/config/default/kustomization.yaml:29:patchesStrategicMerge:
./docs/book/src/multiversion-tutorial/testdata/project/config/crd/kustomization.yaml:8:patchesStrategicMerge:
./docs/book/src/multiversion-tutorial/testdata/project/config/default/kustomization.yaml:29:patchesStrategicMerge:
./docs/book/book/cronjob-tutorial/running-webhook.html:230:patchesStrategicMerge:
./docs/book/book/cronjob-tutorial/testdata/project/config/default/kustomization.yaml:29:patchesStrategicMerge:
./docs/book/book/multiversion-tutorial/testdata/project/config/crd/kustomization.yaml:8:patchesStrategicMerge:
./docs/book/book/multiversion-tutorial/testdata/project/config/default/kustomization.yaml:29:patchesStrategicMerge:
./docs/book/book/component-config-tutorial/api-changes.html:265:<p>Update the file <code>default/kustomization.yaml</code> by adding under the <a href="https://kubectl.docs.kubernetes.io/references/kustomize/builtins/#_patchesstrategicmerge_"><code>patchesStrategicMerge:</code> key</a> the following patch:</p>
./docs/book/book/component-config-tutorial/api-changes.html:266:<pre><code class="language-yaml">patchesStrategicMerge:
./docs/book/book/component-config-tutorial/testdata/project/config/default/kustomization.yaml:29:patchesStrategicMerge:
./docs/book/book/print.html:2530:patchesStrategicMerge:
./docs/book/book/print.html:4291:<p>Update the file <code>default/kustomization.yaml</code> by adding under the <a href="https://kubectl.docs.kubernetes.io/references/kustomize/builtins/#_patchesstrategicmerge_"><code>patchesStrategicMerge:</code> key</a> the following patch:</p>
./docs/book/book/print.html:4292:<pre><code class="language-yaml">patchesStrategicMerge:
./pkg/plugins/common/kustomize/v1/scaffolds/internal/templates/config/crd/kustomization.go:113:patchesStrategicMerge:
./pkg/plugins/common/kustomize/v1/scaffolds/internal/templates/config/kdefault/kustomization.go:73:patchesStrategicMerge:
./pkg/plugins/common/kustomize/v2/scaffolds/internal/templates/config/kdefault/kustomization.go:75:patchesStrategicMerge:
./pkg/plugins/golang/v2/scaffolds/internal/templates/config/crd/kustomization.go:113:patchesStrategicMerge:
./pkg/plugins/golang/v2/scaffolds/internal/templates/config/kdefault/kustomization.go:83:patchesStrategicMerge:
./testdata/project-v2/config/default/kustomization.yaml:27:patchesStrategicMerge:
./testdata/project-v2/config/crd/kustomization.yaml:10:patchesStrategicMerge:
./testdata/project-v3/config/crd/kustomization.yaml:10:patchesStrategicMerge:
./testdata/project-v3/config/default/kustomization.yaml:27:patchesStrategicMerge:
./testdata/project-v4-declarative-v1/config/default/kustomization.yaml:29:patchesStrategicMerge:
./testdata/project-v4-multigroup/config/default/kustomization.yaml:29:patchesStrategicMerge:
./testdata/project-v4-with-deploy-image/config/default/kustomization.yaml:29:patchesStrategicMerge:
./testdata/project-v4-with-grafana/config/default/kustomization.yaml:29:patchesStrategicMerge:
./testdata/project-v4/config/default/kustomization.yaml:29:patchesStrategicMerge:

We could probably look into removing all of those.

camilamacedo86 commented 1 year ago

We should only remove from kustomize/v2 plugin and not v1 since, the v1 is deprecated. Therefore, the projects scaffolds within go/v2 and go/3 layout, which either are deprecated, will not be changed.

So, the change must be done in: ./pkg/plugins/common/kustomize/v2/scaffolds/internal/templates/config/kdefault/kustomization.go:75:patchesStrategicMerge:

And then, we run make generate to update the docs and samples accordingly.

Would you like to contribute with this one @lentzi90?

ashutosh887 commented 1 year ago

I'd like to pick this if not @lentzi90

cc: @camilamacedo86

Sajiyah-Salat commented 1 year ago

/assign

ashutosh887 commented 1 year ago

/assign

lentzi90 commented 1 year ago

Feel free to take it! After I created the issue I have found an issue in kustomize, which is probably the reason this wasn't fixed before. See https://github.com/kubernetes-sigs/kustomize/issues/5049. It seems to be moving along nicely, so perhaps we could just wait for it to be resolved to avoid unnecessary workarounds?

The gist is that pathesStrategicMerge can be easily changed into patches like this:

# Before
pathesStrategicMerge:
- patch_file.yaml

# After
patches:
- path: patch_file.yaml

However, there is one thing that patches cannot do (yet). It does not accept a patch file with multiple patches in it. This is the case we have for the webhookcainjection_patch.yaml.

It is possible to work around by splitting the patch into multiple files, but as mentioned, I think we could just wait for the issue to be fixed in kustomize before we get rid of patchesStrategicMerge.

Sajiyah-Salat commented 1 year ago

They are working on patches and it will accept multiple patches in future. Should we just wait for now? cc: @camilamacedo86 @varshaprasad96

lentzi90 commented 1 year ago

The issue in Kustomize has been fixed :tada: I guess now we need to wait for a release and bump the version somewhere in kubebuilder to make it work

Sajiyah-Salat commented 1 year ago

After the release we just need to run kustomize edit fix to fix the issue as described in description of the above issue mentioned. right?

lentzi90 commented 1 year ago

I think it may be better to do it manually. At least I had some issues with kustomize edit fix before. It can do strange things :sweat_smile: It should be pretty easy to do manually also, all that is needed is this:


# Before
pathesStrategicMerge:
- patch_file.yaml

# After
patches:
- path: patch_file.yaml

Sajiyah-Salat commented 1 year ago

@camilamacedo86 okay so we can do it now as well. right? I can open a pr then. is it fine?

lentzi90 commented 1 year ago

I'm not sure how kubebuilder handles kustomize. Maybe @camilamacedo86 knows?

camilamacedo86 commented 11 months ago

Hi @Sajiyah-Salat

@camilamacedo86 okay so we can do it now as well. right? I can open a pr then. is it fine?

Yes, you can open PRs to solve any issue and as please you. Thank you 🥇

Hi @lentzi90 ,

I'm not sure how kubebuilder handles kustomize. Maybe @camilamacedo86 knows?

The changes should be done in the plugin kustomize/v2 only : kustomize/v2
kustomize plugin is the plugin responsible for scaffolding everything under the config dir.
When we run kubebuilder init|edit|create we are calling by default the init|edit|create implementation of the kustomize/v2 and that of the golang/v4.
Look at the templates, we must make the changes on the templates.
Therefore, if we use kustomize edit fix we need to compare with a version of the same project without the fix, check the changes (e.g. compare the same project under the sample project-v4 with the fix and without the fix) and then update the templates, see: kustomize/v2 templates
After updating the templates run make generate to get the samples under testdata (testdata) and docs with your change.

I hope that it answer your questions Thank you for your collaboration

kubernetes-sigs / kubebuilder