Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
70 changes: 69 additions & 1 deletion src/Aspire.Hosting.JavaScript/JavaScriptHostingExtensions.cs
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,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
Expand Down Expand Up @@ -329,7 +331,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 +875,33 @@ 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);
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);
return Task.CompletedTask;
}
}));
}

resourceBuilder.WithVSCodeDebugging();

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

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

// 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(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}'{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.");
Copy link
Copy Markdown
Member

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
179 changes: 179 additions & 0 deletions tests/Aspire.Hosting.JavaScript.Tests/AddJavaScriptAppTests.cs
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,157 @@ 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]
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<DistributedApplicationException>(() => 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]
public async Task VerifyPnpmDockerfileCopiesWorkspaceFileBeforeInstall()
{
Expand Down Expand Up @@ -245,4 +400,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;
}
}
12 changes: 6 additions & 6 deletions tests/Aspire.Hosting.Tests/PublishAsDockerfileTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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")
]);
Expand Down Expand Up @@ -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 =>
{
Expand Down Expand Up @@ -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

Expand All @@ -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++;
Expand Down
Loading