From e9dca1a825a5ae951c5eb18a6deea25deed65267 Mon Sep 17 00:00:00 2001 From: Sebastien Ros Date: Mon, 11 May 2026 17:24:20 -0700 Subject: [PATCH 1/5] Fail when JavaScript run script conflicts with Dockerfile Detect explicit JavaScript run script configuration during publish when an existing Dockerfile would otherwise supply the container entrypoint. Fail with a clear message instead of silently ignoring runScriptName or WithRunScript, while preserving explicit container entrypoint overrides. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../JavaScriptHostingExtensions.cs | 51 +++++++- .../AddJavaScriptAppTests.cs | 113 ++++++++++++++++++ 2 files changed, 163 insertions(+), 1 deletion(-) diff --git a/src/Aspire.Hosting.JavaScript/JavaScriptHostingExtensions.cs b/src/Aspire.Hosting.JavaScript/JavaScriptHostingExtensions.cs index 2f36f3f8bd2..929ddab9517 100644 --- a/src/Aspire.Hosting.JavaScript/JavaScriptHostingExtensions.cs +++ b/src/Aspire.Hosting.JavaScript/JavaScriptHostingExtensions.cs @@ -14,6 +14,7 @@ using Aspire.Hosting.ApplicationModel.Docker; using Aspire.Hosting.JavaScript; using Aspire.Hosting.Pipelines; +using Aspire.Hosting.Publishing; using Aspire.Hosting.Utils; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Hosting; @@ -29,6 +30,7 @@ public static class JavaScriptHostingExtensions { private const string BrowserCapability = "browser"; private const string DefaultNodeVersion = "22"; + private const string DefaultJavaScriptRunScriptName = "dev"; private const string DefaultYarpImage = Yarp.YarpContainerImageTags.Registry + "/" + Yarp.YarpContainerImageTags.Image + ":" + Yarp.YarpContainerImageTags.Tag; // This is the order of config files that Vite will look for by default @@ -329,7 +331,7 @@ private static IResourceBuilder WithNodeDefaults(this IRes /// integration. /// [AspireExport(Description = "Adds a JavaScript application resource")] - public static IResourceBuilder AddJavaScriptApp(this IDistributedApplicationBuilder builder, [ResourceName] string name, string appDirectory, string runScriptName = "dev") + public static IResourceBuilder AddJavaScriptApp(this IDistributedApplicationBuilder builder, [ResourceName] string name, string appDirectory, string runScriptName = DefaultJavaScriptRunScriptName) { ArgumentNullException.ThrowIfNull(builder); ArgumentException.ThrowIfNullOrEmpty(name); @@ -873,6 +875,19 @@ private static IResourceBuilder CreateDefaultJavaScriptAppBuilder(resource.Name, out var containerBuilder)) + { + Task WriteValidatedContainerAsync(ManifestPublishingContext context) + { + ValidateExistingDockerfileRunScript(resource, containerBuilder.Resource, runScriptName); + return context.WriteContainerAsync(containerBuilder.Resource); + } + + resourceBuilder.WithManifestPublishingCallback(WriteValidatedContainerAsync); + containerBuilder.WithManifestPublishingCallback(WriteValidatedContainerAsync); + } + resourceBuilder.WithVSCodeDebugging(); // ensure the package manager command is set before starting the resource @@ -892,6 +907,40 @@ private static IResourceBuilder CreateDefaultJavaScriptAppBuilder(out var dockerfileBuildAnnotation) || + dockerfileBuildAnnotation.DockerfileFactory is not null || + !containerResource.TryGetLastAnnotation(out var runScript)) + { + return; + } + + var hasMultipleRunScriptAnnotations = + containerResource.TryGetAnnotationsOfType(out var runScriptAnnotations) && + runScriptAnnotations.Skip(1).Any(); + + var hasExplicitRunScript = + !string.Equals(initialRunScriptName, DefaultJavaScriptRunScriptName, StringComparison.Ordinal) || + hasMultipleRunScriptAnnotations || + runScript.Args.Length > 0; + + if (!hasExplicitRunScript) + { + return; + } + + // Existing Dockerfiles are user-authored, so Aspire cannot safely assume that replacing + // their entrypoint with a package-manager script will work for the image shape. + // If the user provides an explicit container entrypoint above, honor it; otherwise fail + // instead of silently publishing an image that ignores the requested run script. + throw new InvalidOperationException( + $"JavaScript app resource '{resource.Name}' is configured to run script '{runScript.ScriptName}', but publish is using the existing Dockerfile '{dockerfileBuildAnnotation.DockerfilePath}'. " + + "An existing Dockerfile entrypoint cannot be changed automatically from runScriptName or WithRunScript. " + + "Remove or rename the Dockerfile so Aspire can generate one, or call PublishAsDockerFile(...) and set the container entrypoint explicitly."); + } + /// /// Adds a Vite app to the distributed application builder. /// diff --git a/tests/Aspire.Hosting.JavaScript.Tests/AddJavaScriptAppTests.cs b/tests/Aspire.Hosting.JavaScript.Tests/AddJavaScriptAppTests.cs index 2302c6a935f..052f611090e 100644 --- a/tests/Aspire.Hosting.JavaScript.Tests/AddJavaScriptAppTests.cs +++ b/tests/Aspire.Hosting.JavaScript.Tests/AddJavaScriptAppTests.cs @@ -7,6 +7,7 @@ using Aspire.Hosting.ApplicationModel; using Aspire.Hosting.Utils; using Aspire.TestUtilities; +using Microsoft.Extensions.DependencyInjection; namespace Aspire.Hosting.JavaScript.Tests; @@ -123,6 +124,94 @@ public async Task VerifyPnpmDockerfile(bool hasLockFile) await Verify(dockerfileContents); } + [Fact] + public async Task PublishWithExistingDockerfileThrowsWhenRunScriptNameIsExplicit() + { + using var tempDir = new TestTempDirectory(); + using var builder = TestDistributedApplicationBuilder.Create(DistributedApplicationOperation.Publish, outputPath: tempDir.Path).WithResourceCleanUp(true); + + var appDir = CreateJavaScriptAppWithDockerfile(tempDir.Path); + var app = builder.AddJavaScriptApp("js", appDir, "migrate") + .WithBun(); + + var exception = await Assert.ThrowsAsync(() => ManifestUtils.GetManifest(app.Resource, tempDir.Path)); + + Assert.Contains("runScriptName", exception.Message); + Assert.Contains("WithRunScript", exception.Message); + Assert.Contains("Dockerfile", exception.Message); + } + + [Fact] + public async Task PublishModelWithExistingDockerfileThrowsWhenRunScriptNameIsExplicit() + { + using var tempDir = new TestTempDirectory(); + using var builder = TestDistributedApplicationBuilder.Create(DistributedApplicationOperation.Publish, outputPath: tempDir.Path).WithResourceCleanUp(true); + + var appDir = CreateJavaScriptAppWithDockerfile(tempDir.Path); + builder.AddJavaScriptApp("js", appDir, "migrate") + .WithBun(); + + using var app = builder.Build(); + var appModel = app.Services.GetRequiredService(); + var exception = await Assert.ThrowsAsync(() => ManifestUtils.GetManifestForModel(appModel, tempDir.Path)); + + Assert.Contains("runScriptName", exception.Message); + Assert.Contains("WithRunScript", exception.Message); + Assert.Contains("Dockerfile", exception.Message); + } + + [Fact] + public async Task PublishWithExistingDockerfileThrowsWhenWithRunScriptOverridesDefault() + { + using var tempDir = new TestTempDirectory(); + using var builder = TestDistributedApplicationBuilder.Create(DistributedApplicationOperation.Publish, outputPath: tempDir.Path).WithResourceCleanUp(true); + + var appDir = CreateJavaScriptAppWithDockerfile(tempDir.Path); + var app = builder.AddJavaScriptApp("js", appDir) + .WithBun() + .WithRunScript("migrate"); + + var exception = await Assert.ThrowsAsync(() => ManifestUtils.GetManifest(app.Resource, tempDir.Path)); + + Assert.Contains("runScriptName", exception.Message); + Assert.Contains("WithRunScript", exception.Message); + Assert.Contains("Dockerfile", exception.Message); + } + + [Fact] + public async Task PublishWithExistingDockerfileAllowsImplicitDefaultRunScript() + { + using var tempDir = new TestTempDirectory(); + using var builder = TestDistributedApplicationBuilder.Create(DistributedApplicationOperation.Publish, outputPath: tempDir.Path).WithResourceCleanUp(true); + + var appDir = CreateJavaScriptAppWithDockerfile(tempDir.Path); + var app = builder.AddJavaScriptApp("js", appDir) + .WithBun(); + + var manifest = await ManifestUtils.GetManifest(app.Resource, tempDir.Path); + + Assert.Equal("container.v1", manifest["type"]?.ToString()); + } + + [Fact] + public async Task PublishWithExistingDockerfileAllowsExplicitEntrypointOverride() + { + using var tempDir = new TestTempDirectory(); + using var builder = TestDistributedApplicationBuilder.Create(DistributedApplicationOperation.Publish, outputPath: tempDir.Path).WithResourceCleanUp(true); + + var appDir = CreateJavaScriptAppWithDockerfile(tempDir.Path); + var app = builder.AddJavaScriptApp("js", appDir, "migrate") + .WithBun() + .PublishAsDockerFile(container => container + .WithEntrypoint("bun") + .WithArgs("src/migrate.ts")); + + var manifest = await ManifestUtils.GetManifest(app.Resource, tempDir.Path); + + Assert.Equal("bun", manifest["entrypoint"]?.ToString()); + Assert.Contains("src/migrate.ts", manifest.ToJsonString()); + } + [Fact] [RequiresFeature(TestFeature.Docker | TestFeature.DockerPluginBuildx)] [OuterloopTest("Builds a Docker image to verify the generated pnpm Dockerfile works")] @@ -214,4 +303,28 @@ public async Task VerifyPnpmDockerfileBuildSucceeds() // Assert the build succeeded Assert.True(process.ExitCode == 0, $"Docker build failed with exit code {process.ExitCode}.\nStdout: {stdout}\nStderr: {stderr}"); } + + private static string CreateJavaScriptAppWithDockerfile(string rootDirectory) + { + var appDir = Path.Combine(rootDirectory, "js"); + Directory.CreateDirectory(appDir); + + var dockerfile = """ + FROM oven/bun:1 + WORKDIR /app + COPY . . + ENTRYPOINT ["bun","src/index.ts"] + """; + + File.WriteAllText(Path.Combine(appDir, "Dockerfile"), dockerfile); + File.WriteAllText(Path.Combine(appDir, "package.json"), """ + { + "scripts": { + "migrate": "bun src/migrate.ts" + } + } + """); + + return appDir; + } } From 4ca0c7a53c1bec01f0d52a95e739f334f14b7fc8 Mon Sep 17 00:00:00 2001 From: Sebastien Ros Date: Mon, 11 May 2026 17:44:06 -0700 Subject: [PATCH 2/5] Validate JavaScript Dockerfile run scripts in pipeline Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../JavaScriptHostingExtensions.cs | 26 +++++++++++++- .../AddJavaScriptAppTests.cs | 36 +++++++++++++++++-- 2 files changed, 58 insertions(+), 4 deletions(-) diff --git a/src/Aspire.Hosting.JavaScript/JavaScriptHostingExtensions.cs b/src/Aspire.Hosting.JavaScript/JavaScriptHostingExtensions.cs index 929ddab9517..3c65c86c409 100644 --- a/src/Aspire.Hosting.JavaScript/JavaScriptHostingExtensions.cs +++ b/src/Aspire.Hosting.JavaScript/JavaScriptHostingExtensions.cs @@ -31,6 +31,7 @@ public static class JavaScriptHostingExtensions private const string BrowserCapability = "browser"; private const string DefaultNodeVersion = "22"; private const string DefaultJavaScriptRunScriptName = "dev"; + private const string PublishManifestStepName = "publish-manifest"; private const string DefaultYarpImage = Yarp.YarpContainerImageTags.Registry + "/" + Yarp.YarpContainerImageTags.Image + ":" + Yarp.YarpContainerImageTags.Tag; // This is the order of config files that Vite will look for by default @@ -878,6 +879,8 @@ private static IResourceBuilder CreateDefaultJavaScriptAppBuilder(resource.Name, out var containerBuilder)) { + var validationStepName = $"validate-javascript-dockerfile-run-script-{resource.Name}"; + Task WriteValidatedContainerAsync(ManifestPublishingContext context) { ValidateExistingDockerfileRunScript(resource, containerBuilder.Resource, runScriptName); @@ -886,6 +889,27 @@ Task WriteValidatedContainerAsync(ManifestPublishingContext context) resourceBuilder.WithManifestPublishingCallback(WriteValidatedContainerAsync); containerBuilder.WithManifestPublishingCallback(WriteValidatedContainerAsync); + containerBuilder.WithAnnotation(new PipelineStepAnnotation(_ => new PipelineStep + { + Name = validationStepName, + Description = $"Validates that JavaScript app '{resource.Name}' does not publish an ignored run script with an existing Dockerfile.", + RequiredBySteps = [WellKnownPipelineSteps.Build, WellKnownPipelineSteps.Publish], + Resource = containerBuilder.Resource, + Action = _ => + { + ValidateExistingDockerfileRunScript(resource, containerBuilder.Resource, runScriptName); + return Task.CompletedTask; + } + })); + containerBuilder.WithAnnotation(new PipelineConfigurationAnnotation(context => + { + var validationStep = context.Steps.FirstOrDefault(s => string.Equals(s.Name, validationStepName, StringComparison.Ordinal)); + var publishManifestStep = context.Steps.FirstOrDefault(s => string.Equals(s.Name, PublishManifestStepName, StringComparison.Ordinal)); + if (validationStep is not null && publishManifestStep is not null) + { + validationStep.RequiredBy(publishManifestStep); + } + })); } resourceBuilder.WithVSCodeDebugging(); @@ -935,7 +959,7 @@ dockerfileBuildAnnotation.DockerfileFactory is not null || // their entrypoint with a package-manager script will work for the image shape. // If the user provides an explicit container entrypoint above, honor it; otherwise fail // instead of silently publishing an image that ignores the requested run script. - throw new InvalidOperationException( + throw new DistributedApplicationException( $"JavaScript app resource '{resource.Name}' is configured to run script '{runScript.ScriptName}', but publish is using the existing Dockerfile '{dockerfileBuildAnnotation.DockerfilePath}'. " + "An existing Dockerfile entrypoint cannot be changed automatically from runScriptName or WithRunScript. " + "Remove or rename the Dockerfile so Aspire can generate one, or call PublishAsDockerFile(...) and set the container entrypoint explicitly."); diff --git a/tests/Aspire.Hosting.JavaScript.Tests/AddJavaScriptAppTests.cs b/tests/Aspire.Hosting.JavaScript.Tests/AddJavaScriptAppTests.cs index 052f611090e..cfe98d3c223 100644 --- a/tests/Aspire.Hosting.JavaScript.Tests/AddJavaScriptAppTests.cs +++ b/tests/Aspire.Hosting.JavaScript.Tests/AddJavaScriptAppTests.cs @@ -2,12 +2,15 @@ // The .NET Foundation licenses this file to you under the MIT license. #pragma warning disable ASPIREJAVASCRIPT001 // Type is for evaluation purposes only +#pragma warning disable ASPIREPIPELINES001 // Type is for evaluation purposes only using System.Diagnostics; using Aspire.Hosting.ApplicationModel; +using Aspire.Hosting.Pipelines; using Aspire.Hosting.Utils; using Aspire.TestUtilities; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; namespace Aspire.Hosting.JavaScript.Tests; @@ -134,7 +137,7 @@ public async Task PublishWithExistingDockerfileThrowsWhenRunScriptNameIsExplicit var app = builder.AddJavaScriptApp("js", appDir, "migrate") .WithBun(); - var exception = await Assert.ThrowsAsync(() => ManifestUtils.GetManifest(app.Resource, tempDir.Path)); + var exception = await Assert.ThrowsAsync(() => ManifestUtils.GetManifest(app.Resource, tempDir.Path)); Assert.Contains("runScriptName", exception.Message); Assert.Contains("WithRunScript", exception.Message); @@ -153,7 +156,7 @@ public async Task PublishModelWithExistingDockerfileThrowsWhenRunScriptNameIsExp using var app = builder.Build(); var appModel = app.Services.GetRequiredService(); - var exception = await Assert.ThrowsAsync(() => ManifestUtils.GetManifestForModel(appModel, tempDir.Path)); + var exception = await Assert.ThrowsAsync(() => ManifestUtils.GetManifestForModel(appModel, tempDir.Path)); Assert.Contains("runScriptName", exception.Message); Assert.Contains("WithRunScript", exception.Message); @@ -171,7 +174,34 @@ public async Task PublishWithExistingDockerfileThrowsWhenWithRunScriptOverridesD .WithBun() .WithRunScript("migrate"); - var exception = await Assert.ThrowsAsync(() => ManifestUtils.GetManifest(app.Resource, tempDir.Path)); + var exception = await Assert.ThrowsAsync(() => ManifestUtils.GetManifest(app.Resource, tempDir.Path)); + + Assert.Contains("runScriptName", exception.Message); + Assert.Contains("WithRunScript", exception.Message); + Assert.Contains("Dockerfile", exception.Message); + } + + [Fact] + public async Task PublishPipelineWithExistingDockerfileThrowsFromValidationStepWhenRunScriptNameIsExplicit() + { + using var tempDir = new TestTempDirectory(); + using var builder = TestDistributedApplicationBuilder.Create(DistributedApplicationOperation.Publish, step: "validate-javascript-dockerfile-run-script-js").WithResourceCleanUp(true); + builder.Services.AddSingleton(); + + var appDir = CreateJavaScriptAppWithDockerfile(tempDir.Path); + builder.AddJavaScriptApp("js", appDir, "migrate") + .WithBun(); + + using var app = builder.Build(); + var pipeline = new DistributedApplicationPipeline(); + var context = new PipelineContext( + app.Services.GetRequiredService(), + app.Services.GetRequiredService(), + app.Services, + app.Services.GetRequiredService>(), + CancellationToken.None); + + var exception = await Assert.ThrowsAsync(() => pipeline.ExecuteAsync(context)); Assert.Contains("runScriptName", exception.Message); Assert.Contains("WithRunScript", exception.Message); From 404e13b2fe9d2bd00177316be94c61eeb269fe72 Mon Sep 17 00:00:00 2001 From: Sebastien Ros Date: Mon, 11 May 2026 18:39:08 -0700 Subject: [PATCH 3/5] Fix Dockerfile publish tests for run script validation Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Aspire.Hosting.Tests/PublishAsDockerfileTests.cs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/Aspire.Hosting.Tests/PublishAsDockerfileTests.cs b/tests/Aspire.Hosting.Tests/PublishAsDockerfileTests.cs index e7deb8d31c4..bb41a98c8f7 100644 --- a/tests/Aspire.Hosting.Tests/PublishAsDockerfileTests.cs +++ b/tests/Aspire.Hosting.Tests/PublishAsDockerfileTests.cs @@ -20,7 +20,7 @@ public async Task PublishAsDockerFileConfiguresManifestWithoutBuildArgs() var path = tempDir.Path; - var frontend = builder.AddJavaScriptApp("frontend", path, "watch") + var frontend = builder.AddJavaScriptApp("frontend", path) .PublishAsDockerFile(); // There should be an equivalent container resource with the same name @@ -59,7 +59,7 @@ public async Task PublishAsDockerFileConfiguresManifestWithBuildArgs() var path = tempDir.Path; #pragma warning disable CS0618 // Type or member is obsolete - var frontend = builder.AddJavaScriptApp("frontend", path, "watch") + var frontend = builder.AddJavaScriptApp("frontend", path) .PublishAsDockerFile(buildArgs: [ new DockerBuildArg("SOME_STRING", "Test"), new DockerBuildArg("SOME_BOOL", true), @@ -112,7 +112,7 @@ public async Task PublishAsDockerFileConfiguresManifestWithBuildArgsThatHaveNoVa var path = tempDir.Path; #pragma warning disable CS0618 // Type or member is obsolete - var frontend = builder.AddJavaScriptApp("frontend", path, "watch") + var frontend = builder.AddJavaScriptApp("frontend", path) .PublishAsDockerFile(buildArgs: [ new DockerBuildArg("SOME_ARG") ]); @@ -158,7 +158,7 @@ public async Task PublishAsDockerFileConfigureContainer() var secret = builder.AddParameter("secret", secret: true); - var frontend = builder.AddJavaScriptApp("frontend", path, "watch") + var frontend = builder.AddJavaScriptApp("frontend", path) .WithArgs("/usr/foo") .PublishAsDockerFile(c => { @@ -363,7 +363,7 @@ public void PublishAsDockerFile_CalledMultipleTimes_IsIdempotent() using var tempDir = CreateDirectoryWithDockerFile(); var path = tempDir.Path; - var frontend = builder.AddJavaScriptApp("frontend", path, "watch") + var frontend = builder.AddJavaScriptApp("frontend", path) .PublishAsDockerFile() .PublishAsDockerFile(); // Call again - should not throw @@ -381,7 +381,7 @@ public void PublishAsDockerFile_CalledMultipleTimesWithCallbacks_IsIdempotent() var path = tempDir.Path; var callbackCount = 0; - var frontend = builder.AddJavaScriptApp("frontend", path, "watch") + var frontend = builder.AddJavaScriptApp("frontend", path) .PublishAsDockerFile(c => { callbackCount++; From d182e0c9363bf1861fec5022964470f51474ee97 Mon Sep 17 00:00:00 2001 From: Sebastien Ros Date: Tue, 12 May 2026 14:34:19 -0700 Subject: [PATCH 4/5] Remove manifest step coupling from JS validation Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../JavaScriptHostingExtensions.cs | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/src/Aspire.Hosting.JavaScript/JavaScriptHostingExtensions.cs b/src/Aspire.Hosting.JavaScript/JavaScriptHostingExtensions.cs index 3c65c86c409..4cabba66161 100644 --- a/src/Aspire.Hosting.JavaScript/JavaScriptHostingExtensions.cs +++ b/src/Aspire.Hosting.JavaScript/JavaScriptHostingExtensions.cs @@ -31,7 +31,6 @@ public static class JavaScriptHostingExtensions private const string BrowserCapability = "browser"; private const string DefaultNodeVersion = "22"; private const string DefaultJavaScriptRunScriptName = "dev"; - private const string PublishManifestStepName = "publish-manifest"; private const string DefaultYarpImage = Yarp.YarpContainerImageTags.Registry + "/" + Yarp.YarpContainerImageTags.Image + ":" + Yarp.YarpContainerImageTags.Tag; // This is the order of config files that Vite will look for by default @@ -901,15 +900,6 @@ Task WriteValidatedContainerAsync(ManifestPublishingContext context) return Task.CompletedTask; } })); - containerBuilder.WithAnnotation(new PipelineConfigurationAnnotation(context => - { - var validationStep = context.Steps.FirstOrDefault(s => string.Equals(s.Name, validationStepName, StringComparison.Ordinal)); - var publishManifestStep = context.Steps.FirstOrDefault(s => string.Equals(s.Name, PublishManifestStepName, StringComparison.Ordinal)); - if (validationStep is not null && publishManifestStep is not null) - { - validationStep.RequiredBy(publishManifestStep); - } - })); } resourceBuilder.WithVSCodeDebugging(); From df796c4160da8095009aaa6bb612c0aeba5eca36 Mon Sep 17 00:00:00 2001 From: Mitch Denny Date: Tue, 19 May 2026 15:57:12 +1000 Subject: [PATCH 5/5] Refine JS Dockerfile run-script conflict detection - Detect 'explicit run script' by inspecting the last JavaScriptRunScriptAnnotation's ScriptName and Args rather than counting annotations. This avoids a false positive when the user re-states the default explicitly (e.g. .WithRunScript("dev")). - When the run-script args are the trigger for the conflict, include them in the thrown message so users can see why a default-named script still produced a conflict. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../JavaScriptHostingExtensions.cs | 27 ++++++++------ .../AddJavaScriptAppTests.cs | 36 +++++++++++++++++++ 2 files changed, 52 insertions(+), 11 deletions(-) diff --git a/src/Aspire.Hosting.JavaScript/JavaScriptHostingExtensions.cs b/src/Aspire.Hosting.JavaScript/JavaScriptHostingExtensions.cs index 4cabba66161..77dcb122142 100644 --- a/src/Aspire.Hosting.JavaScript/JavaScriptHostingExtensions.cs +++ b/src/Aspire.Hosting.JavaScript/JavaScriptHostingExtensions.cs @@ -882,7 +882,7 @@ private static IResourceBuilder CreateDefaultJavaScriptAppBuilder { - ValidateExistingDockerfileRunScript(resource, containerBuilder.Resource, runScriptName); + ValidateExistingDockerfileRunScript(resource, containerBuilder.Resource); return Task.CompletedTask; } })); @@ -921,7 +921,7 @@ Task WriteValidatedContainerAsync(ManifestPublishingContext context) return resourceBuilder; } - private static void ValidateExistingDockerfileRunScript(JavaScriptAppResource resource, ContainerResource containerResource, string initialRunScriptName) + private static void ValidateExistingDockerfileRunScript(JavaScriptAppResource resource, ContainerResource containerResource) { if (containerResource.Entrypoint is not null || !containerResource.TryGetLastAnnotation(out var dockerfileBuildAnnotation) || @@ -931,26 +931,31 @@ dockerfileBuildAnnotation.DockerfileFactory is not null || return; } - var hasMultipleRunScriptAnnotations = - containerResource.TryGetAnnotationsOfType(out var runScriptAnnotations) && - runScriptAnnotations.Skip(1).Any(); - + // The user's effective run-script intent is captured by the last annotation: AddJavaScriptApp + // always adds one with the supplied runScriptName, and any subsequent WithRunScript call + // appends another. Comparing the last annotation against the default avoids false positives + // when the user re-states the default explicitly (e.g. .WithRunScript("dev")). var hasExplicitRunScript = - !string.Equals(initialRunScriptName, DefaultJavaScriptRunScriptName, StringComparison.Ordinal) || - hasMultipleRunScriptAnnotations || - runScript.Args.Length > 0; + !string.Equals(runScript.ScriptName, DefaultJavaScriptRunScriptName, StringComparison.Ordinal) || + runScript.Args is { Length: > 0 }; if (!hasExplicitRunScript) { return; } + // Include the args in the message when they are the trigger, so the user can see why + // a default-named script (e.g. "dev") still produced a conflict. + var argsClause = runScript.Args is { Length: > 0 } + ? $" with args [{string.Join(", ", runScript.Args)}]" + : string.Empty; + // Existing Dockerfiles are user-authored, so Aspire cannot safely assume that replacing // their entrypoint with a package-manager script will work for the image shape. // If the user provides an explicit container entrypoint above, honor it; otherwise fail // instead of silently publishing an image that ignores the requested run script. throw new DistributedApplicationException( - $"JavaScript app resource '{resource.Name}' is configured to run script '{runScript.ScriptName}', but publish is using the existing Dockerfile '{dockerfileBuildAnnotation.DockerfilePath}'. " + + $"JavaScript app resource '{resource.Name}' is configured to run script '{runScript.ScriptName}'{argsClause}, but publish is using the existing Dockerfile '{dockerfileBuildAnnotation.DockerfilePath}'. " + "An existing Dockerfile entrypoint cannot be changed automatically from runScriptName or WithRunScript. " + "Remove or rename the Dockerfile so Aspire can generate one, or call PublishAsDockerFile(...) and set the container entrypoint explicitly."); } diff --git a/tests/Aspire.Hosting.JavaScript.Tests/AddJavaScriptAppTests.cs b/tests/Aspire.Hosting.JavaScript.Tests/AddJavaScriptAppTests.cs index cfe98d3c223..8f77fd6dfb1 100644 --- a/tests/Aspire.Hosting.JavaScript.Tests/AddJavaScriptAppTests.cs +++ b/tests/Aspire.Hosting.JavaScript.Tests/AddJavaScriptAppTests.cs @@ -242,6 +242,42 @@ public async Task PublishWithExistingDockerfileAllowsExplicitEntrypointOverride( Assert.Contains("src/migrate.ts", manifest.ToJsonString()); } + [Fact] + public async Task PublishWithExistingDockerfileAllowsWithRunScriptMatchingDefault() + { + using var tempDir = new TestTempDirectory(); + using var builder = TestDistributedApplicationBuilder.Create(DistributedApplicationOperation.Publish, outputPath: tempDir.Path).WithResourceCleanUp(true); + + var appDir = CreateJavaScriptAppWithDockerfile(tempDir.Path); + var app = builder.AddJavaScriptApp("js", appDir) + .WithBun() + // Re-stating the default script name explicitly should not be treated as a conflict + // with the existing Dockerfile, because the effective run script still matches the default. + .WithRunScript("dev"); + + var manifest = await ManifestUtils.GetManifest(app.Resource, tempDir.Path); + + Assert.Equal("container.v1", manifest["type"]?.ToString()); + } + + [Fact] + public async Task PublishWithExistingDockerfileThrowsAndIncludesArgsWhenDefaultScriptHasArgs() + { + using var tempDir = new TestTempDirectory(); + using var builder = TestDistributedApplicationBuilder.Create(DistributedApplicationOperation.Publish, outputPath: tempDir.Path).WithResourceCleanUp(true); + + var appDir = CreateJavaScriptAppWithDockerfile(tempDir.Path); + var app = builder.AddJavaScriptApp("js", appDir) + .WithBun() + .WithRunScript("dev", ["--port", "8080"]); + + var exception = await Assert.ThrowsAsync(() => ManifestUtils.GetManifest(app.Resource, tempDir.Path)); + + Assert.Contains("run script 'dev'", exception.Message); + Assert.Contains("with args [--port, 8080]", exception.Message); + Assert.Contains("Dockerfile", exception.Message); + } + [Fact] [RequiresFeature(TestFeature.Docker | TestFeature.DockerPluginBuildx)] [OuterloopTest("Builds a Docker image to verify the generated pnpm Dockerfile works")]