Commit Graph
373 Commits
Author SHA1 Message Date
yxxhero 43aafeaad1 fix: ensure OCI registry login when SkipRepos is set (#2701)
* fix: ensure OCI registry login when SkipRepos is set (#1847)

Commands like build, status, list, and show-dag set SkipRepos: true to
avoid slow helm repo add/update for classic repos. However, this also
skipped helm registry login for OCI registries, causing 401 Unauthorized
errors when pulling OCI charts.

Add a variadic SyncOption parameter (backward compatible) with
WithOCIOnly() that limits repo processing to OCI registries only.
When skipRepos is true, callers now pass WithOCIOnly() so that OCI
authentication still happens before chart pulls.

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

* fix: reword OCI login comments per review feedback

RegistryLogin is a no-op when credentials are not configured, so the
word 'always' was misleading. Clarify that login is only needed when
credentials are present.

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

* fix: skip OCI login for commands that don't pull charts

Commands like 'list' and 'write-values' skip chart preparation entirely,
so OCI registry login is unnecessary for them. Extract the skip-command
list into a shared variable and use it to gate OCI-only login in
WithPreparedCharts.

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

* docs: clarify commandsSkipChartPrep comment per review feedback

Clarify that these commands only skip OCI login when skipRepos is true;
when skipRepos is false, SyncReposOnce still runs normally for all repos.

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

---------

Signed-off-by: yxxhero <aiopsclub@163.com>
2026-07-12 09:51:24 +08:00
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
Arthur Garreauandyxxhero cc85562625 feat: Add ConditionTemplate support in releaseSpec (#2669)
* feat: Add ConditionTemplate support in releaseSpec

Signed-off-by: Arthur Garreau <arthur.garreau98@gmail.com>

* feat: improve testing, clarify doc

Signed-off-by: Arthur Garreau <arthur.garreau98@gmail.com>

* feat: update CHANGELOG for ConditionTemplate support and fix test cases for ID generation

Signed-off-by: Arthur Garreau <arthur.garreau98@gmail.com>

* Update pkg/state/state_exec_tmpl.go

Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com>
Signed-off-by: Arthur Garreau <arthur.garreau98@gmail.com>

* Update pkg/state/state_exec_tmpl.go

Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com>
Signed-off-by: Arthur Garreau <arthur.garreau98@gmail.com>

* Update docs/configuration.md

Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com>
Signed-off-by: Arthur Garreau <arthur.garreau98@gmail.com>

* refactor: improve comments for condition checks and clean up whitespace

Signed-off-by: Arthur Garreau <arthur.garreau98@gmail.com>

---------

Signed-off-by: Arthur Garreau <arthur.garreau98@gmail.com>
Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com>
2026-07-01 18:44:43 +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 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
yxxhero 0cf330611e fix: clean up chartify temp directories after helm operations (#2668)
fix: clean up chartify temp directories after helm operations (#1799)

Chartify creates temporary output directories (e.g. /tmp/chartify<random>/)
during chart preparation for commands like build, template, and diff. These
directories were never tracked for cleanup, causing disk space to accumulate
over time with thousands of orphaned chartify* folders.

The existing clean() closure in PrepareChartify only removed generated values
files, not the chartify output directory itself. The chartified chart must
survive until all helm operations complete, so it could not be removed during
chart preparation.

This change:
- Adds chartifyTempDirTracker to HelmState (pointer-based to avoid copy-lock
  issues from HelmState being copied in several places)
- Tracks chartify output dirs via addChartifyTempDir() after c.Chartify()
  succeeds in processChartification
- Cleans them up via CleanupChartifyTempDirs() in WithPreparedCharts after
  all helm operations complete
- Also removes empty parent temp directories (e.g. /tmp/chartify<random>/)

Fixes #1799

Signed-off-by: yxxhero <aiopsclub@163.com>
2026-06-28 11:55:31 +08:00
yxxhero 76613bd0bb fix: serialize concurrent helm operations for same chart to fix #768 (#2662)
When multiple releases in a helmfile use the same remote chart, concurrent
helm upgrade/diff calls race on helm's internal repository cache file rename,
causing intermittent 'cannot rename: Access is denied' errors on Windows.

This fix introduces two layers of serialization:

1. withChartOperationLock (operation-level): wraps SyncRelease/DiffRelease
   calls with a per-chart+version mutex. Only applies to remote charts
   (release.ChartPath is empty); local/pre-fetched/OCI charts bypass the
   lock entirely. Different charts remain fully parallel.

2. Per-chart+version download mutex in forcedDownloadChart/getOCIChart
   (download-level): uses double-check locking to ensure only one
   helm fetch runs per unique chart+version within a process.

The fix does NOT change chart paths passed to helm, preserving backward
compatibility with all existing behavior and tests.

Trade-off: same-chart releases are fully serialized (the entire helm
operation including deployment, not just download). This is unavoidable
without pre-fetching because helm downloads and deploys atomically.
Releases with different charts are unaffected.

Fixes #768

Signed-off-by: yxxhero <aiopsclub@163.com>
2026-06-25 14:37:04 +08:00
yxxhero 08d70bc9a7 fix: ensure OCI charts are prepared for needed releases with --include-needs (#2663)
fix: ensure OCI charts are prepared for needed releases with --include-needs (#923)

When using --include-needs with a selector, releases included via needs
must have their charts prepared (pulled/exported) before diff/sync/apply
can process them. The core fix (ChartPrepareOptions.IncludeTransitiveNeeds
= c.IncludeNeeds()) was already in place, but ForEachState calls in
Diff/Template/Lint/Unittest/Sync/Apply still passed c.IncludeTransitiveNeeds()
instead of c.IncludeNeeds(), creating an inconsistency that would resurface
if SetFilter(true) were ever added.

Changes:
- Use c.IncludeNeeds() in ForEachState for all commands supporting
  --include-needs (Diff, Template, Lint, Unittest, Sync, Apply, Doctor)
- Doctor is the only command with SetFilter(true), so this fixes a real
  bug: helmfile doctor --include-needs was silently ignored for filtering
- Add explanatory doc comment on ForEachState parameter semantics
- Enhance exectest.Helm.ChartPull to create minimal chart files and track
  pulls, enabling OCI chart testing
- Add resetChartCacheForTest() for test isolation from global chart cache
- Add regression tests for issue #923

Signed-off-by: yxxhero <11087727+yxxhero@users.noreply.github.com>
Signed-off-by: yxxhero <aiopsclub@163.com>
2026-06-25 14:36:50 +08:00
yxxhero 9b943adc9e feat: add helmfile doctor command for AI-assisted diff analysis (#2660)
* feat: add `helmfile doctor` command for AI-assisted diff analysis

`helmfile doctor` runs `helmfile diff` and asks an OpenAI-compatible LLM to
summarize the changes and flag risks (data loss, security exposure, breaking
changes, downtime, performance, best-practice issues).

Key design decisions:
- When no LLM is configured, doctor is equivalent to `helmfile diff` with
  one exception: --show-secrets is always forced off (secrets never reach
  stdout, even without an LLM).
- Secrets are ALWAYS redacted via two layers: (1) ShowSecrets() forced to
  false so helm-diff emits <REDACTED> placeholders; (2) a defense-in-depth
  text redactor strips residual secret-looking content (Secret YAML blocks,
  sensitive key/value lines, base64 blobs, JWT tokens) before LLM transmission.
- LLM configuration precedence: env (HELMFILE_LLM_*) < helmfile.yaml (llm:)
  < CLI flags (--llm-*).
- Supports any OpenAI-compatible backend (OpenAI, Azure, One-API, LiteLLM,
  Ollama, etc.) with automatic response_format fallback for backends that
  don't support JSON mode.
- Prompt injection defense: release names and environment values are
  JSON-encoded before insertion into the LLM prompt.
- Exit codes: 0 (success/low-risk), 2 (high-risk gate, bypass with --force),
  1 (other errors). Helm-diff's 'detected changes' exit-2 is swallowed.

New packages:
- pkg/agent/llm: OpenAI-compatible client with JSON response parsing, mock
  client for testing, prompt builder with injection defense.
- pkg/agent/doctor: secret redactor (state machine + regex), report renderer
  (markdown + JSON), config resolver (env < yaml < flag merge).

Testing: 70+ unit tests covering redaction patterns, prompt injection,
response_format fallback, JSON parsing, yaml roundtrip, concurrency safety,
panic recovery, and error propagation. go test -race passes.

Documentation: full doctor section in docs/cli.md, llm: block reference in
docs/configuration.md, updated skills/helmfile for AI agents.

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

* docs: fix doctor equivalence wording per PR review

Per review feedback (PR #2660): the docs claimed doctor is 'equivalent to
helmfile diff — same flags, same output, same exit codes' in the unconfigured
path, but this over-promises because:

  1. doctor --output is the report format (not helm-diff's output format)
  2. helm-diff's --output is exposed as --diff-output in doctor
  3. --show-secrets is silently ignored

Updated all three locations (cli.md, cmd/doctor.go Long + godoc, pkg/app/doctor.go
godoc) to say 'falls back to helmfile diff with --show-secrets forced off' and
explicitly note the --output / --diff-output flag difference.

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

---------

Signed-off-by: yxxhero <aiopsclub@163.com>
2026-06-22 16:52:35 +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 a94fdf4194 feat: add support for helm 4 --server-side upgrade flag (#2641)
* feat: add support for helm 4 --server-side upgrade flag

Add support for the helm 4 upgrade flag --server-side which accepts
"true", "false", or "auto" (default "auto"). This allows users to
explicitly control server-side apply behavior, which is needed for
releases originally installed with Helm 3 and being managed with Helm 4.

The flag can be configured via:
- CLI: --server-side flag on sync, apply, and diff commands
- helmDefaults.serverSide in helmfile.yaml
- releases[].serverSide per-release override

Precedence: release-level > CLI flag > helmDefaults.
Errors are returned when serverSide is set but running Helm 3, or when
an invalid value is provided.

Closes #2640

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

* test: update TestGenerateID expected hashes for new ServerSide field

Adding ServerSide *string to ReleaseSpec changes spew's %#v output and
shifts the FNV hash used by generateValuesID. Update the hard-coded want
values to the new deterministic hashes.

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

---------

Signed-off-by: yxxhero <aiopsclub@163.com>
2026-06-16 08:24:47 +08:00
yxxheroandcopilot-swe-agent[bot] 38e27ee439 fix: include release values in .Values for patch template rendering (#2556)
* fix: include release values in .Values for jsonPatches/strategicMergePatches/transformers gotmpl rendering

Before this fix, .Values in patch template files only contained environment
values, not the release's own values. This meant references like
{{ .Values.ingress.enabled }} would fail when ingress.enabled was set in
the release's values: file rather than environment values.

Now patch gotmpl files see .Values as merged(environment values, release values),
matching user expectations that values defined in the release should be
accessible in conditional patches.

Fixes #1904

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

* test: add more tests for resolveReleaseValues, renderValuesFileToBytesWithData, and generateTemporaryReleaseValuesFilesWithData

Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/5da5c9d8-7464-4146-84b5-1433ed6193f3

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

* test: simplify newTestHelmStateWithFiles by removing empty cleanup func

Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/5da5c9d8-7464-4146-84b5-1433ed6193f3

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

* fix: remove always-constant basePath param from newTestHelmStateWithFiles to fix unparam lint error

Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/b4a669cb-692c-4ca6-a68b-1b04a062b989

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

* fix: address c66017c review comments - error messages, defer-in-loop, map normalization, test cleanup

Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/44988dd8-1c67-465b-995c-80525a24eb93

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

* refactor: extract generateTemporaryReleaseValuesFilesCore to eliminate duplication; fix temp dir leaks in tests

Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/b254ddda-aa95-4e2f-8dd9-1ce4c40eedb6

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

* fix: remove rendered content from debug log; extract prepareReleaseValuesEntries shared helper

Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/321c6ba5-f835-4afd-be5e-ee790bc6b4a5

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

* fix: compute mergedReleaseTemplateData lazily in PrepareChartify

Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/100b7974-3268-4dc8-be21-12bd82aa2dbb

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

* fix: normalize nested YAML keys in resolveReleaseValues via CastKeysToStrings

Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/d0129d85-9c7d-4a31-966e-fc0b05b74867

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

* fix: use %w for error wrapping in release values resolution

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-06-15 16:31:46 +08:00
yxxhero 7391453cbe fix: support array of maps in set/setTemplate values (#2615)
Changed SetValue.Values type from []string to []any to allow passing
maps (not just strings) in the values field of set/setTemplate.

Previously, YAML like:
  setTemplate:
    - name: source.helm.parameters
      values:
        - name: demo
        - version: v2

would fail with 'cannot unmarshal !!map into string'. Map values are
now serialized to JSON when generating --set flags.

Fixes #1021

Signed-off-by: yxxhero <aiopsclub@163.com>
2026-05-31 09:57:35 +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
23802e7a95 fix: refresh Chart.lock after rewriting file:// dependencies (#2587)
* fix: refresh Chart.lock after rewriting file:// dependencies

`rewriteChartDependencies` rewrites relative `file://` repository URLs in
Chart.yaml to absolute paths so chartify can resolve them from a temp
directory. That mutates the Chart.yaml dependencies block, which
invalidates the Chart.lock digest (helm computes it as
`sha256(json.Marshal([2][]Dependency{req, lock}))` over the dependencies).

Once the lock is out of sync, downstream `helm dependency build` errors
with "the lock file (Chart.lock) is out of sync with the dependencies
file (Chart.yaml)" and chartify falls back to `helm dependency update`.
`dep update` then re-resolves Chart.yaml's version constraints against
the chart repo, so any constraint that admits newer versions
(e.g. `version: "*"`, `~1.0`) silently picks up a newer dependency on
every render — even though Chart.lock pins a specific version.

Repro:
  - Chart.yaml has `version: "*"` for some-dep, Chart.lock pins 4.1.0,
    upstream now publishes 4.2.0.
  - `helm template .` honors the cached `charts/some-dep-4.1.0.tgz`.
  - `helmfile template` produces 4.2.0, because it triggered chartify
    (via jsonPatches/strategic-merge/kustomize/etc), which copied the
    chart, ran `dep build` against an out-of-sync lock, fell back to
    `dep up`, and re-resolved the wildcard.

This commit refreshes Chart.lock alongside Chart.yaml in the temp copy:

- Mirror the rewritten file:// repository URLs onto matching entries in
  Chart.lock's dependencies. Without this, `helm dep build` would resolve
  the lock's relative `file://` paths against the temp chart directory
  and fail with "directory ... not found".
- Recompute the digest using helm's resolver.HashReq algorithm
  (`sha256(json.Marshal([2][]chart.Dependency{req, lock}))`). The
  algorithm is small and stable; resolver.HashReq itself lives in an
  internal package, so it's inlined here.
- Locked versions are preserved verbatim — only the repository URL is
  updated and the digest recomputed. Chart.lock remains the source of
  truth for which versions get installed.
- The original Chart.lock on disk is never modified; only the temp copy
  is rewritten.

Adds TestRewriteChartDependencies_RefreshesChartLock covering digest
recomputation, file:// URL mirroring, version preservation, untouched
non-file:// deps, and original-on-disk integrity.


Signed-off-by: Shane Starcher <shane.starcher@gmail.com>

* fix: address Copilot review issues for Chart.lock refresh

- Map all helm Dependency fields (alias, condition, tags, import-values,
  enabled) when building the request slice for digest computation, not
  just name/version/repository. This ensures the recomputed digest
  matches Helm's resolver.HashReq for all dependency shapes.
- Match lock entries by Name + Alias (not Name alone) to correctly
  handle charts with duplicate dependency names distinguished by alias.
- Log a warning when reading Chart.lock fails with a non-NotExist error,
  while still treating a missing Chart.lock as expected.
- Add test case exercising dependencies with alias, condition, tags, and
  import-values fields, including same-name deps disambiguated by alias.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Signed-off-by: Shane Starcher <shane.starcher@gmail.com>

* build(deps): bump github.com/helmfile/chartify to v0.26.4

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Signed-off-by: Shane Starcher <shane.starcher@gmail.com>

* fix: normalize import-values for JSON marshaling and improve test coverage

- Normalize import-values using maputil.RecursivelyStringifyMapKey before
  assigning to helmchart.Dependency.ImportValues. When go-yaml v2 decodes
  nested maps (e.g. import-values entries with child/parent keys), they
  become map[interface{}]interface{} which json.Marshal cannot encode.
  This would silently prevent Chart.lock rewriting. The normalization
  converts all map keys to strings, making the value JSON-safe.
- Improve TestRewriteChartDependencies_RefreshesChartLockWithExtraFields
  to prove that extra fields (condition, tags, import-values) actually
  affect the computed digest by comparing digests with and without those
  fields and asserting they differ.

Signed-off-by: Shane Starcher <shane.starcher@gmail.com>

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix: normalize lock ImportValues and fix digest test isolation

- Normalize lock.Dependencies ImportValues via RecursivelyStringifyMapKey
  before json.Marshal, preventing failures when go-yaml v2 decodes nested
  maps as map[interface{}]interface{}.
- Fix TestRewriteChartDependencies_RefreshesChartLockWithExtraFields to use
  a shared root directory so both chart variants resolve file:// paths to
  the same absolute location, isolating digest differences to field content.
- Add TestRewriteChartDependencies_GoYamlV2ImportValues exercising the
  HELMFILE_GO_YAML_V3=false path with import-values containing nested maps.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Shane Starcher <shane.starcher@gmail.com>

* fix: add exact digest verification test against Helm's HashReq

Add TestRewriteChartDependencies_DigestMatchesHelmHashReq which computes
the expected digest independently using the same algorithm as Helm's
resolver.HashReq and asserts the rewritten Chart.lock matches exactly.
This guards against producing a digest that is "different" yet still
rejected by `helm dependency build`.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Shane Starcher <shane.starcher@gmail.com>

---------

Signed-off-by: Shane Starcher <shane.starcher@gmail.com>
Co-authored-by: Shane Starcher <shane.starcher@gmail.com>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
2026-05-19 05:41:41 +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
Dominik Schmidt 0139304d97 feat(state): add mergeStrategy: fallback for first-file-wins env values (#2578)
* feat(state): add mergeStrategy field to EnvironmentSpec

Introduces a per-environment mergeStrategy with valid values "override"
(default, current behavior) and "fallback". This commit only adds the
field, the constants, and a parse-time validator; the loader still
ignores the value, so behavior is unchanged.

Subsequent commits thread the value through the values loader and
implement the fallback semantics.

Signed-off-by: Dominik Schmidt <dev@dominik-schmidt.de>

* refactor(state): thread mergeStrategy through values loader

Adds a mergeStrategy string parameter to LoadEnvironmentValues,
loadValuesEntries, and mapMerge so the value can flow from
EnvironmentSpec down to the merge call site. Behavior is unchanged in
this commit; mapMerge ignores the strategy and the next commit
implements the fallback semantics.

Top-level state.DefaultValues and the --state-values-file/-set loaders
are passed an empty strategy ("") since they have no per-environment
spec to consult and stay on the default override behavior.

Signed-off-by: Dominik Schmidt <dev@dominik-schmidt.de>

* feat(state): implement fallback merge strategy

Adds a hand-rolled fallbackDeepMerge that, unlike mergo, preserves
keys present in the destination even when their value is the zero
value (false, 0, "", nil, empty list/map). mapMerge dispatches to it
when mergeStrategy == "fallback"; "override" and the empty default
keep using mergo with WithOverride so existing behaviour is unchanged.

Validation lives at the entry of LoadEnvironmentValues so a single
chokepoint guards the field. Invalid values produce an error naming
both the offending value and the valid options.

Tests cover: first-file-wins precedence, gap filling, deep nested
merge, three-file chains, explicit zero-value preservation (the case
naïve mergo gets wrong), explicit nil preservation, inline map
entries, override regression, default-equals-override equivalence,
and invalid-strategy errors.

Signed-off-by: Dominik Schmidt <dev@dominik-schmidt.de>

* feat(state): expose prior-file values in fallback template context

Under mergeStrategy: fallback, .gotmpl values files can now reference
values from earlier files in the same `values:` list via .Values
(e.g. `service.domain: "service.{{ .Values.cluster.domain }}"`).

The accumulated result is layered under env.GetMergedValues so env
defaults, env values, and CLI overrides still win on overlap. Override
mode keeps the historical template context — unchanged — so this is
strictly opt-in via the mergeStrategy field.

Together with the precedence flip from the previous commit, this lets
users replace the brittle two-stage `merged-values.yaml.gotmpl`
workaround with native helmfile syntax.

Tests cover the headline cross-file template reference case and pin
the override-mode contract that prior-file values stay invisible.

Signed-off-by: Dominik Schmidt <dev@dominik-schmidt.de>

* docs: document mergeStrategy and fallback semantics

Adds a new section to values-and-merging.md describing the override vs
fallback strategies, the explicit-zero-value preservation guarantee,
and the cross-file template reference behavior. Adds a brief pointer
to environments.md so users land on the new field from the
environment values discussion.

Signed-off-by: Dominik Schmidt <dev@dominik-schmidt.de>

* refactor(state): reuse maputil.MergeMaps for fallback merge

Replaces the hand-rolled fallbackDeepMerge with a single call to
maputil.MergeMaps, swapping its arguments so the accumulated dest wins
over the new src file. Same first-file-wins semantic, fewer lines, and
the fallback path now inherits the same slice merge strategies the
rest of helmfile already uses.

The one observable behavior shift is for explicit nil values: under
fallback, nil in an earlier file no longer 'wins' over a non-nil value
in a later file — instead it falls through (matching MergeMaps' rule
that nil from the override side only fills missing keys). This is
internally consistent: nil-overwrites is an mergo.WithOverride quirk
that lives only in the override path. The renamed test
NilFallsThroughToFallback pins the new behavior with a comment
referencing the contrast with override mode (Issue1154).

Signed-off-by: Dominik Schmidt <dev@dominik-schmidt.de>

---------

Signed-off-by: Dominik Schmidt <dev@dominik-schmidt.de>
2026-05-07 21:50:05 +08:00
Dominik Schmidt c6d0310029 fix(state): resolve OCI repo prefix in ad-hoc release dependencies (#2579)
When a release `dependencies[].chart` is given as `<repoName>/<chart>`
and the matching `repositories:` entry has `oci: true`, helmfile now
rewrites it to `oci://<repoURL>/<chart>` before passing it to chartify.
Without this, chartify's lookup falls into its `helm repo list` branch,
which never finds OCI repos because helm 3+ does not register OCI
registries as named repos (they live in the `helm registry login`
state instead). The user-visible failure was:

  failed reading adhoc dependencies: no helm list entry found for
  repository "<name>". please `helm repo add` it!

Explicit `oci://` URLs already worked through chartify's OCI branch;
this change makes the `<repoName>/<chart>` form behave the same way.
Non-OCI repo prefixes, unknown prefixes, single-segment names, and
explicit `oci://` URLs all pass through unchanged. A debug log records
each rewrite at the call site for easier troubleshooting.

Fixes #1756.

Signed-off-by: Dominik Schmidt <dev@dominik-schmidt.de>
2026-05-07 17:07:20 +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
Copilotandyxxhero 10deabb142 Honor skipSchemaValidation during chartification when forceNamespace is set (#2550)
* fix(state): honor skipSchemaValidation in chartify template args

Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/a695cbff-c37a-403a-9658-09f4fdaa65d0

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

* test(state): harden chartify skip-schema flag detection

Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/a695cbff-c37a-403a-9658-09f4fdaa65d0

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

* fix(state): propagate cli skip-schema-validation to chartify

Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/70ebf027-0ab5-4bdb-a4b4-5a77c822ee95

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-04-30 12:30:41 -04: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
Diliz 7a60255a9b enabledns flags available on template command (#2511)
* enabledns flags available on template command

Enable dns flag was not available in helmfile template command

Signed-off-by: yxxhero <aiopsclub@163.com>
2026-04-16 15:59:48 +08:00
Vojta Polak 6faa477290 fix: update state values files handling to replace arrays instead of merging (#2537)
* feat: update state values handling to replace arrays instead of merging

Signed-off-by: Vojta Polak <vojta.polak@gmail.com>

* test: non-shallow copy of OverrideCLISetValues in DeepCopy + test

Signed-off-by: Vojta Polak <vojta.polak@gmail.com>

* fix: update error message for empty CalleePath in Load function

Signed-off-by: Vojta Polak <vojta.polak@gmail.com>

---------

Signed-off-by: Vojta Polak <vojta.polak@gmail.com>
2026-04-13 19:46:15 +08:00
yxxheroandcopilot-swe-agent[bot] acb7ce36fc fix: add mutex lock for concurrent rewriteChartDependencies access (#2509)
* fix: add mutex lock for concurrent rewriteChartDependencies access

Prevent race conditions when multiple goroutines concurrently access
the same Chart.yaml by introducing a per-chart-path mutex via sync.Map.

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

* fix: unlock mutex on all error paths and improve race condition test validation

Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/135e06b8-99e8-42cb-859d-524b2d2b1907

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

* fix: remove duplicate mutex, reuse getNamedRWMutex, restore on write failure, add test barrier

Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/f93746bf-82fa-46b1-b8f0-b34bc9aa749c

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

* fix: early unlock when unmodified, assert exact restore in race test

Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/5981aea4-2799-4560-9272-315d28d4b7d7

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

* fix: improve comment wording and strengthen race test start barrier

Agent-Logs-Url: https://github.com/helmfile/helmfile/sessions/9e4d6f4f-cdc1-4e9e-bdc6-81061ebc1dcc

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-04-13 11:59:52 +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
Jinyu c70b20ad7a feat: add an arg that passing description to helm upgrade command (#2497)
* feat: add an arg that passing description to `helm upgrade` command

fix: github actions

Signed-off-by: swimablefish <swimablefish@gmail.com>

* fix: lint and test failed

Signed-off-by: swimablefish <swimablefish@gmail.com>

* feat: encapsulation

Signed-off-by: swimablefish <swimablefish@gmail.com>

* feat: add version gate

Signed-off-by: swimablefish <swimablefish@gmail.com>

* feat: rephrase

Signed-off-by: swimablefish <swimablefish@gmail.com>

---------

Signed-off-by: swimablefish <swimablefish@gmail.com>
2026-03-24 21:01:44 +08:00
yxxhero 43b426892e fix: cleanup hooks not receiving error signal (#2475)
* fix: cleanup hooks not receiving error signal

Closes #1041

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

* Add tests for cleanup hooks error propagation

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

* fix tests

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

* fix tests

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

* fix tests

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

* fix tests

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

---------

Signed-off-by: yxxhero <11087727+yxxhero@users.noreply.github.com>
Signed-off-by: yxxhero <aiopsclub@163.com>
2026-03-21 14:29:32 +08:00
yxxhero d613c5484c feat: add --force-conflicts flag support for Helm 4 (#2480)
* feat: add --force-conflicts flag support for Helm 4

Add support for Helm 4's --force-conflicts flag which forces server-side
apply changes against conflicts. This flag is mutually exclusive with
--force/--force-replace and only available in Helm 4.

Fixes #2429

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

* fix: address review comments on force-conflicts feature

- Fix comment grammar: 'forces' instead of 'force'
- Improve error messages to indicate both sources (releases[] and helmDefaults)
- Add test case for helmDefaults.forceConflicts with Helm 3 (should error)
- Update TestGenerateID expected hashes after adding ForceConflicts field to structs

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

---------

Signed-off-by: yxxhero <aiopsclub@163.com>
2026-03-14 08:44:44 +08:00
yxxhero 607225c34d fix: use --force-replace flag for Helm 4 instead of deprecated --force (#2477)
* fix: use --force-replace flag for Helm 4 instead of deprecated --force

Helm 4 deprecated the --force flag in favor of --force-replace.
This fix detects the Helm version and uses the appropriate flag:
- Helm 4: --force-replace
- Helm 3: --force

Also fixed a nil pointer panic in appendHideNotesFlags when called
with nil SyncOpts.

Fixes #2476

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

* fix(ci): pin semver to v2.12.0 for Go 1.25 compatibility

semver@latest requires Go 1.26.1 but the project uses Go 1.25.4.
Pinning to v2.12.0 which is compatible with Go 1.25.

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

* test: add test cases for force flag from defaults with nil release

Add test cases to cover the scenario where release.Force is nil and
HelmDefaults.Force enables force for both Helm 3 and Helm 4.

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

* test: add nil ops test and rename misleading test names

- Add test case for appendHideNotesFlags with ops=nil to prevent
  regression
- Rename force-from-default-nil-release-* to
  force-from-default-nil-force-* for clarity (release.Force is nil,
  not the release itself)

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

* refactor: add explicit parentheses for force condition

Add explicit parentheses around the two disjuncts in the force
condition to make the intended grouping unambiguous and easier
to read.

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

* refactor: check ops nil before Helm version in appendHideNotesFlags

- Swap the order to check ops == nil first to avoid unnecessary
  IsVersionAtLeast call
- Restore the "see Helm release" comment for consistency with other
  flag helpers

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

---------

Signed-off-by: yxxhero <aiopsclub@163.com>
2026-03-11 21:23:51 +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
yxxheroandCopilot c6e7249eb9 feat: add helm-legacy track mode for Helm v4 compatibility (#2466)
Add support for trackMode: helm-legacy to use Helm v4's --wait=legacy flag,
which maintains compatibility with Helm v3's wait behavior during migration.

Helm v4 changed the default --wait behavior from polling to a watcher-based
approach. This can cause issues with charts that have broken livenessProbe
configurations without startupProbe. The --wait=legacy flag preserves the
Helm v3 polling behavior for smoother migration.

Changes:
- Add TrackModeHelmLegacy constant in pkg/kubedog/options.go
- Use kubedog.TrackMode constants instead of raw strings in helmx.go
- Enhance appendWaitFlags to use --wait=legacy for Helm v4 when trackMode
  is helm-legacy
- Add nil check for logger before logging warning
- Add version check with warning when helm-legacy is used with Helm v3
- Update validation in pkg/config to accept helm-legacy track mode
- Update command-line flags in cmd/apply.go and cmd/sync.go
- Add comprehensive documentation in docs/advanced-features.md
- Add thorough test coverage including warning message verification

Behavior:
- Helm v4 + helm-legacy: Uses --wait=legacy
- Helm v3 + helm-legacy: Falls back to --wait with warning
- Helm v4 + helm: Uses --wait (watcher mode)
- Any + kubedog: Skips --wait flag

Fixes #2464

Signed-off-by: yxxhero <aiopsclub@163.com>
Co-authored-by: Copilot <copilot@github.com>
2026-03-08 11:51:14 +08:00
yxxhero 615e8132ee fix: pass --kubeconfig to chartify's helm template call (#2449)
When using jsonPatches or kustomize patches with helmfile, chartify runs
"helm template" internally to render the chart before applying patches.
The lookup() helm function requires cluster access (--dry-run=server).

Previously, --kubeconfig was passed to helm diff and helm upgrade commands,
but not to chartify's internal helm template call. This caused failures
when users specified --kubeconfig flag with a non-default kubeconfig location.

This fix ensures --kubeconfig is passed to chartify's TemplateArgs for
cluster-requiring commands (sync, apply, diff, etc.), alongside the existing
--kube-context and --dry-run=server flags.

Fixes #2444

Signed-off-by: yxxhero <aiopsclub@163.com>
2026-03-03 20:56:46 +08:00
yxxhero ce09f560d9 fix: configure kubedog rate limiter to prevent context cancellation (#2446) 2026-03-03 19:24:28 +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
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
yxxhero 2b0086b196 fix: only pass --skip-refresh to helm dep build when helm repo update was run (#2419)
When no repositories are defined in helmfile.yaml, local charts with
external dependencies need to refresh the repo cache. Previously, we
always passed --skip-refresh to helm dep build, which broke this case.

Now --skip-refresh is only passed when we actually ran helm repo update,
meaning repos are configured AND no skip refresh flags are set. This
preserves the precomputed skipRefresh value from prepareChartForRelease
which accounts for CLI flags, helmDefaults.skipRefresh, and release.skipRefresh.

Fixes #2417

Signed-off-by: yxxhero <aiopsclub@163.com>
2026-02-25 13:03:30 +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
yxxhero 503c397810 feat: support .Environment.* in --output-dir-template (#2375)
* feat: support .Environment.* in --output-dir-template

This commit adds support for accessing environment values in the --output-dir-template flag.

Previously, users could only access .OutputDir, .State.*, and .Release.* in the template.
Now .Environment.* is also available, allowing users to use environment values in the
output directory path.

Example usage:
  helmfile template -e test-1 --output-dir-template='{{ .OutputDir }}/{{ .Environment.cluster.name }}/{{ .Environment.Name }}/{{ .Release.Name }}'

This produces output like: ./gitops/my-test-cluster/test-1/release-name/

Changes:
- Add Environment field to GenerateOutputDir template data
- Add Environment field to generateChartPath template data (now a method on HelmState)
- Update help text for --output-dir-template flag in template and fetch commands
- Add test cases for Environment in template

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

* fix: address PR review comments for --output-dir-template

- Clarify .Environment.Name, .Environment.KubeContext, .Environment.Values.* in help text
- Update generateChartPath comment to reflect broader usage (fetch, pull, OCI)
- Add tests for GenerateOutputDir with Environment fields

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

* fix: address additional PR review comments

- Move HelmState setup outside test loop to reduce duplication
- Document Environment field (.Name, .KubeContext, .Values) in template data structs

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

---------

Signed-off-by: yxxhero <aiopsclub@163.com>
2026-02-14 11:54:43 +08:00
yxxhero 58df057dcc fix: skip cache refresh for shared cache paths to prevent race conditions (#2396)
* fix: skip cache refresh for shared cache paths to prevent race conditions

When multiple helmfile processes run in parallel (e.g., as ArgoCD plugin),
they share the same OCI chart cache in ~/.cache/helmfile. One process could
delete and re-download (refresh) a cached chart while another process was
still using it, causing "path not found" errors.

This fix:
- Adds isSharedCachePath() helper to detect shared cache paths
- Skips chart deletion/refresh for paths in the shared cache directory
- Users can force refresh by running `helmfile cache cleanup` first

Fixes #2387

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

* docs: document OCI chart caching behavior and multi-process safety

Add documentation for:
- OCI chart cache location and behavior
- How to force cache refresh with `helmfile cache cleanup`
- Multi-process safety when using shared cache
- Cache management commands (info, cleanup)

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

* fix: address review comments for shared cache handling

- Return error instead of chartActionDownload for corrupted shared cache
- Change refresh skip log from Debugf to Infof for user visibility
- Add t.Helper() to createTestLogger test helper

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

* fix: handle symlinks and add debug logging in isSharedCachePath

- Use filepath.EvalSymlinks to resolve symlinks before path comparison
- Add debug logging when filepath.Abs fails
- Add test case for symlink to shared cache directory

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

* fix: address copilot review comments

- Include underlying error in corrupted cache error message
- Add cleanup for test directories created in shared cache
- Clarify --skip-refresh flag documentation

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

* fix: handle edge case when chartPath equals sharedCacheDir

- isSharedCachePath now returns true for exact match with cache dir
- Add test case for exact match with shared cache directory

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

* test: add integration test for acquireChartLock shared cache behavior

Add TestAcquireChartLockSharedCacheSkipRefresh to verify that
acquireChartLock returns chartActionUseCached instead of
chartActionRefresh when the chart exists in the shared cache,
even when refresh is requested. This tests the core fix for
the race condition issue #2387.

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

---------

Signed-off-by: yxxhero <aiopsclub@163.com>
2026-02-14 10:46:05 +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
Manetheren 7500eef7c6 feat: Add option for SkipCRDs to HelmDefaults (#2356)
* feat: Add option for SkipCRDs to HelmDefaults

Signed-off-by: Manetheren <git@manetheren.io>

* fix: nil check not needed on bool

Signed-off-by: Manetheren <git@manetheren.io>

* Fix typo

Signed-off-by: Manetheren <git@manetheren.io>

---------

Signed-off-by: Manetheren <git@manetheren.io>
2026-01-22 16:32:04 +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
Shane StarcherandShane Starcher 61f4a316a6 fix: rewrite relative file:// chart dependencies to absolute paths (#2334)
Fixes an issue where Chart.yaml dependencies with relative file:// paths
fail during chartification because the paths become invalid when the chart
is copied to chartify's temporary directory.

The rewriteChartDependencies function now converts relative file://
dependencies to absolute paths before chartification, then restores the
original Chart.yaml afterwards. Absolute file:// and other repository
types (https, oci) are left unchanged.

Includes comprehensive test coverage for various dependency scenarios.

Signed-off-by: Shane Starcher <shane.starcher@gmail.com>
Co-authored-by: Shane Starcher <shane.starcher@gmail.com>
2025-12-20 09:08:39 +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