Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure unique name for letsencrypt issues #249

Closed

Conversation

dakotabenjamin
Copy link

To be honest I don't know if this is the correct solution, but I ran into an issue trying to setup a second stack where it failed trying to create a ClusterIssuer resource that already existed.

To be honest I don't know if this is the correct solution, but I ran into an issue trying to setup a second stack where it failed trying to create a ClusterIssuer resource that already existed.
@batpad
Copy link
Member

batpad commented Jul 21, 2022

@dakotabenjamin 👋 hey - this is a good catch. The problem is we ideally want the letsencrypt-issuer resource to exist only once per cluster (I think).

I'll try to spend some time with this today or tomorrow - I think ideally we may need to setup letsencrypt and anything else that needs to exist once on the cluster as a separate helm chart. Sorry, I think we hadn't fully tested the SSL / domain work-flow with multiple instances and we missed this. I have some memory of thinking through this, so let me try and re-visit and propose a solution.

Let me know if your solution seems to work for now and if there's anything we can help to get it through, but we definitely want to figure something out where the ClusterIssuer exists only once per cluster.

Thanks again for the catch and PR.

@batpad
Copy link
Member

batpad commented Jul 21, 2022

So, I think roughly what we need to do:

cc @Rub21 @geohacker

@yuvipanda - if you're able to give this a sanity check, I'd be extremely grateful.

@yuvipanda
Copy link

The two options are:

  1. Have a separate 'once-per-cluster' chart that installs things like cert-manager, nginx-ingress, prometheus, etc. (https://github.com/2i2c-org/infrastructure/tree/master/helm-charts/support is ours)
  2. Scope the name of the issuer with .Release.Name, as is done here.

So it totally depends on how much per-cluster infra you have. How is cert-manager being installed? And any monitoring / ingress tools? Whatever is installing that should ideally also setup the issuer. But if you don't have a lot of 'once-per-cluster' tooling (perhaps you just use the cloud provider ingress and monitoring tools), doing (2) is probably fine.

@batpad
Copy link
Member

batpad commented Jul 21, 2022

Thanks @yuvipanda ! Let's go with (2) then. I think eventually it would be nice to have the issuer as a parameter to the ingress definitions and something in values that allows one to not include an issuer.

But, for now the approach in the PR seems fine - so, @dakotabenjamin then the other thing we'd need to do is update all the references to the letsencrypt-prod-issuer resource to also use the {{ template "osm-seed.fullname" . }}- prefix. This would be in all the ingress definitions, like so: https://github.com/developmentseed/osm-seed/blob/develop/osm-seed/templates/nominatim-api/nominatim-api-ingress.yaml#L8

Let me know if you'd like to add it to your PR or I can go ahead and make a new PR.

Thanks again for the catch @dakotabenjamin and for the words of wisdom, @yuvipanda !

cc @Rub21

@batpad
Copy link
Member

batpad commented Jul 26, 2022

Closing in favour of #251

@batpad batpad closed this Jul 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants