Skip to content

Ensure resource non-endpoint URLs are active when initialized #9696

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

Merged
merged 13 commits into from
Jun 20, 2025

Conversation

DamianEdwards
Copy link
Member

@DamianEdwards DamianEdwards commented Jun 4, 2025

Description

Changes resource URL processing so that all resources get their non-endpoint URLs initialized earlier than today. This is done by changing WithUrl<T>(this IResourceBuilder<T> builder, string url, string? displayText = null) to add the ResourceUrlAnnotation directly to the resource, rather than register a ResourceUrlCallbackAnnotation to be processed later. In ApplicationOrchestrator's handler for the DCP OnResourcesPrepared event, resource's initial state is published, including any static URLs. URL callbacks (via ResourceUrlCallbackAnnotation) are now processed in a handler for the ResourceEndpointsAllocatedEvent handler at which point all endpoints are expected to be allocated for the resource. The endpoint URLs are published initially in an inactive state. Endpoint URLs must be published as active by some other actor in the future (for DCP-controlled resources this already happens, for custom resources it's up to the resource custom logic to do this).

New lifecycle is:

  1. ApplicationOrchestrator handles DCP OnResourcesPreparedContext event:
    • Initial resource state is published including any static URLs (URLs not added via a callback, typically endpoint URLs)
    • The InitializeResourceEvent is published to indicate to the resource it should initialize itself, e.g. start its lifecycle
  2. ApplicationOrchestrator handles the ResourceEndpointsAllocatedEvent for each resource:
    • This is fired when a resource's endpoints have been allocated (by DcpExecutor for executables, projects, and containers, and by the resource itself for custom resources)
    • Resources that implement IResourceWithoutLifetime and IValueProvider are processed (their values are published)
    • URL callbacks for the resource are run & thus URLs for the resource are set and then published

Fixes #9516

Checklist

  • Is this feature complete?
    • Yes. Ready to ship.
  • Are you including unit tests for the changes and scenario tests if relevant?
    • Yes
  • Did you add public API?
    • No
  • Does the change make any security assumptions or guarantees?
    • No
  • Does the change require an update in our Aspire docs?
    • No

@Copilot Copilot AI review requested due to automatic review settings June 4, 2025 23:05
@DamianEdwards DamianEdwards requested a review from mitchdenny as a code owner June 4, 2025 23:05
@DamianEdwards DamianEdwards requested a review from davidfowl June 4, 2025 23:05
@github-actions github-actions bot added the area-app-model Issues pertaining to the APIs in Aspire.Hosting, e.g. DistributedApplication label Jun 4, 2025
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR ensures that non-endpoint resource URLs are marked active immediately when resources are initialized, while endpoint URLs remain inactive until explicitly activated.

  • ApplicationOrchestrator now sets IsInactive based on whether a URL has an associated endpoint.
  • Tests in WithUrlsTests.cs have been updated and extended to verify initial active/inactive states for both project and custom resources.

Reviewed Changes

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

File Description
tests/Aspire.Hosting.Tests/WithUrlsTests.cs Renamed and extended test, added CustomResource stub, split URL snapshots, updated assertions for active/inactive URLs
src/Aspire.Hosting/Orchestrator/ApplicationOrchestrator.cs Changed initial UrlSnapshot.IsInactive logic to url.Endpoint is not null; moved and adjusted related comment

@@ -396,50 +397,164 @@ public async Task MultipleUrlsForSingleEndpointAreIncludedInUrlSnapshot()
}

[Fact]
public async Task NonEndpointUrlsAreInactiveUntilResourceRunning()
public async Task UrlsAreInExpectedStateForResourcesGivenTheirLifecycle()
Copy link
Member

Choose a reason for hiding this comment

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

Was hard to figure out what this test is testing since it looks like you are simulating what should happen, and then asserting that the simulation is correct vs internal behavior (might just be that there is a lot going on in this test).

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah this test is verifying that the state of the URLs for project resources and custom resources is as expected as those resources move through their lifecycles.

@DamianEdwards DamianEdwards requested a review from mitchdenny June 20, 2025 01:06
@DamianEdwards DamianEdwards merged commit bab3ebb into main Jun 20, 2025
252 checks passed
@DamianEdwards DamianEdwards deleted the damianedwards/withurls-fixes branch June 20, 2025 05:36
captainsafia pushed a commit that referenced this pull request Jun 20, 2025
* Ensure resource non-endpoint URLs are active when initialized

Fixes #9516

* WIP

* Split static URLs upfront

* Remove commented out line

* Refactoring

* Test tweak

* Tweak test

* Update src/Aspire.Hosting/Orchestrator/ApplicationOrchestrator.cs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Update WithUrlsTests.cs

* Fix race in test

* Add comment

---------

Co-authored-by: Mitch Denny <midenn@microsoft.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-app-model Issues pertaining to the APIs in Aspire.Hosting, e.g. DistributedApplication
Projects
None yet
Development

Successfully merging this pull request may close these issues.

No urls in the CustomResources.AppHost dashboard
2 participants