public_git/helmfile
1
0
Fork 0
mirror of https://github.com/helmfile/helmfile.git synced 2026-10-19 13:42:52 +02:00
Code Issues Projects Releases Wiki Activity

feat: opt-in OpenTelemetry tracing and metrics (experimental) (#2769)

Browse Source 
* feat(telemetry): add opt-in OpenTelemetry tracing (PR 1: lifecycle + root span)

Implements the first increment of docs/proposals/otel-tracing.md (#2767):

- pkg/telemetry: SDK setup from standard OTEL_* env vars (autoexport for
  exporter selection, env-driven sampler/propagators, OTEL_SDK_DISABLED),
  command-span lifecycle, no-op-by-default accessors
- --otel-tracing flag / HELMFILE_OTEL_TRACING env switch
- root span "helmfile <command>" with file/environment/selectors/exit_code
  attributes; TRACEPARENT-based remote-parent extraction for CI correlation
- shutdown flush on both normal-exit and signal paths (nil-safe, 5s bound)
- app.New derives its context from telemetry.CommandContext()
  (Background-identical when tracing is disabled)
- docs: otel.md user guide, experimental-features entry, design proposal
- tests: hermetic unit tests, app context-contract pinning, flag registration

Telemetry problems never fail a run: exporter misconfiguration and export
errors degrade to disabled with a warning. When disabled, behavior and
performance are identical to before (no-op tracer, no goroutines, no
network).

Refs: #2767, #2758
Signed-off-by: yxxhero <aiopsclub@163.com>

* feat(telemetry): trace every external process + trace-context bridges (PR 2)

Implements the second increment of docs/proposals/otel-tracing.md (#2767):

- pkg/helmexec/span.go: one span per external process started by helmfile
  (helm invocations, hooks, plugin execs) at the ShellRunner choke point —
  helm.exec (with helm.subcommand) vs os.exec, with redacted exec.args,
  exec.exit_code, and error status on failure
- pkg/helmexec/redact.go: shared argument redaction with two profiles;
  legacy is byte-identical to the historical exit-error behavior (existing
  goldens unchanged), strict (spans) additionally covers --set=k=v and
  credential flags; exit_error.go now uses the shared helper
- orphan-trace bridges with bit-identical cancellation semantics
  (context.WithoutCancel of the command context): both kubedog call sites
  (state.go) and hook execution (event.Bus gains an optional Ctx consumed
  by its default runner; state.go sets it, nil falls back to TODO as before)
- OTLP end-to-end test (in-process httptest receiver, no external
  collector): span export, error status/exit code, redaction, and
  parent-linkage to the command span
- docs/otel.md updated to the now-traced surface

Verified end-to-end with the console exporter: helmfile template on a
local chart yields the command span plus helm.exec spans for helm
version/dependency/template, all nested under it.

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

* feat(telemetry): state-loading and hook spans (PR 3a)

Implements the third increment of docs/proposals/otel-tracing.md (#2767):

- helmfile.discover_states around findDesiredStateFiles and helmfile.load
  around loadDesiredStateFromYamlWithBaseDir; both cover all callers
  (incl. nested helmfiles) with no signature changes
- helmfile.render / helmfile.parse children per document part, parented
  through a traceCtx field on the unexported desiredStateLoader struct
  (set once at its single construction site)
- helmfile.hook span per hook execution: Trigger's per-hook body extracted
  into runHook (readability win on its own), the hook's subprocess span
  nests under it via a per-hook ctx-swapped ShellRunner clone
  (cancellation unchanged — Bus.Ctx never carries cancellation by contract)
- pkg/telemetry/otlptest: shared in-process OTLP/HTTP receiver harness,
  now used by helmexec, event, and app span tests
- golden span-tree test at the app layer (root -> discover -> load ->
  render/parse, via the exectest fake helm) and a hook-span nesting test
- nil-ctx guard for App literals built directly by tests (App.spanParentCtx)

Verified end-to-end with the console exporter: a template run over a
gotmpl state file with a prepare hook yields the full tree with the hook's
os.exec nested under helmfile.hook.

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

* feat(telemetry): per-release spans nested under the load span (PR 3b)

Implements the per-release increment of docs/proposals/otel-tracing.md
(#2767) — spans nest command -> load -> release -> helm exec:

- helmexec.HelmContext gains an optional Ctx carrying the per-release span
  context; the execer's new execWithContext funnel consumes it via a
  per-call runner clone (runnerWithCtx) so the shared, cached execer is
  never mutated across concurrent workers. The seven Interface methods
  that take a HelmContext (Sync/Diff/ReleaseStatus/List/DecryptSecret/
  Delete/Test) route through it; nil Ctx behaves exactly as before.
  exec() lost its always-nil override parameter on the way (unparam).
- pkg/state/span.go: SetTraceContext + startReleaseSpan/endReleaseSpan
  helpers (release/namespace/chart/labels attributes, sorted for stable
  output); a typed-nil guard (releaseErrAsError) avoids the classic
  nil-pointer-in-interface trap on *ReleaseError.
- release spans in the worker loops: SyncReleases, DiffReleases,
  DeleteReleasesForSync, PrepareCharts, and iterateOnReleases (status/
  delete/test via a new verb parameter); their HelmContext is stamped with
  the release span context where one is built.
- pkg/app sets st.SetTraceContext(loadCtx) right after loading a state
  file, rooting all per-release spans under helmfile.load.
- bridged one more detached tracking call found on the way
  (trackReleaseIfEnabled's context.Background in the sync worker).
- golden test: release span present, nested under load, correct
  attributes; unit test for the runnerWithCtx clone semantics.

Verified with the console exporter: helmfile template yields
release.prepare(demo) under load with full attributes.

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

* feat(telemetry): nest status/delete/test execs under their release spans

Completes the per-release exec nesting for the iterateOnReleases-based
loops (docs/proposals/otel-tracing.md §4.4 phase 2): the do closures now
receive the release span context and stamp it into their HelmContext, so
helm status/delete/test subprocess spans nest under
helmfile.release.<verb> like sync/diff already did.

- scatterGatherReleases/iterateOnReleases/doWithReleaseSpan: do gains a
  context parameter (the release span context)
- ReleaseStatuses/DeleteReleases/TestReleases closures stamp
  HelmContext.Ctx from it
- integration test with a real execer (version-probe shim binary): the
  release's status subprocess nests under helmfile.release.status, same
  trace, with helm.subcommand=status

This also makes the otel.md claim ("upgrade, diff, delete, status, test
nested under the release span") fully accurate.

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

* feat(telemetry): OTel metrics — helm exec duration and release results (PR 4)

Implements the metrics increment of docs/proposals/otel-tracing.md
(#2767) on the same provider, switch, and resource as traces:

- pkg/telemetry/metrics.go: helmfile.helm.exec.duration histogram
  (subcommand, success) and helmfile.release.count counter (verb,
  result). Instruments come from the otel global meter, so recording at
  call sites is branch-free no-op when telemetry is disabled.
- Setup builds the resource once and installs both providers; reader
  selection delegates to autoexport (OTEL_METRICS_EXPORTER: otlp |
  console | prometheus | none), the OTLP reader's interval honors
  OTEL_METRIC_EXPORT_INTERVAL (read by the SDK). Shutdown flushes both
  providers (errors.Join). StartCommandSpan now carries the meter
  provider across state transitions (fixes a nil-shutdown panic).
- helmexec: finishExecSpan records exec duration for helm binaries;
  state: endReleaseSpan counts release outcomes for sync/diff/delete/
  status/test/prepare (diff counted as success when no hard error).
- otlptest: recorder routes by OTLP path (/v1/traces vs /v1/metrics)
  and decodes metrics; new FindMetric helper.
- tests: metrics recorded as no-op when disabled, provider enabled with
  the none exporter, degradation on an invalid metrics exporter, and an
  integration assertion (status exec duration datapoint + one successful
  release.count) in the shim-based state test.

Verified with the console exporter: helmfile template emits
helmfile.helm.exec.duration per subcommand (version/dependency/
template) and helmfile.release.count{verb=prepare,result=success}=1.

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

* docs: complete OTel documentation coverage

- docs/cli.md: --otel-tracing in the CLI reference help block (verbatim
  from the cobra output)
- CHANGELOG.md: [Unreleased] Added entry for tracing + metrics
- docs/index.md: Observability highlight linking docs/otel.md
- docs/proposals/otel-tracing.md: add OTEL_METRICS_EXPORTER /
  OTEL_METRIC_EXPORT_INTERVAL rows to the env-var table and note the
  periodic reader + bounded metric cardinality in §7

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

* fix: drop unused id parameter from parsePart (unparam)

The id parameter was never used inside the span wrapper; the caller's id
variable is still used for the render calls and error messages.

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

* fix(telemetry): address review — redaction gaps, kubedog valve, phantom metrics

Addresses all Copilot review comments on #2769:

Security (span payloads):
- exec.args: positional arguments are additionally passed through
  helmexec.RedactedURL, so credentials embedded in chart/repository URLs
  (AddRepo, RegistryLogin, OCI refs) are masked exactly like log output
- release spans sanitize helmfile.chart the same way
- error statuses no longer embed raw errors (which contain rendered
  commands, arguments, and subprocess output): the command span, release
  spans, hook spans, and exec spans now use generic descriptions; the
  concrete exit code remains an attribute, and RecordError on the root
  span is dropped

Correctness:
- kubedog safety valve restored: execWithContext now attaches the
  per-release span into the runner's own context instead of replacing it,
  so trackHandle.Cancel() can interrupt a wedged helm again and app
  cancellation semantics stay exactly as before the PR
- diff release spans/metrics: real failures are recorded (exit code 2
  "changes detected" still counts as success); previously every diff was
  exported as successful
- skipped releases no longer emit phantom spans and inflate
  helmfile.release.count: iterateOnReleases callers pass a skip predicate
  (skipUndesired for status/test; delete deletes undesired releases and
  passes nil)
- Setup shuts down the already-constructed tracer provider (bounded) when
  the metrics provider fails, instead of abandoning its batch goroutine

Tests: URL redaction cases (masked/untouched), spanAttachedContext
preserves the runner cancellation chain while attaching the caller's
span, skipUndesired, and a failing-hook span asserting the generic
message.

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

* fix: lint — restore nolint placement and avoid nil context literal

- the skipUndesired insertion had displaced the // nolint: unparam
  directive off iterateOnReleases (helm param is intentionally unused
  there); also fixes a skipDesired/skipUndesired comment typo
- use a typed nil in TestSpanAttachedContext (staticcheck SA1012)

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

* fix(telemetry): address review round 2 — remote-ref redaction, wrapper helm binaries, hook release attribution

Addresses all 6 new review comments on #2769:

Security (remote references):
- new helmexec.RedactedRef sanitizes go-getter style references for
  telemetry: forced-form prefixes (git::, s3::) preserved, whole URL
  userinfo masked (usernames carry tokens too), credential-bearing query
  parameters masked using pkg/remote's heuristic (token/password/secret/
  key/signature). Applied to helmfile.file (command span), helmfile.path
  (discover_states), helmfile.chart (release spans), and exec.args —
  log-time RedactedURL is untouched so log output is unchanged

Correctness:
- wrapper helm binaries (--helm-binary custom names) are now classified
  as helm operations by an explicit context marker stamped in the execer
  funnel, instead of the executable-basename heuristic; the same
  classification gates helmfile.helm.exec.duration, so the metric no
  longer misses wrapper invocations (classifyExec)
- release-scoped hooks (presync/postsync/preuninstall/postuninstall/
  cleanup in the sync/delete/diff workers) now attach their helmfile.hook
  spans to the active helmfile.release.* span via a variadic parent on
  the trigger functions; global hooks keep the command context and all
  29 existing call sites compile unchanged; hook cancellation stays
  detached (WithoutCancel) as before
- signal-terminated runs (Shutdown with exitCode 130/143 and nil error)
  now mark the command span with error status, consistent with their
  nonzero exit code

Tests: RedactedRef table (forced forms, userinfo, s3/token query params,
untouched cases), classifyExec marker case, hookTraceContext parent
attribution + non-cancellability + fallback.

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

* fix(telemetry): address review round 3 — redaction corner cases, value runners

Addresses 5 of the 6 new review comments on #2769 (the sixth — an
unused strings import in exit_error.go — is a false positive: Indent
still uses strings.Split/Builder and the package compiles):

- RedactArgs read the previous token from the progressively redacted
  output, so {--set, --set-string, secret} leaked the secret
  (the masked value hid the following flag). Read the previous token
  from the original input, restoring the legacy contract for adjacent
  secret flags
- RedactedRef fails closed for malformed references: URL-like refs with
  invalid percent escapes export a fully redacted value, and an
  unparseable query is dropped entirely instead of exported verbatim
- ShellRunner has value receivers, so a ShellRunner VALUE satisfies the
  Runner API; the helm marker stamping and the per-release span
  attachment now handle both value and pointer forms (matching
  WithContext), so value-runner callers keep release nesting and the
  helm.exec classification/metric

Regression tests: adjacent secret flags (legacy + strict), malformed
URL-like ref, malformed query, value-runner marker + span attachment.

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

* fix(telemetry): stamp the helm marker on the stdin funnel too

execStdIn (registry login, repo add) called the runner directly, so
wrapper --helm-binary names were misclassified as os.exec and omitted
from helmfile.helm.exec.duration on that path. The marking now goes
through a shared markHelmRunner helper (value and pointer ShellRunner
forms) used by both execution funnels.

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

* fix(telemetry): redact helm's --kube-token in strict profile

Helm's global --kube-token carries a bearer token; both the
two-argument and inline forms are now masked in span exec.args
(legacy exit-error output is untouched, matching its historical
behavior).

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

* fix(telemetry): OTel metrics best-practice alignment

- helmfile.helm.exec.duration now declares explicit bucket boundaries
  tuned for seconds-scale helm invocations (5ms…600s); the SDK defaults
  are millisecond-oriented and lumped every sub-5s invocation — the
  common case — into the first bucket, defeating the histogram
- instruments are re-created under the installed provider with the
  instrumentation scope version stamped (Setup-time, race-free)
- helmfile.release.count declares the {release} curly-annotation unit
  per the metrics naming conventions

Tested end-to-end via the OTLP integration test: exported bounds are
the tuned set, units are asserted, and the scope carries the version.

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

* feat(telemetry): per-release duration metrics behind an opt-in switch

New helmfile.release.duration histogram (seconds, same tuned buckets)
with bounded dimensions by default (verb, result). Setting
HELMFILE_OTEL_METRICS_PER_RELEASE=true adds helmfile.release and
helmfile.namespace, answering "which release is slow" from dashboards:

- well-suited to bounded CI runs; long-lived centralized collection
  needs a backend capacity/TTL story (documented in docs/otel.md)
- per-release timing remains available in traces without the flag
- env read per call (release operations are low-frequency, and tests
  toggle it)

endReleaseSpan now takes the release and the operation start time; the
five worker-loop call sites pass them (doWithReleaseSpan, SyncReleases,
DeleteReleasesForSync, PrepareCharts, DiffReleases).

Verified end-to-end with the console exporter (default dims vs
per-release) and OTLP integration tests pinning both modes.

Refs: #2767, #2769
Signed-off-by: yxxhero <aiopsclub@163.com>

* refactor(telemetry): maintainability pass over the runner/metric plumbing

- StartCommandSpan copies the tracingState struct instead of enumerating
  fields by hand — that pattern dropped the meter provider once already
- the two value/pointer ShellRunner switches (helm marker, span
  attachment) are unified into one withRunnerCtx helper; the duplication
  caused two review rounds of value-form misses
- classifyExec derives the helm classification from the span name
  (helmExecSpanName constant) instead of returning a third parallel bool
- metrics: shared outcomeAttrs for the verb/result dimensions, and the
  bucket slice renamed to durationBuckets with a comment covering both
  histograms that use it

No behavior change; full -race suite green, lint clean.

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

* refactor(telemetry): consolidate test env lists, trace bridges, and hook prep; sync the design doc

Maintainability:
- HermeticEnvVars is now exported from pkg/telemetry (the owner of the
  env surface) and used by both telemetry tests and otlptest — the two
  copies had already drifted once (HELMFILE_OTEL_METRICS_PER_RELEASE
  needed updating in both)
- kubedogTraceContext and hookTraceContext were the same concept written
  twice; unified into traceOnlyContext(parent...) in span.go

Readability:
- runHook's nested kubectl rewrite extracted into prepareKubectlHook
  with guard-clause structure

Accuracy (docs ↔ code, drifted over five review rounds):
- §4.4 now describes the implemented mechanism: the release span is
  INJECTED into the runner's own context (preserving the kubedog safety
  valve) rather than the runner context being replaced, and helm
  classification is marker-based for wrapper binaries
- §5 exec span rows list the actual attributes incl. URL/query masking
- §6 strict profile documents RedactedRef, --kube-token, and the
  adjacent-token guarantee

No behavior change; full -race suite green, lint clean.

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

* refactor(telemetry): drop the dead noop state, relocate skipUndesired, sync user-facing accuracy

- tracingState.noop was dead weight in the enabled state and a
  copy-surface in every transition; a single package-level
  noopTracerProvider now backs Tracer while disabled
- skipUndesired moved next to doWithReleaseSpan in span.go, its only
  conceptual home (span/metric suppression, not run plumbing)
- accuracy: the package doc, --otel-tracing flag help,
  experimental-features entry, and CHANGELOG now all say tracing AND
  metrics and list the third instrument (helmfile.release.duration with
  the HELMFILE_OTEL_METRICS_PER_RELEASE opt-in) — these had drifted
  when the metric was added; the PR description's metric table is
  updated to match as well

No behavior change; full -race suite green (except the pre-existing
network-dependent TestStorage_resolveFile flake), lint clean.

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

* refactor(telemetry): flatten Setup, name the prefix bound, dedupe test fake; fix instrument-count drift

Readability/maintainability:
- Setup drops from 56 to 39 lines: provider construction (including the
  shutdown-tracer-on-meter-failure recovery) moves to newProviders in
  exporter.go next to the constructors it composes
- refredact's magic 16 becomes maxForcedFormPrefix with a comment
- span_test's hand-rolled fakeRunner removed in favor of the existing
  mockRunner (same package)

Accuracy:
- "Two instruments" wording survived in docs/otel.md and the design
  proposal §7 after helmfile.release.duration was added; both now say
  three and mention the per-release opt-in

No behavior change; full -race suite green (except the pre-existing
network flake), lint clean.

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

* refactor(telemetry): co-locate span machinery, drop a dead export, fix docs nits

- the span plumbing helpers (markHelmExec, withRunnerCtx,
  markHelmRunner, spanAttachedContext) move from exec.go to span.go,
  next to the marker type and classifiers they serve — exec.go keeps
  only the funnel call sites
- otlptest.SpanNames was never used outside the package; unexported
- isHelmBinary's comment now states it is the FALLBACK classifier
  (funnel invocations are marker-classified), replacing the outdated
  "cosmetic distinction" framing from before the marker existed
- docs/otel.md: release-scoped hooks nest under their release span
  (added in review round 2, never documented)

No behavior change; full -race suite green, lint clean.

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

---------

Signed-off-by: yxxhero <aiopsclub@163.com>
This commit is contained in:
yxxhero
2026-09-07 20:34:10 +08:00
committed by GitHub
parent c36dbfd417
commit 16259008d5
44 changed files with 4027 additions and 172 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/helmexec/context.go
+9
View File
@@ -1,6 +1,7 @@
package helmexec
import (
"context"
"io"
)
@@ -8,4 +9,12 @@ type HelmContext struct {
HistoryMax int
WorkerIndex int
Writer io.Writer
// Ctx, when set, carries the per-release span context so that helm
// subprocesses started for this release nest under the release span
// (consumed by the execer's execWithContext funnel; nil falls back to the
// runner's own context, i.e. the historical behavior). Cancellation
// semantics are unchanged either way: callers derive it from the app
// context, which is where the runner's context comes from too.
Ctx context.Context
}
pkg/helmexec/exec.go
+59 -30
View File
@@ -181,12 +181,20 @@ func GetPluginVersion(name, pluginsDir string) (*semver.Version, error) {
return nil, fmt.Errorf("plugin %s not installed", name)
}
func redactedURL(chart string) string {
chartURL, err := url.ParseRequestURI(chart)
// RedactedURL returns ref with any password in the URL userinfo replaced,
// matching helmfile's log redaction (url.URL.Redacted). Non-URL strings are
// returned unchanged. Telemetry reuses it so span attributes are sanitized at
// least as strictly as log output.
func RedactedURL(ref string) string {
refURL, err := url.ParseRequestURI(ref)
if err != nil {
return chart
return ref
}
return chartURL.Redacted()
return refURL.Redacted()
}
func redactedURL(chart string) string {
return RedactedURL(chart)
}
// New for running helm commands
@@ -415,7 +423,7 @@ func (helm *execer) AddRepo(name, repository, cafile, certfile, keyfile, usernam
fmt.Fprintf(&buffer, "%s\n", password)
return helm.execStdIn(args, map[string]string{}, &buffer)
}
return helm.exec(args, map[string]string{}, nil)
return helm.exec(args, map[string]string{})
})
default:
helm.logger.Errorf("ERROR: unknown type '%v' for repository %v", managed, name)
@@ -434,7 +442,7 @@ func (helm *execer) UpdateRepo() error {
helm.extra = savedExtra
}()
out, err := helm.retryRepoOp("update", func() ([]byte, error) {
return helm.exec([]string{"repo", "update"}, map[string]string{}, nil)
return helm.exec([]string{"repo", "update"}, map[string]string{})
})
helm.info(out)
return err
@@ -624,7 +632,7 @@ func (helm *execer) BuildDeps(name, chart string, flags ...string) error {
args = append(args, "--plain-http")
}
out, err := helm.exec(args, map[string]string{}, nil)
out, err := helm.exec(args, map[string]string{})
helm.info(out)
return err
}
@@ -646,7 +654,7 @@ func (helm *execer) UpdateDeps(chart string) error {
args = append(args, "--plain-http")
}
out, err := helm.exec(args, map[string]string{}, nil)
out, err := helm.exec(args, map[string]string{})
helm.info(out)
return err
}
@@ -658,7 +666,7 @@ func (helm *execer) SyncRelease(context HelmContext, name, chart, namespace stri
flags = append(flags, "--history-max", strconv.Itoa(context.HistoryMax))
out, err := helm.exec(append(append(preArgs, "upgrade", "--install", name, chart), flags...), env, nil)
out, err := helm.execWithContext(context.Ctx, append(append(preArgs, "upgrade", "--install", name, chart), flags...), env, nil)
helm.info(out)
return err
}
@@ -667,7 +675,7 @@ func (helm *execer) ReleaseStatus(context HelmContext, name string, flags ...str
helm.logger.Infof("Getting status %v", name)
preArgs := make([]string, 0)
env := make(map[string]string)
out, err := helm.exec(append(append(preArgs, "status", name), flags...), env, nil)
out, err := helm.execWithContext(context.Ctx, append(append(preArgs, "status", name), flags...), env, nil)
helm.info(out)
return err
}
@@ -679,7 +687,7 @@ func (helm *execer) List(context HelmContext, filter string, flags ...string) (s
args := []string{"list", "--filter", filter}
enableLiveOutput := false
out, err := helm.exec(append(append(preArgs, args...), flags...), env, &enableLiveOutput)
out, err := helm.execWithContext(context.Ctx, append(append(preArgs, args...), flags...), env, &enableLiveOutput)
// In v2 we have been expecting `helm list FILTER` prints nothing.
// In v3 helm still prints the header like `NAME NAMESPACE REVISION UPDATED STATUS CHART APP VERSION`,
// which confuses helmfile's existing logic that treats any non-empty output from `helm list` is considered as the indication
@@ -734,7 +742,7 @@ func (helm *execer) DecryptSecret(context HelmContext, name string, flags ...str
secretArg = "decrypt"
}
enableLiveOutput := false
secretBytes, err := helm.exec(append(append(preArgs, "secrets", secretArg, absPath), flags...), env, &enableLiveOutput)
secretBytes, err := helm.execWithContext(context.Ctx, append(append(preArgs, "secrets", secretArg, absPath), flags...), env, &enableLiveOutput)
if err != nil {
secret.err = err
return "", err
@@ -844,7 +852,7 @@ func (helm *execer) TemplateRelease(name string, chart string, flags ...string)
return fmt.Errorf("output dir not found for template command")
}
out, err := helm.exec(append(args, filteredFlags...), map[string]string{}, nil)
out, err := helm.exec(append(args, filteredFlags...), map[string]string{})
if err != nil {
return err
}
@@ -876,7 +884,7 @@ func (helm *execer) TemplateRelease(name string, chart string, flags ...string)
return nil
}
out, err := helm.exec(append(args, flags...), map[string]string{}, nil)
out, err := helm.exec(append(args, flags...), map[string]string{})
if outputToFile {
// With --output-dir is passed to helm-template,
@@ -921,7 +929,7 @@ func (helm *execer) DiffRelease(context HelmContext, name, chart, namespace stri
flags = helm.filterColorFlagsForHelm4(flags, env)
}
out, err := helm.exec(append(append(preArgs, "diff", "upgrade", "--allow-unreleased", name, chart), flags...), env, overrideEnableLiveOutput)
out, err := helm.execWithContext(context.Ctx, append(append(preArgs, "diff", "upgrade", "--allow-unreleased", name, chart), flags...), env, overrideEnableLiveOutput)
// Do our best to write STDOUT only when diff existed
// Unfortunately, this works only when you run helmfile with `--detailed-exitcode`
detailedExitcodeEnabled := false
@@ -978,7 +986,7 @@ func (helm *execer) filterColorFlagsForHelm4(flags []string, env map[string]stri
func (helm *execer) Lint(name, chart string, flags ...string) error {
helm.logger.Infof("Linting release=%v, chart=%v", name, chart)
out, err := helm.exec(append([]string{"lint", chart}, flags...), map[string]string{}, nil)
out, err := helm.exec(append([]string{"lint", chart}, flags...), map[string]string{})
// Always write to stdout to write the linting result to eg. a file
helm.write(nil, out)
return err
@@ -1003,14 +1011,14 @@ func (helm *execer) Unittest(name, chart string, flags ...string) error {
}
helm.logger.Infof("Unit testing release=%v, chart=%v", name, chart)
out, err := helm.exec(append([]string{"unittest", chart}, flags...), map[string]string{}, nil)
out, err := helm.exec(append([]string{"unittest", chart}, flags...), map[string]string{})
helm.write(nil, out)
return err
}
func (helm *execer) Fetch(chart string, flags ...string) error {
helm.logger.Infof("Fetching %v", redactedURL(chart))
out, err := helm.exec(append([]string{"fetch", chart}, flags...), map[string]string{}, nil)
out, err := helm.exec(append([]string{"fetch", chart}, flags...), map[string]string{})
helm.info(out)
return err
}
@@ -1032,7 +1040,7 @@ func (helm *execer) ChartPull(chart string, path string, flags ...string) error
} else {
helmArgs = []string{"chart", "pull", chart}
}
out, err := helm.exec(helmArgs, map[string]string{"HELM_EXPERIMENTAL_OCI": "1"}, nil)
out, err := helm.exec(helmArgs, map[string]string{"HELM_EXPERIMENTAL_OCI": "1"})
helm.info(out)
return err
}
@@ -1048,7 +1056,7 @@ func (helm *execer) ChartExport(chart string, path string) error {
helm.logger.Infof("Exporting %v", chart)
helmArgs = []string{"chart", "export", chart, "--destination", path}
// no extra flags for before v3.7.0, details in helm chart export --help
out, err := helm.exec(helmArgs, map[string]string{"HELM_EXPERIMENTAL_OCI": "1"}, nil)
out, err := helm.exec(helmArgs, map[string]string{"HELM_EXPERIMENTAL_OCI": "1"})
helm.info(out)
return err
}
@@ -1057,7 +1065,7 @@ func (helm *execer) DeleteRelease(context HelmContext, name string, flags ...str
helm.logger.Infof("Deleting %v", name)
preArgs := make([]string, 0)
env := make(map[string]string)
out, err := helm.exec(append(append(preArgs, "delete", name), flags...), env, nil)
out, err := helm.execWithContext(context.Ctx, append(append(preArgs, "delete", name), flags...), env, nil)
helm.info(out)
return err
}
@@ -1067,7 +1075,7 @@ func (helm *execer) TestRelease(context HelmContext, name string, flags ...strin
preArgs := make([]string, 0)
env := make(map[string]string)
args := []string{"test", name}
out, err := helm.exec(append(append(preArgs, args...), flags...), env, nil)
out, err := helm.execWithContext(context.Ctx, append(append(preArgs, args...), flags...), env, nil)
helm.info(out)
return err
}
@@ -1081,7 +1089,7 @@ func (helm *execer) AddPlugin(name, path, version string) error {
}
// Try with verification first
out, err := helm.exec([]string{"plugin", "install", path, "--version", version}, map[string]string{}, nil)
out, err := helm.exec([]string{"plugin", "install", path, "--version", version}, map[string]string{})
// If verification fails, retry without verification (unless enforced)
if err != nil && strings.Contains(err.Error(), "does not support verification") {
@@ -1090,7 +1098,7 @@ func (helm *execer) AddPlugin(name, path, version string) error {
return fmt.Errorf("plugin %s does not support verification (remove --enforce-plugin-verification flag to allow unverified plugins)", name)
}
helm.logger.Debugf("Plugin %v does not support verification, retrying with --verify=false", name)
out, err = helm.exec([]string{"plugin", "install", path, "--version", version, "--verify=false"}, map[string]string{}, nil)
out, err = helm.exec([]string{"plugin", "install", path, "--version", version, "--verify=false"}, map[string]string{})
}
helm.info(out)
@@ -1121,7 +1129,7 @@ func (helm *execer) installHelmSecretsV4(version string) error {
args = append(args, verifyFlag)
}
out, err := helm.exec(args, map[string]string{}, nil)
out, err := helm.exec(args, map[string]string{})
if err != nil {
return fmt.Errorf("failed to install %s: %w", plugin, err)
}
@@ -1159,7 +1167,7 @@ func helmSecretsRequiresSplitInstall(version string) bool {
func (helm *execer) uninstallPlugin(name string) error {
helm.logger.Infof("Uninstalling helm plugin %v", name)
out, err := helm.exec([]string{"plugin", "uninstall", name}, map[string]string{}, nil)
out, err := helm.exec([]string{"plugin", "uninstall", name}, map[string]string{})
if err == nil {
helm.info(out)
}
@@ -1204,7 +1212,28 @@ func (helm *execer) UpdatePlugin(name, repo, version string) error {
return helm.AddPlugin(name, repo, version)
}
func (helm *execer) exec(args []string, env map[string]string, overrideEnableLiveOutput *bool) ([]byte, error) {
func (helm *execer) exec(args []string, env map[string]string) ([]byte, error) {
return helm.execWithRunner(helm.runner, args, env, nil)
}
// execWithContext behaves like exec but attaches the span carried by ctx
// (from HelmContext.Ctx) so the subprocess span nests under the per-release
// span. Crucially, the subprocess keeps the runner's own context: replacing
// it would override specialized cancellation contexts such as the kubedog
// safety valve installed via execer.WithContext. A nil ctx is exactly exec.
func (helm *execer) execWithContext(ctx context.Context, args []string, env map[string]string, overrideEnableLiveOutput *bool) ([]byte, error) {
runner := helm.runner
if ctx != nil {
runner = withRunnerCtx(runner, func(runnerCtx context.Context) context.Context {
return spanAttachedContext(runnerCtx, ctx)
})
}
return helm.execWithRunner(runner, args, env, overrideEnableLiveOutput)
}
func (helm *execer) execWithRunner(runner Runner, args []string, env map[string]string, overrideEnableLiveOutput *bool) ([]byte, error) {
runner = markHelmRunner(runner)
cmdargs := args
if len(helm.extra) > 0 {
cmdargs = append(cmdargs, helm.extra...)
@@ -1221,7 +1250,7 @@ func (helm *execer) exec(args []string, env map[string]string, overrideEnableLiv
if overrideEnableLiveOutput != nil {
enableLiveOutput = *overrideEnableLiveOutput
}
outBytes, err := helm.runner.Execute(helm.helmBinary, cmdargs, env, enableLiveOutput)
outBytes, err := runner.Execute(helm.helmBinary, cmdargs, env, enableLiveOutput)
return outBytes, err
}
@@ -1238,7 +1267,7 @@ func (helm *execer) execStdIn(args []string, env map[string]string, stdin io.Rea
}
cmd := fmt.Sprintf("exec: %s %s", helm.helmBinary, strings.Join(cmdargs, " "))
helm.logger.Debug(cmd)
outBytes, err := helm.runner.ExecuteStdIn(helm.helmBinary, cmdargs, env, stdin)
outBytes, err := markHelmRunner(helm.runner).ExecuteStdIn(helm.helmBinary, cmdargs, env, stdin)
return outBytes, err
}
@@ -1359,7 +1388,7 @@ func (helm *execer) ShowChart(chartPath string) (chart.Metadata, error) {
// metadata.Version to obtain the resolved version.
func (helm *execer) ShowChartWithFlags(chartPath string, flags ...string) (chart.Metadata, error) {
helmArgs := append([]string{"show", "chart", chartPath}, flags...)
out, err := helm.exec(helmArgs, map[string]string{}, nil)
out, err := helm.exec(helmArgs, map[string]string{})
if err != nil {
return chart.Metadata{}, err
}
pkg/helmexec/exec_test.go
+6 -6
View File
@@ -1182,7 +1182,7 @@ func Test_exec(t *testing.T) {
t.Errorf("unexpected error: %v", err)
}
env := map[string]string{}
_, err = helm.exec([]string{"version"}, env, nil)
_, err = helm.exec([]string{"version"}, env)
expected := `exec: helm version
`
if err != nil {
@@ -1196,7 +1196,7 @@ func Test_exec(t *testing.T) {
if err != nil {
t.Errorf("unexpected error: %v", err)
}
ret, _ := helm.exec([]string{"diff"}, env, nil)
ret, _ := helm.exec([]string{"diff"}, env)
if len(ret) != 0 {
t.Error("helmexec.exec() - expected empty return value")
}
@@ -1206,7 +1206,7 @@ func Test_exec(t *testing.T) {
if err != nil {
t.Errorf("unexpected error: %v", err)
}
_, err = helm.exec([]string{"diff", "release", "chart", "--timeout 10", "--wait", "--wait-for-jobs"}, env, nil)
_, err = helm.exec([]string{"diff", "release", "chart", "--timeout 10", "--wait", "--wait-for-jobs"}, env)
expected = `exec: helm --kubeconfig config --kube-context dev diff release chart --timeout 10 --wait --wait-for-jobs
`
if err != nil {
@@ -1217,7 +1217,7 @@ func Test_exec(t *testing.T) {
}
buffer.Reset()
_, err = helm.exec([]string{"version"}, env, nil)
_, err = helm.exec([]string{"version"}, env)
expected = `exec: helm --kubeconfig config --kube-context dev version
`
if err != nil {
@@ -1229,7 +1229,7 @@ func Test_exec(t *testing.T) {
buffer.Reset()
helm.SetExtraArgs("foo")
_, err = helm.exec([]string{"version"}, env, nil)
_, err = helm.exec([]string{"version"}, env)
expected = `exec: helm --kubeconfig config --kube-context dev version foo
`
if err != nil {
@@ -1245,7 +1245,7 @@ func Test_exec(t *testing.T) {
t.Errorf("unexpected error: %v", err)
}
helm.SetHelmBinary("overwritten")
_, err = helm.exec([]string{"version"}, env, nil)
_, err = helm.exec([]string{"version"}, env)
expected = `exec: overwritten version
`
if err != nil {
pkg/helmexec/exit_error.go
+7 -4
View File
@@ -11,10 +11,13 @@ func newExitError(path string, args []string, exitStatus int, err error, stderr,
out += fmt.Sprintf("PATH:\n%s", Indent(path, " "))
out += "\n\nARGS:"
for i, a := range args {
if i > 0 && strings.HasPrefix(args[i-1], "--set") && stripArgsValuesOnExitError {
a = "*** STRIP ***"
}
// The legacy profile is byte-identical to the historical inline logic;
// the goldens in exit_error_test.go pin its exact output.
redacted := RedactArgs(args, RedactionLegacy)
if !stripArgsValuesOnExitError {
redacted = args
}
for i, a := range redacted {
out += fmt.Sprintf("\n%s", Indent(fmt.Sprintf("%d: %s (%d bytes)", i, a, len(a)), " "))
}
pkg/helmexec/redact.go
+106
View File
@@ -0,0 +1,106 @@
package helmexec
import "strings"
// RedactionProfile selects how aggressively RedactArgs masks secret-bearing
// command-line arguments.
type RedactionProfile int
const (
// RedactionLegacy reproduces the historical exit-error behavior
// byte-for-byte: only the argument *following* a flag whose name starts
// with "--set" is masked. The goldens in exit_error_test.go pin this
// output; changing it changes observable error messages.
RedactionLegacy RedactionProfile = iota
// RedactionStrict masks every secret-bearing argument form known today:
// in addition to the legacy behavior it covers single-argument forms
// (--set=key=value) and credential flags (--username, --password,
// --key-file). Used for telemetry span attributes; span visibility must
// be at least as redacted as error messages.
RedactionStrict
)
// redactedArg is the placeholder written in place of secret values; its exact
// bytes are pinned by exit_error_test.go.
const redactedArg = "*** STRIP ***"
// strictNextArgFlags are the flags whose following argument is a secret.
var strictNextArgFlags = []string{
"--set",
"--set-string",
"--set-file",
"--set-json",
"--set-literal",
"--username",
"--password",
"--key-file",
"--kube-token",
}
// RedactArgs returns a copy of args with secret-bearing values masked
// according to profile. The input slice is never mutated.
func RedactArgs(args []string, profile RedactionProfile) []string {
if len(args) == 0 {
return args
}
out := make([]string, len(args))
copy(out, args)
for i := range out {
// The previous token must be read from the ORIGINAL slice: reading
// the progressively redacted output would let a masked value hide a
// following secret (e.g. {"--set", "--set-string", "secret"}).
var prev string
if i > 0 {
prev = args[i-1]
}
switch profile {
case RedactionLegacy:
if strings.HasPrefix(prev, "--set") {
out[i] = redactedArg
}
case RedactionStrict:
if isStrictNextArgFlag(prev) {
out[i] = redactedArg
} else if flag := strictInlineFlag(out[i]); flag != "" {
out[i] = flag + "=" + redactedArg
}
}
}
return out
}
func isStrictNextArgFlag(arg string) bool {
for _, flag := range strictNextArgFlags {
if arg == flag {
return true
}
}
return false
}
// strictInlineFlag returns the flag name when arg is a single-argument secret
// form such as "--set=key=value", or "" otherwise.
func strictInlineFlag(arg string) string {
for _, flag := range strictNextArgFlags {
if strings.HasPrefix(arg, flag+"=") {
return flag
}
}
return ""
}
func equalArgs(a, b []string) bool {
if len(a) != len(b) {
return false
}
for i := range a {
if a[i] != b[i] {
return false
}
}
return true
}
pkg/helmexec/redact_test.go
+241
View File
@@ -0,0 +1,241 @@
package helmexec
import (
"context"
"fmt"
"reflect"
"testing"
"github.com/stretchr/testify/assert"
"go.opentelemetry.io/otel/attribute"
)
func TestRedactArgsLegacy(t *testing.T) {
tests := []struct {
name string
args []string
want []string
}{
{
name: "two-argument --set is masked",
args: []string{"upgrade", "--set", "secret=1"},
want: []string{"upgrade", "--set", redactedArg},
},
{
name: "--set prefix covers --set-string/--set-file/--set-json",
args: []string{"--set-string", "a=b", "--set-file", "f", "--set-json", "{}"},
want: []string{"--set-string", redactedArg, "--set-file", redactedArg, "--set-json", redactedArg},
},
{
name: "single-argument --set=k=v is NOT masked (documented legacy gap)",
args: []string{"--set=secret=1"},
want: []string{"--set=secret=1"},
},
{
name: "credential flags are NOT masked (documented legacy gap)",
args: []string{"--username", "bob", "--password", "hunter2"},
want: []string{"--username", "bob", "--password", "hunter2"},
},
{
name: "first argument is never masked",
args: []string{"--set"},
want: []string{"--set"},
},
{
name: "adjacent secret flags do not leak (previous token read from original)",
args: []string{"--set", "--set-string", "secret"},
want: []string{"--set", redactedArg, redactedArg},
},
{
name: "strict: adjacent secret flags do not leak",
args: []string{"--set", "--set", "a=b"},
want: []string{"--set", redactedArg, redactedArg},
},
{
name: "empty args",
args: []string{},
want: []string{},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := RedactArgs(tt.args, RedactionLegacy)
assert.Equal(t, tt.want, got)
})
}
}
func TestRedactArgsStrict(t *testing.T) {
tests := []struct {
name string
args []string
want []string
}{
{
name: "legacy two-argument forms remain masked",
args: []string{"upgrade", "--set", "secret=1", "--set-string", "s=2"},
want: []string{"upgrade", "--set", redactedArg, "--set-string", redactedArg},
},
{
name: "single-argument --set=k=v is masked",
args: []string{"--set=secret=1", "--set-string=a=b", "--set-json={}"},
want: []string{"--set=" + redactedArg, "--set-string=" + redactedArg, "--set-json=" + redactedArg},
},
{
name: "credential flags are masked",
args: []string{"registry", "login", "--username", "bob", "--password", "hunter2"},
want: []string{"registry", "login", "--username", redactedArg, "--password", redactedArg},
},
{
name: "key-file is masked",
args: []string{"--key-file", "/etc/secrets/key"},
want: []string{"--key-file", redactedArg},
},
{
name: "password-stdin has no value to mask",
args: []string{"--password-stdin"},
want: []string{"--password-stdin"},
},
{
name: "kube-token is masked (two-argument form)",
args: []string{"--kube-context", "ctx", "upgrade", "--kube-token", "eyJhbGciOi..."},
want: []string{"--kube-context", "ctx", "upgrade", "--kube-token", redactedArg},
},
{
name: "kube-token is masked (inline form)",
args: []string{"upgrade", "--kube-token=eyJhbGciOi..."},
want: []string{"upgrade", "--kube-token=" + redactedArg},
},
{
name: "benign flags untouched",
args: []string{"upgrade", "--install", "envoy", "./chart", "--namespace", "ingress", "--reset-values"},
want: []string{"upgrade", "--install", "envoy", "./chart", "--namespace", "ingress", "--reset-values"},
},
{
name: "prefix lookalikes untouched",
args: []string{"--settlement", "x", "--usernames", "y"},
want: []string{"--settlement", "x", "--usernames", "y"},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := RedactArgs(tt.args, RedactionStrict)
assert.Equal(t, tt.want, got)
})
}
}
func TestRedactArgsDoesNotMutateInput(t *testing.T) {
orig := []string{"--set", "secret=1", "--set=x=y"}
before := reflect.ValueOf(orig).Pointer()
_ = RedactArgs(orig, RedactionStrict)
assert.Equal(t, []string{"--set", "secret=1", "--set=x=y"}, orig, "input slice must not be mutated")
assert.Equal(t, before, reflect.ValueOf(orig).Pointer(), "input backing array must be reused by the caller, not modified")
}
func TestClassifyExec(t *testing.T) {
tests := []struct {
name string
cmd string
args []string
wantName string
wantAttrVals map[string][]string
ctxMarked bool
}{
{
name: "helm invocation",
cmd: "/usr/local/bin/helm",
args: []string{"upgrade", "--install", "envoy", "./chart"},
wantName: "helm.exec",
wantAttrVals: map[string][]string{
"exec.command": {"helm"},
"helm.subcommand": {"upgrade"},
},
},
{
name: "helm with leading flags: subcommand is first positional",
cmd: "helm",
args: []string{"--kube-context", "ctx", "repo", "update"},
wantName: "helm.exec",
wantAttrVals: map[string][]string{
"helm.subcommand": {"repo"},
},
},
{
name: "non-helm binary",
cmd: "./scripts/migrate.sh",
args: []string{"--verbose"},
wantName: "os.exec",
wantAttrVals: map[string][]string{
"exec.command": {"migrate.sh"},
},
},
{
name: "wrapper helm binary classified via marker, not name",
cmd: "/opt/bin/custom-wrapper",
args: []string{"upgrade", "demo", "./chart"},
wantName: "helm.exec",
wantAttrVals: map[string][]string{
"exec.command": {"custom-wrapper"},
"helm.subcommand": {"upgrade"},
},
ctxMarked: true,
},
{
name: "secret args are redacted and flagged",
cmd: "helm",
args: []string{"upgrade", "--set", "pw=1", "--set=pw2=2"},
wantName: "helm.exec",
wantAttrVals: map[string][]string{
"exec.args": {"upgrade", "--set", redactedArg, "--set=" + redactedArg},
"exec.redacted": {"true"},
},
},
{
name: "credentials embedded in positional URLs are masked",
cmd: "helm",
args: []string{"repo", "add", "name", "https://user:token@charts.example.com/repo"},
wantName: "helm.exec",
wantAttrVals: map[string][]string{
"exec.args": {"repo", "add", "name", "https://xxxxx@charts.example.com/repo"},
"exec.redacted": {"true"},
},
},
{
name: "plain URLs are untouched",
cmd: "helm",
args: []string{"upgrade", "demo", "https://charts.example.com/repo/chart"},
wantName: "helm.exec",
wantAttrVals: map[string][]string{
"exec.args": {"upgrade", "demo", "https://charts.example.com/repo/chart"},
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
ctx := context.Background()
if tt.ctxMarked {
ctx = context.WithValue(ctx, helmExecMarker{}, true)
}
name, attrs := classifyExec(ctx, tt.cmd, tt.args)
assert.Equal(t, tt.wantName, name)
got := map[string][]string{}
for _, kv := range attrs {
switch kv.Value.Type() {
case attribute.STRING:
got[string(kv.Key)] = []string{kv.Value.AsString()}
case attribute.STRINGSLICE:
got[string(kv.Key)] = kv.Value.AsStringSlice()
case attribute.BOOL:
got[string(kv.Key)] = []string{fmt.Sprintf("%t", kv.Value.AsBool())}
}
}
for key, want := range tt.wantAttrVals {
assert.Equal(t, want, got[key], "attribute %s", key)
}
})
}
}
pkg/helmexec/refredact.go
+91
View File
@@ -0,0 +1,91 @@
package helmexec
import (
"net/url"
"strings"
)
// redactedRefValue is the placeholder substituted for credential-bearing
// userinfo and query-parameter values in telemetry attributes.
const redactedRefValue = "xxxxx"
// sensitiveQueryKeys reports whether a query-parameter key is considered
// credential-bearing, mirroring the heuristic used by pkg/remote for cache
// keys (token/password/secret/key/signature substrings).
func sensitiveQueryKey(key string) bool {
lk := strings.ToLower(key)
return strings.Contains(lk, "token") || strings.Contains(lk, "password") ||
strings.Contains(lk, "secret") || strings.Contains(lk, "key") ||
strings.Contains(lk, "signature")
}
// RedactedRef sanitizes a remote reference (state-file path, chart reference,
// repository URL) for telemetry export: go-getter forced-form prefixes
// (git::, s3::, …) are preserved, URL userinfo is masked, and
// credential-bearing query parameters have their values replaced. Non-URL
// strings are returned unchanged. It is deliberately at least as strict as
// the log-time RedactedURL.
// maxForcedFormPrefix bounds the length of a recognized go-getter forced-form
// prefix ("git::", "s3::", "hg::" …); longer "::"-containing prefixes are
// treated as part of an opaque reference instead.
const maxForcedFormPrefix = 16
func RedactedRef(ref string) string {
force := ""
rest := ref
// A go-getter forced form looks like "git::https://…": a short
// alphanumeric prefix followed by "::" at the start of the reference.
if i := strings.Index(rest, "::"); i > 0 && i <= maxForcedFormPrefix && isAlphanumericPrefix(rest[:i]) {
force = rest[:i+len("::")]
rest = rest[i+len("::"):]
}
u, err := url.Parse(rest)
if err != nil {
if strings.Contains(rest, "://") {
// URL-like but malformed (e.g. a bad percent escape): fail
// closed — export a fully redacted value rather than risk
// leaking userinfo or query credentials.
return force + redactedRefValue
}
return ref
}
if u.Scheme == "" || u.Host == "" {
// Not a URL (e.g. "./charts/demo"); leave it alone.
return ref
}
// Mask the whole userinfo: usernames are as likely to carry tokens as
// passwords are (e.g. "https://x-access-token@host").
if u.User != nil {
u.User = url.User(redactedRefValue)
}
if u.RawQuery != "" {
q, err := url.ParseQuery(u.RawQuery)
if err != nil {
// Malformed query: fail closed by dropping it entirely.
u.RawQuery = ""
} else {
for key, values := range q {
if sensitiveQueryKey(key) {
for i := range values {
values[i] = redactedRefValue
}
}
}
u.RawQuery = q.Encode()
}
}
return force + u.String()
}
func isAlphanumericPrefix(s string) bool {
for _, r := range s {
if !(r >= 'a' && r <= 'z' || r >= 'A' && r <= 'Z' || r >= '0' && r <= '9') {
return false
}
}
return len(s) > 0
}
pkg/helmexec/refredact_test.go
+66
View File
@@ -0,0 +1,66 @@
package helmexec
import (
"testing"
"github.com/stretchr/testify/assert"
)
func TestRedactedRef(t *testing.T) {
tests := []struct {
name string
ref string
want string
}{
{
name: "plain chart path unchanged",
ref: "./charts/demo",
want: "./charts/demo",
},
{
name: "https userinfo fully masked (username may carry the token)",
ref: "https://user:token@charts.example.com/repo",
want: "https://xxxxx@charts.example.com/repo",
},
{
name: "go-getter forced form preserved and sanitized",
ref: "git::https://x-access-token@github.com/org/repo.git//helmfile?ref=v1",
want: "git::https://xxxxx@github.com/org/repo.git//helmfile?ref=v1",
},
{
name: "s3 query credentials masked",
ref: "s3::https://s3.amazonaws.com/bucket/helmfile?aws_access_key_id=AKIA&aws_secret_access_key=zzz&region=us-east-1",
want: "s3::https://s3.amazonaws.com/bucket/helmfile?aws_access_key_id=xxxxx&aws_secret_access_key=xxxxx&region=us-east-1",
},
{
name: "generic token query param masked",
ref: "https://example.com/file?token=abc&path=ok",
want: "https://example.com/file?path=ok&token=xxxxx",
},
{
name: "non-credential query untouched",
ref: "https://example.com/chart?ref=main&verify=true",
want: "https://example.com/chart?ref=main&verify=true",
},
{
name: "URL without scheme or host returned unchanged",
ref: "git@github.com:org/repo.git",
want: "git@github.com:org/repo.git",
},
{
name: "malformed URL-like ref fails fully redacted",
ref: "https://user:token@host/%zz",
want: redactedRefValue,
},
{
name: "malformed query is dropped entirely",
ref: "https://host/file?token=secret%zz&ok=1",
want: "https://host/file",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
assert.Equal(t, tt.want, RedactedRef(tt.ref))
})
}
}
pkg/helmexec/runner.go
+29 -5
View File
@@ -12,6 +12,7 @@ import (
"path/filepath"
"strings"
"syscall"
"time"
"go.uber.org/zap"
@@ -34,28 +35,51 @@ type ShellRunner struct {
Ctx context.Context
}
// Execute a shell command
// Execute a shell command. The whole execution is wrapped in one span
// (helm.exec/os.exec, see span.go); span overhead is nil when telemetry is
// disabled.
func (shell ShellRunner) Execute(cmd string, args []string, env map[string]string, enableLiveOutput bool) ([]byte, error) {
ctx, span, isHelm := startExecSpan(shell.Ctx, cmd, args)
defer span.End()
start := time.Now()
out, err := shell.run(ctx, cmd, args, env, enableLiveOutput)
finishExecSpan(span, isHelm, args, start, err)
return out, err
}
func (shell ShellRunner) run(ctx context.Context, cmd string, args []string, env map[string]string, enableLiveOutput bool) ([]byte, error) {
preparedCmd := exec.Command(cmd, args...)
preparedCmd.Dir = shell.Dir
preparedCmd.Env = mergeEnv(os.Environ(), env)
if !enableLiveOutput {
return Output(shell.Ctx, preparedCmd, shell.StripArgsValuesOnExitError, &logWriterGenerator{
return Output(ctx, preparedCmd, shell.StripArgsValuesOnExitError, &logWriterGenerator{
log: shell.Logger,
})
} else {
return LiveOutput(shell.Ctx, preparedCmd, shell.StripArgsValuesOnExitError, os.Stdout)
return LiveOutput(ctx, preparedCmd, shell.StripArgsValuesOnExitError, os.Stdout)
}
}
// Execute a shell command
// Execute a shell command with the given stdin; wrapped in an exec span like
// Execute.
func (shell ShellRunner) ExecuteStdIn(cmd string, args []string, env map[string]string, stdin io.Reader) ([]byte, error) {
ctx, span, isHelm := startExecSpan(shell.Ctx, cmd, args)
defer span.End()
start := time.Now()
out, err := shell.runStdIn(ctx, cmd, args, env, stdin)
finishExecSpan(span, isHelm, args, start, err)
return out, err
}
func (shell ShellRunner) runStdIn(ctx context.Context, cmd string, args []string, env map[string]string, stdin io.Reader) ([]byte, error) {
preparedCmd := exec.Command(cmd, args...)
preparedCmd.Dir = shell.Dir
preparedCmd.Env = mergeEnv(os.Environ(), env)
preparedCmd.Stdin = stdin
return Output(shell.Ctx, preparedCmd, shell.StripArgsValuesOnExitError, &logWriterGenerator{
return Output(ctx, preparedCmd, shell.StripArgsValuesOnExitError, &logWriterGenerator{
log: shell.Logger,
})
}
pkg/helmexec/span.go
+168
View File
@@ -0,0 +1,168 @@
package helmexec
import (
"context"
"errors"
"path/filepath"
"strings"
"time"
"go.opentelemetry.io/otel/attribute"
"go.opentelemetry.io/otel/codes"
"go.opentelemetry.io/otel/trace"
"github.com/helmfile/helmfile/pkg/telemetry"
)
// startExecSpan starts one span per external process started by helmfile
// (helm invocations, hooks, helmfile plugins). It is effectively free when
// telemetry is disabled: telemetry.Tracer then returns the OTel no-op tracer.
// The returned context derives from ctx (or Background when nil) and may be
// used for the subprocess itself without changing cancellation semantics.
// markHelmExec returns ctx (or Background when nil) stamped with the
// helm-invocation marker used for span classification.
func markHelmExec(ctx context.Context) context.Context {
if ctx == nil {
ctx = context.Background()
}
return context.WithValue(ctx, helmExecMarker{}, true)
}
// withRunnerCtx returns a runner (value or pointer ShellRunner form) whose
// context is transformed by f; non-ShellRunner runners pass through
// unchanged. ShellRunner has value receivers, so both forms satisfy Runner.
func withRunnerCtx(runner Runner, f func(context.Context) context.Context) Runner {
switch shell := runner.(type) {
case *ShellRunner:
clone := *shell
clone.Ctx = f(clone.Ctx)
return &clone
case ShellRunner:
clone := shell
clone.Ctx = f(clone.Ctx)
return clone
default:
return runner
}
}
// markHelmRunner returns a runner whose context carries the helm-invocation
// marker, so span classification and the helm duration metric do not rely on
// the executable basename.
func markHelmRunner(runner Runner) Runner {
return withRunnerCtx(runner, markHelmExec)
}
// spanAttachedContext returns a context that keeps runnerCtx's cancellation
// chain but carries the span from spanCtx, so the subprocess span nests under
// the caller's span while the subprocess itself stays governed by the
// runner's own context (e.g. the kubedog safety valve). A nil runnerCtx falls
// back to spanCtx.
func spanAttachedContext(runnerCtx, spanCtx context.Context) context.Context {
if runnerCtx == nil {
return spanCtx
}
return trace.ContextWithSpan(runnerCtx, trace.SpanFromContext(spanCtx))
}
// helmExecMarker marks contexts of invocations made through the execer
// funnel, so span classification is authoritative even for wrapper binaries
// whose name does not start with "helm".
type helmExecMarker struct{}
func startExecSpan(ctx context.Context, cmd string, args []string) (context.Context, trace.Span, bool) {
if ctx == nil {
ctx = context.Background()
}
name, attrs := classifyExec(ctx, cmd, args)
ctx, span := telemetry.Tracer(telemetry.ScopeHelm).Start(ctx, name, trace.WithAttributes(attrs...))
return ctx, span, name == helmExecSpanName
}
// classifyExec builds the span name and attributes for one subprocess.
// Secret-bearing arguments are always redacted with the strict profile: span
// visibility must be at least as redacted as error messages (see redact.go).
// helmExecSpanName is the single source of truth for what counts as a helm
// invocation: the span name drives both classification and the helm duration
// metric gate.
const helmExecSpanName = "helm.exec"
func classifyExec(ctx context.Context, cmd string, args []string) (string, []attribute.KeyValue) {
base := filepath.Base(cmd)
isHelm := isHelmBinary(base)
if marked, ok := ctx.Value(helmExecMarker{}).(bool); ok {
isHelm = marked
}
attrs := []attribute.KeyValue{
attribute.String("exec.command", base),
}
redacted := RedactArgs(args, RedactionStrict)
for i, a := range redacted {
// Positional arguments can be chart/repository URLs with embedded
// credentials (AddRepo, RegistryLogin, OCI and go-getter refs).
redacted[i] = RedactedRef(a)
}
attrs = append(attrs, attribute.StringSlice("exec.args", redacted))
if !equalArgs(args, redacted) {
attrs = append(attrs, attribute.Bool("exec.redacted", true))
}
if isHelm {
if sub := helmSubcommand(args); sub != "" {
attrs = append(attrs, attribute.String("helm.subcommand", sub))
}
return helmExecSpanName, attrs
}
return "os.exec", attrs
}
// isHelmBinary is the FALLBACK classifier: a base name starting with "helm"
// ("helm", "helm3", "helm-dev") for processes started outside the execer
// funnels (e.g. the version probe in helmexec.New). Invocations through the
// funnels are classified authoritatively by the helmExecMarker, so wrapper
// --helm-binary names classify correctly too.
func isHelmBinary(base string) bool {
return strings.HasPrefix(base, "helm")
}
// helmSubcommand returns the best-effort helm subcommand ("upgrade",
// "repo", ...) from raw args. Bare flags are assumed to consume the next
// argument as their value (true for the global --kube-context/--kubeconfig
// helmfile prepends); a rare bare boolean flag before the subcommand can
// misattribute the value — cosmetic only.
func helmSubcommand(args []string) string {
skipValue := false
for _, a := range args {
if skipValue {
skipValue = false
continue
}
if strings.HasPrefix(a, "-") {
skipValue = !strings.Contains(a, "=")
continue
}
return a
}
return ""
}
// finishExecSpan records a finished process's outcome on its span and, for
// helm invocations (as classified by startExecSpan, wrapper binaries
// included), the helmfile.helm.exec.duration metric.
func finishExecSpan(span trace.Span, isHelm bool, args []string, start time.Time, err error) {
if isHelm {
telemetry.RecordHelmExecDuration(time.Since(start).Seconds(), helmSubcommand(args), err == nil)
}
if err == nil {
return
}
var exitErr ExitError
if errors.As(err, &exitErr) {
span.SetAttributes(attribute.Int("exec.exit_code", exitErr.ExitStatus()))
}
// The raw error may embed command arguments and subprocess output; keep
// the span description generic (the exit code is an attribute).
span.SetStatus(codes.Error, "command failed")
}
pkg/helmexec/span_test.go
+166
View File
@@ -0,0 +1,166 @@
package helmexec
import (
"context"
"io"
"runtime"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.opentelemetry.io/otel/trace"
"go.opentelemetry.io/otel/trace/noop"
commonpb "go.opentelemetry.io/proto/otlp/common/v1"
v1 "go.opentelemetry.io/proto/otlp/trace/v1"
"github.com/helmfile/helmfile/pkg/telemetry"
"github.com/helmfile/helmfile/pkg/telemetry/otlptest"
)
// TestShellRunnerExecSpansExported drives ShellRunner.Execute through a real
// OTLP export and asserts names, attributes, error status, and parent linkage
// to the command span (i.e. the runner spans nest, never orphan).
func TestShellRunnerExecSpansExported(t *testing.T) {
if runtime.GOOS == "windows" {
t.Skip("uses the unix true/false binaries")
}
rec := otlptest.NewRecorder(t)
otlptest.SetupTelemetry(t, rec, "helmfile test")
shell := ShellRunner{
Logger: NewLogger(io.Discard, "warn"),
Ctx: telemetry.CommandContext(),
}
out, err := shell.Execute("true", nil, nil, false)
require.NoError(t, err)
assert.Empty(t, out)
// A failing command must surface its exit code and error status.
_, err = shell.Execute("false", nil, nil, false)
var exitErr ExitError
require.ErrorAs(t, err, &exitErr)
// Secret-bearing arguments must be redacted on the span.
_, err = shell.Execute("true", []string{"--set", "secret=1", "--set=also=2"}, nil, false)
require.NoError(t, err)
otlptest.ShutdownTelemetry(t)
spans := rec.Spans(t)
root := otlptest.FindSpanWhere(t, spans, func(s *v1.Span) bool { return s.Name == "helmfile test" }, "command span")
success := otlptest.FindSpanWhere(t, spans, func(s *v1.Span) bool {
cmd, ok := otlptest.AttrString(s, "exec.command")
return s.Name == "os.exec" && ok && cmd == "true" && !otlptest.HasAttr(s, "exec.redacted")
}, "success os.exec span")
if success.Status != nil {
assert.Equal(t, v1.Status_STATUS_CODE_UNSET, success.Status.Code, "success span must not be marked error")
}
failure := otlptest.FindSpanWhere(t, spans, func(s *v1.Span) bool {
cmd, ok := otlptest.AttrString(s, "exec.command")
return ok && cmd == "false"
}, "failure os.exec span")
require.NotNil(t, failure.Status)
assert.Equal(t, v1.Status_STATUS_CODE_ERROR, failure.Status.Code)
assert.EqualValues(t, 1, spanAttr(t, failure, "exec.exit_code").Value.GetIntValue())
redactedSpan := otlptest.FindSpanWhere(t, spans, func(s *v1.Span) bool { return otlptest.HasAttr(s, "exec.redacted") }, "redacted os.exec span")
assert.Equal(t, []string{"--set", redactedArg, "--set=" + redactedArg}, spanAttrStrings(t, redactedSpan, "exec.args"))
// Every os.exec span must be a child of the command span, proving the
// runner-span nesting (no orphan traces).
for _, s := range spans {
if s.Name == "os.exec" {
assert.Equal(t, root.TraceId, s.TraceId, "os.exec span must join the command trace")
assert.Equal(t, root.SpanId, s.ParentSpanId, "os.exec span must nest under the command span")
}
}
}
func spanAttr(t *testing.T, span *v1.Span, key string) *commonpb.KeyValue {
t.Helper()
for _, kv := range span.Attributes {
if kv.Key == key {
return kv
}
}
t.Fatalf("attribute %q not found on span %q", key, span.Name)
return nil
}
func spanAttrStrings(t *testing.T, span *v1.Span, key string) []string {
t.Helper()
values := spanAttr(t, span, key).Value.GetArrayValue().GetValues()
out := make([]string, 0, len(values))
for _, v := range values {
out = append(out, v.GetStringValue())
}
return out
}
// TestSpanAttachedContext pins the per-call context merge that lets helm
// subprocess spans nest under per-release spans WITHOUT overriding the
// runner's own cancellation context (the kubedog safety valve).
func TestSpanAttachedContext(t *testing.T) {
runnerCtx, valveCancel := context.WithCancel(context.Background())
defer valveCancel()
spanCtx, span := noop.NewTracerProvider().Tracer("test").Start(context.Background(), "release")
merged := spanAttachedContext(runnerCtx, spanCtx)
// The merged context still cancels with the runner's context.
assert.Equal(t, runnerCtx.Done(), merged.Done(), "cancellation authority must stay with the runner context")
// ...and carries the caller's span for nesting.
assert.Equal(t, span, trace.SpanFromContext(merged), "the caller's span must be attached")
// A typed nil runner context (distinct from the nil literal, per
// staticcheck) falls back to the span context.
var nilRunnerCtx context.Context
assert.Equal(t, spanCtx, spanAttachedContext(nilRunnerCtx, spanCtx))
}
// TestMarkHelmRunnerCoversBothFunnels pins that the shared marker helper —
// used by BOTH execWithRunner and execStdIn — stamps value and pointer
// runners alike, so wrapper binaries stay classified as helm operations on
// stdin-based invocations (registry login, repo add) too.
func TestMarkHelmRunnerCoversBothFunnels(t *testing.T) {
ptr := &ShellRunner{}
if v, ok := markHelmRunner(ptr).(*ShellRunner).Ctx.Value(helmExecMarker{}).(bool); !ok || !v {
t.Error("pointer runner must carry the helm marker")
}
val := ShellRunner{}
markedValue, ok := markHelmRunner(val).(ShellRunner)
if !ok {
t.Fatal("value runner must stay a value runner")
}
if v, ok := markedValue.Ctx.Value(helmExecMarker{}).(bool); !ok || !v {
t.Error("value runner must carry the helm marker")
}
fake := &mockRunner{}
assert.Same(t, fake, markHelmRunner(fake), "non-ShellRunner runners pass through unchanged")
}
// TestValueRunnerClassification pins that ShellRunner values (which satisfy
// Runner via value receivers) receive the helm marker and span attachment
// exactly like pointer runners.
func TestValueRunnerClassification(t *testing.T) {
spanCtx, span := noop.NewTracerProvider().Tracer("test").Start(context.Background(), "release")
// marker stamping
valueRunner := ShellRunner{}
marked := markHelmExec(valueRunner.Ctx)
if v, ok := marked.Value(helmExecMarker{}).(bool); !ok || !v {
t.Fatal("value runner ctx must carry the helm marker")
}
// span attachment via the same path execWithContext uses for values
merged := spanAttachedContext(valueRunner.Ctx, spanCtx)
assert.Equal(t, span, trace.SpanFromContext(merged))
}