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.

Sign up for GitHub

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

Jump to bottom

[release/8.0-preview5] Endpoint changes #3278

Merged
merged 2 commits into from
Mar 29, 2024
Merged

[release/8.0-preview5] Endpoint changes #3278

merged 2 commits into from
Mar 29, 2024

Conversation

davidfowl
Copy link
Member

@davidfowl davidfowl commented Mar 29, 2024 •
edited
Loading

Backport of #3274 to release/8.0-preview5

Customer Impact

This change does 3 things:

  1. Makes it possible to describe multiple endpoints in the manifest by allocating ports for resources with binding that don't have them.
  2. Exposes a method that makes all http endpoints external. This replaces the prompting that happens today in deployment tools (like azd) from having to prompt the user for a port selection.
  3. Renames containerPort to targetPort and fixes overloads of WithEndpoint, WithHttpEndpoint and With HttpsEndpoint.

For

  1. Support aspire apps with multiple exposed ports. Azure/azure-dev#3267
  2. Do not hardcode services.app.config.exposedServices in vsrpc server Azure/azure-dev#3284

Testing

Unit tests, manual testing. azd end to end testing has not happened yet.

Risk

Medium

Microsoft Reviewers: Open in CodeFlow

@davidfowl
- In order for tools to better describe multiple endpoints int the ma…
c4a0297 
…nifest, we must include port information. At runtime most of the port information is dynamically generated and thus not described (for anything but containers), that doesn't work well when trying to deploy to various environments. We allocate ports in situations where there are none to match the runtime behavior. (#3274)

- Change also renames ContainerPort to TargetPort everywhere.
- Added `WithExternalHttpEndpoints` to mark all http and https endpoints as external.
@dotnet-issue-labeler dotnet-issue-labeler bot added the area-app-model Issues pertaining to the APIs in Aspire.Hosting, e.g. DistributedApplication label Mar 29, 2024
eerhardt
eerhardt approved these changes Mar 29, 2024
View reviewed changes
src/Aspire.Hosting/ResourceBuilderExtensions.cs Outdated Show resolved Hide resolved
src/Aspire.Hosting/ResourceBuilderExtensions.cs Outdated Show resolved Hide resolved
@davidfowl @eerhardt
Apply suggestions from code review
38c9984 
Co-authored-by: Eric Erhardt <eric.erhardt@microsoft.com>
@davidfowl davidfowl enabled auto-merge (squash) March 29, 2024 14:44
@davidfowl davidfowl merged commit 3736c12 into release/8.0-preview5 Mar 29, 2024
8 checks passed
@davidfowl davidfowl deleted the davidfowl/backport-endpoint-changes branch March 29, 2024 15:32
@rajeshkamal5050 rajeshkamal5050 mentioned this pull request Mar 29, 2024
Merged
@danmoseley danmoseley mentioned this pull request Apr 12, 2024
Closed
@github-actions github-actions bot locked and limited conversation to collaborators Apr 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@eerhardt eerhardt eerhardt approved these changes

@mitchdenny mitchdenny mitchdenny approved these changes

@joperezr joperezr Awaiting requested review from joperezr

Assignees
No one assigned
Labels
area-app-model Issues pertaining to the APIs in Aspire.Hosting, e.g. DistributedApplication
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@davidfowl @mitchdenny @eerhardt