Optionally allow to disable cert-manager's http01-edit-in-place annotation
What does this MR do?
Adds a new global configuration global.ingress.useNewIngressForCerts
. Which defaults to false
for full backwards compatibility.
Once enabled, the functional change is that a new temporary Ingress will now be created for each ACME certificate validation request.
There are two changes to achieve this:
- On the cert-manager
Issuer
, the Ingress property key for thehttp-01
solver is changed fromingress.class.name
toingress.ingressClassName
. - The annotation
"acme.cert-manager.io/http01-edit-in-place": "true"
is removed from theIssuer
.
Explanation
See cert-manager's docs on ingressClassName
. This property unsupported until the recent upgrade (!3446 (merged)) of cert-manager to 1.12+ (cert-manager release notes)
This removes the need for the "acme.cert-manager.io/http01-edit-in-place": "true"
Ingress annotation. The workaround was previously needed with the old cert-manager version because without the ingress.ingressClassName
property, a new Ingress could not be successfully added for most current Ingress-controllers (e.g. nginx-ingress has required this new property from 2020). So we would set this edit-in-place annotation to true in order for us to re-use an existing Ingress.
The previous logic of re-using the original Ingress through edit-in-place has the problem that the original Ingress might have annotations that make it unreachable from Let's Encrypt. One example of this is on the GitLab dedicated project we often have an IP address restriction on the existing Ingresses (nginx.ingress.kubernetes.io/whitelist-source-range
), which will not allow Let's Encrypt connections.
Related issues
GitLab Dedicated internal issue: gitlab-com/gl-infra/gitlab-dedicated/team#3045
Test Plan
Fresh install
- Set the
global.ingress.useNewIngressForCerts
value totrue
- Checkout the branch and run
helm dep up
(this pulls the updated cert-manager chart) - Deploy the branch to an cluster with a public IP (to allow ACME challenges to succeed)
- Verify cert-manager was installed and TLS/SSL works as expected
Upgrade path
- Deploy a chart with previous version of the chart (e.g. tag
7.5.1
to jump before the recent and related cert-manager version upgrade) - Verify cert-manager was installed and TLS/SSL works as expected
- Upgrade the release to this branch
- Verify cert-manager was installed and TLS/SSL works as expected
- Verify certificate renew works as expected (e.g. by using
cmctl
(cmctl renew --namespace <namespace> <release>-gitlab-tls
) or by setting arenewBefore
on the certificate resource)
Author checklist
See Definition of done.
For anything in this list which will not be completed, please provide a reason in the MR discussion.
Required
-
Merge Request Title and Description are up to date, accurate, and descriptive -
MR targeting the appropriate branch -
MR has a green pipeline on GitLab.com -
When ready for review, follow the instructions in the "Reviewer Roulette" section of the Danger Bot MR comment, as per the Distribution experimental MR workflow
Expected (please provide an explanation if not completing)
-
Test plan indicating conditions for success has been posted and passes -
Documentation created/updated -
Tests added/updated -
Integration tests added to GitLab QA -
Equivalent MR/issue for omnibus-gitlab opened -
Equivalent MR/issue for Gitlab Operator project opened (see Operator documentation on impact of Charts changes) -
Validate potential values for new configuration settings. Formats such as integer10
, duration10s
, URIscheme://user:passwd@host:port
may require quotation or other special handling when rendered in a template and written to a configuration file.