Cert-manager: Standard set of conditions
/kind feature
We currently don't use conditions in a consistent manner. We should design a standard set of a small number of conditions that make it easier to programmatically check the status of resources.
kragniz
All 4 comments
Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
If this issue is safe to close now please do so with /close.
Send feedback to jetstack.
/lifecycle stale
retest-bot
on 23 Oct 2018
Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.
If this issue is safe to close now please do so with /close.
Send feedback to jetstack.
/lifecycle rotten
/remove-lifecycle stale
retest-bot
on 22 Nov 2018
/remove-lifecycle rotten
/assign
/lifecycle active
munnerz
on 28 Nov 2018
So after some discussion with @kragniz, @mikebryant and @wwwil we're going to adjust the meaning of the 'Ready' condition to be:
- x509 certificate is not expired
- the dnsNames, commonName, isCA fields match their corresponding fields on the x509 certificate
- the stored private key matches the stored certificate (i.e. they are a 'keypair')
If any of these are not met, the Ready condition will be set to false.
If the x509 certificate does not exist, the Ready condition will also be set to false.
Still to incorporate:
- set Ready to false if the keySize or keyAlgorithm do not match
(this requires 'centralising' private key generation logic into the Certificates controller to do cleanly)
We currently utilise the Ready condition to convey information about certificate issuance failures in the Vault, CA and selfsigned issuers. These will be updated to log information using Events, and instead of setting Ready to false, will instead set the status.LastFailureTime field to a non-nil value. This should ideally be done by the Certificates controller iff the Issue function returns a non-nil error.
This will then also allow us to centralise 'failure backoff' handling, to avoid bespoke failure handling within each issuer implementation.
munnerz
on 28 Nov 2018
Related issues
apetheriotis
路
3Comments
Stono
路
3Comments
jonathan-kosgei
路
4Comments
jbartus
路
4Comments
howardjohn
路
3Comments
Most helpful comment
So after some discussion with @kragniz, @mikebryant and @wwwil we're going to adjust the meaning of the 'Ready' condition to be:
If any of these are not met, the Ready condition will be set to false.
If the x509 certificate does not exist, the Ready condition will also be set to false.
Still to incorporate:
(this requires 'centralising' private key generation logic into the Certificates controller to do cleanly)
We currently utilise the Ready condition to convey information about certificate issuance failures in the Vault, CA and selfsigned issuers. These will be updated to log information using Events, and instead of setting Ready to false, will instead set the
status.LastFailureTimefield to a non-nil value. This should ideally be done by the Certificates controller iff the Issue function returns a non-nil error.This will then also allow us to centralise 'failure backoff' handling, to avoid bespoke failure handling within each issuer implementation.