[AppConfig] az appconfig kv: Add snapshot reference support#33278
Open
ChristineWanjau wants to merge 5 commits intoAzure:devfrom
Open
[AppConfig] az appconfig kv: Add snapshot reference support#33278ChristineWanjau wants to merge 5 commits intoAzure:devfrom
az appconfig kv: Add snapshot reference support#33278ChristineWanjau wants to merge 5 commits intoAzure:devfrom
Conversation
❌AzureCLI-FullTest
|
|
| rule | cmd_name | rule_message | suggest_message |
|---|---|---|---|
| appconfig kv list | cmd appconfig kv list added parameter resolve_snapshot_references |
||
| appconfig kv set-snapshot-reference | cmd appconfig kv set-snapshot-reference added |
Collaborator
|
Thank you for your contribution! We will review the pull request and get back to you soon. |
|
The git hooks are available for azure-cli and azure-cli-extensions repos. They could help you run required checks before creating the PR. Please sync the latest code with latest dev branch (for azure-cli) or main branch (for azure-cli-extensions). pip install azdev --upgrade
azdev setup -c <your azure-cli repo path> -r <your azure-cli-extensions repo path>
|
microsoft-github-policy-service
Bot
requested review from
avanigupta,
jsntcy,
yanzhudd,
yonzhan and
zhoxing-ms
microsoft-github-policy-service
Bot
added
App Configuration
Auto-Assign
ChristineWanjau
changed the title
az appconfig kv: Add snapshot reference supportaz appconfig kv: Add snapshot reference support
Contributor
There was a problem hiding this comment.
Pull request overview
Adds Azure App Configuration snapshot reference support to az appconfig kv by introducing a new command to create/update snapshot reference key-values and a new list flag to expand references into the snapshot’s key-values.
Changes:
- Add
az appconfig kv set-snapshot-referenceto write snapshot reference key-values with the appropriate content type/value format. - Add
az appconfig kv list --resolve-snapshot-referencesto expand snapshot references into the referenced snapshot’s key-values (in sequence). - Add constants, CLI params/validators/help text, and scenario tests for the new behaviors.
Reviewed changes
Copilot reviewed 7 out of 10 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
src/azure-cli/azure/cli/command_modules/appconfig/tests/latest/test_appconfig_snapshot_reference_commands.py |
Adds scenario coverage for setting snapshot references and listing with reference resolution. |
src/azure-cli/azure/cli/command_modules/appconfig/keyvalue.py |
Implements set_snapshot_reference and snapshot-reference expansion in kv list. |
src/azure-cli/azure/cli/command_modules/appconfig/commands.py |
Registers the new appconfig kv set-snapshot-reference command. |
src/azure-cli/azure/cli/command_modules/appconfig/_validators.py |
Adds validation for --snapshot-name. |
src/azure-cli/azure/cli/command_modules/appconfig/_params.py |
Wires new command arguments and the --resolve-snapshot-references flag. |
src/azure-cli/azure/cli/command_modules/appconfig/_help.py |
Documents the new command and list flag with examples. |
src/azure-cli/azure/cli/command_modules/appconfig/_constants.py |
Introduces snapshot reference constants (content type + JSON key). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
907
to
+928
| @@ -841,9 +922,46 @@ def list_key(cmd, | |||
| top=top, | |||
| all_=all_, | |||
| cli_ctx=cmd.cli_ctx if resolve_keyvault else None) | |||
|
|
|||
| if resolve_snapshot_references: | |||
| keyvalues = __resolve_snapshot_references(azconfig_client, keyvalues) | |||
|
|
|||
There was a problem hiding this comment.
--resolve-snapshot-references currently has no compatibility check with --fields. When --fields is used, __read_kv_from_config_store returns dicts (partial KVs), so __resolve_snapshot_references won't detect/resolve snapshot references and behavior becomes inconsistent. Either disallow --fields with --resolve-snapshot-references (similar to --resolve-keyvault), or update the resolver to support dict-shaped items and apply the same field trimming to expanded snapshot key-values.
Comment on lines
+954
to
+960
| try: | ||
| snapshot_keyvalues = __read_kv_from_config_store(azconfig_client, snapshot=snapshot_name) | ||
| except Exception as ex: # pylint: disable=broad-except | ||
| logger.warning("Skipping snapshot reference '%s': %s", snapshot_name, str(ex)) | ||
| continue | ||
|
|
||
| resolved_keyvalues.extend(snapshot_keyvalues) |
There was a problem hiding this comment.
When both --resolve-keyvault and --resolve-snapshot-references are set, snapshot-expanded key-values are fetched via __read_kv_from_config_store(..., snapshot=...) without cli_ctx, so Key Vault references inside the snapshot results will not be resolved. Consider passing cmd.cli_ctx (or a cli_ctx parameter) through the snapshot resolver when resolve_keyvault is enabled.
| resolved_keyvalues = [] | ||
| for keyvalue in keyvalues: | ||
| content_type = getattr(keyvalue, 'content_type', None) | ||
| if not (content_type and SnapshotReferenceConstants.SNAPSHOT_REFERENCE_CONTENT_TYPE in content_type): |
There was a problem hiding this comment.
Snapshot reference detection uses substring matching (SNAPSHOT_REFERENCE_CONTENT_TYPE in content_type) without type/case normalization. This can mis-detect or fail to detect references if casing differs or if other content types contain the profile string. Align with the Key Vault ref check in _kv_helpers.__is_key_vault_ref by verifying isinstance(content_type, str) and comparing content_type.lower() for exact equality.
Suggested change
| if not (content_type and SnapshotReferenceConstants.SNAPSHOT_REFERENCE_CONTENT_TYPE in content_type): | |
| if not ( | |
| isinstance(content_type, str) and | |
| content_type.lower() == SnapshotReferenceConstants.SNAPSHOT_REFERENCE_CONTENT_TYPE.lower()): |
| c.argument('secret_identifier', validator=validate_secret_identifier, help="ID of the Key Vault object. Can be found using 'az keyvault {collection} show' command, where collection is key, secret or certificate. To set reference to the latest version of your secret, remove version information from secret identifier.") | ||
|
|
||
| with self.argument_context('appconfig kv set-snapshot-reference') as c: | ||
| c.argument('key', validator=validate_key, help="Key to be set. Key cannot be a '.' or '..', or contain the '%%' character.") |
There was a problem hiding this comment.
The help text says the key cannot contain the '%%' character, but the validator rejects a single '%' (see validate_key). This looks like an escaping mistake and will surface incorrect CLI help. Update the string to '%'.
Suggested change
| c.argument('key', validator=validate_key, help="Key to be set. Key cannot be a '.' or '..', or contain the '%%' character.") | |
| c.argument('key', validator=validate_key, help="Key to be set. Key cannot be a '.' or '..', or contain the '%' character.") |
| def validate_snapshot_reference(namespace): | ||
| if not namespace.snapshot_name or str(namespace.snapshot_name).isspace(): | ||
| raise RequiredArgumentMissingError("--snapshot-name is required and cannot be empty.") | ||
|
|
There was a problem hiding this comment.
PEP8 requires two blank lines between top-level function definitions. Add a blank line between validate_snapshot_reference and validate_resolve_keyvault for consistency with the rest of this module.
Suggested change
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.
Related command
az appconfig kv set-snapshot-reference (new)
az appconfig kv list --resolve-snapshot-references (new flag)
Description
New command — az appconfig kv set-snapshot-reference
Creates or updates a snapshot reference key-value at the given key (and optional label) pointing at a snapshot by name.
New flag — az appconfig kv list --resolve-snapshot-references
When set, every snapshot reference returned by the list is replaced in sequence by the key-values of the snapshot it points to. Non-reference key-values pass through unchanged.
Testing Guide
History Notes
[Component Name 1]
az appconfig kv set-snapshot-reference: Adds support to create a snapshot reference key-value[Component Name 2]
az appconfig kv list: Adds support to list key-values from a snapshot referenceThis checklist is used to make sure that common guidelines for a pull request are followed.
The PR title and description has followed the guideline in Submitting Pull Requests.
I adhere to the Command Guidelines.
I adhere to the Error Handling Guidelines.