Skip to content

Link resources in more cases #8425

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
merged 3 commits into from
Mar 31, 2025
Merged

Link resources in more cases #8425

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
39d7f77
Link resources in more cases
davidfowl Mar 30, 2025
d16515b
Remove unnecessary blank lines in WithReferenceTests.cs
davidfowl Mar 30, 2025
2cbebf2
Refactor resource linking logic to avoid duplicate references and imp…
davidfowl Mar 31, 2025
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
17 changes: 17 additions & 0 deletions src/Aspire.Hosting.Azure/AzureBicepResourceExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,8 @@ public static BicepSecretOutputReference GetSecretOutput(this IResourceBuilder<A
public static IResourceBuilder<T> WithEnvironment<T>(this IResourceBuilder<T> builder, string name, BicepOutputReference bicepOutputReference)
where T : IResourceWithEnvironment
{
builder.WithResourceRelationship(bicepOutputReference.Resource);

return builder.WithEnvironment(ctx =>
{
ctx.EnvironmentVariables[name] = bicepOutputReference;
Expand Down Expand Up @@ -205,6 +207,9 @@ public static IResourceBuilder<T> WithParameter<T>(this IResourceBuilder<T> buil
where T : AzureBicepResource
{
BicepIdentifierHelpers.ThrowIfInvalid(name);

builder.WithResourceRelationship(value);

builder.Resource.Parameters[name] = value;
return builder;
}
Expand All @@ -221,6 +226,9 @@ public static IResourceBuilder<T> WithParameter<T>(this IResourceBuilder<T> buil
where T : AzureBicepResource
{
BicepIdentifierHelpers.ThrowIfInvalid(name);

builder.WithResourceRelationship(value.Resource);

builder.Resource.Parameters[name] = value.Resource;
return builder;
}
Expand All @@ -237,6 +245,9 @@ public static IResourceBuilder<T> WithParameter<T>(this IResourceBuilder<T> buil
where T : AzureBicepResource
{
BicepIdentifierHelpers.ThrowIfInvalid(name);

builder.WithResourceRelationship(value.Resource);

builder.Resource.Parameters[name] = value;
return builder;
}
Expand All @@ -253,6 +264,9 @@ public static IResourceBuilder<T> WithParameter<T>(this IResourceBuilder<T> buil
where T : AzureBicepResource
{
BicepIdentifierHelpers.ThrowIfInvalid(name);

builder.WithResourceRelationship(value);

builder.Resource.Parameters[name] = value;
return builder;
}
Expand All @@ -269,6 +283,9 @@ public static IResourceBuilder<T> WithParameter<T>(this IResourceBuilder<T> buil
where T : AzureBicepResource
{
BicepIdentifierHelpers.ThrowIfInvalid(name);

builder.WithResourceRelationship(value.Resource);

builder.Resource.Parameters[name] = value;
return builder;
}
Expand Down
1 change: 1 addition & 0 deletions src/Aspire.Hosting/ConnectionStringBuilderExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ public static IResourceBuilder<ConnectionStringResource> AddConnectionString(thi
{
var cs = new ConnectionStringResource(name, connectionStringExpression);
return builder.AddResource(cs)
.WithResourceRelationship(connectionStringExpression)
.WithInitialState(new CustomResourceSnapshot
{
ResourceType = "ConnectionString",
Expand Down
99 changes: 97 additions & 2 deletions src/Aspire.Hosting/ResourceBuilderExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ public static IResourceBuilder<T> WithEnvironment<T>(this IResourceBuilder<T> bu

var expression = value.GetExpression();

builder.WithResourceRelationship(expression);

return builder.WithEnvironment(context =>
{
context.EnvironmentVariables[name] = expression;
Expand All @@ -73,6 +75,8 @@ public static IResourceBuilder<T> WithEnvironment<T>(this IResourceBuilder<T> bu
ArgumentNullException.ThrowIfNull(name);
ArgumentNullException.ThrowIfNull(value);

builder.WithResourceRelationship(value);

return builder.WithEnvironment(context =>
{
context.EnvironmentVariables[name] = value;
Expand Down Expand Up @@ -140,6 +144,8 @@ public static IResourceBuilder<T> WithEnvironment<T>(this IResourceBuilder<T> bu
ArgumentNullException.ThrowIfNull(name);
ArgumentNullException.ThrowIfNull(endpointReference);

builder.WithResourceRelationship(endpointReference.Resource);

return builder.WithEnvironment(context =>
{
context.EnvironmentVariables[name] = endpointReference;
Expand All @@ -160,6 +166,8 @@ public static IResourceBuilder<T> WithEnvironment<T>(this IResourceBuilder<T> bu
ArgumentNullException.ThrowIfNull(name);
ArgumentNullException.ThrowIfNull(parameter);

builder.WithResourceRelationship(parameter.Resource);

return builder.WithEnvironment(context =>
{
context.EnvironmentVariables[name] = parameter.Resource;
Expand All @@ -184,6 +192,8 @@ public static IResourceBuilder<T> WithEnvironment<T>(
ArgumentNullException.ThrowIfNull(envVarName);
ArgumentNullException.ThrowIfNull(resource);

builder.WithResourceRelationship(resource.Resource);

return builder.WithEnvironment(context =>
{
context.EnvironmentVariables[envVarName] = new ConnectionStringReference(resource.Resource, optional: false);
Expand Down Expand Up @@ -217,6 +227,8 @@ public static IResourceBuilder<T> WithArgs<T>(this IResourceBuilder<T> builder,
ArgumentNullException.ThrowIfNull(builder);
ArgumentNullException.ThrowIfNull(args);

WalkAndLinkResourceReferences(builder, args);

return builder.WithArgs(context => context.Args.AddRange(args));
}

Expand Down Expand Up @@ -351,7 +363,7 @@ public static IResourceBuilder<TDestination> WithReference<TDestination>(this IR
var resource = source.Resource;
connectionName ??= resource.Name;

builder.WithRelationship(resource, KnownRelationshipTypes.Reference);
builder.WithResourceRelationship(resource);

return builder.WithEnvironment(context =>
{
Expand Down Expand Up @@ -457,7 +469,7 @@ private static void ApplyEndpoints<T>(this IResourceBuilder<T> builder, IResourc
endpointReferenceAnnotation.EndpointNames.Add(endpointName);
}

builder.WithRelationship(resourceWithEndpoints, KnownRelationshipTypes.Reference);
builder.WithResourceRelationship(resourceWithEndpoints);
}

/// <summary>
Expand Down Expand Up @@ -1660,6 +1672,89 @@ public static IResourceBuilder<T> WithRelationship<T>(
return builder.WithAnnotation(new ResourceRelationshipAnnotation(resource, type));
}

/// <summary>
/// Adds a <see cref="ResourceRelationshipAnnotation"/> to the resource annotations to add a reference to another resource.
/// </summary>
/// <typeparam name="T">The type of the resource.</typeparam>
/// <param name="builder">The resource builder.</param>
/// <param name="resource">The resource that the relationship is to.</param>
/// <returns>A resource builder.</returns>
public static IResourceBuilder<T> WithResourceRelationship<T>(
Copy link
Member

@JamesNK JamesNK Mar 31, 2025

Choose a reason for hiding this comment

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

What is the difference between this and WithRelationship? Just hardcoding a relationship type of Reference?

ResourceRelationship is a generic name that doesn't mean anything. Aren't all relationships between resources resource relationships?

Shouldn't this be called WithReferenceRelationship?

Copy link
Member Author

@davidfowl davidfowl Mar 31, 2025

Choose a reason for hiding this comment

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

I'm not hung up on the naming, and yes it's to avoid the magic "Reference" string.

this IResourceBuilder<T> builder,
IResource resource) where T : IResource
{
ArgumentNullException.ThrowIfNull(builder);
ArgumentNullException.ThrowIfNull(resource);

return builder.WithAnnotation(new ResourceRelationshipAnnotation(resource, KnownRelationshipTypes.Reference));
}

/// <summary>
/// Walks the reference expression and adds <see cref="ResourceRelationshipAnnotation"/>s for all resources found in the expression.
/// </summary>
/// <typeparam name="T">The type of the resource.</typeparam>
/// <param name="builder">The resource builder.</param>
/// <param name="expression">The reference expression.</param>
/// <returns>A resource builder.</returns>
public static IResourceBuilder<T> WithResourceRelationship<T>(
this IResourceBuilder<T> builder,
ReferenceExpression expression) where T : IResource
{
ArgumentNullException.ThrowIfNull(builder);
ArgumentNullException.ThrowIfNull(expression);

WalkAndLinkResourceReferences(builder, expression.ValueProviders);

return builder;
}

private static void WalkAndLinkResourceReferences<T>(IResourceBuilder<T> builder, IEnumerable<object> values)
where T : IResource
{
foreach (var value in values)
{
if (value is IResource resource)
{
builder.WithResourceRelationship(resource);
}
else if (value is IResourceBuilder<IResource> resourceBuilder)
{
builder.WithResourceRelationship(resourceBuilder);
}
else if (value is IValueWithReferences valueWithReferences)
{
foreach (var reference in valueWithReferences.References)
{
if (reference is IResource resourceReference)
{
builder.WithResourceRelationship(resourceReference);
}
else if (reference is IResourceBuilder<IResource> resourceBuilderReference)
{
builder.WithResourceRelationship(resourceBuilderReference);
}
}
}
}
}

/// <summary>
/// Adds a <see cref="ResourceRelationshipAnnotation"/> to the resource annotations to add a reference to another resource.
/// </summary>
/// <typeparam name="T">The type of the resource.</typeparam>
/// <param name="builder">The resource builder.</param>
/// <param name="resourceBuilder">The resource builder that the relationship is to.</param>
/// <returns>A resource builder.</returns>
public static IResourceBuilder<T> WithResourceRelationship<T>(
this IResourceBuilder<T> builder,
IResourceBuilder<IResource> resourceBuilder) where T : IResource
{
ArgumentNullException.ThrowIfNull(builder);
ArgumentNullException.ThrowIfNull(resourceBuilder);

return builder.WithAnnotation(new ResourceRelationshipAnnotation(resourceBuilder.Resource, KnownRelationshipTypes.Reference));
}

/// <summary>
/// Adds a <see cref="ResourceRelationshipAnnotation"/> to the resource annotations to add a parent-child relationship.
/// </summary>
Expand Down
20 changes: 20 additions & 0 deletions tests/Aspire.Hosting.Azure.Tests/AzureBicepProvisionerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,26 @@ public async Task SetParametersTranslatesCompatibleParameterTypes()
Assert.Equal("paramValue", parameters["param"]?["value"]?.ToString());
Assert.Equal("paramValue/1", parameters["expr"]?["value"]?.ToString());
Assert.Equal("http://localhost:1023", parameters["endpoint"]?["value"]?.ToString());

// We don't yet process relationships set via the callbacks
// so we don't see the testResource2 nor exe1
Assert.True(bicep0.Resource.TryGetAnnotationsOfType<ResourceRelationshipAnnotation>(out var relationships));
Assert.Collection(relationships.DistinctBy(r => (r.Resource, r.Type)),
r =>
{
Assert.Equal("Reference", r.Type);
Assert.Same(connectionStringResource.Resource, r.Resource);
},
r =>
{
Assert.Equal("Reference", r.Type);
Assert.Same(param.Resource, r.Resource);
},
r =>
{
Assert.Equal("Reference", r.Type);
Assert.Same(container.Resource, r.Resource);
});
}

[Fact]
Expand Down
4 changes: 4 additions & 0 deletions tests/Aspire.Hosting.Containers.Tests/ContainerResourceTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,10 @@ public async Task AddContainerWithArgs()
arg => Assert.Equal("http://c1:1234", arg), // this is the container hostname
arg => Assert.Equal("connectionString", arg));

// We don't yet process relationships set via the callbacks
// so we don't see the testResource2 nor exe1
Assert.False(c2.Resource.TryGetAnnotationsOfType<ResourceRelationshipAnnotation>(out var relationships));

var manifest = await ManifestUtils.GetManifest(c2.Resource);

var expectedManifest =
Expand Down
10 changes: 10 additions & 0 deletions tests/Aspire.Hosting.Tests/ExecutableResourceTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,16 @@ public async Task AddExecutableWithArgs()
arg => Assert.Equal("anotherConnectionString", arg)
);

Assert.True(exe2.Resource.TryGetAnnotationsOfType<ResourceRelationshipAnnotation>(out var relationships));
// We don't yet process relationships set via the callbacks
// so we don't see the testResource2 nor exe1
Assert.Collection(relationships,
r =>
{
Assert.Equal("Reference", r.Type);
Assert.Same(testResource, r.Resource);
});

var manifest = await ManifestUtils.GetManifest(exe2.Resource).DefaultTimeout();

var expectedManifest =
Expand Down
3 changes: 3 additions & 0 deletions tests/Aspire.Hosting.Tests/ProjectResourceTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -591,6 +591,9 @@ public async Task AddProjectWithArgs()
Assert.Collection(args,
arg => Assert.Equal("arg1", arg),
arg => Assert.Equal("http://localhost:1234", arg));

// We don't yet process relationships set via the callbacks
Assert.False(project.Resource.TryGetAnnotationsOfType<ResourceRelationshipAnnotation>(out var relationships));
Copy link
Member Author

@davidfowl davidfowl Mar 30, 2025

Choose a reason for hiding this comment

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

This is a case we're not handling #8426

Copy link
Member

@mitchdenny mitchdenny Mar 31, 2025

Choose a reason for hiding this comment

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

How might we solve this in the future. I think it makes sense to immediately add resource relationships when you can like this, but being able to catch these late discovered relationships is also useful.

So do we put the logic in two places.

If we try to put the logic in one place like in DCP integration, then any non-DCP resources won't be able to play (anything that evaluates expressions would need to the logic to add these relationships).

Copy link
Member Author

@davidfowl davidfowl Mar 31, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I started that way but decided to back out for 9.2. We’d do it in the application orchestrator before start.

}

[Theory]
Expand Down
37 changes: 37 additions & 0 deletions tests/Aspire.Hosting.Tests/WithEnvironmentTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,14 @@ public async Task EnvironmentReferencingEndpointPopulatesWithBindingUrl()
var config = await EnvironmentVariableEvaluator.GetEnvironmentVariablesAsync(projectB.Resource, DistributedApplicationOperation.Run, TestServiceProvider.Instance).DefaultTimeout();

Assert.Equal("https://localhost:2000", config["myName"]);

Assert.True(projectB.Resource.TryGetAnnotationsOfType<ResourceRelationshipAnnotation>(out var relationships));
Assert.Collection(relationships,
r =>
{
Assert.Equal("Reference", r.Type);
Assert.Same(projectA.Resource, r.Resource);
});
}

[Fact]
Expand Down Expand Up @@ -117,6 +125,14 @@ public async Task EnvironmentCallbackPopulatesValueWhenParameterResourceProvided
var config = await EnvironmentVariableEvaluator.GetEnvironmentVariablesAsync(projectA.Resource, DistributedApplicationOperation.Run, TestServiceProvider.Instance).DefaultTimeout();

Assert.Equal("MY_PARAMETER_VALUE", config["MY_PARAMETER"]);

Assert.True(projectA.Resource.TryGetAnnotationsOfType<ResourceRelationshipAnnotation>(out var relationships));
Assert.Collection(relationships,
r =>
{
Assert.Equal("Reference", r.Type);
Assert.Same(parameter.Resource, r.Resource);
});
}

[Fact]
Expand Down Expand Up @@ -236,6 +252,19 @@ public async Task EnvironmentVariableExpressions()
Assert.Equal("{container1.bindings.primary.port}", manifestConfig["PORT"]);
Assert.Equal("{container1.bindings.primary.targetPort}", manifestConfig["TARGET_PORT"]);
Assert.Equal("{test.connectionString};name=1", manifestConfig["HOST"]);

Assert.True(containerB.Resource.TryGetAnnotationsOfType<ResourceRelationshipAnnotation>(out var relationships));
Assert.Collection(relationships,
r =>
{
Assert.Equal("Reference", r.Type);
Assert.Same(container.Resource, r.Resource);
},
r =>
{
Assert.Equal("Reference", r.Type);
Assert.Same(test.Resource, r.Resource);
});
}

[Fact]
Expand Down Expand Up @@ -289,6 +318,14 @@ public async Task EnvironmentWithConnectionStringSetsProperEnvironmentVariable()

// Assert
Assert.Single(publishConfig, kvp => kvp.Key == envVarName && kvp.Value == "{sourceService.connectionString}");

Assert.True(targetBuilder.Resource.TryGetAnnotationsOfType<ResourceRelationshipAnnotation>(out var relationships));
Assert.Collection(relationships,
r =>
{
Assert.Equal("Reference", r.Type);
Assert.Same(sourceBuilder.Resource, r.Resource);
});
}

private sealed class TestResource(string name, string connectionString) : Resource(name), IResourceWithConnectionString
Expand Down
Loading
Loading