Let parent config AKV settings propagate to child configs#3323
Open
aaronburtle wants to merge 3 commits intomainfrom
Open
Let parent config AKV settings propagate to child configs#3323aaronburtle wants to merge 3 commits intomainfrom
aaronburtle wants to merge 3 commits intomainfrom
Conversation
When loading child data-source-files, the parent's AzureKeyVault options were not passed to the DeserializationVariableReplacementSettings, causing @akv() references in child configs to go unresolved. Fix: pass this.AzureKeyVault instead of null so child configs inherit the parent's Key Vault configuration for secret resolution. Regression test uses a local .akv file to verify end-to-end resolution. Fixes part of #3271
aaronburtle
requested review from
Alekhya-Polavarapu,
Aniruddh25,
JerryNixon,
RubenCerna2079,
anushakolan,
rusamant,
sourabh1007,
souvikghosh04,
stuartpa and
vadeveka
as code owners
Contributor
There was a problem hiding this comment.
Pull request overview
Propagates the parent runtime config’s Azure Key Vault (AKV) options when deserializing child configs in multi–data source scenarios, so @akv('...') replacements in child files can succeed (fix for issue #3322).
Changes:
- Pass parent
AzureKeyVaultoptions intoDeserializationVariableReplacementSettingswhen loading each child config. - Add a regression test that loads a parent config referencing a local
.akvsecrets file and a child config with an@akv()connection string.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/Config/ObjectModel/RuntimeConfig.cs | Use parent AKV options when creating replacement settings for child config loading. |
| src/Service.Tests/Configuration/RuntimeConfigLoaderTests.cs | Add regression test validating child @akv() resolution via parent AKV options using a local .akv file. |
Contributor
Author
|
@copilot can you review this again please |
Contributor
Both issues from the prior review were already addressed in commit
The changes look good to me. |
Aniruddh25
reviewed
Apr 4, 2026
|
|
||
| // Pass the parent's AKV options so @akv() references in child configs can | ||
| // be resolved using the parent's Key Vault configuration. | ||
| DeserializationVariableReplacementSettings replacementSettings = new(azureKeyVaultOptions: this.AzureKeyVault, doReplaceEnvVar: true, doReplaceAkvVar: true); |
Collaborator
There was a problem hiding this comment.
Are we sure if the child config has its own AKV options, those will override the ones passed onto by the parent?
Aniruddh25
reviewed
Apr 4, 2026
| // Verify the child's connection string was resolved from the .akv file. | ||
| string childDataSourceName = runtimeConfig.GetDataSourceNameFromEntityName("AkvChildEntity"); | ||
| DataSource childDataSource = runtimeConfig.GetDataSourceFromDataSourceName(childDataSourceName); | ||
| Assert.IsTrue( |
Collaborator
There was a problem hiding this comment.
Can we also verify in a different case that the child AKV settings override the ones passed onto by the parent?
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.
Why make this change?
When a parent config has
azure-key-vaultconfigured, child configs referenced viadata-source-fileswere unable to resolve@akv('...')references because the parent's AKV options were not passed during child config deserialization.What is this change?
AzureKeyVaultoptions intoDeserializationVariableReplacementSettingswhen loading each child config, enabling@akv('...')references in child configs to be resolved using the parent's Key Vault configuration.DeserializationVariableReplacementSettingsobject is created once before the child configforeachloop and reused for all child configs, avoiding redundant Key Vault client initialization or secrets file reads per iteration.How was this tested?
Regression test added (
ChildConfigResolvesAkvReferencesFromParentAkvOptions) that loads a parent config referencing a local.akvsecrets file and a child config with an@akv()connection string, validating the connection string is correctly resolved without requiring a real Azure Key Vault.