From cd6a9b3a8f611b077a12e42c038588e59ffcacce Mon Sep 17 00:00:00 2001 From: Aleksei Sviridkin Date: Mon, 7 Sep 2026 00:52:34 +0300 Subject: [PATCH] fix(platform): correct what disabledPackages does to ouroboros MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both the publishing.proxyProtocol comment in values.yaml and the disable-hazard guard comment in the system bundle told operators that adding a package name to bundles.disabledPackages triggers the vendored ouroboros chart's pre-delete Corefile cleanup hook, alongside an explicit `kubectl delete package.cozystack.io cozystack.ouroboros`. It does not. A name in disabledPackages only makes the rendering helper in _helpers.tpl skip emitting the Package document, which is the same suppression that leaving proxyProtocol false already performs. Every emitted Package carries helm.sh/resource-policy: keep, so Helm does not prune the Package it has stopped rendering — that annotation was added precisely to stop disabling a package from deleting it. The Package CR survives, the HelmRelease it owns via ownerReference survives with it, ouroboros keeps running and keeps re-applying its rewrite block to kube-system/coredns, and the pre-delete hook never runs because nothing uninstalls the release. No platform values setting reaches that hook; the operator has to delete the Package CR. Anyone who followed the old advice was left with ouroboros still installed and, on the proxyProtocol false transition, still facing the render guard with no idea why. Pins the behaviour with a case asserting the guard still fails while cozystack.ouroboros sits in disabledPackages and the Package CR is live. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin --- .../platform/templates/bundles/system.yaml | 13 +++++++----- .../bundles_proxy_protocol_lookup_test.yaml | 20 +++++++++++++++++++ packages/core/platform/values.yaml | 12 +++++++++-- 3 files changed, 38 insertions(+), 7 deletions(-) diff --git a/packages/core/platform/templates/bundles/system.yaml b/packages/core/platform/templates/bundles/system.yaml index e5df54aaf..58ed369c0 100644 --- a/packages/core/platform/templates/bundles/system.yaml +++ b/packages/core/platform/templates/bundles/system.yaml @@ -186,11 +186,14 @@ present-on-cluster Package. ouroboros keeps running, the cleanup hook never fires, the rewrite block stays live. The operator must `kubectl delete package.cozystack.io - cozystack.ouroboros` (or add the package to - bundles.disabledPackages) for the chart to actually uninstall - and the hook to actually clean up. This guard fails the render - on the dangerous transition path so the operator confirms - they understand the asymmetry by setting + cozystack.ouroboros` for the chart to actually uninstall and + the hook to actually clean up. Adding the name to + bundles.disabledPackages is not a second path to that: it only + stops _helpers.tpl emitting the Package document, which is the + same suppression the flag flip already performs, and + resource-policy: keep leaves the live Package running. This + guard fails the render on the dangerous transition path so the + operator confirms they understand the asymmetry by setting publishing.proxyProtocolAcknowledgeUnclean: true. Known hole: an operator who manually `kubectl delete package cozystack.ouroboros` before flipping the flag back bypasses the diff --git a/packages/core/platform/tests/bundles_proxy_protocol_lookup_test.yaml b/packages/core/platform/tests/bundles_proxy_protocol_lookup_test.yaml index fc6cee8a1..a0751ee00 100644 --- a/packages/core/platform/tests/bundles_proxy_protocol_lookup_test.yaml +++ b/packages/core/platform/tests/bundles_proxy_protocol_lookup_test.yaml @@ -78,3 +78,23 @@ tests: path: metadata.annotations["helm.sh/resource-policy"] value: keep + # bundles.disabledPackages is not a route out of the guard. A name in + # that list only stops _helpers.tpl emitting the Package document; the + # guard keys off a lookup for the live Package CR, which the list + # never touches. An operator who reaches for disabledPackages instead + # of `kubectl delete package` still gets the failure, and ouroboros is + # still installed when they get it. + - it: still fails when cozystack.ouroboros is in bundles.disabledPackages but the Package CR is live + set: + bundles: + system: + enabled: true + variant: isp-full + disabledPackages: + - cozystack.ouroboros + publishing: + proxyProtocol: false + proxyProtocolAcknowledgeUnclean: false + asserts: + - failedTemplate: + errorPattern: "publishing.proxyProtocol was previously enabled.*helm.sh/resource-policy: keep leaves the existing Package" diff --git a/packages/core/platform/values.yaml b/packages/core/platform/values.yaml index e6ef9c9ad..afbc05647 100644 --- a/packages/core/platform/values.yaml +++ b/packages/core/platform/values.yaml @@ -252,8 +252,7 @@ publishing: # # kubectl delete package.cozystack.io cozystack.ouroboros # - # or adds the package name to bundles.disabledPackages. THAT is the - # path that triggers the chart's pre-delete Helm hook + # THAT is the path that triggers the chart's pre-delete Helm hook # (charts/ouroboros/templates/coredns-cleanup-hook.yaml, vendored at # 0.8.0) — the hook quiesces the controller, sed-strips the # BEGIN/END block from the live kube-system/coredns Corefile, and @@ -262,6 +261,15 @@ publishing: # the normal disable path. Documentation of the hook's exact # behaviour lives in the upstream chart's README. # + # bundles.disabledPackages does NOT do this. Putting this name in + # that list only makes templates/_helpers.tpl skip emitting the + # Package document — the same suppression as leaving proxyProtocol + # false — and resource-policy: keep then stops Helm pruning a + # Package it no longer renders. The Package CR stays, so does the + # HelmRelease it owns, ouroboros keeps running, and the pre-delete + # hook never fires. No platform values setting reaches that hook: + # the operator has to delete the Package CR. + # # Manual cleanup recipe (only if the chart's pre-delete hook fails # to land — controller pod stuck in CrashLoop blocking the quiesce # step, ConfigMap RBAC drift on the namespace, helm uninstall