fix(templating): hooks conflicting with templates in post-renderers#32049
Conversation
pull-request-size
Bot
added
the
size/L
There was a problem hiding this comment.
Pull request overview
This PR restores Helm 3–style behavior for post-renderers by preventing hook resources from being merged into the same YAML stream as regular templates before post-rendering, avoiding duplicate-resource failures (e.g., with Kustomize) when hooks and templates overlap.
Changes:
- Split hook manifests and non-hook manifests into separate streams before running the post-renderer, then recombine the results.
- Add a regression test covering a duplicate resource declared both as a hook and as a normal template.
- Update an existing post-renderer merge-error test assertion to match the new error surfaced from manifest sorting/parsing.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| pkg/action/action.go | Post-render hooks and templates separately and merge outputs back into the file map before final sorting. |
| pkg/action/action_test.go | Add regression coverage for hook/template duplicate resources; adjust expected error substring for merge/parse failure. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Promptless prepared a documentation update related to this change. Triggered by PR #32049 Documents that in Helm 4, chart hooks are processed by post-renderers as separate streams from templates, allowing the same resource (like a ServiceAccount) to appear in both without conflict. |
matheuscscp
force-pushed
the
fix-flux2-5791
branch
from
2e18e1a to
bd4f519
Compare
matheuscscp
force-pushed
the
fix-flux2-5791
branch
from
bd4f519 to
49f9fe7
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
matheuscscp
force-pushed
the
fix-flux2-5791
branch
from
49f9fe7 to
d9caee7
Compare
matheuscscp
force-pushed
the
fix-flux2-5791
branch
from
d9caee7 to
ef8ac94
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
matheuscscp
force-pushed
the
fix-flux2-5791
branch
from
ef8ac94 to
5a491d5
Compare
matheuscscp
force-pushed
the
fix-flux2-5791
branch
from
5a491d5 to
2c92787
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Matheus Pimenta <[email protected]>
matheuscscp
force-pushed
the
fix-flux2-5791
branch
from
2c92787 to
d52a872
Compare
banjoh
left a comment
banjoh
left a comment
There was a problem hiding this comment.
Initial review of code looks good. I'll need to run some manual tests to validate the functionality won't cause regressions
|
When enabling This test shows an obvious flaw in streaming hooks and templates separately to a Kustomize post-renderer: patches targeting templates will error out in the hooks stream. Conclusion: We need to ship |
Signed-off-by: Matheus Pimenta <[email protected]>
pull-request-size
Bot
removed
the
size/L
pull-request-size
Bot
added
the
size/XL
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Promptless prepared a documentation update related to this change. Triggered by PR #32049 Updated documentation to clarify all three |
Resolves conflicts introduced by upstream's PostRenderStrategy work (helm#32049) and Go 1.26 / Kubernetes 1.36 / kstatus 1.1 bump (helm#32078) against the HIP-0025 sequencing changes. Conflict resolution: - pkg/action/action.go: combined upstream's three-way post-render strategy switch with the renderResources/renderResourcesWithFiles split from this branch; all error returns from renderResourcesWithFiles updated to the 5-tuple shape. - pkg/action/install.go: thread PostRenderStrategy through both the ordered-wait (renderResourcesWithFiles) and standard (renderResources) render paths. - pkg/action/upgrade.go: same thread-through, plus restore the 5-tuple cerr return. - pkg/action/uninstall.go: pass PostRenderStrategyCombined to the recovery path that re-renders manifests for sequenced uninstall. Also: - gofmt fixes for pkg/kube/readiness.go (strconv import indent; flagged by Copilot review) and pkg/action/rollback.go (struct field alignment). - Add post-rewrite contract tests for BuildSubchartDAG covering metadata-only-with-no-loaded-deps and annotation-references- unloaded-dep cases (locks in joejulian's review feedback). - .gitignore: ignore .mission/ tooling directory.
Fixes: #31985
Fixes: fluxcd/helm-controller#1449