Skip to content
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

Do not set TargetPort to Port when IsProxying is also set #3372

Merged
merged 5 commits into from
Apr 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion src/Aspire.Hosting/ApplicationModel/EndpointAnnotation.cs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,16 @@ public EndpointAnnotation(ProtocolType protocol, string? uriScheme = null, strin
_transport = transport;
Name = name;
Port = port;
TargetPort = targetPort ?? port;

TargetPort = targetPort;

// 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)
{
TargetPort = port;
}

IsExternal = isExternal ?? false;
IsProxied = isProxied;
}
Expand Down
13 changes: 12 additions & 1 deletion src/Aspire.Hosting/Dcp/ApplicationExecutor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1692,7 +1692,7 @@ private void AddServicesProducedInfo(IResource modelResource, IAnnotationHolder
{
if (sp.EndpointAnnotation.TargetPort is null)
{
throw new InvalidOperationException($"The endpoint for container resource {modelResourceName} must specify the ContainerPort");
throw new InvalidOperationException($"The endpoint for container resource '{modelResourceName}' must specify the TargetPort");
}

sp.DcpServiceProducerAnnotation.Port = sp.EndpointAnnotation.TargetPort;
Expand All @@ -1708,6 +1708,17 @@ private void AddServicesProducedInfo(IResource modelResource, IAnnotationHolder
// so we need to specify what the port is so DCP is aware of it.
sp.DcpServiceProducerAnnotation.Port = sp.EndpointAnnotation.Port;
}
else
{
Debug.Assert(sp.EndpointAnnotation.IsProxied);

var ep = sp.EndpointAnnotation;
if (ep.TargetPort is int && ep.Port is int && ep.TargetPort == ep.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.");
}
}

dcpResource.AnnotateAsObjectList(CustomResource.ServiceProducerAnnotation, sp.DcpServiceProducerAnnotation);
appResource.ServicesProduced.Add(sp);
Expand Down
11 changes: 7 additions & 4 deletions src/Aspire.Hosting/Publishing/ManifestPublishingContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -278,14 +278,17 @@ public void WriteBindings(IResource resource)
Writer.WriteString("protocol", endpoint.Protocol.ToString().ToLowerInvariant());
Writer.WriteString("transport", endpoint.Transport);

int? targetPort = (resource, endpoint.UriScheme, endpoint.TargetPort) switch
int? targetPort = (resource, endpoint.UriScheme, endpoint.TargetPort, endpoint.Port) switch
{
// The port was specified so use it
(_, _, int port) => port,
(_, _, int target, _) => target,

// Project resources get their default port from the deployment tool
// Container resources get their default listening port from the exposed port.
(ContainerResource, _, null, int port) => port,

// Project resources get their default listening port from the deployment tool
// ideally we would default to a known port but we don't know it at this point
(ProjectResource, var scheme, null) when scheme is "http" or "https" => null,
(ProjectResource, var scheme, null, _) when scheme is "http" or "https" => null,

// Allocate a dynamic port
_ => PortAllocator.AllocatePort()
Expand Down