Upgrade HTTP endpoints to HTTPS in Azure Container Apps#14267
Upgrade HTTP endpoints to HTTPS in Azure Container Apps#14267
Conversation
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 14267Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 14267" |
- HTTP endpoints are now upgraded to HTTPS:443 by default - Explicit dev ports (e.g., 8080) are treated as dev-only hints - Add WithHttpsUpgrade(false) to opt out at the environment level - Removes NotSupportedException throws for port validation
cdd10b4 to
cec509a
Compare
There was a problem hiding this comment.
Pull request overview
This PR addresses issues with HTTP endpoint handling in Azure Container Apps by automatically upgrading HTTP endpoints to HTTPS (port 443) by default, which prevents WebSocket upgrade failures caused by ACA's HTTP→HTTPS redirect behavior.
Changes:
- HTTP endpoints are now automatically upgraded to HTTPS (port 443) by default when deploying to Azure Container Apps
- Removed
NotSupportedExceptionfor non-standard ports (8080, etc.), replacing with informational logging - Added
WithHttpsUpgrade(false)method to opt out and preserve HTTP scheme when needed
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/Aspire.Hosting.Azure.AppContainers/AzureContainerAppEnvironmentResource.cs | Added internal PreserveHttpEndpoints property to control HTTP→HTTPS upgrade behavior |
| src/Aspire.Hosting.Azure.AppContainers/AzureContainerAppExtensions.cs | Added public WithHttpsUpgrade() extension method to configure HTTP endpoint upgrade behavior |
| src/Aspire.Hosting.Azure.AppContainers/ContainerAppContext.cs | Modified endpoint processing logic to upgrade HTTP to HTTPS by default and log informational messages instead of throwing exceptions |
| tests/Aspire.Hosting.Azure.Tests/AzureContainerAppsTests.cs | Updated tests from expecting exceptions to verifying correct behavior; added test for HTTP preservation opt-out |
| tests/Aspire.Hosting.Azure.Tests/Snapshots/*.bicep | Updated snapshots to reflect HTTP→HTTPS upgrades in generated bicep (scheme, port, and environment variable values) |
Comments suppressed due to low confidence (1)
src/Aspire.Hosting.Azure.AppContainers/AzureContainerAppExtensions.cs:390
- According to the XML Documentation Standards guideline 1000002, public APIs should include practical examples using the
<example>tag. This method should include an example showing how to useWithHttpsUpgrade(false)to preserve HTTP endpoints, similar to how other public extension methods in the codebase provide usage examples (e.g.,PublishAsAzureContainerAppmethods have examples within their<remarks>sections).
/// <summary>
/// Configures whether HTTP endpoints should be upgraded to HTTPS in Azure Container Apps.
/// By default, HTTP endpoints are upgraded to HTTPS for security and WebSocket compatibility.
/// </summary>
/// <param name="builder">The AzureContainerAppEnvironmentResource to configure.</param>
/// <param name="upgrade">Whether to upgrade HTTP endpoints to HTTPS. Default is true.</param>
/// <returns><see cref="IResourceBuilder{T}"/></returns>
/// <remarks>
/// When disabled (<c>false</c>), HTTP endpoints will use HTTP scheme and port 80 in Azure Container Apps.
/// Note that explicit ports specified for development (e.g., port 8080) are still normalized
/// to standard ports (80/443) as required by Azure Container Apps.
/// </remarks>
public static IResourceBuilder<AzureContainerAppEnvironmentResource> WithHttpsUpgrade(this IResourceBuilder<AzureContainerAppEnvironmentResource> builder, bool upgrade = true)
{
builder.Resource.PreserveHttpEndpoints = !upgrade;
return builder;
}
- Log once per environment instead of per endpoint - Shows all affected resources/endpoints in one message - Example: 'HTTP endpoints will use HTTPS: keycloak:http, vault:http, app:http'
🎬 CLI E2E Test RecordingsThe following terminal recordings are available for commit
📹 Recordings uploaded automatically from CI run #21540816187 |
|
Question about API granularity The For example: // Current: environment-level only
builder.AddAzureContainerAppEnvironment("env")
.WithHttpsUpgrade(false);
// Proposed addition: per-app override
builder.AddContainer("legacy-api", "myimage")
.WithHttpEndpoint()
.PublishAsContainerApp(options =>
{
options.PreserveHttpEndpoints = true;
});This would allow the environment to default to HTTPS upgrade while letting specific apps opt out individually. The precedence would be: per-app setting > environment setting > default (upgrade=true). Is this level of granularity needed, or is environment-level sufficient for the intended use cases? |
|
Yea I went back and forth on this but you can always use PublishAsContainerApp today to override everything and set ingress data at the bicep level. That's your escape hatch here, I don't think we need resource level logic. |
|
/deployment-test |
|
🚀 Starting deployment tests on PR #14267... This will deploy to real Azure infrastructure. Results will be posted here when complete. |
Description
This PR changes how HTTP endpoints are handled when deploying to Azure Container Apps:
Problem:
NotSupportedExceptionat deploy time, even though they work fine for local devSolution:
NotSupportedExceptionthrows - instead logs informational messagesWithHttpsUpgrade(false)on the environment to opt out and preserve HTTP schemeUsage:
Checklist
<remarks />and<code />elements on your triple slash comments?