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

Change the way extra ingresses are created #178

Closed
herodes1991 opened this issue Mar 31, 2022 · 7 comments
Closed

Change the way extra ingresses are created #178

herodes1991 opened this issue Mar 31, 2022 · 7 comments

Comments

@herodes1991
Copy link
Contributor

These days we are migrating some ingresses to migrate them from HTTP01 to DNS and we are facing some problems.

Due to this migrations, some requests that were previously working, are starting to respond with error while the certificate is not validated. Our idea is to copy the secrets from the original ingress to the new ones to avoid this downtime.

@oyvindio
Copy link
Member

Is it possible to add some more details to describe this issue? It is not entirely clear to me which situation or configuration(s) that make this a problem and because of that it is difficult for me to consider the benefit/potential drawbacks of the suggested solution

@herodes1991
Copy link
Contributor Author

herodes1991 commented Jun 16, 2022

Yes! For example, when you have the ingress with tls enabled defined as internal and you want to create an extra ingress defined as nginx. During the time when the certificate is being created, the nginx one will not be working. With this change, the nginx one will use the same certificate as the internal one until the new certificate is created. Also note the time for solving a certificate challenge could range easily from 20 to 40 minutes without issues, which of course is not acceptable in a production environment

@oyvindio
Copy link
Member

I'm sorry, I still don't entirely understand this. I'd appreciate if you could help make some of these points clearer to me:

For example, when you have the ingress with tls enabled defined as internal and you want to create an extra ingress defined as nginx.

  • What do you mean by "defined as internal" and "defined as nginx" here? Does it have to do with certificate issuers or ingress class or something else entirely?
  • How do the two ingresses look (hosts, paths, relevant annotations etc.) in this example?

In the description of the issue you mentioned this:

These days we are migrating some ingresses to migrate them from HTTP01 to DNS and we are facing some problems.

Does this refer to a "one-off" migration to switch cert-manager ACME solvers for some/all ingresses?

@portega-adv
Copy link
Contributor

portega-adv commented Jun 16, 2022

Let me try to help understanding the issue:

About HTTP/DNS migration: yes, it refers to cert manager and cert generation challenges. While HTTP has no delay (once you publish the challenge response file, it's available), moving to DNS challenge adds a delay for DNS propagation. Copying secrets here helps to avoid that, by providing the current certificate until the new one is generated.

This will be a one time operation, but there are other cases where the change will be helpful too, and those will be recurring cases. The internal/nginx example refers to that, when you add annotations and the update will enforce the Ingress to split in two objects, one per domain, the "whatever-app" Ingress will keep the certificate but the "whatever-app-1" will not, and will be unavailable until a new one is generated.

The generic case will be: whenever an update on an Ingress object generates a secondary one, the latest will be unable to receive traffic until a cert has been generated. Our proposed workaround is to copy the secret from the former Ingress to the new one.

@herodes1991 please correct me if I missed (or messed) something.

@oyvindio
Copy link
Member

Thanks for the clarification. My understanding of the issue is: you want to move some or all host+path items from ingresses that currently use TLS certificates created via one issuer/ACME solver to another ingress which is using another issuer/ACME solver. During this process some requests can be served an invalid certificate (or none at all, depending on cert-manager behaviour) because it takes some time for a certificate to be provisioned for the new ingress. Feel free to correct me if I misunderstand.

Based on that I have some concerns with the suggested solution:

  • Copying a certificate+key pair from an existing ingress only helps when "moving" some or all of the existing host+path items from one ingress to another ingress via the annotation feature. To my understanding, it does not solve the generic case of a new ingress waiting for a certificate to be provisioned.
  • A certificate+key pair copied from an existing ingress to another could be valid for hostnames other than those included in the new ingress. I'm not sure if this is a problem in practice but it doesn't seem ideal.
  • It seems to me to couple fiaas-deploy-daemon closer to cert-manager behaviour.
  • Copying secrets require giving fiaas-deploy-daemon RBAC privileges to read and write secrets, which it currently doesn't have (or need).

Because of this I am not sure if implementing this behaviour in fiaas-deploy-daemon is the best fit - especially if it is mainly to support a one-off migration. Could it be an option to instead solve the underlying issue with a process outside fiaas-deploy-daemon, via an external process, a mutating webhook, as a "FIAAS extension", or similar?

@mortenlj
Copy link
Member

I tend to agree with @oyvindio here. I'm not sure this is a good fit with what fiaas-deploy-daemon should be focused on.

I'm somewhat confused about why this is a problem that needs fixing, so I think I might be misunderstanding something.
When a new Ingress resource is created, which needs a new certificate, isn't that also on a new host+path? It isn't going to receive traffic instantly when created, is it? If not, why can't the normal ACME process be allowed to complete before you start directing traffic to the new ingress?

@portega-adv
Copy link
Contributor

We'll find a different way of solving this particular issue, thanks for the feedback and the support!

@portega-adv portega-adv closed this as not planned Won't fix, can't repro, duplicate, stale Jul 6, 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 a pull request may close this issue.

4 participants