Update service discovery environment variables#16210
Update service discovery environment variables#16210danegsta merged 6 commits intomicrosoft:mainfrom
Conversation
…e support Fix scheme key computation in service discovery environment variables: - Endpoints named 'http'/'https' use the actual URI scheme as the key, correctly handling TLS upgrades (e.g., endpoint 'http' with scheme 'https' produces services__svc__https__0). - All other endpoint names (e.g., 'prometheus', 'management') use the endpoint name as the key, restoring the ability to differentiate multiple logical services from a single resource. Add optional 'name' parameter to WithReference(EndpointReference): - Allows custom service names for individual endpoint references, enabling scenarios like services__kafka-admin__admin__0. - ApplyEndpoints now keys annotations on (resource, serviceName) to support multiple named references to the same resource. Fix YARP destination address generation: - Endpoints named 'http'/'https' use standard resolution without the underscore prefix (http://resourceName). - Other endpoint names keep the named endpoint DNS-SD convention (http://_endpointName.resourceName), which now works correctly because the service discovery key uses the endpoint name. Apply same scheme key fix to DevTunnels for consistency. Fixes microsoft#15925 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Updates service-discovery environment variable key generation to better handle endpoint name vs scheme mismatches (especially HTTP→HTTPS TLS upgrades), while still supporting DNS-SD-style named endpoints for non-HTTP endpoints.
Changes:
- Generate service discovery env-var keys using the actual scheme for endpoints named
http/https, otherwise use the endpoint name. - Update YARP cluster destination address formatting to omit DNS-SD
_name.prefix forhttp/httpsendpoints. - Refresh unit tests and snapshot expectations to reflect the new key/address formats.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Aspire.Hosting.Yarp.Tests/YarpClusterTests.cs | Updates expectations for YARP cluster target address formatting for http/https. |
| tests/Aspire.Hosting.Yarp.Tests/Snapshots/YarpConfigGeneratorTests.GenerateEnvVariablesConfigurationDockerCompose.verified.env | Updates snapshot for cluster destination addresses (drops _http. prefix). |
| tests/Aspire.Hosting.Tests/WithReferenceTests.cs | Updates expected service-discovery env-var keys and adds new coverage for scheme/name mismatch and named endpoints. |
| src/Aspire.Hosting/ResourceBuilderExtensions.cs | Changes service-discovery key generation logic; adds helper for http/https endpoint-name detection. |
| src/Aspire.Hosting.Yarp/ConfigurationBuilder/YarpCluster.cs | Changes YARP target address generation to special-case http/https endpoint names. |
| src/Aspire.Hosting.DevTunnels/DevTunnelResourceBuilderExtensions.cs | Mirrors service-discovery key changes in Dev Tunnels integration; adds local helper. |
| playground/yarp/Yarp.AppHost/Properties/launchSettings.json | Removes commented env var entries. |
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 16210Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 16210" |
Fix scheme key computation in service discovery environment variables: - Endpoints named 'http'/'https' use the actual URI scheme as the key, correctly handling TLS upgrades (e.g., endpoint 'http' with scheme 'https' produces services__svc__https__0). - All other endpoint names (e.g., 'prometheus', 'management') use the endpoint name as the key, restoring the ability to differentiate multiple logical services from a single resource. Fix YARP destination address generation: - Endpoints named 'http'/'https' use standard resolution without the underscore prefix (http://resourceName). - Other endpoint names keep the named endpoint DNS-SD convention (http://_endpointName.resourceName), which now works correctly because the service discovery key uses the endpoint name. Apply same scheme key fix to DevTunnels for consistency. Fixes microsoft#15925 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
24515ae to
6b0972a
Compare
| var config = await EnvironmentVariableEvaluator.GetEnvironmentVariablesAsync(projectB.Resource, DistributedApplicationOperation.Run, TestServiceProvider.Instance).DefaultTimeout(); | ||
|
|
||
| Assert.Equal("https://localhost:2000", config["services__projecta__https__0"]); | ||
| Assert.Equal("https://localhost:2000", config["services__projecta__mybinding__0"]); |
There was a problem hiding this comment.
Does ServiceDiscovery like this?
There was a problem hiding this comment.
Yeah, there's two different ways ServiceDiscovery handles service resolution; the standard scheme based resolution (i.e. http+https://servicename) and DNS style resolution (http://_mybinding.projecta). Yarp was taking advantage of the latter and some fixes we made to better handle service discovery environment variables when switching endpoints from http to https ended up breaking the usage of WithReference for non scheme based endpoints.
Update TwoPassScanningGeneratedAspire snapshots for all polyglot languages (Go, Java, Python, Rust, TypeScript) to include the new EndpointReference.IsHttpSchemeNamedEndpoint property. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
/backport to release/13.2 |
|
Started backporting to |
|
@danegsta backporting to git am output$ git am --3way --empty=keep --ignore-whitespace --keep-non-patch changes.patch
Applying: Fix service discovery key generation and add per-endpoint service name support
Applying: Fix service discovery key generation for named endpoints
error: sha1 information is lacking or useless (src/Aspire.Hosting/ResourceBuilderExtensions.cs).
error: could not build fake ancestor
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config set advice.mergeConflict false"
Patch failed at 0002 Fix service discovery key generation for named endpoints
Error: The process '/usr/bin/git' failed with exit code 128 |
The custom1-custom4 endpoints now use their endpoint names as service discovery keys instead of being indexed under the http scheme. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
we typically dont add public APIs in patches but. |
Description
This change strikes a balance between handling http/https endpoint name and scheme mismatches and allowing non-http endpoints to be referenced using DNS style service discovery notation instead of by scheme.
Fixes #15925
Checklist
<remarks />and<code />elements on your triple slash comments?aspire.devissue: