-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
🐛 (kustomize/v2, go/v4): Fix ca injection for conversion webhooks #4282
Open
camilamacedo86
wants to merge
1
commit into
kubernetes-sigs:master
Choose a base branch
from
camilamacedo86:fix-ca-injection-conversion-webhooks
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
🐛 (kustomize/v2, go/v4): Fix ca injection for conversion webhooks #4282
camilamacedo86
wants to merge
1
commit into
kubernetes-sigs:master
from
camilamacedo86:fix-ca-injection-conversion-webhooks
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
k8s-ci-robot
added
cncf-cla: yes
Indicates the PR's author has signed the CNCF CLA.
approved
Indicates a PR has been approved by an approver from all required OWNERS files.
labels
Nov 2, 2024
k8s-ci-robot
added
the
size/XXL
Denotes a PR that changes 1000+ lines, ignoring generated files.
label
Nov 2, 2024
camilamacedo86
force-pushed
the
fix-ca-injection-conversion-webhooks
branch
from
November 2, 2024 12:56
1b5cddf
to
41e2c58
Compare
camilamacedo86
changed the title
(Blocked by #4280) WIP - Fix ca injection conversion webhooks
(Blocked by #4280) WIP - 🐛 (kustomize/v2, go/v4): Fix ca injection for conversion webhooks
Nov 2, 2024
camilamacedo86
force-pushed
the
fix-ca-injection-conversion-webhooks
branch
2 times, most recently
from
November 2, 2024 17:40
771d28b
to
fd37dfc
Compare
camilamacedo86
changed the title
(Blocked by #4280) WIP - 🐛 (kustomize/v2, go/v4): Fix ca injection for conversion webhooks
🐛 (kustomize/v2, go/v4): Fix ca injection for conversion webhooks
Nov 2, 2024
camilamacedo86
changed the title
🐛 (kustomize/v2, go/v4): Fix ca injection for conversion webhooks
🐛 WIP: (kustomize/v2, go/v4): Fix ca injection for conversion webhooks
Nov 2, 2024
camilamacedo86
force-pushed
the
fix-ca-injection-conversion-webhooks
branch
from
November 2, 2024 18:15
fd37dfc
to
093c664
Compare
camilamacedo86
changed the title
🐛 WIP: (kustomize/v2, go/v4): Fix ca injection for conversion webhooks
🐛 (kustomize/v2, go/v4): Fix ca injection for conversion webhooks
Nov 2, 2024
camilamacedo86
added
release-blocker
priority/backlog
Higher priority than priority/awaiting-more-evidence.
priority/important-soon
Must be staffed and worked on either currently, or very soon, ideally in time for the next release.
and removed
priority/backlog
Higher priority than priority/awaiting-more-evidence.
labels
Nov 2, 2024
grzesuav
approved these changes
Nov 2, 2024
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: camilamacedo86, grzesuav 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:
Approvers can indicate their approval by writing |
camilamacedo86
force-pushed
the
fix-ca-injection-conversion-webhooks
branch
4 times, most recently
from
November 6, 2024 07:40
6cc8aba
to
eb167ea
Compare
camilamacedo86
changed the title
🐛 (kustomize/v2, go/v4): Fix ca injection for conversion webhooks
🐛 (WIP) (kustomize/v2, go/v4): Fix ca injection for conversion webhooks
Nov 9, 2024
camilamacedo86
changed the title
🐛 (WIP) (kustomize/v2, go/v4): Fix ca injection for conversion webhooks
🐛 (kustomize/v2, go/v4): Fix ca injection for conversion webhooks
Nov 9, 2024
camilamacedo86
force-pushed
the
fix-ca-injection-conversion-webhooks
branch
3 times, most recently
from
November 9, 2024 20:19
5a198df
to
d5055b5
Compare
camilamacedo86
changed the title
🐛 (kustomize/v2, go/v4): Fix ca injection for conversion webhooks
🐛 (WIP - TODO FIX AFTER REBASES )(kustomize/v2, go/v4): Fix ca injection for conversion webhooks
Nov 9, 2024
camilamacedo86
changed the title
🐛 (WIP - TODO FIX AFTER REBASES )(kustomize/v2, go/v4): Fix ca injection for conversion webhooks
(WIP - TODO FIX AFTER REBASES ) 🐛 (kustomize/v2, go/v4): Fix ca injection for conversion webhooks
Nov 9, 2024
k8s-ci-robot
added
the
do-not-merge/work-in-progress
Indicates that a PR should not merge because it is a work in progress.
label
Nov 9, 2024
camilamacedo86
changed the title
(WIP - TODO FIX AFTER REBASES ) 🐛 (kustomize/v2, go/v4): Fix ca injection for conversion webhooks
(WIP - fix rebase changes ) 🐛 (kustomize/v2, go/v4): Fix ca injection for conversion webhooks
Nov 9, 2024
camilamacedo86
force-pushed
the
fix-ca-injection-conversion-webhooks
branch
from
November 10, 2024 04:15
d5055b5
to
2d60e80
Compare
camilamacedo86
changed the title
(WIP - fix rebase changes ) 🐛 (kustomize/v2, go/v4): Fix ca injection for conversion webhooks
🐛 (kustomize/v2, go/v4): Fix ca injection for conversion webhooks
Nov 10, 2024
k8s-ci-robot
removed
the
do-not-merge/work-in-progress
Indicates that a PR should not merge because it is a work in progress.
label
Nov 10, 2024
The CA injection patch has **not** worked for `go/v4` and `kustomize/v2` (release `3.5.0`) due to the need to replace `vars` with `replacements`, as `vars` are no longer supported in the latest major versions of Kustomize. However, since webhook `--conversion` was an incomplete feature until the upcoming Kubebuilder future release `v4.4.0` (where [PR kubernetes-sigs#4254](kubernetes-sigs#4254) is expected to be merged), users likely didn’t encounter this issue or addressed it manually by fixing the scaffold. **Note:** This change only affects projects that require a **conversion webhook**.
camilamacedo86
force-pushed
the
fix-ca-injection-conversion-webhooks
branch
from
November 10, 2024 07:43
2d60e80
to
c8ab90e
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
approved
Indicates a PR has been approved by an approver from all required OWNERS files.
cncf-cla: yes
Indicates the PR's author has signed the CNCF CLA.
priority/important-soon
Must be staffed and worked on either currently, or very soon, ideally in time for the next release.
release-blocker
size/XXL
Denotes a PR that changes 1000+ lines, ignoring generated files.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PR Description
The CA injection patch has not worked for
go/v4
andkustomize/v2
(release3.5.0
) due to the need to replacevars
withreplacements
, asvars
are no longer supported in the latest major versions of Kustomize.However, since webhook
--conversion
was an incomplete feature until the upcoming Kubebuilder future releasev4.4.0
(where PR #4254 is expected to be merged), users likely didn’t encounter this issue or addressed it manually by fixing the scaffold.Note: This change only affects projects that require a conversion webhook.
To better understand the issue and context please see: #4285
Closes: #4285
To manually fix projects scaffolded with previous versions, users can:
Remove the CERTMANAGER Section from
config/crd/kustomization.yaml
:Delete the
CERTMANAGER
section to avoid unintended CA injection patches for CRDs. Ensure the following lines are removed or commented out:Add CA Injection Configuration in
config/default/kustomization.yaml
:In
config/default/kustomization.yaml
, add the following code under[CERTMANAGER]
for CA injection:Important: Ensure that these scaffold markers are included:
+kubebuilder:scaffold:crdkustomizecainjectionns
+kubebuilder:scaffold:crdkustomizecainjectioname
Ensure Only Conversion Webhook Patches in config/crd/patches:
The
config/crd/patches
directory and the corresponding entries inconfig/crd/kustomization.yaml
should onlycontain files for conversion webhooks. Previously, a bug (🐛 (kustomize/v2, go/v4): Fix incorrect generation of manifests under config/crd/patches. Previously, the /convert service patch was being generated for all webhooks instead of only for those with --conversion enabled. #4280) caused the patch file to be generated for any webhook,
but only patches for webhooks created with the
--conversion
option should be included.For further guidance, you can refer to examples in the
testdata/
directory in the Kubebuilder repository.