Skip to content

Use scheme instead of endpoint name when registering service discovery environment variables#14626

Merged
davidfowl merged 3 commits into
release/13.2from
danegsta/serviceDiscovery
Feb 24, 2026
Merged

Use scheme instead of endpoint name when registering service discovery environment variables#14626
davidfowl merged 3 commits into
release/13.2from
danegsta/serviceDiscovery

Conversation

@danegsta

Copy link
Copy Markdown
Member

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}__0 instead of services__{service name}__{endpoint name}__0).

Fixes #14411

@github-actions

github-actions Bot commented Feb 23, 2026

Copy link
Copy Markdown
Contributor

🚀 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 -- 14626

Or

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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Updates Aspire Hosting’s service discovery environment variable naming so the key segment uses the endpoint scheme (e.g., https, http) instead of the endpoint name, aligning emitted configuration with how .NET service discovery matches URIs.

Changes:

  • Generate services__{serviceName}__{scheme}__{index} keys (with per-scheme indexing for multiple endpoints sharing a scheme).
  • Update unit tests and publisher snapshot baselines to reflect the new key format.
  • Refresh XML docs/comments describing the service discovery env-var format.

Reviewed changes

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

Show a summary per file
File Description
tests/Aspire.Hosting.Tests/WithReferenceTests.cs Updates assertions to expect scheme-based services__... keys.
tests/Aspire.Hosting.Kubernetes.Tests/Snapshots/KubernetesPublisherTests.KubernetesWithProjectResources#06.verified.yaml Updates Helm chart snapshot to use scheme-based service discovery keys and indices.
tests/Aspire.Hosting.Kubernetes.Tests/Snapshots/KubernetesPublisherTests.KubernetesWithProjectResources#01.verified.yaml Updates values snapshot to use scheme-based service discovery keys and indices.
tests/Aspire.Hosting.Docker.Tests/Snapshots/DockerComposePublisherTests.DockerComposeWithProjectResources.verified.yaml Updates Docker Compose snapshot to use scheme-based service discovery keys and indices.
src/Aspire.Hosting/ResourceBuilderExtensions.cs Switches service discovery key generation from endpoint name to scheme; updates some related XML docs.
src/Aspire.Hosting/ApplicationModel/ReferenceEnvironmentInjectionFlags.cs Updates enum XML docs to describe scheme-based service discovery key format.

Comment thread src/Aspire.Hosting/ResourceBuilderExtensions.cs
Comment thread src/Aspire.Hosting/ResourceBuilderExtensions.cs
@github-actions

github-actions Bot commented Feb 23, 2026

Copy link
Copy Markdown
Contributor

🎬 CLI E2E Test Recordings

The following terminal recordings are available for commit d7fbc6b:

Test Recording
AddPackageInteractiveWhileAppHostRunningDetached ▶️ View Recording
AddPackageWhileAppHostRunningDetached ▶️ View Recording
AgentCommands_AllHelpOutputs_AreCorrect ▶️ View Recording
AgentInitCommand_MigratesDeprecatedConfig ▶️ View Recording
AgentInitCommand_WithMalformedMcpJson_ShowsErrorAndExitsNonZero ▶️ View Recording
AspireUpdateRemovesAppHostPackageVersionFromDirectoryPackagesProps ▶️ View Recording
Banner_DisplayedOnFirstRun ▶️ View Recording
Banner_DisplayedWithExplicitFlag ▶️ View Recording
CreateAndDeployToDockerCompose ▶️ View Recording
CreateAndDeployToDockerComposeInteractive ▶️ View Recording
CreateAndPublishToKubernetes ▶️ View Recording
CreateAndRunAspireStarterProject ▶️ View Recording
CreateAndRunAspireStarterProjectWithBundle ▶️ View Recording
CreateAndRunJsReactProject ▶️ View Recording
CreateAndRunPythonReactProject ▶️ View Recording
CreateEmptyAppHostProject ▶️ View Recording
CreateStartAndStopAspireProject ▶️ View Recording
CreateStartWaitAndStopAspireProject ▶️ View Recording
CreateTypeScriptAppHostWithViteApp ▶️ View Recording
DoctorCommand_DetectsDeprecatedAgentConfig ▶️ View Recording
DoctorCommand_WithSslCertDir_ShowsTrusted ▶️ View Recording
DoctorCommand_WithoutSslCertDir_ShowsPartiallyTrusted ▶️ View Recording
LogsCommandShowsResourceLogs ▶️ View Recording
PsCommandListsRunningAppHost ▶️ View Recording
ResourcesCommandShowsRunningResources ▶️ View Recording
StagingChannel_ConfigureAndVerifySettings_ThenSwitchChannels ▶️ View Recording
StopAllAppHostsFromAppHostDirectory ▶️ View Recording
StopAllAppHostsFromUnrelatedDirectory ▶️ View Recording
StopNonInteractiveMultipleAppHostsShowsError ▶️ View Recording
StopNonInteractiveSingleAppHost ▶️ View Recording
StopWithNoRunningAppHostExitsSuccessfully ▶️ View Recording

📹 Recordings uploaded automatically from CI run #22329806400

@danegsta
danegsta requested a review from jfversluis as a code owner February 23, 2026 23:43
@davidfowl

Copy link
Copy Markdown
Contributor

This is a breaking change right?

@davidfowl davidfowl added the breaking-change Issue or PR that represents a breaking API or functional change over a prerelease. label Feb 24, 2026
@danegsta

Copy link
Copy Markdown
Member Author

This is a breaking change right?

It’s both a breaking change and a bug fix (we’d made similar breaking changes in 13.1 as part of the new https support, but missed this one).

@davidfowl
davidfowl merged commit 08e7ce3 into release/13.2 Feb 24, 2026
679 of 683 checks passed
@davidfowl
davidfowl deleted the danegsta/serviceDiscovery branch February 24, 2026 05:40
@davidfowl

Copy link
Copy Markdown
Contributor

I tagged it as such.

@dotnet-policy-service dotnet-policy-service Bot added this to the 13.2 milestone Feb 24, 2026
Copilot AI pushed a commit that referenced this pull request Mar 10, 2026
…y environment variables (#14626)

* Use scheme instead of endpoint name when registering service discovery environment variables

* Respond to PR comments

* Simplify OTLP extraction to avoid service discovery lookup
@fdohrendorfG

Copy link
Copy Markdown

After thinking about it I think this should have combined endpointName and scheme instead of just replacing one with the other.

I think its very valid that more then one endpoint is going to use the https/http scheme but unlikely that it is for the same endpoint name.

See:

.WithHttpEndpoint(port: port, targetPort: DefaultContainerPort)
.WithHttpEndpoint(targetPort: ManagementInterfaceContainerPort, name: ManagementEndpointName)

Those have nothing in common, but now the management endpoint is used instead of the http endpoint.

See:

return $"https+http://{serviceName}/realms/{realm}";

@github-actions github-actions Bot locked and limited conversation to collaborators Apr 24, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

breaking-change Issue or PR that represents a breaking API or functional change over a prerelease.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants