-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
馃悰 Remove unneeded caBundle value from webhook patches (V3 only) #1699
馃悰 Remove unneeded caBundle value from webhook patches (V3 only) #1699
Conversation
Welcome @matthchr! |
Hi @matthchr. 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 Once the patch is verified, the new status will be reflected by the 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. |
a94d406
to
e7474ff
Compare
This comment has been minimized.
This comment has been minimized.
e7474ff
to
5098c72
Compare
@DirectXMan12 can you weigh in when you get a chance on the question I asked above about the files that still have |
testdata/project-v2-addon/config/crd/patches/webhook_in_admirals.yaml
Outdated
Show resolved
Hide resolved
hmm... @camilamacedo86 would know that better, but |
Yeah, if I delete those files they are in fact regenerated (and they have
I must be missing someplace - will look through how |
I think it's being created by controller-tools here? Looks like maybe I need to send a PR there as well? Not entirely sure how to coordinate between the two repos - maybe controller_tools needs to merge first? |
yeah, just file a PR on controller-tools. You can either merge that first, or merge this then file another PR here to update the CT version in v3 & update the golden files. |
I've sent a PR for controller-tools too. |
Hi @matthchr, The |
- Only removing in v3 as v2 isn't accepting changes.
5098c72
to
e1f7121
Compare
/ok-to-test It shows ok for me 馃憤 |
/test pull-kubebuilder-e2e-k8s-1-15-3 |
@camilamacedo86 - it looks like a test timed out in the v2 tests... I don't think I changed anything there though (it was all reverted). Is this just a flaky test? |
/test pull-kubebuilder-e2e-k8s-1-14-1 |
@camilamacedo86 @DirectXMan12 - pinging on this, would be good to get it merged now that the controller-tools PR has merged. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/approve
It shows ok for me. 馃憤
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: camilamacedo86, matthchr 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 |
I understand that we can /lgtm this one. |
/lgtm |
Instead of having a dummy value in
caBundle
, (caBundle: Cg==
), instead just omit that field entirely as it is not required.Doing this means that repeated
kubectl apply
operators on a CRD work better, in accordance with the documented behavior of apply and how it considers last-applied-configuration, since they do not change theresourceVersion
of the CRD.This fixes #1684.