feat(platform): ACME wildcard certificate on the default ingress-nginx path - #2988
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughIntroduces opt-in DNS-01 wildcard certificate auto-issuance for the default ingress-nginx path. A new ChangesDNS-01 Wildcard Certificate Auto-Issuance (WS2)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
On the default ingress-nginx path (gateway.enabled=false) a DNS-01 solver still minted a per-host ACME certificate for every system service, hitting Let's Encrypt rate limits once a deployment exceeds ~50 endpoints. Add an opt-in publishing.certificates.wildcard toggle (default false) and compute, in apps.yaml, an effective wildcard secret name plus a wildcard-issue signal: when enabled with solver=dns01 and no operator-provided wildcardSecretName, the platform targets one shared wildcard cert (*.<root-host> + <root-host>) instead of per-host certs. Issuance is off by default because a single-label wildcard does not cover a custom service host outside *.<root-host> (e.g. a keycloak ingress.host, harbor host, or grafana host pointed at another domain); enabling it would otherwise switch such a host to the controller's default certificate and serve the wrong certificate. The operator-provided (BYO) name always wins and never triggers issuance; http01 cannot issue wildcards; and the Gateway API path issues its own wildcard via the controller, so all three are excluded. This reuses the existing wildcard-secret-name consumption path, so no per-service ingress template changes are needed. Part of #2811. Signed-off-by: Aleksei Sviridkin <f@lex.la>
When the platform signals shared wildcard issuance via _cluster.wildcard-issue, the publishing controller (Release.Namespace == expose-ingress) renders one cert-manager Certificate for <root-host> + *.<root-host>. The resulting Secret is served as the controller's default SSL certificate and the existing wildcard-secret-name consumption path makes the system services drop per-host ACME. The Certificate uses the DNS-01 ClusterIssuer that cert-manager-issuers already renders in dns01 mode, so no per-tenant Issuer is minted. Only the publishing controller renders it — the Secret must be same-namespace for ingress-nginx to read it. Part of #2811. Signed-off-by: Aleksei Sviridkin <f@lex.la>
c3ab660 to
4218e4c
Compare
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request implements shared wildcard certificate issuance for the ingress-nginx path to mitigate Let's Encrypt rate limits at scale. By consolidating certificate management for system services into a single wildcard, the platform achieves parity with the Gateway API path. The feature is strictly opt-in to protect existing deployments that may rely on custom service hosts or specific tenant configurations that fall outside the scope of a single-label wildcard. Highlights
New Features🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on Gemini (@gemini-code-assist) comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces support for shared ACME wildcard certificate issuance on the default ingress-nginx path when using a DNS-01 solver. It adds a new opt-in configuration option publishing.certificates.wildcard in values.yaml, updates the platform templates to compute wildcard settings, and introduces a new wildcard-certificate.yaml template in the ingress package to render the cert-manager Certificate. Comprehensive unit tests have been added across several packages to verify the wildcard issuance logic. The reviewer noted that values.schema.json should be regenerated using make generate to include the new configuration option.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| # its self-signed default cert (browser warning), then self-heals — | ||
| # the same delay per-host ACME has on first issue, which an | ||
| # operator-provided wildcardSecretName avoids only by pre-existing. | ||
| wildcard: false |
There was a problem hiding this comment.
A new configuration option publishing.certificates.wildcard has been added to values.yaml, but the corresponding values.schema.json file has not been updated in this pull request.
Please run make generate to regenerate the schema file and ensure the new property is included for validation and dashboard UI integration.
References
- The repository style guide states that packages follow an umbrella chart pattern containing a
values.schema.jsonfor dashboard UI and input validation, which is regenerated viamake generate. (link)
There was a problem hiding this comment.
The core/platform chart has no values.schema.json and no make generate target — its publishing.certificates.* keys (solver, issuerName, wildcardSecretName, dns01.*) are documented via values.yaml comments only, not a generated schema. wildcardSecretName was added the same way in #2819. The values.schema.json / make generate convention applies to the catalog charts under packages/apps and packages/extra, not this core chart, so there is nothing to regenerate here. The new wildcard key is documented in values.yaml and in cozystack/website#588.
myasnikovdaniil
left a comment
There was a problem hiding this comment.
APPROVE — clean, well-guarded opt-in feature; ingress-nginx path now reaches parity with the Gateway wildcard path.
Reviewed for the central safety property (default-off, existing installs unchanged) and the dns01/wildcard wiring. Both hold.
Opt-in / default-off (verified)
- The issuance gate is a 4-way
and:not $byoWildcard∧solver==dns01∧wildcard∧not gateway.enabled. Under shipped defaults (wildcard=false,solver=http01,gateway.enabled=false) it never fires. - Toggle-off render is byte-additive:
wildcard-secret-namestays"", plus one newwildcard-issue: "false"line on the_clusterchannel (harmless to existing consumers, which index specific keys). The pre-existing BYO test (apps_wildcard_secret_test.yaml) still passes unchanged.
dns01 / wildcard wiring (verified)
- The
Certificatereferences theletsencrypt-prod/-stageClusterIssuerby name, andcert-manager-issuers/cluster-issuers.yamlattaches the multi-provider dns01 solver to exactly that issuer whensolver=dns01— so no per-tenant Issuer is minted and the solver isn't hardcoded to one provider. SAN covers apex +*.<root-host>as intended. - Fail-fast guards (
failonwildcard-issue=truewith empty secret-name or empty root-host) prevent a Certificate pointing at a nonexistent secret / empty dnsNames ifapps.yamlever regresses.
Consumption / parity (verified)
- Setting
wildcard-secret-nameis the single trigger that (a) drops per-host ACME on every system-service Ingress, (b) sets--default-ssl-certificate, (c) renders the one Certificate — services consume the wildcard, they don't double-issue. This is the rate-limit win. - Same-namespace
default-ssl-certificate($exposeIngress/$wildcardSecret), gated to the publishing controller (Release.Namespace == expose-ingress); child controllers can't read another ns's secret and are correctly excluded. extra/gateway/tenantgateway.yamlalready issues a per-apex wildcard in dns01 mode — the ingress path mirrors it, and the gateway path is explicitly excluded from auto-issuance, so no overlap.
Tests: ran helm unittest on all four touched packages locally — platform 65/65, ingress 15/15, keycloak 4/4, bucket 3/3. The coverage tradeoff (custom host / nested child-tenant host outside *.<root-host>) is pinned as explicit contract tests rather than left implicit. Nice.
Schema: the gemini-bot schema-regen flag is a non-issue — core/platform is a core chart with no values.schema.json and no make generate target (the wildcard key follows the same comment-documented pattern as wildcardSecretName from #2819). Already addressed in-thread.
Not tested against a live cluster — a real DNS-01 wildcard issuance needs a delegated domain + solver, so this is a static + render review. The documented first-issuance browser-warning window and the coverage/blast-radius caveats are honest and match why this is correctly opt-in.
## What this PR does Document two `publishing.certificates.*` options in the platform-package value table that were missing from the reference docs. - `publishing.certificates.wildcard` (new): opt-in shared wildcard certificate issuance on the default ingress-nginx path. When enabled with a DNS-01 solver it issues one `*.<root-host>` wildcard for system services instead of a per-host ACME certificate, avoiding Let's Encrypt rate limits at scale. Documented with its default (`false`), the dns01 / gateway-disabled gating, and the coverage / blast-radius caveat. - `publishing.certificates.wildcardSecretName` (pre-existing, previously undocumented): operator-provided wildcard TLS Secret that platform services serve under instead of minting per-host ACME certificates. Documents the code change in cozystack/cozystack#2988. Part of cozystack/cozystack#2811. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Documentation** * Added documentation for new Platform Package Publishing certificate configuration options enabling wildcard certificate support with opt-in capability and custom TLS Secret assignment. * Documented wildcard certificate behavior, including precedence rules between configuration options, override conditions, and hostname coverage considerations for tenant and service host patterns. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
What this PR does
On the default ingress-nginx path (
gateway.enabled=false) every published hostname still minted its own per-host ACME certificate via ingress-shim, even with a DNS-01 solver configured. That hits the Let's Encrypt rate limit (50 certs per registered domain per week) once a deployment exceeds ~50 endpoints. The Gateway API path already issues a single per-apex wildcard in DNS-01 mode; this brings the ingress-nginx path to parity. It implements the remaining ingress-nginx gap that #2400 was narrowed to in its triage comment (DNS-01 is already multi-provider, and the Gateway path already issues wildcards).The new
publishing.certificates.wildcardtoggle is opt-in and OFF by default. When set totruewithsolver=dns01and no operator-provided wildcard Secret, the platform issues one shared wildcardCertificatefor<root-host>+*.<root-host>and serves the resulting Secret as the publishing controller's--default-ssl-certificate. The system service Ingresses (dashboard, grafana, keycloak, harbor, …) then stop requesting a per-host cert.How it works:
core/platformapps.yamlcomputes the effective wildcard Secret name and a new_cluster.wildcard-issuesignal. An operator-providedwildcardSecretName(the BYO path) always wins and never triggers issuance; HTTP-01 cannot issue wildcards; and when Gateway API is enabled the TenantGateway controller issues the wildcard instead — so all three are excluded from auto-issuance.extra/ingressrenders the wildcardCertificateonly on the publishing controller (Release.Namespace == expose-ingress), because the Secret must be same-namespace for ingress-nginx to read it. The Certificate references the DNS-01ClusterIssuerthatcert-manager-issuersalready renders indns01mode, so no per-tenant Issuer is minted.This reuses the existing wildcard-secret consumption path, so no per-service Ingress template changes were needed — only the issuance side is new.
Why opt-in (default off): the chosen wildcard Secret name rides the
_clusterchannel, which every child tenant inherits verbatim, so this is not root-tenant scoped. A single-label wildcard*.<root-host>covers<service>.<root-host>but not a custom service host outside it (a keycloakingress.host, harborhost, or grafanahostpointed at another domain) nor a child tenant's nested host (<service>.<tenant>.<root-host>). Enabling issuance makes every such service drop its per-host ACME cert and fall back to the default certificate, which does not cover it. Leaving it off by default means a dns01 cluster is never silently switched on upgrade; an operator who enables it accepts the same coverage responsibility as the operator-provided wildcard path, which propagates identically. The tradeoff is pinned by contract tests (keycloak custom host, bucket nested child-tenant host). Per-tenant wildcards remain on the Gateway path and are tracked separately.Docs: cozystack/website#588
Closes #2400. Part of #2811.
Screenshots
N/A — no UI changes.
Release note
Summary by CodeRabbit
New Features
certificates.wildcardconfiguration option to enable shared wildcard TLS certificates.Tests