Skip to content

Commit 2487afa

Browse files
lexfreilexfrei
committed
fix(ouroboros): address review pass 3
Four blockers from pass 3. 1. Tenant ouroboros no longer forces --gateway-api on. The cozy-ouroboros parent values.yaml drops the gatewayApi.enabled=true override; the bundle template injects it for the host (which always has Gateway-API CRDs cluster-wide), and the apps/kubernetes wrapper mirrors the per-tenant addons.gatewayAPI.enabled value via cozystack.defaultOuroborosValues. Without this, a tenant whose Kubernetes CR has addons.gatewayAPI.enabled=false (the default) would CrashLoopBackOff the controller on WaitForCacheSync against missing Gateway-API CRDs. New tests/ouroboros_test.yaml cases pin the gating in both directions. 2. hack/e2e-apps/ouroboros.bats now creates a TLS Ingress and polls the tenant kube-system/coredns-custom ConfigMap for a matching `rewrite name <host>` line within 180s. Pod-readiness on its own is true the moment the container runs (no probes on the upstream chart), so the previous test passed vacuously even when the controller never reconciled. The reconciliation assertion catches missing CRDs, narrow RBAC, gateway-api gate misfires — exactly the failure modes pass-3 review #1 just bit on. 3. publishing.proxyProtocol disable hazard now has a guard. Flipping the flag from true to false uninstalls ouroboros but leaves stale rewrites in the host Corefile. The bundle template now does a `lookup` for an existing cozystack.ouroboros Package CR; if it exists and the new flag is false, render fails with the explicit cleanup recipe and a hint to set publishing.proxyProtocolAcknowledgeUnclean: true once the Corefile has been hand-cleaned. Default of the acknowledge gate is false. Operators on a clean cluster (never enabled) are unaffected. (`feedback-flux-helm-lookup-digest` does not apply here — this is a platform-bundle template, not a HelmRelease.) 4. Added a Talos re-render note to the publishing.proxyProtocol comment: a Talos machine-config push will overwrite the BEGIN/END markers in kube-system/coredns Corefile, but the controller re-applies on next reconcile (≤10 minutes). Brief flap window during a Talos roll, acceptable but worth knowing. helm unittest: packages/system/ouroboros: 10/10 (controller test updated for new default) packages/apps/kubernetes: 109/109 (added 2 gateway-api gating cases) Assisted-By: Claude <[email protected]> Signed-off-by: Aleksei Sviridkin <[email protected]>
1 parent 4b70484 commit 2487afa

7 files changed

Lines changed: 154 additions & 13 deletions

File tree

‎hack/e2e-apps/ouroboros.bats‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,5 +76,55 @@ EOF
7676
wait pod --selector=app.kubernetes.io/component=controller \
7777
--timeout=5m --for=condition=ready
7878

79+
print_log "Step 5: create a TLS Ingress and assert the controller writes a rewrite into coredns-custom"
80+
# Pod-readiness is true the moment the container Runs, regardless of
81+
# whether the controller actually reconciles (no probes on the
82+
# upstream chart). To assert the addon does the thing it claims, we
83+
# create an Ingress with a TLS host and poll the import ConfigMap
84+
# for a matching rewrite line. Anything else (CRD missing,
85+
# Gateway-API gate misfiring, RBAC narrowed wrong) shows up here
86+
# rather than as a vacuously-passing pod-ready check.
87+
hairpin_host="hairpin-cozystack-e2e.example.invalid"
88+
KUBECONFIG="${kubeconfig_file}" kubectl --namespace default \
89+
apply --filename - <<EOF
90+
apiVersion: networking.k8s.io/v1
91+
kind: Ingress
92+
metadata:
93+
name: hairpin-probe
94+
spec:
95+
ingressClassName: nginx
96+
tls:
97+
- hosts:
98+
- ${hairpin_host}
99+
secretName: hairpin-probe-tls
100+
rules:
101+
- host: ${hairpin_host}
102+
http:
103+
paths:
104+
- path: /
105+
pathType: Prefix
106+
backend:
107+
service:
108+
name: hairpin-probe
109+
port:
110+
number: 80
111+
EOF
112+
113+
deadline=$(($(date +%s) + 180))
114+
while [[ $(date +%s) -lt ${deadline} ]]; do
115+
snippet=$(KUBECONFIG="${kubeconfig_file}" kubectl --namespace kube-system \
116+
get configmap coredns-custom \
117+
--output go-template='{{ index .data "ouroboros.override" }}' 2>/dev/null || true)
118+
if echo "${snippet}" | grep -q "rewrite name ${hairpin_host}"; then
119+
print_log " reconciliation observed: ${snippet}"
120+
break
121+
fi
122+
sleep 5
123+
done
124+
echo "${snippet}" | grep -q "rewrite name ${hairpin_host}" \
125+
|| { echo "ouroboros did not reconcile within 180s"; exit 1; }
126+
127+
KUBECONFIG="${kubeconfig_file}" kubectl --namespace default \
128+
delete ingress hairpin-probe --ignore-not-found 2>/dev/null || true
79129
rm -f "${kubeconfig_file}"
80130
}

‎packages/apps/kubernetes/templates/helmreleases/ouroboros.yaml‎

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,24 @@
11
{{- /*
2-
Tenant deployments override the host default mode=coredns back to
3-
coredns-import: tenants run cozystack-coredns (which adds the
4-
`import /etc/coredns/custom/*.override` directive to its Corefile),
5-
so the import-CM mode is the right shape inside the tenant — it
6-
keeps ouroboros out of contention with whatever Helm/Flux owner
7-
re-renders the tenant's coredns Corefile from values.
2+
Tenant deployments override the cozy-ouroboros parent defaults:
3+
4+
* mode flips from coredns to coredns-import — tenants run
5+
cozystack-coredns (which adds the `import /etc/coredns/custom/*.override`
6+
directive to its Corefile), so the import-CM mode keeps ouroboros
7+
out of contention with the chart that re-renders the tenant's
8+
Corefile from values.
9+
10+
* controller.gatewayApi.enabled mirrors the tenant's own
11+
addons.gatewayAPI.enabled value. Forcing --gateway-api on a
12+
tenant without Gateway-API CRDs would CrashLoopBackOff the
13+
controller on WaitForCacheSync (LIST against a non-registered
14+
GVR never marks the informer synced).
815
*/ -}}
916
{{- define "cozystack.defaultOuroborosValues" -}}
1017
ouroboros:
1118
controller:
1219
mode: coredns-import
20+
gatewayApi:
21+
enabled: {{ $.Values.addons.gatewayAPI.enabled }}
1322
{{- end }}
1423

1524
{{- if and .Values.addons.ouroboros.enabled (not .Values.addons.ingressNginx.enabled) }}

‎packages/apps/kubernetes/tests/ouroboros_test.yaml‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,3 +164,37 @@ tests:
164164
- equal:
165165
path: spec.values.ouroboros.controller.mode
166166
value: coredns
167+
168+
- it: should NOT enable --gateway-api when tenant addons.gatewayAPI.enabled is false (default)
169+
set:
170+
addons:
171+
ouroboros:
172+
enabled: true
173+
valuesOverride: {}
174+
ingressNginx:
175+
enabled: true
176+
hosts: []
177+
valuesOverride: {}
178+
gatewayAPI:
179+
enabled: false
180+
asserts:
181+
- equal:
182+
path: spec.values.ouroboros.controller.gatewayApi.enabled
183+
value: false
184+
185+
- it: should mirror tenant addons.gatewayAPI.enabled into ouroboros gatewayApi
186+
set:
187+
addons:
188+
ouroboros:
189+
enabled: true
190+
valuesOverride: {}
191+
ingressNginx:
192+
enabled: true
193+
hosts: []
194+
valuesOverride: {}
195+
gatewayAPI:
196+
enabled: true
197+
asserts:
198+
- equal:
199+
path: spec.values.ouroboros.controller.gatewayApi.enabled
200+
value: true

‎packages/core/platform/templates/bundles/system.yaml‎

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,8 +116,31 @@
116116

117117
# Common Packages
118118
{{include "cozystack.platform.package.default" (list "cozystack.cert-manager" $) }}
119+
{{- /* Disable hazard guard. The cozy-ouroboros chart in coredns mode
120+
does not have a cleanup hook for the inline Corefile rewrite block,
121+
so flipping proxyProtocol back to false uninstalls ouroboros but
122+
leaves stale rewrites in kube-system/coredns Corefile pointing at
123+
a deleted Service. Refuse to render unless the operator explicitly
124+
acknowledges they have cleaned the Corefile by hand (see the
125+
publishing.proxyProtocol comment in core/platform/values.yaml for
126+
the exact recipe). The lookup checks for an existing Package CR
127+
on the cluster — if proxyProtocol is now false but the Package
128+
used to be there, we are on the dangerous transition path. */}}
129+
{{- if and (not .Values.publishing.proxyProtocol) (not .Values.publishing.proxyProtocolAcknowledgeUnclean) -}}
130+
{{- $existingOuroboros := lookup "cozystack.io/v1alpha1" "Package" "" "cozystack.ouroboros" -}}
131+
{{- if $existingOuroboros -}}
132+
{{- fail "publishing.proxyProtocol was previously enabled (the cozystack.ouroboros Package CR exists on the cluster). Flipping it back to false uninstalls ouroboros but leaves a # === BEGIN ouroboros block in kube-system/coredns Corefile pointing at a deleted Service. Either: (a) restore publishing.proxyProtocol to true, or (b) run the cleanup recipe in core/platform/values.yaml (sed-strip + rollout restart) and set publishing.proxyProtocolAcknowledgeUnclean: true to confirm." -}}
133+
{{- end -}}
134+
{{- end -}}
119135
{{- if .Values.publishing.proxyProtocol }}
120-
{{include "cozystack.platform.package.default" (list "cozystack.ouroboros" $) }}
136+
{{- $ouroborosComponents := dict -}}
137+
{{- /* Host always has Gateway-API CRDs cluster-wide, so it's safe (and
138+
useful) to flip --gateway-api on. Tenants don't get this — the
139+
apps/kubernetes wrapper mirrors per-tenant addons.gatewayAPI.enabled
140+
into the same field. */ -}}
141+
{{- $ouroborosValues := dict "ouroboros" (dict "controller" (dict "gatewayApi" (dict "enabled" true))) -}}
142+
{{- $_ := set $ouroborosComponents "ouroboros" (dict "values" $ouroborosValues) -}}
143+
{{include "cozystack.platform.package" (list "cozystack.ouroboros" "default" $ $ouroborosComponents) }}
121144
{{- end }}
122145
{{include "cozystack.platform.package.default" (list "cozystack.flux-plunger" $) }}
123146
{{include "cozystack.platform.package.default" (list "cozystack.victoria-metrics-operator" $) }}

‎packages/core/platform/values.yaml‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,17 @@ publishing:
8888
# pieces in one go — ingress-nginx use-proxy-protocol and the
8989
# cozystack.ouroboros Package CR.
9090
#
91+
# TALOS RE-RENDER NOTE (host scope only): the host CoreDNS is a
92+
# Talos-managed Deployment; ouroboros mutates its live Corefile in
93+
# place between BEGIN/END markers. A Talos machine-config push or
94+
# upgrade re-renders the Corefile and overwrites the markers — there
95+
# is a flap window of one ouroboros reconcile loop (≤10 minutes by
96+
# default; less if any Ingress/Gateway event fires) until the
97+
# controller re-applies the block. In-cluster DNS for hairpinned
98+
# hostnames briefly returns the upstream LB IP instead of the proxy
99+
# ClusterIP during that window, which fails the same way pre-PR
100+
# behaviour fails. Acceptable, but worth knowing during a Talos roll.
101+
#
91102
# PRECONDITION: the upstream L4 load balancer / WAN edge in front of
92103
# ingress-nginx MUST already be injecting PROXY-protocol v1 headers
93104
# before flipping this flag. Without that, every external request
@@ -127,6 +138,16 @@ publishing:
127138
# --output yaml | kubectl apply --filename -
128139
# kubectl --namespace kube-system rollout restart deployment/coredns
129140
proxyProtocol: false
141+
# Acknowledgement gate for the disable hazard described above. Set
142+
# this to true ONLY after running the cleanup recipe (sed-strip
143+
# BEGIN/END markers from kube-system/coredns Corefile + rollout
144+
# restart deployment/coredns). Without this acknowledgement, flipping
145+
# publishing.proxyProtocol from true to false fails fast at platform
146+
# render time so an operator cannot silently break in-cluster DNS by
147+
# toggling a "single switch". The platform reads this only on the
148+
# transition path; once the cluster has been clean for one reconcile
149+
# cycle, the operator can flip it back to false.
150+
proxyProtocolAcknowledgeUnclean: false
130151
certificates:
131152
solver: http01 # "http01" or "dns01"
132153
issuerName: letsencrypt-prod

‎packages/system/ouroboros/tests/controller_test.yaml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,10 @@ tests:
2020
path: spec.template.spec.containers[0].args
2121
content: --proxy-fqdn=ouroboros-proxy.cozy-ouroboros.svc.cluster.local.
2222

23-
- it: enables Gateway-API watching by default (cozystack ships Gateway-API)
23+
- it: leaves Gateway-API watching off at the parent default (consumer enables)
2424
template: charts/ouroboros/templates/controller-deployment.yaml
2525
asserts:
26-
- contains:
26+
- notContains:
2727
path: spec.template.spec.containers[0].args
2828
content: --gateway-api
2929

‎packages/system/ouroboros/values.yaml‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,14 @@ ouroboros:
1616
# tenants do run cozystack-coredns, which DOES include the import
1717
# directive, so the import-CM mode works there.
1818
mode: coredns
19-
# cozystack ships Gateway-API CRDs and most operators expose hostnames via
20-
# Gateway/HTTPRoute as well as Ingress, so default to watching both.
21-
gatewayApi:
22-
enabled: true
19+
# gatewayApi watching is left at the upstream default (false) at the
20+
# parent level: the host scope flips it on via the bundle template
21+
# (cozystack ships Gateway-API CRDs cluster-wide on the host), but
22+
# tenants must NOT inherit it — tenant Gateway-API is gated behind
23+
# addons.gatewayAPI.enabled, and forcing --gateway-api on a tenant
24+
# without those CRDs would CrashLoopBackOff the controller on
25+
# WaitForCacheSync. The apps/kubernetes wrapper mirrors the tenant
26+
# addons.gatewayAPI.enabled value into this field.
2327
proxy:
2428
target:
2529
# cozystack deploys ingress-nginx into cozy-ingress-nginx in both

0 commit comments

Comments
 (0)