renew certs within Kubernetes secrets before they expire #112
Conversation
| @@ -25,6 +26,9 @@ func (r *Resource) GetDesiredState(ctx context.Context, obj interface{}) (interf | |||
| secret := &apiv1.Secret{ | |||
| ObjectMeta: apismetav1.ObjectMeta{ | |||
| Name: key.SecretName(customObject), | |||
| Annotations: map[string]string{ | |||
| UpdateTimestampAnnotation: r.currentTimeFactory().In(time.UTC).Format(UpdateTimestampLayout), | |||
| }, | |||
There was a problem hiding this comment.
The secrets that contain the certificates have now an annotation for the timestamp they where they were last updated. This timestamp is used to calculate whether the certificate tracked by the secret should be renewed.
| type VaultCrt struct { | ||
| ExpirationThreshold string | ||
| Namespace string | ||
| } |
There was a problem hiding this comment.
This is trying to make it right in the first place, plus boyscouting. ExpirationThreshold is a new option and Namespace already existed but was only hard coded in the resource implementation.
| @@ -77,48 +75,3 @@ func (r *Resource) newCreateChange(ctx context.Context, obj, currentState, desir | |||
|
|
|||
| return secretToCreate, nil | |||
| } | |||
|
|
|||
| func (r *Resource) ensureVaultRole(customObject certificatetpr.CustomObject) error { | |||
There was a problem hiding this comment.
This is now used for creation and deletion, so I moved it to the resource.go to have the code in a more common place.
| @@ -104,14 +105,18 @@ func Test_Resource_VaultCrt_newCreateChange(t *testing.T) { | |||
| var err error | |||
| var newResource *Resource | |||
| { | |||
| resourceConfig := DefaultConfig() | |||
| c := DefaultConfig() | |||
|
|
||
| newResource, err = New(resourceConfig) | ||
| c.ExpirationThreshold = 24 * time.Hour | ||
| c.Namespace = "default" |
There was a problem hiding this comment.
The new flags are required.
| VaultRole vaultrole.Interface | ||
|
|
||
| Namespace string | ||
| CurrentTimeFactory func() time.Time |
There was a problem hiding this comment.
I had to add this to keep the code testable.
| return false, microerror.Mask(err) | ||
| } | ||
|
|
||
| if t.Add(TTL).Add(-threshold).Before(r.currentTimeFactory()) { |
There was a problem hiding this comment.
t is the time of the last update. t plus the certificate TTL is the point in time where the certificate expires. We want to renew certificates before they expire. Thus we substitute a threshold from the point in time of the expiration and compare this value against the current time.
| resource: | ||
| vaultCrt: | ||
| expirationThreshold: '24h' | ||
| namespace: 'default' |
There was a problem hiding this comment.
The new settings are properly set in the config map. Note that the 24 hours is a magic number I just made up. IMO it should be good enough to let the cert-operator know 24 hours before the certificates expire that it should actually renew them in the Kubernetes secrets.
xh3b4sd
requested review from
jgsqware,
JosephSalisbury,
rossf7,
kopiczko and
MarcelMue
| UpdateTimestampAnnotation = "giantswarm.io/update-timestamp" | ||
| // UpdateTimestampLayout is the time layout used to format and parse the | ||
| // update timestamps tracked in the annotations of the Kubernetes secrets. | ||
| UpdateTimestampLayout = "2006-01-02T15:04:05.000000Z" |
There was a problem hiding this comment.
is Layout the right term here? I was confused here. I think the better term is Format
There was a problem hiding this comment.
This is the golang naming.
| } | ||
|
|
||
| renew, err := r.shouldCertBeRenewed(currentSecret, TTL, r.expirationThreshold) | ||
| if IsMissingAnnotation(err) { |
There was a problem hiding this comment.
If the annotation is missing, could we add it?
There was a problem hiding this comment.
We add it with new clusters. I think there will be a transition period like in the kvm-operator with the version bundle versions tracked in the deployments.
There was a problem hiding this comment.
My thinking was we could avoid the transition period. No strong opinion.
No description provided.