From 2c025533a0938c10610189a7f6e14fb410d609cd Mon Sep 17 00:00:00 2001 From: Rohit Gudi <50377477+caretak3r@users.noreply.github.com> Date: Sun, 3 May 2026 15:04:29 -0400 Subject: [PATCH] fix(hip-0025): tighten custom-readiness gate and add ordered-template fallback Two more findings from Copilot's review of the consolidated commit: - batchHasCustomReadiness returned true when a single resource had only ONE of the two readiness annotations. That triggered the custom-readiness status reader for the whole batch, redundant with warnIfPartialReadinessAnnotations (which already warns) and with EvaluateCustomReadiness (which falls back to kstatus when only one is set). Require both annotations to be non-empty before swapping in the custom reader. - helm template --wait=ordered hard-failed on YAML parse errors, breaking the --debug contract that "we always want to print the YAML, even if it is not valid." Fall back to the flat output path with a stderr warning when ordered rendering fails, so --debug investigation flows still surface the manifests. Signed-off-by: Rohit Gudi <50377477+caretak3r@users.noreply.github.com> --- pkg/action/sequencing.go | 7 ++++++- pkg/cmd/template.go | 25 ++++++++++++++++--------- 2 files changed, 22 insertions(+), 10 deletions(-) diff --git a/pkg/action/sequencing.go b/pkg/action/sequencing.go index 102e30649..d290d0fd2 100644 --- a/pkg/action/sequencing.go +++ b/pkg/action/sequencing.go @@ -223,6 +223,11 @@ func warnIfIsolatedGroups(logger *slog.Logger, result releaseutil.ResourceGroupR } } +// batchHasCustomReadiness reports whether at least one resource in the batch has +// BOTH the success and failure readiness annotations set. Resources with only +// one annotation are treated as misconfigured (warnIfPartialReadinessAnnotations +// emits a warning) and fall back to kstatus, so they should not trigger the +// custom-readiness status reader for the whole batch. func batchHasCustomReadiness(manifests []releaseutil.Manifest) bool { for _, manifest := range manifests { if manifest.Head == nil || manifest.Head.Metadata == nil { @@ -232,7 +237,7 @@ func batchHasCustomReadiness(manifests []releaseutil.Manifest) bool { if annotations == nil { continue } - if annotations[kube.AnnotationReadinessSuccess] != "" || annotations[kube.AnnotationReadinessFailure] != "" { + if annotations[kube.AnnotationReadinessSuccess] != "" && annotations[kube.AnnotationReadinessFailure] != "" { return true } } diff --git a/pkg/cmd/template.go b/pkg/cmd/template.go index ca9dd69fa..5e7086b4e 100644 --- a/pkg/cmd/template.go +++ b/pkg/cmd/template.go @@ -124,19 +124,26 @@ func newTemplateCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { // We ignore a potential error here because, when the --debug flag was specified, // we always want to print the YAML, even if it is not valid. The error is still returned afterwards. if rel != nil { + orderedRendered := false if orderedTemplateOutput { - if err := renderOrderedTemplate(rel.Chart, strings.TrimSpace(rel.Manifest), out); err != nil { - return err - } - if !client.DisableHooks { - for _, m := range rel.Hooks { - if skipTests && isTestHook(m) { - continue + if renderErr := renderOrderedTemplate(rel.Chart, strings.TrimSpace(rel.Manifest), out); renderErr != nil { + // Honor the --debug contract: always print the manifests, even if + // ordered rendering fails (e.g., a document fails YAML structural + // parsing). Fall back to the flat path with a stderr warning. + fmt.Fprintf(os.Stderr, "WARNING: ordered template rendering failed (%v); falling back to flat output\n", renderErr) + } else { + orderedRendered = true + if !client.DisableHooks { + for _, m := range rel.Hooks { + if skipTests && isTestHook(m) { + continue + } + fmt.Fprintf(out, "---\n# Source: %s\n%s\n", m.Path, m.Manifest) } - fmt.Fprintf(out, "---\n# Source: %s\n%s\n", m.Path, m.Manifest) } } - } else { + } + if !orderedRendered { var manifests bytes.Buffer fmt.Fprintln(&manifests, strings.TrimSpace(rel.Manifest)) if !client.DisableHooks {