Keep one of the duplicated secrets

[ankon]

There is really no problem with duplicated secrets: these exist, and will be shared
by the ingresses that refer to them. The existing code excluded a secret as soon as
it was referenced by more one than place, but we should really just de-duplicate
the given list based on namespace/name.

---

In my specific situation I have multiple applications, and each of them has an associated ingress. In each of these ingresses I also define a secondary rule for exposing the application on a shared "legacy" hostname.
I want a certificate for that one as well :)

[simonswine]

Not too sure if that is a good idea. I think that will have non-deterministic behaviour. I might be convinced if we have a better test coverage to merge that though

[ankon]

Hmm. Non-deterministic where? The secrets with the same name would also have the same content, as we're just referencing them here.

I might be missing something though, so if you can share your concerns I could maybe try to write a suitable test case.

[bartoszhernas]

Please, someone, merge this? :)

[bartoszhernas]

@simonswine this is a serious issue, I realised today that these duplicated entries are also not refreshed, could you merge this or share your concerns?

[bartoszhernas]

If someone needs to use this fork, I've build public docker image here. Usage

    image: ahoy/kube-lego:canary

[munnerz]

So one particular case here that I'd want to be aware of is if there are two ingresses referencing the same secret, and the user has specified two unique lists of domains in each to provide TLS for.

This would cause kube-lego to enter an infinite loop of obtaining certificates, until rate limits are hit on the ACME server. There are a few options what to do in this case:

• kube-lego could merge the two ingresses, and attempt to obtain a certificate for all. This makes the two ingresses co-dependent on each other in a way, and any change to one ingress will affect the secrets of both. It could potentially also cause problems at renewal time, if one of the two ingresses no longer is valid/pointed at the ingress controller

• kube-lego could refuse to operate iff the list of domains is different. This is probably the safest option, and doesn't lock us into this behaviour in future.

Another area we need to be careful of is around the workqueue - as it stands, I think kube-lego processes renewals (and actually obtaining the certs) in serial, which should prevent a lot of race conditions. However, we'd need to clearly document in some development guidelines that this must always be the case, to prevent these kinds of races.

As an alternative, we could consider the planned Certificate CustomResourceDefinition as a workaround for this issue. A user could create a Certificate request, that lists the whole list of domains required and a target secret, then the other ingresses could reference just that secret. The issue I see here is that currently, we are unsure how these Certificate resources play into HTTP based challenges. This is something I'd hope we can achieve, but it's a complex problem as a user may be running multiple ingress controllers within their cluster.

[ankon]

Thanks for that one, it seems indeed a valid issue with keeping the secrets.

In terms of what to do here it seems easier for this PR to add an error condition if we find a reference to a secret with a differing set of hosts to be covered. Your other options look "bigger"/"riskier", and especially the suggestion of using a CRD would imply that older kubernetes versions could no longer be used, right?

[munnerz]

@ankon I think your suggestion there may work (detecting if the ingresses have a differing set of hosts before proceeding). I'd be happy to accept a pull request that takes this into account.

As an aside, we now have cert-manager that is backed by CRDs, and thus does allow for this use case. It is not 'blessed' yet (ie. still in development), however is coming along very well and I'd be keen to hear anyone's experience, especially around issues like these!

[Globegitter]

@ankon yeah would be great if this PR could be updated.

[DocValerian]

Our use case is simmilar, we have different stages / namespaces with individual certs per stage. Then there are multiple apps running on these stages (but they may vary). So each app has it's own ingress and adds its basepath to the Stage-Host, using the respective host and the host_stage_secret.
We used to have a secret for each app to mitigate this limitation. But since they all request basically the same cert, the Let's Encrypt limit of 5 max duplicates/week is reached pretty fast.

I think the check should not even need to look for the hosts. Just warn that only the first detected ingress hostset will be used to get the secret and if you configure different hosts then you're doing it wrong.

... it would really be great to have this made into a stable release.

[adamdecaf]

I'm running into the same underlying problem, but solved it a bit differently.

We run a cluster per env, so on our stage environment we've got several namespaces with tls-acme'd Ingress objects. At the crux is a dynamic "index.html" type page which offers links through.

For now we've consolidated the Ingress objects to share one tls Secret per namespace, but I can see us still running over the LE rate limits as we add more namespaces.

Edit: I didn't realize the ingress controllers can support SNI. We've dropped the tls: objects from all but one Ingress and are only using that one cert now. Much better!

[adamdecaf]

Has anyone proposed kube-lego naming secrets based on the hostname? For infra.foo.com the secret could be called kube-lego-infra-foo-com.

Edit: It looks like Ingress objects will be locked to their same ns until kubernetes/kubernetes#57325 is fixed.