Add optional `serviceName` parameter to `WithReference` methods

[paulomorgado]

Updated the WithReference methods in ResourceBuilderExtensions.cs to include an optional serviceName parameter for overriding the source resource's name for the service name. The ApplyEndpoints and CreateEndpointReferenceEnvironmentPopulationCallback methods have been updated to use this new parameter. Updated and added tests in WithReferenceTests.cs to cover these changes. The PublicAPI.Unshipped.txt file has also been updated to reflect these changes.

Closes #4109

Microsoft Reviewers: Open in CodeFlow

[davidfowl]

Can't make breaking changes like this now. Add an overload.

[paulomorgado]

Can't make breaking changes like this now. Add an overload.

Done!

[davidfowl]

@ReubenBond can you take a look?

[mitchdenny]

@ReubenBond and @davidfowl I'm thinking that a better pattern here might be to do something like this:

public static IResourceBuilder<TDestination> WithReference<TDestination>(this IResourceBuilder<TDestination> builder, IResourceBuilder<IResourceWithServiceDiscovery> source, ServiceDiscoveryOptions options)

[paulomorgado]

@ReubenBond and @davidfowl I'm thinking that a better pattern here might be to do something like this:

public static IResourceBuilder<TDestination> WithReference<TDestination>(this IResourceBuilder<TDestination> builder, IResourceBuilder<IResourceWithServiceDiscovery> source, ServiceDiscoveryOptions options)

Does ServiceDiscoveryOptions have a service name property?

[mitchdenny]

Does ServiceDiscoveryOptions have a service name property?

Yes. That would be the idea. We should also consider making it Action<ServiceDiscoveryOptions>

[paulomorgado]

Does ServiceDiscoveryOptions have a service name property?

Yes. That would be the idea. We should also consider making it Action<ServiceDiscoveryOptions>

@ReubenBond and @davidfowl, would this be the way to go?

[paulomorgado]

Just found out that this implementation fails to add the same endpoint with different service names, which should be possible, by design.

@mitchdenny, would Action<ServiceDiscoveryOptions> make it possible to have distinct (endpoint, service name)?

[mitchdenny]

Just found out that this implementation fails to add the same endpoint with different service names, which should be possible, by design.

@mitchdenny, would Action<ServiceDiscoveryOptions> make it possible to have distinct (endpoint, service name)?

I'm not sure I'm following?

[paulomorgado]

Just found out that this implementation fails to add the same endpoint with different service names, which should be possible, by design. @mitchdenny, would Action<ServiceDiscoveryOptions> make it possible to have distinct (endpoint, service name)?

I'm not sure I'm following?

Currently, WithReference for IResourceBuilder<IResourceWithServiceDiscovery> and EndpointReference allows only one reference for the same endpoint.

If I'm specifying the service name, I should be able to reference the same endpoint more than once, providing the service name is not the same.

[mitchdenny]

Can you give me a scenario where you would want this? I get why you might want to rename the service name. But renaming it and keeping the existing one, or registering it twice?

[paulomorgado]

Can you give me a scenario where you would want this? I get why you might want to rename the service name. But renaming it and keeping the existing one, or registering it twice?

I have a project that uses more than one identity providers to authenticate requests received and sent.

To not have to spin up as many keycloack containers, I'm spinning up just one with multiple realms, and they all have the same hostname and port.

Or, is there a way to declare more than one port for a container? That would solve the issue.

[mitchdenny]

Yeah you can map multiple endpoints to the same container port:

var builder = DistributedApplication.CreateBuilder(args);
var grafana = builder.AddContainer("grafana", "grafana/grafana")
                     .WithEndpoint(targetPort: 3000, scheme: "http", name: "ep1")
                     .WithEndpoint(targetPort: 3000, scheme: "http", name: "ep2")
                     .WithEndpoint(targetPort: 3000, scheme: "http", name: "ep3")
                     .WithEndpoint(targetPort: 3000, scheme: "http", name: "ep4");

builder.AddProject<Projects.MyApp>("webfrontend")
    .WithExternalHttpEndpoints()
    .WithEnvironment("EP1", grafana.GetEndpoint("ep1"))
    .WithEnvironment("EP2", grafana.GetEndpoint("ep2"))
    .WithEnvironment("EP3", grafana.GetEndpoint("ep3"))
    .WithEnvironment("EP4", grafana.GetEndpoint("ep4"));

builder.Build().Run();

[mitchdenny]

Note these aren't setting service discovery variables, just regular environment variables. But you can set variables that match the service discovery format.

It might make sense to have an overload for WithReference(...) where you can take an endpoint and provide a service name for local testing scenarios like this.

[paulomorgado]

It might make sense to have an overload for WithReference(...) where you can take an endpoint and provide a service name for local testing scenarios like this.

That's the subject of this PR. Isn't it?

[paulomorgado]

@mitchdenny, did some tests with adding more than one endpoint to the project by naming them, but the problem now is that it fails to resolve https+http schemes because the endpoint is registered in service discovery as its name instead of http or https.

I could use $"services__{serviceName}__{endpoint.Scheme}__0" instead of $"services__{serviceName}__{endpointName}__0", but that should have an option to control that.

[davidfowl]

@paulomorgado do you still want to work on this PR? Should we go back to the issue and try to resolve the design challenges you found? I feel like we could do with a reset here.

[paulomorgado]

@paulomorgado do you still want to work on this PR? Should we go back to the issue and try to resolve the design challenges you found? I feel like we could do with a reset here.

@davidfowl, yes, I still want to work on this PR!. But after more than 6 months, I'll have to go over it all to recall what those challenges were. What do you mean by reset?

[davidfowl]

What do you mean by reset?

Look at the APIs and the scenarios you are trying to accomplish (with code samples), given this implementation to see if this is the right approach.

[dotnet-policy-service[bot]]

This submission has been automatically marked as stale because it has been marked as requiring author action but has not had any activity for 14 days. It will be closed if no further activity occurs within 7 days of this comment.