/kind bug
What happened: For security reasons, our k8s installation includes an nginx-ingress-controller deployment configured to block all incoming external traffic, and whitelist only internal networks. This is accomplished including whitelist-source-range in the controller's configmap, which applies a global filter on all Ingresses managed by nginx-ingress-controller. By demand, only a few applications get their ingresses open to external traffic.
Ceritificates can only be issued using cert-manager when the global whitelist is removed from nginx-ingress-controller, and working around this does not appear to be simple.
A similar issue has been raised before (#498), including a PR (#444) that was refused for some reason. The issue was closed with the mention of ingressClass, but it's still unclear how this solves the issue, since it involves allowing cert-manager to modify an existing ingress -- presumably the ingress object created for your application.
For one thing, we use helm for all application deployments. This means, ingress objects created for all applications are managed by helm. They cannot be altered by a third party agent without breaking the deployment and causing chaos. Helm takes care to keep a hash of all deployed objects, and if that checksum does not match on the next deploy, modified objects will not be updated.
Keeping letsencrypt IP addresses in the whitelist does not seem like an elegant solution.
On the other hand, nginx-ingress-controller could be configured to accept all traffic. But keeping a separate whitelist for all deployments means a lot of duplicate configuration everywhere. This also seems innapropriate, especially at scale.
What you expected to happen: I expect a way to configure cert-manager, defining annotations on the ingress it creates to accept ACME challenges. I should be able to customize that ingress to fit the needs of our environment.
How to reproduce it (as minimally and precisely as possible): Testing should be straight forward. Deploy nginx-ingress-controller. Block all traffic using a global configmap and whitelist only a number of networks.
NOTE: The only drawback for testing it this way is that you have to be carefull not be rate limited by letsencrypt servers. Since local networks are allowed, cert-manager local check passes, and it will open the acme challenge for remote access... except letsencrypt will keep trying until you hit the rate limit.
Thanks for the request, and sorry for the delay getting back to you here.
Would it be sufficient if we added the nginx.ingress.kubernetes.io/whitelist-source-range: 0.0.0.0/0 annotation to any ingress resource that cert-manager creates automatically, so that it is excluded from the denylist?
Allowing arbitrary annotations/labels is an API change, and creates a binding from how we represent certificates to how we actually solve them, something I am trying to avoid (in order to keep our API surface clean).
As you say however, cert-manager will not currently work well if a global denylist is configured, so we could/should do something here to work better. I know that kube-lego previously did this, which seemed to work well.
The area of code in question that'll need updating, is here: https://github.com/jetstack/cert-manager/blob/master/pkg/issuer/acme/http/ingress.go#L119-L123
Had not thought of that, but yes! This would work around it in a very simple way.
I can't think of any use of LetsEncrypt that would require cert-manager's ingress to be black listed. It needs external ingress by definition. Thinking of keeping backwards compatibility for some reason, this could be a flag/option to enable in case it's needed.
So, yes. I would appreciate it!
Best regards.
We are experiencing the same issue - is there any workaround for now?
For now, we literally added all letsencrypt IPs we could find to the whitelist.
We are aggregating logs remotely into Graylog, so we can more easily filter nginx logs to find them.
@munnerz The idea to overwrite global whitelist-source-range on ingress resources created by cert-manager is good for me.
Configuring annotations added to acme challenge ingresses in a configmap would be a great improvement. And this way, you let to user the choice/responsability to override it's IP restriction.
I've updated the issue title and added some labels. If anyone wants to pick this up, feel free 馃槃
I can take a look at this during the week (if no one gets there before me). I got the dev env up yesterday and built the project so there should not be any rocket science left. Just some issues running the unit tests, but if I don't manage to solve that I will reach out on the dev slack channel.
Nothing special here @munnerz, just add the annotation to ingress resources created? This will be a good first issue for me to start browsing the project code and hopefully I can find some more good first issues to do later on.
Yep exactly @larsha 馃槃
Feel free to reach out if you run into issues getting set up!
You should only need Bazel (version 0.17.1 or later) + Docker installed to run/build/test the project. Once you have these two, unit tests can be run with make verify_unit 馃槃
Added PR here: https://github.com/jetstack/cert-manager/pull/1005
Most helpful comment
Thanks for the request, and sorry for the delay getting back to you here.
Would it be sufficient if we added the
nginx.ingress.kubernetes.io/whitelist-source-range: 0.0.0.0/0annotation to any ingress resource that cert-manager creates automatically, so that it is excluded from the denylist?Allowing arbitrary annotations/labels is an API change, and creates a binding from how we represent certificates to how we actually solve them, something I am trying to avoid (in order to keep our API surface clean).
As you say however, cert-manager will not currently work well if a global denylist is configured, so we could/should do something here to work better. I know that kube-lego previously did this, which seemed to work well.
The area of code in question that'll need updating, is here: https://github.com/jetstack/cert-manager/blob/master/pkg/issuer/acme/http/ingress.go#L119-L123