diff --git a/src/Aspire.Hosting.JavaScript/JavaScriptHostingExtensions.cs b/src/Aspire.Hosting.JavaScript/JavaScriptHostingExtensions.cs index eba856c25c5..4c0c0f11c12 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,33 @@ 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); + 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 @@ -892,6 +921,45 @@ private static IResourceBuilder CreateDefaultJavaScriptAppBuilder(out var dockerfileBuildAnnotation) || + dockerfileBuildAnnotation.DockerfileFactory is not null || + !containerResource.TryGetLastAnnotation(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."); + } + /// /// 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 2cfab7d71f5..5a9333f4b0d 100644 --- a/tests/Aspire.Hosting.JavaScript.Tests/AddJavaScriptAppTests.cs +++ b/tests/Aspire.Hosting.JavaScript.Tests/AddJavaScriptAppTests.cs @@ -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; @@ -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(() => 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 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); + 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(() => 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() { @@ -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; + } } 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++;