From 3a088b9edf30f363d1fd0ee058b3b19121d70ce6 Mon Sep 17 00:00:00 2001 From: Mitch Denny Date: Tue, 26 May 2026 12:47:38 +1000 Subject: [PATCH 1/7] Validate Helm CLI version (>= 4.2.0) before Kubernetes deploy Aspire's Kubernetes deployment pipeline shells out to 'helm upgrade --install' for the main application chart and for any AddHelmChart(...) resources on a KubernetesEnvironmentResource. Previously we only checked that 'helm' was on PATH; we never asserted the installed Helm version was new enough for the flags and behaviors we depend on (e.g. '--server-side=true --force-conflicts' in the Helm 4 form). Missing or older Helm produced confusing low-level errors like 'unknown flag: --force-conflicts', 'Flag --force has been deprecated', or raw process-spawn failures. Changes: * Add internal HelmVersionValidator that runs 'helm version --short --client', parses the SemVer, and asserts a minimum of Helm 4.2.0. Throws a clear actionable InvalidOperationException (detected vs required + link to https://helm.sh/docs/intro/install/) when the version is too old, unparseable, or the command fails. * Wire the validator into the existing check-helm-prereqs-{env} pipeline step in HelmDeploymentEngine. One check per environment covers both the main chart deploy and AddHelmChart(...) flows since they all DependsOn this step. * Update the 'Helm CLI not found' message to also mention the minimum version requirement. * Remove the now-redundant ad-hoc 'helm version --short' probe at the top of HelmDeployAsync (the prereq step covers it with a much better error). * Promote FakeHelmRunner to a file-scoped test helper that emits canned 'helm version' stdout (defaults to v4.2.0+gfa15ec0) and supports a separate VersionExitCode, so any test exercising the deploy path automatically passes the prereq check. * Add HelmVersionValidatorTests covering: SemVer parsing of v3/v4/v5 outputs with and without '+gitsha' build metadata, rejection of unparseable output, threshold behavior for too-old versions (v4.1.0, v4.0.0, v3.18.0, v3.14.4), and that error messages include the detected version, the required version, and the install docs URL. * Document the Helm 4.2.0+ requirement in the Aspire.Hosting.Kubernetes and Aspire.Hosting.Azure.Kubernetes READMEs. Fixes #16977 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/Aspire.Hosting.Azure.Kubernetes/README.md | 1 + .../Deployment/HelmDeploymentEngine.cs | 31 ++--- .../Deployment/HelmVersionValidator.cs | 126 ++++++++++++++++++ src/Aspire.Hosting.Kubernetes/README.md | 4 + .../FakeHelmRunner.cs | 62 +++++++++ .../HelmVersionValidatorTests.cs | 88 ++++++++++++ .../KubernetesDeployTests.cs | 22 --- 7 files changed, 295 insertions(+), 39 deletions(-) create mode 100644 src/Aspire.Hosting.Kubernetes/Deployment/HelmVersionValidator.cs create mode 100644 tests/Aspire.Hosting.Kubernetes.Tests/FakeHelmRunner.cs create mode 100644 tests/Aspire.Hosting.Kubernetes.Tests/HelmVersionValidatorTests.cs diff --git a/src/Aspire.Hosting.Azure.Kubernetes/README.md b/src/Aspire.Hosting.Azure.Kubernetes/README.md index c31a66af4d0..f530521f389 100644 --- a/src/Aspire.Hosting.Azure.Kubernetes/README.md +++ b/src/Aspire.Hosting.Azure.Kubernetes/README.md @@ -7,6 +7,7 @@ Provides extension methods and resource definitions for an Aspire AppHost to con ### Prerequisites - An Azure subscription - [create one for free](https://azure.microsoft.com/free/) +- [Helm](https://helm.sh/docs/intro/install/) **v4.2.0 or later** on your `PATH`. Aspire shells out to `helm upgrade --install` to deploy the generated chart and any `AddHelmChart(...)` releases, and validates the Helm version up front so missing or older installs produce a clear actionable error instead of cryptic flag failures. ### Install the package diff --git a/src/Aspire.Hosting.Kubernetes/Deployment/HelmDeploymentEngine.cs b/src/Aspire.Hosting.Kubernetes/Deployment/HelmDeploymentEngine.cs index d6b7663f5c0..f27e8584bc8 100644 --- a/src/Aspire.Hosting.Kubernetes/Deployment/HelmDeploymentEngine.cs +++ b/src/Aspire.Hosting.Kubernetes/Deployment/HelmDeploymentEngine.cs @@ -120,23 +120,30 @@ internal static Task> CreateStepsAsync( var model = factoryContext.PipelineContext.Model; var steps = new List(); - // Step 0: Check prerequisites — verify Helm CLI is available + // Step 0: Check prerequisites — verify Helm CLI is available and meets the + // minimum supported version. Doing this once per environment, before any + // helm invocation in either the main chart deploy or AddHelmChart(...) flows, + // turns confusing low-level errors (unknown-flag, deprecated-flag, raw + // spawn errors) into a single actionable message. var checkPrereqStep = new PipelineStep { Name = $"check-helm-prereqs-{environment.Name}", Description = $"Verifies Helm CLI is available for {environment.Name}.", - Action = ctx => + Action = async ctx => { var helmPath = PathLookupHelper.FindFullPathFromPath("helm"); if (helmPath is null) { throw new InvalidOperationException( - "Helm CLI not found. Install it from https://helm.sh/docs/intro/install/ " + + $"Helm CLI not found. Aspire requires Helm {HelmVersionValidator.MinimumHelmVersion} or later. " + + "Install it from https://helm.sh/docs/intro/install/ " + "and ensure it is available on your PATH."); } ctx.Logger.LogDebug("Helm CLI found at: {HelmPath}", helmPath); - return Task.CompletedTask; + + var helmRunner = ctx.Services.GetRequiredService(); + await HelmVersionValidator.EnsureMinimumVersionAsync(helmRunner, ctx.CancellationToken).ConfigureAwait(false); } }; steps.Add(checkPrereqStep); @@ -415,19 +422,9 @@ private static async Task HelmDeployAsync(PipelineStepContext context, Kubernete { var helmRunner = context.Services.GetRequiredService(); - // Verify helm is available - try - { - var versionExitCode = await helmRunner.RunAsync("version --short", cancellationToken: context.CancellationToken).ConfigureAwait(false); - if (versionExitCode != 0) - { - throw new InvalidOperationException("'helm' is installed but returned an error. Ensure 'helm' is properly configured and your cluster is accessible."); - } - } - catch (Exception ex) when (ex is not InvalidOperationException and not OperationCanceledException) - { - throw new InvalidOperationException("'helm' was not found. Please install 'helm' and ensure it is available on your PATH to deploy to Kubernetes.", ex); - } + // Helm presence + version validation already ran in + // check-helm-prereqs-{env}, which is a transitive predecessor of this + // step. No need to re-verify here. var valuesFilePath = Path.Combine(outputPath, "values.yaml"); var arguments = new StringBuilder(); diff --git a/src/Aspire.Hosting.Kubernetes/Deployment/HelmVersionValidator.cs b/src/Aspire.Hosting.Kubernetes/Deployment/HelmVersionValidator.cs new file mode 100644 index 00000000000..1ce99d32d76 --- /dev/null +++ b/src/Aspire.Hosting.Kubernetes/Deployment/HelmVersionValidator.cs @@ -0,0 +1,126 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Globalization; +using System.Text; +using System.Text.RegularExpressions; + +namespace Aspire.Hosting.Kubernetes; + +/// +/// Validates that the installed Helm CLI is new enough for the flags and behaviors +/// Aspire's Kubernetes deployment pipeline depends on. +/// +/// +/// Aspire authors `helm upgrade --install` invocations against Helm 4.x. In particular +/// the `--server-side=true --force-conflicts` combination emitted by +/// WithForceConflicts() matches the Helm 4 form of --server-side (string +/// valued: "true" | "false" | "auto", https://github.com/helm/helm/pull/13649). +/// Validating up-front turns errors like unknown flag: --force-conflicts or +/// Flag --force has been deprecated, use --force-replace instead into a single +/// clear actionable message. +/// +internal static partial class HelmVersionValidator +{ + /// + /// Minimum supported Helm version. See class remarks for rationale. + /// + public static readonly Version MinimumHelmVersion = new(4, 2, 0); + + private const string InstallDocsUrl = "https://helm.sh/docs/intro/install/"; + + // `helm version --short --client` returns a single line in the shape + // v4.2.0+gfa15ec0 + // v4.0.0 + // v3.18.0+gb88f836 + // Match the leading optional `v` followed by MAJOR.MINOR.PATCH; ignore any + // `+gitsha` build metadata. Anchored at the start because some shells/wrappers + // can prepend banner lines. + [GeneratedRegex(@"v?(?\d+)\.(?\d+)\.(?\d+)")] + private static partial Regex HelmVersionRegex(); + + /// + /// Runs helm version --short --client, parses the SemVer, and throws + /// if the installed version is older than + /// or if the output cannot be parsed. + /// + public static async Task EnsureMinimumVersionAsync( + IHelmRunner helmRunner, + CancellationToken cancellationToken) + { + ArgumentNullException.ThrowIfNull(helmRunner); + + var stdout = new StringBuilder(); + var stderr = new StringBuilder(); + + int exitCode; + try + { + exitCode = await helmRunner.RunAsync( + "version --short --client", + onOutputData: line => stdout.AppendLine(line), + onErrorData: line => stderr.AppendLine(line), + cancellationToken: cancellationToken).ConfigureAwait(false); + } + catch (Exception ex) when (ex is not OperationCanceledException) + { + // ProcessUtil throws when the process itself can't be spawned (helm not on + // PATH on some platforms, permission denied, etc.). The prereq step's PATH + // lookup runs before us, so this is a defensive fallback. + throw new InvalidOperationException( + $"Failed to invoke 'helm version --short --client'. Install Helm {MinimumHelmVersion} or later from {InstallDocsUrl} and ensure it is available on your PATH.", + ex); + } + + if (exitCode != 0) + { + var errorText = stderr.ToString().Trim(); + var detail = string.IsNullOrEmpty(errorText) ? $"exit code {exitCode}" : errorText; + throw new InvalidOperationException( + $"'helm version --short --client' failed ({detail}). Aspire requires Helm {MinimumHelmVersion} or later. See {InstallDocsUrl}."); + } + + var rawOutput = stdout.ToString().Trim(); + if (!TryParseHelmVersion(rawOutput, out var detected)) + { + throw new InvalidOperationException( + $"Could not parse Helm version from 'helm version --short --client' output: '{rawOutput}'. Aspire requires Helm {MinimumHelmVersion} or later. See {InstallDocsUrl}."); + } + + if (detected < MinimumHelmVersion) + { + throw new InvalidOperationException( + string.Format( + CultureInfo.InvariantCulture, + "Helm {0} was detected, but Aspire requires Helm {1} or later to deploy Kubernetes resources. Upgrade Helm from {2}.", + detected, + MinimumHelmVersion, + InstallDocsUrl)); + } + } + + /// + /// Extracts the first MAJOR.MINOR.PATCH token from the given Helm version + /// output. Returns if no version token is present. + /// + internal static bool TryParseHelmVersion(string output, out Version version) + { + version = new Version(0, 0, 0); + if (string.IsNullOrWhiteSpace(output)) + { + return false; + } + + var match = HelmVersionRegex().Match(output); + if (!match.Success) + { + return false; + } + + var major = int.Parse(match.Groups["major"].ValueSpan, CultureInfo.InvariantCulture); + var minor = int.Parse(match.Groups["minor"].ValueSpan, CultureInfo.InvariantCulture); + var patch = int.Parse(match.Groups["patch"].ValueSpan, CultureInfo.InvariantCulture); + version = new Version(major, minor, patch); + return true; + } +} diff --git a/src/Aspire.Hosting.Kubernetes/README.md b/src/Aspire.Hosting.Kubernetes/README.md index 07a9022a188..abf0461056c 100644 --- a/src/Aspire.Hosting.Kubernetes/README.md +++ b/src/Aspire.Hosting.Kubernetes/README.md @@ -4,6 +4,10 @@ Provides publishing extensions to Aspire for Kubernetes. ## Getting started +### Prerequisites + +- [Helm](https://helm.sh/docs/intro/install/) **v4.2.0 or later** on your `PATH`. Aspire shells out to `helm upgrade --install` to deploy the generated chart, and validates the Helm version up front so missing or older installs produce a clear actionable error instead of cryptic flag failures. + ### Install the package In your AppHost project, install the Aspire Kubernetes Hosting library with [NuGet](https://www.nuget.org): diff --git a/tests/Aspire.Hosting.Kubernetes.Tests/FakeHelmRunner.cs b/tests/Aspire.Hosting.Kubernetes.Tests/FakeHelmRunner.cs new file mode 100644 index 00000000000..b455676cf18 --- /dev/null +++ b/tests/Aspire.Hosting.Kubernetes.Tests/FakeHelmRunner.cs @@ -0,0 +1,62 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace Aspire.Hosting.Kubernetes.Tests; + +/// +/// In-memory fake of for tests. Records arguments, +/// returns a configurable exit code, and emits canned stdout for +/// helm version probes so the prereq validator sees a valid response by +/// default. +/// +internal sealed class FakeHelmRunner : IHelmRunner +{ + public bool WasUninstallCalled { get; private set; } + + public string? LastArguments { get; private set; } + + public int ExitCode { get; set; } + + /// + /// Output emitted to onOutputData when arguments start with + /// "version". Defaults to a recent stable Helm 4.x release so the + /// prereq version validator passes. + /// + public string VersionOutput { get; set; } = "v4.2.0+gfa15ec0"; + + /// + /// Exit code returned specifically for helm version calls. Defaults to + /// 0 so version probing succeeds regardless of , which + /// is used to model failures in the main command under test. + /// + public int VersionExitCode { get; set; } + + public Task RunAsync( + string arguments, + string? workingDirectory = null, + Action? onOutputData = null, + Action? onErrorData = null, + CancellationToken cancellationToken = default) + { + LastArguments = arguments; + + // Match any `helm version ...` probe (the validator passes + // `version --short --client`). + if (arguments.StartsWith("version", StringComparison.OrdinalIgnoreCase)) + { + if (onOutputData is not null && !string.IsNullOrEmpty(VersionOutput)) + { + onOutputData(VersionOutput); + } + + return Task.FromResult(VersionExitCode); + } + + if (arguments.StartsWith("uninstall", StringComparison.OrdinalIgnoreCase)) + { + WasUninstallCalled = true; + } + + return Task.FromResult(ExitCode); + } +} diff --git a/tests/Aspire.Hosting.Kubernetes.Tests/HelmVersionValidatorTests.cs b/tests/Aspire.Hosting.Kubernetes.Tests/HelmVersionValidatorTests.cs new file mode 100644 index 00000000000..dff4322b670 --- /dev/null +++ b/tests/Aspire.Hosting.Kubernetes.Tests/HelmVersionValidatorTests.cs @@ -0,0 +1,88 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace Aspire.Hosting.Kubernetes.Tests; + +public class HelmVersionValidatorTests +{ + [Theory] + [InlineData("v4.2.0+gfa15ec0", 4, 2, 0)] + [InlineData("v4.0.0", 4, 0, 0)] + [InlineData("v4.5.1+g123abc", 4, 5, 1)] + [InlineData("v5.0.0", 5, 0, 0)] + [InlineData("v3.18.0+gb88f836", 3, 18, 0)] + [InlineData("4.2.0", 4, 2, 0)] + public void TryParseHelmVersion_ValidOutput_ReturnsTrueAndVersion(string output, int major, int minor, int patch) + { + Assert.True(HelmVersionValidator.TryParseHelmVersion(output, out var version)); + Assert.Equal(new Version(major, minor, patch), version); + } + + [Theory] + [InlineData("")] + [InlineData(" ")] + [InlineData("not a version")] + [InlineData("helm: command not found")] + public void TryParseHelmVersion_InvalidOutput_ReturnsFalse(string output) + { + Assert.False(HelmVersionValidator.TryParseHelmVersion(output, out _)); + } + + [Fact] + public async Task EnsureMinimumVersionAsync_AtMinimum_Passes() + { + var runner = new FakeHelmRunner { VersionOutput = "v4.2.0+gfa15ec0" }; + await HelmVersionValidator.EnsureMinimumVersionAsync(runner, CancellationToken.None); + } + + [Fact] + public async Task EnsureMinimumVersionAsync_NewerVersion_Passes() + { + var runner = new FakeHelmRunner { VersionOutput = "v5.1.0+g111111" }; + await HelmVersionValidator.EnsureMinimumVersionAsync(runner, CancellationToken.None); + } + + [Theory] + [InlineData("v4.1.0+gfa15ec0")] + [InlineData("v4.0.0")] + [InlineData("v3.18.0+gb88f836")] + [InlineData("v3.14.4+gb88f836")] + public async Task EnsureMinimumVersionAsync_TooOld_ThrowsWithDetectedAndRequired(string oldVersion) + { + var runner = new FakeHelmRunner { VersionOutput = oldVersion }; + var ex = await Assert.ThrowsAsync( + () => HelmVersionValidator.EnsureMinimumVersionAsync(runner, CancellationToken.None)); + + // Detected version (without leading 'v' and without build metadata) + var trimmed = oldVersion.TrimStart('v').Split('+')[0]; + Assert.Contains(trimmed, ex.Message, StringComparison.Ordinal); + Assert.Contains(HelmVersionValidator.MinimumHelmVersion.ToString(), ex.Message, StringComparison.Ordinal); + Assert.Contains("https://helm.sh/docs/intro/install/", ex.Message, StringComparison.Ordinal); + } + + [Fact] + public async Task EnsureMinimumVersionAsync_UnparseableOutput_ThrowsWithRawOutput() + { + var runner = new FakeHelmRunner { VersionOutput = "garbage banner" }; + var ex = await Assert.ThrowsAsync( + () => HelmVersionValidator.EnsureMinimumVersionAsync(runner, CancellationToken.None)); + + Assert.Contains("garbage banner", ex.Message, StringComparison.Ordinal); + Assert.Contains(HelmVersionValidator.MinimumHelmVersion.ToString(), ex.Message, StringComparison.Ordinal); + } + + [Fact] + public async Task EnsureMinimumVersionAsync_NonZeroExitCode_ThrowsWithInstallHint() + { + var runner = new FakeHelmRunner + { + VersionOutput = string.Empty, + VersionExitCode = 1, + }; + + var ex = await Assert.ThrowsAsync( + () => HelmVersionValidator.EnsureMinimumVersionAsync(runner, CancellationToken.None)); + + Assert.Contains("https://helm.sh/docs/intro/install/", ex.Message, StringComparison.Ordinal); + } +} diff --git a/tests/Aspire.Hosting.Kubernetes.Tests/KubernetesDeployTests.cs b/tests/Aspire.Hosting.Kubernetes.Tests/KubernetesDeployTests.cs index 3510288da0e..a17488668f8 100644 --- a/tests/Aspire.Hosting.Kubernetes.Tests/KubernetesDeployTests.cs +++ b/tests/Aspire.Hosting.Kubernetes.Tests/KubernetesDeployTests.cs @@ -1686,28 +1686,6 @@ public async Task DestroyHelm_WithNoState_ReportsNothingToDestroy() Assert.Contains(completedSteps, s => s.CompletionText.Contains("Nothing to destroy", StringComparison.OrdinalIgnoreCase)); } - private sealed class FakeHelmRunner : IHelmRunner - { - public bool WasUninstallCalled { get; private set; } - public string? LastArguments { get; private set; } - public int ExitCode { get; set; } - - public Task RunAsync( - string arguments, - string? workingDirectory = null, - Action? onOutputData = null, - Action? onErrorData = null, - CancellationToken cancellationToken = default) - { - LastArguments = arguments; - if (arguments.StartsWith("uninstall", StringComparison.OrdinalIgnoreCase)) - { - WasUninstallCalled = true; - } - return Task.FromResult(ExitCode); - } - } - [Fact] public async Task DestroyHelm_WhenUninstallFails_PreservesState() { From ced45a67a2c75325d2b03f1cfcff0384fc1c6af7 Mon Sep 17 00:00:00 2001 From: Mitch Denny Date: Tue, 26 May 2026 15:16:24 +1000 Subject: [PATCH 2/7] Drop --client flag from helm version invocation The validator was invoking 'helm version --short --client', but the --client flag was removed in Helm 4 (it existed in Helm 2 for the real client/server split, was kept as a no-op in Helm 3, and is unknown in Helm 4). Since this validator's purpose is to enforce Helm 4.2.0 or later, passing --client guarantees a failure against the very minimum version we require, surfacing the exact kind of confusing prereq error this step exists to prevent. Caught by dogfood testing of PR #17491 against a local Helm 4.2.0 install, which produced: Step 'check-helm-prereqs-k8s' failed: 'helm version --short --client' failed (Error: unknown flag: --client). Aspire requires Helm 4.2.0 or later. Switch to 'helm version --short', which produces identical output shape (e.g. v4.2.0+gfa15ec0) on Helm 3 and Helm 4. Add a regression test that records the arguments passed to the runner and asserts --client is never included. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Deployment/HelmVersionValidator.cs | 19 +++++++---- .../HelmVersionValidatorTests.cs | 33 +++++++++++++++++++ 2 files changed, 46 insertions(+), 6 deletions(-) diff --git a/src/Aspire.Hosting.Kubernetes/Deployment/HelmVersionValidator.cs b/src/Aspire.Hosting.Kubernetes/Deployment/HelmVersionValidator.cs index 1ce99d32d76..acfbff25d83 100644 --- a/src/Aspire.Hosting.Kubernetes/Deployment/HelmVersionValidator.cs +++ b/src/Aspire.Hosting.Kubernetes/Deployment/HelmVersionValidator.cs @@ -29,7 +29,7 @@ internal static partial class HelmVersionValidator private const string InstallDocsUrl = "https://helm.sh/docs/intro/install/"; - // `helm version --short --client` returns a single line in the shape + // `helm version --short` returns a single line in the shape // v4.2.0+gfa15ec0 // v4.0.0 // v3.18.0+gb88f836 @@ -40,10 +40,17 @@ internal static partial class HelmVersionValidator private static partial Regex HelmVersionRegex(); /// - /// Runs helm version --short --client, parses the SemVer, and throws + /// Runs helm version --short, parses the SemVer, and throws /// if the installed version is older than /// or if the output cannot be parsed. /// + /// + /// We deliberately do not pass --client. That flag existed in Helm 2 (where + /// Tiller meant there was a separate server version), was kept as a no-op in + /// Helm 3, and was removed entirely in Helm 4 — which is our minimum. Passing + /// --client against Helm 4 fails with Error: unknown flag: --client, + /// which is exactly the cryptic failure mode this validator exists to prevent. + /// public static async Task EnsureMinimumVersionAsync( IHelmRunner helmRunner, CancellationToken cancellationToken) @@ -57,7 +64,7 @@ public static async Task EnsureMinimumVersionAsync( try { exitCode = await helmRunner.RunAsync( - "version --short --client", + "version --short", onOutputData: line => stdout.AppendLine(line), onErrorData: line => stderr.AppendLine(line), cancellationToken: cancellationToken).ConfigureAwait(false); @@ -68,7 +75,7 @@ public static async Task EnsureMinimumVersionAsync( // PATH on some platforms, permission denied, etc.). The prereq step's PATH // lookup runs before us, so this is a defensive fallback. throw new InvalidOperationException( - $"Failed to invoke 'helm version --short --client'. Install Helm {MinimumHelmVersion} or later from {InstallDocsUrl} and ensure it is available on your PATH.", + $"Failed to invoke 'helm version --short'. Install Helm {MinimumHelmVersion} or later from {InstallDocsUrl} and ensure it is available on your PATH.", ex); } @@ -77,14 +84,14 @@ public static async Task EnsureMinimumVersionAsync( var errorText = stderr.ToString().Trim(); var detail = string.IsNullOrEmpty(errorText) ? $"exit code {exitCode}" : errorText; throw new InvalidOperationException( - $"'helm version --short --client' failed ({detail}). Aspire requires Helm {MinimumHelmVersion} or later. See {InstallDocsUrl}."); + $"'helm version --short' failed ({detail}). Aspire requires Helm {MinimumHelmVersion} or later. See {InstallDocsUrl}."); } var rawOutput = stdout.ToString().Trim(); if (!TryParseHelmVersion(rawOutput, out var detected)) { throw new InvalidOperationException( - $"Could not parse Helm version from 'helm version --short --client' output: '{rawOutput}'. Aspire requires Helm {MinimumHelmVersion} or later. See {InstallDocsUrl}."); + $"Could not parse Helm version from 'helm version --short' output: '{rawOutput}'. Aspire requires Helm {MinimumHelmVersion} or later. See {InstallDocsUrl}."); } if (detected < MinimumHelmVersion) diff --git a/tests/Aspire.Hosting.Kubernetes.Tests/HelmVersionValidatorTests.cs b/tests/Aspire.Hosting.Kubernetes.Tests/HelmVersionValidatorTests.cs index dff4322b670..134498cfc38 100644 --- a/tests/Aspire.Hosting.Kubernetes.Tests/HelmVersionValidatorTests.cs +++ b/tests/Aspire.Hosting.Kubernetes.Tests/HelmVersionValidatorTests.cs @@ -85,4 +85,37 @@ public async Task EnsureMinimumVersionAsync_NonZeroExitCode_ThrowsWithInstallHin Assert.Contains("https://helm.sh/docs/intro/install/", ex.Message, StringComparison.Ordinal); } + + [Fact] + public async Task EnsureMinimumVersionAsync_DoesNotPassClientFlag() + { + // Regression: --client was removed in Helm 4 (the minimum version Aspire + // requires), so passing it makes the validator's own probe fail with + // "Error: unknown flag: --client" against the very baseline it is meant + // to validate. + var runner = new RecordingHelmRunner(); + await HelmVersionValidator.EnsureMinimumVersionAsync(runner, CancellationToken.None); + + Assert.NotNull(runner.LastArguments); + Assert.DoesNotContain("--client", runner.LastArguments, StringComparison.Ordinal); + Assert.Contains("version", runner.LastArguments, StringComparison.Ordinal); + Assert.Contains("--short", runner.LastArguments, StringComparison.Ordinal); + } + + private sealed class RecordingHelmRunner : IHelmRunner + { + public string? LastArguments { get; private set; } + + public Task RunAsync( + string arguments, + string? workingDirectory = null, + Action? onOutputData = null, + Action? onErrorData = null, + CancellationToken cancellationToken = default) + { + LastArguments = arguments; + onOutputData?.Invoke("v4.2.0+gfa15ec0"); + return Task.FromResult(0); + } + } } From d03916c5f2cc8e93fb5c9b1d72dd5648d6b716f8 Mon Sep 17 00:00:00 2001 From: Mitch Denny Date: Tue, 26 May 2026 15:20:37 +1000 Subject: [PATCH 3/7] Address PR review feedback Three review items from the automated PR reviewer: 1. Gate destroy/uninstall on the same Helm prereq check as deploy. Both 'destroy-helm-{env}' and 'helm-uninstall-{env}' invoke 'helm uninstall', so a missing or too-old Helm would surface as a raw process-spawn / unknown-flag error during teardown instead of the actionable validator message. Add a 'DependsOn(check-helm-prereqs- {env})' on both, and add a regression test that asserts the dependency edge exists. 2. Fix the misleading comment above HelmVersionRegex. The regex is intentionally unanchored so we tolerate banner/shim lines that some shells, oh-my-zsh plugins, or asdf-style shims can prepend to the version output. Update the comment to describe that intent instead of claiming a start anchor that isn't there. 3. Shorten the Helm prerequisite bullets in both Kubernetes README files. Keep the bullet to the requirement itself and move the 'why we validate up front' narrative into a short paragraph below, matching the scannable style of the other hosting READMEs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/Aspire.Hosting.Azure.Kubernetes/README.md | 4 ++- .../Deployment/HelmDeploymentEngine.cs | 5 +++ .../Deployment/HelmVersionValidator.cs | 8 +++-- src/Aspire.Hosting.Kubernetes/README.md | 4 ++- .../KubernetesDeployTests.cs | 36 +++++++++++++++++++ 5 files changed, 52 insertions(+), 5 deletions(-) diff --git a/src/Aspire.Hosting.Azure.Kubernetes/README.md b/src/Aspire.Hosting.Azure.Kubernetes/README.md index f530521f389..99ff5691bf0 100644 --- a/src/Aspire.Hosting.Azure.Kubernetes/README.md +++ b/src/Aspire.Hosting.Azure.Kubernetes/README.md @@ -7,7 +7,9 @@ Provides extension methods and resource definitions for an Aspire AppHost to con ### Prerequisites - An Azure subscription - [create one for free](https://azure.microsoft.com/free/) -- [Helm](https://helm.sh/docs/intro/install/) **v4.2.0 or later** on your `PATH`. Aspire shells out to `helm upgrade --install` to deploy the generated chart and any `AddHelmChart(...)` releases, and validates the Helm version up front so missing or older installs produce a clear actionable error instead of cryptic flag failures. +- [Helm](https://helm.sh/docs/intro/install/) **v4.2.0 or later** on your `PATH`. + +Aspire shells out to `helm upgrade --install` to deploy the generated chart and any `AddHelmChart(...)` releases, and validates the Helm version up front so missing or older installs produce a clear actionable error instead of cryptic flag failures. ### Install the package diff --git a/src/Aspire.Hosting.Kubernetes/Deployment/HelmDeploymentEngine.cs b/src/Aspire.Hosting.Kubernetes/Deployment/HelmDeploymentEngine.cs index f27e8584bc8..c42077f0d1a 100644 --- a/src/Aspire.Hosting.Kubernetes/Deployment/HelmDeploymentEngine.cs +++ b/src/Aspire.Hosting.Kubernetes/Deployment/HelmDeploymentEngine.cs @@ -219,6 +219,10 @@ await ctx.ReportingStep.CompleteAsync( }, DependsOnSteps = [WellKnownPipelineSteps.DestroyPrereq] }; + // Destroy invokes `helm uninstall`, so it needs the same version-validated + // Helm as the deploy path. Otherwise a missing or too-old Helm surfaces as + // a raw process-spawn or unknown-flag error during teardown. + helmDestroyStep.DependsOn($"check-helm-prereqs-{environment.Name}"); helmDestroyStep.RequiredBy(WellKnownPipelineSteps.Destroy); steps.Add(helmDestroyStep); @@ -230,6 +234,7 @@ await ctx.ReportingStep.CompleteAsync( Tags = [HelmUninstallTag], Action = ctx => HelmUninstallAsync(ctx, environment) }; + helmUninstallStep.DependsOn($"check-helm-prereqs-{environment.Name}"); steps.Add(helmUninstallStep); return Task.FromResult>(steps); diff --git a/src/Aspire.Hosting.Kubernetes/Deployment/HelmVersionValidator.cs b/src/Aspire.Hosting.Kubernetes/Deployment/HelmVersionValidator.cs index acfbff25d83..ab103b94ece 100644 --- a/src/Aspire.Hosting.Kubernetes/Deployment/HelmVersionValidator.cs +++ b/src/Aspire.Hosting.Kubernetes/Deployment/HelmVersionValidator.cs @@ -33,9 +33,11 @@ internal static partial class HelmVersionValidator // v4.2.0+gfa15ec0 // v4.0.0 // v3.18.0+gb88f836 - // Match the leading optional `v` followed by MAJOR.MINOR.PATCH; ignore any - // `+gitsha` build metadata. Anchored at the start because some shells/wrappers - // can prepend banner lines. + // Match the optional `v` followed by MAJOR.MINOR.PATCH; ignore any `+gitsha` + // build metadata. Intentionally unanchored: some shells / wrappers (oh-my-zsh + // plugins, asdf shims, alias output, etc.) can prefix banner or shim lines + // before the actual version token, so we accept the first valid token anywhere + // in the captured output rather than requiring it at column 0. [GeneratedRegex(@"v?(?\d+)\.(?\d+)\.(?\d+)")] private static partial Regex HelmVersionRegex(); diff --git a/src/Aspire.Hosting.Kubernetes/README.md b/src/Aspire.Hosting.Kubernetes/README.md index abf0461056c..d9f3ed3b7ed 100644 --- a/src/Aspire.Hosting.Kubernetes/README.md +++ b/src/Aspire.Hosting.Kubernetes/README.md @@ -6,7 +6,9 @@ Provides publishing extensions to Aspire for Kubernetes. ### Prerequisites -- [Helm](https://helm.sh/docs/intro/install/) **v4.2.0 or later** on your `PATH`. Aspire shells out to `helm upgrade --install` to deploy the generated chart, and validates the Helm version up front so missing or older installs produce a clear actionable error instead of cryptic flag failures. +- [Helm](https://helm.sh/docs/intro/install/) **v4.2.0 or later** on your `PATH`. + +Aspire shells out to `helm upgrade --install` to deploy the generated chart and validates the Helm version up front, so missing or older installs produce a clear actionable error instead of cryptic flag failures. ### Install the package diff --git a/tests/Aspire.Hosting.Kubernetes.Tests/KubernetesDeployTests.cs b/tests/Aspire.Hosting.Kubernetes.Tests/KubernetesDeployTests.cs index a17488668f8..e326a8888b4 100644 --- a/tests/Aspire.Hosting.Kubernetes.Tests/KubernetesDeployTests.cs +++ b/tests/Aspire.Hosting.Kubernetes.Tests/KubernetesDeployTests.cs @@ -532,6 +532,42 @@ public async Task HelmUninstallStep_RequiredByDestroy() Assert.Contains(helmUninstallLines, msg => msg.Contains("destroy-helm-env")); } + [Fact] + public async Task HelmDestroyAndUninstallSteps_DependOnCheckHelmPrereqs() + { + // Regression coverage for PR #17491 review feedback: destroy and uninstall + // also invoke `helm`, so they must gate on the same prereq check as deploy. + // Otherwise a missing or too-old Helm surfaces as a raw spawn / unknown-flag + // error during teardown instead of the actionable validator message. + using var tempDir = new TestTempDirectory(); + + var builder = TestDistributedApplicationBuilder.Create( + DistributedApplicationOperation.Publish, + tempDir.Path, + step: WellKnownPipelineSteps.Diagnostics); + var mockActivityReporter = new TestPipelineActivityReporter(output); + + builder.Services.AddSingleton(); + builder.Services.AddSingleton(mockActivityReporter); + + builder.AddKubernetesEnvironment("env"); + builder.AddContainer("api", "myimage"); + + using var app = builder.Build(); + await app.RunAsync(); + + var logs = mockActivityReporter.LoggedMessages + .Where(s => s.StepTitle == "diagnostics") + .Select(s => s.Message) + .ToList(); + + var destroyLines = logs.Where(l => l.Contains("destroy-helm-env")).ToList(); + Assert.Contains(destroyLines, msg => msg.Contains("check-helm-prereqs-env")); + + var uninstallLines = logs.Where(l => l.Contains("helm-uninstall-env")).ToList(); + Assert.Contains(uninstallLines, msg => msg.Contains("check-helm-prereqs-env")); + } + [Fact] public async Task MultipleContainersGenerateMultiplePrintSummarySteps() { From 140143838ca46fb3924a2453a15900ca39c84bdb Mon Sep 17 00:00:00 2001 From: Mitch Denny Date: Wed, 27 May 2026 09:48:16 +1000 Subject: [PATCH 4/7] Bump E2E Helm install version to v4.2.0 to match new floor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The 11 DeployK8s* CLI E2E tests failed on commit d03916c because the container install scripts default HELM_VERSION to v3.17.3 — below the new HelmVersionValidator.MinimumHelmVersion (v4.2.0) that the check-helm-prereqs-{env} pipeline step now enforces. Centralize the version constants in a new tests/Aspire.Cli.EndToEnd.Tests/Helpers/KubernetesE2EVersions.cs so the default lives in one place (and points at the validator's documented minimum), then bump HelmVersion default v3.17.3 -> v4.2.0 (used by every DeployK8s* test and by the quarantined KubernetesPublishTests). HELM_VERSION / KIND_VERSION / KUBECTL_VERSION env-var overrides are preserved so CI can still bump to a newer point release without a code change. The AKS deployment workflow (deployment-tests.yml) still pins azure/setup-helm to v4.1.4 and needs the same bump to v4.2.0 to avoid breaking AKS scenarios under the new validator; that workflow file edit will land in a separate push that has 'workflow' OAuth scope. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Helpers/KubernetesDeployTestHelpers.cs | 6 ++-- .../Helpers/KubernetesE2EVersions.cs | 33 +++++++++++++++++++ .../KubernetesPublishTests.cs | 4 +-- 3 files changed, 38 insertions(+), 5 deletions(-) create mode 100644 tests/Aspire.Cli.EndToEnd.Tests/Helpers/KubernetesE2EVersions.cs diff --git a/tests/Aspire.Cli.EndToEnd.Tests/Helpers/KubernetesDeployTestHelpers.cs b/tests/Aspire.Cli.EndToEnd.Tests/Helpers/KubernetesDeployTestHelpers.cs index 33773f0bb9f..71f90ede8a6 100644 --- a/tests/Aspire.Cli.EndToEnd.Tests/Helpers/KubernetesDeployTestHelpers.cs +++ b/tests/Aspire.Cli.EndToEnd.Tests/Helpers/KubernetesDeployTestHelpers.cs @@ -11,9 +11,9 @@ namespace Aspire.Cli.EndToEnd.Tests.Helpers; /// internal static class KubernetesDeployTestHelpers { - private static string KindVersion => Environment.GetEnvironmentVariable("KIND_VERSION") ?? "v0.31.0"; - private static string HelmVersion => Environment.GetEnvironmentVariable("HELM_VERSION") ?? "v3.17.3"; - private static string KubectlVersion => Environment.GetEnvironmentVariable("KUBECTL_VERSION") ?? "v1.34.3"; + private static string KindVersion => KubernetesE2EVersions.KindVersion; + private static string HelmVersion => KubernetesE2EVersions.HelmVersion; + private static string KubectlVersion => KubernetesE2EVersions.KubectlVersion; /// /// Generates a unique KinD cluster name (max 32 chars). diff --git a/tests/Aspire.Cli.EndToEnd.Tests/Helpers/KubernetesE2EVersions.cs b/tests/Aspire.Cli.EndToEnd.Tests/Helpers/KubernetesE2EVersions.cs new file mode 100644 index 00000000000..bcd81655525 --- /dev/null +++ b/tests/Aspire.Cli.EndToEnd.Tests/Helpers/KubernetesE2EVersions.cs @@ -0,0 +1,33 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace Aspire.Cli.EndToEnd.Tests.Helpers; + +/// +/// Single source of truth for the tool versions installed into the E2E test +/// container during Kubernetes scenarios. +/// +/// +/// Each value falls back to an environment variable so CI can pin or bump a +/// version without a code change. The defaults are chosen to satisfy Aspire's +/// own minimums: +/// +/// +/// +/// HelmVersion must be at least +/// Aspire.Hosting.Kubernetes.HelmVersionValidator.MinimumHelmVersion +/// (currently 4.2.0). The Kubernetes deployment pipeline now +/// fails fast at check-helm-prereqs-{env} for older Helm CLIs, +/// so an older default here would break every DeployK8s* test. +/// +/// +/// +/// +internal static class KubernetesE2EVersions +{ + public static string KindVersion => Environment.GetEnvironmentVariable("KIND_VERSION") ?? "v0.31.0"; + + public static string HelmVersion => Environment.GetEnvironmentVariable("HELM_VERSION") ?? "v4.2.0"; + + public static string KubectlVersion => Environment.GetEnvironmentVariable("KUBECTL_VERSION") ?? "v1.34.3"; +} diff --git a/tests/Aspire.Cli.EndToEnd.Tests/KubernetesPublishTests.cs b/tests/Aspire.Cli.EndToEnd.Tests/KubernetesPublishTests.cs index e8d963110a1..02960bd64a3 100644 --- a/tests/Aspire.Cli.EndToEnd.Tests/KubernetesPublishTests.cs +++ b/tests/Aspire.Cli.EndToEnd.Tests/KubernetesPublishTests.cs @@ -20,8 +20,8 @@ public sealed class KubernetesPublishTests(ITestOutputHelper output) private const string ProjectName = "AspireKubernetesPublishTest"; private const string ClusterNamePrefix = "aspire-e2e"; - private static string KindVersion => Environment.GetEnvironmentVariable("KIND_VERSION") ?? "v0.31.0"; - private static string HelmVersion => Environment.GetEnvironmentVariable("HELM_VERSION") ?? "v3.17.3"; + private static string KindVersion => KubernetesE2EVersions.KindVersion; + private static string HelmVersion => KubernetesE2EVersions.HelmVersion; private static string GenerateUniqueClusterName() => $"{ClusterNamePrefix}-{Guid.NewGuid():N}"[..32]; // KinD cluster names max 32 chars From df5b2077d055b1c432d19c1deff64eddcc8d2b74 Mon Sep 17 00:00:00 2001 From: Mitch Denny Date: Wed, 27 May 2026 10:14:26 +1000 Subject: [PATCH 5/7] Bump deployment-tests.yml Helm pin to v4.2.0 Match Aspire.Hosting.Kubernetes' new minimum supported Helm version (HelmVersionValidator.MinimumHelmVersion). The check-helm-prereqs-{env} pipeline step now fails fast on older Helm CLIs, so leaving the AKS deployment workflow pinned to v4.1.4 would break every AKS deployment scenario. Also refresh the surrounding rationale comment, which still referred to the historical v3.18 server-side narrative. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .github/workflows/deployment-tests.yml | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/.github/workflows/deployment-tests.yml b/.github/workflows/deployment-tests.yml index 45d1a02ae3c..f44343e91da 100644 --- a/.github/workflows/deployment-tests.yml +++ b/.github/workflows/deployment-tests.yml @@ -251,17 +251,15 @@ jobs: docker info | head -20 echo "✅ Docker is available" - # Pin helm to a version that supports `helm install --server-side`/--force-conflicts - # (added in helm v3.18.0; helm v4 keeps both but `--server-side` is now a string flag - # accepting true|false|auto, which is why our code passes `--server-side=true` rather - # than the bare flag — that form parses identically on v3.18+ and v4). The default - # helm preinstalled on the runner image lags behind and rejects these flags, which - # breaks AKS cert-manager scenarios that need server-side apply to coexist with the - # AKS Azure Policy add-on's webhook mutations. + # Pin Helm to satisfy Aspire.Hosting.Kubernetes' minimum supported version + # (HelmVersionValidator.MinimumHelmVersion, currently v4.2.0). The new + # check-helm-prereqs-{env} pipeline step now fails fast on older Helm + # CLIs, so the runner image's preinstalled Helm — which lags behind — + # would otherwise break every AKS deployment scenario. - name: Setup Helm uses: azure/setup-helm@1a275c3b69536ee54be43f2070a358922e12c8d4 # v4.3.1 with: - version: v4.1.4 + version: v4.2.0 - name: Run deployment test (${{ matrix.shortname }}) id: run_tests From b5c75355a8c4f947818496f35ad47c0336f5136f Mon Sep 17 00:00:00 2001 From: Mitch Denny Date: Wed, 27 May 2026 11:19:00 +1000 Subject: [PATCH 6/7] Gate AddHelmChart uninstall on prereqs and unify Helm CLI check Address self-review follow-ups on PR #17491: - Per-chart `helm-uninstall-{name}` step (AddHelmChart(...).WithDestroy()) now depends on `check-helm-prereqs-{env}`. Previously only the env-level destroy/uninstall steps were gated, so chart teardown could still hit the cryptic spawn / unknown-flag error the validator exists to prevent. - Drop the standalone PathLookupHelper probe from the prereq step. The validator already wraps spawn failures with the same actionable hint, and routing everything through IHelmRunner lets tests inject a fake without needing real Helm on PATH (fixes 3 pre-existing K8s test failures in environments without helm installed). - Refresh validator catch comment + error wording accordingly. - Drop stale `--client` mention in FakeHelmRunner comment. - Add regression test PerChartHelmUninstallStep_DependsOnCheckHelmPrereqs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Deployment/HelmDeploymentEngine.cs | 17 +++------ .../Deployment/HelmVersionValidator.cs | 7 ++-- .../KubernetesHelmChartExtensions.cs | 6 +++ .../FakeHelmRunner.cs | 2 +- .../KubernetesDeployTests.cs | 38 +++++++++++++++++++ 5 files changed, 55 insertions(+), 15 deletions(-) diff --git a/src/Aspire.Hosting.Kubernetes/Deployment/HelmDeploymentEngine.cs b/src/Aspire.Hosting.Kubernetes/Deployment/HelmDeploymentEngine.cs index c42077f0d1a..119c5cfce2c 100644 --- a/src/Aspire.Hosting.Kubernetes/Deployment/HelmDeploymentEngine.cs +++ b/src/Aspire.Hosting.Kubernetes/Deployment/HelmDeploymentEngine.cs @@ -125,23 +125,18 @@ internal static Task> CreateStepsAsync( // helm invocation in either the main chart deploy or AddHelmChart(...) flows, // turns confusing low-level errors (unknown-flag, deprecated-flag, raw // spawn errors) into a single actionable message. + // + // The validator drives everything through IHelmRunner: a missing binary + // surfaces as a spawn failure that the validator wraps with the same + // "install Helm" hint, so we deliberately don't do a separate + // PathLookupHelper probe here. That also lets tests inject a fake runner + // without needing real Helm on PATH. var checkPrereqStep = new PipelineStep { Name = $"check-helm-prereqs-{environment.Name}", Description = $"Verifies Helm CLI is available for {environment.Name}.", Action = async ctx => { - var helmPath = PathLookupHelper.FindFullPathFromPath("helm"); - if (helmPath is null) - { - throw new InvalidOperationException( - $"Helm CLI not found. Aspire requires Helm {HelmVersionValidator.MinimumHelmVersion} or later. " + - "Install it from https://helm.sh/docs/intro/install/ " + - "and ensure it is available on your PATH."); - } - - ctx.Logger.LogDebug("Helm CLI found at: {HelmPath}", helmPath); - var helmRunner = ctx.Services.GetRequiredService(); await HelmVersionValidator.EnsureMinimumVersionAsync(helmRunner, ctx.CancellationToken).ConfigureAwait(false); } diff --git a/src/Aspire.Hosting.Kubernetes/Deployment/HelmVersionValidator.cs b/src/Aspire.Hosting.Kubernetes/Deployment/HelmVersionValidator.cs index ab103b94ece..ebd9de8b8b4 100644 --- a/src/Aspire.Hosting.Kubernetes/Deployment/HelmVersionValidator.cs +++ b/src/Aspire.Hosting.Kubernetes/Deployment/HelmVersionValidator.cs @@ -74,10 +74,11 @@ public static async Task EnsureMinimumVersionAsync( catch (Exception ex) when (ex is not OperationCanceledException) { // ProcessUtil throws when the process itself can't be spawned (helm not on - // PATH on some platforms, permission denied, etc.). The prereq step's PATH - // lookup runs before us, so this is a defensive fallback. + // PATH, permission denied, etc.). The exception message from the runner + // is typically a low-level "No such file or directory" or + // "permission denied" that doesn't tell users what to do, so wrap it. throw new InvalidOperationException( - $"Failed to invoke 'helm version --short'. Install Helm {MinimumHelmVersion} or later from {InstallDocsUrl} and ensure it is available on your PATH.", + $"Helm CLI not found or could not be invoked. Aspire requires Helm {MinimumHelmVersion} or later. Install it from {InstallDocsUrl} and ensure it is available on your PATH.", ex); } diff --git a/src/Aspire.Hosting.Kubernetes/KubernetesHelmChartExtensions.cs b/src/Aspire.Hosting.Kubernetes/KubernetesHelmChartExtensions.cs index c0a49ebc44d..702d4de5470 100644 --- a/src/Aspire.Hosting.Kubernetes/KubernetesHelmChartExtensions.cs +++ b/src/Aspire.Hosting.Kubernetes/KubernetesHelmChartExtensions.cs @@ -113,6 +113,12 @@ public static IResourceBuilder AddHelmChart( Action = ctx => UninstallHelmChartAsync(ctx, environment, resource, releaseName, @namespace), DependsOnSteps = [WellKnownPipelineSteps.DestroyPrereq] }; + // The uninstall path shells out to `helm uninstall`, so it must observe the same + // Helm CLI / version preflight as the deploy path. Without this dep, a missing or + // too-old Helm during teardown would surface as the raw spawn / unknown-flag error + // the env-wide `check-helm-prereqs-{env}` step exists to convert into an actionable + // message. Install is already covered transitively via `helm-deploy-{env}`. + uninstallStep.DependsOn($"check-helm-prereqs-{environment.Name}"); uninstallStep.RequiredBy(WellKnownPipelineSteps.Destroy); steps.Add(uninstallStep); } diff --git a/tests/Aspire.Hosting.Kubernetes.Tests/FakeHelmRunner.cs b/tests/Aspire.Hosting.Kubernetes.Tests/FakeHelmRunner.cs index b455676cf18..e3413835bfb 100644 --- a/tests/Aspire.Hosting.Kubernetes.Tests/FakeHelmRunner.cs +++ b/tests/Aspire.Hosting.Kubernetes.Tests/FakeHelmRunner.cs @@ -41,7 +41,7 @@ public Task RunAsync( LastArguments = arguments; // Match any `helm version ...` probe (the validator passes - // `version --short --client`). + // `version --short`). if (arguments.StartsWith("version", StringComparison.OrdinalIgnoreCase)) { if (onOutputData is not null && !string.IsNullOrEmpty(VersionOutput)) diff --git a/tests/Aspire.Hosting.Kubernetes.Tests/KubernetesDeployTests.cs b/tests/Aspire.Hosting.Kubernetes.Tests/KubernetesDeployTests.cs index e326a8888b4..9021684f809 100644 --- a/tests/Aspire.Hosting.Kubernetes.Tests/KubernetesDeployTests.cs +++ b/tests/Aspire.Hosting.Kubernetes.Tests/KubernetesDeployTests.cs @@ -568,6 +568,44 @@ public async Task HelmDestroyAndUninstallSteps_DependOnCheckHelmPrereqs() Assert.Contains(uninstallLines, msg => msg.Contains("check-helm-prereqs-env")); } + [Fact] + public async Task PerChartHelmUninstallStep_DependsOnCheckHelmPrereqs() + { + // Regression coverage for PR #17491 review feedback: per-chart + // `helm-uninstall-{name}` steps created by `AddHelmChart(...).WithDestroy()` + // must depend on `check-helm-prereqs-{env}`. The install side is covered + // transitively (via `helm-deploy-{env}`), but the uninstall side previously + // only set `DependsOnSteps = [DestroyPrereq]`, so a missing or too-old + // Helm during chart teardown would bypass the validator and surface as + // the cryptic spawn / unknown-flag error this PR exists to prevent. + using var tempDir = new TestTempDirectory(); + + var builder = TestDistributedApplicationBuilder.Create( + DistributedApplicationOperation.Publish, + tempDir.Path, + step: WellKnownPipelineSteps.Diagnostics); + var mockActivityReporter = new TestPipelineActivityReporter(output); + + builder.Services.AddSingleton(); + builder.Services.AddSingleton(mockActivityReporter); + + var k8s = builder.AddKubernetesEnvironment("env"); + k8s.AddHelmChart("podinfo", "oci://ghcr.io/stefanprodan/charts/podinfo", "6.7.1") + .WithDestroy(); + + using var app = builder.Build(); + await app.RunAsync(); + + var logs = mockActivityReporter.LoggedMessages + .Where(s => s.StepTitle == "diagnostics") + .Select(s => s.Message) + .ToList(); + + var chartUninstallLines = logs.Where(l => l.Contains("helm-uninstall-podinfo")).ToList(); + Assert.NotEmpty(chartUninstallLines); + Assert.Contains(chartUninstallLines, msg => msg.Contains("check-helm-prereqs-env")); + } + [Fact] public async Task MultipleContainersGenerateMultiplePrintSummarySteps() { From 647c2417416f518eac987945ca6f7e6614770b3d Mon Sep 17 00:00:00 2001 From: Mitch Denny Date: Wed, 27 May 2026 21:15:13 +1000 Subject: [PATCH 7/7] Defer Helm version check in destroy-helm-{env} until state is found Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Deployment/HelmDeploymentEngine.cs | 10 ++++--- .../FakeHelmRunner.cs | 11 ++++++++ .../KubernetesDeployTests.cs | 28 ++++++++++++------- 3 files changed, 35 insertions(+), 14 deletions(-) diff --git a/src/Aspire.Hosting.Kubernetes/Deployment/HelmDeploymentEngine.cs b/src/Aspire.Hosting.Kubernetes/Deployment/HelmDeploymentEngine.cs index 119c5cfce2c..31175562bfc 100644 --- a/src/Aspire.Hosting.Kubernetes/Deployment/HelmDeploymentEngine.cs +++ b/src/Aspire.Hosting.Kubernetes/Deployment/HelmDeploymentEngine.cs @@ -204,6 +204,12 @@ await ctx.ReportingStep.CompleteAsync( // Use saved state for the confirmation message (more accurate than recomputing) var @namespace = savedNamespace ?? "default"; await ConfirmDestroyAsync(ctx, $"Uninstall Helm release '{savedReleaseName}' from namespace '{@namespace}'? This action cannot be undone.").ConfigureAwait(false); + + var helmRunner = ctx.Services.GetRequiredService(); + // Defer the prereq check until state exists so `aspire destroy` against a + // never-deployed environment can still report "Nothing to destroy" without + // requiring Helm on PATH. + await HelmVersionValidator.EnsureMinimumVersionAsync(helmRunner, ctx.CancellationToken).ConfigureAwait(false); await HelmUninstallAsync(ctx, environment, savedReleaseName, @namespace).ConfigureAwait(false); ctx.Summary.Add("🗑️ Helm Release", savedReleaseName); @@ -214,10 +220,6 @@ await ctx.ReportingStep.CompleteAsync( }, DependsOnSteps = [WellKnownPipelineSteps.DestroyPrereq] }; - // Destroy invokes `helm uninstall`, so it needs the same version-validated - // Helm as the deploy path. Otherwise a missing or too-old Helm surfaces as - // a raw process-spawn or unknown-flag error during teardown. - helmDestroyStep.DependsOn($"check-helm-prereqs-{environment.Name}"); helmDestroyStep.RequiredBy(WellKnownPipelineSteps.Destroy); steps.Add(helmDestroyStep); diff --git a/tests/Aspire.Hosting.Kubernetes.Tests/FakeHelmRunner.cs b/tests/Aspire.Hosting.Kubernetes.Tests/FakeHelmRunner.cs index e3413835bfb..721f025d98c 100644 --- a/tests/Aspire.Hosting.Kubernetes.Tests/FakeHelmRunner.cs +++ b/tests/Aspire.Hosting.Kubernetes.Tests/FakeHelmRunner.cs @@ -13,6 +13,10 @@ internal sealed class FakeHelmRunner : IHelmRunner { public bool WasUninstallCalled { get; private set; } + public bool WasVersionCalled { get; private set; } + + public bool ThrowOnVersion { get; set; } + public string? LastArguments { get; private set; } public int ExitCode { get; set; } @@ -44,6 +48,13 @@ public Task RunAsync( // `version --short`). if (arguments.StartsWith("version", StringComparison.OrdinalIgnoreCase)) { + WasVersionCalled = true; + + if (ThrowOnVersion) + { + throw new InvalidOperationException("Helm version should not be probed."); + } + if (onOutputData is not null && !string.IsNullOrEmpty(VersionOutput)) { onOutputData(VersionOutput); diff --git a/tests/Aspire.Hosting.Kubernetes.Tests/KubernetesDeployTests.cs b/tests/Aspire.Hosting.Kubernetes.Tests/KubernetesDeployTests.cs index 9021684f809..6be1948ad35 100644 --- a/tests/Aspire.Hosting.Kubernetes.Tests/KubernetesDeployTests.cs +++ b/tests/Aspire.Hosting.Kubernetes.Tests/KubernetesDeployTests.cs @@ -533,12 +533,12 @@ public async Task HelmUninstallStep_RequiredByDestroy() } [Fact] - public async Task HelmDestroyAndUninstallSteps_DependOnCheckHelmPrereqs() + public async Task HelmUninstallStep_DependsOnCheckHelmPrereqs() { - // Regression coverage for PR #17491 review feedback: destroy and uninstall - // also invoke `helm`, so they must gate on the same prereq check as deploy. - // Otherwise a missing or too-old Helm surfaces as a raw spawn / unknown-flag - // error during teardown instead of the actionable validator message. + // Regression coverage for PR #17491 review feedback: direct uninstall + // invokes `helm`, so it must gate on the same prereq check as deploy. + // `destroy-helm-{env}` defers the check until saved state exists so the + // no-state path can still report "Nothing to destroy" without Helm. using var tempDir = new TestTempDirectory(); var builder = TestDistributedApplicationBuilder.Create( @@ -561,11 +561,18 @@ public async Task HelmDestroyAndUninstallSteps_DependOnCheckHelmPrereqs() .Select(s => s.Message) .ToList(); - var destroyLines = logs.Where(l => l.Contains("destroy-helm-env")).ToList(); - Assert.Contains(destroyLines, msg => msg.Contains("check-helm-prereqs-env")); + var diagnosticLines = string.Join('\n', logs) + .Split('\n') + .Select(l => l.Trim()) + .ToList(); + + var destroyTargetLine = diagnosticLines.IndexOf("If targeting 'destroy-helm-env':"); + Assert.InRange(destroyTargetLine, 0, diagnosticLines.Count - 2); + Assert.Equal("Direct dependencies: destroy-prereq", diagnosticLines[destroyTargetLine + 1]); - var uninstallLines = logs.Where(l => l.Contains("helm-uninstall-env")).ToList(); - Assert.Contains(uninstallLines, msg => msg.Contains("check-helm-prereqs-env")); + var uninstallTargetLine = diagnosticLines.IndexOf("If targeting 'helm-uninstall-env':"); + Assert.InRange(uninstallTargetLine, 0, diagnosticLines.Count - 2); + Assert.Equal("Direct dependencies: check-helm-prereqs-env", diagnosticLines[uninstallTargetLine + 1]); } [Fact] @@ -1731,7 +1738,7 @@ public async Task DestroyHelm_WithNoState_ReportsNothingToDestroy() { using var tempDir = new TestTempDirectory(); - var fakeHelm = new FakeHelmRunner(); + var fakeHelm = new FakeHelmRunner { ThrowOnVersion = true }; var stateManager = new InMemoryDeploymentStateManager(); var mockActivityReporter = new TestPipelineActivityReporter(output); @@ -1753,6 +1760,7 @@ public async Task DestroyHelm_WithNoState_ReportsNothingToDestroy() await app.RunAsync(); // Verify helm was NOT called + Assert.False(fakeHelm.WasVersionCalled); Assert.False(fakeHelm.WasUninstallCalled); // Verify it reported nothing to destroy