Updates for Cert Manager #1129
Updates for Cert Manager #1129
Conversation
slaskawi
force-pushed
the
cert-manager-updates
branch
from
4cc789c
to
13cc46d
Compare
|
/lgtm |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: slaskawi, stlaz, sttts 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 |
openshift-ci
bot
added
the
approved
| if clients.kubeClient == nil { | ||
| result.Error = fmt.Errorf("missing kubeClient") | ||
| } else { | ||
| result.Result, result.Changed, result.Error = ApplyValidatingWebhookConfiguration(ctx, clients.kubeClient.AdmissionregistrationV1(), recorder, t, -1) |
There was a problem hiding this comment.
@slaskawi this -1 for expectedGeneration means that every time this function is called (i.e., during the static controller sync), the resource will be updated in the apiserver. Is that what we want?
Coincidentally I was working on this and I was thinking about this this to solve this issue:
https://github.com/openshift/library-go/compare/master...bertinatto:webhook-config-static?expand=1#
There was a problem hiding this comment.
Also, I believe this won't work without registering the scheme in the init() function above.
CC @stlaz
There was a problem hiding this comment.
Also, I believe this won't work without registering the scheme in the init() function above.
@bertinatto I believe you're right. Let me send an update in a few minutes.
this -1 for expectedGeneration means that every time this function is called (i.e., during the static controller sync), the resource will be updated in the apiserver. Is that what we want?
Coincidentally I was working on this and I was thinking about this this to solve this issue:
https://github.com/openshift/library-go/compare/master...bertinatto:webhook-config-static?expand=1#
Yes, I was aware of that. Unfortunately I couldn't come up with anything better.
So I decided to do a tradeoff here and always update the resource. Otherwise, a user could modify it and we would never revert his changes.
Perhaps @stlaz has a better idea how to solve this?
There was a problem hiding this comment.
PR#1135 does not fixes the issue @bertinatto pointed out though right?
There was a problem hiding this comment.
No. The code as it is now always updates performs an update.
https://issues.redhat.com/browse/AUTH-5
This Pull Request introduces CRD and Admission Registration types into the StaticResourceController