docs: fix mermaid edge label and document wildcardSecretName for v1.5#615
Conversation
✅ Deploy Preview for cozystack ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughUpdated Gateway API Mermaid diagrams with quoted edge labels and added documentation for configuring an existing wildcard TLS Secret through platform package publishing settings. ChangesGateway documentation
Estimated code review effort: 1 (Trivial) | ~3 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
There was a problem hiding this comment.
Code Review
This pull request updates the documentation by quoting a label in a Mermaid diagram in gateway-api.md (both next and v1.5 versions) and adding documentation for the publishing.certificates.wildcardSecretName parameter in platform-package.md. Feedback on the documentation change suggests wrapping the <root-host> placeholders in backticks to prevent them from being parsed as HTML tags, and renaming them to <host> for consistency with other parameters.
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.
| | `publishing.certificates.dns01.rfc2136.tsigAlgorithm` | `"HMACSHA256"` | TSIG HMAC algorithm. | | ||
| | `publishing.certificates.dns01.rfc2136.secretName` | `""` | Secret name holding the TSIG key material. Required when `provider=rfc2136`. | | ||
| | `publishing.certificates.dns01.rfc2136.secretKey` | `"tsig-secret-key"` | Key inside the Secret holding the TSIG key. | | ||
| | `publishing.certificates.wildcardSecretName` | `""` | Operator-provided wildcard TLS Secret. When set, platform services and the root tenant's ingress/Gateway serve under this pre-existing Secret instead of minting per-host ACME certificates; only the NAME travels through the platform values channel, never the certificate or private key. The Secret must already exist in the publishing namespace (`tenant-root` by default — the namespace running the root ingress controller selected by `publishing.ingressName`), be of type `kubernetes.io/tls`, and cover the served hosts (typically `*.<root-host>` + `<root-host>`). Leave empty to keep ACME issuance (`solver` / `issuerName` above). | |
There was a problem hiding this comment.
The placeholders <root-host> are not wrapped in backticks. In Markdown, unescaped angle brackets like <root-host> can be interpreted as HTML tags by some parsers, causing them to be omitted from the rendered output. Additionally, using <host> instead of <root-host> is more consistent with the publishing.host parameter defined earlier in this table.
Here is the suggested replacement:
| `publishing.certificates.wildcardSecretName` | `""` | Operator-provided wildcard TLS Secret. When set, platform services and the root tenant's ingress/Gateway serve under this pre-existing Secret instead of minting per-host ACME certificates; only the NAME travels through the platform values channel, never the certificate or private key. The Secret must already exist in the publishing namespace (`tenant-root` by default — the namespace running the root ingress controller selected by `publishing.ingressName`), be of type `kubernetes.io/tls`, and cover the served hosts (typically `*.<host>` and `<host>`). Leave empty to keep ACME issuance (`solver` / `issuerName` above). |858f8de to
33ec767
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@content/en/docs/v1.5/operations/configuration/platform-package.md`:
- Line 83: Update the v1.5 documentation entry for
publishing.certificates.wildcardSecretName to state that it takes precedence
over publishing.certificates.wildcard when both are configured and disables ACME
issuance. Keep the existing Secret requirements and empty-value behavior intact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3b0067d6-99e0-416d-8530-51acedb42113
📒 Files selected for processing (3)
content/en/docs/next/networking/gateway-api.mdcontent/en/docs/v1.5/networking/gateway-api.mdcontent/en/docs/v1.5/operations/configuration/platform-package.md
myasnikovdaniil
left a comment
There was a problem hiding this comment.
LGTM.
The mermaid fix is correct and well-justified: unquoted parentheses inside a pipe-delimited edge label (|issues Certificate(s)|) break Mermaid's parser and drop the entire flowchart; quoting matches the convention already used in these docs, and it's applied to both the next and v1.5 copies. The wildcardSecretName row is accurate and belongs in v1.5, which genuinely lacked it — next already carries the row on current main, so this brings v1.5 to parity (no next gap).
One coordination note: #619 also adds a wildcardSecretName row to this same v1.5 file, so the two will collide on merge — worth sequencing them, or folding this two-line mermaid fix into #619.
The Gateway API traffic-path flowchart failed to render: an edge label contained unquoted parentheses, which Mermaid's parser rejects. Quote it, matching the convention used by every other edge label in these docs. The v1.5 platform-package reference was also missing the row for publishing.certificates.wildcardSecretName, an option that ships in v1.5. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la>
33ec767 to
0ad3469
Compare
…t cert mode (#619) Split out of #615 so the corrections below get reviewed as what they are — behavioural claims about the TenantGateway controller and the publishing path — rather than riding along in a typo fix. Both the v1.5 and the `next` snapshots are updated. ## `publishing.exposure` is gone The reference documented `publishing.exposure`. That key was added and then reverted (`revert(ingress)!: remove broken publishing.exposure=loadBalancer mode`) before v1.5 shipped: it exists in `release-1.4` and in no branch since. An operator on v1.5 following the table set a key that is silently ignored. The row is removed from both snapshots, along with a stale cross-reference to it on the Gateway API pages, and a note tells v1.4 upgraders to drop it. The v1.4 docs are deliberately left alone — there the key is real. That cross-reference also asserted `externalTrafficPolicy: Local` for the legacy ingress path. It is only `Local` when `publishing.externalIPs` is empty; with external IPs set — the usual bare-metal install — the host ingress Service is a `ClusterIP` with `externalTrafficPolicy: Cluster` and NATs client source IPs exactly like the Gateway path. Corrected. ## `publishing.exposureClass` is documented Removing the dead key left `next` with no documented way to publish the host ingress at all. Its successor, `publishing.exposureClass`, is now in the reference: the four backends and what each renders, the guard that fails the render when a pool or cloud backend meets a non-empty `publishing.externalIPs`, and the KEP-5707 deprecation phasing (the later release numbers are approximate upstream, and the text says so rather than inventing precision). ## The third cert mode The Gateway API pages described a binary HTTP-01 / DNS-01 choice throughout — intro, controller responsibilities, listener layout, migration runbook, rollback, threat model and known limitations all assumed a per-tenant `Issuer` and `Certificate` always exist. A third mode ships and is selected by `publishing.certificates.wildcardSecretName`: the controller points listeners at an operator-supplied wildcard Secret, mints neither `Issuer` nor `Certificate`, and garbage-collects what it previously owned. It now has its own section, and the surrounding prose no longer contradicts it. Two consequences that were undocumented: - The ACME account private key outlives a mode switch. Only the `Issuer` referencing it is collected. On `next` a post-delete hook removes it when the gateway release goes away — not when the mode changes; in v1.5 nothing removes it at all. - The mode removes a safety net. On the ACME paths a hostname the tenant does not control simply fails to get a certificate. Here nothing is issued, nothing proves domain control, and the Gateway serves the operator Secret with whatever SANs it carries. ## What the option does to a tenant tree The Secret *name* — never the key material — rides the cluster values channel that every tenant inherits, so this is not root-scoped in practice even though root is the only supported configuration. Three cases, with different remedies: 1. An inheriting child gets a `*.<child-apex>` listener on the owner Gateway against the same Secret, so the SAN list must cover each child apex. 2. A child that owns a Gateway resolves the Secret in its own namespace — the controller renders `certificateRefs` without a namespace and issues no `ReferenceGrant`, so the reference never crosses namespaces — and the Secret must be replicated there. 3. On the default ingress path, a child running its own ingress controller gets neither: its apps drop their ACME annotation cluster-wide while `default-ssl-certificate` reaches only the publishing controller, so ingress-nginx serves its built-in self-signed certificate. Replicating the Secret does not help, because the flag is gated on the namespace. That is an open bug — cozystack/cozystack#3296 — and the docs say so rather than presenting it as a constraint to configure around. ## Verification Built with the Hugo version CI uses, in both environments — production (1491 pages) and development (1694 pages, the only one that renders `next/`, which production excludes). Every claim above was checked against `release-1.5` and `main` rather than inferred.
The traffic-path flowchart on the Gateway API page does not render. The edge label
CM -.->|issues Certificate(s)| GWcontains unquoted parentheses inside a pipe-delimited label, which Mermaid's parser rejects, so the whole flowchart fails — the diagram is simply absent from the page. Quoting the label fixes it and matches the convention already used by every other edge label in these docs (see the-->|"L2 (ARP) or BGP"| S1labels on the networking architecture page). The same line is present in both the v1.5 and the next snapshot, so both are fixed.Verified by rendering rather than by inspection: run through mermaid's own renderer, the current version throws and produces no SVG at all, while the quoted version produces the full diagram. A Hugo build cannot catch this — the fenced block is emitted verbatim and parsed client-side in the browser, so the build stays green either way.
Separately, the v1.5 platform-package reference is missing the row for
publishing.certificates.wildcardSecretName. The option genuinely ships in v1.5 and the v1.5 release blog post describes the feature, but the reference table jumps straight from the DNS-01 rows topublishing.proxyProtocol. The row is already present in the next docs, so only the v1.5 snapshot was stale. The wording is adapted from the next row, with the cross-reference topublishing.certificates.wildcardremoved — that key does not exist in v1.5.Summary by CodeRabbit
publishing.certificates.wildcardSecretName, detailing how to use a pre-existing operator wildcard TLS Secret instead of per-host ACME issuance.