Tests and improvements for endpoint handling (dotnet#3611)

src/Aspire.Hosting/ApplicationModel/EndpointAnnotation.cs

@@ -44,18 +44,28 @@ public EndpointAnnotation(ProtocolType protocol, string? uriScheme = null, strin
  _transport = transport;
  Name = name;
  Port = port;
-
  TargetPort = targetPort;
+ IsExternal = isExternal ?? false;
+ IsProxied = isProxied;
 
- // If the target port was not explicitly set and the service is not being proxied,
- // we can set the target port to the port.
- if (TargetPort is null && !isProxied)
+ if (!isProxied)
  {
- TargetPort = port;
+ // For proxy-less Endpoints the client port and target port should be the same.
+ // Note that this is just a "sensible default"--the consumer of the EndpointAnnotation is free
+ // to change Port and TargetPort after the annotation is created, but if the final values are inconsistent,
+ // the associated resource may fail to run.
+ // It also depends on what the EndpointAnnotation is applied to.
+ // In the Container case the TargetPort is the port that the process listens on inside the container,
+ // and the Port is the host interface port, so it is fine for them to be different.
+ if (port is null && targetPort is not null)
+ {
+ Port = targetPort;
+ }
+ if (port is not null && targetPort is null)
+ {
+ TargetPort = port;
+ }
  }
-
- IsExternal = isExternal ?? false;
- IsProxied = isProxied;
  }
 
  /// <summary>

src/Aspire.Hosting/Dcp/ApplicationExecutor.cs

@@ -41,7 +41,6 @@ internal sealed class ServiceAppResource : AppResource
 {
  public Service Service => (Service)DcpResource;
  public EndpointAnnotation EndpointAnnotation { get; }
- public ServiceProducerAnnotation DcpServiceProducerAnnotation { get; }
 
  public override List<ServiceAppResource> ServicesProduced
  {
@@ -55,7 +54,6 @@ public override List<ServiceAppResource> ServicesConsumed
  public ServiceAppResource(IResource modelResource, Service service, EndpointAnnotation sba) : base(modelResource, service)
  {
  EndpointAnnotation = sba;
- DcpServiceProducerAnnotation = new(service.Metadata.Name);
  }
 }
 
@@ -1423,20 +1421,16 @@ private async Task CreateContainerAsync(AppResource cr, ILogger resourceLogger,
 
  foreach (var sp in cr.ServicesProduced)
  {
+ var ea = sp.EndpointAnnotation;
+
  var portSpec = new ContainerPortSpec()
  {
- ContainerPort = sp.DcpServiceProducerAnnotation.Port,
+ ContainerPort = ea.TargetPort,
  };
 
- if (!sp.EndpointAnnotation.IsProxied)
+ if (!ea.IsProxied && ea.Port is int)
  {
- // When DCP isn't proxying the container we need to set the host port that the containers internal port will be mapped to
- portSpec.HostPort = sp.EndpointAnnotation.Port;
- }
-
- if (!string.IsNullOrEmpty(sp.DcpServiceProducerAnnotation.Address))
- {
- portSpec.HostIP = sp.DcpServiceProducerAnnotation.Address;
+ portSpec.HostPort = ea.Port;
  }
 
  switch (sp.EndpointAnnotation.Protocol)
@@ -1583,43 +1577,48 @@ private void AddServicesProducedInfo(IResource modelResource, IAnnotationHolder
  var servicesProduced = _appResources.OfType<ServiceAppResource>().Where(r => r.ModelResource == modelResource);
  foreach (var sp in servicesProduced)
  {
- // Projects/Executables have their ports auto-allocated; the port specified by the EndpointAnnotation
- // is applied to the Service objects and used by clients.
- // Containers use the port from the EndpointAnnotation directly.
+ var ea = sp.EndpointAnnotation;
 
  if (modelResource.IsContainer())
  {
- if (sp.EndpointAnnotation.TargetPort is null)
+ if (ea.TargetPort is null)
  {
- throw new InvalidOperationException($"The endpoint for container resource '{modelResourceName}' must specify the TargetPort");
+ throw new InvalidOperationException($"The endpoint '{ea.Name}' for container resource '{modelResourceName}' must specify the {nameof(EndpointAnnotation.TargetPort)} value");
  }
-
- sp.DcpServiceProducerAnnotation.Port = sp.EndpointAnnotation.TargetPort;
  }
- else if (!sp.EndpointAnnotation.IsProxied)
+ else if (!ea.IsProxied)
  {
  if (appResource.DcpResource is ExecutableReplicaSet ers && ers.Spec.Replicas > 1)
  {
- throw new InvalidOperationException($"'{modelResourceName}' specifies multiple replicas and at least one proxyless endpoint. These features do not work together.");
+ throw new InvalidOperationException($"Resource '{modelResourceName}' uses multiple replicas and a proxy-less endpoint '{ea.Name}'. These features do not work together.");
  }
 
- // DCP will not allocate a port for this proxyless service
- // so we need to specify what the port is so DCP is aware of it.
- sp.DcpServiceProducerAnnotation.Port = sp.EndpointAnnotation.Port;
+ if (ea.Port is int && ea.Port != ea.TargetPort)
+ {
+ throw new InvalidOperationException($"The endpoint '{ea.Name}' for resource '{modelResourceName}' is not using a proxy, and it has a value of {nameof(EndpointAnnotation.Port)} property that is different from the value of {nameof(EndpointAnnotation.TargetPort)} property. For proxy-less endpoints they must match.");
+ }
  }
  else
  {
- Debug.Assert(sp.EndpointAnnotation.IsProxied);
+
+ Debug.Assert(ea.IsProxied);
 
- var ep = sp.EndpointAnnotation;
- if (ep.TargetPort is int && ep.Port is int && ep.TargetPort == ep.Port)
+ if (ea.TargetPort is int && ea.Port is int && ea.TargetPort == ea.Port)
  {
  throw new InvalidOperationException(
- $"The endpoint for non-container resource '{modelResourceName}' requested a proxy ({nameof(ep.IsProxied)} is true). Non-container resources cannot be proxied when both {nameof(ep.TargetPort)} and {nameof(ep.Port)} are specified with the same value.");
+ $"The endpoint '{ea.Name}' for resource '{modelResourceName}' requested a proxy ({nameof(ea.IsProxied)} is true). Non-container resources cannot be proxied when both {nameof(ea.TargetPort)} and {nameof(ea.Port)} are specified with the same value.");
  }
+
+ if (appResource.DcpResource is ExecutableReplicaSet && ea.TargetPort is int)
+ {
+ throw new InvalidOperationException(
+ $"Resource '{modelResourceName}' can have multiple replicas, and it uses endpoint '{ea.Name}' that has {nameof(ea.TargetPort)} property set. Each replica must have a unique port; setting {nameof(ea.TargetPort)} is not allowed.");
+ } 
  }
 
- dcpResource.AnnotateAsObjectList(CustomResource.ServiceProducerAnnotation, sp.DcpServiceProducerAnnotation);
+ var spAnn = new ServiceProducerAnnotation(sp.Service.Metadata.Name);
+ spAnn.Port = ea.TargetPort;
+ dcpResource.AnnotateAsObjectList(CustomResource.ServiceProducerAnnotation, spAnn);
  appResource.ServicesProduced.Add(sp);
  }
  }

src/Aspire.Hosting/Dcp/Model/ExecutableReplicaSet.cs

@@ -3,6 +3,7 @@
 
 namespace Aspire.Hosting.Dcp.Model;
 
+using System.Diagnostics.CodeAnalysis;
 using System.Text.Json.Serialization;
 using k8s.Models;
 
@@ -39,6 +40,11 @@ public void AnnotateAsObjectList<TValue>(string annotationName, TValue value)
 
  CustomResource.AnnotateAsObjectList(Annotations, annotationName, value);
  }
+
+ public bool TryGetAnnotationAsObjectList<TValue>(string annotationName, [NotNullWhen(true)] out List<TValue>? list)
+ {
+ return CustomResource.TryGetAnnotationAsObjectList(Annotations, annotationName, out list);
+ }
 }
 
 internal sealed class ExecutableReplicaSetSpec

src/Aspire.Hosting/Dcp/Model/ModelCommon.cs

@@ -53,16 +53,21 @@ public void AnnotateAsObjectList<TValue>(string annotationName, TValue value)
  }
 
  public bool TryGetAnnotationAsObjectList<TValue>(string annotationName, [NotNullWhen(true)] out List<TValue>? list)
+ {
+ return TryGetAnnotationAsObjectList<TValue>(Metadata.Annotations, annotationName, out list);
+ }
+
+ internal static bool TryGetAnnotationAsObjectList<TValue>(IDictionary<string, string>? annotations, string annotationName, [NotNullWhen(true)] out List<TValue>? list)
  {
  list = null;
 
- if (Metadata.Annotations is null)
+ if (annotations is null)
  {
  return false;
  }
 
  string? annotationValue;
- bool found = Metadata.Annotations.TryGetValue(annotationName, out annotationValue);
+ bool found = annotations.TryGetValue(annotationName, out annotationValue);
  if (!found || string.IsNullOrWhiteSpace(annotationValue))
  {
  return false;