Make WithEndpoint update existing endpoints instead of throwing#16039
Make WithEndpoint update existing endpoints instead of throwing#16039
Conversation
Add WithHttpPort<T> and WithHttpsPort<T> extension methods on IResourceBuilder<T> where T : IResourceWithEndpoints. These provide a simple way to set the host port on existing HTTP/HTTPS endpoints without needing to use the lower-level WithEndpoint callback API. This addresses a common pain point where users want to pin a port for local development (e.g., Vite apps, Keycloak OIDC flows, API gateways) but the existing API either requires knowledge of endpoint internals or throws when trying to re-add an endpoint that was auto-created. Relates to #13807 Relates to #15873 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 16039Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 16039" |
The WithHttpPort/WithHttpsPort methods are tested in the core Aspire.Hosting.Tests project which covers the same code path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds WithHttpPort and WithHttpsPort extension methods to make it easier to pin host ports for existing HTTP/HTTPS endpoints on resources implementing IResourceWithEndpoints, addressing common local-dev ergonomics issues (e.g., Vite apps and Keycloak).
Changes:
- Introduces
WithHttpPort(int?)/WithHttpsPort(int?)onIResourceBuilder<T>to set host ports on the"http"/"https"endpoints. - Adds unit tests in
Aspire.Hosting.Testscovering port-setting for container endpoints. - Adds JavaScript hosting tests validating port-setting behavior for Vite/Node resources.
Show a summary per file
| File | Description |
|---|---|
| src/Aspire.Hosting/ResourceBuilderExtensions.cs | Adds the new WithHttpPort / WithHttpsPort APIs and their XML docs/exports. |
| tests/Aspire.Hosting.Tests/WithEndpointTests.cs | Adds tests for setting ports on existing container HTTP/HTTPS endpoints. |
| tests/Aspire.Hosting.JavaScript.Tests/ResourceCreationTests.cs | Adds tests for Vite/Node resources using the new port helpers. |
Copilot's findings
Comments suppressed due to low confidence (1)
src/Aspire.Hosting/ResourceBuilderExtensions.cs:1434
- Same issue as
WithHttpPort: this usesWithEndpoint("https", endpoint => ...)which will create a new endpoint by default if one isn't present. If created, the annotation will haveUriSchemeinferred as "tcp" (protocol-based) and onlyNameset to "https", so the resulting endpoint isn't actually HTTPS. This also contradicts the docs stating the endpoint must already exist.
Recommend enforcing existence (throw with guidance to call WithHttpsEndpoint first) or at least pass createIfNotExists:false and avoid creating a malformed endpoint annotation.
[AspireExport(Description = "Sets the host port for the HTTPS endpoint")]
public static IResourceBuilder<T> WithHttpsPort<T>(this IResourceBuilder<T> builder, int? port) where T : IResourceWithEndpoints
{
ArgumentNullException.ThrowIfNull(builder);
return builder.WithEndpoint("https", endpoint =>
{
endpoint.Port = port;
});
- Files reviewed: 2/2 changed files
- Comments generated: 2
…e tests - Pass createIfNotExists: false to WithEndpoint in both WithHttpPort and WithHttpsPort to avoid silently creating malformed endpoints when the target endpoint doesn't exist. - Add negative tests verifying no endpoint is created when http/https endpoints are missing. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The new [AspireExport] methods are picked up by the polyglot codegen, updating snapshots for TypeScript, Go, Java, Python, and Rust. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Keycloak creates its HTTPS endpoint lazily, so WithHttpsPort would be a no-op when chained directly after AddKeycloak. Replace with a container example that accurately demonstrates the API contract. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Instead of throwing when an endpoint with the same name already exists, update the existing endpoint with any non-null parameter values. This makes WithHttpEndpoint/WithHttpsEndpoint idempotent and removes the need for separate WithHttpPort/WithHttpsPort convenience methods. This directly solves the common pain point where resources like AddViteApp auto-create an 'http' endpoint and users cannot call WithHttpEndpoint to configure it. Relates to #13807 Relates to #15873 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ations - Remove unconditional isProxied override on update path. Since the parameter defaults to true in the public API, we can't distinguish 'user passed true' from 'default true', so we leave it unchanged. - Only add EnvironmentCallbackAnnotation when env is newly configured (TargetPortEnvironmentVariable was null) to avoid duplicates. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
When the env parameter changes on an update (e.g., PORT -> NEW_PORT), the callback now reads TargetPortEnvironmentVariable from the captured annotation at evaluation time rather than closing over the string. This ensures only the current env var name is set, not the stale one. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Both the create and update paths now call the same helper method for configuring the endpoint environment variable callback. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Scheme is not changed on update (by design) - isProxied is not changed on update (can't distinguish default from explicit) - isExternal is updated when explicitly set (nullable, so null = don't change) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Re-running the failed jobs in the CI workflow for this pull request because 1 job was identified as retry-safe transient failures in the CI run attempt.
|
Since the default is true, passing false is always intentional and safe to apply. Passing true is indistinguishable from the default so it's left as a no-op to avoid accidentally overwriting an explicit false. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Move the EndpointAnnotation construction after the existing-endpoint check so we don't allocate an annotation only to throw it away on the update path. Resolve the endpoint name inline using the same logic as the EndpointAnnotation constructor. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
🎬 CLI E2E Test Recordings — 58 recordings uploaded (commit View recordings
📹 Recordings uploaded automatically from CI run #24268589828 |
Description
Change
WithEndpoint,WithHttpEndpoint, andWithHttpsEndpointto update an existing endpoint when one with the same name already exists, instead of throwing aDistributedApplicationException.When updating, only non-null parameter values are applied — null parameters preserve the existing endpoint's values. This makes the API idempotent and intuitive:
With*means "set this property", not "create a new thing".Before (throws)
After (updates)
Motivation
Setting a fixed host port is a common need but the previous API threw when an endpoint with the same name already existed. This came up repeatedly:
GitHub Issues:
addViteAppauto-createshttpendpoint, so callingwithHttpEndpointto set a port throwsDiscord feedback:
The workaround was
WithEndpoint("http", e => e.Port = 3000)which requires knowing the endpoint name convention.Fixes #13807
Fixes #15873
Fixes #15854
Checklist
aspire.devissue: