Vault 13350 acme add purge old unused accounts to tidy auto tidy #20227
Conversation
…ts-to-tidy-auto-tidy
| deleteCount := 0 | ||
|
|
||
| for _, thumbprint := range list { | ||
| revoked, deleted, err := a.TidyAcmeAccount(acmeCtx, thumbprint, time.Now().Add(config.DeleteAcmeAccountsSafetyBuffer), time.Now().Add(config.RevokeAcmeAccountsSafetyBuffer)) |
There was a problem hiding this comment.
It's early, without coffee kicking in yet, but should we not be either subtracting the safety buffers here, or using After within TidyAcmeAccount and not the value .Before(<time.Now()+safetybuffer>)
There was a problem hiding this comment.
Adding does work too - I've updated that on the new PR
| @@ -146,6 +146,8 @@ type acmeAccount struct { | |||
| Contact []string `json:"contact"` | |||
| TermsOfServiceAgreed bool `json:"termsOfServiceAgreed"` | |||
| Jwk []byte `json:"jwk"` | |||
| LastAccessDate time.Time `json:"last_access_date"` | |||
| MarkedAsRevokedDate *time.Time `json:"marked_as_revoked_date"` | |||
There was a problem hiding this comment.
Is there any real value of differentiating between nil and IsZero() for MarkedAsRevokedDate to be a pointer?
There was a problem hiding this comment.
The value that I saw was that it is far easier to remember when coding to check for nil value than for "0". It feels awkward because jan 1, 1970 is a totally valid date (and if something was revoked then, we'd totally want to delete it, so that's a bad error).
|
We've changed the approach here: rather than revoke/remove old accounts after some last-used-by-date (which needs to be constantly updated), we'll be looking at whether the account has an active order or certificate associated with it - see the new PR: #20494 |
Basics of Tidy and Auto-Tidy of Acme Accounts