Cherry pick variable replacement and MCP refactor for release 1.7 #3014
Conversation
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
… settings class and add AKV replacement logic. (#2882) ## Why make this change? Adds AKV variable replacement and expands our design for doing variable replacements to be more extensible when new variable replacement logic is added. Closes #2708 Closes #2748 Related to #2863 ## What is this change? Change the way that variable replacement is handled to instead of simply using a `bool` to indicate that we want env variable replacement, we add a class which holds all of the replacement settings. This will hold whether or not we will do replacement for each kind of variable that we will handle replacement for during deserialization. We also include the replacement failure mode, and put the logic for handling the replacements into a strategy dictionary which pairs the replacement variable type with the strategy for doing that replacement. Because Azure Key Vault secret replacement requires having the retry and connection settings in order to do the AKV replacement, we must do a first pass where we only do non-AKV replacement and get the required settings so that if AKV replacement is used we have the required settings to do that replacement. We also have to keep in mind that the legacy of the `Configuration Controller` will ignore all variable replacement, so we construct the replacement settings for this code path to not use any variable replacement at all. ## How was this tested? We have updated the logic for the tests to use the new system, however manual testing using an actual AKV is still required. ## Sample Request(s) - Example REST and/or GraphQL request to demonstrate modifications - Example of CLI usage to demonstrate modifications --------- Co-authored-by: Copilot <[email protected]> Co-authored-by: Aniruddh Munde <[email protected]>
## Why make this change? Closes #2748 ## What is this change? Adds the option to use a local .akv file instead of Azure Key Vault for @akv('') replacement in the config file during deserialization. Similar to how we handle .env files. ## How was this tested? A new test was added that verifies we are able to do the replacement and get the correct resultant configuration. --------- Co-authored-by: Aniruddh Munde <[email protected]>
## Why make this change? Closes #2932 ## What is this change? Add helper class `McpMetadataHelper`, extend `McpArgumentParser`, and utilize `McpAuthorizationHelper` to factor out common code. We now do the initialization of the metadata, the parsing of arguments, and the authorization checks in these shared helper classes. ## How was this tested? With MCP Inspector and against the normal test suite. * DESCRIBE_ENTITIES <img width="427" height="653" alt="image" src="https://github.com/user-attachments/assets/7ba74cfb-5a71-402b-afd2-17f7a24d0295" /> * CREATE <img width="1435" height="655" alt="image" src="https://github.com/user-attachments/assets/f189bb22-6f25-46ef-b2f0-20e80bc2850f" /> * READ <img width="1131" height="651" alt="image" src="https://github.com/user-attachments/assets/16f3e6f6-24e9-4613-a8fd-61546b199305" /> * UPDATE <img width="1083" height="292" alt="image" src="https://github.com/user-attachments/assets/ce284b6f-1f2f-4dc4-b73d-8242605ee20a" /> * DELETE <img width="1425" height="648" alt="image" src="https://github.com/user-attachments/assets/8768baf6-96f3-441c-b47d-c17e5fae9300" /> ## Sample Request(s) N/A --------- Co-authored-by: Copilot <[email protected]> Co-authored-by: Souvik Ghosh <[email protected]>
aaronburtle
requested review from
Alekhya-Polavarapu,
Aniruddh25,
RubenCerna2079,
anushakolan,
neeraj-sharma2592,
rusamant,
sourabh1007,
souvikghosh04 and
vadeveka
as code owners
Contributor
Author
|
/azp run |
|
Azure Pipelines successfully started running 6 pipeline(s). |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR cherry-picks commits that modernize variable replacement during configuration deserialization by introducing Azure Key Vault (AKV) support alongside environment variables, and refactors the MCP project to use shared utility functions with standardized response formatting.
Key Changes:
- Introduced
DeserializationVariableReplacementSettingsclass to replace multiple boolean parameters with a unified configuration object - Added Azure Key Vault variable replacement support via
@akv('secret-name')pattern, with both remote AKV and local .akv file support - Refactored MCP tools to use shared utility classes (McpResponseBuilder, McpMetadataHelper, McpAuthorizationHelper, McpArgumentParser) for consistent error handling and response formatting
Reviewed changes
Copilot reviewed 48 out of 48 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Config/DeserializationVariableReplacementSettings.cs | New class encapsulating variable replacement logic for both @env and @akv patterns with local file and remote AKV support |
| src/Config/RuntimeConfigLoader.cs | Updated to use DeserializationVariableReplacementSettings and added two-pass AKV extraction logic |
| src/Config/Converters/*.cs | Refactored all converter factories to accept DeserializationVariableReplacementSettings instead of boolean flags |
| src/Config/ObjectModel/AzureKeyVaultOptions.cs | Added UserProvidedEndpoint/RetryPolicy flags and constructor for proper serialization control |
| src/Azure.DataApiBuilder.Mcp/Utils/*.cs | New utility classes for standardized MCP tool operations |
| src/Azure.DataApiBuilder.Mcp/BuiltInTools/*.cs | Refactored to use shared utilities, reducing code duplication |
| src/Service.Tests/UnitTests/*.cs | Updated tests to use new DeserializationVariableReplacementSettings API and added AKV-specific tests |
| src/Directory.Packages.props | Added Azure.Security.KeyVault.Secrets package dependency |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
RubenCerna2079
requested changes
Dec 13, 2025
Contributor
RubenCerna2079
left a comment
RubenCerna2079
left a comment
There was a problem hiding this comment.
Could you fix the PR description so that it shows all of the cherry picks in the What is this change section? Similar to this PR #2852
Contributor
souvikghosh04
left a comment
souvikghosh04
left a comment
There was a problem hiding this comment.
Are we not porting the changes for #2943 ? please check with @anushakolan once.
Aniruddh25
reviewed
Dec 17, 2025
Aniruddh25
approved these changes
Dec 17, 2025
aaronburtle
changed the title
Contributor
Author
I think each person is creating their own cherry pick PR into the release branch with their commits. |
RubenCerna2079
approved these changes
Dec 17, 2025
Contributor
RubenCerna2079
left a comment
RubenCerna2079
left a comment
There was a problem hiding this comment.
Just waiting for the final cherry-pick. Besides that LGTM :)
Contributor
Author
|
/azp run |
|
Azure Pipelines successfully started running 6 pipeline(s). |
RubenCerna2079
approved these changes
Dec 18, 2025
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.
Why make this change?
This change cherry picks the commits associated with the updating of how we do variable replacement during deserialization, adding AKV to those variables that we look for. It further includes the commits for refactoring the MCP project so that the tools have their common code shared in utility functions, and have their responses aligned.
What is this change?
Cherry picked PRs:
Azure Key Vault Support:
MCP Refactoring:
How was this tested?
The PRs in question were tested against the regular test suite and had tests added to cover new code changes, as well as being manually tested e.g. mcp inspector tool.
Sample Request(s)
N/A