Skip to content

Commit ac8add0

Browse files
committed
Move resource name validation to AddResource/Build with SuppressNameValidationAnnotation
Resource names have a 64-character limit enforced by ModelName.ValidateName. Internal resources like ProjectRebuilderResource, JavaScriptInstallerResource, PythonInstallerResource, and PythonVenvCreatorResource append suffixes to user-provided resource names, which can push them over the limit. Previously, validation happened in the Resource constructor, requiring internal skip-validation constructors that couldn't be accessed from external assemblies. This change moves validation from the Resource constructor to DistributedApplicationBuilder.AddResource() and Build(), and introduces a public SuppressNameValidationAnnotation that any resource can use to opt out of name validation before being added to the app model.
1 parent 760514e commit ac8add0

File tree

10 files changed

+164
-2
lines changed

10 files changed

+164
-2
lines changed

src/Aspire.Hosting.JavaScript/JavaScriptHostingExtensions.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1134,6 +1134,7 @@ private static void AddInstaller<TResource>(IResourceBuilder<TResource> resource
11341134
}
11351135

11361136
var installer = new JavaScriptInstallerResource(installerName, resource.Resource.WorkingDirectory);
1137+
installer.Annotations.Add(new SuppressNameValidationAnnotation());
11371138
var installerBuilder = resource.ApplicationBuilder.AddResource(installer)
11381139
.WithParentRelationship(resource.Resource)
11391140
.ExcludeFromManifest()

src/Aspire.Hosting.Python/PythonAppResourceBuilderExtensions.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1320,6 +1320,7 @@ private static void AddInstaller<T>(IResourceBuilder<T> builder, bool install) w
13201320
}
13211321

13221322
var installer = new PythonInstallerResource(installerName, builder.Resource);
1323+
installer.Annotations.Add(new SuppressNameValidationAnnotation());
13231324
var installerBuilder = builder.ApplicationBuilder.AddResource(installer)
13241325
.WithParentRelationship(builder.Resource)
13251326
.ExcludeFromManifest()
@@ -1393,6 +1394,7 @@ private static void CreateVenvCreatorIfNeeded<T>(IResourceBuilder<T> builder) wh
13931394

13941395
// Create new venv creator resource
13951396
var venvCreator = new PythonVenvCreatorResource(venvCreatorName, builder.Resource, venvPath);
1397+
venvCreator.Annotations.Add(new SuppressNameValidationAnnotation());
13961398

13971399
// Determine which Python command to use
13981400
string pythonCommand;

src/Aspire.Hosting/ApplicationModel/Resource.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ public abstract class Resource : IResource
2727
/// <param name="name">The name of the resource.</param>
2828
protected Resource(string name)
2929
{
30-
ModelName.ValidateName(nameof(Resource), name);
30+
ArgumentNullException.ThrowIfNull(name);
3131

3232
Name = name;
3333
}
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
using System.Diagnostics;
5+
6+
namespace Aspire.Hosting.ApplicationModel;
7+
8+
/// <summary>
9+
/// Represents an annotation that indicates the resource name should not be validated against naming rules when
10+
/// the resource is added to the application model.
11+
/// </summary>
12+
/// <remarks>
13+
/// This is useful for internal resources (such as installers or rebuilders) that append suffixes to user-provided
14+
/// resource names and may exceed the 64-character name limit, but are never deployed.
15+
/// </remarks>
16+
[DebuggerDisplay("Type = {GetType().Name,nq}")]
17+
public sealed class SuppressNameValidationAnnotation : IResourceAnnotation
18+
{
19+
}

src/Aspire.Hosting/DistributedApplicationBuilder.cs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -735,6 +735,15 @@ public DistributedApplication Build()
735735
throw new DistributedApplicationException($"Multiple resources with the name '{duplicateResourceName}'. Resource names are case-insensitive.");
736736
}
737737

738+
// Validate resource names. Resources added directly to the collection bypass AddResource validation.
739+
foreach (var resource in Resources)
740+
{
741+
if (!resource.HasAnnotationOfType<SuppressNameValidationAnnotation>())
742+
{
743+
ModelName.ValidateName(nameof(Resource), resource.Name);
744+
}
745+
}
746+
738747
var application = new DistributedApplication(_innerBuilder.Build());
739748

740749
_executionContextOptions.ServiceProvider = application.Services.GetRequiredService<IServiceProvider>();
@@ -753,6 +762,11 @@ public IResourceBuilder<T> AddResource<T>(T resource) where T : IResource
753762
{
754763
ArgumentNullException.ThrowIfNull(resource);
755764

765+
if (!resource.HasAnnotationOfType<SuppressNameValidationAnnotation>())
766+
{
767+
ModelName.ValidateName(nameof(Resource), resource.Name);
768+
}
769+
756770
if (Resources.FirstOrDefault(r => string.Equals(r.Name, resource.Name, StringComparisons.ResourceName)) is { } existingResource)
757771
{
758772
throw new DistributedApplicationException($"Cannot add resource of type '{resource.GetType()}' with name '{resource.Name}' because resource of type '{existingResource.GetType()}' with that name already exists. Resource names are case-insensitive.");

src/Aspire.Hosting/ProjectResourceBuilderExtensions.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -921,6 +921,7 @@ private static void AddRebuilderResource<TProjectResource>(IResourceBuilder<TPro
921921

922922
var rebuilderName = $"{projectResource.Name}-rebuilder";
923923
var rebuilder = new ProjectRebuilderResource(rebuilderName, projectResource, projectMetadata.ProjectPath);
924+
rebuilder.Annotations.Add(new SuppressNameValidationAnnotation());
924925
var rebuilderBuilder = builder.ApplicationBuilder.AddResource(rebuilder);
925926

926927
rebuilderBuilder

tests/Aspire.Hosting.JavaScript.Tests/PackageInstallationTests.cs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -561,6 +561,21 @@ public void WithBun_DefaultsArgsInPublishMode()
561561
Assert.Equal(["install", "--frozen-lockfile"], installCommand.Args);
562562
}
563563

564+
[Fact]
565+
public void InstallerResourceHasSuppressNameValidationAnnotation()
566+
{
567+
var builder = DistributedApplication.CreateBuilder();
568+
569+
builder.AddJavaScriptApp("nodeApp", "./test-app")
570+
.WithNpm(install: true);
571+
572+
using var app = builder.Build();
573+
574+
var appModel = app.Services.GetRequiredService<DistributedApplicationModel>();
575+
var installerResource = Assert.Single(appModel.Resources.OfType<JavaScriptInstallerResource>());
576+
Assert.True(installerResource.TryGetLastAnnotation<SuppressNameValidationAnnotation>(out _));
577+
}
578+
564579
[UnsafeAccessor(UnsafeAccessorKind.Method, Name = "ExecuteBeforeStartHooksAsync")]
565580
private static extern Task ExecuteBeforeStartHooksAsync(DistributedApplication app, CancellationToken cancellationToken);
566581
}

tests/Aspire.Hosting.Python.Tests/AddPythonAppTests.cs

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2390,6 +2390,49 @@ public async Task WithUv_InstallFalse_CreatesInstallerWithExplicitStart()
23902390
Assert.Empty(waitAnnotations);
23912391
}
23922392

2393+
[Fact]
2394+
public void InstallerResourceHasSuppressNameValidationAnnotation()
2395+
{
2396+
using var builder = TestDistributedApplicationBuilder.Create().WithTestAndResourceLogging(outputHelper);
2397+
using var tempDir = new TestTempDirectory();
2398+
2399+
var scriptName = "main.py";
2400+
2401+
builder.AddPythonApp("pythonProject", tempDir.Path, scriptName)
2402+
.WithPip();
2403+
2404+
var app = builder.Build();
2405+
var appModel = app.Services.GetRequiredService<DistributedApplicationModel>();
2406+
2407+
var installerResource = appModel.Resources.OfType<PythonInstallerResource>().Single();
2408+
Assert.True(installerResource.HasAnnotationOfType<SuppressNameValidationAnnotation>());
2409+
}
2410+
2411+
[Fact]
2412+
public void VenvCreatorResourceHasSuppressNameValidationAnnotation()
2413+
{
2414+
using var builder = TestDistributedApplicationBuilder.Create().WithTestAndResourceLogging(outputHelper);
2415+
using var tempDir = new TestTempDirectory();
2416+
using var tempVenvDir = new TestTempDirectory();
2417+
2418+
var scriptName = "main.py";
2419+
var scriptPath = Path.Combine(tempDir.Path, scriptName);
2420+
File.WriteAllText(scriptPath, "print('Hello')");
2421+
2422+
var requirementsPath = Path.Combine(tempDir.Path, "requirements.txt");
2423+
File.WriteAllText(requirementsPath, "requests");
2424+
2425+
builder.AddPythonApp("pythonProject", tempDir.Path, scriptName)
2426+
.WithVirtualEnvironment(tempVenvDir.Path, createIfNotExists: true);
2427+
2428+
var app = builder.Build();
2429+
var appModel = app.Services.GetRequiredService<DistributedApplicationModel>();
2430+
2431+
var venvCreatorResource = appModel.Resources.OfType<PythonVenvCreatorResource>().SingleOrDefault();
2432+
Assert.NotNull(venvCreatorResource);
2433+
Assert.True(venvCreatorResource.HasAnnotationOfType<SuppressNameValidationAnnotation>());
2434+
}
2435+
23932436
/// <summary>
23942437
/// Helper method to manually trigger BeforeStartEvent for tests.
23952438
/// This is needed because BeforeStartEvent is normally triggered during StartAsync(),

tests/Aspire.Hosting.Tests/DistributedApplicationBuilderTests.cs

Lines changed: 54 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -347,10 +347,63 @@ public void LegacyShaUsesProjectNameShaInPublishMode()
347347
Assert.Equal(projectNameSha, legacySha);
348348
}
349349

350+
[Fact]
351+
public void AddResource_InvalidName_Error()
352+
{
353+
var appBuilder = DistributedApplication.CreateBuilder();
354+
355+
var longName = new string('a', 65);
356+
var resource = new ContainerResource(longName);
357+
358+
var ex = Assert.Throws<ArgumentException>(() => appBuilder.AddResource(resource));
359+
Assert.Equal($"Resource name '{longName}' is invalid. Name must be between 1 and 64 characters long. (Parameter 'name')", ex.Message);
360+
}
361+
362+
[Fact]
363+
public void AddResource_InvalidNameWithExcludeAnnotation_Success()
364+
{
365+
var appBuilder = DistributedApplication.CreateBuilder();
366+
367+
var longName = new string('a', 65);
368+
var resource = new ContainerResource(longName);
369+
resource.Annotations.Add(new SuppressNameValidationAnnotation());
370+
371+
appBuilder.AddResource(resource);
372+
373+
Assert.Contains(appBuilder.Resources, r => r.Name == longName);
374+
}
375+
376+
[Fact]
377+
public void Build_InvalidName_Error()
378+
{
379+
var appBuilder = DistributedApplication.CreateBuilder();
380+
381+
var longName = new string('a', 65);
382+
appBuilder.Resources.Add(new ContainerResource(longName));
383+
384+
var ex = Assert.Throws<ArgumentException>(appBuilder.Build);
385+
Assert.Equal($"Resource name '{longName}' is invalid. Name must be between 1 and 64 characters long. (Parameter 'name')", ex.Message);
386+
}
387+
388+
[Fact]
389+
public void Build_InvalidNameWithExcludeAnnotation_Success()
390+
{
391+
var appBuilder = DistributedApplication.CreateBuilder();
392+
393+
var longName = new string('a', 65);
394+
var resource = new ContainerResource(longName);
395+
resource.Annotations.Add(new SuppressNameValidationAnnotation());
396+
appBuilder.Resources.Add(resource);
397+
398+
var app = appBuilder.Build();
399+
400+
Assert.NotNull(app);
401+
}
402+
350403
private sealed class TestResource : IResource
351404
{
352405
public string Name => nameof(TestResource);
353406

354-
public ResourceAnnotationCollection Annotations => throw new NotImplementedException();
407+
public ResourceAnnotationCollection Annotations { get; } = new();
355408
}
356409
}

tests/Aspire.Hosting.Tests/ProjectResourceTests.cs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -932,6 +932,20 @@ public void AddCSharpAppAddsSupportsDebuggingAnnotationInRunMode()
932932
Assert.Equal("project", annotation.LaunchConfigurationType);
933933
}
934934

935+
[Fact]
936+
public void AddProjectCreatesRebuilderWithSuppressNameValidationAnnotation()
937+
{
938+
using var builder = TestDistributedApplicationBuilder.Create(DistributedApplicationOperation.Run);
939+
builder.AddProject<TestProject>("projectName", options => { options.ExcludeLaunchProfile = true; });
940+
941+
var app = builder.Build();
942+
var appModel = app.Services.GetRequiredService<DistributedApplicationModel>();
943+
944+
var rebuilder = appModel.Resources.OfType<ProjectRebuilderResource>().SingleOrDefault();
945+
Assert.NotNull(rebuilder);
946+
Assert.True(rebuilder.HasAnnotationOfType<SuppressNameValidationAnnotation>());
947+
}
948+
935949
internal static IDistributedApplicationBuilder CreateBuilder(string[]? args = null, DistributedApplicationOperation operation = DistributedApplicationOperation.Publish)
936950
{
937951
var resolvedArgs = new List<string>();

0 commit comments

Comments
 (0)