Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow disabling proxy for persistent lifetime container endpoints #7232

Merged
merged 11 commits into from
Jan 28, 2025

Conversation

danegsta
Copy link
Member

@danegsta danegsta commented Jan 24, 2025

Description

The endpoint port assignment for persistent lifetime containers can be confusing due to Aspire creating proxied endpoints for all resources by default. This means that the ports reported for persistent containers in the Aspire dashboard are only valid while the App Host is running and the only way to access persistent containers while the App Host is not running is to inspect them and figure out what random port the container endpoint was exposed on.

This PR adds a new opt-in (and experimental) resource builder API .WithEndpointProxySupport(false) that can disable all proxies for the endpoints of a given container resource. In the long run we'd like to make this the new default behavior for persistent containers, but there is additional service discovery work that will need to happen before we can safely make proxy-less persistent containers the default behavior.

The main thing to be aware of when applying .WithEndpointProxySupport(false) to a container resource is that by default Aspire will attempt to use the (internal container) target port as the host port to expose the service. This significantly increases the risk of port conflicts or other issues allocating ports with the default behavior endpoint behavior. It's recommended that any resources being run with endpoint proxies disabled has explicit (unique) host ports set for all its endpoints.

Fixes # (issue)

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?

@davidfowl
Copy link
Member

@JamesNK moved stuff around 😄

@danegsta danegsta force-pushed the dev/danegsta/persistentProxy branch from f55adeb to 9e040d5 Compare January 24, 2025 18:27
@danegsta danegsta requested a review from mitchdenny as a code owner January 27, 2025 23:01
@danegsta
Copy link
Member Author

/AzurePipelines run

Copy link

Pull request contains merge conflicts.

@danegsta danegsta changed the title Disable proxy for persistent lifetime container endpoints Allow disabling proxy for persistent lifetime container endpoints Jan 27, 2025
@davidfowl davidfowl merged commit 3342152 into dotnet:main Jan 28, 2025
9 checks passed
@davidfowl
Copy link
Member

@danegsta good discussion and tradeoffs 😄. Can you make sure we document and update the relevant issue in 9.1?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants