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 diff --git a/src/Aspire.Hosting.Azure.Kubernetes/README.md b/src/Aspire.Hosting.Azure.Kubernetes/README.md index c31a66af4d0..99ff5691bf0 100644 --- a/src/Aspire.Hosting.Azure.Kubernetes/README.md +++ b/src/Aspire.Hosting.Azure.Kubernetes/README.md @@ -7,6 +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. ### Install the package diff --git a/src/Aspire.Hosting.Kubernetes/Deployment/HelmDeploymentEngine.cs b/src/Aspire.Hosting.Kubernetes/Deployment/HelmDeploymentEngine.cs index d6b7663f5c0..31175562bfc 100644 --- a/src/Aspire.Hosting.Kubernetes/Deployment/HelmDeploymentEngine.cs +++ b/src/Aspire.Hosting.Kubernetes/Deployment/HelmDeploymentEngine.cs @@ -120,23 +120,25 @@ 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. + // + // 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 = 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/ " + - "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); @@ -202,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); @@ -223,6 +231,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); @@ -415,19 +424,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..ebd9de8b8b4 --- /dev/null +++ b/src/Aspire.Hosting.Kubernetes/Deployment/HelmVersionValidator.cs @@ -0,0 +1,136 @@ +// 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` returns a single line in the shape + // v4.2.0+gfa15ec0 + // v4.0.0 + // v3.18.0+gb88f836 + // 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(); + + /// + /// 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) + { + ArgumentNullException.ThrowIfNull(helmRunner); + + var stdout = new StringBuilder(); + var stderr = new StringBuilder(); + + int exitCode; + try + { + exitCode = await helmRunner.RunAsync( + "version --short", + 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, 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( + $"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); + } + + if (exitCode != 0) + { + var errorText = stderr.ToString().Trim(); + var detail = string.IsNullOrEmpty(errorText) ? $"exit code {exitCode}" : errorText; + throw new InvalidOperationException( + $"'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' 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/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/src/Aspire.Hosting.Kubernetes/README.md b/src/Aspire.Hosting.Kubernetes/README.md index 07a9022a188..d9f3ed3b7ed 100644 --- a/src/Aspire.Hosting.Kubernetes/README.md +++ b/src/Aspire.Hosting.Kubernetes/README.md @@ -4,6 +4,12 @@ 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.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 diff --git a/tests/Aspire.Hosting.Kubernetes.Tests/FakeHelmRunner.cs b/tests/Aspire.Hosting.Kubernetes.Tests/FakeHelmRunner.cs new file mode 100644 index 00000000000..721f025d98c --- /dev/null +++ b/tests/Aspire.Hosting.Kubernetes.Tests/FakeHelmRunner.cs @@ -0,0 +1,73 @@ +// 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 bool WasVersionCalled { get; private set; } + + public bool ThrowOnVersion { get; 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`). + 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); + } + + 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..134498cfc38 --- /dev/null +++ b/tests/Aspire.Hosting.Kubernetes.Tests/HelmVersionValidatorTests.cs @@ -0,0 +1,121 @@ +// 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); + } + + [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); + } + } +} diff --git a/tests/Aspire.Hosting.Kubernetes.Tests/KubernetesDeployTests.cs b/tests/Aspire.Hosting.Kubernetes.Tests/KubernetesDeployTests.cs index 3510288da0e..6be1948ad35 100644 --- a/tests/Aspire.Hosting.Kubernetes.Tests/KubernetesDeployTests.cs +++ b/tests/Aspire.Hosting.Kubernetes.Tests/KubernetesDeployTests.cs @@ -532,6 +532,87 @@ public async Task HelmUninstallStep_RequiredByDestroy() Assert.Contains(helmUninstallLines, msg => msg.Contains("destroy-helm-env")); } + [Fact] + public async Task HelmUninstallStep_DependsOnCheckHelmPrereqs() + { + // 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( + 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 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 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] + 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() { @@ -1657,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); @@ -1679,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 @@ -1686,28 +1768,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() {