Skip to content

Add ExternalServiceResource for modeling external services with service discovery support #9965

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 35 commits into from
Jul 2, 2025

Conversation

Copilot
Copy link
Contributor

@Copilot Copilot AI commented Jun 20, 2025

This PR introduces a new ExternalServiceResource type that allows developers to model external services (APIs, databases, etc.) in their Aspire applications with service discovery support.

Problem

Previously, developers had to create custom implementations to represent external services in their Aspire applications:

public class ResourceWithServiceDiscovery : IResourceWithServiceDiscovery
{
    public required string Name { get; init; }
    public required ResourceMetadataCollection Annotations { get; init; }
}

var remoteResource = new ResourceWithServiceDiscovery
{
    Name = "someremoteresource",
    Annotations = [
        new AllocatedEndpointAnnotation(
            "http",
            ProtocolType.Tcp,
            remoteHubConfig.GetValue("Address")!,
            remoteHubConfig.GetValue<int>("Port"),
            remoteHubConfig.GetValue<string>("Scheme")!)
    ]
};

Solution

Added ExternalServiceResource and AddExternalService extension methods that provide a clean, built-in way to model external services:

var builder = DistributedApplication.CreateBuilder(args);

// Basic usage with literal URL
var weatherApi = builder.AddExternalService("weatherapi", "https://api.weather.gov/");

// With Uri object
var weatherApi = builder.AddExternalService("weatherapi", new Uri("https://api.weather.gov/"))
    // Support for HTTP health checks
    .WithHttpHealthCheck(path: "/health");

// With parameterized URL
var urlParam = builder.AddParameter("weather-url");
var weatherApi = builder.AddExternalService("weatherapi", urlParam);

// Service discovery integration
builder.AddProject<Projects.MyWeatherApp>("webapp")
    .WithReference(weatherApi)  // Injects service discovery configuration
    .WithExternalHttpEndpoints();

// Environment variable support  
builder.AddProject<Projects.MyWeatherApp>("webapp")
    .WithEnvironment("WEATHER_URL", weatherApi);

Features

  • Multiple overloads for different use cases (string URL, Uri, parameter-based)
  • URL validation at build time with helpful error messages
  • Service discovery integration via WithReference() - works seamlessly with existing service discovery infrastructure
  • Dashboard integration - external services appear in the Aspire dashboard with unique icon, etc.
  • Parameterization support - URLs can be parameterized for different environments

Fixes #2311

Screenshots of external resources in the dashboard

image

image

image

image

image

@DamianEdwards
Copy link
Member

Use the proposal from this comment on the issue: #2311 (comment)

Copilot AI and others added 2 commits June 20, 2025 01:35
Co-authored-by: DamianEdwards <249088+DamianEdwards@users.noreply.github.com>
… support

Co-authored-by: DamianEdwards <249088+DamianEdwards@users.noreply.github.com>
@Copilot Copilot AI changed the title [WIP] Consider providing an external endpoint resource Add ExternalServiceResource for modeling external services with service discovery support Jun 20, 2025
@Copilot Copilot AI requested a review from DamianEdwards June 20, 2025 01:47
Copilot finished work on behalf of DamianEdwards June 20, 2025 01:47
Copy link
Contributor Author

Copilot AI commented Jun 20, 2025

Use the proposal from this comment on the issue: #2311 (comment)

I've updated the implementation to follow your proposal more closely. The key changes:

  1. Created ExternalServiceParameterResource for parameter-based configuration
  2. Simplified endpoint handling approach
  3. Added URL validation at build time
  4. Integrated with existing service discovery infrastructure

The implementation now supports all the overloads from your proposal with proper URL validation. Currently addressing some integration issues with the endpoint system to ensure the URLs resolve correctly.

I'm working on fixing the remaining test failures where the endpoint URLs need to preserve the original external service URL format rather than the standard scheme://host:port format used by the endpoint system.

…t integration

Co-authored-by: DamianEdwards <249088+DamianEdwards@users.noreply.github.com>
Copilot finished work on behalf of DamianEdwards June 20, 2025 02:11
@wertzui
Copy link

wertzui commented Jun 20, 2025

Does this solution also work with health checks?
It would be good to be able to add health checks here so one can see the status of the external resource in the aspire dashboard.

@DamianEdwards
Copy link
Member

@wertzui that's a really good point.

Updated GetPortFromUrl method to use scheme-appropriate default ports for parameterized expressions:
- HTTPS: 443 (was hardcoded 80)
- HTTP: 80
- FTP: 21
- WebSocket (ws): 80
- Secure WebSocket (wss): 443
- Unknown schemes: 80 (fallback)

Addresses @DamianEdwards' comment about considering URL scheme for default port selection.

Co-authored-by: DamianEdwards <249088+DamianEdwards@users.noreply.github.com>
@DamianEdwards
Copy link
Member

@copilot, please try again

…, and update tests

Co-authored-by: DamianEdwards <249088+DamianEdwards@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Jun 20, 2025

@copilot, please try again

I've addressed all the code review feedback:

  1. Added HTTP health checks - External services now register HTTP health checks so they must be healthy before getting running state
  2. Fixed endpoint naming - Removed explicit "default" name, endpoints now default to scheme name ("https", "http")
  3. Updated tests - All tests now work with scheme-based endpoint names and the correct URL formats

All external service tests are now passing. The implementation follows the proposal and integrates cleanly with the existing Aspire infrastructure.

Fixed in commit b28113b.

Copilot finished work on behalf of DamianEdwards June 20, 2025 22:27
- Removed 44 Bootstrap CSS/JS files from playground/ExternalServices/WebFrontEnd/wwwroot/lib/
- Updated App.razor to use CDN links instead of local Bootstrap files
- Fixed YARP integration to work with new ExternalServiceResource.UrlExpression property
- Fixed ExternalServices playground AppHost to use correct AddExternalService syntax

This reduces the PR diff from +61,210 lines to a much more reasonable size by eliminating vendor CSS/JS files.

Co-authored-by: JamesNK <303201+JamesNK@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Jul 1, 2025

image

😱

Fixed the massive diff size! Removed 44 Bootstrap vendor files (60,000+ lines) from the playground project and switched to CDN links instead. The diff is now much more reasonable. Fixed in commit 71175bd.

@Copilot Copilot AI requested a review from JamesNK July 1, 2025 00:24
Copilot finished work on behalf of JamesNK July 1, 2025 00:24
@DamianEdwards
Copy link
Member

@copilot revert your changes from 56848cb. The PR was already up to date with the required implementation. Ensure the tests you added in e579d05 are updated appropriately.

Co-authored-by: DamianEdwards <249088+DamianEdwards@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Jul 1, 2025

@copilot revert your changes from 56848cb. The PR was already up to date with the required implementation. Ensure the tests you...

I've reverted the changes from commit 56848cb and updated the tests to work with the restored implementation. The ExternalServiceResource now uses the original Uri/UrlParameter approach instead of UrlExpression. Build passes successfully. Fixed in commit 6392b16.

Copilot finished work on behalf of DamianEdwards July 1, 2025 15:33
@DamianEdwards DamianEdwards marked this pull request as ready for review July 1, 2025 20:22
@DamianEdwards DamianEdwards requested a review from mitchdenny as a code owner July 1, 2025 20:22
@DamianEdwards DamianEdwards requested a review from davidfowl July 1, 2025 20:22
Copy link
Member

@JamesNK JamesNK left a comment

Choose a reason for hiding this comment

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

Dashboard changes are good.

I took a look at the app host stuff but I'm less fimilar with adding new resources. I'm curious whether extenal service should implement IResourceWithEndpoints? I don't know what features that interface enables.

@DamianEdwards
Copy link
Member

@JamesNK we decided against endpoints as they don't fit very ergonomically for external addresses where nothing needs to be allocated by Aspire/DCP. Instead we have specific overloads of WithReference and WithEnvironment for ExternalServiceResource, as does the Yarp config API.

@davidfowl davidfowl merged commit 3ec9e5a into main Jul 2, 2025
252 checks passed
@davidfowl davidfowl deleted the copilot/fix-2311 branch July 2, 2025 08:52
@davidfowl
Copy link
Member

@DamianEdwards some follow ups for you. Logging logs for *every instance of the same parameter. It's nosy, if you want to log then we need to find a better place to do it.

I spent some time looking at how to make yarp work and we can do it once @benjaminpetit fixes the YARP publish story

@davidfowl
Copy link
Member

cc @paulomorgado

Copy link
Member

@mitchdenny mitchdenny left a comment

Choose a reason for hiding this comment

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

This is looking good but I'm having trouble getting it to deploy cleanly to Docker Compose (just trying to figure out if this PR breaks something or something is broken in main.

@mitchdenny
Copy link
Member

@davidfowl what issue did you have with Yarp publishing? I managed to get it running locally on Windows with Docker Compose up?

@mitchdenny
Copy link
Member

Pleased to report that the issue I was having on the branch resolved itself once this was merged onto main (yay?)

@davidfowl
Copy link
Member

Container files don’t get resolved outside of docker compose scenarios and there are issues with URLs as parameters in yarp.

We’re switching to env variables so it works everywhere

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

Error loading sessions

Retrying...

Successfully merging this pull request may close these issues.

Consider providing an external endpoint resource
6 participants