[App Service] Fix #29403, #28722, #27950, #30357, #18697: SSL cert pagination, bind, and --wait flag#33058
Open
[App Service] Fix #29403, #28722, #27950, #30357, #18697: SSL cert pagination, bind, and --wait flag#33058
--wait flag#33058Conversation
…ificate pagination The list_by_resource_group() results for certificates were not being fully paginated. Wrapped the SDK pager calls in list() to ensure all pages are consumed, matching the pattern used by other list operations in the codebase. This fixes: - ssl list showing incomplete results - ssl delete failing with "thumbprint not found" for certs beyond first page - ssl binding operations missing certificates Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
seligj95
requested review from
NoriZC,
teresaritorto and
yanzhudd
as code owners
❌AzureCLI-FullTest
|
|
Hi @seligj95, |
|
| rule | cmd_name | rule_message | suggest_message |
|---|---|---|---|
| functionapp config ssl create | cmd functionapp config ssl create added parameter wait |
||
| webapp config ssl create | cmd webapp config ssl create added parameter wait |
Collaborator
|
Thank you for your contribution! We will review the pull request and get back to you soon. |
microsoft-github-policy-service
bot
requested review from
jsntcy and
zhoxing-ms
|
The git hooks are available for azure-cli and azure-cli-extensions repos. They could help you run required checks before creating the PR. Please sync the latest code with latest dev branch (for azure-cli) or main branch (for azure-cli-extensions). pip install azdev --upgrade
azdev setup -c <your azure-cli repo path> -r <your azure-cli-extensions repo path>
|
microsoft-github-policy-service
bot
requested review from
wangzelin007 and
yonzhan
microsoft-github-policy-service
bot
added
Web Apps
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes App Service SSL certificate pagination for az webapp config ssl list/delete by ensuring certificate list operations fully consume the SDK pageable results, preventing incomplete listings and “thumbprint not found” failures when the target cert is beyond the first page.
Changes:
- Materialize
certificates.list_by_resource_group(...)results to a concrete list inlist_ssl_certs,delete_ssl_cert, and_update_ssl_binding. - Add unit tests intended to cover pagination scenarios for list/delete and the not-found error path.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/azure-cli/azure/cli/command_modules/appservice/custom.py |
Forces consumption of certificate pageable results in list/delete/bind flows to avoid first-page-only behavior. |
src/azure-cli/azure/cli/command_modules/appservice/tests/latest/test_webapp_commands_thru_mock.py |
Adds unit tests targeting the SSL cert pagination regression scenarios. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...zure-cli/azure/cli/command_modules/appservice/tests/latest/test_webapp_commands_thru_mock.py
Show resolved
Hide resolved
…ve test pagination mocks - Add break statements in _update_ssl_binding cert search loops for early termination once a matching thumbprint is found - Replace plain iter() test mocks with _FakePagedIterator that simulates real SDK multi-page behavior and tracks pages fetched - Tests now assert pages_fetched to verify full pagination is exercised - Keep list() materialization pattern consistent with codebase convention (web_apps.list_by_resource_group, app_service_plans.list_by_resource_group, etc.) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…Site object to avoid Azure Policy denial Previously, _update_host_name_ssl_state constructed a minimal Site object with only host_name_ssl_states, location, and tags. When passed to begin_create_or_update, this caused Azure Policy (e.g. 'HTTPS Only must be enabled') to deny the operation because policy-sensitive fields like httpsOnly were missing from the payload. The fix reuses the full existing Site object fetched from Azure, updating only the host_name_ssl_states field. This preserves all policy-sensitive properties (httpsOnly, ftpsState, etc.) in the PUT request. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- In _update_ssl_binding, use get_slot() when slot is provided to avoid sending production Site payload for slot updates (policy/settings issue) - Replace assertIs with property assertions in slot test for robustness Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…wait` flag for managed certificate automation When --wait is set: - Extends polling timeout from 2 minutes to 10 minutes - Raises CLIError on timeout instead of silently returning None - Enables automation scripts to reliably chain ssl bind after ssl create Default behavior (without --wait) is unchanged: 2-minute timeout with warning. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
seligj95
changed the title
az webapp config ssl list/delete: Fix certificate pagination--wait flag
- delete_ssl_cert: iterate pager lazily with early return instead of list() - _update_ssl_binding: iterate pagers lazily with early break instead of list() - _update_ssl_binding: use next() generator expression for subscription search - Fix test_webapp_ssl recording to match get_slot calls for slot bind/unbind - Add test_delete_ssl_cert_early_break_skips_remaining_pages test - Update _FakePagedIterator docstring for early-break behavior Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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.
Consolidated SSL Fixes
This PR consolidates three related SSL fixes for App Service:
Fix #29403, #28722, #27950: SSL certificate pagination
az webapp config ssl listnow iterates all pages of certificatesaz webapp config ssl bind/unbindfind certificates across all pagesFix #30357:
az webapp config ssl bind— use full Site objectSiteobject before callingcreate_or_updateduring SSL bindFix #18697:
az webapp config ssl create— add--waitflag--wait/--no-waitsupport toaz webapp config ssl create--waitis specified, polls until the managed certificate is provisionedTesting
azdev style appservicepasses