Skip to content

Fail on JavaScript run script Dockerfile conflict #16969

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mitchdenny merged 7 commits into from
May 19, 2026
+254 −7
Merged

Fail on JavaScript run script Dockerfile conflict #16969

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
e9dca1a
Fail when JavaScript run script conflicts with Dockerfile
sebastienros May 12, 2026
4ca0c7a
Validate JavaScript Dockerfile run scripts in pipeline
sebastienros May 12, 2026
404e13b
Fix Dockerfile publish tests for run script validation
sebastienros May 12, 2026
02cec45
Merge branch 'main' into sebastienros/sebros-fix-javascriptapp-entryp…
sebastienros May 12, 2026
d182e0c
Remove manifest step coupling from JS validation
sebastienros May 12, 2026
df796c4
Refine JS Dockerfile run-script conflict detection
May 19, 2026
64ea719
Merge remote-tracking branch 'origin/main' into sebastienros/sebros-f…
May 19, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 74 additions & 1 deletion src/Aspire.Hosting.JavaScript/JavaScriptHostingExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -29,6 +30,8 @@ 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";
Copy link
Copy Markdown
Contributor

@davidfowl davidfowl May 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before start? @eerhardt ?

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
Expand Down Expand Up @@ -329,7 +332,7 @@ private static IResourceBuilder<TResource> WithNodeDefaults<TResource>(this IRes
/// integration.
/// </remarks>
[AspireExport(Description = "Adds a JavaScript application resource")]
public static IResourceBuilder<JavaScriptAppResource> AddJavaScriptApp(this IDistributedApplicationBuilder builder, [ResourceName] string name, string appDirectory, string runScriptName = "dev")
public static IResourceBuilder<JavaScriptAppResource> AddJavaScriptApp(this IDistributedApplicationBuilder builder, [ResourceName] string name, string appDirectory, string runScriptName = DefaultJavaScriptRunScriptName)
{
ArgumentNullException.ThrowIfNull(builder);
ArgumentException.ThrowIfNullOrEmpty(name);
Expand Down Expand Up @@ -873,6 +876,42 @@ private static IResourceBuilder<TResource> CreateDefaultJavaScriptAppBuilder<TRe
.WithBuildScript("build")
.WithRunScript(runScriptName);

if (builder.ExecutionContext.IsPublishMode &&
builder.TryCreateResourceBuilder<ContainerResource>(resource.Name, out var containerBuilder))
{
var validationStepName = $"validate-javascript-dockerfile-run-script-{resource.Name}";

Task WriteValidatedContainerAsync(ManifestPublishingContext context)
{
ValidateExistingDockerfileRunScript(resource, containerBuilder.Resource, runScriptName);
return context.WriteContainerAsync(containerBuilder.Resource);
}

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();

// ensure the package manager command is set before starting the resource
Expand All @@ -892,6 +931,40 @@ private static IResourceBuilder<TResource> CreateDefaultJavaScriptAppBuilder<TRe
return resourceBuilder;
}

private static void ValidateExistingDockerfileRunScript(JavaScriptAppResource resource, ContainerResource containerResource, string initialRunScriptName)
{
if (containerResource.Entrypoint is not null ||
!containerResource.TryGetLastAnnotation<DockerfileBuildAnnotation>(out var dockerfileBuildAnnotation) ||
dockerfileBuildAnnotation.DockerfileFactory is not null ||
!containerResource.TryGetLastAnnotation<JavaScriptRunScriptAnnotation>(out var runScript))
{
return;
}

var hasMultipleRunScriptAnnotations =
containerResource.TryGetAnnotationsOfType<JavaScriptRunScriptAnnotation>(out var runScriptAnnotations) &&
runScriptAnnotations.Skip(1).Any();

var hasExplicitRunScript =
!string.Equals(initialRunScriptName, DefaultJavaScriptRunScriptName, StringComparison.Ordinal) ||
hasMultipleRunScriptAnnotations ||
runScript.Args.Length > 0;
Copy link
Copy Markdown
Member

@mitchdenny mitchdenny May 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hasMultipleRunScriptAnnotations is true whenever WithRunScript(...) is called after AddJavaScriptApp, because CreateDefaultJavaScriptAppBuilder itself already adds one annotation at line 876. As a result, this code throws even when the user's explicit WithRunScript matches the default:

builder.AddJavaScriptApp("js", appDir)
       .WithBun()
       .WithRunScript("dev"); // same as the default

Repro (verified against this PR build with dotnet run -- --publisher manifest): publish fails with JavaScript app resource 'js' is configured to run script 'dev', but publish is using the existing Dockerfile .... The user explicitly affirmed the default value, so the failure is surprising.

Suggested fix: compare the last JavaScriptRunScriptAnnotation's ScriptName (and Args) against the default instead of treating annotation count alone as a proxy for "explicit".


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 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.");
Copy link
Copy Markdown
Member

@mitchdenny mitchdenny May 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hasExplicitRunScript can be set purely by runScript.Args.Length > 0, even when runScript.ScriptName equals the default "dev". But the thrown message only mentions runScriptName and WithRunScript (the script name), not the args. So this:

builder.AddJavaScriptApp("js", appDir) // default script: "dev"
       .WithBun()
       .WithRunScript("dev", new[] { "--port", "8080" });

fails with ... is configured to run script 'dev', but publish is using the existing Dockerfile ... — no mention of the args, which are the actual reason for the failure. Verified locally: the message produced for the args-only case is identical to the case where the user passed a non-default script name.

Suggested fix: when the trigger is runScript.Args.Length > 0, include the args in the message (e.g. ... is configured to run script 'dev' with args [--port, 8080], ...).

}

/// <summary>
/// Adds a Vite app to the distributed application builder.
/// </summary>
Expand Down
143 changes: 143 additions & 0 deletions tests/Aspire.Hosting.JavaScript.Tests/AddJavaScriptAppTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +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;

Expand Down Expand Up @@ -123,6 +127,121 @@ 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<DistributedApplicationException>(() => 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<DistributedApplicationModel>();
var exception = await Assert.ThrowsAsync<DistributedApplicationException>(() => 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<DistributedApplicationException>(() => 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<IPipelineActivityReporter, NullPublishingActivityReporter>();

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<DistributedApplicationModel>(),
app.Services.GetRequiredService<DistributedApplicationExecutionContext>(),
app.Services,
app.Services.GetRequiredService<ILogger<AddJavaScriptAppTests>>(),
CancellationToken.None);

var exception = await Assert.ThrowsAsync<DistributedApplicationException>(() => pipeline.ExecuteAsync(context));

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")]
Expand Down Expand Up @@ -214,4 +333,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;
}
}
Loading