feat: allow "runtime-only" configuration without default issuer #395
Conversation
cert-manager-prow
bot
added
do-not-merge/work-in-progress
ThatsMrTalbot
force-pushed
the
feat/idle-startup
branch
2 times, most recently
from
a445350
to
7dc33f8
Compare
ThatsMrTalbot
changed the title
cert-manager-prow
bot
removed
the
do-not-merge/work-in-progress
pkg/certmanager/certmanager.go
Outdated
| // IssuerChangeSubscription is a subscription that can be used to get changes | ||
| // to issuer config | ||
| type IssuerChangeSubscription struct { | ||
| C <-chan *cmmeta.ObjectReference | ||
|
|
||
| // The same channel as above is mirrored in c, but without the "<-" | ||
| // restriction. This allows the channel to be written to by this package. | ||
| c chan *cmmeta.ObjectReference | ||
|
|
||
| lock sync.Mutex | ||
| closed bool | ||
| } |
There was a problem hiding this comment.
suggestion: I'm a bit squeamish about having two variables with the same name differing only in casing - I personally find that pretty hard to keep in my head.
I like C for the exported one, but maybe something like sendCh, internalCh or something else instead of c?
There was a problem hiding this comment.
Changed it to sendChannel
pkg/istiodcert/worker.go
Outdated
| @@ -57,7 +57,7 @@ type DynamicIstiodCertProvisioner struct { | |||
|
|
|||
| issuerRefMutex sync.Mutex | |||
|
|
|||
| issuerChangeChan <-chan *cmmeta.ObjectReference | |||
| issuerChangeChan *certmanager.IssuerChangeSubscription | |||
There was a problem hiding this comment.
nitpick (non-blocking): maybe rename because this isn't a channel any more?
| issuerChangeChan *certmanager.IssuerChangeSubscription | |
| issuerChangeSub *certmanager.IssuerChangeSubscription |
There was a problem hiding this comment.
Makes sense - updated
Signed-off-by: Adam Talbot <adam.talbot@venafi.com>
ThatsMrTalbot
force-pushed
the
feat/idle-startup
branch
from
7dc33f8
to
3cf2311
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: SgtCoDFish 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 |
cert-manager-prow
bot
added
the
approved
Changes
Helm
app.certmanager.issuer.enabledto disable "default" non runtime issuer.app.go
options.go
--issuer-enabledflag to disable "default" non runtime issuer. By setting this to false you can start istio-csr up and have it idle waiting for runtime config (while being ready).certmanager.go
Updated the runtime config watcher to prune subscriptions that are not in use. Functions like
WaitForIssuerConfigonly need a single channel event then stopped monitoring it. The addition of the ability to "close" the subscription means these functions can "unsubscribe".Moved
WaitForIssuerConfiginto the manager, it makes sense in my mind for it to be there so multiple things can call it if needed.configmap.go / tls.go
tls.Interface.RootCAswas blocking, this meant that if you had no "default" issuer config and the runtime config was not yet configured then the configmap controller Reconcile would block forever and delay graceful shutdown. The new implementation ofRootCAsaccepts a context so the controller can shut down gracefully.