Sachim/documentdb gap analysis fixes - #10188
Open
sachim wants to merge 20 commits into
Open
Conversation
Addresses PR review feedback. Adds end to end scenario coverage for mongocluster user, cross region replica (GeoAsyncReplica) and point in time restore, recorded live and verified in playback. A _wait_for_restore_point helper waits (only while recording) for the first backup, and _cmd_retry reissues a mutating command while the service still reports an operation in progress. Fixes _keep_only_args so deregistered arguments are also marked optional. Previously a hidden but still required argument (the base create password on a replica) failed schema validation with a missing required field error, so replica create could not run. Renames the administrator password flag to --password/-p on create and update for consistency with reset-password and restore. Fixes a typo in the user show help text. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8bf5e49a-3c3e-4dfe-851d-1f2790a2852b
Each mongocluster scenario (crud, firewall, user, identity, replica, restore, cmk) is now an independent test with its own recording, so a failure in one does not mask the others and a single scenario can be re-recorded on its own. Adds coverage for managed identity (assign/show/remove) and customer-managed key (CMK) encryption at rest, and replaces the monolithic lifecycle recording. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8bf5e49a-3c3e-4dfe-851d-1f2790a2852b
The generated replica promote command sent {mode, promoteOption} at the
request root, which the service rejects with a schema error, and passed a
null final-result callback to the long-running-operation poller, which
raised a TypeError once the operation completed. The custom.py wrapper now
nests the body under "properties" and supplies a no-op callback, mirroring
how the generated delete handles its empty response. Adds an end-to-end
promote scenario test (source + cross-region replica, forced switchover,
asserts the replica settles into the primary role) plus a helper that waits
out the brief post-operation Updating state.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 8bf5e49a-3c3e-4dfe-851d-1f2790a2852b
Adds a negative test covering client-side validation (invalid --storage-type and --high-availability enum values, invalid --mode, and a missing required --promote-option, all asserted via SystemExit) and service rejections (show/firewall-rule show/user show/replica list against a cluster that does not exist, asserted with expect_failure). This matches the negative-case convention used by the official cosmosdb CLI tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8bf5e49a-3c3e-4dfe-851d-1f2790a2852b
…identity check Resolve mongocluster scenario test review feedback: * Expose all three password aliases (--admin-password, --password, -p) on create, update, reset-password, and restore. Examples show --admin-password. * Exercise the native wait command explicitly: firewall uses create --no-wait then firewall-rule wait --created. Replace the two hand authored pollers with mongocluster wait --custom in the restore and promote tests. * Use 0.0.0.0 firewall ranges instead of a hardcoded test IP. * Validate the user assigned identity in the CMK test through identity show (type, key presence, and principalId). * Fix user command examples to a pattern valid username (alice-entra). Recorded the affected cassettes again (firewall, restore, promote, cmk). All nine scenario tests pass in playback. Style and linter pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8bf5e49a-3c3e-4dfe-851d-1f2790a2852b
…id identifier Address Oliver Towers' PR review on the mongocluster user commands: the users API only supports Microsoft Entra ID principals today, so the generic `user` naming misleads customers about what is provisioned and what identifier to pass. - Rename command group `mongocluster user` -> `mongocluster entra-user`. - Rename the identifier arg `--user-name` -> `--object-id` (keeps `-n`/`--name` aliases); help now states it is the Entra object (client) ID GUID, not a friendly name or UPN. - Make all command/group help Entra-specific; fix "defintion" typo in show. - Update examples to use a GUID object id. - Fix roles singular option in the aaz model (was generating ----role). - Re-record the entra-user and negative scenario tests live; both pass playback. The flat --type wrapper (custom.py) and its command-table registration are rewired to the entra-user name so the flattening survives the rename. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 40a4fbc4-fd62-4bcc-9b7d-6e70f80e114d
Address Oliver Towers' PR review: an entra-user binding grants an already-existing Microsoft Entra principal data-plane access to the cluster (a roles assignment), rather than creating a new user. Rename the commands `entra-user create` -> `assign` and `entra-user delete` -> `remove` (verbs only; group and update/show/list stay), matching the sibling `mongocluster identity assign/remove`. - Regenerated aaz code: _assign.py/_remove.py with classes Assign/Remove. - custom.py/commands.py: the flat --type wrapper is rewired to `entra-user assign`. - Updated README and the scenario test; re-recorded the user cassette live. Pairs with the aaz model change in Azure/aaz. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 40a4fbc4-fd62-4bcc-9b7d-6e70f80e114d
Version bump for the entra-user rename work (user -> entra-user, --object-id, create/delete -> assign/remove, password aliases). Adds the 1.0.0b2 changelog entry. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 40a4fbc4-fd62-4bcc-9b7d-6e70f80e114d
Two release-readiness fixes for the extension: - Mark all command groups and commands as Preview (stage=Preview in the aaz model, regenerated here) so the CLI reference docs show Preview instead of GA. - Rebrand the product name to "Azure DocumentDB": update README, HISTORY, the service_name.json mapping, and the root/mongocluster group help. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 40a4fbc4-fd62-4bcc-9b7d-6e70f80e114d
Regenerate replica list from the aaz model and update the hand written replica create and restore wrappers so the parent cluster and its region use parent-cluster-name and parent-location instead of source-cluster and source-location, per review feedback. Also updates the scenario tests and the README. The ARM property names sourceResourceId and sourceLocation are unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 40a4fbc4-fd62-4bcc-9b7d-6e70f80e114d
- Rename replica/restore source arg to --source-cluster (name-or-ARM-ID); derive replica source location from the source cluster; drop --parent-location - Rename 'replica list' arg to --source-cluster - Add 'replica promote' --source-cluster guard (validate vs replica sourceResourceId) - Add confirmations to 'identity remove' and 'replica promote' - Add 'identity list' command - Rename 'entra-user' group to 'microsoft-entra-user' - Remove 'entra-user update' (unsupported by service) - Update scenario tests for renamed args/commands
|
Hi sachim, |
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
sachim
marked this pull request as draft
August 6, 2026 17:54
|
Automated sensitive-information remediation ran on this pull request.
If a credential was exposed, rotate or revoke it immediately. Detected values are never copied into this comment. ✅ Keep the redaction · ❌ Dispute the redaction GitHub only supports a fixed reaction set, so 👍 represents ✅ and 👎 represents ❌. The bot-created reactions are only poll choices. Posted by agent-assist (autonomous bug-fix pipeline). |
azure-client-tools-agent
Bot
added
the
azure-client-tools-agent
Author
@microsoft-github-policy-service agree company="Microsoft" |
microsoft-github-policy-service
Bot
added
the
customer-reported
Contributor
|
Thank you for your contribution sachim! We will review the pull request and get back to you soon. |
…EADME and recordings
…luster guard; re-record crud/identity/promote
… 4.5); reset-password includes current admin login; re-record crud
sachim
marked this pull request as ready for review
August 6, 2026 22:12
Oliver Towers (olivertowers)
suggested changes
Aug 6, 2026
Oliver Towers (olivertowers)
left a comment
Oliver Towers (olivertowers)
left a comment
Member
There was a problem hiding this comment.
lgtm - just some test gaps we should resolve.
I largely used the test scenarios as my way of reviewing how things worked, so use the test scenarios as the full functional spec definition for the commands for coverage.
Goal should be coverage of each command and each parameter specific to command that we are adding.
sachim
marked this pull request as ready for review
August 6, 2026 23:16
sachim
marked this pull request as draft
August 6, 2026 23:16
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
added 3 commits
August 6, 2026 20:15
…plicas from replica scenarios (PR feedback)
…list (avoid first-class preview enum)
sachim
changed the title
sachim
marked this pull request as ready for review
August 10, 2026 14:58
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 40 out of 50 changed files in this pull request and generated 1 comment.
Suppressed comments (6)
src/documentdb/HISTORY.rst:27
- This rewrites the beta1 history to claim that
entra-userwas initially released, but the beta1 command wasmongocluster user(as also implied by the migration note above). Preserve the historical command name so users can understand the upgrade path.
* Add ``az documentdb mongocluster entra-user`` commands to manage Microsoft Entra ID
database users.
src/documentdb/azext_documentdb/aaz/latest/documentdb/mongocluster/replica/_list.py:53
--source-clusteris documented for this PR as accepting a name or full ARM ID, but this schema rejects IDs with its name-only regex and the request always uses the current resource group/subscription. As a result,replica list --source-cluster <ARM-ID>cannot work, unlike create/restore/promote. Resolve and decompose the source ID before building the URL, and cover the ARM-ID form in the scenario test.
src/documentdb/azext_documentdb/custom.py:149- This prerequisite is contradicted by the added replica scenario:
_create_cluster()does not enableGeoReplicas, yet the recorded replica creation succeeds, and the properties scenario explicitly says the flag is unrelated to replica creation. Keeping this sentence will incorrectly tell users to modify the source cluster first.
src/documentdb/README.md:85 - The added live recording creates a replica from a source without
previewFeatures, and the test states thatGeoReplicasis unrelated to replica creation. This prerequisite is therefore stale and would mislead users.
# Create a cross-region GeoReplica (the source must have the GeoReplicas preview feature enabled).
src/documentdb/HISTORY.rst:14
- The release note uses the intermediate
entra-username twice, but the shipped command in this release ismicrosoft-entra-userand beta1 exposeduser. Release notes should describe the actual migration so existing users can update their scripts correctly.
This issue also appears on line 26 of the same file.
* Rename ``az documentdb mongocluster user`` to ``entra-user``; the group only supports
Microsoft Entra ID principals, so the name makes that explicit.
* The user identifier is now ``--object-id`` (the Entra object/client ID GUID), keeping the
``-n``/``--name`` aliases.
* Rename ``entra-user create``/``delete`` to ``assign``/``remove``: the Entra principal already
src/documentdb/azext_documentdb/tests/latest/recordings/test_documentdb_mongocluster_negative.yaml:158
- This recording still identifies the old
--cluster-nameparameter even though the test now invokesreplica list --source-cluster. Re-record or update the cassette so it actually reflects the renamed command surface described by the PR.
added 2 commits
August 10, 2026 12:35
…uster update via GET+PATCH The sparse-PATCH conversion (844515b) dropped AZ_SUPPORT_GENERIC_UPDATE and the instance-update pipeline, silently removing 'update --set/--add/--remove'. Restore the GET -> InstanceUpdateByJson -> InstanceUpdateByGeneric pipeline but send the merged instance via HTTP PATCH (not PUT), so untouched properties are preserved (read-only fields are skipped by serialization). Adds a --set generic-update assertion to the properties scenario; crud and properties re-recorded live.
… identity list, and confirmations in HISTORY
Author
|
/azp run |
|
Commenter does not have sufficient privileges for PR 10188 in repo Azure/azure-cli-extensions |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 40 out of 50 changed files in this pull request and generated no new comments.
Suppressed comments (4)
src/documentdb/HISTORY.rst:12
- This second release-note entry also uses the nonexistent
entra-usersurface and obscures that both the group and operations changed. Name the old and new commands explicitly.
* Rename ``entra-user create``/``delete`` to ``assign``/``remove``: the Entra principal already
src/documentdb/HISTORY.rst:36
- The 1.0.0b1 release exposed
az documentdb mongocluster user(as shown by the removed generated commands), notentra-user. Keep the historical entry accurate rather than recording a command that never existed in that release.
* Add ``az documentdb mongocluster entra-user`` commands to manage Microsoft Entra ID
src/documentdb/azext_documentdb/aaz/latest/documentdb/mongocluster/replica/_list.py:53
replica liststill accepts only a same-resource-group cluster name: this formatter rejects ARM IDs, and the value is serialized directly into the{mongoClusterName}path while the request keeps the current resource group/subscription. This contradicts the PR contract that--source-clusteraccepts a name or full ARM ID, so listing a source in another resource group or subscription fails before/requesting the wrong path. Parse full IDs into subscription, resource group, and cluster name (as the other source-cluster wrappers do), and cover that form in the scenario test.
src/documentdb/HISTORY.rst:9- The released command group is
microsoft-entra-user, notentra-user, so this new release note directs users to a nonexistent group. Use the exact public command name.
This issue also appears in the following locations of the same file:
- line 12
- line 36
* Rename ``az documentdb mongocluster user`` to ``entra-user``; the group only supports
Collaborator
|
/azp run |
|
Azure Pipelines: Successfully started running 2 pipeline(s). |
|
Automated sensitive-information remediation ran on this pull request.
If a credential was exposed, rotate or revoke it immediately. Detected values are never copied into this comment. ✅ Keep the redaction · ❌ Dispute the redaction GitHub only supports a fixed reaction set, so 👍 represents ✅ and 👎 represents ❌. The bot-created reactions are only poll choices. Posted by agent-assist (autonomous bug-fix pipeline). |
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.
🤖 PR Validation — ❌ Action needed
❌Azure CLI Extensions Breaking Change Test
Summary
This PR delivers the Azure DocumentDB CLI extension (
az documentdb mongocluster, theMicrosoft.DocumentDB/mongoClustersresource, API2026-06-01) together with a round of gap-analysis fixes that make the command surface consistent, safer, and better documented.Command surface
az documentdb mongocluster— create/update/show/list/delete clusters,list-connection-strings,reset-passwordfirewall-rule— manage IP firewall rules (public access)microsoft-entra-user— assign/show/list/remove Microsoft Entra ID database usersidentity— assign/show/list/remove user-assigned managed identityreplica— list/create/promote cross-region read replicasrestore— point-in-time restore to a new clusterGap-analysis fixes
--source-clusterrename — replaced--parent-cluster-name/--parent-locationonreplica list,replica create,replica promote, andrestorewith a single--source-clusterargument that accepts a cluster name or full ARM ID. The source location is now resolved automatically from the source cluster instead of being passed by hand.microsoft-entra-userrename — theentra-usercommand group is nowmicrosoft-entra-user; removed the unsupportedentra-user updatecommand (the service does not support updating an existing Entra user).identity list— added alistcommand for the cluster's user-assigned managed identities.replica promotenow requires--source-clusterand validates that the named replica actually belongs to that source, raising a clear error on mismatch to prevent promoting the wrong replica. Added a destructive-operation confirmation prompt.--yes/confirmation prompts to destructive operations (replica promote,identity remove).--idssupport — enabled resource-ID addressing viaid_parton the relevant commands.--password/-p/--admin-passwordacrosscreate,update,reset-password, andrestore.README.mdupdated to reflect all of the above; staleentra-user/parent-cluster-namereferences removed from the README and test recordings.Tests
End-to-end scenario tests (recorded live and verified in playback) cover cluster CRUD + connection strings + reset-password, firewall rules, Microsoft Entra users, managed identity, CMK encryption, cross-region replica create, replica promote, and point-in-time restore. Each scenario is an independent test with its own recording.
This checklist is used to make sure that common guidelines for a pull request are followed.
Related command
az documentdb mongoclusterGeneral 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.