Use scheme instead of endpoint name when registering service discovery environment variables#14625
Use scheme instead of endpoint name when registering service discovery environment variables#14625danegsta wants to merge 1 commit into
Conversation
…y environment variables
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 14625Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 14625" |
|
Closing this as I accidentally opened against main instead of release/13.2. Superseded by #14626. |
There was a problem hiding this comment.
Pull request overview
This PR fixes service discovery environment variable naming by using the endpoint scheme (e.g., https) rather than the endpoint name when emitting services__... keys, aligning with .NET service discovery expectations and addressing the reported Python/uvicorn discovery regression.
Changes:
- Emit service discovery keys as
services__{serviceName}__{scheme}__{index}and track per-scheme indices to handle multiple endpoints sharing a scheme. - Update unit tests and Docker/Kubernetes publisher snapshots to reflect the new key format and indexing behavior.
- Update related XML documentation/comments to reference
{endpointScheme}(partially; one doc spot still mentions{endpointName}).
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Aspire.Hosting/ResourceBuilderExtensions.cs | Changes service discovery env var key generation to use endpoint scheme and handle per-scheme indexing; updates some docs. |
| src/Aspire.Hosting/ApplicationModel/ReferenceEnvironmentInjectionFlags.cs | Updates enum doc comment to reflect scheme-based service discovery key format. |
| tests/Aspire.Hosting.Tests/WithReferenceTests.cs | Updates assertions for scheme-based services__... keys and scheme-collision indexing. |
| tests/Aspire.Hosting.Kubernetes.Tests/Snapshots/KubernetesPublisherTests.KubernetesWithProjectResources#01.verified.yaml | Updates Helm values snapshot keys/indexing for scheme-based service discovery env vars. |
| tests/Aspire.Hosting.Kubernetes.Tests/Snapshots/KubernetesPublisherTests.KubernetesWithProjectResources#06.verified.yaml | Updates manifest snapshot keys/indexing for scheme-based service discovery env vars. |
| tests/Aspire.Hosting.Docker.Tests/Snapshots/DockerComposePublisherTests.DockerComposeWithProjectResources.verified.yaml | Updates docker-compose snapshot keys/indexing for scheme-based service discovery env vars. |
| /// <summary> | ||
| /// Injects service discovery and endpoint information from the specified endpoint into the project resource using the source resource's name as the service name. | ||
| /// Each endpoint uri will be injected using the format defined by the <see cref="ReferenceEnvironmentInjectionAnnotation"/> on the destination resource, i.e. | ||
| /// either "services__{name}__{endpointName}__{endpointIndex}={uriString}" for .NET service discovery, or "{NAME}_{ENDPOINT}={uri}" for endpoint injection. | ||
| /// </summary> |
There was a problem hiding this comment.
The XML doc for this overload still describes service discovery keys as using the endpoint name segment, but the implementation (and other WithReference overload docs) now use the endpoint scheme segment. Update this doc string to match the new key format (services__{name}{endpointScheme}{endpointIndex}).
| @@ -1,4 +1,4 @@ | |||
| --- | |||
| --- | |||
There was a problem hiding this comment.
This snapshot file now starts with a UTF-8 BOM character (U+FEFF), which can cause unnecessary diffs and can confuse YAML tooling/parsers. Please remove the BOM so the first line starts with '---' only.
| --- | |
| --- |
| @@ -1,4 +1,4 @@ | |||
| parameters: | |||
| parameters: | |||
There was a problem hiding this comment.
This snapshot file now starts with a UTF-8 BOM character (U+FEFF) before 'parameters:'. Please remove the BOM to keep encoding consistent with other snapshots and avoid YAML key/name issues.
| parameters: | |
| parameters: |
| @@ -1,4 +1,4 @@ | |||
| services: | |||
| services: | |||
There was a problem hiding this comment.
This snapshot file now starts with a UTF-8 BOM character (U+FEFF) before 'services:'. Please remove the BOM to avoid inconsistent encoding and unnecessary diffs.
| services: | |
| services: |
Description
Updates the service discovery environment variable names to use the endpoint scheme rather than the endpoint name when building the environment variable name (i.e.
services__{service name}__{scheme}__0instead ofservices__{service name}__{endpoint name}__0.Fixes #14411
Checklist
<remarks />and<code />elements on your triple slash comments?aspire.devissue: