Skip to content

Conversation

@davidfowl
Copy link
Member

@davidfowl davidfowl commented Feb 28, 2025

Description

Introduces the IResourceWithoutLifetime interface for resources without a lifetime, such as parameters and connection strings. Updates ParameterResource to implement this interface and adds the ConnectionStringParameterResource class for better management of connection string parameters.

Implements the AddConnectionString method in ConnectionStringBuilderExtensions and updates ApplicationOrchestrator to handle resources without a lifetime correctly.

Tests have been added and modified to verify the new functionality, and Program.cs has been updated to demonstrate the new connection string capabilities, including the addition of a second connection string with a parameter value.

These changes improve the overall resource management in the application, particularly for connection strings and parameters.

Fixes #7641
Fixes #7642
Fixes #2964

Checklist

  • Is this feature complete?
    • Yes. Ready to ship.
    • No. Follow-up changes expected.
  • Are you including unit tests for the changes and scenario tests if relevant?
    • Yes
    • No
  • Did you add public API?
    • Yes
      • If yes, did you have an API Review for it?
        • Yes
        • No
      • Did you add <remarks /> and <code /> elements on your triple slash comments?
        • Yes
        • No
    • No
  • Does the change make any security assumptions or guarantees?
    • Yes
      • If yes, have you done a threat model and had a security review?
        • Yes
        • No
    • No
  • Does the change require an update in our Aspire docs?

Introduces the `IResourceWithoutLifetime` interface for resources without a lifetime, such as parameters and connection strings. Updates `ParameterResource` to implement this interface and adds the `ConnectionStringParameterResource` class for better management of connection string parameters.

Implements the `AddConnectionString` method in `ConnectionStringBuilderExtensions` and updates `ApplicationOrchestrator` to handle resources without a lifetime correctly.

Tests have been added and modified to verify the new functionality, and `Program.cs` has been updated to demonstrate the new connection string capabilities, including the addition of a second connection string with a parameter value.

These changes improve the overall resource management in the application, particularly for connection strings and parameters.
Copilot AI review requested due to automatic review settings February 28, 2025 09:28
@davidfowl
Copy link
Member Author

@JamesNK and @adamint, I'd like to follow up on this work with a way to show these resources without a lifetime in the dashboard. Right now, we hide parameters and connection strings in the dashboard but that isn't ideal. There's also no way to unhide resources.

I'd like to solve a couple of things here:

  • Make sure that it is possible to see these in the dashboard
  • Make sure we have a status that isn't running that means this resource is "in the right state" that isn't running.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

PR Overview

This pull request enhances resource management by introducing the IResourceWithoutLifetime interface and a dedicated ConnectionStringParameterResource to improve handling of connection strings and parameter resources. Key changes include:

  • Adding IResourceWithoutLifetime and updating ParameterResource to implement it.
  • Introducing ConnectionStringParameterResource and updating related extension methods.
  • Extending orchestrator logic and tests to support resources without a lifetime.

Reviewed Changes

File Description
src/Aspire.Hosting/ConnectionStringBuilderExtensions.cs Added extension methods for connection string resources with updated XML comments.
src/Aspire.Hosting/ApplicationModel/IResourceWithoutLifetime.cs Introduced new interface for resources without a lifetime.
src/Aspire.Hosting/ConnectionStringResource.cs Created a new resource class for connection strings implementing the new interface.
tests/Aspire.Hosting.Tests/* Added/updated tests to verify connection string and parameter resource functionality.
src/Aspire.Hosting/ParameterResource.cs Updated ParameterResource to implement IResourceWithoutLifetime and added a ConfigurationKey property.
src/Aspire.Hosting/ParameterResourceBuilderExtensions.cs Updated resource builder extensions to use the new ConnectionStringParameterResource.
src/Aspire.Hosting/ApplicationModel/ExpressionResolver.cs Adjusted pattern matching to work with new resource types.
src/Aspire.Hosting/Orchestrator/ApplicationOrchestrator.cs Added processing logic for resources without a lifetime in orchestrator.

Copilot reviewed 19 out of 19 changed files in this pull request and generated 2 comments.

Comment on lines +71 to +75
internal string ConfigurationKey
{
get => _configurationKey ?? (IsConnectionString ? $"ConnectionStrings:{Name}" : $"Parameters:{Name}");
set => _configurationKey = value;
}
Copy link
Member

Choose a reason for hiding this comment

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

Glad this is internal. I want to avoid strictly tying the concept of a parameter/connection string to the .NET configuration store. They are connected for convienence however.

Copy link
Member

@mitchdenny mitchdenny left a comment

Choose a reason for hiding this comment

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

A few suggestions for future enhancements and test improvements, but overall this looks good and seems to work with the run throughs that I made. I can see how this will be useful for rapidly creating wrapper APIs where the input is primarily something like an API key with a few extra bits and pieces bolted on.

Assert.Equal("Endpoint=http://localhost:3452;Key=secretKey", config["ConnectionStrings__cs"]);
}

[Fact]
Copy link
Member

Choose a reason for hiding this comment

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

Add a test like we have in the ParameterEndToEnd playground where we use a resource (such as a SQL DB) as an input to the expression.

@davidfowl
Copy link
Member Author

davidfowl commented Mar 3, 2025

@JamesNK new dashboard failures? Are they related to your changes to tests? (https://github.com/dotnet/aspire/actions/runs/13622390377)

@davidfowl davidfowl merged commit 14f1b4e into main Mar 3, 2025
72 checks passed
@davidfowl davidfowl deleted the davidfowl/cs-resource branch March 3, 2025 05:03
@github-actions github-actions bot added the area-app-model Issues pertaining to the APIs in Aspire.Hosting, e.g. DistributedApplication label Mar 10, 2025
@github-actions github-actions bot locked and limited conversation to collaborators Apr 9, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-app-model Issues pertaining to the APIs in Aspire.Hosting, e.g. DistributedApplication

Projects

None yet

4 participants