feat: support container deployment to App Service#8847
Merged
Conversation
jongio
requested review from
JeffreyCA,
RickWinter,
hemarina and
tg-msft
as code owners
📋 Prioritization NoteThanks for the contribution! The linked issue isn't in the current milestone yet. |
jongio
force-pushed
the
jongio/appservice-container-deploy
branch
from
26277e1 to
a2eb4bc
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Adds first-class container image deployment support to the built-in App Service service target (host: appservice) by treating container artifacts as the deployable unit (instead of zip) and updating the site’s container runtime config via ARM.
Changes:
- Extend App Service service target to pass through container artifacts, publish images to ACR, and deploy by updating
linuxFxVersion/acrUseManagedIdentityCreds. - Add an
azapi.AzureClienthelper to PATCH App Service container configuration. - Expand unit tests for the new container path and regressions in the existing zip path.
Show a summary per file
| File | Description |
|---|---|
| cli/azd/pkg/project/service_target_appservice.go | Adds container-aware Package/Publish/Deploy logic for App Service, including a container-config deploy path. |
| cli/azd/pkg/project/service_target_appservice_test.go | Adds/updates tests for container passthrough packaging and container deploy behavior, plus zip-path regression checks. |
| cli/azd/pkg/azapi/webapp.go | Introduces UpdateAppServiceContainerConfig to PATCH linuxFxVersion and enable ACR MSI creds. |
| cli/azd/pkg/azapi/webapp_test.go | Adds tests covering success/error cases for the new azapi update method. |
Review details
- Files reviewed: 7/7 changed files
- Comments generated: 5
- Review effort level: Low
wbreza
reviewed
Jun 29, 2026
wbreza
left a comment
wbreza
left a comment
Contributor
There was a problem hiding this comment.
Code Review — container deployment to App Service
Solid feature. Reuses ContainerHelper, preserves the zip path, good test coverage, and all 10 prior Copilot comments are addressed. A few items worth a guard or doc note before merge:
High
linuxFxVersionset without OS-type check (pkg/azapi/webapp.go, UpdateAppServiceContainerConfig/slot): both unconditionally setDOCKER|<image>. On a Windows App Service this is accepted by ARM but ignored, so deploy appears to succeed while the container is never applied.isLinuxWebApp()already exists for exactly this distinction — consider verifyingKindcontainslinuxand erroring otherwise.- External-registry images + ACR managed identity (
pkg/project/service_target_appservice.go+webapp.go): Publish() bypasses the push when the image already has a registry, but deploy always setsacrUseManagedIdentityCreds=true. A non-ACR image (e.g.ghcr.io/...) deploys but the Web App can't pull it. Suggest only bypassing for the target ACR and failing fast for non-ACR registries.
Medium
- Schema drops source requirement (
schemas/*/azure.yaml.json):host: appservicenow passes with neitherprojectnorimage, failing late at deploy instead of validation. Suggest requiring a source unless docker/image is set.
Low
NewAppServiceTargetsignature 3→5 params: no compat shim; downstream importers break on upgrade.
Items 1/2 may be intentional (Linux-only/managed-identity scoping) — if so, a fast-fail error or doc note keeps the failure mode clear.
vhvb1989
reviewed
Jun 29, 2026
Member
Author
|
Great point. Agreed that deploy should only mutate the running artifact, not infrastructure config. Fixed in dc026dc. The refactored approach:
This aligns with the containerapp target principle: deploy updates the artifact, IaC owns the configuration. |
hemarina
reviewed
Jun 29, 2026
hemarina
left a comment
hemarina
left a comment
Contributor
There was a problem hiding this comment.
Thanks for adding container support to the App Service target — the zipDeploy/containerDeploy split is clean, and the refactor to the validate-and-error model (per Victor's thread) keeps IaC as the source of truth, which is the right call. Two items:
1. ValidateAppServiceForContainerDeploy has no unit tests
This is a new public method with three distinct failure branches (nil Kind, non-Linux Kind, missing/non-DOCKER| linuxFxVersion) plus user-facing error strings, but webapp_test.go only covers UpdateAppServiceContainerImage/UpdateAppServiceSlotContainerImage. Given the coverage gate, please add a table-driven test exercising each branch (and the happy path).
2. Stale PR description / overview
The PR body (and the auto-generated overview) still say azd "enables acrUseManagedIdentityCreds" and reference UpdateAppServiceContainerConfig. After the pivot, deploy only updates linuxFxVersion via UpdateAppServiceContainerImage. Please refresh the description so reviewers/users aren't misled about what deploy mutates.
vhvb1989
reviewed
Jun 29, 2026
vhvb1989
reviewed
Jun 29, 2026
vhvb1989
approved these changes
Jun 29, 2026
Open
3 tasks
Member
|
📝 Follow-up docs issue created to update the official documentation (Microsoft Learn) for App Service container deployment once this merges: #8864 |
Member
Author
|
@vhvb1989 Addressed both concerns from your last round. Here's the approach for each: 1. env: support (2f61772)
2. Polyglot containerization (2f61772)
// Before:
if serviceConfig.Host.RequiresContainer() && requiresLanguage {
// After:
if (serviceConfig.Host.RequiresContainer() || serviceConfig.Docker.Path != "") && requiresLanguage {This means: "if the host always requires containers (containerapp/aks) OR the user explicitly provided a Dockerfile, wrap the language framework in the docker composite." Key properties:
Does this approach work for you, or do you see concerns with either change? |
Add container image deployment support to the appservice service target. When a service is configured with host: appservice and language: docker (or docker.path is set), azd now pushes the container image to ACR and updates the site's linuxFxVersion and acrUseManagedIdentityCreds, rather than failing with 'no package artifacts found'. The implementation reuses ContainerHelper (shared with the containerapp target) for image build/tag/push, and adds a new UpdateAppServiceContainerConfig method to AzureClient for the ARM update. The existing zip deploy path is unchanged for non-container services. Fixes #1608 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Wire determineDeploymentTargets() into the container deploy path so
AZD_DEPLOY_{SERVICE}_SLOT_NAME and interactive slot selection work for
container deployments (not just zip deploy)
- Add UpdateAppServiceSlotContainerConfig for deploying containers to slots
- Validate empty image name before calling ARM (clear error instead of
sending linuxFxVersion=DOCKER| which ARM rejects)
- Strengthen RequiredExternalTools test to actually call the method and
assert on returned tools
- Validate request payload in UpdateAppServiceContainerConfig test (assert
linuxFxVersion and acrUseManagedIdentityCreds are in the body)
- Add user documentation in docs/reference/azure-yaml-schema.md explaining
the container deployment mode for App Service
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add Test_appServiceTarget_Deploy_ContainerSlotPath that sets AZD_DEPLOY_WEB_SLOT_NAME=staging and verifies the slot PATCH endpoint is called with the container image - Add Test_AzureClient_UpdateAppServiceSlotContainerConfig that asserts the request targets /slots/<name> and includes linuxFxVersion + acrUseManagedIdentityCreds in the body Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ows) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Package() and Publish() now use isContainerDeploy() which checks service configuration (language=docker or docker.path set) in addition to artifact presence. This correctly handles Docker remote build and dotnet-publish docker flows where ContainerHelper.Package() returns no artifacts by design (build + push happen together in the Publish phase). Added regression test for remote-build scenario (empty package artifacts with docker config should be a no-op, not an error). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add 'Currently Linux App Service only' to the container deploy docs - Add Test_appServiceTarget_Publish/ContainerDeploy_RemoteImageShortCircuit that verifies the remote image short-circuit path, IMAGE_NAME persistence, and envManager.Save() call Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Address vhvb1989's design feedback: azd deploy should only mutate the running artifact (container image), not infrastructure configuration. This aligns with how the containerapp target works. Changes: - Rename UpdateAppServiceContainerConfig -> UpdateAppServiceContainerImage (only sets linuxFxVersion, does NOT set acrUseManagedIdentityCreds) - Add ValidateAppServiceForContainerDeploy that checks the site is Linux with an existing DOCKER| linuxFxVersion before deploying. Returns actionable error messages directing users to fix their IaC if the site is not configured for containers. - containerDeploy now validates first, then updates only the image ref - Tests assert acrUseManagedIdentityCreds is NOT in the request body Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Address vhvb1989's feedback on env: and polyglot docker.path: - Block env: in schema for appservice (not yet wired to app settings; silently ignoring is a trap for users migrating from containerapp) - Detect polyglot containerization (non-docker language + docker.path) in Initialize() and return actionable error instead of silently failing (composite docker framework only wraps for containerapp/aks targets) - Update docs to clarify: IaC must configure ACR auth, azd only updates the image ref; polyglot containerization not yet supported - Remove docker.path from RequiredExternalTools and isContainerDeploy (only language: docker triggers the container path) - Update Initialize test to cover polyglot rejection case Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Enable two previously-blocked capabilities: 1. env: support - Apply service-level environment variables as App Settings during container deploy via UpdateAppServiceAppSettings. Merges with existing settings (preserves settings not managed by azd). Matches the containerapp target's behavior of applying env vars at deploy time. 2. Polyglot containerization (e.g., language: python + docker.path) - Add docker.path to the composite framework condition in service_manager.go so the docker wrapper builds a container image from any language source when a Dockerfile is provided. This is a one-line change that only fires when the user explicitly sets docker.path (opt-in, no impact on plain zip deploys). Schema updated: env now allowed for appservice (alongside docker/image). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
jongio
force-pushed
the
jongio/appservice-container-deploy
branch
from
4645015 to
8f7faf4
Compare
vhvb1989
approved these changes
Jun 29, 2026
hemarina
approved these changes
Jun 30, 2026
…eAppServiceAppSettings Add tests for: - ValidateAppServiceForContainerDeploy: valid Linux container (pass), non-Linux (error), no DOCKER| linuxFxVersion (error) - UpdateAppServiceAppSettings: merges new env vars with existing settings, verifies both EXISTING_KEY and NEW_KEY in request body Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Azure Dev CLI Install InstructionsInstall scriptsMacOS/Linux
bash: pwsh: WindowsPowerShell install MSI install Standalone Binary
MSI
Documentationlearn.microsoft.com documentationtitle: Azure Developer CLI reference
|
JeffreyCA
approved these changes
Jun 30, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds container image deployment support to the App Service service target, fixing #1608.
When a service is configured with
host: appserviceandlanguage: docker, azd now:ContainerHelper.Publish()(reusing the same helper as thecontainerapptarget)linuxFxVersionto point to the new imageazd does NOT mutate infrastructure configuration at deploy time. ACR auth (
acrUseManagedIdentityCreds), managed identity assignment, and other site config must be set via IaC (bicep/terraform) and provisioned before deploying. This follows the same principle as thecontainerapptarget: deploy updates the running artifact, IaC owns configuration.Scope and Limitations
linuxFxVersion)language: dockerorimage:only (polyglot containerization via non-docker language + docker.path is not yet supported for App Service; clear error returned)env:blocked in schema (not yet wired to App Settings for container deploy; will be tracked separately)Changes
cli/azd/pkg/azapi/webapp.go:UpdateAppServiceContainerImage(only sets linuxFxVersion),UpdateAppServiceSlotContainerImage,ValidateAppServiceForContainerDeploycli/azd/pkg/project/service_target_appservice.go: Container deploy support in Package/Publish/Deploy with slot support,isContainerDeploy,validateContainerConfig(rejects polyglot)schemas/: Allowdocker/imagefor appservice, blockenv/k8s/apiVersiondocs/reference/azure-yaml-schema.md: Document container deploy mode with limitationscli/azd/CHANGELOG.md: Feature entryDesign Decisions
host: appservicealready resolves to the built-in targetacrUseManagedIdentityCredsat deploy time (per vhvb1989's design feedback)Fixes #1608