Cert-manager: Support adding custom annotations to generated secret

[michaelgeorgeattard]

Is there support for adding custom annotations to generated secret for syncing using kubed or similar?

If not, is this feature in the pipeline?

[munnerz]

There was some discussion on this previously here: https://github.com/jetstack/cert-manager/issues/566

It's not something that we're working at the moment or see as a priority as there are known workarounds for it already, and it'll require special handling for e.g. what if the values have changed/are out of sync.

For your use-case (kubed), can you match on one of the existing labels/annotations we already add to Secret resources?

[kfox1111]

I've hit this issue a few times myself. We haven't found a solution. We would like to setup our tenant-namespace chart https://hub.helm.sh/charts/pnnl-miscscripts/tenant-namespace to automatically create letsencrypt based wildcard certs for each tenant created. The way we're doing this is to have two namespaces:
<tenant-namespace> and <tenant-namespace>-admin.
<tenant-namespace>-admin is where we keep tenant specific services such as nginx-ingress so that only the cluster-admins can manage them. Ideally our Issuers/Certificates are in this namespace too so the users are not bothered by them.
But nginx-ingress only will look for secrets in <tenant-namespace> so we need to sync them.
We want the chart to be the only thing we have to manage, so the workaround specified other places would be to stick the secret into the chart. But at least with helm2, I believe it will overwrite the whole secret on upgrade. So the workaround doesn't work around the issue in our environment.

Do we think it would be a hard feature to add? Is there any opposition to adding the feature, or just lack of hands?

Thanks,
Kevin

[munnerz]

The specific issue/concerns are around:

1) The API schema was expose for this
2) How we 'reconcile' changes to these fields - i.e. if someone updates them on the Certificate, or on the Secret, what should happen?
3) How to allow users to opt-in/out of this behaviour

(1) is probably the harder of these problems to solve - trivially we could have spec.secretTemplate.metadata.annotations and spec.secretTemplate.metadata.labels, but I'm keen to hear if anyone else has thoughts on API format/schema 😄

Do we think it would be a hard feature to add? Is there any opposition to adding the feature, or just lack of hands?

The questions above are pretty much it :) I don't think this would be _particularly_ tough, aside from the need for updating documentation, writing tests etc. We are also going to be undertaking some ~fairly significant refactoring in the Certificates controller for this release (tracked here: https://github.com/jetstack/cert-manager/issues/2691) - this should not impact the feature too much, but worth noting :)
& yes, lack of hands is definitely a problem! I'd ask that if someone is keen to see this feature added, please take the lead on it 😄

/help-wanted
/priority backlog
/kind feature

[kfox1111]

spec.secretTemplate.metadata.annotations and spec.secretTemplate.metadata.labels would allow for the things we need to be done right now, while leaving room for doing all sorts of things in the future. So sounds good to me.

1. can we make it behave as kubectl apply would?
2. the existence of secretTempate triggers the new behavior?

[sfynx]

@munnerz

• How we 'reconcile' changes to these fields - i.e. if someone updates them on the Certificate, or on the Secret, what should happen?

I think the Secret should always reflect its associated Certificate, because the Certificate declares its desired configuration. So these fields should be overwritten in the Secret and should get overwritten again when they are changed behind cert-manager's back. Any extra annotations or data should be left alone, but I think that is already the case (in my ArgoCD setup I noticed cert-manager actually added its TLS data to an existing Secret becaue I used the same name, but did not touch the existing fields).

I can't think of a use case where you would expect to keep your changes if you change things by yourself while a k8s operator is actively managing it.

• How to allow users to opt-in/out of this behaviour

I'd say by annotating the Certificate. Default to false (opt-in) for backward compatibility.

The https://github.com/emberstack/kubernetes-reflector project actually does this by looking for its own annotations on a Certificate which will then be copied to a Secret (but I feel this creates an unnecessary inter-project dependency on your API, and would therefore be better style if the cert-manager project itself would handle this on its own CRD).

[kfox1111]

What about a kubectl apply style? 3-way merge

a copy of the annotations/labels applied by cert-manager is saved in an annotation at the time of application.

If the CR is changed, it can compare the annotations/labels in the CR vs the ones that it previously applied. That can allow it to delete annotations/labels that were removed from the CR that were previously managed. It also can ignore annotations/labels added by the user directly to the secret without ever being in the CR.

[kfox1111]

Any updates for this? Still a problem for us.

[benlangfeld]

I also need this. Is there a bounty for it?

[kfox1111]

Any objections to adding
extraSecretLabels
and
extraSecretAnnotations
to the Certificate CRD under spec?

Just digging through the code real quick, it does not look like it would be very hard to add the functionality?

Is it just need to be added here? :
pkg/controller/expcertificates/internal/secretsmanager/secret.go line 238

[onmyflow]

We also need this for kubed, it makes it really cumbersome to distribute a secret around namespaces if you can't even annotate it.

[cuotos]

We also need this for kubed, it makes it really cumbersome to distribute a secret around namespaces if you can't even annotate it.

@onmyflow We moved from Kubed to https://github.com/emberstack/kubernetes-reflector for this very reason.

[onmyflow]

We also need this for kubed, it makes it really cumbersome to distribute a secret around namespaces if you can't even annotate it.

@onmyflow We moved from Kubed to https://github.com/emberstack/kubernetes-reflector for this very reason.

Thanks for the suggestion! I'll try to switch to reflector since all we use kubed for, is to sync secrets anyway.

[kfox1111]

Yeah, this looks very promising. Thanks.

[benlangfeld]

I continue to use Kubed because kubernetes-reflector is not a full replacement; it does not allow syncing between clusters, nor is it possible to create a dummy mirror Secret without valid data.

This feature requirement remains open on cert-manager for me. I'm willing to implement it or potentially fund its implementation, but need @munnerz to acknowledge the suggestion from @kfox1111 before investing time in something that might be rejected.

[ronaldmiranda]

For me too +1

[kfox1111]

@munnerz what do you think?