Closed charleszheng44 closed 3 years ago
[APPROVALNOTIFIER] This PR is APPROVED
This pull-request has been approved by: charleszheng44
The full list of commands accepted by this bot can be found here.
The pull request process is described here
@vincent-pli The VC should be able to run on Kind after this PR is merged.
@christopherhein @Fei-Guo
@charleszheng44 do you think VC should be managing these resources for the user? or should we be prescribing that the operator use cert-manager or manage their cluster certificates themselves?
I know cert-manager itself did a lot of cludgy work just to enable this type of seamless setup process for auto-deploying their own certs and it was fairly painful to write and maintain. It feels like we could make this easier with just supplying configs to use cert manager auto-injection and then we could rip out all this setup code and expect that if they aren't using cert-manager that they have an existing flow for certs and thus can just use that path.
Code looks fine otherwise more just commenting on the overall approach.
@christopherhein I prefer using cert-manager to manage all certifications. But do you think it would be an overkill to deploy cert-manager to just manage the certification for the webhook server. Or we can use the cert-manager to manage all certifications for VC control plane components. However, I think we will use the CAPN to replace the VC native mode, and maybe we should import the cert-manager by then? We can use the cert-manager to manage certifications for all components (i.e., CAPN control plane components and VC webhook servers).
@charleszheng44 mmk, that seems like a reasonable approach instead.
/lgtm
What this PR does / why we need it:
Use a self-signed certificate for the virtual cluster webhook. See issue #125 for more details.
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close the issue(s) when PR gets merged): fixes #125