fix: use of kustomize deprecated features #52
Conversation
cert-manager-prow
bot
added
the
dco-signoff: no
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
cert-manager-prow
bot
added
the
size/L
Signed-off-by: Erik Godding Boye <[email protected]>
erikgb
force-pushed
the
fix-kustomize-deprecated
branch
from
533d963 to
22a4174
Compare
cert-manager-prow
bot
added
dco-signoff: yes
| - manager_auth_proxy_patch.yaml | ||
|
|
||
|
|
||
| - path: manager_auth_proxy_patch.yaml |
There was a problem hiding this comment.
I expected this patch to change or be deleted, since it's what adds the kube-rbac-proxy sidecar:
Did you forget to commit something?
There was a problem hiding this comment.
I could extend the PR to include migration away from kube-rbac-proxy, but that will increase the number of changes significantly. But can still make sense. WDYT?
There was a problem hiding this comment.
Oh, I misunderstood your PR description.
But since I noticed some use of deprecated kustomize features, I made some additional improvements.
I thought you meant that you'd removed kube-rbac-proxy and additionally replaced some deprecated kustomize features.
Now I understand that this PR is ground work for a follow up PR which removes kube-rbac-proxy.
There was a problem hiding this comment.
Thanks @erikgb
Now I understand that this is ground work for the replacement of kube-rbac-proxy in a followup PR.
I could find the kubebuilder PRs for most of these changes, but some of the changes do not seem to be found upstream.
Please write a little more about how you generated this PR and highlight which parts are mechanical and which parts are hand crafted.
There was a problem hiding this comment.
Looks like this new file was renamed from the original certificate.yaml in kubernetes-sigs/kubebuilder#4400 👍
There was a problem hiding this comment.
Reflects the upstream changes which were introduced in:
| @@ -12,6 +12,3 @@ namespace: | |||
| group: apiextensions.k8s.io | |||
| path: spec/conversion/webhookClientConfig/service/namespace | |||
| create: false | |||
|
|
|||
| varReference: | |||
| - path: metadata/annotations | |||
There was a problem hiding this comment.
Why is this removed? It is still in the kubebuilder skaffolding AFAICS:
There was a problem hiding this comment.
Why did these filenames change? Is it because the CRD name was changed in #42 and we forgot to re-run the kubebuilder crd generator tool?
If so, it really annoys me that issuer-lib prevents us from using the CRD name issuer and clusterissuer.
Basically everything was had-crafted, but based on the content in https://github.com/kubernetes-sigs/kubebuilder/tree/master/testdata/project-v4. Do you want me to perform a full rescaffolding? Would that be easier to review? |
Maybe that would be cleaner. The PR description says:
I vote to close that PR for the reasons given by Ash in his review.
Update the PR description to explain how these changes makes it easier to accomplish that goal. I really just want to understand the reasoning for this particular set of changes. |
This PR's main motivation is to make it easier to replace the long-deprecated
kube-rbac-proxyimage with similar functionality now available in controller-runtime. But since I noticed some use of deprecated kustomize features, I made some additional improvements.Some of the proposed changes are based on the latest updates in kubebuilder (go/v4). I don't want to attempt migrating to kubebuilder go/v4 before #49 is merged (or closed) to avoid interference between PRs.