public_git/helmfile
1
0
Fork 0
mirror of https://github.com/helmfile/helmfile.git synced 2026-09-30 16:12:19 +02:00
Code Issues Projects Releases Wiki Activity
2,800 Commits 21 Branches 370 Tags
a440e4eaf400c38ce86ff46dc526e834d7245ed2
Commit Graph
172 Commits
Author SHA1 Message Date
yxxhero a9e4a8518a feat: center section divider to match table width (#2698) 
The '========== Updated Releases ==========' header was a fixed 38
chars regardless of the table width below it. Now the divider line
is extended to match the table's visual width, with the title text
centered within the '=' borders.

- Add TableVisualWidth() to measure table width via runewidth
- Add HeaderDividerCentered() and HeaderDividerCenteredStyled()
  for centered dividers with optional bold+blue ANSI styling
- Refactor DisplayAffectedReleases to build the table first, then
  compute its width before logging the header
- Update all test snapshots and integration test output files

Signed-off-by: yxxhero <aiopsclub@163.com>
 2026-07-10 21:17:35 +08:00
yxxhero afdb2487a6 feat: add inherits: for sub-helmfile config inheritance (#2680) 
* feat: add `inherits:` for sub-helmfile config inheritance

Add an opt-in `inherits:` field to `helmfiles:` entries so a sub-helmfile
can inherit specific configuration categories from its parent:

  helmfiles:
  - path: myapp.yaml
    inherits: [repositories, environments]

Allowed values: repositories, helmDefaults, commonLabels, apiVersions,
kubeVersion, templates, environments. Child values win; parent fills gaps
(consistent with `bases:`). This directly fixes #1495, where a repository
declared in the parent was unavailable to sub-helmfiles, producing a
confusing "repo not found" error.

Implementation notes:
- The 6 pure fields (repositories, helmDefaults, commonLabels, apiVersions,
  kubeVersion, templates) are merged post-load via MergeInherited; verified
  all are consumed post-load (ExecuteTemplates/converge), never at parse.
- environments is injected pre-load as ctxEnv, because RenderedValues is
  baked at load time; the parent's resolved values become the base and the
  child's own environments: block overrides per key.
- helmDefaults uses a *HelmSpec pointer (value type is non-comparable) with
  a no-override mergo merge, so a child that omits helmDefaults inherits the
  parent's fully.
- A footgun warning (WarnUninheritedRepos) suggests
  `inherits: [repositories]` when a release references a repo the parent
  declares but the child lacks.
- Unknown inherits keys are rejected at parse time with the allowed set.

Inheritance is opt-in and fully backward compatible: empty (the default)
preserves the historical independent-sub-helmfile behavior.

Fixes #1495

Signed-off-by: yxxhero <aiopsclub@163.com>

* fix: address review — deep-copy inherited config and fix bases: doc link

- BuildInheritedConfig now deep-copies the pure fields via a YAML round-trip
  (Env via environment.DeepCopy) so the returned config never aliases the
  parent state's slices/maps, matching its doc comment. Now returns an error
  to surface round-trip failures; the call site in processNestedHelmfiles is
  updated. Added TestBuildInheritedConfig_PureFieldsAreDeepCopied to lock
  in the no-aliasing guarantee.
- Fix the broken `bases:` anchor (#) in shared-configuration-across-teams.md
  to point to writing-helmfile.md#layering-state-files.

Signed-off-by: yxxhero <aiopsclub@163.com>

* fix: address review — reject inherits without path and document helmDefaults caveat

Signed-off-by: yxxhero <aiopsclub@163.com>

* fix: address review — make AllowedInherits immutable and clarify effective-repo wording

Signed-off-by: yxxhero <aiopsclub@163.com>

---------

Signed-off-by: yxxhero <aiopsclub@163.com>
 2026-07-01 14:44:16 +08:00
yxxhero 365afc0577 test: add integration test for selector filtering with requiredEnv (#1172) (#2678) 
* fix: retry rendering with lenient requiredEnv when selectors are active (#1172)

The entire helmfile document is rendered as a Go template before selector
labels filter releases. This means requiredEnv calls in releases excluded
by selectors still fail, blocking the whole run.

When selectors are active and rendering fails due to a requiredEnv error,
helmfile now retries with lenient mode: unset env vars produce empty
strings instead of failing. The document can then be parsed and filtered
by selectors normally.

Behavior:
- Without selectors: requiredEnv fails as before (validation preserved)
- With selectors, all env vars set: strict render succeeds, no retry
- With selectors, some env vars missing: lenient retry, excluded releases
  get empty values and are filtered out by selectors

Implementation:
- Add RequiredEnvError type + ErrRequiredEnvNotSet sentinel for type-safe
  error detection via errors.As
- Add lenientRequiredEnv flag to tmpl.Context; requiredEnv returns empty
  string instead of failing when set
- Add NewLenientFileRenderer via functional options pattern
  (FileRendererOption / WithPreRender / WithLenientRequiredEnv)
- Extract renderWithSelectorFallback in two_pass_renderer.go
- Pass selectors from LoadOpts to desiredStateLoader

Closes #1172

Signed-off-by: yxxhero <yxxhero@example.com>
Signed-off-by: yxxhero <aiopsclub@163.com>

* test: add integration test for selector filtering with requiredEnv (#1172)

Verify that selector-based filtering works correctly when requiredEnv is
used in release values. Users set all required env vars, so rendering
succeeds, and selectors filter out non-matching releases.

Test scenarios:
- helmfile template -l tier=label2: only rel2 is templated (rel1 excluded)
- helmfile template without selector: both releases are templated

Closes #1172

Signed-off-by: yxxhero <yxxhero@example.com>
Signed-off-by: yxxhero <aiopsclub@163.com>

---------

Signed-off-by: yxxhero <yxxhero@example.com>
Signed-off-by: yxxhero <aiopsclub@163.com>
 2026-06-30 09:04:05 +08:00
yxxhero bbf925f154 test: add integration test for issue #1880 transformers with file:// deps (#2673) 
* test: add integration test for issue #1880 transformers with file:// deps

Add an integration test reproducing the exact scenario from issue #1880:
a local chart with a relative file:// dependency (file://../library) used
together with kustomize transformers.

Before the fix (rewriteChartDependencies in PR #2334), chartify copied
the chart to a temp directory, breaking the relative file:// path and
causing helm dependency up to fail with:
  Error: directory /tmp/chartify.../monitoring/library not found

Also fix test artifact leak in issue923_test.go where OCI chart downloads
wrote to CWD because OutputDirTemplate lacked {{ .OutputDir }}.

Closes #1880

Signed-off-by: yxxhero <aiopsclub@163.com>

* fix(test): guard exit-code captures against set -e in integration tests

Under set -e (enabled in run.sh), a failing command exits the shell
before 'var=$?' can execute, defeating diagnostic cat+fail blocks and
breaking helm diff tests that expect exit code 2.

Replaced 'cmd; var=$?' with 'var=0; cmd || var=$?' across 12 test
files (29 sites), matching the pattern already used in oci-parallel-pull.sh.

Signed-off-by: yxxhero <aiopsclub@163.com>

---------

Signed-off-by: yxxhero <aiopsclub@163.com>
 2026-06-29 08:49:51 +08:00
yxxhero 8f1bb404d6 fix: support go-getter URLs in ad-hoc dependencies to fix #821 (#2670) 
* fix: support go-getter URLs in ad-hoc dependencies to fix #821

Ad-hoc release dependencies (release.dependencies[].chart) that used
go-getter URLs like "git::https://host/repo.git@path?ref=tag" were passed
to chartify as-is. chartify then tried to resolve them via `helm repo
list`, which fails with "no helm list entry found for repository
\"git::https:\". please `helm repo add` it!".

The primary chart already fetched such URLs via downloadChartWithGoGetter,
but the ad-hoc dependency path in PrepareChartify only handled local
directories and OCI rewrites, so go-getter URLs fell through.

This adds a branch that detects remote go-getter URLs (remote.IsRemote)
and fetches them to a local cache directory via a new
downloadAdhocDepChartWithGoGetter helper, mirroring the primary-chart
fetch path. chartify then sees a local chart and takes its file:// branch.

Fixes #821

Signed-off-by: yxxhero <aiopsclub@163.com>

* test: add integration test for go-getter ad-hoc dependencies (#821)

Adds an integration test that commits a chart to a throwaway local git repo
and references it via a "git::file://..." go-getter URL as a release
ad-hoc dependency, then asserts `helmfile template` renders both the main
chart and the fetched dependency.

Using file:// (rather than https://) keeps the test deterministic and
network-free while exercising the exact fix path (remote.IsRemote +
downloadAdhocDepChartWithGoGetter in PrepareChartify). Verified to fail on
the unfixed code with the original "no helm list entry found for repository
\"git::file:\"" error and pass on the fixed code.

Signed-off-by: yxxhero <aiopsclub@163.com>

* test: surface helmfile error in issue #821 integration test

The integration runner (run.sh) enables `set -e`, so a non-zero helmfile
exit aborted the script before the captured output could be printed,
hiding the real failure in CI. Disable `set -e` around the helmfile
invocation and report the exit code plus full output on failure.

Signed-off-by: yxxhero <aiopsclub@163.com>

* test: fix issue-821 integration test chart path and errexit handling

Two issues caused the integration test to fail in CI (while passing in my
local smoke test, which used an absolute path):

1. The main chart path was relative to the integration CWD, but the
   helmfile.yaml is generated in a temp directory and helmfile resolves
   `chart:` relative to that directory (its basePath). The relative path
   was interpreted as a named-repo chart and failed instantly with
   `Error: repo test not found`. Resolve the case dir to an absolute path
   with `$(cd ... && pwd)`.

2. run.sh runs under `set -e`, so the unguarded helmfile invocation exited
   the whole script on failure before the captured output could be printed,
   hiding the real error. Capture the exit code via `cmd || rc=$?` so
   failures are reported.

Signed-off-by: yxxhero <aiopsclub@163.com>

---------

Signed-off-by: yxxhero <aiopsclub@163.com>
 2026-06-28 15:28:05 +08:00
yxxhero e3f757d5ed feat: add --template-args flag to template/apply/sync for helm lookup() support (#2666) 
* feat: add --template-args to enable helm lookup() during template/apply/sync (#1833)

Add a --template-args flag to the template, apply, and sync subcommands so
extra args (most notably --dry-run=server) can be passed to the helm template
invocation, enabling Helm's lookup() function to resolve live cluster values.

- template: --template-args reaches both chartify's pre-render helm template
  and the final helm template output (flagsForTemplate).
- apply/sync: --template-args reaches chartify's pre-render helm template.
  apply/sync already inject --dry-run=server automatically for cluster
  operations; the flag is an explicit opt-in for the template subcommand or
  for passing additional flags.
- When --dry-run is present in template args, kube-context/kubeconfig are also
  injected into chartify so lookup() can actually reach the cluster.
- Resolves the long-stale PR #1833 rebased onto current main, which already
  contains the cluster-connectivity infrastructure (issues #2271, #2309,
  #2355, #2444).
- Includes integration test (lookup.sh) covering both chartify and
  non-chartify scenarios.

Signed-off-by: yxxhero <aiopsclub@163.com>

* test: make lookup template nil-safe to fix integration CI

The lookup() function returns an empty map when the chart is rendered
without a cluster connection (notably the helm-diff phase of `helmfile
apply`). The original fixture chained `index` over the lookup result,
panicking with "index of untyped nil" during apply's diff rendering.

Guard every index with `default dict` so the template falls back to
"overwritten" when lookup is empty, while still resolving to the live
value ("init") when cluster access is available (--dry-run=server via
--template-args, or a real helm upgrade).

Signed-off-by: yxxhero <aiopsclub@163.com>

* feat: enable lookup() during apply/diff via --template-args in helm-diff

Thread --template-args into the helm-diff rendering path so that
`helmfile apply`/`diff --template-args="--dry-run=server"` resolves
Helm's lookup() function during the diff phase too. helm-diff supports
`--dry-run=server`, which explicitly "enables the cluster access ...
and the lookup template function".

Previously --template-args only reached chartify's pre-render (which is a
no-op for plain charts due to chartify's early-return when there is no
forceNamespace/patches/injections) and the final `helm template` of the
`template` subcommand. As a result `helmfile apply` on a lookup chart
rendered client-side during the diff phase.

Changes:
- pkg/state: add TemplateArgs to DiffOpts; append it in appendExtraDiffFlags
  (reaches every helm-diff invocation: apply, standalone diff, interactive
  sync), mirroring the existing flagsForTemplate handling.
- pkg/config + cmd: add --template-args to the diff/doctor commands and to
  DiffConfigProvider, so lookup works for `helmfile diff` as well.
- pkg/app: populate DiffOpts.TemplateArgs from apply/diff/sync-interactive.
- docs/cli.md: correct the previous overpromising wording and document diff
  support plus the nil-safe lookup guidance.
- tests: unit-test the TemplateArgs handling in appendExtraDiffFlags and
  flagsForTemplate; integration lookup.sh now exercises apply with
  --template-args="--dry-run=server".

Signed-off-by: yxxhero <aiopsclub@163.com>

* refactor: de-duplicate chartify template-args logic, add helmDefaults.templateArgs

Address review feedback on #2666:

1. Eliminate stale duplicated test helpers (issue_2444_test.go, issue_2355_test.go).
   Both files intentionally copied the processChartification flag-building logic
   with explicit SYNC WARNING comments, then drifted out of sync when #2666
   refactored the production code (needsKubeConnection gate, user-args merge).
   Extract the real logic into pure, unit-tested helpers
   (buildChartifyTemplateArgs, commandRequiresCluster) and delete the copies.

2. Add unit coverage for the new chartify merge path: template +
   --template-args=--dry-run=server now triggers kubeconfig/kube-context
   injection (TestTemplateArgsDryRunTriggersKubeInjection,
   TestTemplateArgsMergedBeforeInjection) — previously only covered by the
   cluster-dependent integration test.

3. Add a negative integration case (lookup.sh assert_template_fallback)
   verifying lookup() falls back to the default value WITHOUT --template-args,
   guarding against a regression that silently always connects to the cluster.

4. Add helmDefaults.templateArgs for parity with diffArgs/syncArgs, so users
   can enable lookup() support permanently instead of passing the flag on every
   invocation. CLI --template-args overrides (does not merge with) the default.
   Resolved via effectiveTemplateArgs, wired into the chartify, flagsForTemplate,
   and appendExtraDiffFlags paths.

5. Minor: capitalize --template-args help text to match surrounding flags;
   document helmDefaults.templateArgs precedence in docs/cli.md.

Signed-off-by: yxxhero <aiopsclub@163.com>

* test: cover helmDefaults->chartify composition; fix helm helm-diff typo

Address remaining review nits on #2666:

- Add TestHelmDefaultsTemplateArgsReachesChartify, a belt-and-suspenders test
  for the processChartification composition (effectiveTemplateArgs ->
  buildChartifyTemplateArgs), closing the last unit-level coverage gap for
  helmDefaults.templateArgs reaching the chartify path.

- Fix pre-existing typo in cmd/bind_diff_flags.go: 'pass args to helm helm-diff'
  -> 'Pass args to helm-diff' (doubled 'helm', lowercase).

Signed-off-by: yxxhero <aiopsclub@163.com>

* fix: correct 'helm helm-diff' typo in apply --diff-args help text

Sibling of the bind_diff_flags.go fix; the same doubled-'helm' typo and
lowercase help existed in cmd/apply.go's --diff-args registration, leaving
the apply and diff/doctor help strings inconsistent.

Signed-off-by: yxxhero <aiopsclub@163.com>

* docs: add helmDefaults.templateArgs to configuration reference

The complete helmfile.yaml schema in docs/configuration.md documents
diffArgs and syncArgs under helmDefaults but was missing the new
templateArgs field added in #2666. Add it beside syncArgs for
discoverability, noting the --template-args CLI override.

Signed-off-by: yxxhero <aiopsclub@163.com>

---------

Signed-off-by: yxxhero <aiopsclub@163.com>
 2026-06-28 12:19:33 +08:00
yxxheroandRoman Mykhailiuk 9d3a9ad6c7 feat: parallel kubedog tracking with progress printer and safety valves (#2654) 
* feat: parallel kubedog tracking with progress printer and safety valves

Rework kubedog integration so resource tracking runs in parallel with
helm upgrade/install, giving live progress output and recovering from
known helm/kubedog wedge conditions.

Core:
- kubedogTrackingHandle runs tracking in a background goroutine alongside
  the helm subprocess (startBackgroundKubedogTracking); helm output is
  buffered and replayed as a single block so it no longer interleaves
  with progress ticks.
- Capture UID+generation baselines before handing off to helm so each
  tracker waits until the resource actually changes (freshness gate).

Progress printer (pkg/kubedog/printer.go):
- Styled, auto-sized progress table with a heartbeat flusher, child (pod)
  status roll-up, pre-ready pod-phase handling, multi-namespace support,
  and optional color. PreviewBreakdown summarizes kept/filtered resources.

Safety valves (verify cluster state via the live API):
- Tracker-race valve (always on): when helm succeeds but a dyntracker
  goroutine is wedged, poll the API and cancel the tracker so wait()
  returns success instead of blocking until --track-timeout.
- Helm-stuck killer (opt-in via helmStuckGrace): if the cluster stays
  converged while helm v4's hook waiter is wedged, SIGINT the helm
  subprocess to recover.
- Failure watchdog (pkg/kubedog/watchdog.go): surface failing pods that
  never made it into dyntracker's resource graph.

Options: trackFailedLogs, helmStuckGrace, trackTimeout, color
(Color/NoColor), and resource filtering (trackKinds/skipKinds/
trackResources). PersistentVolumeClaim support in resource classification.

Signed-off-by: Roman Mykhailiuk <romanm@cybellum.com>

* test: add unit tests for kubedog tracking

Cover the progress printer, resource classification, the failure
watchdog, helm-output trimming/dedup, the release hard-timeout helper,
and the color/track option plumbing.

Signed-off-by: Roman Mykhailiuk <romanm@cybellum.com>

* test: update golden logs, e2e snapshots, and values-id fixtures

Refresh pkg/app testapply/testdestroy golden logs, e2e template
snapshots, and the TestGenerateID values-id golden hashes for the new
kubedog progress output and the merged release struct layout.

Signed-off-by: Roman Mykhailiuk <romanm@cybellum.com>

* refactor: remove dead kubedog display code and dedupe tracker setup

Deep-review pass on the parallel kubedog tracking changes:

- Remove pkg/kubedog/display.go (308 lines) and display_test.go (453 lines).
  These rendered progress for the legacy per-kind trackers that this PR
  replaces; in the merged tree every function is unreferenced outside its
  own tests. The new progressPrinter (printer.go) supersedes them.
  TestMain (color.ForceColor for deterministic ANSI in tests) is preserved
  in a new main_test.go.
- Dedupe trackWithKubedog: the post-helm fallback rebuilt the exact same
  tracker options as buildReleaseTracker. Reuse buildReleaseTracker instead,
  dropping ~40 lines of duplicated timeout/log/filter/tracker construction.

No behavior change; build, go vet, golangci-lint, and the kubedog/state
unit tests all pass.

Signed-off-by: yxxhero <aiopsclub@163.com>

* fix: stop waitForFreshness busy-looping the API after helm finishes

Once upstreamDoneCh closes it is always ready, so the select in
waitForFreshness stopped blocking on the ticker and re-ran probe() (a
live GET) as fast as the round-trip allowed for the whole 3s grace
window — hammering the API server once per tracked resource.

Track a local view of the channel and nil it out on first delivery so
the first hit records the timestamp (one fast retry, as intended) and
all subsequent polls are ticker-throttled. Functional behavior is
unchanged: return nil when fresh, errUpstreamDoneNoChange after grace.

Signed-off-by: yxxhero <aiopsclub@163.com>

* test: drop trailing blank line from helm4 OCI pull snapshots

The trailing-newline trim in helmexec.info() removes the blank line helm
prints after the OCI chart "Digest:" line. The helm3 (output.yaml)
snapshots never captured that line, but the helm4 (output-helm4.yaml)
snapshots for oci_chart_pull{,_direct,_once,_once2} and
issue_473_oci_chart_url_fetch still expected it, so they failed under
helm 4. Remove the blank line so the snapshots match the trimmed output.

Signed-off-by: yxxhero <aiopsclub@163.com>

* test: refresh diff-args integration goldens for styled release headers

DisplayAffectedReleases now emits a styled "========== Updated Releases
==========" header (matching the app/e2e goldens already updated by this
PR) and helmexec.info() trims the trailing blank after helm's install
status. Update the diff-args apply-stderr{,-helm4} and apply-live-
stderr{,-helm4} goldens accordingly so they match the actual stderr.

Signed-off-by: yxxhero <aiopsclub@163.com>

* test: drop trimmed blank line from v1-subhelmfile template golden

The trailing-newline trim in helmexec.info() removes the blank line helm
prints after '"incubator" has been added to your repositories'. Update
the v1-subhelmfile-multi-bases-with-array-values result and result-live
goldens so the template stdout comparison matches.

Signed-off-by: yxxhero <aiopsclub@163.com>

---------

Signed-off-by: Roman Mykhailiuk <romanm@cybellum.com>
Signed-off-by: yxxhero <aiopsclub@163.com>
Co-authored-by: Roman Mykhailiuk <romanm@cybellum.com>
 2026-06-21 13:43:17 +08:00
yxxhero bbceb05b31 feat: add helm 4 --server-side flag support for diff and bump helm-diff to v3.15.10 (#2645) 
Add appendServerSideFlagsForDiff to validate helm-diff plugin version
(v3.15.10+) before passing the --server-side flag to helm diff upgrade.
Extract resolveServerSideValue helper to share precedence logic between
upgrade and diff paths. Bump helm-diff recommended version to v3.15.10
across Dockerfiles, CI, and integration scripts.

Signed-off-by: yxxhero <aiopsclub@163.com>
 2026-06-17 09:07:44 +08:00
yxxhero 047fdcd17d build(deps): bump helm-diff to v3.15.9 (#2642) 
Update helm-diff plugin version from v3.15.8 to v3.15.9 across
Dockerfiles, recommended version constant, CI matrix, and
integration test default.

Signed-off-by: yxxhero <aiopsclub@163.com>
 2026-06-16 08:24:34 +08:00
Copilot 27382a27af Bump helm-diff to v3.15.8 across runtime defaults and execution environments (#2624) 
chore: bump helm-diff to v3.15.8

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
 2026-06-07 17:48:39 +08:00
yxxheroandcopilot-swe-agent[bot] 781d28a47a feat: add defaultInherit for automatic release template inheritance (#2600) 
* feat: add defaultInherit for automatic release template inheritance

Add a top-level defaultInherit field to helmfile.yaml that automatically
applies template inheritance to all releases without requiring explicit
inherit on each release.

The field accepts a single template name as a string or a list of
template names. Releases that already explicitly inherit from the same
template are not duplicated.

Fixes #2599

Signed-off-by: yxxhero <aiopsclub@163.com>

* style: fix gci formatting in app_template_test.go

Signed-off-by: yxxhero <aiopsclub@163.com>

* fix: correct relative chart path in integration test

Signed-off-by: yxxhero <aiopsclub@163.com>

* fix: use absolute chart path in bad-helmfile test

Signed-off-by: yxxhero <aiopsclub@163.com>

* fix: use clean chart path in bad-helmfile test

Signed-off-by: yxxhero <aiopsclub@163.com>

* fix: use dir variable for chart path

Signed-off-by: yxxhero <aiopsclub@163.com>

* test: fix flaky defaultInherit integration assertions

Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/d0884e8e-8b1b-456d-8250-dec1566b8a37

Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com>

* test: tighten defaultInherit integration assertions

Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/d0884e8e-8b1b-456d-8250-dec1566b8a37

Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com>

* test: harden release block parsing in issue-2599 case

Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/d0884e8e-8b1b-456d-8250-dec1566b8a37

Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com>

* test: make issue-2599 assertions format-tolerant

Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/d0884e8e-8b1b-456d-8250-dec1566b8a37

Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com>

* test: fix section extraction and regex matching in issue-2599 case

Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/d0884e8e-8b1b-456d-8250-dec1566b8a37

Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com>

* fix: sanitize defaultInherit values and dedupe applied templates

Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/85a8e815-3701-4b48-a28d-6bb2d50a3b40

Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com>

* chore: address validation feedback on defaultInherit fixes

Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/85a8e815-3701-4b48-a28d-6bb2d50a3b40

Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com>

* fix: sanitize releaseInherit entries in applyDefaultInherit; add cleanup trap and quote vars in integration test

Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/1fbf62d5-7ce2-42e5-898b-30151c0c1ef9

Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com>

* refactor: combine releaseInherit loops in applyDefaultInherit to avoid double TrimSpace; clarify test comment

Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/1fbf62d5-7ce2-42e5-898b-30151c0c1ef9

Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com>

* test: align default inherit tests with yaml wrapper and assertions

Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/3ea9b8e4-633f-43c4-899f-e063ec576486

Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com>

* test: address review feedback on defaultInherit tests

Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/3ea9b8e4-633f-43c4-899f-e063ec576486

Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com>

* test: fix issue-2599 integration script helmfile invocation

Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/9452bb65-7086-459f-b5ae-0b00c1e021eb

Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com>

---------

Signed-off-by: yxxhero <aiopsclub@163.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
 2026-05-20 18:21:03 +08:00
Copilotzhaque44yxxhero
 		2f2e8617ad Add jsonPatches regression coverage for chartify lookup rendering (#2586) 
* test: cover jsonPatches lookup regression

Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/72b7ba14-50b8-4407-ba37-6da202609603

Co-authored-by: zhaque44 <20215376+zhaque44@users.noreply.github.com>

* test: simplify json patch fixture

Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/72b7ba14-50b8-4407-ba37-6da202609603

Co-authored-by: zhaque44 <20215376+zhaque44@users.noreply.github.com>

* test: target existing json patch path

Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/72b7ba14-50b8-4407-ba37-6da202609603

Co-authored-by: zhaque44 <20215376+zhaque44@users.noreply.github.com>

* test: validate diff exit codes in issue-2271

Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/5615c543-f96b-44ed-be25-ca1559ee6ab0

Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: zhaque44 <20215376+zhaque44@users.noreply.github.com>
Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com>
 2026-05-20 17:02:45 +08:00
yxxhero 81ab674017 fix: normalize dependency chart path before DirectoryExistsAt check (#2598) 
When helmfile.d contains multiple release files and one release has a
local chart dependency (e.g. chart: ../chart), the dependency path was
passed to DirectoryExistsAt without normalizing against basePath. This
caused the path to be resolved against CWD instead of the helmfile
directory, so the local chart was not detected and helmfile tried to
resolve it as a remote repo, failing with:
  'failed reading adhoc dependencies: no helm list entry found for repository'

Fixes #2596

Signed-off-by: yxxhero <aiopsclub@163.com>
 2026-05-20 08:44:14 +08:00
Copilotandyxxhero e96a27734a bump helm-diff to v3.15.7 (#2591) 
Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/1ee2cfe9-dd7e-4477-80f9-ff62b0cdeab7

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com>
 2026-05-17 22:19:57 +08:00
Copilotandyxxhero 7ffd4a21ac Bump Helm support to 3.21.0 and 4.2.0 (#2588) 
* chore: bump pinned helm versions

Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/b1cfacaa-52d2-46c8-9fc7-67beaca43df0

Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com>

* fix: align kubernetes module pins for helm bump

Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/b1cfacaa-52d2-46c8-9fc7-67beaca43df0

Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com>

* docs: clarify helm and k8s version pins

Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/b1cfacaa-52d2-46c8-9fc7-67beaca43df0

Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com>

* test: update helm 4 snapshot outputs

Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/b430f041-d8fb-407f-af06-070f2d0e9293

Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com>

* test: update helm 4 postrender integration fixture

Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/fea792b4-b24c-43a9-a391-1fd52e59f843

Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com>

* test: update helm 4 postrender template fixture

Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/7ca16c9d-e398-46ce-849b-6299214b2b60

Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com>
 2026-05-15 13:14:02 +08:00
yxxhero c82c61e061 fix: template helmDefaults.postRendererArgs with release data (#2583) 
PR #1839 introduced template rendering for postRendererArgs, but PR #2510
reverted it while fixing a separate regression. This left helmDefaults-level
postRendererArgs containing template expressions (e.g. {{ .Release.Name }})
passed to helm as literal strings instead of being resolved per-release.

Add renderPostRendererArgs() that templates helmDefaults.postRendererArgs
at flag-generation time using the release's template data, reusing the
existing createReleaseTemplateData() helper. Release-level args are already
templated by ExecuteTemplateExpressions and CLI args are static, so only
the helmDefaults path needs rendering.

Fixes #2580

Signed-off-by: opencode <opencode@users.noreply.github.com>
Signed-off-by: yxxhero <aiopsclub@163.com>
 2026-05-11 09:05:06 +08:00
yxxheroandcopilot-swe-agent[bot] 420cc3ba9c fix: add trackFailOnError option to control kubedog exit code (#2576) 
* fix: add trackFailOnError option to control kubedog exit code behavior

When kubedog release tracking fails (e.g. pod ImagePullBackOff), helmfile
exits with code 0 instead of a non-zero exit code. Add a trackFailOnError
configuration option (default: false) that when set to true, propagates
kubedog tracking failures to the exit code.

The option is available as:
- Per-release YAML: trackFailOnError: true
- CLI flag: --track-fail-on-error (sync and apply commands)

Extract trackReleaseIfEnabled helper to consolidate kubedog tracking logic
from two duplicated call sites into a single maintainable method.

Fixes #2507

Signed-off-by: yxxhero <aiopsclub@163.com>

* fix: add //go:build ignore to server.go to fix go test CI failure

The test/integration/test-cases/issue-2103/input/server.go is a
package main helper binary used by the issue-2103 integration test.
When go test -coverprofile runs on this package, it fails with
"go: no such tool covdata" in the CI environment.

Adding //go:build ignore excludes the file from go list ./... (and
therefore from PKGS in the Makefile), while still allowing the
integration test to build it explicitly via file path:
  go build -o server ./path/to/server.go

Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/8a7000af-72b7-48f8-8a82-24813b5df341

Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com>

* fix: update TestGenerateID expected hashes after adding TrackFailOnError field

Adding TrackFailOnError *bool to ReleaseSpec changed the spew
serialization of the struct, which changed the FNV-32a hash values
produced by generateValuesID. Update temp_test.go with the new
expected hash strings.

Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/caa86cd9-73d1-4894-b745-fd70c0811fd6

Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com>

---------

Signed-off-by: yxxhero <aiopsclub@163.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
 2026-05-04 14:20:03 +08:00
yxxheroandcopilot-swe-agent[bot] 08a22772f7 feat: add --write-output flag to helmfile fetch for air-gapped environments (#2572) 
* feat: add --write-output flag to helmfile fetch for air-gapped environments

Add --write-output flag to helmfile fetch that outputs a modified
helmfile.yaml with chart references updated to point to downloaded
local chart paths. Combined with --output-dir, this enables preparing
all charts for deployment in air-gapped environments.

Usage:
  helmfile fetch --output-dir ./charts --write-output > helmfile-airgapped.yaml

Fixes #2571

Signed-off-by: yxxhero <yxxhero@users.noreply.github.com>
Signed-off-by: yxxhero <aiopsclub@163.com>

* fix: update fetch-write-output integration test grep to match YAML list item chart field

Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/d00f71ab-d40d-4220-9b11-97674597685f

Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com>

* fix: send status messages to stderr and enforce sequential processing in --write-output mode

Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/d338e24c-4f6f-4a59-a319-4b975e0efdcb

Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com>

* fix: restore SequentialHelmfiles after Fetch and use %s for YAML string formatting

Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/cfa9f3f4-c72f-4760-9c51-88bc6f30add2

Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com>

* test: add test for SequentialHelmfiles restore after Fetch with --write-output

Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/cfa9f3f4-c72f-4760-9c51-88bc6f30add2

Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com>

* fix: disable live output on --write-output and fix shell quoting/portability in integration test

Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/b0eb0d3d-493b-4d77-b8eb-2a5c0ce70d86

Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com>

* fix: use unquoted ${helmfile} variable to allow word splitting for EXTRA_HELMFILE_FLAGS

Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/d025a111-f7d0-439e-bf14-5508c40d0b51

Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com>

* fix: restore helm.EnableLiveOutput after Fetch --write-output via defer

Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/ddb8c5fc-ebd1-4f09-9474-5da58938a219

Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com>

* test: strengthen enableLiveOutput restore assertion with non-trivial initial value

Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/d1d0ba9e-5c97-48e1-b761-8bdee391efb2

Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com>

* feat: restrict --write-output to a single helmfile state file with clear error

Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/f608a0d0-7f52-4e3f-9fac-ab966bd01efb

Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com>

* refactor: apply code review suggestions for variable and test naming

Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/f608a0d0-7f52-4e3f-9fac-ab966bd01efb

Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com>

* fix: replace naked return with explicit return ok, errs to fix nakedret lint error

Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/4b060131-a977-44b0-98f7-42bc108ae8e8

Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com>

* fix: buffer YAML output and update --write-output flag description

Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/50c6ad2e-125c-43c1-b9c3-37fe1686a8eb

Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com>

* fix: shorten --write-output flag description, move detail to Long help

Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/50c6ad2e-125c-43c1-b9c3-37fe1686a8eb

Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com>

---------

Signed-off-by: yxxhero <yxxhero@users.noreply.github.com>
Signed-off-by: yxxhero <aiopsclub@163.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
 2026-05-03 18:32:30 +08:00
yxxheroandcopilot-swe-agent[bot] 9fa0529304 fix: apply post-renderer to output-dir-template output (#2531) 
* fix: apply post-renderer to output-dir-template output

When --output-dir and --post-renderer are both passed to helm template,
Helm writes pre-post-renderer content to files and sends post-renderer
output to stdout. This workaround strips --output-dir from helm flags,
captures the post-renderer-processed stdout, and writes it to the output
directory.

Fixes #2515

Signed-off-by: yxxhero <aiopsclub@163.com>

* test: add integration test for issue-2515 (post-renderer with output-dir-template)

Verifies that --post-renderer output is written to files when
--output-dir-template is set, instead of pre-renderer content.

Signed-off-by: yxxhero <aiopsclub@163.com>

* fix: address review comments - correct HasPrefix args, fix output dir structure, fix test mock init

Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/33d92423-fc47-4080-8307-5af9b16dd9c6

Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com>

* fix: wrap file operation errors with context in post-renderer workaround

Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/33d92423-fc47-4080-8307-5af9b16dd9c6

Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com>

* fix: correct chart path and use absolute case dir path in integration test

Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/43b7a794-1e7b-4577-8829-deb544a1a105

Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com>

* fix: restrict --output-dir + --post-renderer workaround to Helm 3 only

Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/229b14e2-b1ad-4f19-bd00-b8f7821383cd

Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com>

* fix: clean up stale templates dir on re-runs in Helm 3 post-renderer workaround

Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/f6c66284-8eca-4db3-8711-c9b6d3a9c179

Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com>

* fix: detect --post-renderer=<path> form and use targeted file cleanup

Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/8c9e4af4-84ae-4cbd-bc0a-8fcd9adddaed

Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com>

* feat: add Helm 4 post-renderer plugin and enable Helm 4 issue-2515 integration test

Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/3da2949c-a9d6-4e16-9b4a-a7e241080089

Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com>

* fix: search recursively for YAML files in Helm 4 output-dir integration test

Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/c5d33143-f611-40db-b73a-e5189d944ffd

Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com>

* fix: limit find depth and truncate log in Helm 4 integration test fallback message

Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/c5d33143-f611-40db-b73a-e5189d944ffd

Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com>

---------

Signed-off-by: yxxhero <aiopsclub@163.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
 2026-04-28 09:01:48 +08:00
yxxheroandcopilot-swe-agent[bot] 5368ab8d95 fix: skip subhelmfiles when selectors conflict with CLI selectors (#2545) 
* fix: skip subhelmfiles when selectors conflict with CLI selectors (#2544)

When CLI selectors are provided (e.g. -l name=b), subhelmfiles whose
explicit selectors are provably incompatible are now skipped entirely,
avoiding unnecessary YAML loading and template rendering.

Two selector sets are incompatible when every pair has a positive label
conflict: same key with different values (e.g. name=b vs name=a).
Negative labels are not compared.

Fixes #2544

Signed-off-by: yxxhero <aiopsclub@163.com>

* fix: address PR review comments - use CLI selectors, fix doc comment, add malformed selector test

Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/1f1c33ce-e50d-4781-85b8-d606b5d4ca54

Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com>

* fix: add debug logging, unit tests, docs, and fix integration test for subhelmfile selector skip

- Add debug log when skipping subhelmfile due to selector conflict
- Add TestSubhelmfileSelectorsConflict with 11 cases for direct unit coverage
- Document the selector-based subhelmfile skip optimization in docs/index.md
- Fix integration test: use 'app' label key instead of reserved 'name' key
  (GetReleasesWithLabels overwrites labels["name"] with the release name)

Signed-off-by: yxxhero <aiopsclub@163.com>

* refactor: avoid map allocation in positiveLabelsCompatibleWith

Compare positive label slices directly instead of allocating a map per
comparison, as label counts are typically small (1-3 entries).

Addresses Copilot review comment on PR #2545.

Signed-off-by: yxxhero <aiopsclub@163.com>

* docs: clarify subhelmfile selector docs per Copilot review feedback

Reword the first two bullets to avoid the contradiction between
'CLI selectors are ignored' and the new skip optimization.

Signed-off-by: yxxhero <aiopsclub@163.com>

* fix: address Copilot review comments round 3

- Log parse errors from SelectorsAreCompatible at debug level instead of
  silently discarding them
- Hoist regex compilation to package-level vars in ParseLabels to avoid
  repeated compilation per selector
- Replace EXIT traps with explicit cleanup calls in integration test to
  avoid interfering with the parent runner's trap

Signed-off-by: yxxhero <aiopsclub@163.com>

---------

Signed-off-by: yxxhero <aiopsclub@163.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
 2026-04-25 17:18:39 +08:00
yxxhero fc31dbfc5e fix: eliminate race condition in rewriteChartDependencies (#2541) 
* fix: eliminate race condition in rewriteChartDependencies by copying chart before modifying

Instead of modifying the original Chart.yaml in-place (which causes race
conditions when multiple releases reference the same local chart), copy the
chart to a temporary directory and rewrite the copy's dependencies. This
eliminates the need for per-chart mutex locks and prevents file corruption
when concurrent goroutines process releases sharing the same local chart.

Fixes #2502

Signed-off-by: yxxhero <aiopsclub@163.com>

* fix: address PR review comments for rewriteChartDependencies

- Handle non-NotExist errors from st.fs.Stat to surface permission/IO failures
- Reword function doc to clarify temp copy is conditional on rewrite being needed
- Assert rewrittenPath vs tempDir based on expectModified in test table

Signed-off-by: yxxhero <aiopsclub@163.com>

* test: add integration test for issue #2502 race condition with shared local chart

Signed-off-by: yxxhero <aiopsclub@163.com>

* fix: separate environments and releases with --- in helmfile.yaml

Signed-off-by: yxxhero <aiopsclub@163.com>

* fix: correct file:// path and remove --skip-deps for dependency build

Signed-off-by: yxxhero <aiopsclub@163.com>

* fix: correct file:// dependency path (5 levels up to test/integration/)

Signed-off-by: yxxhero <aiopsclub@163.com>

* fix: remove output validation from race condition test

Signed-off-by: yxxhero <aiopsclub@163.com>

* fix: assert WriteFile/MkdirTemp/RemoveAll/CopyDir in DefaultFileSystem test

Signed-off-by: yxxhero <aiopsclub@163.com>

* fix: add strategicMergePatches to trigger chartify in race condition test

Signed-off-by: yxxhero <aiopsclub@163.com>

* fix: scope test values under raw subchart and align ConfigMap name with strategic merge patches

The race condition test values.yaml had templates at the top level instead
of scoped under the raw subchart key, causing helm template to produce no
output and chartify's ReplaceWithRendered to fail with an empty
helmx.1.rendered directory. Also align the ConfigMap name to match the
strategicMergePatches target.

Signed-off-by: yxxhero <aiopsclub@163.com>

---------

Signed-off-by: yxxhero <aiopsclub@163.com>
 2026-04-20 10:15:47 +08:00
Etienne Champetier 4bdb6f097c fix: keep all chart dependencies key / values (#2501) 
* feat: Refactor TestRewriteChartDependencies

Signed-off-by: Etienne Champetier <e.champetier@ateme.com>

* fix: keep all chart dependencies key / values

In rewriteChartDependencies we were only parsing name / repository / version,
thus dropping keys like condition / import-values.

This at least fixes the use of condition.

Signed-off-by: Etienne Champetier <e.champetier@ateme.com>

---------

Signed-off-by: Etienne Champetier <e.champetier@ateme.com>
 2026-03-26 13:16:54 +08:00
yxxhero 732e4ad913 fix: helmfile fetch fails for kustomization directories (#2504) 
* fix: helmfile fetch fails for kustomization directories

Fixes #2503

When running `helmfile fetch` on a release that points to a local
kustomization directory (without Chart.yaml), the command failed with
"Chart.yaml is missing".

The issue was that the condition `helmfileCommand != "pull"` in
prepareChartForRelease skipped chartification for ALL cases during
fetch, including local kustomization directories that NEED chartify
to convert them to Helm charts.

Solution:
- Added `NeedsChartifyForLocalDir` field to the Chartify struct to
  track when chartification is needed because the local directory
  is not a Helm chart (no Chart.yaml)
- Modified the condition to skip chartification for "pull" ONLY when
  it's not a local directory without Chart.yaml

This preserves the original fix (commit 1f134d93) for remote charts
with transformers while fixing local kustomization directories.

Signed-off-by: yxxhero <aiopsclub@163.com>

* test: add integration test for helmfile fetch with kustomization

Add test case for issue #2503 to verify helmfile fetch works correctly
with local kustomization directories (without Chart.yaml).

Signed-off-by: yxxhero <aiopsclub@163.com>

---------

Signed-off-by: yxxhero <aiopsclub@163.com>
 2026-03-26 09:44:28 +08:00
yxxhero e72315a876 build: update helm-diff to v3.15.3 (#2498) 
Signed-off-by: yxxhero <aiopsclub@163.com>
 2026-03-24 17:28:41 +08:00
Aditya Menon c375b48550 fix: nested helmfile values should replace arrays, not merge element-by-element (#2458) 
PR #2367 introduced CLIOverrides to give --state-values-set element-by-element
array merge semantics. However, nested helmfile values (helmfiles[].values:)
were also routed into CLIOverrides, causing their arrays to merge instead of
replace. This broke the pre-v1.3.0 behavior where passing an array via
helmfiles[].values: would fully replace the child's default array.

Add OverrideValuesAreCLI flag to SubhelmfileEnvironmentSpec so the loader can
distinguish CLI flags from nested helmfile values. CLI values continue using
CLIOverrides (element-by-element merge); nested helmfile values now use Values
(Sparse merge strategy → full array replacement).

Fixes #2451

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>
 2026-03-09 18:31:21 +08:00
yxxhero 6e21671228 feat: kubedog integration with unified resource handling (#2383) 
* feat: add kubedog-based resource tracking integration

Add kubedog tracking as an alternative to Helm's --wait flag with:
- Real-time deployment progress tracking
- Container log streaming
- Fine-grained resource filtering (trackKinds/skipKinds/trackResources)

Features:
- New pkg/resource package for unified manifest parsing and filtering
- New pkg/kubedog package wrapping kubedog library
- CLI flags: --track-mode, --track-timeout, --track-logs
- Helmfile YAML support for trackMode, trackTimeout, trackLogs, trackKinds, skipKinds, trackResources
- Case-insensitive kind matching for filtering
- Multi-context support with proper kubeconfig/kubeContext handling

Tracking supports: Deployment, StatefulSet, DaemonSet, Job

Resource filtering priority (highest to lowest):
1. trackResources - explicit resource whitelist
2. skipKinds - blacklist specific kinds
3. trackKinds - whitelist specific kinds

Integration:
- Disable Helm --wait when using kubedog tracking
- Track after successful Helm sync/apply
- Respect release.Namespace as fallback for resources without namespace
- Use getKubeContext() for correct cluster targeting

Tests:
- Unit tests for resource filtering and kubedog options
- Integration test with httpbin chart
- E2E snapshot tests for YAML serialization
- Documentation in docs/advanced-features.md

Signed-off-by: yxxhero <aiopsclub@163.com>

* fix: address PR #2383 review comments (round 4)

1. resource/filter.go: Skip empty whitelist entries in matchWhitelist
   - At least one field (kind/name/namespace) must be specified
   - Prevents matching all resources with empty TrackResources entries

2. config/apply.go: Add ValidateConfig for track-mode validation
   - Validate --track-mode must be 'helm' or 'kubedog'
   - Reject invalid values like --track-mode foo

3. config/sync.go: Add ValidateConfig for track-mode validation
   - Same validation as apply command
   - Ensures consistent behavior across commands

Signed-off-by: yxxhero <aiopsclub@163.com>

---------

Signed-off-by: yxxhero <aiopsclub@163.com>
 2026-03-02 17:15:12 +08:00
yxxhero 2be73dd21d build: update helm-diff to v3.15.1 (#2442) 
Signed-off-by: yxxhero <aiopsclub@163.com>
 2026-02-28 09:56:11 +08:00
yxxheroandCopilot b5dc75ad72 fix: local chart with external dependencies error when repos configured (#2433) 
* fix: local chart with external dependencies error when repos configured

When helm repo update was run, the code unconditionally set skipRefresh=true
for all builds, causing helm dep build --skip-refresh to fail for local charts
with external dependencies not listed in helmfile.yaml.

Now only non-local charts (precomputed skipRefresh=true) get --skip-refresh,
while local charts preserve their skipRefresh=false to allow refreshing repos
for external dependencies.

Fixes #2431

Signed-off-by: yxxhero <aiopsclub@163.com>

* test: update snapshot tests for local chart refresh behavior

Local charts now run helm repo update during helm dep build to support
external dependencies not listed in helmfile.yaml (fixes #2431).

Signed-off-by: yxxhero <aiopsclub@163.com>

* refactor: remove redundant skipRefresh assignment

The condition 'if didUpdateRepo && r.skipRefresh { r.skipRefresh = true }'
was a no-op since setting true to true has no effect. The precomputed
skipRefresh value from prepareChartForRelease is already correct, so we
simply preserve it without modification.

Signed-off-by: yxxhero <aiopsclub@163.com>

* refactor: only call UpdateRepo when at least one build uses --skip-refresh

Avoid redundant helm repo update when all builds have skipRefresh=false,
as each helm dep build will refresh repos itself in that case.

Co-authored-by: Copilot <copilot@github.com>
Signed-off-by: yxxhero <aiopsclub@163.com>

* test: update release_template_inheritance snapshot for skipRefresh optimization

UpdateRepo is now only called when at least one build uses --skip-refresh,
so local charts without skipRefresh no longer trigger the global repo update.

Signed-off-by: yxxhero <aiopsclub@163.com>

* test: add regression test for issue #2431

Add TestIssue2431_LocalChartWithExternalDependency to verify that local
charts with external dependencies on repos NOT in helmfile.yaml work
correctly. The test ensures:
- UpdateRepo is NOT called when all builds have skipRefresh=false
- helm dep build does NOT receive --skip-refresh flag

Signed-off-by: yxxhero <aiopsclub@163.com>

* test: add integration test for issue #2431

Add test case to verify that local charts with repos configured in
helmfile.yaml work correctly. The test ensures that helmfile template
does not fail with 'no cached repository' or 'no repository definition'
errors when:
- helmfile.yaml has non-OCI repos configured
- Local chart is used (which may have external dependencies not in helmfile.yaml)

Signed-off-by: yxxhero <aiopsclub@163.com>

* test: update issue #2431 integration test to match issue scenario

Add external dependency (karma chart from wiremind repo) to local chart's
Chart.yaml, matching the exact scenario described in issue #2431 where:
- helmfile.yaml has repos configured (vector)
- Local chart depends on a repo NOT in helmfile.yaml (wiremind)

Signed-off-by: yxxhero <aiopsclub@163.com>

* revert: remove unit tests and restore e2e snapshot outputs

Remove pkg/state/run_helm_dep_builds_skip_refresh_test.go and restore
chart_need snapshot outputs to original state. The fix is verified by
the integration test for issue #2431.

Signed-off-by: yxxhero <aiopsclub@163.com>

* test: remove snapshot outputs to regenerate them

Remove chart_need snapshot outputs so they can be regenerated by tests.

Signed-off-by: yxxhero <aiopsclub@163.com>

* revert: restore release_template_inheritance snapshot output

Signed-off-by: yxxhero <aiopsclub@163.com>

* restore: add back unit tests for skipRefresh behavior

Signed-off-by: yxxhero <aiopsclub@163.com>

* restore: add back chart_need snapshot outputs

Signed-off-by: yxxhero <aiopsclub@163.com>

* test: update snapshot outputs for skipRefresh optimization

- Remove TestIssue2431_LocalChartWithExternalDependency unit test
- Update chart_need outputs: local chart runs helm dep build with repo refresh
- Update release_template_inheritance: no deps so no repo refresh output

Signed-off-by: yxxhero <aiopsclub@163.com>

* fix: update test comments and names per review feedback

- Update TestRunHelmDepBuilds_MultipleBuilds comment to remove reference
  to removed didUpdateRepo variable
- Rename test case to accurately describe condition being tested
  (build with skipRefresh=true instead of misleading 'non-local chart')

Signed-off-by: yxxhero <aiopsclub@163.com>

---------

Signed-off-by: yxxhero <aiopsclub@163.com>
Co-authored-by: Copilot <copilot@github.com>
 2026-02-28 09:23:04 +08:00
Aditya Menon 69bed171ab fix: use absolute baseDir in sequential helmfiles for correct values path resolution (#2425) 
* fix: use absolute baseDir in sequential helmfiles for correct values path resolution (#2424)

PR #2410 introduced a regression where a relative directory was passed as
baseDir instead of an absolute one, causing values and secrets file paths
to resolve incorrectly when using --sequential-helmfiles with helmfile.d/.

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>

* fix: mirror reporter's bases/templates/inherit setup in issue-2424 integration test

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>

---------

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>
 2026-02-26 07:47:39 +08:00
yxxhero 27c78a123e fix: skip helm repo update when only OCI repos are configured (#2420) 
When using only OCI repositories, helmfile would attempt to run
'helm repo update' which fails with 'no repositories found' error.
OCI repositories don't need 'helm repo update' as they use
'helm registry login' instead.

This fix adds a HasNonOCIRepositories() helper function and uses it
to determine whether to run 'helm repo update'.

Fixes #2418

Signed-off-by: yxxhero <aiopsclub@163.com>
 2026-02-25 12:13:20 +08:00
Aditya Menon abfe73fa6f fix: helmDefaults.skipRefresh ignored in runHelmDepBuilds (#2415) 
fix: helmDefaults.skipRefresh ignored in runHelmDepBuilds (#2269)

`runHelmDepBuilds()` only checked the CLI flag (`opts.SkipRefresh`) when
deciding whether to run `helm repo update` before building dependencies.
This meant that setting `helmDefaults.skipRefresh: true` in helmfile.yaml
had no effect on the repo update call inside dep builds.

Add `!st.HelmDefaults.SkipRefresh` to the guard condition so that
`helmDefaults.skipRefresh: true` is respected alongside the CLI flag.

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>
 2026-02-22 14:13:05 +05:30
Aditya Menon c63947483c fix: eliminate os.Chdir in sequential helmfiles to fix relative path resolution (#2410) 
* fix: eliminate os.Chdir in sequential helmfiles to fix relative path resolution

The sequential code path used within() → os.Chdir() to change the
process-wide working directory when processing helmfile.d files.
This broke relative environment variable paths (e.g. KUBECONFIG=kubeconfig.yaml)
because they resolved from the wrong directory after chdir.

Replace the chdir-based approach with the same baseDir parameter pattern
used by the parallel code path, passing explicit directory context through
loadDesiredStateFromYamlWithBaseDir() instead of mutating global process state.

Closes #2409

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>

* fix: restore within() for single-file sequential to preserve chart path format

The previous approach used baseDir for all sequential processing, which
changed chart path format in output (e.g. from "../../../../charts/raw"
to "test/integration/charts/raw"). This broke integration tests that
compare chart paths in expected output.

Now the sequential branch uses two strategies:
- Single file: use os.Chdir via within() to preserve backward-compatible
  relative chart paths in output
- Multiple files with --sequential-helmfiles: use baseDir parameter to
  avoid os.Chdir, fixing relative env var paths like KUBECONFIG (#2409)

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>

* fix: revert e2e snapshot outputs to match within() behavior

The previous commit restored within() for single-file sequential
processing, which produces relative chart paths (e.g. ../../charts/raw)
and filename-only FilePath. Revert the e2e snapshot expected outputs
to match main branch since single-file behavior is now identical.

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>

* fix: restructure integration test for multi-file sequential processing

- Point -f at helmfile.d/ directly (not parent dir) so findDesiredStateFiles
  discovers the yaml files
- Add second helmfile to trigger baseDir path (len > 1)
- Inline environment config to avoid base file relative path issues
- Verify both releases appear in output instead of comparing with parallel
  (which may differ in ordering)

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>

* fix: reduce cognitive complexity and improve accuracy of sequential helmfiles

Replace inline visitSubHelmfiles closure with calls to the existing
processNestedHelmfiles() method, matching the parallel path. This
eliminates duplicated nested logic and reduces gocognit complexity
below the CI threshold of 110. Also fixes help text and docs to
accurately describe that single-file processing still uses within(),
and adds kubeContext verification to the integration test.

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>

* test: validate kubeContext resolution in sequential helmfiles integration test

Restructure the integration test to replicate the exact user scenario
from issue #2409:
  - Multiple files in helmfile.d/ using bases: with relative paths
    (../bases/) for environments and defaults
  - Environment values set kubeContext via .Environment.Values
  - helmDefaults.kubeContext rendered from gotmpl
  - Local chart references (../../../../charts/raw) from helmfile.d/
  - Run diff against the minikube cluster to exercise kubeContext
    resolution, which would fail with "context does not exist" if
    os.Chdir() broke relative path resolution
  - Also verify template output for both releases and relative values
    file (values/common.yaml) resolution

Fix normalizeChart() in util.go to be idempotent — skip re-prefixing
when the chart path already starts with basePath. This prevents
double-prefixing of local chart paths (e.g. helmfile.d/test/.../raw)
when normalizeChart is called multiple times (once during chart
preparation and again during diff/sync).

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>

---------

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>
 2026-02-22 09:21:46 +08:00
Aditya Menon 0129681222 feat: add helmfile unittest command for helm-unittest integration (#2400) 
Adds a new `helmfile unittest` command that integrates the helm-unittest
plugin, allowing users to define unit test paths per release and run them
via helmfile.

Closes #2376

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>
 2026-02-16 09:45:10 +08:00
Aditya MenonandCopilot c6b962dbbf fix: include query params in HTTP getter cache key (#2399) 
* fix: include query params in HTTP getter cache key (#2103)

When helmfile caches remote HTTP files fetched via the "normal" getter
(plain https:// URLs without a git:: prefix), the cache key did not
include query parameters. This caused URLs that differ only in query
params (e.g. ?ref=commit1 vs ?ref=commit2) to share the same cache
directory, silently returning the wrong file version.

The root cause was in Fetch() where the "normal" getter branch
overwrote the cache key with only scheme + host, discarding query
params that were correctly computed earlier.

Fix: extract the query-params suffix into a reusable variable and
apply it in both the default and "normal" getter cache key paths.

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>

* Update pkg/remote/remote.go

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>

* Update pkg/remote/remote_test.go

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>

---------

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
 2026-02-14 09:31:53 +08:00
Aditya Menon 5c43fa6465 fix: support OCI chart digest syntax (@sha256:...) (#2398) 
fix: support OCI chart digest syntax in chart URLs and version fields

Helm supports pinning OCI chart images by digest (@sha256:...), version
tag (:version), or both (:version@sha256:digest) since helm/helm#12690.
Helmfile failed to parse these formats, incorrectly constructing helm
commands and losing version/digest information embedded in chart URLs.

Root causes:
- resolveOciChart() used last ":" to find version tag, but sha256:abc
  contains ":", so digest URLs were split incorrectly
- getOCIQualifiedChartName() included :version and @digest in chartName
  with no parsing of either source
- appendChartVersionFlags() passed release.Version verbatim to --version
  flag, including any digest suffix
- ChartPull() discarded the tag from resolveOciChart but did not
  preserve digest in the URL

This commit adds parseOCIChartRef() and parseVersionDigest() utilities,
then updates the OCI chart handling pipeline so that:
- Digests are preserved in the chart URL passed to helm pull
- Version tags are extracted cleanly for the --version flag
- Both chart URL and version field are parsed for version/digest info

Fixes #2097

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>
 2026-02-12 20:20:43 +08:00
yxxheroandJavex 9964a2eacb feat: Ensure repo update is only run once (#2378) 
* feat: Ensure repo update is only run once

Perform a single Hang tight while we grab the latest from your chart repositories...
...Successfully got an update from the "glm-bitnami" chart repository
...Unable to get an update from the "fluent" chart repository (https://fluent.github.io/helm-charts):
	Get "https://fluent.github.io/helm-charts/index.yaml": read tcp 192.168.0.104:51893->185.199.108.153:443: read: connection reset by peer
...Unable to get an update from the "grafana" chart repository (https://grafana.github.io/helm-charts):
	Get "https://grafana.github.io/helm-charts/index.yaml": read tcp 192.168.0.104:51897->185.199.109.153:443: read: connection reset by peer
...Unable to get an update from the "ingress-nginx" chart repository (https://kubernetes.github.io/ingress-nginx):
	Get "https://kubernetes.github.io/ingress-nginx/index.yaml": read tcp 192.168.0.104:51894->185.199.110.153:443: read: connection reset by peer
...Unable to get an update from the "chartmuseum" chart repository (https://chartmuseum.github.io/charts):
	Get "https://chartmuseum.github.io/charts/index.yaml": read tcp 192.168.0.104:51896->185.199.110.153:443: read: connection reset by peer
...Successfully got an update from the "glm-chartmuseum" chart repository
...Successfully got an update from the "apollo" chart repository
...Successfully got an update from the "kyverno" chart repository
...Unable to get an update from the "mysql-operator" chart repository (https://mysql.github.io/mysql-operator/):
	Get "https://mysql.github.io/mysql-operator/index.yaml": read tcp 192.168.0.104:51903->185.199.111.153:443: read: connection reset by peer
...Unable to get an update from the "metallb" chart repository (https://metallb.github.io/metallb):
	Get "https://metallb.github.io/metallb/index.yaml": read tcp 192.168.0.104:51904->185.199.111.153:443: read: connection reset by peer
...Unable to get an update from the "dragonfly" chart repository (https://dragonflyoss.github.io/helm-charts/):
	Get "https://dragonflyoss.github.io/helm-charts/index.yaml": read tcp 192.168.0.104:51905->185.199.108.153:443: read: connection reset by peer
...Unable to get an update from the "openfga" chart repository (https://openfga.github.io/helm-charts):
	Get "https://openfga.github.io/helm-charts/index.yaml": read tcp 192.168.0.104:51907->185.199.111.153:443: read: connection reset by peer
...Unable to get an update from the "cnpg" chart repository (https://cloudnative-pg.github.io/charts):
	Get "https://cloudnative-pg.github.io/charts/index.yaml": read tcp 192.168.0.104:51910->185.199.111.153:443: read: connection reset by peer
...Unable to get an update from the "metrics-server" chart repository (https://kubernetes-sigs.github.io/metrics-server/):
	Get "https://kubernetes-sigs.github.io/metrics-server/index.yaml": read tcp 192.168.0.104:51913->185.199.111.153:443: read: connection reset by peer
...Unable to get an update from the "ot-helm" chart repository (https://ot-container-kit.github.io/helm-charts/):
	Get "https://ot-container-kit.github.io/helm-charts/index.yaml": read tcp 192.168.0.104:51914->185.199.111.153:443: read: connection reset by peer
...Unable to get an update from the "coredns" chart repository (https://coredns.github.io/helm):
	Get "https://coredns.github.io/helm/index.yaml": read tcp 192.168.0.104:51917->185.199.111.153:443: read: connection reset by peer
...Unable to get an update from the "redis-operator" chart repository (https://ot-container-kit.github.io/helm-charts/):
	Get "https://ot-container-kit.github.io/helm-charts/index.yaml": read tcp 192.168.0.104:51912->185.199.111.153:443: read: connection reset by peer
...Unable to get an update from the "andrcuns" chart repository (https://andrcuns.github.io/charts):
	Get "https://andrcuns.github.io/charts/index.yaml": read tcp 192.168.0.104:51915->185.199.111.153:443: read: connection reset by peer
...Successfully got an update from the "gitlab-jh" chart repository
...Successfully got an update from the "hashicorp" chart repository
...Successfully got an update from the "incubator" chart repository
...Successfully got an update from the "jenkins" chart repository
...Successfully got an update from the "nvidia" chart repository
...Successfully got an update from the "elastic" chart repository
...Successfully got an update from the "projectcalico" chart repository
...Unable to get an update from the "juicefs" chart repository (https://juicedata.github.io/charts/):
	Get "https://juicedata.github.io/charts/index.yaml": read tcp 192.168.0.104:51919->185.199.111.153:443: read: connection reset by peer
...Successfully got an update from the "bitnami" chart repository
Update Complete. ⎈Happy Helming!⎈ before running any
commands, allowing us to safely pass --skip-refresh to avoid redundant
repo updates for each chart with external dependencies.

This reduces the number of repository refresh operations from O(n) to O(1)
where n is the number of charts with remote dependencies.

Co-authored-by: Javex <github@javex.eu>
Signed-off-by: yxxhero <aiopsclub@163.com>

* fix: ensure repo update only runs when repositories are configured

This fixes CI issues where tests fail with 'no repositories found' error.

The PR #2378 adds a single helm.UpdateRepo() call before running helm dep
build commands. However, when no repositories are configured, this call
fails. The fix adds a check for len(st.Repositories) > 0 before calling
UpdateRepo().

Additionally, updated snapshot files to reflect the new output ordering
where repo update happens before building dependencies.

Signed-off-by: yxxhero <aiopsclub@163.com>

* feat: Update test snapshots for single repo update

The code changes in PR #2378 ensure that helm repo update is only run once
before building dependencies. This requires updating test snapshots to include
the 'Updating repo' output that now appears before 'Building dependency' messages.

Updated snapshots:
- chart_need/output.yaml
- chart_need_enable_live_output/output.yaml
- release_template_inheritance/output.yaml
- environments_releases_without_same_yaml_part/output.yaml
- environment_missing_in_subhelmfile/output.yaml
- pr_560/output.yaml
- environments_values_gotmpl_with_environment_name/output.yaml
- postrenderer/output.yaml (fixed YAML structure)
- oci_need/output.yaml

Signed-off-by: yxxhero <aiopsclub@163.com>

* fix: Correctly update test snapshots based on repository configuration

Only update snapshots for tests that have repositories defined:
- chart_need/output.yaml (has repositories - shows 'Updating repo')
- chart_need_enable_live_output/output.yaml (has repositories - shows 'Updating repo')
- release_template_inheritance/output.yaml (has repositories - shows 'Updating repo')

Tests without repositories should NOT show 'Updating repo':
- environments_releases_without_same_yaml_part/output.yaml
- environments_values_gotmpl_with_environment_name/output.yaml
- pr_560/output.yaml
- environment_missing_in_subhelmfile/output.yaml
- postrenderer/output.yaml (uses OCI dependencies)
- oci_need/output.yaml (uses OCI dependencies)

This matches the conditional logic in the code that only runs
helm.UpdateRepo() when len(st.Repositories) > 0.

Signed-off-by: yxxhero <aiopsclub@163.com>

* fix: correct snapshot test expectations for repo update optimization

- Re-add trailing newlines to environment_missing_in_subhelmfile output
- Restore correct chart paths (/... instead of ../../...)
- Restore postrenderer output with cm2 ConfigMap and correct field order
- Fixes CI test failures introduced by incorrect snapshot updates

Signed-off-by: yxxhero <aiopsclub@163.com>

* fix: update integration test expected lint output for repo update

Include 'Updating repo' messages in expected lint output files
to match the new behavior where helm repo update is run once
before building dependencies.

Signed-off-by: yxxhero <aiopsclub@163.com>

* fix: remove extra blank line from lint output files

Integration test output files had an extra blank line that was
not present in the expected output, causing test failures.

Signed-off-by: yxxhero <aiopsclub@163.com>

* fix: update lint output for single repo update

With the repo update optimization, lint runs only once
with 'Updating repo' messages instead of running twice.
Update expected output to match new single-run behavior.

Signed-off-by: yxxhero <aiopsclub@163.com>

* fix: filter out repo update messages in lint test

Update test runner to filter out repo update messages that are
now generated by the single helm.UpdateRepo() call, keeping
the expected lint output consistent with the original behavior.

Signed-off-by: yxxhero <aiopsclub@163.com>

* fix: filter repo update messages from diff test

Filter out repo update messages in diff test output to
match new behavior where helm.UpdateRepo() is called once.

Signed-off-by: yxxhero <aiopsclub@163.com>

* Fix missing closing parenthesis in grep command

Signed-off-by: yxxhero <aiopsclub@163.com>

* fix: prevent --args flags from being passed to helm repo commands

When helmfile template --args is used, the extra flags were being
passed to helm repo update and helm repo add commands, which don't
support all flags that helm template/install support. This caused
failures when flags like --dry-run were passed via --args.

The fix saves the extra flags before executing helm repo commands,
clears them, and restores them afterwards to ensure repo commands
run without unsupported flags.

Fixes CI issue in PR #2378 where test issue-1749 fails
with "Error: unknown flag: --dry-run" during helm repo update.

Signed-off-by: yxxhero <aiopsclub@163.com>

---------

Signed-off-by: yxxhero <aiopsclub@163.com>
Co-authored-by: Javex <github@javex.eu>
 2026-01-29 19:15:44 -05:00
yxxhero e555ade4c0 feat: upgrade Helm version to v3.20.0 and v4.1.0 (#2373) 
* feat: upgrade Helm version to v3.20.0 and v4.1.0

This commit updates the recommended Helm version from v3.19.5/v4.0.5 to
v3.20.0/v4.1.0 across all workflows, Dockerfiles, and application constants.

Changes:
- Update CI matrix to test with Helm v3.20.0 and v4.1.0
- Update .github/workflows/Makefile HELM_VERSION to v4.1.0
- Update Dockerfiles with new version and SHA256 checksums
- Update pkg/app/init.go HelmRecommendedVersion to v4.1.0
- Update go.mod helm.sh/helm/v3 to v3.20.0 and helm.sh/helm/v4 to v4.1.0

Signed-off-by: yxxhero <aiopsclub@163.com>

* fix: remove source field from e2e test helm plugin configs

Signed-off-by: yxxhero <aiopsclub@163.com>

* fix: remove source field from integration test helm plugin config

Signed-off-by: yxxhero <aiopsclub@163.com>

---------

Signed-off-by: yxxhero <aiopsclub@163.com>
 2026-01-23 12:02:58 +08:00
yxxhero 63f589016a Fix 2337 helm4 stale repo indexes (#2369) 
* fix: add --force-update flag for Helm 4 to prevent stale repository indexes

Fixes #2337

Problem:
Helmfile with Helm v4 doesn't update repository indexes when adding repos,
leading to stale indexes and errors like:
  "chart matching version not found in example index. (try 'helm repo update')"

This happens because Helm 4 changed behavior compared to Helm 3:
- Helm 3: Always downloads index when running "helm repo add", even if repo exists
- Helm 4: Skips downloading index if repo already exists with same config
  (see: https://github.com/helm/helm/blob/v4.0.4/pkg/cmd/repo_add.go#L200)

Without --force-update, helmfile only works initially because Helm 4
downloads index on fresh repo setup, but subsequent "helmfile repos"
commands result in stale indexes.

Root Cause:
The code only added --force-update for Helm 3.3.2+, but not for Helm 4,
since it was believed to be default behavior in Helm 4. However, Helm 4
requires explicit --force-update flag to update indexes for existing repos.

Solution:
Add --force-update flag for Helm 4 in AddRepo function to ensure
repository indexes are updated even when repository already exists.

Refactoring:
Simplified the conditional logic from nested if statements to a single
readable condition using existing IsVersionAtLeast() helper:
  if !helm.options.DisableForceUpdate &&
     (helm.IsHelm4() || helm.IsVersionAtLeast("3.3.2")) {
    args = append(args, "--force-update")
  }

Changes:
- pkg/helmexec/exec.go: Add --force-update for Helm 4
- pkg/helmexec/exec_test.go: Update test expectations for both Helm 3.3.2+ and Helm 4
- AGENTS.md: Add development guide for the repository

Testing:
- All helmexec package tests pass
- Verified build succeeds
- Tested against Helm 3.2.0 (no force-update)
- Tested against Helm 3.3.2+ (with force-update)
- Tested against Helm 4.0.1 (with force-update)

Signed-off-by: opencode <opencode@users.noreply.github.com>
Signed-off-by: yxxhero <aiopsclub@163.com>

* test: update expected output for Helm 4 repo add message

Update integration test expectations to match Helm 4 behavior with --force-update flag.
When --force-update is used, Helm 4 now outputs "has been added to your
repositories" instead of "already exists with the same configuration, skipping",
because it forcibly updates the repository index.

Related to #2337

Signed-off-by: opencode <opencode@users.noreply.github.com>
Signed-off-by: yxxhero <aiopsclub@163.com>

---------

Signed-off-by: opencode <opencode@users.noreply.github.com>
Signed-off-by: yxxhero <aiopsclub@163.com>
 2026-01-21 19:55:56 -05:00
Aditya Menon 70645e0622 fix: array merge regression - layer arrays now replace defaults (#2367) 
* fix: array merge regression - layer arrays now replace defaults (#2353)

PR #2288 introduced element-by-element array merging to fix #2281, but this
caused a regression where layer/environment arrays were merged instead of
replacing base arrays entirely.

This fix uses automatic sparse array detection:
- Arrays with nil values (from --state-values-set) merge element-by-element
- Arrays without nils (from layer YAML) replace entirely

This follows Helm's documented behavior where arrays replace rather than merge.

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>

* fix: use separate CLIOverrides field for element-by-element array merging

The previous approach using ArrayMergeStrategySparse detection didn't work
for --state-values-set array[0]=value because setting index 0 produces no
nils in the array.

This fix adds a CLIOverrides field to Environment that keeps CLI values
separate from layer values. CLI overrides are merged last using
ArrayMergeStrategyMerge (always element-by-element), while layer values
use the default strategy (arrays replace).

This ensures:
- --state-values-set array[0]=x only changes index 0, preserving other elements
- Layer/environment file arrays still replace base arrays entirely
- Issue #2281 fix is preserved (--state-values-set array[1].field=x works)

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>

* fix: correct comment about array merge strategy in test

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>

* fix: propagate Defaults in multi-part helmfiles and fix merge order

- Add Defaults field merging from ctxEnv to preserve base values across
  helmfile parts separated by ---
- Fix merge order: current part values now correctly override previous
  parts (was reversed, causing older values to win)
- Update 147 snapshot test files for new Environment log format with
  CLIOverrides field

This completes the fix for issue #2353 by ensuring:
1. Layer arrays replace entirely (not element-by-element merge)
2. CLI --state-values-set sparse arrays still merge element-by-element
3. Multi-part helmfiles properly inherit and override values

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>

* fix: address Copilot review comments

- Initialize EmptyEnvironment with empty maps to match New() constructor
- Update test comment to accurately describe ArrayMergeStrategySparse

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>

* fix: ensure templates access merged values via .Environment.Values

This commit fixes a regression in the CLIOverrides integration where
templates accessing .Environment.Values couldn't see CLI override values.

Changes:
- Remove CLIOverrides-into-Values merge from Merge() to keep proper
  layering order (Defaults → Values → CLIOverrides) in GetMergedValues()
- Update NewEnvironmentTemplateData to set envCopy.Values to the merged
  values, ensuring templates see the same values via both .Values and
  .Environment.Values

This ensures:
- Issue #2353: Layer arrays still replace entirely (Sparse strategy)
- Issue #2281: CLI sparse arrays still merge element-by-element
- Templates can access CLI overrides via .Environment.Values

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>

* docs: improve mergeSlices documentation per Copilot review

Address Copilot review comments on PR #2367:
- Document empty array edge case: explicitly setting [] clears base array
- Document recursive strategy propagation for nested map merging
- Add comprehensive behavior description for all array merge strategies

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>

* fix: use merged values when rendering environment value files

Environment value files (*.yaml.gotmpl) can reference CLI values via
.Values. Previously, only env.Values was passed to template rendering,
which didn't include CLIOverrides.

Now we call env.GetMergedValues() to get Defaults + Values + CLIOverrides
before rendering, so templates can access CLI values like:
  --state-values-set foo=bar

This fixes the state-values-set-cli-args-in-environments integration test.

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>

---------

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>
 2026-01-18 14:04:54 +08:00
Aditya Menon c71648c060 fix: resolve --validate flag conflict with kustomize in Helm 4 (#2362) 
* fix: resolve --validate flag conflict with kustomize in Helm 4

Fixes #2355

In Helm 4, the --validate and --dry-run flags are mutually exclusive.
When using kustomize/chartify charts with helmfile diff --validate,
the code was adding both --validate AND --dry-run=server to the
helm template command, causing the error:

  Error: if any flags in the group [validate dry-run] are set none
  of the others can be; [dry-run validate] were all set

The fix checks if --validate is already set before adding --dry-run=server.
Since --validate already provides server-side validation (it was deprecated
in favor of --dry-run=server in Helm 4), adding --dry-run=server is
redundant when --validate is present.

Changes:
- Add !opts.Validate condition to processChartification() in state.go
- Add comprehensive unit tests for validate/dry-run mutual exclusion
- Add integration test with kustomize chart to prevent regression

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>

* address review feedback from Copilot

- Add missing test cases for destroy, delete, test, status WITH --validate
- Update integration test to use 'diff' instead of 'template' to properly
  exercise the cluster-requiring code path that triggers --dry-run=server
- Add sync warning comments to the test helper function noting it must be
  kept in sync with processChartification() in state.go

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>

* add missing 'build with validate' test case

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>

* address additional review feedback from Copilot

- Fix integration test to capture output and exit code in single execution
  instead of running helmfile twice (more efficient)
- Add detailed documentation explaining why test helper duplication is
  intentional: extracting shared function would require exposing internal
  API and complex refactoring of processChartification dependencies
- Note that integration test exercises actual code path end-to-end

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>

* fix: correct go doc comment formatting for gci linter

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>

* fix: update line number reference from 1497-1523 to 1497-1524

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>

---------

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>
 2026-01-18 13:59:55 +08:00
Aditya Menon c4a828686e fix: pass --kube-context to helm template when using jsonPatches (#2363) 
fix: pass --kube-context to helm template when using jsonPatches (#2309)

When using jsonPatches or strategicMergePatches in helmfile, the
`helm template` command was not receiving the `--kube-context` flag.
This caused issues when `--dry-run=server` was used (introduced in
PR #2271 to support lookup() functions), because helm would connect
to the wrong cluster context.

Root Cause:
1. `flagsForTemplate()` did not call `appendConnectionFlags()`, unlike
   `flagsForUpgrade()` and `flagsForDiff()` which both include this call.
2. `processChartification()` did not include `--kube-context` when
   setting `chartifyOpts.TemplateArgs` for internal helm template calls.

Fix:
1. Added `appendConnectionFlags()` call to `flagsForTemplate()` to ensure
   kube-context and other connection flags are passed to helm template.
2. Added `getKubeContext()` helper function that resolves kube-context
   with proper priority: release > environment > helmDefaults.
3. Modified `processChartification()` to include `--kube-context` in
   chartifyOpts.TemplateArgs when chartify needs to run helm template.
4. Added compatibility check for `--validate` flag to avoid Helm 4
   mutual exclusion error between --validate and --dry-run (Issue #2355).

Fixes #2309

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>
 2026-01-16 20:32:33 +08:00
Aditya Menon 534d0b618c build(deps): update Helm v4 to 4.0.1 and helm-secrets to 4.7.4 (#2304) 
* build(deps): update Helm v4 from 4.0.0 to 4.0.1

Update Helm v4 binary and Go library dependency to version 4.0.1.

Changes:
- Update helm.sh/helm/v4 Go module from v4.0.0 to v4.0.1
- Update Helm binary version in all Dockerfiles (alpine, ubuntu, debian)
- Update SHA256 checksums for linux/amd64 and linux/arm64
- Update CI workflow matrix to test against v4.0.1
- Update HelmRecommendedVersion constant in pkg/app/init.go
- Update test mocks to return v4.0.1 version string
- Update test plugin fixture version

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>

* build(deps): update helm-secrets from 4.7.0 to 4.7.4

Update helm-secrets plugin version across all configurations:
- Docker images (all 3 variants) - use ARG variable for version
- CI test matrix
- Integration test defaults
- Unit test fixtures and expectations
- HelmSecretsRecommendedVersion constant
- Dynamic plugin installation in exec.go

Also update plugin filename format from helm-secrets-*.tgz to
secrets-{version}.tgz to match the new release naming convention.

Update suppress-output-line-regex test expected output for Helm 4.0.1
which now suppresses Service diff after ipFamily normalization.

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>

---------

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>
 2025-11-28 08:43:54 +08:00
Aditya Menon 9c70adc038 fix: resolve issues #2295, #2296, and #2297 (#2298) 
* fix: resolve issues #2295, #2296, #2297 and OCI registry login

This PR fixes four related bugs affecting chart preparation, caching,
and OCI registry authentication.

Issue #2295: OCI chart cache conflicts with parallel helmfile processes
- Added filesystem-level locking using flock for cross-process sync
- Implements double-check locking pattern for efficiency
- Retry logic with 5-minute timeout and 3 retries
- Refactored into reusable acquireChartLock() helper function
- Added refresh marker coordination for cross-process cache management

Issue #2296: helmDefaults.skipDeps and helmDefaults.skipRefresh ignored
- Check both CLI options AND helmDefaults when deciding to skip repo sync

Issue #2297: Local chart + transformers causes panic
- Normalize local chart paths to absolute before calling chartify

OCI Registry Login URL Fix:
- Added extractRegistryHost() to extract just the registry host from URLs
- Fixed SyncRepos to use extracted host for OCI registry login
- e.g., "account.dkr.ecr.region.amazonaws.com/charts" ->
        "account.dkr.ecr.region.amazonaws.com"

Test Plan:
- Unit tests for issues #2295, #2296, #2297
- Unit tests for OCI registry login (extractRegistryHost, SyncRepos_OCI)
- Integration tests for issues #2295 and #2297
- All existing unit tests pass (including TestLint)

Fixes #2295
Fixes #2296
Fixes #2297

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>

* fix: replace 60s timeout with reader-writer locks for OCI chart caching

Address PR review feedback from @champtar about the OCI chart caching
mechanism. The previous implementation used a 60-second timeout which
was arbitrary and caused race conditions when helm deployments took
longer (e.g., deployments triggering scaling up/down).

Changes:
- Replace 60s refresh marker timeout with proper reader-writer locks
- Use shared locks (RLock) when using cached charts (allows concurrent reads)
- Use exclusive locks (Lock) when refreshing/downloading charts
- Hold locks during entire helm operation lifecycle (not just during download)
- Add getNamedRWMutex() for in-process RW coordination
- Update PrepareCharts() to return locks map for lifecycle management
- Add chartLockReleaser in run.go to release locks after helm callback
- Remove unused mutexMap and getNamedMutex (replaced by RW versions)
- Add comprehensive tests for shared/exclusive lock behavior

This eliminates the race condition where one process could delete a
cached chart while another process's helm command was still using it.

Fixes review comment on PR #2298

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>

* fix: prevent deadlock when multiple releases share the same chart

When multiple releases use the same OCI chart (e.g., same chart different
values), workers in PrepareCharts would deadlock:

1. Worker 1 acquires lock for chart/path, downloads, adds to cache
2. Worker 2 finds chart in cache, tries to acquire lock on same path
3. Worker 2 blocks waiting for Worker 1's lock
4. Collector waits for Worker 2's result
5. Worker 1's lock held until PrepareCharts finishes -> deadlock

The fix: when using the in-memory chart cache (which means another worker
in the same process already downloaded the chart), don't acquire another
lock. This is safe because:
- The in-memory cache is only used within a single helmfile process
- The tempDir cleanup is deferred until after helm callback completes
- Cross-process coordination is still handled by file locks during downloads

This fixes the "signal: killed" test failures in CI for:
- oci_chart_pull_direct
- oci_chart_pull_once
- oci_chart_pull_once2

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>

* fix: resolve deadlock by releasing OCI chart locks immediately after download

This commit simplifies the OCI chart locking mechanism to fix deadlock
issues that occurred when multiple releases shared the same chart.

Problem:
When multiple releases used the same OCI chart, workers in PrepareCharts
would deadlock because:
1. Worker 1 acquires lock for chart/path, downloads chart
2. Worker 2 tries to acquire lock on same path, blocks waiting
3. PrepareCharts waits for all workers to complete
4. Worker 1's lock held until PrepareCharts finishes -> deadlock

Solution:
Release locks immediately after chart download completes. This is safe
because:
- The tempDir cleanup is deferred until after helm operations complete
  in withPreparedCharts(), so charts won't be deleted mid-use
- The in-memory chart cache prevents redundant downloads within a process
- Cross-process coordination via file locks still works during download

Changes:
- Remove chartLock field from chartPrepareResult struct
- Release locks immediately in getOCIChart() and forcedDownloadChart()
- Simplify PrepareCharts() by removing lock collection and release logic
- Update function signatures to return only (path, error)

This also fixes the "signal: killed" test failures in CI for:
- oci_chart_pull_direct
- oci_chart_pull_once
- oci_chart_pull_once2

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>

* fix: add double-check locking for in-memory chart cache

When multiple workers concurrently process releases using the same chart,
they all check the in-memory cache before acquiring locks. If none have
populated the cache yet, all workers miss and try to download.

Previously, even after acquiring the exclusive lock, the code would
re-download the chart when needsRefresh=true (the default). This caused
multiple "Pulling" messages in tests like oci_chart_pull_once.

The fix adds a second in-memory cache check AFTER acquiring the lock.
This implements proper double-check locking:

1. Check cache (outside lock) → miss
2. Acquire lock
3. Check cache again (inside lock) → hit if another worker populated it
4. If still miss, download and add to cache

This ensures only one worker downloads the chart, while others use
the cached version populated by the first worker.

Changes:
- Add in-memory cache double-check in getOCIChart() after acquiring lock
- Add in-memory cache double-check in forcedDownloadChart() after acquiring lock

This fixes the oci_chart_pull_once and oci_chart_pull_direct test failures
where charts were being pulled multiple times instead of once.

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>

* fix: use callback to prevent redundant chart downloads within a process

When multiple workers concurrently process releases using the same chart,
they need to coordinate to avoid redundant downloads. The previous fix
set SkipRefresh=true for OCI charts, which prevented legitimate refresh
scenarios (e.g., floating tags).

This commit implements a better solution using a callback mechanism:

1. acquireChartLock() now accepts an optional skipRefreshCheck callback
2. Before deleting a cached chart for refresh, the callback is invoked
3. If the callback returns true (in-memory cache has the chart), skip refresh
4. This allows deduplication within a process while respecting cross-run refresh

The flow is now:
- Worker 1 downloads chart, adds to in-memory cache, releases lock
- Worker 2 acquires lock, sees needsRefresh=true, but callback sees
  in-memory cache is populated → uses cached instead of deleting

This correctly handles:
- Within-process deduplication: only one download per chart
- Cross-run refresh: respects --skip-refresh flag for floating tags
- Immutable versions: cached and reused as expected

Changes:
- Add skipRefreshCheck callback parameter to acquireChartLock()
- Update getOCIChart() to pass in-memory cache check callback
- Update forcedDownloadChart() to pass in-memory cache check callback
- Remove SkipRefresh=true workaround for OCI charts

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>

* fix: address Copilot review comments on PR #2298

This commit addresses the automated review comments from GitHub Copilot:

1. pkg/state/state.go: Add nil check for logger in Release() method
   to prevent potential nil pointer dereference when logger is nil.

2. pkg/state/state.go: Fix misleading comment about "external callers"
   to accurately reflect that Logger() is used by the app package.

3. pkg/state/issue_2296_test.go: Add comment noting that boolPtr helper
   is already defined in skip_test.go (shared across test files).

4. test/integration/test-cases/oci-parallel-pull.sh: Replace hardcoded
   /tmp paths with a dedicated temp directory for test outputs. Add
   cleanup for the output directory in the cleanup function.

5. test/integration/test-cases/issue-2297-local-chart-transformers.sh:
   Add cleanup trap to remove temp directory on exit, preventing
   leftover files from accumulating.

6. Remove dead code: The chartLocks map in PrepareCharts was always
   empty since locks are released immediately after download. Removed
   the unused return value and corresponding handling in run.go to
   improve code clarity and maintainability.

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>

* fix: make oci-parallel-pull test resilient to registry issues

The integration test was intermittently failing in CI due to Docker Hub
rate limiting or network issues. These failures are not helmfile bugs.

Changes:
- Add is_registry_error() function to detect external registry issues
  (rate limits, network timeouts, connection refused, etc.)
- Check for the race condition bug (issue #2295) first and fail fast
- If other failures occur, check if they're registry-related
- Skip test gracefully when registry issues are detected instead of
  failing CI on external infrastructure problems

This ensures the test still catches the actual race condition bug while
not causing false failures due to Docker Hub rate limits in CI.

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>

* fix: make oci-parallel-pull test resilient to registry issues

The integration test was failing in CI for two reasons:

1. Docker Hub rate limiting or network issues causing helmfile to fail
2. The test script exits early due to `set -e` when `wait` returns non-zero

Changes:
- Use `wait $pid || exit=$?` pattern to capture exit codes without triggering
  set -e. When wait returns non-zero, the || branch captures the exit code
  into the variable, preventing script termination.
- Add is_registry_error() function to detect external registry issues
  (rate limits, network timeouts, connection refused, etc.)
- Check for the race condition bug (issue #2295) first and fail fast
- Skip test gracefully when registry issues are detected instead of
  failing CI on external infrastructure problems

This ensures the test still catches the actual race condition bug while
not causing false failures due to Docker Hub rate limits in CI.

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>

* fix: address PR #2298 review - reinitialize fileLock after release

Address Copilot review comments:

1. pkg/state/state.go: Reinitialize fileLock after releasing shared lock
   When upgrading from shared to exclusive lock, the fileLock needs to be
   reinitialized with flock.New() after calling Release(). This ensures
   a fresh flock object is used for the exclusive lock acquisition.

2. test/integration/test-cases/oci-parallel-pull.sh: Add lock file
   verification warning if no lock files are found, to ensure the
   locking mechanism is actually being tested.

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>

* fix: address PR #2298 Copilot review comments (round 4)

Address 8 Copilot review comments:

1. pkg/state/state.go: Release in-process mutex during retry backoff
   to avoid blocking other goroutines for up to 90 seconds.

2. pkg/state/state.go: Include chartPath in shared lock error message
   for better debugging.

3. pkg/state/state.go: Document that extractRegistryHost does not handle
   URLs with query parameters or fragments (uncommon for OCI registries).

4. pkg/state/state.go: Document that skipRefreshCheck callback should be
   fast and non-blocking since it runs while holding exclusive lock.

5. oci-parallel-pull.sh: Use case-insensitive grep (-i flag) to catch
   error variations like "I/O timeout".

6. helmfile.yaml: Expand comment explaining why library charts can't be
   used for this test (they can't be templated by Helm).

Skipped (with justification):
- PrepareChartKey helper: Only 2 usages with different source structs
- Context reuse in retry: Per-attempt contexts provide clearer semantics

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>

* fix: address PR #2298 Copilot review comments (round 5)

1. Make race condition detection grep more robust (oci-parallel-pull.sh)
   - Use case-insensitive extended regex (-iqE)
   - Add multiple pattern variations to catch different tar/helm versions

2. Remove unused Logger() method from HelmState (state.go)
   - Method was never called; all lock releases use st.logger directly

3. Add clarifying comments for lock retry behavior (state.go)
   - Document why file system errors are retried but timeouts are not
   - Explain flock returns (false, nil) on context deadline exceeded

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>

* fix: clarify lock file check is informational only

Lock files are ephemeral and may be cleaned up immediately after
helmfile processes complete. Update comments and warning message
to make clear their absence doesn't indicate locking wasn't used.

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>

* fix: add HELM_BIN env var to Dockerfiles

The helm-git plugin requires HELM_BIN environment variable to be set.
Without it, the plugin fails with "HELM_BIN: parameter not set".

Add HELM_BIN=/usr/local/bin/helm to all Dockerfile variants.

Fixes #2303

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>

---------

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>
 2025-11-27 22:13:03 +08:00
Aditya Menon d40bfced56 test: add integration test for issue #2291 (CRD preservation with strategicMergePatches) (#2292) 
* test: add integration test for issue #2291 with all fixes

Add comprehensive integration test for issue #2291 that validates CRD
preservation when using strategicMergePatches with chartify.

Problem:
When using strategicMergePatches, chartify was relocating CRDs from
templates/crds/ to root crds/ directory, changing how Helm manages them.
This caused helm diff to incorrectly show CRDs as being removed, even
though they were still present.

Solution:
Chartify now preserves the original CRD location in templates/crds/.
This integration test validates the fix by:
1. Templating a chart with CRDs in templates/crds/
2. Applying the chart with strategicMergePatches
3. Verifying CRD is installed
4. Running helm diff to ensure CRD is NOT marked for removal
5. Verifying the strategic merge patch was applied

Additional fixes included in this commit:
- Fixed grep command error when matching YAML deletion patterns
- Updated expected test output for Helm 4 diff behavior
- Fixed EXIT trap interference between test cases
- Added --plain-http flag for Helm 4 OCI registry compatibility
- Ensured CRD templates are valid (cluster-scoped, no namespace)
- Fixed strategic merge patch namespace matching

Test coverage:
- CRD preservation in templates/crds/ subdirectory
- Strategic merge patch application
- Helm diff behavior with CRDs
- Integration with chartify kustomize processing

Fixes #2291

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>

* switch chartify package to upstream one

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>

* implement copilot suggestion

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>

---------

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>
 2025-11-25 09:19:41 +08:00
Aditya Menon b91fd534ec Fix four critical bugs: array merging (#2281), AWS SDK logging (#2270), helmDefaults skip flags (#2269), and OCI chart versions (#2247) (#2288) 
* fix: resolve issues #2281, #2270, #2269, and #2247

This commit addresses four critical bugs in helmfile:

1. **Issue #2281**: Fix array merging in --state-values-set
   - Problem: Arrays were being replaced entirely instead of merged element-by-element
   - Root cause: MergeMaps() didn't handle arrays, and mergo.Merge was used in some places
   - Solution:
     * Enhanced MergeMaps() with mergeSlices() and toInterfaceSlice() functions
     * Replaced mergo.Merge calls with MergeMaps in environment.go and create.go
     * Arrays now merge element-by-element, with nested maps merged recursively
   - Files changed:
     * pkg/maputil/maputil.go - Added array merging logic
     * pkg/maputil/maputil_test.go - Added comprehensive unit tests
     * pkg/environment/environment.go - Use MergeMaps instead of mergo.Merge
     * pkg/state/create.go - Use MergeMaps instead of mergo.Merge
     * test/integration/test-cases/issue-2281-array-merge/ - Integration test
     * test/integration/run.sh - Added new integration test

2. **Issue #2270**: Suppress AWS SDK debug logging
   - Problem: AWS SDK debug logs exposing sensitive information (tokens, auth headers)
   - Root cause: vals.New() called without LogOutput option
   - Solution: Set LogOutput to io.Discard in ValsInstance()
   - Files changed:
     * pkg/plugins/vals.go - Added LogOutput: io.Discard option

3. **Issue #2269**: Fix helmDefaults.skipDeps and helmDefaults.skipRefresh being ignored
   - Problem: skipRefresh only checked CLI flags, not helmDefaults or release settings
   - Root cause: Incomplete calculation at line 1559 in state.go
   - Solution: Added proper skipRefresh calculation mirroring skipDeps logic
   - Files changed:
     * pkg/state/state.go - Fixed skipRefresh calculation (lines 1522-1525, 1564)
     * pkg/state/skip_test.go - Added unit tests for skipDeps and skipRefresh

4. **Issue #2247**: Allow OCI charts without explicit version
   - Problem: OCI charts without version defaulted to "latest" which was then rejected
   - Root cause: getOCIQualifiedChartName() defaulted chartVersion to "latest"
   - Solution: Use release.Version directly without defaulting, only reject explicit "latest"
   - Files changed:
     * pkg/state/state.go - Remove default to "latest", use empty string
     * pkg/state/oci_chart_version_test.go - Added comprehensive unit tests
     * test/integration/test-cases/issue-2247/ - Integration test with registry
     * test/integration/run.sh - Added new integration test

Fixes #2281, #2270, #2269, #2247

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>

* fix: correct integration test for issue #2281 array merging

The helmfile template needed to pass the 'top' values to the chart
so that .Values.top is accessible in the template context.

Changes:
- Pass state values to chart values using toYaml
- Adjusted indentation for proper YAML structure
- Template now correctly accesses .Values.top for array data

Test output now matches expected output with proper element-by-element
array merging.

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>

* fix: make Helm version parsing more robust in issue-2247 test

Improved version parsing to handle edge cases in CI environments:
- Added fallback to 3.8 if version parsing fails
- Added default values for HELM_MAJOR and HELM_MINOR
- Prevents test failures due to version detection issues

This ensures the test runs correctly across different environments
and Helm versions.

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>

* debug: add diagnostic output for issue-2247 test failure

Added debug logging to show:
- helmfile command output when it succeeds unexpectedly
- Helm version being used by the test

This will help diagnose why the validation isn't triggering in CI.

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>

* fix: make OCI 'latest' validation work for all Helm versions

The validation for explicit 'latest' in OCI charts was depending on
helm.IsVersionAtLeast("3.8.0") which could fail if Helm version
detection has issues in CI environments.

Changes:
- Remove Helm version check from validation
- Always reject explicit 'latest' for OCI charts
- Remove Helm version check from integration test
- Update unit tests to expect 'latest' to fail for all Helm versions

This ensures consistent behavior across all environments and
Helm versions, fixing the CI failure where helm version detection
was problematic.

Fixes integration test failure in CI.

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>

* fix: remove unused helm parameter from getOCIQualifiedChartName

Since the Helm version check was removed from the OCI validation,
the helm parameter is no longer needed in getOCIQualifiedChartName.

Changes:
- Removed helm parameter from function signature
- Updated all callers to not pass helm argument
- Removed unused mockHelmExec test implementation
- Removed unused imports (testutil, helmexec, chart)

This resolves the golangci-lint unparam error.

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>

* test: update TestGetOCIQualifiedChartName to expect 'latest' rejection

Updated test case for Helm 3.7.0 to expect error when using 'latest'
since we now reject explicit 'latest' for all Helm versions, not just
>= 3.8.0.

This aligns the test with the updated validation logic that ensures
consistent behavior across all Helm versions.

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>

* fix: handle set -e in issue-2247 integration test

The integration test script is sourced by run.sh which has `set -e`
enabled. When helmfile commands fail (as expected for validation tests),
the script would exit immediately before capturing the exit code.

This fix temporarily disables `set -e` around each helmfile command that
may fail, allowing proper exit code capture and validation.

This resolves the persistent CI test failure where the test would exit
at Test 1.1 without showing any error message.

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>

* fix: add set -e handling for helm commands in issue-2247 test

Extends the previous set -e fix to cover helm package and push commands
in the registry tests (Test 2.2). These commands can fail and need proper
error handling without triggering immediate script exit.

This ensures:
- helm package failures are caught and handled gracefully
- helm push failures are caught and handled gracefully
- Test can skip registry tests and pass with validation-only results
- set -e is properly re-enabled after each command sequence

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>

---------

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>
 2025-11-22 09:27:51 +08:00
Aditya Menon c8bcbcd629 🐛 Fix four critical issues: environment merging, kubeVersion detection, lookup() with kustomize, and Helm 4 color flags (#2276) 
* fix: deep merge environments from multiple bases (#2273)

Problem:
When using multiple base helmfiles, environment values were being
completely replaced instead of deep-merged due to mergo.WithOverride
introduced in PR #2228.

Solution:
- Created mergeEnvironments() function for proper deep merging
- Manually merge environment Values and Secrets slices before struct merge
- Preserves all environment values from both base and current helmfile

Testing:
- Added TestEnvironmentMergingWithBases with two scenarios:
  1. Multiple bases with overlapping environment values
  2. Environment values with array merging

Fixes #2273

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>

* fix: auto-detect Kubernetes version for helm-diff (#2275)

Problem:
When helmfile runs helm-diff without specifying kubeVersion, helm-diff
falls back to v1.20.0. This causes chart compatibility checks to fail
for charts requiring newer Kubernetes versions (e.g., kubeVersion: ">=1.25.0").

Root Cause:
- flagsForDiff() was not passing kubeVersion to helm-diff plugin
- Without --kube-version flag, helm-diff uses default v1.20.0

Solution:
- Created pkg/cluster package with DetectServerVersion() function
- Auto-detect cluster version using k8s.io/client-go discovery API
- Pass detected version to helm-diff via --kube-version flag
- Priority: helmfile.yaml kubeVersion > auto-detected version
- Works with both Helm 3 and Helm 4

Implementation:
- pkg/cluster/version.go: Cluster version detection
- pkg/app/app.go: detectKubeVersion() helper used in diff() and apply()
- pkg/state/state.go: Added DetectedKubeVersion field to DiffOpts
- Integrated into flagsForDiff() with proper precedence

Testing:
- Unit tests for cluster version detection
- Unit tests for kubeVersion precedence logic
- Integration test with chart requiring Kubernetes >=1.25.0
- Tests verify upgrade scenario (critical failure case from issue)
- Validated with both Helm 3 and Helm 4

Fixes #2275

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>

* fix: enable lookup() function with strategicMergePatches (#2271)

Problem:
When using strategicMergePatches (kustomize), Helm's lookup() function
stops working. Charts like Grafana use lookup() to preserve existing
resource values (e.g., PVC volumeName), which get lost when using patches.

Root Cause:
- Chartify runs "helm template" to render charts before applying patches
- By default, "helm template" runs client-side without cluster access
- The lookup() function requires cluster connectivity to query resources
- Without cluster access, lookup() returns empty values

Solution:
- Pass --dry-run=server to helm template when using kustomize patches
- This enables cluster connectivity for lookup() while keeping client-side rendering
- Only applied to commands requiring cluster access (diff, apply, sync, etc.)
- Offline commands (template, lint, build) remain cluster-independent

Implementation:
- Modified processChartification() to accept helmfileCommand parameter
- Added switch-based logic to determine cluster requirement per command
- Conditionally set chartifyOpts.TemplateArgs = "--dry-run=server"
- Safe default: unknown commands assume cluster access

Command Behavior:
- helmfile diff/apply/sync: Uses --dry-run=server, lookup() works
- helmfile template/lint/build: No cluster requirement, works offline
- Charts without lookup(): Unaffected
- Charts with lookup() + cluster: Lookup values preserved correctly

Testing:
- Integration test with ConfigMap using lookup() to preserve values
- Verifies lookup works with strategicMergePatches
- Tests both with and without cluster access
- Validates offline template command still works

Fixes #2271

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>

* fix: remove unnecessary error return from mergeEnvironments

The mergeEnvironments function always returns nil, making the error
return value unnecessary. This fixes the unparam linter warning.

- Changed function signature to not return error
- Updated call site to not handle error
- All tests still pass

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>

* fix: handle nil Environments map in mergeEnvironments

Fixes panic when base helmfile has nil Environments map.
Initialize the destination map if nil before merging to prevent
"assignment to entry in nil map" panic.

- Added nil check in mergeEnvironments to return early
- Initialize layers[0].Environments before merge if nil
- Fixes TestVisitDesiredStatesWithReleasesFiltered_Issue1008_MissingNonDefaultEnvInBase

The panic occurred when a base helmfile didn't define any environments
but a subsequent layer did. Now we properly initialize an empty map
to merge into.

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>

* test: disable kubeVersion auto-detection in unit tests

Add DisableKubeVersionAutoDetection field to App struct to prevent
unit tests from connecting to real Kubernetes clusters during testing.

The kubeVersion auto-detection feature (issue #2275) was causing
unit tests to fail because:
1. Tests use mock helm implementations without real cluster access
2. Auto-detection was connecting to local minikube cluster (v1.34.0)
3. Test expectations didn't include --kube-version flag in diff keys

Solution:
- Add DisableKubeVersionAutoDetection bool field to App struct
- Check this flag in detectKubeVersion() before attempting detection
- Set flag to true in all pkg/app/*_test.go files

This ensures unit tests remain isolated and don't depend on
external cluster state while preserving auto-detection for
production use.

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>

* chore: upgrade helm-diff plugin to v3.14.1

Update helm-diff plugin from v3.14.0 to v3.14.1 across all environments:
- Dockerfiles (main, debian-stable-slim, ubuntu)
- CI workflow matrix configurations
- Integration test default version

This ensures consistency across development, testing, and production
environments.

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>

* test: fix table formatting and improve E2E test infrastructure

This commit addresses multiple test failures and improves the testing
infrastructure for better reliability and maintainability.

Table Formatting Fixes:
- Added trimTrailingWhitespace() helper function to remove trailing
  whitespace from table output in both FormatAsTable() and printDAG()
- Fixes TestList and TestDAG failures caused by tabwriter padding
  empty columns with trailing spaces
- Updated golden file for table output test to match new behavior

E2E Test Infrastructure Improvements:
- Implemented dynamic port allocation for Docker registry tests to
  prevent port conflicts (replaced hardcoded port 5000/5001)
- Added getFreePort() function using kernel-allocated unused ports
- Added waitForRegistry() function with proper health check polling
  of Docker Registry /v2/ endpoint (replaces sleep hack)
- Added prepareInputFile() function to handle port substitution and
  path resolution when copying helmfile configs to temp directories
- Extracted setupLocalDockerRegistry() helper to reduce cognitive
  complexity from 111 to ≤110 (gocognit threshold)
- Added port normalization in test output to replace dynamic ports
  with $REGISTRY_PORT placeholder for deterministic comparisons

Test Configuration Updates:
- Updated OCI chart tests to use dynamic port allocation via
  $REGISTRY_PORT placeholder in helmfile configs
- Converted relative chart paths to absolute paths when input files
  are copied to temp directories (fixes path resolution issues)
- Left postrenderer paths as relative since they're resolved from
  working directory (works for both Helm 3 and Helm 4)

Golden File Updates:
- Updated all OCI-related test expected outputs to use $REGISTRY_PORT
  placeholder instead of hardcoded ports
- Removed trailing whitespace from issue_493 test expected output
- Updated postrenderer test outputs to reflect chart path normalization

Test Cleanup:
- Removed unused fakeInit struct and CheckHelmPlugins() call from
  snapshot tests (not needed for template/fetch/list commands)
- Removed unused imports (app, helmexec packages)

Technical Details:
- Port allocation uses net.Listen with port 0 for kernel assignment
- Registry health check polls with 500ms intervals and 30s timeout
- Chart paths: ../../charts/* → absolute paths (input file moves to temp)
- Postrenderer paths: remain relative (resolved from working directory)
- OCI cache paths normalized: oci__localhost_PORT → oci__localhost_$REGISTRY_PORT

All originally failing tests now pass:
- TestList ✓
- TestDAG ✓
- TestHelmfileTemplateWithBuildCommand (all OCI tests) ✓
- TestFormatAsTable ✓

Fixes three test failures reported in issue.

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>

* fix(test): convert postrenderer paths to absolute for Helm 3

Helm 3 resolves postrenderer script paths relative to the helmfile
location. When the input file is copied to a temp directory for
port substitution, relative postrenderer paths break.

Solution:
- Added postrenderersDir parameter to prepareInputFile()
- Convert ../../postrenderers/* to absolute paths for Helm 3 only
- Use existing isHelm4() function to detect Helm version
- Helm 4 extracts plugin names from paths, so works with relative

This fixes the postrenderer test failure in CI where Helm 3 could
not find the postrenderer script at the relative path.

Fixes: Error: unable to find binary at ../../postrenderers/add-cm2.bash
Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>

* fix(test): remove remaining hardcoded port 5001 in OCI tests

Updated 4 remaining OCI chart tests that still had hardcoded port 5001:
- oci_chart_pull
- oci_chart_pull_once
- oci_chart_pull_once2
- oci_chart_pull_direct

Changes:
- config.yaml: Removed hardcoded port, use dynamic allocation
- input.yaml.gotmpl: Replaced localhost:5001 with localhost:$REGISTRY_PORT

This ensures all OCI chart tests use dynamic port allocation to
prevent port conflicts during parallel test execution.

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>

* fix: prevent helm-diff from normalizing server-side defaults

Problem:
The suppress-output-line-regex integration test was failing because
helm-diff was reporting "has changed, but diff is empty after suppression"
for Service resources when it should have shown ipFamilyPolicy and ipFamilies
fields being removed.

Root Cause:
When auto-detected kubeVersion (e.g., 1.34.0) is passed to helm-diff via
--kube-version flag, helm-diff normalizes server-side defaults. This makes
fields like ipFamilyPolicy and ipFamilies appear unchanged, even though they
don't exist in the chart template and will be removed by the upgrade.

After applying suppressOutputLineRegex patterns, only label changes remained
(helm.sh/chart and app.kubernetes.io/version). These were correctly suppressed,
leaving an empty diff - hence the "diff is empty after suppression" message.

Solution:
Added a new configuration option 'disableAutoDetectedKubeVersionForDiff' to allow
disabling auto-detected kubeVersion being passed to helm-diff. This prevents
helm-diff from normalizing server-side defaults when needed.

Default behavior: Pass auto-detected kubeVersion (fixes issue #2275, existing behavior)
Opt-out behavior: Set flag to true to only use explicit kubeVersion from helmfile.yaml

helmDefaults:
  disableAutoDetectedKubeVersionForDiff: true  # false by default

releases:
- name: myrelease
  disableAutoDetectedKubeVersionForDiff: true  # override per-release

Implementation:
- Added DisableAutoDetectedKubeVersionForDiff field to HelmSpec and ReleaseSpec
- Updated flagsForDiff() to check this flag before passing kubeVersion
- Default (false): pass auto-detected kubeVersion (fixes issue #2275)
- Opt-out (true): only pass explicit kubeVersion from helmfile.yaml
- Updated suppress-output-line-regex test to disable auto-detected kubeVersion

This approach:
- Maintains backward compatibility (default passes auto-detected kubeVersion)
- Fixes issue #2275 for charts requiring newer Kubernetes versions
- Allows users to opt-out when server-side normalization causes issues
- Fixes suppress-output-line-regex test regression

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>

* test: update hash values in TestGenerateID after adding DisableAutoDetectedKubeVersionForDiff field

The hash values in TestGenerateID needed to be updated because adding the
DisableAutoDetectedKubeVersionForDiff field to ReleaseSpec changed the structure's
hash representation. This is expected behavior as generateValuesID() hashes the
entire ReleaseSpec structure.

Updated all expected hash values to match the new values:
- baseline: foo-values-66f7fd6f7b
- different bytes content: foo-values-6664979cd7
- different map content: foo-values-78897dfd49
- different chart: foo-values-64b7846cb7
- different name: bar-values-576cb7ddc7
- specific ns: myns-foo-values-6c567f54c

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>

* fix: address PR review comments and resolve issue #2280

This commit addresses all review comments from GitHub Copilot and
resolves issue #2280 regarding --color flag conflict with Helm 4.

Changes:
1. Fixed documentation in pkg/cluster/version.go
   - Updated function comment to reflect error return behavior
   - Corrected version format example and comment

2. Added complete command categorization in pkg/state/state.go
   - Added all helmfile commands to cluster access switch statement
   - Properly categorized 15+ commands based on cluster requirements
   - Added clarifying comments for command groups

3. Resolved issue #2280: --color flag conflict with Helm 4
   - In Helm 4, --color expects a value (never/auto/always)
   - Converts --color to --color=always for Helm 4
   - Converts --no-color to --color=never for Helm 4
   - Prevents Helm from consuming next argument as color value
   - Added comprehensive unit tests
   - Added integration test (Helm 4 only)

Issue #2280 Details:
When running helmfile diff with --color and --context flags on Helm 4,
the --color flag would consume --context as its value, resulting in:
"invalid color mode '--context': must be one of: never, auto, always"

The fix detects Helm 4 and converts boolean color flags to the format
Helm 4 expects, preventing the argument consumption issue.

Fixes #2280

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>

* fix: correct kubeVersion precedence comment in test

The comment incorrectly stated that state.KubeVersion takes precedence
over paramKubeVersion, but the actual implementation (getKubeVersion in
state.go:3354-3364) shows the correct order is:

1. paramKubeVersion (auto-detected from cluster)
2. release.KubeVersion (per-release override)
3. state.KubeVersion (helmfile.yaml global setting)

Updated the comment to match the implementation and the test cases.

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>

* fix: resolve Helm 4 --color flag conflict (issue #2280)

This commit resolves issue #2280 where the --color flag causes Helm 4
to consume the next argument, resulting in errors like:
"invalid color mode '--context': must be one of: never, auto, always"

Root Cause:
In Helm 4, the --color flag is parsed by the Helm binary before being
passed to plugins like helm-diff. This causes Helm to interpret the
next argument (e.g., --context) as the value for --color.

Solution:
Remove --color and --no-color flags from helm-diff commands when using
Helm 4, and instead use the HELM_DIFF_COLOR environment variable.
The helm-diff plugin supports HELM_DIFF_COLOR=[true|false] as an
alternative to the --color/--no-color flags.

Changes:
1. Added filterColorFlagsForHelm4() function in pkg/helmexec/exec.go
   - Removes --color and --no-color flags from flags slice
   - Sets HELM_DIFF_COLOR=true for --color
   - Sets HELM_DIFF_COLOR=false for --no-color

2. Modified DiffRelease() to call filterColorFlagsForHelm4() on Helm 4

3. Added comprehensive unit tests in pkg/helmexec/exec_test.go
   - Test_DiffRelease_ColorFlagHelm4: Verifies flags are filtered
   - Test_FilterColorFlagsForHelm4: Tests all flag combinations

4. Added integration test in test/integration/test-cases/issue-2280.sh
   - Tests the exact scenario from issue #2280
   - Verifies --color and --context flags work together
   - Helm 4 only test (skipped on Helm 3)

Fixes #2280

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>

* refactor: apply Copilot code review nitpicks

This commit addresses minor code quality improvements suggested by
GitHub Copilot's automated review.

Changes:
1. pkg/app/formatters.go - Optimize trimTrailingWhitespace()
   - Only modify lines that actually have trailing whitespace
   - Avoids unnecessary string allocations for clean lines
   - Performance optimization for table formatting

2. test/e2e/template/helmfile/snapshot_test.go
   - Use 0600 permissions for temporary input files (was 0644)
   - Improves security by making temp files owner-only read/write
   - Prevents potential exposure of sensitive test data

   - Improve error messages in getFreePort()
   - Wrap errors with context using fmt.Errorf("%w")
   - Better error debugging when port allocation fails

   - Add retry logic to setupLocalDockerRegistry()
   - Handles race condition where port gets taken between allocation and use
   - Retries up to 3 times with new ports on "address already in use" errors
   - Fails fast on other Docker errors for better test diagnostics

All tests passing. These are non-functional improvements that enhance
code quality, performance, security, and test reliability.

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>

* docs: improve code comments based on Copilot feedback

This commit addresses documentation nitpicks from GitHub Copilot's
automated review to improve code clarity and maintainability.

Changes:
1. pkg/app/app.go - Clarify detectKubeVersion() return conditions
   - Updated comment to explicitly list all three cases when empty
     string is returned: kubeVersion already set, auto-detection
     disabled, or detection fails
   - Improves function documentation clarity

2. test/e2e/template/helmfile/snapshot_test.go
   - Added reference to retry logic in getFreePort() comment
   - Points callers to setupLocalDockerRegistry() for proper race
     condition handling example
   - Better guidance for future code maintainers

3. pkg/state/state.go - Explain patches check rationale
   - Added comment explaining why --dry-run=server is only enabled
     when patches are used
   - Clarifies that this is a conservative approach to minimize
     unnecessary cluster connections
   - Documents primary use case (Grafana chart with PVC preservation)

All changes are documentation-only with no functional impact.
All tests passing.

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>

* refactor: enable lookup() for all cluster commands and add defensive check

This commit addresses two Copilot review suggestions to improve code
robustness and functionality.

Changes:
1. pkg/state/state.go - Remove patches requirement for lookup()
   - Previously only enabled --dry-run=server when patches were present
   - Now enables it for ALL cluster-requiring commands
   - Rationale: lookup() function can be used without patches
   - Improves compatibility with charts using lookup() standalone
   - Trade-off: Slightly more cluster connections vs broader support

2. pkg/helmexec/exec.go - Add defensive check for HELM_DIFF_COLOR
   - Only set environment variable if not already present
   - Makes code more defensive for future implementation changes
   - Note: Changes behavior from "last wins" to "first wins"
   - In practice, env map is freshly created so check is precautionary

3. pkg/helmexec/exec_test.go - Update test expectations
   - Changed test case to reflect "first wins" behavior
   - Updated test name and comment for clarity

Breaking behavior change:
- When both --color and --no-color are present, the FIRST flag now
  wins instead of the LAST flag
- This deviates from standard CLI conventions where later flags
  override earlier ones
- However, this is unlikely to affect real usage as users rarely
  specify conflicting flags

All tests passing.

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>

---------

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>
 2025-11-21 08:32:54 +08:00
Aditya Menon 4f275b3667 feat: add Helm 4 support while maintaining Helm 3 compatibility (#2262) 
This commit adds comprehensive support for Helm 4 while maintaining
full backward compatibility with Helm 3. The implementation includes:

- Updated helm version detection to support both Helm 3 and Helm 4
- Added HELMFILE_HELM4 environment variable to control Helm version
- Modified helm execution paths to handle version-specific binaries
- Updated helm plugin installation to support split architecture

- Helm 4: Uses split plugin architecture (3 separate .tgz files)
  - helm-secrets.tgz
  - helm-secrets-getter.tgz
  - helm-secrets-post-renderer.tgz
- Helm 3: Continues using single plugin installation
- Updated Dockerfiles, CI workflows, and core installation code

- Helm 4 requires post-renderers to be plugins, not executable scripts
- Created Helm plugin structure for integration tests
- Updated helmfile.yaml templates to dynamically select renderer type
- Added test plugins: add-cm, add-cm1, add-cm2

- Updated integration tests for Helm 3/4 compatibility
- Created Helm 4 variant expected output files
- Fixed test determinism issues (repo cleanup between iterations)
- Added version-specific output filtering for warnings/messages

- Updated workflows to test both Helm 3 and Helm 4
- Matrix testing across Helm versions
- Updated helm-diff to v3.14.0 for compatibility

- Updated README and docs with Helm 4 information
- Added migration guidance
- Updated version requirements

All changes are backward compatible - existing Helm 3 users will
see no behavior changes.



fix: update Helm 4 lint expected output to match filtered output

The grep filter removes the semver warning, so the expected output
should not include it. Updated lint-helm4 files to match the filtered
output (warning removed, no extra blank line).

Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>
 2025-11-19 07:49:30 +08:00
yxxhero 444275281f Update recommended Helm versions in init.go and run.sh (#2129) 
- Bump HelmDiffRecommendedVersion from v3.12.3 to v3.12.5 in pkg/app/init.go
- Bump default HELM_DIFF_VERSION from 3.12.3 to 3.12.5 in test/integration/run.sh
- Update HelmRecommendedVersion from v3.18.4 to v3.18.5 in pkg/app/init.go

Signed-off-by: yxxhero <aiopsclub@163.com>
 2025-08-14 08:41:43 +08:00
yxxhero a76bec234c refactor(filesystem): add CopyDir method and optimize Fetch function (#2111) 
* refactor(filesystem): add CopyDir method and optimize Fetch function

Signed-off-by: yxxhero <aiopsclub@163.com>

* fix(state): conditionally prepare charts for local helmfile command

Signed-off-by: yxxhero <aiopsclub@163.com>

* fix(state): conditionally prepare charts for local helmfile command

Signed-off-by: yxxhero <aiopsclub@163.com>

* refactor(state): optimize chart path generation and update dependencies

Signed-off-by: yxxhero <aiopsclub@163.com>

* fix(test): update path in fetch-forl-local-chart test

Signed-off-by: yxxhero <aiopsclub@163.com>

* add more test cases

Signed-off-by: yxxhero <aiopsclub@163.com>

---------

Signed-off-by: yxxhero <aiopsclub@163.com>
 2025-07-28 16:10:25 -04:00
yxxhero 687159a65b build: update Helm and plugin versions to v3.18.4 and v3.12.3 (#2093) 
Signed-off-by: yxxhero <aiopsclub@163.com>
 2025-07-09 18:13:21 +08:00