Skip to content

Conversation

@karolz-ms
Copy link
Member

@karolz-ms karolz-ms commented Apr 11, 2024

Backpoirt #3184 to release/8.0
Fixes #2367

Customer Impact

Described in #2367. Basically, if a DAPR-enabled Aspire app uses anything other than HTTP for communication, it will fail to communicate with the DAPR sidecar, so pretty bad.

Testing

@paule96 did manual testing and added pretty comprehensive automated test coverage of the change.

Risk

Low IMO. The change is well localized and small.

Regression

Not really--the initial implementation of the feature only covered HTTP case

Microsoft Reviewers: Open in CodeFlow

* dapr change the app port depending on the app protocol

* fix endpoint annotations for dapr.

* fix the rest of the endpoints uri schemas

* Apply suggestions from code review

Co-authored-by: David Fowler <davidfowl@gmail.com>

* fix white spaces

* now we can define both, dapr protocol or endpoint from the configured endpoint

* add tests

* revert changes in the playground

* fix spelling issue

* add a large comment for all of this logic

* fix formatting

* Update src/Aspire.Hosting.Dapr/DaprDistributedApplicationLifecycleHook.cs

* change to switch expression fix test

* revert changes in playground

* format stuff

* fix is null and formating

* improve switch

---------

Co-authored-by: David Fowler <davidfowl@gmail.com>
@ghost ghost added the area-codeflow for labeling automated codeflow. intentionally a different color! label Apr 11, 2024
@dotnet-policy-service dotnet-policy-service bot added the Servicing-consider Issue for next servicing release review label Apr 11, 2024
@danmoseley
Copy link
Member

Where did we end up with using HTTPS by default as well? Separate issue?

@danmoseley danmoseley added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Apr 11, 2024
@danmoseley
Copy link
Member

approved for obvious reasons.

@danmoseley danmoseley enabled auto-merge (squash) April 11, 2024 23:27
@danmoseley danmoseley added area-app-model Issues pertaining to the APIs in Aspire.Hosting, e.g. DistributedApplication and removed area-codeflow for labeling automated codeflow. intentionally a different color! labels Apr 11, 2024
@davidfowl
Copy link
Member

Where did we end up with using HTTPS by default as well? Separate issue?

Needs more design, post GA.

@danmoseley danmoseley merged commit f80e6ab into release/8.0 Apr 12, 2024
@danmoseley danmoseley deleted the backport/pr-3184 branch April 12, 2024 00:08
@danmoseley danmoseley mentioned this pull request Apr 12, 2024
@github-actions github-actions bot locked and limited conversation to collaborators May 12, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-app-model Issues pertaining to the APIs in Aspire.Hosting, e.g. DistributedApplication Servicing-approved Approved for servicing release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants