Repository navigation
[Container Apps] Fix #10103: az containerapp up: Fail closed when app lookup fails - #10519
Conversation
…hen app lookup fails Root cause: In src/containerapp/azext_containerapp/_up_utils.py, ContainerApp inherited Resource.get, which swallowed every lookup exception and reported absence. In src/containerapp/azext_containerapp/custom.py, containerapp_up_logic independently swallowed every show failure and selected create. ContainerApp.create also used a suppressing core existence helper solely for logging. This allowed a transient ARM lookup failure to select full-spec creation against an existing app. Image-specific ingress defaults were applied before knowing whether the app existed. The restored candidate already fixes these paths; this repair additionally explains its arguments-differ suppression … Change: Require explicit HTTP 404 for app absence during preflight and final create/update selection; propagate uncertain errors unchanged and reject empty successful responses. Preserve successful-show updates and defer quickstart/foundry ingress defaults until confirmed creation. Remove the logging-only suppressing existence probe and retain cloud-build placeholder behavior. Add focused unit and live scenario coverage, patch release 1.3.1b1, and explain the existing app-specific create signature suppression. Origin: Supplied history attributes containerapp_up_logic's create/update branch to 2f16dd4 (PR Azure#4837, 2022-05-23) and the Resource/ContainerApp wrappers to cf4bdeb (PR Azure#4726, 2022-04-29). Their intent was to create an absent app or update a present app. The fix preserves that intent but fails closed when state is unknown. No Git metadata exists here … Why now: A transient ARM/runner connectivity failure exposed a latent catch-all branch. The supplied runner logs suggest an infrastructure incident but do not establish its exact cause. No dependency bump has been proven to cause this failure. Blast radius: Only ContainerApp's lookup override changes Resource.get behavior; ContainerAppEnvironment and other Resource subclasses are unchanged. Related commands/flows are az containerapp up with images, source, repository deployments, and cloud-build placeholders. Uncertain app lookups now stop rather than provisioning; explicit 404 creation and successful-show updates remain. Fixing only the final decision would leave … Alternatives considered: - Changing Resource.get globally was rejected because it would alter unrelated resource wrappers. - Fixing only containerapp_up_logic was rejected because earlier preflight could still provision after an uncertain lookup. Verified: - Trusted validation passed (1 command). - New or changed tests fail on the base revision and pass with the change: `src/containerapp/azext_containerapp/tests/test_containerapp_up_unit.py::ContainerAppUpCommandUnitTest::test_app_appearing_after_preflight_updates_without_defaults`, `src/containerapp/azext_containerapp/tests/test_containerapp_up_unit.py::ContainerAppUpCommandUnitTest::test_final_lookup_failure_never_deploys_after_successful_preflight`, `src/containerapp/azext_containerapp/tests/test_containerapp_up_unit.py::ContainerAppUpCommandUnitTest::test_preflight_failure_stops_before_provisioning`. - Unit tests: 17 executed, 0 skipped. - flake8 and pylint passed. - Pre-publication gates passed: release-artifact, generated-ownership, command-help, suppression, regression-test, independent-review. - Scenario recordings must be refreshed by a live test run. - Repository CI and live tests run on this PR. Fixes Azure#10103
01f9710 to
2b7db3c
Compare
|
Automated repair stopped Automated repair stopped after 3 attemptsReviewThis PR is still failing after three automated attempts. The fail-closed lookup handling addresses ambiguous lookup failures and restricts implicit ingress defaults to creation, but regression validation remains incomplete. Upstream CIAzure.azure-cli-extensions, build 365639 failed, including these checks:
Relevance: PR-related for the newly added test; other failures remain uncertain. Each job quotes Test validation
Agent live-test relevance: uncertain. The supplied evidence is only "the run concluded Confirmed recording gapP2 - Test semantic-strength:
Despite the structured coverage applicability result above, issue-scenario regression coverage is not verified. The scenarios contain meaningful assertions, but absent recordings leave their request/response sequences unverified in playback. Replay all three identically named No additional production defect or unresolved Copilot reviewer feedback was confirmed. Question for maintainersWill the Container Apps owning squad take over repairing the missing scenario recordings and diagnosing the outstanding CI/live-test failures? Risk assessment48/100 · Medium The Medium rating is driven by public CLI behavior, dependency or supply chain, failure-handling behavior.
Posted by x-engineering-agent (Reviewer) Live test — FailedFAIL (exit 1) Selectors: Changed test files runWorkflow run: https://github.com/Azure/issue-sentinel/actions/runs/40870107709 Live-test recordings: 2 regenerated — archived in workflow artifact
Recording filesLast 80 lines of azdev output
Ask the Agent Tick a box to queue an action. Requires write access to this repository.
|
Live reproduction on the PR head is inconclusiveThe issue was reproduced live before this change, but the re-run on head This PR stays a draft until a maintainer verifies the fix (or a new push re-runs the reproduction). Run: https://github.com/Azure/issue-sentinel/actions/runs/40210011398 |
|
Container Apps |
🤖 PR Validation —⚠️ Review suggested
This PR adds warning or check suppressions. Fix the underlying warning where possible and justify each remaining suppression in a code comment or the PR description.
1 added suppression
src/containerapp/azext_containerapp/_up_utils.pydef create(self, no_registry=False, apply_image_defaults=True): # pylint: disable=arguments-differDescription
Fixes #10103.
Fail closed when app lookup fails.
Root cause: In src/containerapp/azext_containerapp/_up_utils.py, ContainerApp inherited Resource.get, which swallowed every lookup exception and reported absence. In src/containerapp/azext_containerapp/custom.py, containerapp_up_logic independently swallowed every show failure and selected create. ContainerApp.create also used a suppressing core existence helper solely for logging. This allowed a transient ARM lookup failure to select full-spec creation against an existing app. Image-specific ingress defaults were applied before knowing whether the app existed. The restored candidate already fixes these paths; this repair additionally explains its arguments-differ suppression …
Fix: Require explicit HTTP 404 for app absence during preflight and final create/update selection; propagate uncertain errors unchanged and reject empty successful responses. Preserve successful-show updates and defer quickstart/foundry ingress defaults until confirmed creation. Remove the logging-only suppressing existence probe and retain cloud-build placeholder behavior. Add focused unit and live scenario coverage, patch release 1.3.1b1, and explain the existing app-specific create signature suppression.
Origin: Supplied history attributes containerapp_up_logic's create/update branch to 2f16dd4 (PR #4837, 2022-05-23) and the Resource/ContainerApp wrappers to cf4bdeb (PR #4726, 2022-04-29). Their intent was to create an absent app or update a present app. The fix preserves that intent but fails closed when state is unknown. No Git metadata exists here …
Why now: A transient ARM/runner connectivity failure exposed a latent catch-all branch. The supplied runner logs suggest an infrastructure incident but do not establish its exact cause. No dependency bump has been proven to cause this failure.
Blast radius: Only ContainerApp's lookup override changes Resource.get behavior; ContainerAppEnvironment and other Resource subclasses are unchanged. Related commands/flows are az containerapp up with images, source, repository deployments, and cloud-build placeholders. Uncertain app lookups now stop rather than provisioning; explicit 404 creation and successful-show updates remain. Fixing only the final decision would leave …
How verified (trusted validation output):
src/containerapp/azext_containerapp/tests/test_containerapp_up_unit.py::ContainerAppUpCommandUnitTest::test_app_appearing_after_preflight_updates_without_defaults,src/containerapp/azext_containerapp/tests/test_containerapp_up_unit.py::ContainerAppUpCommandUnitTest::test_final_lookup_failure_never_deploys_after_successful_preflight,src/containerapp/azext_containerapp/tests/test_containerapp_up_unit.py::ContainerAppUpCommandUnitTest::test_preflight_failure_stops_before_provisioning.Reproduction run: https://github.com/Azure/issue-sentinel/actions/runs/40193867782
Risks and limitations (from the job report):
This checklist is used to make sure that common guidelines for a pull request are followed.
Related command
az containerapp upGeneral Guidelines
azdev style <YOUR_EXT>locally? (pip install azdevrequired)python scripts/ci/test_index.py -qlocally? (pip install azdevrequired)For new extensions:
About Extension Publish
There is a pipeline to automatically build, upload and publish extension wheels.
Once your pull request is merged into main branch, a new pull request will be created to update
src/index.jsonautomatically.You only need to update the version information in file setup.py and historical information in file HISTORY.rst in your PR but do not modify
src/index.json.Independent review: an independent reviewer checked this change against 11 acceptance criteria derived from the issue (11 required); every required criterion is backed by evidence in the diff.