fix(falcon-sensor): exclude release namespace from pullSecrets.allNamespaces loop - #534
Open
venu1202 wants to merge 1 commit into
Open
fix(falcon-sensor): exclude release namespace from pullSecrets.allNamespaces loop#534venu1202 wants to merge 1 commit into
venu1202 wants to merge 1 commit into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #533 —
container.image.pullSecrets.allNamespacesrenders two Secret manifests with the same name and namespace: the unconditional base Secret (always placed in the release's namespace), and a duplicate of it via theallNamespaceslookuploop, which enumerates every namespace on the cluster with no exclusion for the release's own namespace.Root cause
lookup "v1" "Namespace" "" ""returns every namespace, including the one this chart is installed into — which already gets a Secret from the unconditional block earlier in this same template. That produces two manifest entries with an identical{kind, namespace, name}key.Impact
Helm's
pkg/kube/client.goprocesses the second occurrence of that key by checking whether it's present inoriginal(the previous release revision's tracked manifest). On the first upgrade that ever introduces this configuration, it isn't — so Helm v3 SDK clients (anything built onhelm.sh/helm/v3, e.g.terraform-provider-helm) fail with:If
atomic: trueis set, this rolls back to the pre-secret state every time, so every retry hits the identical first-time-introduction failure — the release can never progress past it with this exact config. (Helm v4 CLI instead logs a warning and adopts the resource using cluster state as a baseline, which is why this wasn't caught in ad-hoc CLI testing.)Fix
Exclude the release's own namespace from the
allNamespacesloop:include "falcon-sensor.namespace" .resolves to wherever the release is actually installed (defaultfalcon-system, or a custom namespace if overridden), so this excludes exactly the namespace already covered by the unconditional base Secret — no other namespace is affected.Testing
helm templatebefore/after confirms the duplicatefalcon-systementry is gone, while all other requested namespaces still receive the Secret as before.container.image.pullSecrets.allNamespacesrenders a duplicate Secret in the release's own namespace, breaking Helm v3-SDK clients on first introduction #533: a release introducingpullSecrets.enable: true+allNamespaces: truefor the first time via a Helm v3 SDK client, withatomic: true, now upgrades successfully instead of failing/rolling back.