Repository navigation
[Backup] az backup container register: Add managed identity support for Azure Files backup - #34146
Bharat Purwar (bharatpurwar) wants to merge 5 commits into
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
microsoft-github-policy-service
Bot
added
Auto-Assign
|
/azp run |
|
Azure Pipelines: Successfully started running 3 pipeline(s). |
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Registration can misroute Azure Files and send incorrect storage-account resource-group metadata.
Review effort: Balanced
Findings: 3
Open (3)
What changed in this PR
Extends Azure Files Backup with managed-identity registration, protection, and cross-subscription restore support.
Changes:
- Adds KeyBased, system-assigned, and user-assigned identity workflows.
- Adds subscription-aware restore targeting and output/help updates.
- Adds focused unit and recorded scenario coverage.
| File | Description |
|---|---|
tests/latest/test_custom_afs.py |
Adds focused identity and restore tests. |
tests/latest/test_afs_commands.py |
Adds an end-to-end backup scenario. |
tests/latest/recordings/test_afs_msi_reregistration_protection_restore.yaml |
Records scenario service interactions. |
custom_base.py |
Routes and validates new options. |
custom_afs.py |
Implements registration and restore behavior. |
commands.py |
Registers the generalized container handler. |
_params.py |
Defines new CLI arguments. |
_help.py |
Adds usage examples. |
_format.py |
Displays authentication details. |
_client_factory.py |
Supports subscription-specific clients. |
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| source_resource_id = helper.get_model_property( | ||
| properties, 'container_id', 'containerId') or helper.get_model_property( | ||
| properties, 'source_resource_id', 'sourceResourceId') | ||
|
|
||
| payload = AzureStorageContainer( | ||
| friendly_name=helper.get_model_property(properties, 'friendly_name', 'friendlyName'), | ||
| backup_management_type=backup_management_type, | ||
| source_resource_id=source_resource_id, | ||
| resource_group=resource_group_name, | ||
| operation_type=operation_type, | ||
| access_type=access_type, | ||
| identity_info=identity_info) |
There was a problem hiding this comment.
Had tested, it doesnt affect. There is sourceResourceId which is correctly populated
| if backup_management_type.lower() != "azureworkload": | ||
| raise InvalidArgumentValueError( | ||
| "Container registration supports AzureWorkload and AzureStorage backup management types.") | ||
| if workload_type is None: | ||
| raise RequiredArgumentMissingError( | ||
| "--workload-type is required with --backup-management-type AzureWorkload.") |
There was a problem hiding this comment.
Fixed
| if storage_account is not None or access_type is not None or mi_system_assigned or mi_user_assigned: | ||
| raise ArgumentUsageError( | ||
| "Azure Files managed identity arguments are only supported with " | ||
| "--backup-management-type AzureStorage.") |
There was a problem hiding this comment.
AzureWorkload registration has no confirmation prompt, so --yes has no functional effect. So no need to change
|
🔔 Routing this PR to @Azure/act-observability-squad. |
|
Please fix CI issues |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
/azp run |
|
Azure Pipelines: Successfully started running 3 pipeline(s). |
|
Hi team, Yong Zhang (@yonzhan) , can you please review if the CI issues are due to current pr changes. Based on copilot |
code freeze until next Mon, will take a look during this period. |
|
Hi Ethan Yang (@necusjz) , Yong Zhang (@yonzhan) |
|
/azp run |
|
Azure Pipelines: Successfully started running 3 pipeline(s). |
it was fixed by d295b94. |
|
/azp run |
|
Azure Pipelines: Successfully started running 3 pipeline(s). |
|
Hi Ethan Yang (@necusjz) |
|
Hi NoriZC |
Teresa Ritorto (teresaritorto)
added
the
Request X Engineering Agent
|
🔔 Routing this PR to @Azure/act-observability-squad. |
X Engineering Agent ReportReview — CommentsReviewReviewed head Confirmed findingP3 — AzureWorkload silently ignores the new Evaluated reviewer feedbackThe default-AzureWorkload/AzureFileShare concern is already addressed: Test validation
Paths below are relative to In Before approval, perform a focused rerun/re-record against this SHA with separate vault/account resource groups and distinct subscriptions, asserting outgoing identity/target/folder fields and restored content. Cover fresh registration, KeyBased↔identity switching, cancellation/ Risk assessment59/100 · High · High confidence The High rating is driven by security-sensitive behavior, public CLI behavior.
Posted by x-engineering-agent (Reviewer) |
x-engineering-agent
Bot
added
the
X Engineering Agent Reviewed
|
🔔 Routing this PR to @Azure/act-observability-squad. |
|
/azp run |
|
Azure Pipelines: Successfully started running 3 pipeline(s). |
🤖 PR Validation —⚠️ Review suggested
Summary
Replacement PR
az backup container register: Add managed identity support for Azure Files backup #34109 after its revert in [Backup]az backup container register: Revert managed identity support for Azure Files backup #34142dev2026-07-01to the current SDK API2026-08-01Validation
api-version=2026-08-01; no2026-07-01requests remaingit diff --checkpasses