Repository navigation
[system] Add Gateway API support via Cilium - #2208
Kirill Ilin (sircthulhu) wants to merge 3 commits into
Conversation
📝 WalkthroughWalkthroughAdds a new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
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 establishes the foundational infrastructure for integrating the Kubernetes Gateway API into Cozystack, paving the way for Envoy Gateway to potentially replace the existing NGINX Ingress Controller. It involves updating the core Gateway API CRDs and packaging Envoy Gateway as an optional component. These changes are purely additive and do not alter any current system behavior, providing a seamless path for future adoption of the Gateway API. Highlights
🧠 New Feature in Public Preview: You can now enable Memory 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. Changelog
Activity
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 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 counter productive. 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. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request successfully integrates Envoy Gateway as an optional platform package, upgrading Gateway API CRDs to v1.5.0 and enabling cert-manager support for Gateway API. The changes are well-structured, including dedicated package sources, Makefile updates for pulling the correct Helm chart version and removing conflicting CRDs, and appropriate value overrides. The introduction of a gateway.type field in platform values provides a clear path for future ingress/gateway-api switching. Overall, the implementation aligns with the stated objective of laying foundational infrastructure without altering existing behavior.
Aleksei Sviridkin (lexfrei)
left a comment
There was a problem hiding this comment.
LGTM
Reviewed the 9 project-specific files (PackageSources, bundle template, platform values, cert-manager values, Envoy Gateway wrapper chart + Makefile). Vendored upstream Envoy Gateway helm chart (30 files) spot-checked — standard upstream content.
Changes follow existing patterns correctly:
- Manual
hascheck in the bundle template matches the bootbox pattern - CRD deduplication done properly (removed
gatewayapi-crds.yamlfrom envoy-gateway chart, separategateway-api-crdspackage with v1.5.0 CRDs) - Dependency declaration in PackageSources is correct (
envoy-gatewaydepends ongateway-api-crds)
Two non-blocking observations for follow-up:
-
cert-manager startup ordering:
enableGatewayAPI: trueis set unconditionally, butcozystack.cert-managerhas nodependsOnforcozystack.gateway-api-crds. cert-manager v1.19.3 degrades gracefully (no crash), but the gateway-shim controller discovers CRDs only at startup — so Gateway API certificate automation won't function until cert-manager is restarted. Worth a follow-up when this feature moves to actual usage. -
gateway.typeis intentionally a no-op for now — prepares for future ingress/gateway-api switching. Nothing reads.Values.gateway.typeyet.
fa01af5 to
d147742
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/system/cert-manager/values.yaml`:
- Around line 1-3: The cert-manager values file currently hard-sets
config.enableGatewayAPI: true; change the default to false in
packages/system/cert-manager/values.yaml (set config.enableGatewayAPI to false)
and remove the hard-coded override so the platform bundle can control it;
instead wire this value from the platform bundle when gateway.gatewayAPI is true
(update the platform bundle templating to set
cert-manager.config.enableGatewayAPI only when gateway.gatewayAPI is enabled),
ensuring deployments that select cozystack.networking/noop or omit
gateway-api-crds keep cert-manager Gateway API support disabled by default.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2bed142a-7f80-4e75-815f-a870269f3c17
📒 Files selected for processing (6)
packages/core/platform/sources/networking.yamlpackages/core/platform/templates/bundles/system.yamlpackages/core/platform/values.yamlpackages/system/cert-manager/values.yamlpackages/system/gateway-api-crds/Makefilepackages/system/gateway-api-crds/templates/crds-experimental.yaml
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/core/platform/templates/bundles/system.yaml`:
- Around line 160-162: Currently the template overwrites external-dns.sources
when gateway.gatewayAPI is enabled; instead, read the existing
.Values.external-dns.sources (or default to an empty list), append
"gateway-httproute" if it isn't already present, ensure uniqueness, and then set
$externalDnsValues to use that merged list before calling set on
externalDnsComponents; reference the symbols .Values.gateway.gatewayAPI,
.Values.external-dns.sources, $externalDnsValues, and the set call that assigns
"external-dns" to externalDnsComponents to locate where to modify the template.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c8f4475a-d517-46ad-b82a-4ab0200649f3
📒 Files selected for processing (1)
packages/core/platform/templates/bundles/system.yaml
| {{- if .Values.gateway.gatewayAPI -}} | ||
| {{- $externalDnsValues := dict "external-dns" (dict "sources" (list "service" "ingress" "gateway-httproute")) -}} | ||
| {{- $_ := set $externalDnsComponents "external-dns" (dict "values" $externalDnsValues) -}} |
There was a problem hiding this comment.
Append gateway-httproute instead of replacing external-dns.sources.
Line 161 hardcodes the full external-dns.sources list when gateway.gatewayAPI is enabled. That will silently discard any existing external-dns source customization and changes behavior for clusters that already override this chart. Please merge gateway-httproute into the current sources list rather than overwriting it.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/core/platform/templates/bundles/system.yaml` around lines 160 - 162,
Currently the template overwrites external-dns.sources when gateway.gatewayAPI
is enabled; instead, read the existing .Values.external-dns.sources (or default
to an empty list), append "gateway-httproute" if it isn't already present,
ensure uniqueness, and then set $externalDnsValues to use that merged list
before calling set on externalDnsComponents; reference the symbols
.Values.gateway.gatewayAPI, .Values.external-dns.sources, $externalDnsValues,
and the set call that assigns "external-dns" to externalDnsComponents to locate
where to modify the template.
- Upgrade Gateway API CRDs from v1.2.0 to v1.5.0 (experimental channel) - Add gateway-api-crds as a component in all networking variants, with Cilium depending on it to ensure CRDs are available first - Add gateway.gatewayAPI platform config (enables Cilium gatewayAPI.enabled=true), can be used alongside gateway.ingress - Enable Gateway API support in cert-manager Co-Authored-By: Claude <[email protected]> Signed-off-by: Kirill Ilin <[email protected]>
When gateway.gatewayAPI is true, pass gateway-httproute source to the system external-dns package via components values override. Co-Authored-By: Claude <[email protected]> Signed-off-by: Kirill Ilin <[email protected]>
9c0783f to
7247edc
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (2)
packages/system/cert-manager/values.yaml (1)
1-3:⚠️ Potential issue | 🟠 MajorDon't hard-code Gateway API support in the chart defaults.
Line 3 enables cert-manager's Gateway API integration for every deployment, but the platform default still keeps
gateway.gatewayAPIoff and the system bundle does not pass a matching opt-in override when it installscozystack.cert-manager. That turns the feature on even for variants likeisp-hosted, where the networking bundle selectscozystack.networking/noopand never installsgateway-api-crds. Keep this defaultfalsehere and wire it from the platform bundle only whengateway.gatewayAPIis enabled.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/system/cert-manager/values.yaml` around lines 1 - 3, The chart default currently hard-codes cert-manager.config.enableGatewayAPI: true; change this default to false in the values.yaml (update cert-manager.config.enableGatewayAPI to false) and instead propagate the true value from the platform bundle only when the platform setting gateway.gatewayAPI is enabled; ensure the platform/system bundle that installs cozystack.cert-manager adds the opt-in override for cert-manager.config.enableGatewayAPI when gateway.gatewayAPI is true (so variants using cozystack.networking/noop and no gateway-api-crds keep the feature off).packages/core/platform/templates/bundles/system.yaml (1)
160-162:⚠️ Potential issue | 🟠 MajorAppend
gateway-httproute; don't replaceexternal-dns.sources.These lines write a fixed three-item source list whenever Gateway API is enabled, so any existing
external-dnssource customization gets dropped. The vendored chart consumes.Values.sourcesverbatim as--sourceflags, so this is a real behavior change. Mergegateway-httprouteinto the current list instead of resetting it.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/platform/templates/bundles/system.yaml` around lines 160 - 162, The template unconditionally overwrites external-dns sources when .Values.gateway.gatewayAPI is true; instead update the logic around $externalDnsValues and the set into $externalDnsComponents to merge "gateway-httproute" into the existing sources list (preserving any user-provided .Values.external-dns.sources). Implement by reading the current sources (.Values.external-dns.sources), ensure it's a list (default to empty list if nil), append "gateway-httproute" only if not already present, and then set $externalDnsValues (and call set $_ $externalDnsComponents "external-dns") using that merged list so you don't discard custom sources.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/core/platform/templates/bundles/system.yaml`:
- Around line 160-162: The template unconditionally overwrites external-dns
sources when .Values.gateway.gatewayAPI is true; instead update the logic around
$externalDnsValues and the set into $externalDnsComponents to merge
"gateway-httproute" into the existing sources list (preserving any user-provided
.Values.external-dns.sources). Implement by reading the current sources
(.Values.external-dns.sources), ensure it's a list (default to empty list if
nil), append "gateway-httproute" only if not already present, and then set
$externalDnsValues (and call set $_ $externalDnsComponents "external-dns") using
that merged list so you don't discard custom sources.
In `@packages/system/cert-manager/values.yaml`:
- Around line 1-3: The chart default currently hard-codes
cert-manager.config.enableGatewayAPI: true; change this default to false in the
values.yaml (update cert-manager.config.enableGatewayAPI to false) and instead
propagate the true value from the platform bundle only when the platform setting
gateway.gatewayAPI is enabled; ensure the platform/system bundle that installs
cozystack.cert-manager adds the opt-in override for
cert-manager.config.enableGatewayAPI when gateway.gatewayAPI is true (so
variants using cozystack.networking/noop and no gateway-api-crds keep the
feature off).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: de727b7b-9981-420b-bc96-d3b0011655f4
📒 Files selected for processing (6)
packages/core/platform/sources/networking.yamlpackages/core/platform/templates/bundles/system.yamlpackages/core/platform/values.yamlpackages/system/cert-manager/values.yamlpackages/system/gateway-api-crds/Makefilepackages/system/gateway-api-crds/templates/crds-experimental.yaml
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/system/gateway-api-crds/Makefile
- packages/core/platform/values.yaml
Aleksei Sviridkin (lexfrei)
left a comment
There was a problem hiding this comment.
Overall the approach is solid — CRD dependency ordering, conditional Cilium gatewayAPI injection, and external-dns source extension all follow existing Cozystack patterns.
One blocker and a few observations below.
| @@ -0,0 +1,3 @@ | |||
| cert-manager: | |||
| config: | |||
| enableGatewayAPI: true | |||
There was a problem hiding this comment.
enableGatewayAPI: true is set unconditionally for all platform variants. In the isp-hosted variant, networking uses noop (no components), so Gateway API CRDs are never installed. cert-manager is a common package deployed for all variants — with this setting it will attempt to start the gateway-shim controller without CRDs present, resulting in startup failures.
This should be conditional on gateway.gatewayAPI, similar to how Cilium's gatewayAPI flag is handled in system.yaml. Alternatively, cert-manager's values could be templated through system.yaml with component overrides (like external-dns).
| # Gateway configuration | ||
| gateway: | ||
| ingress: true | ||
| gatewayAPI: false |
There was a problem hiding this comment.
Observation (non-blocking): gateway.ingress: true is defined but not read by any template. If this is preparation for future ingress/gateway switching — consider adding a comment explaining the intent, or removing it until the consuming logic is implemented.
| {{- $enabled := default (list) .Values.bundles.enabledPackages -}} | ||
| {{- if and (has "cozystack.external-dns" $enabled) (not (has "cozystack.external-dns" $disabled)) -}} | ||
| {{include "cozystack.platform.package" (list "cozystack.external-dns" "default" $ $externalDnsComponents) }} | ||
| {{- end }} |
There was a problem hiding this comment.
Suggestion (non-blocking): The inline optional-package logic here duplicates cozystack.platform.package.optional.default from _helpers.tpl. This was necessary because the helper doesn't support $components. Consider extending the helper (e.g., cozystack.platform.package.optional.default.components) to keep this DRY — especially if more packages will need conditional component overrides in the future.
…ayAPI Previously enableGatewayAPI was hardcoded to true in cert-manager values, causing startup failures in isp-hosted variant where Gateway API CRDs are not installed. Now passed via component override only when gateway.gatewayAPI is enabled. Co-Authored-By: Claude <[email protected]> Signed-off-by: Kirill Ilin <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/core/platform/templates/bundles/system.yaml (1)
165-167:⚠️ Potential issue | 🟠 MajorMerge
gateway-httprouteinto the existingexternal-dns.sourceslist.Line 166 still hardcodes the full list, so enabling Gateway API drops any existing
external-dnssource customization instead of extending it. Please appendgateway-httprouteto the current sources and dedupe it rather than replacing the field outright.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/platform/templates/bundles/system.yaml` around lines 165 - 167, The template currently replaces external-dns.sources with a hardcoded list; instead, read the existing sources (e.g. .Values.external-dns.sources or the current value in $externalDnsComponents), append "gateway-httproute", deduplicate the list (uniq) and set that merged list into $externalDnsValues so you update $externalDnsComponents["external-dns"].values.sources rather than overwriting it; use Helm functions (append/uniq/default) to build the merged list and assign it to $externalDnsValues before calling set.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/core/platform/templates/bundles/system.yaml`:
- Around line 129-134: The template currently enables cert-manager Gateway API
unconditionally when .Values.gateway.gatewayAPI is true; change this so the
cert-manager Gateway API override (the $certManagerValues and the set into
$certManagerComponents for "cert-manager") is only applied when the bundle's
networking variant is a Cilium-based variant (e.g., .Values.networking.variant
== "cilium" or matches a list of cilium variants) to avoid enabling Gateway API
for noop networking; update the conditional around the $certManagerValues block
(the code that sets $certManagerComponents and the include
"cozystack.platform.package" usage for "cozystack.cert-manager") to check both
.Values.gateway.gatewayAPI and the networking variant before adding the
override.
---
Duplicate comments:
In `@packages/core/platform/templates/bundles/system.yaml`:
- Around line 165-167: The template currently replaces external-dns.sources with
a hardcoded list; instead, read the existing sources (e.g.
.Values.external-dns.sources or the current value in $externalDnsComponents),
append "gateway-httproute", deduplicate the list (uniq) and set that merged list
into $externalDnsValues so you update
$externalDnsComponents["external-dns"].values.sources rather than overwriting
it; use Helm functions (append/uniq/default) to build the merged list and assign
it to $externalDnsValues before calling set.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 37c8abdb-2623-4b57-ada5-5666e9f060ed
📒 Files selected for processing (3)
packages/core/platform/templates/bundles/system.yamlpackages/core/platform/values.yamlpackages/system/cert-manager/values.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/system/cert-manager/values.yaml
| {{- $certManagerComponents := dict -}} | ||
| {{- if .Values.gateway.gatewayAPI -}} | ||
| {{- $certManagerValues := dict "cert-manager" (dict "config" (dict "enableGatewayAPI" true)) -}} | ||
| {{- $_ := set $certManagerComponents "cert-manager" (dict "values" $certManagerValues) -}} | ||
| {{- end -}} | ||
| {{include "cozystack.platform.package" (list "cozystack.cert-manager" "default" $ $certManagerComponents) }} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python - <<'PY'
from pathlib import Path
import re
path = Path("packages/core/platform/sources/networking.yaml")
if not path.exists():
raise SystemExit(f"missing {path}")
lines = path.read_text().splitlines()
targets = {"noop", "kubeovn-cilium", "kubeovn-cilium-generic", "cilium", "cilium-generic", "cilium-kilo"}
for i, line in enumerate(lines):
m = re.match(r'^(\s*)-\s+name:\s+["\']?([^"\']+)["\']?\s*$', line)
if not m:
continue
name = m.group(2)
if name not in targets:
continue
indent = len(m.group(1))
j = i + 1
while j < len(lines):
m2 = re.match(r'^(\s*)-\s+name:\s+', lines[j])
if m2 and len(m2.group(1)) == indent:
break
j += 1
block = "\n".join(f"{n+1}:{lines[n]}" for n in range(i, j))
print(f"\n## variant: {name}")
print(block)
print(f"-- gateway-api-crds present: {'gateway-api-crds' in block}")
PYRepository: cozystack/cozystack
Length of output: 6947
Gate cert-manager Gateway API override by bundle variant.
Lines 130 enables cert-manager Gateway API mode for all bundles whenever gateway.gatewayAPI is true. However, the noop networking variant (used by isp-hosted) does not install gateway-api-crds. If isp-hosted is deployed with gateway.gatewayAPI: true, cert-manager will start and fail when it tries to reconcile the Gateway API resources, reintroducing the startup failure this PR fixes.
This override must be gated to apply only to bundles that use a Cilium-based networking variant.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/core/platform/templates/bundles/system.yaml` around lines 129 - 134,
The template currently enables cert-manager Gateway API unconditionally when
.Values.gateway.gatewayAPI is true; change this so the cert-manager Gateway API
override (the $certManagerValues and the set into $certManagerComponents for
"cert-manager") is only applied when the bundle's networking variant is a
Cilium-based variant (e.g., .Values.networking.variant == "cilium" or matches a
list of cilium variants) to avoid enabling Gateway API for noop networking;
update the conditional around the $certManagerValues block (the code that sets
$certManagerComponents and the include "cozystack.platform.package" usage for
"cozystack.cert-manager") to check both .Values.gateway.gatewayAPI and the
networking variant before adding the override.
|
Will be merged with #2213 |
Register a dedicated cozystack.gateway-api-crds PackageSource and wire it into the system bundle before networking/cert-manager load. Without this the host cluster has no gateway.networking.k8s.io CRDs, Cilium's Gateway API controller cannot bootstrap (no GatewayClass registered), and cert-manager's gateway-shim controller silently disables Gateway API certificate automation. - New PackageSource cozystack.gateway-api-crds → installs packages/system/gateway-api-crds into namespace cozy-gateway-api. - cozystack.networking (variants cilium, cilium-kilo, cilium-generic, kubeovn-cilium, kubeovn-cilium-generic) dependsOn cozystack.gateway-api-crds so Cilium only starts once the CRDs are present. - cozystack.cert-manager dependsOn cozystack.gateway-api-crds, closing the startup race that the #2208 discussion flagged — cert-manager's gateway-shim discovers CRDs at process start, so racing with CRD install left Gateway API solvers permanently disabled until the pod was restarted. - system bundle loads cozystack.gateway-api-crds unconditionally for every variant (isp-full, isp-hosted, isp-full-generic). Assisted-By: Claude <[email protected]> Signed-off-by: Aleksei Sviridkin <[email protected]>
Register a dedicated cozystack.gateway-api-crds PackageSource and wire it into the system bundle before networking/cert-manager load. Without this the host cluster has no gateway.networking.k8s.io CRDs, Cilium's Gateway API controller cannot bootstrap (no GatewayClass registered), and cert-manager's gateway-shim controller silently disables Gateway API certificate automation. - New PackageSource cozystack.gateway-api-crds → installs packages/system/gateway-api-crds into namespace cozy-gateway-api. - cozystack.networking (variants cilium, cilium-kilo, cilium-generic, kubeovn-cilium, kubeovn-cilium-generic) dependsOn cozystack.gateway-api-crds so Cilium only starts once the CRDs are present. - cozystack.cert-manager dependsOn cozystack.gateway-api-crds, closing the startup race that the #2208 discussion flagged — cert-manager's gateway-shim discovers CRDs at process start, so racing with CRD install left Gateway API solvers permanently disabled until the pod was restarted. - system bundle loads cozystack.gateway-api-crds unconditionally for every variant (isp-full, isp-hosted, isp-full-generic). Assisted-By: Claude <[email protected]> Signed-off-by: Aleksei Sviridkin <[email protected]>
Register a dedicated cozystack.gateway-api-crds PackageSource and wire it into the system bundle before networking/cert-manager load. Without this the host cluster has no gateway.networking.k8s.io CRDs, Cilium's Gateway API controller cannot bootstrap (no GatewayClass registered), and cert-manager's gateway-shim controller silently disables Gateway API certificate automation. - New PackageSource cozystack.gateway-api-crds → installs packages/system/gateway-api-crds into namespace cozy-gateway-api. - cozystack.networking (variants cilium, cilium-kilo, cilium-generic, kubeovn-cilium, kubeovn-cilium-generic) dependsOn cozystack.gateway-api-crds so Cilium only starts once the CRDs are present. - cozystack.cert-manager dependsOn cozystack.gateway-api-crds, closing the startup race that the #2208 discussion flagged — cert-manager's gateway-shim discovers CRDs at process start, so racing with CRD install left Gateway API solvers permanently disabled until the pod was restarted. - system bundle loads cozystack.gateway-api-crds unconditionally for every variant (isp-full, isp-hosted, isp-full-generic). Assisted-By: Claude <[email protected]> Signed-off-by: Aleksei Sviridkin <[email protected]>
Register a dedicated cozystack.gateway-api-crds PackageSource and wire it into the system bundle before networking/cert-manager load. Without this the host cluster has no gateway.networking.k8s.io CRDs, Cilium's Gateway API controller cannot bootstrap (no GatewayClass registered), and cert-manager's gateway-shim controller silently disables Gateway API certificate automation. - New PackageSource cozystack.gateway-api-crds → installs packages/system/gateway-api-crds into namespace cozy-gateway-api. - cozystack.networking (variants cilium, cilium-kilo, cilium-generic, kubeovn-cilium, kubeovn-cilium-generic) dependsOn cozystack.gateway-api-crds so Cilium only starts once the CRDs are present. - cozystack.cert-manager dependsOn cozystack.gateway-api-crds, closing the startup race that the #2208 discussion flagged — cert-manager's gateway-shim discovers CRDs at process start, so racing with CRD install left Gateway API solvers permanently disabled until the pod was restarted. - system bundle loads cozystack.gateway-api-crds unconditionally for every variant (isp-full, isp-hosted, isp-full-generic). Assisted-By: Claude <[email protected]> Signed-off-by: Aleksei Sviridkin <[email protected]>
Register a dedicated cozystack.gateway-api-crds PackageSource and wire it into the system bundle before networking/cert-manager load. Without this the host cluster has no gateway.networking.k8s.io CRDs, Cilium's Gateway API controller cannot bootstrap (no GatewayClass registered), and cert-manager's gateway-shim controller silently disables Gateway API certificate automation. - New PackageSource cozystack.gateway-api-crds → installs packages/system/gateway-api-crds into namespace cozy-gateway-api. - cozystack.networking (variants cilium, cilium-kilo, cilium-generic, kubeovn-cilium, kubeovn-cilium-generic) dependsOn cozystack.gateway-api-crds so Cilium only starts once the CRDs are present. - cozystack.cert-manager dependsOn cozystack.gateway-api-crds, closing the startup race that the #2208 discussion flagged — cert-manager's gateway-shim discovers CRDs at process start, so racing with CRD install left Gateway API solvers permanently disabled until the pod was restarted. - system bundle loads cozystack.gateway-api-crds unconditionally for every variant (isp-full, isp-hosted, isp-full-generic). Assisted-By: Claude <[email protected]> Signed-off-by: Aleksei Sviridkin <[email protected]>
Register a dedicated cozystack.gateway-api-crds PackageSource and wire it into the system bundle before networking/cert-manager load. Without this the host cluster has no gateway.networking.k8s.io CRDs, Cilium's Gateway API controller cannot bootstrap (no GatewayClass registered), and cert-manager's gateway-shim controller silently disables Gateway API certificate automation. - New PackageSource cozystack.gateway-api-crds → installs packages/system/gateway-api-crds into namespace cozy-gateway-api. - cozystack.networking (variants cilium, cilium-kilo, cilium-generic, kubeovn-cilium, kubeovn-cilium-generic) dependsOn cozystack.gateway-api-crds so Cilium only starts once the CRDs are present. - cozystack.cert-manager dependsOn cozystack.gateway-api-crds, closing the startup race that the #2208 discussion flagged — cert-manager's gateway-shim discovers CRDs at process start, so racing with CRD install left Gateway API solvers permanently disabled until the pod was restarted. - system bundle loads cozystack.gateway-api-crds unconditionally for every variant (isp-full, isp-hosted, isp-full-generic). Assisted-By: Claude <[email protected]> Signed-off-by: Aleksei Sviridkin <[email protected]>
Register a dedicated cozystack.gateway-api-crds PackageSource and wire it into the system bundle before networking/cert-manager load. Without this the host cluster has no gateway.networking.k8s.io CRDs, Cilium's Gateway API controller cannot bootstrap (no GatewayClass registered), and cert-manager's gateway-shim controller silently disables Gateway API certificate automation. - New PackageSource cozystack.gateway-api-crds → installs packages/system/gateway-api-crds into namespace cozy-gateway-api. - cozystack.networking (variants cilium, cilium-kilo, cilium-generic, kubeovn-cilium, kubeovn-cilium-generic) dependsOn cozystack.gateway-api-crds so Cilium only starts once the CRDs are present. - cozystack.cert-manager dependsOn cozystack.gateway-api-crds, closing the startup race that the #2208 discussion flagged — cert-manager's gateway-shim discovers CRDs at process start, so racing with CRD install left Gateway API solvers permanently disabled until the pod was restarted. - system bundle loads cozystack.gateway-api-crds unconditionally for every variant (isp-full, isp-hosted, isp-full-generic). Assisted-By: Claude <[email protected]> Signed-off-by: Aleksei Sviridkin <[email protected]>
…2470) ## What this PR does Replaces ingress-nginx as the default ingress layer for cozystack-native services with **Cilium Gateway API**, materialised per-tenant via a new `TenantGateway` CRD reconciled by `cozystack-controller`. Supersedes #2213 / #2208. Stacked on top of #2468. ## Architecture The chart renders one `gateway.cozystack.io/v1alpha1 TenantGateway` CR per tenant that owns its own Gateway. The cozystack-controller reconciles the actual `Gateway`, per-tenant `Issuer`, `Certificate` (wildcard or per-listener), and HTTP→HTTPS redirect `HTTPRoute` from there. Helm does not render `Gateway` or `Certificate` directly — that prevents the Helm-vs-controller race on `Gateway.spec.listeners` that route-driven listener additions would otherwise cause. ``` ┌── TenantGateway CR (rendered by extra/gateway chart for owning tenants) │ └──▶ cozystack-controller reconciles: ├─ Gateway (with dynamic listeners, label-based allowedRoutes) ├─ Issuer (per-tenant ACME account) ├─ Certificate(s) — wildcard + per-child SANs in DNS-01, per-listener in HTTP-01 ├─ HTTPRoute (controller-owned http→https redirect) ├─ Namespace label patching (cozy-* system namespaces from spec.AttachedNamespaces) └─ Watches HTTPRoute / TLSRoute attachments to materialise listeners ``` ## Inheritance: tenants attach by default, opt into their own Gateway A tenant gets a dedicated Gateway (own LB Service, own LB IP, own Certificate) only when it explicitly asks via `tenant.spec.gateway=true`. Every other tenant in the tree publishes through the Gateway of the nearest ancestor that owns one — same shape as the existing `_namespace.ingress` inheritance. The `apps/tenant` chart writes a `namespace.cozystack.io/gateway` label on each tenant namespace, carrying either the tenant's own name (when owning a Gateway) or the inherited ancestor name (when inheriting). The Gateway's listener `allowedRoutes.namespaces.selector` is keyed on that label, so the same selector admits routes from every namespace pointing at the owner — descendants and cozy-* system namespaces alike. cozystack-controller patches the same label onto every namespace in `tgw.Spec.AttachedNamespaces` and garbage-collects labels it wrote when an entry is removed from the attach list (annotation-gated GC so labels written by the apps/tenant chart are never stripped). In DNS-01 mode, the controller extends the parent's wildcard Certificate with `<child-apex>` + `*.<child-apex>` SANs per inheriting child, and adds a `*.<child-apex>` HTTPS listener per child apex referencing the same cert. Without this expansion the parent's single-level wildcard cannot match a child route's hostname (`harbor.alice.example.org` is two labels past the parent's `*.example.org`). Opt-out into own Gateway is the right choice when: a tenant needs its own LB IP (DNS pinned, firewall rule), the apex is not derived from the parent (custom `host` value the ancestor's cert can't cover), or the tenant wants its own ACME account / cert authority. Otherwise leave `gateway` unset and inherit. ## End-to-end flow 1. Platform: `gateway.enabled=true` toggles cert-manager solver, switches per-system-app templates from Ingress to HTTPRoute / TLSRoute. 2. Per tenant: explicit `tenant.spec.gateway=true` renders a `TenantGateway` CR and triggers the controller to materialise the owner Gateway. Tenants with the field unset inherit through `_namespace.gateway` propagation in the tenant chart. 3. Adding a published app under any tenant (owner or inheriting) is just deploying its HTTPRoute. The controller picks up the hostname, adds a per-listener Certificate (HTTP-01) or extends the wildcard cert's SANs (DNS-01), and updates the owner Gateway's listeners. Defaults stay legacy: `gateway.enabled=false`, `tenant.spec.gateway=false`. Existing clusters upgrade unchanged. ## External IP allocation — mechanism-agnostic The per-tenant Gateway's auto-created `LoadBalancer` Service draws its IP from whatever LB allocator the cluster admin has configured at the platform layer — same shape as ingress-nginx today. The tenant API stays mechanism-agnostic — no `gatewayIP` field, no allocator-specific manifest in the tenant chart. Cozystack itself ships MetalLB installed but does not render any `IPAddressPool` / `L2Advertisement` / `BGPAdvertisement` from this chart; admins set up the allocator that suits their environment (MetalLB pool with L2 / BGP, Cilium LB-IPAM with announcer, robotlb against a cloud provider, or `Service.spec.externalIPs` pinning). If a tenant needs a specific address (DNS already pinned, firewall rule, etc.), the operator pre-allocates it on the admin side: either pre-create the Service with `loadBalancerIP` set, or hand the tenant a reference to a named admin-managed pool. Per-Service IP uniqueness is the allocator's responsibility — same as for any other LoadBalancer Service. ## Cert mode: HTTP-01 (default) vs DNS-01 (opt-in) `publishing.certificates.solver` selects how the controller sources TLS certs. **HTTP-01 (default)** — per-app HTTPS listener + per-app Certificate. New apps require zero platform-side config: deploy the HTTPRoute, controller does the rest. Inheriting children's hostnames pick up per-listener certs the same way the owner's own apps do. **DNS-01 (opt-in)** — single wildcard Certificate (`<apex>` + `*.<apex>`) covering every published app under the apex, plus per-child SANs (`<child-apex>` + `*.<child-apex>`) for every inheriting tenant. Pick this for clusters where many apps share the apex and Let's Encrypt rate limits matter. The DNS provider account must be able to write TXT records under every apex level the parent serves — for deeply-nested inherited children that requires zone delegation or a wide-scope provider credential. | `publishing.certificates.dns01.provider` | Required `publishing.certificates.dns01.<provider>` keys | | --- | --- | | `cloudflare` (default) | `cloudflare.secretName`, `cloudflare.secretKey` | | `route53` | `route53.region`, `route53.secretName` (and `accessKeyID` if not on IRSA) | | `digitalocean` | `digitalocean.secretName` | | `rfc2136` | `rfc2136.nameserver`, `rfc2136.tsigKeyName`, `rfc2136.secretName` | The platform chart wires every provider's keys into `_cluster.dns01-*`; the per-tenant gateway chart and the cluster-wide `cluster-issuers.yaml` both read them. ## Strengthens admission coverage on apps.cozystack.io/* The `cozystack-tenant-host-policy` probe in `gateway.bats` initially looked like a flake — the apply that was supposed to be VAP-rejected just succeeded. Investigation traced it to the custom REST handler at `pkg/registry/apps/application/rest.go`. genericapiserver hands every storage method a callback into the validating admission chain: `createValidation` for Create, `updateValidation` for Update, `deleteValidation` for Delete. `Update` was correctly invoking its callback since 23e399b, but `Create` and `Delete` accepted the parameter and never called it — admission was silently bypassed for both verbs on every `apps.cozystack.io` resource (Tenant, MariaDB, Postgres, Kubernetes, …). This PR wires the missing `createValidation` and `deleteValidation` calls into Create and Delete. Every ValidatingAdmissionPolicy and ValidatingWebhook against `apps.cozystack.io/*` now fires on all three verbs as the API contract requires. Per KEP-3488 aggregated APIServers are responsible for enforcing admission on their own kinds; cozystack-api is configured to do so via `genericoptions.NewRecommendedOptions`, the only thing missing was the explicit `*Validation` callbacks in the custom REST. ## Security — five guards, grouped by what they defend Tenants in Cozystack interact with the platform exclusively through `apps.cozystack.io/*` resources (Tenant, Bucket, Kubernetes, …) served by `cozystack-api`. Tenant RBAC (`cozy:tenant:*` aggregated to a RoleBinding in the tenant's own namespace) does not grant write access to `gateway.networking.k8s.io/*`, core `Namespaces`, or `cozystack.io/Package`. The protections below split into three groups by who they defend against: **Tenant-user-input gates.** Layer 4 (`cozystack-tenant-host-policy`). `Tenant.spec.host` is the user-supplied field that surfaces as a security boundary at the hostname layer; gated on every Create / Update via `cozystack-api`'s admission chain. **Defense-in-depth.** Layers 1, 2, 5, 7. These do not protect against tenant-user input (tenants don't hold the relevant RBAC). They guard against bugs in cozystack-controller / Flux, supply-chain compromise of an app chart that emits Gateway API resources, and confused-deputy mistakes by a cluster admin. Fail-closed via `failurePolicy: Fail` + `validationActions: [Deny]`. The five layers themselves: 1. **Listener `allowedRoutes` label selector** — `namespace.cozystack.io/gateway = <owner-tenant-name>` (controller-written for cozy-* via `AttachedNamespaces`, Helm-written for tenant namespaces via the tenant chart, both feeding into the same Gateway selector). 2. **`cozystack-gateway-hostname-policy`** — VAP on `Gateway` CREATE/UPDATE. Restricts listener hostnames to the namespace's `namespace.cozystack.io/host` apex. 4. **`cozystack-tenant-host-policy`** — VAP on `Tenant` CREATE/UPDATE. 5. **`cozystack-namespace-host-label-policy`** — VAP on core `Namespace` CREATE/UPDATE. Immutability gate on the host label. 7. **`cozystack-route-hostname-policy`** — VAP on `HTTPRoute` and `TLSRoute` CREATE/UPDATE. Closes the hostname-hijack vector across namespaces under inheritance — a child cannot claim a hostname outside its own apex. The previous Layer 3 (`cozystack-gateway-attached-namespaces-policy`) and Layer 6 (render-time `fail` in cozystack-basics) banned `tenant-*` entries in `publishing.gateway.attachedNamespaces` on the assumption that the attach list was the hijack vector. Under inheritance the attach surface is the label selector, not the static list; the hijack vector is closed by Layers 4/5/7 independently. Both Layer 3 and Layer 6 are dropped — kept in numbering above for continuity with the earlier review threads. ## Foreign-takeover guards Five reconcile paths refuse to silently rewrite a pre-existing object that shares the controller-derived name but carries no `OwnerReference` back to the TenantGateway: `Gateway`, redirect `HTTPRoute`, per-tenant `Issuer`, wildcard `Certificate`, per-listener `Certificate`. An operator who hand-pinned a Certificate or Issuer at the controller's derived name (private CA, manual cert pinning) gets an explicit `Ready=False/ReconcileError` condition instead of having their config silently destroyed and the resource re-issued from a different ACME account. The namespace-label patching path applies the same discipline: cozystack-controller only writes / strips `namespace.cozystack.io/gateway` on namespaces it annotates with `cozystack.io/gateway-attached-by`. Labels written by the apps/tenant chart (no annotation) are never touched, so inheritance for tenant namespaces survives every reconcile. ## Tests - **Go controller tests** — 50+ cases in `internal/controller/tenantgateway/`: idempotency, OwnerReference cascade, multi-parentRef status, HTTP-01↔DNS-01 mode-transition cert lifecycle, foreign-takeover refusals (Gateway / HTTPRoute / Issuer / Certificate / per-listener Certificate), DNS-01 provider matrix (cloudflare / route53 / digitalocean / rfc2136), HTTPS Kinds=[HTTPRoute] restriction, fail-closed CEL on missing host label, status condition wiring. Plus, for inheritance: label-based allowedRoutes selector shape, namespace label patch + garbage-collect via owner annotation, Helm-owned label safety, DNS-01 wildcard SAN expansion per inheriting child, per-child wildcard listener materialisation. - **Admission-chain tests** — `pkg/registry/apps/application/rest_admission_test.go` pins `createValidation`, `updateValidation`, `deleteValidation` paths via sentinel callbacks. - **helm-unittest** across `extra/gateway` (TenantGateway CR rendering, both modes), `core/platform` (DNS-01 wiring), `apps/tenant` (gateway-effective decision matrix including inheritance), `system/cozystack-basics` (three VAPs + Layer 7 VAP + tenant-root namespace label including the conditional `namespace.cozystack.io/gateway: tenant-root` + cozystack-values-secret `_namespace.gateway` propagation), `system/cert-manager-issuers` (full DNS-01 provider matrix), `system/cozystack-api` and `system/dashboard` (Ingress↔TLSRoute/HTTPRoute toggle), `system/monitoring` (Grafana / Alerta Ingress↔HTTPRoute toggle). - **e2e bats** — `hack/e2e-apps/gateway.bats` (17 cases) covering GatewayClass Accepted, Gateway Programmed + LB Service, HTTPRoute Accepted with status, VAP rejects cross-tenant hostname hijack, VAP rejects `tenant.spec.host` writes from non-trusted SA (regression-detector for the admission-chain fix), VAP rejects `namespace.cozystack.io/host` label changes from non-trusted SA, route-hostname VAP rejects HTTPRoute hostnames outside the apex, and three new cases for inheritance — Package admission accepts `tenant-*` in `attachedNamespaces` (regression guard for the dropped ban), child tenant without explicit gateway inherits `_namespace.gateway` from a Gateway-owning parent in lockstep across the namespace label and the cozystack-values Secret, and the cross-namespace attach end-to-end where a child tenant's HTTPRoute reaches Accepted on the parent's Gateway via the inheritance label. - **CI wiring** — `make test-controllers` target invoked from `pull-requests.yaml` runs `go test ./internal/...`. ## Backward compatibility The only always-on change is the flip of `cilium.envoy.enabled` + `cilium.gatewayAPI.enabled` to `true` (extra `cilium-envoy` DaemonSet, ~100 MB RAM/node idle). Everything else is gated behind `gateway.enabled=false` / `tenant.spec.gateway=false`. Existing clusters see no behavioural change until an operator explicitly opts in. The cozystack-api admission-chain fix is unconditional. Existing operators who do not define any VAP / admission webhook on `apps.cozystack.io` resources see no observable change. Operators who DO have their own webhooks/VAPs against these kinds will start seeing them fire on Create and Delete (as intended) — review your policies before merging if you maintain custom admission for `apps.cozystack.io/*`. Operators who set `publishing.gateway.attachedNamespaces` to include `tenant-*` entries used to hit a render-time fail or an admission rejection — now the apply succeeds and the listed tenant namespaces simply pick up the gateway-attach label alongside the cozy-* system namespaces. This matches the inheritance model and was kvaps' explicit ask in the May 25 review. ## Release note ```release-note - Adds Gateway API support via Cilium, materialised per-tenant via the new `gateway.cozystack.io/v1alpha1 TenantGateway` CRD reconciled by `cozystack-controller`. Opt-in via `publishing.gateway.enabled=true` at the platform level; per-tenant opt-in via `tenant.spec.gateway=true` for dedicated Gateway / LB IP, otherwise the tenant inherits the nearest ancestor's Gateway via a label-based `allowedRoutes` selector (same shape as the existing `_namespace.ingress` inheritance). Includes HTTP-01 (default) and DNS-01 (opt-in, with cloudflare / route53 / digitalocean / rfc2136 providers) cert solvers; DNS-01 extends the parent's wildcard Certificate with per-child apex SANs and adds a `*.<child-apex>` listener per inheriting tenant. Tenant API is mechanism-agnostic for IP allocation — the LoadBalancer Service draws its IP from the cluster's admin-configured allocator (MetalLB / Cilium LB-IPAM / robotlb / externalIPs). - Strengthens admission coverage on `apps.cozystack.io/*` resources: `cozystack-api` now invokes `createValidation` and `deleteValidation` callbacks on Create and Delete (Update was already wired). Operators with custom ValidatingAdmissionPolicies or webhooks targeting `apps.cozystack.io/*` will see them fire on all three verbs as the API contract requires; review your policies before merging if you maintain such admission. ```
What this PR does
First step toward Gateway API support in Cozystack as a future replacement for the archived NGINX Ingress Controller.
This PR adds the foundational infrastructure only — no existing behavior is changed:
gateway-api-crdsas a component in all networking variants, with Cilium depending on it to ensure CRDs are available before Cilium starts watching Gateway API resourcesgateway.gatewayAPIplatform config flag — when enabled, Cilium is configured withgatewayAPI.enabled=truegateway.ingressandgateway.gatewayAPIcan be enabled simultaneouslyenableGatewayAPI: true)To enable Gateway API alongside existing ingress:
Release note
Summary by CodeRabbit
New Features
Chores