Skip to content

Conversation

@ankushbindlish2
Copy link
Member

@ankushbindlish2 ankushbindlish2 commented Dec 10, 2025

What does this PR do?

This pull request adds comprehensive Azure Storage Sync (File Sync) support to the Azure MCP Server with 24 new commands organized into 5 resource categories:

  • StorageSyncService (5 commands): Create, delete, get, list, and update Storage Sync Services for managing cloud synchronization of file shares.
  • RegisteredServer (5 commands): Get, list, register, unregister, and update servers registered with a Storage Sync Service.
  • SyncGroup (4 commands): Create, delete, get, and list sync groups that define synchronization topology between cloud and server endpoints.
  • CloudEndpoint (5 commands): Create, delete, get, list, and trigger change detection on cloud endpoints representing Azure file shares.
  • ServerEndpoint (5 commands): Create, delete, get, list, and update server endpoints with cloud tiering and tiering policy configuration.

All commands include proper error handling, metadata annotations (destructive, idempotent, readOnly flags), comprehensive unit tests (49 passing tests), and live test infrastructure with Azure resource deployment support.

Open Questions

  • Managed Identity Support ?
  • ** RoleAssignments creation as part of resource creation ? **
  • ** Hybrid API which has no significance without being run at a windows server like PUT REGISTEREDSERVER ? **
  • ** PrivateEndpointConnections API for connection management ?**

GitHub issue number?

[ONBOARD] Add Support for Microsoft.StorageSync Resource Provider to Azure MCP Server #1354

Pre-merge Checklist

  • Required for All PRs
    • Read contribution guidelines
    • PR title clearly describes the change
    • Commit history is clean with descriptive messages (cleanup guide)
    • Added comprehensive tests for new/modified functionality
    • Updated servers/Azure.Mcp.Server/CHANGELOG.md and/or servers/Fabric.Mcp.Server/CHANGELOG.md for product changes (features, bug fixes, UI/UX, updated dependencies)
  • For MCP tool changes:
    • One tool per PR: This PR adds or modifies only one MCP tool for faster review cycles
    • Updated servers/Azure.Mcp.Server/README.md and/or servers/Fabric.Mcp.Server/README.md documentation
    • Validate README.md changes using script at eng/scripts/Process-PackageReadMe.ps1. See Package README
    • Updated command list in /servers/Azure.Mcp.Server/docs/azmcp-commands.md and/or /docs/fabric-commands.md
    • Run .\eng\scripts\Update-AzCommandsMetadata.ps1 to update tool metadata in azmcp-commands.md (required for CI)
    • For new or modified tool descriptions, ran ToolDescriptionEvaluator and obtained a score of 0.4 or more and a top 3 ranking for all related test prompts
    • For tools with new names, including new tools or renamed tools, update consolidated-tools.json
    • For new tools associated with Azure services or publicly available tools/APIs/products, add URL to documentation in the PR description
  • Extra steps for Azure MCP Server tool changes:
    • Updated test prompts in /servers/Azure.Mcp.Server/docs/e2eTestPrompts.md
    • 👉 For Community (non-Microsoft team member) PRs:
      • Security review: Reviewed code for security vulnerabilities, malicious code, or suspicious activities before running tests (crypto mining, spam, data exfiltration, etc.)
      • Manual tests run: added comment /azp run mcp - pullrequest - live to run Live Test Pipeline

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 introduces unit tests for the Azure.Mcp.Tools.StorageSync tool, adding comprehensive test coverage for all command classes. The PR correctly implements logger and service injection patterns for 24 command test classes.

Key changes:

  • Added 24 unit test files for StorageSync commands across 5 categories (StorageSyncService, SyncGroup, CloudEndpoint, ServerEndpoint, RegisteredServer)
  • Implemented complete StorageSync tool infrastructure including commands, services, models, options, and setup classes
  • Added help documentation files for Azure PowerShell cmdlet compatibility
  • Created test project configuration with proper dependencies

Reviewed changes

Copilot reviewed 130 out of 132 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
All test files in Commands/*CommandTests.cs New unit tests with correct logger and service injection order (logger first, service second)
StorageSyncSetup.cs Service registration and command hierarchy setup
StorageSyncService.cs Service implementation with stub methods
IStorageSyncService.cs Service interface defining all operations
All command files Command implementations using primary constructors with correct parameter order
Models/*.cs Data model classes for StorageSync resources
Options/*.cs Option classes for command parameters
Help/*.md PowerShell cmdlet documentation
Azure.Mcp.Tools.StorageSync.csproj Project configuration
Azure.Mcp.Tools.StorageSync.UnitTests.csproj Test project configuration

@ankushbindlish2 ankushbindlish2 changed the title [DRAFT] Fix StorageSync unit tests - correct logger and service injection [DRAFT] [ONBOARD] Add Support for Microsoft.StorageSync Resource Provider to Azure MCP Server Dec 11, 2025
- Fix all 49 unit tests across 24 test files (100% passing)
- Create live test framework with 6 test methods (RecordedCommandTestsBase pattern)
- Deploy Azure test resources: StorageSyncService, SyncGroup
- Add mock HTTP session recordings for Playback mode tests
- Create comprehensive test documentation (TESTING.md)
- Configure test infrastructure: assets.json, .testsettings.json
- Simplify Bicep template for stable resource deployment

All unit tests validated and passing. Live tests configured and ready for integration testing.

- Unit Tests: 49/49 passing
- Build: Successful with no errors
- Azure Resources: StorageSyncService (mcp252dc347), SyncGroup (mcp252dc347-sg) deployed
Deleted outdated session record files for StorageSyncCommandTests to clean up test artifacts and reduce repository clutter. These files are no longer needed for current test runs.
Removed outdated references to unit test coverage and help documentation from CHANGELOG.md. Updated README.md to fix minor formatting issues and clarify the description for Azure Storage Sync.
Deleted the README and all help documentation files for Azure.Mcp.Tools.StorageSync, including cmdlet references and usage guides. This cleans up the documentation and help content from the tools directory.
Add comprehensive command documentation for all 24 StorageSync commands:
- Storage Sync Service: create, delete, get, list, update
- Sync Group: create, delete, get, list
- Cloud Endpoint: create, delete, get, list, changedetection
- Registered Server: get, list, register, unregister, update
- Server Endpoint: create, delete, get, list, update

Each command includes usage examples, flags, and operation metadata.
ankushbindlish2 and others added 3 commits December 13, 2025 02:27
Added [RequiresDynamicCode] and [RequiresUnreferencedCode] attributes to InitializeConfigurationAndOptions to indicate potential issues with trimming and dynamic code. Also added a newline at the end of StorageSyncServiceListCommand.cs for consistency.
ankushbindlish2 and others added 6 commits December 16, 2025 13:41
Updated the Azure.ResourceManager.StorageSync package version from 1.3.0 to 1.3.1 to include the latest fixes and improvements.
Replaces and expands 'manage' operations in consolidated-tools.json with granular CRUD operations for Azure File Sync resources, providing detailed metadata for each tool. Adds new tool definitions for get, create, update, delete, and trigger operations on services, groups, endpoints, and registered servers. Also, enables a previously commented-out test in ConsolidatedModeTests.cs and removes an unused TESTING.md file.
Merged 'get' and 'list' operations for StorageSyncService, SyncGroup, CloudEndpoint, RegisteredServer, and ServerEndpoint into single 'get' commands that support both single and multiple resource retrieval. Removed all separate 'list' command implementations, updated documentation, registration, and serialization context accordingly, and refactored tests to match the new structure.
Regenerates the Id property values for all StorageSync command classes to new GUIDs. No other logic or functionality is changed.
* Fix versions in Fabric changelog and add debug text to script (#1391)

* Remove unnecessary build targets and resources (#1375)

* Move azure icon to images folder
* Remove duplicate resources and unnecessary build targets
* Remove unnecessary reference to eng/dnx from pack-nuget

* Prevent HashTable.Count from affecting result count check (#1397)

* Migrate Authorization to recordings (#1399)

* Fix execution of parallel testclasses within testassembly (#1393)

* assets.json longer optional
* changes to prevent multiple proxy instances from restoring simultaneously

* Fabric MCP: Add OneLake namespace to VSCode options (#1398)

* Fabric MCP: Add OneLake namespace to VSCode options

* Update descriptions

---------

Co-authored-by: Patrick Hallisey <pahallis@microsoft.com>
Co-authored-by: Alan Zimmer <48699787+alzimmermsft@users.noreply.github.com>
Co-authored-by: Scott Beddall <45376673+scbedd@users.noreply.github.com>
Co-authored-by: Amos Hersch <39293413+AmosHersch@users.noreply.github.com>
Applied [RequiresDynamicCode] and [RequiresUnreferencedCode] attributes to InitializeConfigurationAndOptions to indicate potential issues with trimming and dynamic code. Also fixed minor whitespace in ConsolidatedModeTests.
/// Abstraction for resolving recording asset paths and session directories.
/// Enables tests to substitute custom paths when exercising record/playback infrastructure.
/// </summary>
public interface IRecordingPathResolver
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@scbedd is this needed for recording

* Fix versions in Fabric changelog and add debug text to script (#1391)

* Remove unnecessary build targets and resources (#1375)

* Move azure icon to images folder
* Remove duplicate resources and unnecessary build targets
* Remove unnecessary reference to eng/dnx from pack-nuget

* Prevent HashTable.Count from affecting result count check (#1397)

* Migrate Authorization to recordings (#1399)

* Fix execution of parallel testclasses within testassembly (#1393)

* assets.json longer optional
* changes to prevent multiple proxy instances from restoring simultaneously

* Fabric MCP: Add OneLake namespace to VSCode options (#1398)

* Fabric MCP: Add OneLake namespace to VSCode options

* Update descriptions

* Increment versions and update CHANGELOGs after release (#1371)

Updated CHANGELOGs and project version after release

* Migrate `marketplace` to recordings (#1396)

* simplify the client creation a bit, remove unnecessary test parts, as the newing that is being exercised in the construction of the test isn't actually used for anything. the product commands ARE though

* Migrate AKS to recordings (#1384)

* Migrate AKS to recordings

* Fix linting, synchronize starting proxy

* Revert lock change

---------

Co-authored-by: Scott Beddall (from Dev Box) <scbedd@microsoft.com>

* Migrate Function App to recordings (#1410)

---------

Co-authored-by: Patrick Hallisey <pahallis@microsoft.com>
Co-authored-by: Alan Zimmer <48699787+alzimmermsft@users.noreply.github.com>
Co-authored-by: Scott Beddall <45376673+scbedd@users.noreply.github.com>
Co-authored-by: Amos Hersch <39293413+AmosHersch@users.noreply.github.com>
Co-authored-by: vcolin7 <victor.y.asi@gmail.com>
Co-authored-by: Scott Beddall (from Dev Box) <scbedd@microsoft.com>
ankushbindlish2 and others added 3 commits December 18, 2025 21:27
* Fix versions in Fabric changelog and add debug text to script (#1391)

* Remove unnecessary build targets and resources (#1375)

* Move azure icon to images folder
* Remove duplicate resources and unnecessary build targets
* Remove unnecessary reference to eng/dnx from pack-nuget

* Prevent HashTable.Count from affecting result count check (#1397)

* Migrate Authorization to recordings (#1399)

* Fix execution of parallel testclasses within testassembly (#1393)

* assets.json longer optional
* changes to prevent multiple proxy instances from restoring simultaneously

* Fabric MCP: Add OneLake namespace to VSCode options (#1398)

* Fabric MCP: Add OneLake namespace to VSCode options

* Update descriptions

* Increment versions and update CHANGELOGs after release (#1371)

Updated CHANGELOGs and project version after release

* Migrate `marketplace` to recordings (#1396)

* simplify the client creation a bit, remove unnecessary test parts, as the newing that is being exercised in the construction of the test isn't actually used for anything. the product commands ARE though

* Migrate AKS to recordings (#1384)

* Migrate AKS to recordings

* Fix linting, synchronize starting proxy

* Revert lock change

---------

Co-authored-by: Scott Beddall (from Dev Box) <scbedd@microsoft.com>

* Migrate Function App to recordings (#1410)

---------

Co-authored-by: Patrick Hallisey <pahallis@microsoft.com>
Co-authored-by: Alan Zimmer <48699787+alzimmermsft@users.noreply.github.com>
Co-authored-by: Scott Beddall <45376673+scbedd@users.noreply.github.com>
Co-authored-by: Amos Hersch <39293413+AmosHersch@users.noreply.github.com>
Co-authored-by: vcolin7 <victor.y.asi@gmail.com>
Co-authored-by: Scott Beddall (from Dev Box) <scbedd@microsoft.com>
* Fix versions in Fabric changelog and add debug text to script (#1391)

* Remove unnecessary build targets and resources (#1375)

* Move azure icon to images folder
* Remove duplicate resources and unnecessary build targets
* Remove unnecessary reference to eng/dnx from pack-nuget

* Prevent HashTable.Count from affecting result count check (#1397)

* Migrate Authorization to recordings (#1399)

* Fix execution of parallel testclasses within testassembly (#1393)

* assets.json longer optional
* changes to prevent multiple proxy instances from restoring simultaneously

* Fabric MCP: Add OneLake namespace to VSCode options (#1398)

* Fabric MCP: Add OneLake namespace to VSCode options

* Update descriptions

* Increment versions and update CHANGELOGs after release (#1371)

Updated CHANGELOGs and project version after release

* Migrate `marketplace` to recordings (#1396)

* simplify the client creation a bit, remove unnecessary test parts, as the newing that is being exercised in the construction of the test isn't actually used for anything. the product commands ARE though

* Migrate AKS to recordings (#1384)

* Migrate AKS to recordings

* Fix linting, synchronize starting proxy

* Revert lock change

---------

Co-authored-by: Scott Beddall (from Dev Box) <scbedd@microsoft.com>

* Migrate Function App to recordings (#1410)

---------

Co-authored-by: Patrick Hallisey <pahallis@microsoft.com>
Co-authored-by: Alan Zimmer <48699787+alzimmermsft@users.noreply.github.com>
Co-authored-by: Scott Beddall <45376673+scbedd@users.noreply.github.com>
Co-authored-by: Amos Hersch <39293413+AmosHersch@users.noreply.github.com>
Co-authored-by: vcolin7 <victor.y.asi@gmail.com>
Co-authored-by: Scott Beddall (from Dev Box) <scbedd@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Untriaged

Development

Successfully merging this pull request may close these issues.

3 participants