Skip to content

Conversation

benjaminpetit
Copy link
Member

Description

Add a method to build a YarpCluster with a collection of string to use as addresses.

Fixes #10751

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?

@Copilot Copilot AI review requested due to automatic review settings September 12, 2025 13:24
Copy link
Contributor

github-actions bot commented Sep 12, 2025

🚀 Dogfood this PR with:

⚠️ WARNING: Do not do this without first carefully reviewing the code of this PR to satisfy yourself it is safe.

curl -fsSL https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 11368

Or

  • Run remotely in PowerShell:
iex "& { $(irm https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 11368"

Copy link
Contributor

@Copilot 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.

Pull Request Overview

This PR adds a new constructor to the YarpCluster class that accepts a list of string addresses, enabling load balancing across multiple endpoints. This change resolves issue #10751 by allowing YARP clusters to target multiple destinations directly from string addresses.

  • Adds a new YarpCluster constructor that accepts a params string[] addresses parameter
  • Updates the internal data structure from a single Target to an array of Targets
  • Modifies the environment variable generation to support multiple destinations

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
src/Aspire.Hosting.Yarp/ConfigurationBuilder/YarpCluster.cs Adds new constructor for string addresses and changes internal structure to support multiple targets
src/Aspire.Hosting.Yarp/YarpEnvConfigGenerator.cs Updates environment variable generation to iterate through multiple targets
tests/Aspire.Hosting.Yarp.Tests/YarpClusterTests.cs Adds test for new constructor and updates existing tests to use new Targets array

@github-actions github-actions bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Sep 12, 2025
/// </summary>
/// <param name="clusterName">The name of the cluster.</param>
/// <param name="addresses">The external addresses.</param>
internal YarpCluster(string clusterName, params string[] addresses)
Copy link
Member Author

Choose a reason for hiding this comment

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

Not a big fan to have a string and then an optional addrya for the list of addresses. We could use Uri instead?

@davidfowl
Copy link
Member

This allows for creating a cluster without any destinations, I think we need to support passing an array not params

@davidfowl davidfowl merged commit 513d8a0 into dotnet:main Sep 15, 2025
308 checks passed
@dotnet-policy-service dotnet-policy-service bot added this to the 9.5 milestone Sep 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unable to set YARP cluster destinations using new configuration
2 participants