[release/9.1] Resolve DistributedApplicationResourceBuilder<ResourceWithConnectionStringSurrogate> correctly in args evaluation #7727
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
added 4 commits
February 21, 2025 01:16
…tringSurrogate> correctly in args evaluation
danmoseley
approved these changes
Feb 21, 2025
|
@JamesNK since you're online could you be the 2nd review? |
mitchdenny
approved these changes
Feb 21, 2025
JamesNK
approved these changes
Feb 21, 2025
There was a problem hiding this comment.
The code change is modest and makes sense.
There's a unit test for resolving the value for a container. I see there is an argument in the code to check whether the resource is a container or not. I assume that has some behavior impact.
Should there be a unit test to check this works for a project/exe?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Backport of #7716 to release/9.1
/cc @mitchdenny @adamint
Customer Impact
Currently in .NET Aspire we have had inconsistent handling of secret parameters depending on whether it is a connection string or a regular secret parameter. This manifested in the
WithArgs(...)extension method incorrectly rendering connection string args as the type name of the resource builder. This was exposed by a recent improvement to the dashboard where we suppress display of secrets.This PR fixes the underlying issue with secret connection string resources being used as arguments to resources (this was not a regression, but an existing issue that became more obvious with the dashboard improvements).
Testing
Manual testing and added unit tests.
Risk
Low. This wasn't working previously anyway - code change is very targeted.
Regression?