[MERGE] Adding support for cert-manager certificates to manage webhook TLS #2083
Conversation
|
@yuchaoran2011 / @ChenYi015 - I moved over the changes from #2079 here due to merge and trailers issues, let me know what else is required. |
|
@yuchaoran2011 / @ChenYi015 - Anything that needs to be done on my side? |
|
@YanivKunda Missed this one earlier. Looks like another set of conflicts has happened. could you fix them? |
| @@ -101,6 +101,11 @@ spec: | |||
| - -webhook-timeout={{ .Values.webhook.timeout }} | |||
| - -webhook-namespace-selector={{ .Values.webhook.namespaceSelector }} | |||
| - -webhook-object-selector={{ .Values.webhook.objectSelector }} | |||
| {{- if .Values.webhook.certManager.enable }} | |||
There was a problem hiding this comment.
I don't think this is going to work, as these flags were removed in a previous PR by @ChenYi015
However, there is value in being able to provide our own certs, perhaps they should be added back @ChenYi015
There was a problem hiding this comment.
@Cian911 Make sense, I will do it.
There was a problem hiding this comment.
@ChenYi015 I've accidentally discarded the changes in my fork -
but trying to re-apply them now looks a bit more complicated after the big controller-runtime restructuring.
What are your thoughts?
YanivKunda
force-pushed
the
feature/certmanager-merge
branch
from
d8e6410
to
e2693f1
Compare
|
[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 |
This PR combines changes from #1567 and #2079, with the following content:
webhook-cleanup-job.yaml&webhook-init-job.yamldeleted)cert-manager.ioapiVersion to v1This should resolve issue #1178