Skip to content

sync our development extension with azure-cli upstream module#4630

Merged
mociarain merged 3 commits intomasterfrom
cadenmarchese/cli-downstreaming
Mar 10, 2026
Merged

sync our development extension with azure-cli upstream module#4630
mociarain merged 3 commits intomasterfrom
cadenmarchese/cli-downstreaming

Conversation

@cadenmarchese
Copy link
Member

Which issue this PR addresses:

Fixes No JIRA

What this PR does / why we need it:

This PR brings in some changes to our CLI that were required as part of latest azure-cli upstreaming efforts:

  • Unregister az identity in our own CLI to avoid conflicts with the actual az identity module
  • Commit the test recordings that allow our integration tests to run with local mocks rather than with real Azure credentials. This is required for azure-cli CI to pass. One of them was already present in the upstream, the other one (python/az/aro/azext_aro/tests/latest/integration/recordings/test_aro_get_versions.yaml) was added by me.
  • Fix a few broken docs links and outdated error message

Test plan for issue:

Is there any documentation that needs to be updated for this PR?

How do you know this will function as expected in production?

@openshift-ci openshift-ci bot added the LGTM Looks Good To Me label Feb 26, 2026
@openshift-ci
Copy link

openshift-ci bot commented Feb 26, 2026

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: cadenmarchese, komidore64

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@cadenmarchese cadenmarchese added ready-for-review chainsaw Pull requests or issues owned by Team Chainsaw labels Mar 3, 2026
Copilot AI review requested due to automatic review settings March 3, 2026 18:13
@cadenmarchese cadenmarchese force-pushed the cadenmarchese/cli-downstreaming branch from 05c6830 to 6036c88 Compare March 3, 2026 18:13
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

This PR syncs the ARO development extension with recent upstream azure-cli changes by removing conflicting az identity command registrations, adding a missing integration test recording for aro get-versions, and updating a couple of documentation/help/error-message related items.

Changes:

  • Add VCR recording for aro get-versions integration test playback.
  • Update docs.microsoft.com links to learn.microsoft.com in generated AAZ command group docstrings.
  • Unregister the extension’s identity command group/commands to avoid conflict with upstream az identity, and refine pull-secret validation error handling.

Reviewed changes

Copilot reviewed 7 out of 8 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
python/az/aro/azext_aro/tests/latest/integration/recordings/test_aro_get_versions.yaml Adds recording to enable playback for aro get-versions scenario test.
python/az/aro/azext_aro/aaz/latest/network/vnet/subnet/__cmd_group.py Updates subnet docs link to learn.microsoft.com.
python/az/aro/azext_aro/aaz/latest/network/vnet/__cmd_group.py Updates VNet docs link to learn.microsoft.com.
python/az/aro/azext_aro/aaz/latest/identity/_delete.py Removes command registration decorator for identity delete.
python/az/aro/azext_aro/aaz/latest/identity/_create.py Removes command registration decorator for identity create.
python/az/aro/azext_aro/aaz/latest/identity/__cmd_group.py Removes command group registration decorator for identity.
python/az/aro/azext_aro/_validators.py Improves pull-secret validation by using specific exceptions and a more meaningful internal error.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

* this causes a conflict with existing identity command group
* without these, the tests rely on the user's credentials
@cadenmarchese cadenmarchese force-pushed the cadenmarchese/cli-downstreaming branch from 6036c88 to 64aca84 Compare March 3, 2026 20:16
@mociarain
Copy link
Collaborator

Do we need any testing (other than the unit tests) for this? It LGTM but I don't know a huge amount about the CLI

@cadenmarchese
Copy link
Member Author

@mociarain we don't explicitly test the CLI in CI other than python lint and unit, but if it helps confidence, the azure-cli repo did substantial CI testing against these changes (specifically the integration tests) in this PR: Azure/azure-cli#32796

The large scary file that is making this PR large is actually just a test recording that allows the test to run locally without real Azure credentials, it isn't a change to the CLI code itself.

@mociarain mociarain merged commit 1db8fd5 into master Mar 10, 2026
33 of 34 checks passed
@mociarain mociarain deleted the cadenmarchese/cli-downstreaming branch March 10, 2026 14:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

chainsaw Pull requests or issues owned by Team Chainsaw LGTM Looks Good To Me ready-for-review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants