From 1ce27f1b9f8a0d900181d0f95e1124003d231eae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Cassanji?= Date: Wed, 4 Feb 2026 19:24:04 -0300 Subject: [PATCH 01/35] feat: Add keyvault copy command --- .../cli/command_modules/keyvault/_params.py | 9 +++ .../cli/command_modules/keyvault/commands.py | 1 + .../cli/command_modules/keyvault/custom.py | 77 +++++++++++++++++++ .../tests/latest/test_keyvault_copy.py | 66 ++++++++++++++++ 4 files changed, 153 insertions(+) create mode 100644 src/azure-cli/azure/cli/command_modules/keyvault/tests/latest/test_keyvault_copy.py diff --git a/src/azure-cli/azure/cli/command_modules/keyvault/_params.py b/src/azure-cli/azure/cli/command_modules/keyvault/_params.py index 5718281f2b4..9a47d1f8307 100644 --- a/src/azure-cli/azure/cli/command_modules/keyvault/_params.py +++ b/src/azure-cli/azure/cli/command_modules/keyvault/_params.py @@ -568,6 +568,15 @@ class CLISecurityDomainOperation(str, Enum): with self.argument_context('keyvault secret restore') as c: c.extra('vault_base_url', vault_name_type, required=True, arg_group='Id', type=get_vault_base_url_type(self.cli_ctx), id_part=None) + + with self.argument_context('keyvault secret copy') as c: + c.extra('vault_base_url', vault_name_type, type=get_vault_base_url_type(self.cli_ctx), + options_list=['--source-vault'], help='Name of the source Key Vault.', required=True) + c.extra('destination_vault', vault_name_type, type=get_vault_base_url_type(self.cli_ctx), + options_list=['--destination-vault'], help='Name of the destination Key Vault.', required=True) + c.argument('name', options_list=['--name', '-n'], help='Name of the secret to copy.', required=False) + c.extra('all_secrets', arg_type=get_three_state_flag(), options_list=['--all'], help='Copy all secrets.') + c.extra('rewrite', arg_type=get_three_state_flag(), help='Overwrite existing secrets in destination.') # endregion # region keyvault security-domain diff --git a/src/azure-cli/azure/cli/command_modules/keyvault/commands.py b/src/azure-cli/azure/cli/command_modules/keyvault/commands.py index c70ae0b600c..ca5a2085e41 100644 --- a/src/azure-cli/azure/cli/command_modules/keyvault/commands.py +++ b/src/azure-cli/azure/cli/command_modules/keyvault/commands.py @@ -204,6 +204,7 @@ def load_command_table(self, _): g.keyvault_custom('download', 'download_secret') g.keyvault_custom('backup', 'backup_secret') g.keyvault_custom('restore', 'restore_secret', transform=transform_secret_set_attributes) + g.keyvault_custom('copy', 'copy_secret') # certificate track2 with self.command_group('keyvault certificate', data_certificate_entity.command_type) as g: diff --git a/src/azure-cli/azure/cli/command_modules/keyvault/custom.py b/src/azure-cli/azure/cli/command_modules/keyvault/custom.py index 9cc5f7e3916..e3785ab1b7d 100644 --- a/src/azure-cli/azure/cli/command_modules/keyvault/custom.py +++ b/src/azure-cli/azure/cli/command_modules/keyvault/custom.py @@ -2517,3 +2517,80 @@ def set_attributes_certificate(client, certificate_name, version=None, policy=No if kwargs.get('enabled') is not None or kwargs.get('tags') is not None: return client.update_certificate_properties(certificate_name=certificate_name, version=version, **kwargs) return client.get_certificate(certificate_name=certificate_name) + + +def copy_secret(cmd, client, destination_vault, name=None, all_secrets=False, rewrite=False): + from azure.core.exceptions import ResourceNotFoundError, HttpResponseError + from azure.keyvault.secrets import SecretClient + from azure.cli.core._profile import Profile + from azure.cli.core.commands.client_factory import prepare_client_kwargs_track2 + + if not name: + all_secrets = True + + if name and all_secrets: + all_secrets = False + + # Validation + if client.vault_url.rstrip('/') == destination_vault.rstrip('/'): + raise CLIError("Source and destination Key Vaults cannot be the same.") + + profile = Profile(cli_ctx=cmd.cli_ctx) + credential, _, _ = profile.get_login_credentials(subscription_id=cmd.cli_ctx.data.get('subscription_id')) + + # Use standard client kwargs for consistent logging/telemetry + client_kwargs = prepare_client_kwargs_track2(cmd.cli_ctx) + client_kwargs.pop('http_logging_policy', None) # KeyVault clients handle this internally or differently sometimes, mimicking _client_factory + + dest_client = SecretClient(vault_url=destination_vault, credential=credential, **client_kwargs) + + secrets_to_copy = [] + if name: + secrets_to_copy.append(name) + else: + logger.warning("Copying all secrets from source...") + try: + source_secrets = client.list_properties_of_secrets() + for s in source_secrets: + if s.managed: + logger.warning(f"Skipping managed secret: {s.name}") + continue + secrets_to_copy.append(s.name) + except HttpResponseError as e: + raise CLIError(f"Failed to list secrets from source: {str(e)}") + + for secret_name in secrets_to_copy: + try: + # Check destination + if not rewrite: + try: + dest_client.get_secret(secret_name) + logger.warning(f"Secret '{secret_name}' already exists in destination. Skipping.") + continue + except ResourceNotFoundError: + pass + except HttpResponseError as e: + logger.warning(f"Error checking secret '{secret_name}' in destination: {str(e)}") + if e.status_code == 403: + logger.error(f"Access denied checking secret '{secret_name}' in destination.") + continue + + # Copy + logger.info(f"Copying secret: {secret_name}") + s = client.get_secret(secret_name) + + dest_client.set_secret( + s.name, + s.value, + content_type=s.properties.content_type, + tags=s.properties.tags, + enabled=s.properties.enabled, + not_before=s.properties.not_before, + expires_on=s.properties.expires_on + ) + + except HttpResponseError as e: + if e.status_code == 403: # Forbidden + logger.error(f"Access denied (403) for secret '{secret_name}': {str(e)}") + else: + logger.error(f"Failed to copy secret '{secret_name}': {str(e)}") diff --git a/src/azure-cli/azure/cli/command_modules/keyvault/tests/latest/test_keyvault_copy.py b/src/azure-cli/azure/cli/command_modules/keyvault/tests/latest/test_keyvault_copy.py new file mode 100644 index 00000000000..28d41e27786 --- /dev/null +++ b/src/azure-cli/azure/cli/command_modules/keyvault/tests/latest/test_keyvault_copy.py @@ -0,0 +1,66 @@ +# -------------------------------------------------------------------------------------------- +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. See License.txt in the project root for license information. +# -------------------------------------------------------------------------------------------- + +from azure.cli.testsdk import ( + ResourceGroupPreparer, + KeyVaultPreparer, + ScenarioTest +) + +class KeyVaultCopyScenarioTest(ScenarioTest): + @ResourceGroupPreparer(name_prefix='cli_test_keyvault_copy') + @KeyVaultPreparer(name_prefix='cli-test-kv-src-') + def test_keyvault_secret_copy(self, resource_group, key_vault): + src_kv = key_vault + dest_kv = self.create_random_name('cli-test-kv-dest-', 24) + secret_name = self.create_random_name('secret-', 24) + secret_value = 'mysecretvalue' + + # Create Dest KV + # Use simple creation to ensure speed and reliability in playback + self.cmd('keyvault create -g {rg} -n ' + dest_kv) + + # Set secret in Source + self.cmd('keyvault secret set --vault-name {kv} -n ' + secret_name + ' --value ' + secret_value) + + # 1. Copy specific secret + self.cmd('keyvault secret copy --source-vault {kv} --destination-vault ' + dest_kv + ' --name ' + secret_name) + self.cmd('keyvault secret show --vault-name ' + dest_kv + ' -n ' + secret_name, checks=[ + self.check('value', secret_value) + ]) + + # 2. Copy all secrets + # Add another secret to source + secret_name_2 = self.create_random_name('secret2-', 24) + self.cmd('keyvault secret set --vault-name {kv} -n ' + secret_name_2 + ' --value ' + secret_value) + + # Run copy --all + self.cmd('keyvault secret copy --source-vault {kv} --destination-vault ' + dest_kv + ' --all') + + # Verify both exist in dest + self.cmd('keyvault secret show --vault-name ' + dest_kv + ' -n ' + secret_name_2, checks=[ + self.check('value', secret_value) + ]) + + # 3. Test overwrite protection (default behavior: skip) + new_val = 'newval' + # Update source + self.cmd('keyvault secret set --vault-name {kv} -n ' + secret_name + ' --value ' + new_val) + + # Copy without rewrite (should skip) + self.cmd('keyvault secret copy --source-vault {kv} --destination-vault ' + dest_kv + ' --name ' + secret_name) + + # Verify destination still has old value + self.cmd('keyvault secret show --vault-name ' + dest_kv + ' -n ' + secret_name, checks=[ + self.check('value', secret_value) + ]) + + # 4. Test Rewrite + self.cmd('keyvault secret copy --source-vault {kv} --destination-vault ' + dest_kv + ' --name ' + secret_name + ' --rewrite') + + # Verify destination has new value + self.cmd('keyvault secret show --vault-name ' + dest_kv + ' -n ' + secret_name, checks=[ + self.check('value', new_val) + ]) From 2e413922da28b37dabaa1743531cfff1a09e218b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Cassanji?= Date: Wed, 4 Feb 2026 20:27:18 -0300 Subject: [PATCH 02/35] keyvault secret copy command improvements --- .../azure/cli/command_modules/keyvault/_help.py | 14 ++++++++++++++ .../azure/cli/command_modules/keyvault/_params.py | 2 +- .../azure/cli/command_modules/keyvault/custom.py | 12 +++++++++--- .../keyvault/tests/latest/test_keyvault_copy.py | 2 +- 4 files changed, 25 insertions(+), 5 deletions(-) diff --git a/src/azure-cli/azure/cli/command_modules/keyvault/_help.py b/src/azure-cli/azure/cli/command_modules/keyvault/_help.py index 525b4c5b159..7fc77f3465a 100644 --- a/src/azure-cli/azure/cli/command_modules/keyvault/_help.py +++ b/src/azure-cli/azure/cli/command_modules/keyvault/_help.py @@ -906,6 +906,20 @@ the secret will be downloaded. This operation requires the secrets/backup permission. """ +helps['keyvault secret copy'] = """ +type: command +short-summary: Copy a secret from one Key Vault to another. +long-summary: Copies the latest version of a secret from a source Key Vault to a destination Key Vault. + This operation copies the secret value and its metadata (tags, content-type, attributes). +examples: + - name: Copy a specific secret from one vault to another. + text: az keyvault secret copy --source-vault SourceVault --destination-vault DestVault --name MySecret + - name: Copy all secrets from one vault to another. + text: az keyvault secret copy --source-vault SourceVault --destination-vault DestVault --all + - name: Copy a secret and overwrite if it already exists in the destination. + text: az keyvault secret copy --source-vault SourceVault --destination-vault DestVault --name MySecret --overwrite +""" + helps['keyvault secret restore'] = """ type: command short-summary: Restores a backed up secret to a vault. diff --git a/src/azure-cli/azure/cli/command_modules/keyvault/_params.py b/src/azure-cli/azure/cli/command_modules/keyvault/_params.py index 9a47d1f8307..b8b20b0ba91 100644 --- a/src/azure-cli/azure/cli/command_modules/keyvault/_params.py +++ b/src/azure-cli/azure/cli/command_modules/keyvault/_params.py @@ -576,7 +576,7 @@ class CLISecurityDomainOperation(str, Enum): options_list=['--destination-vault'], help='Name of the destination Key Vault.', required=True) c.argument('name', options_list=['--name', '-n'], help='Name of the secret to copy.', required=False) c.extra('all_secrets', arg_type=get_three_state_flag(), options_list=['--all'], help='Copy all secrets.') - c.extra('rewrite', arg_type=get_three_state_flag(), help='Overwrite existing secrets in destination.') + c.extra('overwrite', arg_type=get_three_state_flag(), help='Overwrite existing secrets in destination.') # endregion # region keyvault security-domain diff --git a/src/azure-cli/azure/cli/command_modules/keyvault/custom.py b/src/azure-cli/azure/cli/command_modules/keyvault/custom.py index e3785ab1b7d..0b7c21e46c5 100644 --- a/src/azure-cli/azure/cli/command_modules/keyvault/custom.py +++ b/src/azure-cli/azure/cli/command_modules/keyvault/custom.py @@ -2519,7 +2519,7 @@ def set_attributes_certificate(client, certificate_name, version=None, policy=No return client.get_certificate(certificate_name=certificate_name) -def copy_secret(cmd, client, destination_vault, name=None, all_secrets=False, rewrite=False): +def copy_secret(cmd, client, destination_vault, name=None, all_secrets=False, overwrite=False): from azure.core.exceptions import ResourceNotFoundError, HttpResponseError from azure.keyvault.secrets import SecretClient from azure.cli.core._profile import Profile @@ -2559,10 +2559,11 @@ def copy_secret(cmd, client, destination_vault, name=None, all_secrets=False, re except HttpResponseError as e: raise CLIError(f"Failed to list secrets from source: {str(e)}") + copied_secrets = [] for secret_name in secrets_to_copy: try: # Check destination - if not rewrite: + if not overwrite: try: dest_client.get_secret(secret_name) logger.warning(f"Secret '{secret_name}' already exists in destination. Skipping.") @@ -2579,7 +2580,7 @@ def copy_secret(cmd, client, destination_vault, name=None, all_secrets=False, re logger.info(f"Copying secret: {secret_name}") s = client.get_secret(secret_name) - dest_client.set_secret( + new_secret = dest_client.set_secret( s.name, s.value, content_type=s.properties.content_type, @@ -2588,9 +2589,14 @@ def copy_secret(cmd, client, destination_vault, name=None, all_secrets=False, re not_before=s.properties.not_before, expires_on=s.properties.expires_on ) + + logger.warning(f"Successfully copied secret: {secret_name}") + copied_secrets.append({'name': new_secret.name, 'id': new_secret.id}) except HttpResponseError as e: if e.status_code == 403: # Forbidden logger.error(f"Access denied (403) for secret '{secret_name}': {str(e)}") else: logger.error(f"Failed to copy secret '{secret_name}': {str(e)}") + + return copied_secrets diff --git a/src/azure-cli/azure/cli/command_modules/keyvault/tests/latest/test_keyvault_copy.py b/src/azure-cli/azure/cli/command_modules/keyvault/tests/latest/test_keyvault_copy.py index 28d41e27786..1851bea9318 100644 --- a/src/azure-cli/azure/cli/command_modules/keyvault/tests/latest/test_keyvault_copy.py +++ b/src/azure-cli/azure/cli/command_modules/keyvault/tests/latest/test_keyvault_copy.py @@ -58,7 +58,7 @@ def test_keyvault_secret_copy(self, resource_group, key_vault): ]) # 4. Test Rewrite - self.cmd('keyvault secret copy --source-vault {kv} --destination-vault ' + dest_kv + ' --name ' + secret_name + ' --rewrite') + self.cmd('keyvault secret copy --source-vault {kv} --destination-vault ' + dest_kv + ' --name ' + secret_name + ' --overwrite') # Verify destination has new value self.cmd('keyvault secret show --vault-name ' + dest_kv + ' -n ' + secret_name, checks=[ From ec4d3789f16c0b44741c77f389a67d1e6a725f08 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Cassanji?= Date: Wed, 4 Feb 2026 20:39:38 -0300 Subject: [PATCH 03/35] Move test case to file --- .../tests/latest/test_keyvault_commands.py | 58 ++++++++++++++++ .../tests/latest/test_keyvault_copy.py | 66 ------------------- 2 files changed, 58 insertions(+), 66 deletions(-) delete mode 100644 src/azure-cli/azure/cli/command_modules/keyvault/tests/latest/test_keyvault_copy.py diff --git a/src/azure-cli/azure/cli/command_modules/keyvault/tests/latest/test_keyvault_commands.py b/src/azure-cli/azure/cli/command_modules/keyvault/tests/latest/test_keyvault_commands.py index a1df39f8ac5..72082b43b28 100644 --- a/src/azure-cli/azure/cli/command_modules/keyvault/tests/latest/test_keyvault_commands.py +++ b/src/azure-cli/azure/cli/command_modules/keyvault/tests/latest/test_keyvault_commands.py @@ -2780,3 +2780,61 @@ def test_keyvault_mhsm_region(self, resource_group, managed_hsm): if __name__ == '__main__': unittest.main() + +class KeyVaultCopyScenarioTest(ScenarioTest): + @ResourceGroupPreparer(name_prefix='cli_test_keyvault_copy') + @KeyVaultPreparer(name_prefix='cli-test-kv-src-') + def test_keyvault_secret_copy(self, resource_group, key_vault): + src_kv = key_vault + dest_kv = self.create_random_name('cli-test-kv-dest-', 24) + secret_name = self.create_random_name('secret-', 24) + secret_value = 'mysecretvalue' + + # Create Dest KV + # Use simple creation to ensure speed and reliability in playback + self.cmd('keyvault create -g {rg} -n ' + dest_kv) + + # Set secret in Source with tags and content-type + self.cmd('keyvault secret set --vault-name {kv} -n ' + secret_name + ' --value ' + secret_value + ' --tags tag1=value1 --content-type text/plain') + + # 1. Copy specific secret + self.cmd('keyvault secret copy --source-vault {kv} --destination-vault ' + dest_kv + ' --name ' + secret_name) + self.cmd('keyvault secret show --vault-name ' + dest_kv + ' -n ' + secret_name, checks=[ + self.check('value', secret_value), + self.check('tags.tag1', 'value1'), + self.check('contentType', 'text/plain') + ]) + + # 2. Copy all secrets + # Add another secret to source + secret_name_2 = self.create_random_name('secret2-', 24) + self.cmd('keyvault secret set --vault-name {kv} -n ' + secret_name_2 + ' --value ' + secret_value) + + # Run copy --all + self.cmd('keyvault secret copy --source-vault {kv} --destination-vault ' + dest_kv + ' --all') + + # Verify both exist in dest + self.cmd('keyvault secret show --vault-name ' + dest_kv + ' -n ' + secret_name_2, checks=[ + self.check('value', secret_value) + ]) + + # 3. Test overwrite protection (default behavior: skip) + new_val = 'newval' + # Update source + self.cmd('keyvault secret set --vault-name {kv} -n ' + secret_name + ' --value ' + new_val) + + # Copy without rewrite (should skip) + self.cmd('keyvault secret copy --source-vault {kv} --destination-vault ' + dest_kv + ' --name ' + secret_name) + + # Verify destination still has old value + self.cmd('keyvault secret show --vault-name ' + dest_kv + ' -n ' + secret_name, checks=[ + self.check('value', secret_value) + ]) + + # 4. Test Rewrite + self.cmd('keyvault secret copy --source-vault {kv} --destination-vault ' + dest_kv + ' --name ' + secret_name + ' --overwrite') + + # Verify destination has new value + self.cmd('keyvault secret show --vault-name ' + dest_kv + ' -n ' + secret_name, checks=[ + self.check('value', new_val) + ]) diff --git a/src/azure-cli/azure/cli/command_modules/keyvault/tests/latest/test_keyvault_copy.py b/src/azure-cli/azure/cli/command_modules/keyvault/tests/latest/test_keyvault_copy.py deleted file mode 100644 index 1851bea9318..00000000000 --- a/src/azure-cli/azure/cli/command_modules/keyvault/tests/latest/test_keyvault_copy.py +++ /dev/null @@ -1,66 +0,0 @@ -# -------------------------------------------------------------------------------------------- -# Copyright (c) Microsoft Corporation. All rights reserved. -# Licensed under the MIT License. See License.txt in the project root for license information. -# -------------------------------------------------------------------------------------------- - -from azure.cli.testsdk import ( - ResourceGroupPreparer, - KeyVaultPreparer, - ScenarioTest -) - -class KeyVaultCopyScenarioTest(ScenarioTest): - @ResourceGroupPreparer(name_prefix='cli_test_keyvault_copy') - @KeyVaultPreparer(name_prefix='cli-test-kv-src-') - def test_keyvault_secret_copy(self, resource_group, key_vault): - src_kv = key_vault - dest_kv = self.create_random_name('cli-test-kv-dest-', 24) - secret_name = self.create_random_name('secret-', 24) - secret_value = 'mysecretvalue' - - # Create Dest KV - # Use simple creation to ensure speed and reliability in playback - self.cmd('keyvault create -g {rg} -n ' + dest_kv) - - # Set secret in Source - self.cmd('keyvault secret set --vault-name {kv} -n ' + secret_name + ' --value ' + secret_value) - - # 1. Copy specific secret - self.cmd('keyvault secret copy --source-vault {kv} --destination-vault ' + dest_kv + ' --name ' + secret_name) - self.cmd('keyvault secret show --vault-name ' + dest_kv + ' -n ' + secret_name, checks=[ - self.check('value', secret_value) - ]) - - # 2. Copy all secrets - # Add another secret to source - secret_name_2 = self.create_random_name('secret2-', 24) - self.cmd('keyvault secret set --vault-name {kv} -n ' + secret_name_2 + ' --value ' + secret_value) - - # Run copy --all - self.cmd('keyvault secret copy --source-vault {kv} --destination-vault ' + dest_kv + ' --all') - - # Verify both exist in dest - self.cmd('keyvault secret show --vault-name ' + dest_kv + ' -n ' + secret_name_2, checks=[ - self.check('value', secret_value) - ]) - - # 3. Test overwrite protection (default behavior: skip) - new_val = 'newval' - # Update source - self.cmd('keyvault secret set --vault-name {kv} -n ' + secret_name + ' --value ' + new_val) - - # Copy without rewrite (should skip) - self.cmd('keyvault secret copy --source-vault {kv} --destination-vault ' + dest_kv + ' --name ' + secret_name) - - # Verify destination still has old value - self.cmd('keyvault secret show --vault-name ' + dest_kv + ' -n ' + secret_name, checks=[ - self.check('value', secret_value) - ]) - - # 4. Test Rewrite - self.cmd('keyvault secret copy --source-vault {kv} --destination-vault ' + dest_kv + ' --name ' + secret_name + ' --overwrite') - - # Verify destination has new value - self.cmd('keyvault secret show --vault-name ' + dest_kv + ' -n ' + secret_name, checks=[ - self.check('value', new_val) - ]) From fe468ec8454f4b670fff282cf2824361bae1bd98 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Cassanji?= Date: Wed, 4 Feb 2026 22:50:11 -0300 Subject: [PATCH 04/35] Apply PR feedbacks --- .../cli/command_modules/keyvault/custom.py | 20 +++++- .../tests/latest/test_keyvault_commands.py | 64 ++++++++++++------- 2 files changed, 57 insertions(+), 27 deletions(-) diff --git a/src/azure-cli/azure/cli/command_modules/keyvault/custom.py b/src/azure-cli/azure/cli/command_modules/keyvault/custom.py index 0b7c21e46c5..1a96a6cba35 100644 --- a/src/azure-cli/azure/cli/command_modules/keyvault/custom.py +++ b/src/azure-cli/azure/cli/command_modules/keyvault/custom.py @@ -2525,11 +2525,15 @@ def copy_secret(cmd, client, destination_vault, name=None, all_secrets=False, ov from azure.cli.core._profile import Profile from azure.cli.core.commands.client_factory import prepare_client_kwargs_track2 - if not name: + from azure.cli.core.azclierror import MutuallyExclusiveArgumentError + + # If neither a specific secret name nor --all is provided, default to copying all secrets. + if not name and not all_secrets: all_secrets = True + # A specific secret name and --all are mutually exclusive. if name and all_secrets: - all_secrets = False + raise MutuallyExclusiveArgumentError("Specify either a secret name or --all, but not both.") # Validation if client.vault_url.rstrip('/') == destination_vault.rstrip('/'): @@ -2544,6 +2548,16 @@ def copy_secret(cmd, client, destination_vault, name=None, all_secrets=False, ov dest_client = SecretClient(vault_url=destination_vault, credential=credential, **client_kwargs) + # Fail fast if destination vault is not accessible or does not exist + try: + # Force a call to the service; if the vault is empty this will raise StopIteration, which is fine + next(dest_client.list_properties_of_secrets()) + except StopIteration: + # Vault is accessible but has no secrets yet + pass + except HttpResponseError as e: + raise CLIError(f"Failed to access destination Key Vault '{destination_vault}': {str(e)}") + secrets_to_copy = [] if name: secrets_to_copy.append(name) @@ -2590,7 +2604,7 @@ def copy_secret(cmd, client, destination_vault, name=None, all_secrets=False, ov expires_on=s.properties.expires_on ) - logger.warning(f"Successfully copied secret: {secret_name}") + logger.info(f"Successfully copied secret: {secret_name}") copied_secrets.append({'name': new_secret.name, 'id': new_secret.id}) except HttpResponseError as e: diff --git a/src/azure-cli/azure/cli/command_modules/keyvault/tests/latest/test_keyvault_commands.py b/src/azure-cli/azure/cli/command_modules/keyvault/tests/latest/test_keyvault_commands.py index 72082b43b28..1340858530c 100644 --- a/src/azure-cli/azure/cli/command_modules/keyvault/tests/latest/test_keyvault_commands.py +++ b/src/azure-cli/azure/cli/command_modules/keyvault/tests/latest/test_keyvault_commands.py @@ -2785,56 +2785,72 @@ class KeyVaultCopyScenarioTest(ScenarioTest): @ResourceGroupPreparer(name_prefix='cli_test_keyvault_copy') @KeyVaultPreparer(name_prefix='cli-test-kv-src-') def test_keyvault_secret_copy(self, resource_group, key_vault): - src_kv = key_vault - dest_kv = self.create_random_name('cli-test-kv-dest-', 24) - secret_name = self.create_random_name('secret-', 24) - secret_value = 'mysecretvalue' + self.kwargs.update({ + 'src_kv': key_vault, + 'dest_kv': self.create_random_name('cli-test-kv-dest-', 24), + 'secret_name': self.create_random_name('secret-', 24), + 'secret_value': 'mysecretvalue', + 'new_val': 'newval', + 'secret_name_2': self.create_random_name('secret2-', 24) + }) # Create Dest KV # Use simple creation to ensure speed and reliability in playback - self.cmd('keyvault create -g {rg} -n ' + dest_kv) + self.cmd('keyvault create -g {rg} -n {dest_kv}') + self.addCleanup(self.cmd, 'keyvault delete -g {rg} -n {dest_kv}') + self.addCleanup(self.cmd, 'keyvault purge -n {dest_kv} -l eastus') # Set secret in Source with tags and content-type - self.cmd('keyvault secret set --vault-name {kv} -n ' + secret_name + ' --value ' + secret_value + ' --tags tag1=value1 --content-type text/plain') + self.cmd('keyvault secret set --vault-name {kv} -n {secret_name} --value {secret_value} --tags tag1=value1 --content-type text/plain') # 1. Copy specific secret - self.cmd('keyvault secret copy --source-vault {kv} --destination-vault ' + dest_kv + ' --name ' + secret_name) - self.cmd('keyvault secret show --vault-name ' + dest_kv + ' -n ' + secret_name, checks=[ - self.check('value', secret_value), + self.cmd('keyvault secret copy --source-vault {kv} --destination-vault {dest_kv} --name {secret_name}') + self.cmd('keyvault secret show --vault-name {dest_kv} -n {secret_name}', checks=[ + self.check('value', '{secret_value}'), self.check('tags.tag1', 'value1'), self.check('contentType', 'text/plain') ]) # 2. Copy all secrets # Add another secret to source - secret_name_2 = self.create_random_name('secret2-', 24) - self.cmd('keyvault secret set --vault-name {kv} -n ' + secret_name_2 + ' --value ' + secret_value) + self.cmd('keyvault secret set --vault-name {kv} -n {secret_name_2} --value {secret_value}') # Run copy --all - self.cmd('keyvault secret copy --source-vault {kv} --destination-vault ' + dest_kv + ' --all') + self.cmd('keyvault secret copy --source-vault {kv} --destination-vault {dest_kv} --all') # Verify both exist in dest - self.cmd('keyvault secret show --vault-name ' + dest_kv + ' -n ' + secret_name_2, checks=[ - self.check('value', secret_value) + self.cmd('keyvault secret show --vault-name {dest_kv} -n {secret_name_2}', checks=[ + self.check('value', '{secret_value}') ]) # 3. Test overwrite protection (default behavior: skip) - new_val = 'newval' # Update source - self.cmd('keyvault secret set --vault-name {kv} -n ' + secret_name + ' --value ' + new_val) + self.cmd('keyvault secret set --vault-name {kv} -n {secret_name} --value {new_val}') - # Copy without rewrite (should skip) - self.cmd('keyvault secret copy --source-vault {kv} --destination-vault ' + dest_kv + ' --name ' + secret_name) + # Copy without overwrite (should skip) + self.cmd('keyvault secret copy --source-vault {kv} --destination-vault {dest_kv} --name {secret_name}') # Verify destination still has old value - self.cmd('keyvault secret show --vault-name ' + dest_kv + ' -n ' + secret_name, checks=[ - self.check('value', secret_value) + self.cmd('keyvault secret show --vault-name {dest_kv} -n {secret_name}', checks=[ + self.check('value', '{secret_value}') ]) - # 4. Test Rewrite - self.cmd('keyvault secret copy --source-vault {kv} --destination-vault ' + dest_kv + ' --name ' + secret_name + ' --overwrite') + # 4. Test overwrite + self.cmd('keyvault secret copy --source-vault {kv} --destination-vault {dest_kv} --name {secret_name} --overwrite') # Verify destination has new value - self.cmd('keyvault secret show --vault-name ' + dest_kv + ' -n ' + secret_name, checks=[ - self.check('value', new_val) + self.cmd('keyvault secret show --vault-name {dest_kv} -n {secret_name}', checks=[ + self.check('value', '{new_val}') ]) + + # 5. Test Mutual Exclusivity + self.cmd('keyvault secret copy --source-vault {kv} --destination-vault {dest_kv} --name {secret_name} --all', expect_failure=True) + + # 6. Test Source == Destination (Should fail) + self.cmd('keyvault secret copy --source-vault {kv} --destination-vault {kv} --name {secret_name}', expect_failure=True) + + # 7. Test Non-existent Destination (Should fail fast) + self.cmd('keyvault secret copy --source-vault {kv} --destination-vault non_existent_kv_12345 --name {secret_name}', expect_failure=True) + + # 8. Test Non-existent Secret in Source (Should fail) + self.cmd('keyvault secret copy --source-vault {kv} --destination-vault {dest_kv} --name non_existent_secret_123', expect_failure=True) From ad02119379a1daf7c75a90c182aa4af129fc0f6e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Cassanji?= <111895926+jcassanji-southworks@users.noreply.github.com> Date: Wed, 4 Feb 2026 23:02:35 -0300 Subject: [PATCH 05/35] Update src/azure-cli/azure/cli/command_modules/keyvault/tests/latest/test_keyvault_commands.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .../keyvault/tests/latest/test_keyvault_commands.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/azure-cli/azure/cli/command_modules/keyvault/tests/latest/test_keyvault_commands.py b/src/azure-cli/azure/cli/command_modules/keyvault/tests/latest/test_keyvault_commands.py index 1340858530c..5d9d566e6a1 100644 --- a/src/azure-cli/azure/cli/command_modules/keyvault/tests/latest/test_keyvault_commands.py +++ b/src/azure-cli/azure/cli/command_modules/keyvault/tests/latest/test_keyvault_commands.py @@ -2798,7 +2798,7 @@ def test_keyvault_secret_copy(self, resource_group, key_vault): # Use simple creation to ensure speed and reliability in playback self.cmd('keyvault create -g {rg} -n {dest_kv}') self.addCleanup(self.cmd, 'keyvault delete -g {rg} -n {dest_kv}') - self.addCleanup(self.cmd, 'keyvault purge -n {dest_kv} -l eastus') + self.addCleanup(self.cmd, 'keyvault purge -n {dest_kv}') # Set secret in Source with tags and content-type self.cmd('keyvault secret set --vault-name {kv} -n {secret_name} --value {secret_value} --tags tag1=value1 --content-type text/plain') From 0b8e4cc218241809d455ae8c8a7f390b72116c7c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Cassanji?= <111895926+jcassanji-southworks@users.noreply.github.com> Date: Wed, 4 Feb 2026 23:06:48 -0300 Subject: [PATCH 06/35] Update src/azure-cli/azure/cli/command_modules/keyvault/custom.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- src/azure-cli/azure/cli/command_modules/keyvault/custom.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/azure-cli/azure/cli/command_modules/keyvault/custom.py b/src/azure-cli/azure/cli/command_modules/keyvault/custom.py index 1a96a6cba35..399c321623b 100644 --- a/src/azure-cli/azure/cli/command_modules/keyvault/custom.py +++ b/src/azure-cli/azure/cli/command_modules/keyvault/custom.py @@ -2544,7 +2544,6 @@ def copy_secret(cmd, client, destination_vault, name=None, all_secrets=False, ov # Use standard client kwargs for consistent logging/telemetry client_kwargs = prepare_client_kwargs_track2(cmd.cli_ctx) - client_kwargs.pop('http_logging_policy', None) # KeyVault clients handle this internally or differently sometimes, mimicking _client_factory dest_client = SecretClient(vault_url=destination_vault, credential=credential, **client_kwargs) From e90b9bd4ee0bfc0c31650211241ee71a34f2c30b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Cassanji?= <111895926+jcassanji-southworks@users.noreply.github.com> Date: Wed, 4 Feb 2026 23:07:03 -0300 Subject: [PATCH 07/35] Update src/azure-cli/azure/cli/command_modules/keyvault/custom.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- src/azure-cli/azure/cli/command_modules/keyvault/custom.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/azure-cli/azure/cli/command_modules/keyvault/custom.py b/src/azure-cli/azure/cli/command_modules/keyvault/custom.py index 399c321623b..b2445664ca0 100644 --- a/src/azure-cli/azure/cli/command_modules/keyvault/custom.py +++ b/src/azure-cli/azure/cli/command_modules/keyvault/custom.py @@ -2534,7 +2534,6 @@ def copy_secret(cmd, client, destination_vault, name=None, all_secrets=False, ov # A specific secret name and --all are mutually exclusive. if name and all_secrets: raise MutuallyExclusiveArgumentError("Specify either a secret name or --all, but not both.") - # Validation if client.vault_url.rstrip('/') == destination_vault.rstrip('/'): raise CLIError("Source and destination Key Vaults cannot be the same.") From e9722b8da81664541e8d708ed1c1c68c4b8b04fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Cassanji?= <111895926+jcassanji-southworks@users.noreply.github.com> Date: Wed, 4 Feb 2026 23:07:17 -0300 Subject: [PATCH 08/35] Update src/azure-cli/azure/cli/command_modules/keyvault/tests/latest/test_keyvault_commands.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .../keyvault/tests/latest/test_keyvault_commands.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/azure-cli/azure/cli/command_modules/keyvault/tests/latest/test_keyvault_commands.py b/src/azure-cli/azure/cli/command_modules/keyvault/tests/latest/test_keyvault_commands.py index 5d9d566e6a1..dffd59b8213 100644 --- a/src/azure-cli/azure/cli/command_modules/keyvault/tests/latest/test_keyvault_commands.py +++ b/src/azure-cli/azure/cli/command_modules/keyvault/tests/latest/test_keyvault_commands.py @@ -2832,7 +2832,7 @@ def test_keyvault_secret_copy(self, resource_group, key_vault): # Verify destination still has old value self.cmd('keyvault secret show --vault-name {dest_kv} -n {secret_name}', checks=[ - self.check('value', '{secret_value}') + self.check('value', '{secret_value}') ]) # 4. Test overwrite From 90c664fc1a3dd2637a87d796fe8a0b7787d37897 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Cassanji?= <111895926+jcassanji-southworks@users.noreply.github.com> Date: Wed, 4 Feb 2026 23:09:37 -0300 Subject: [PATCH 09/35] Update src/azure-cli/azure/cli/command_modules/keyvault/_params.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .../azure/cli/command_modules/keyvault/_params.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/azure-cli/azure/cli/command_modules/keyvault/_params.py b/src/azure-cli/azure/cli/command_modules/keyvault/_params.py index b8b20b0ba91..06b668d53b5 100644 --- a/src/azure-cli/azure/cli/command_modules/keyvault/_params.py +++ b/src/azure-cli/azure/cli/command_modules/keyvault/_params.py @@ -574,8 +574,12 @@ class CLISecurityDomainOperation(str, Enum): options_list=['--source-vault'], help='Name of the source Key Vault.', required=True) c.extra('destination_vault', vault_name_type, type=get_vault_base_url_type(self.cli_ctx), options_list=['--destination-vault'], help='Name of the destination Key Vault.', required=True) - c.argument('name', options_list=['--name', '-n'], help='Name of the secret to copy.', required=False) - c.extra('all_secrets', arg_type=get_three_state_flag(), options_list=['--all'], help='Copy all secrets.') + c.argument('name', options_list=['--name', '-n'], + help='Name of the secret to copy. Cannot be used with --all. If omitted, you must specify --all.', + required=False) + c.extra('all_secrets', arg_type=get_three_state_flag(), options_list=['--all'], + help='Copy all secrets from the source vault. Cannot be used with --name. If omitted, you must ' + 'specify --name.') c.extra('overwrite', arg_type=get_three_state_flag(), help='Overwrite existing secrets in destination.') # endregion From a140d6c04d79d4083d92a865da4cadd98acc3d81 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Cassanji?= Date: Wed, 4 Feb 2026 23:11:40 -0300 Subject: [PATCH 10/35] Apply PR feedback --- .../azure/cli/command_modules/keyvault/custom.py | 13 +++++++++++++ .../keyvault/tests/latest/test_keyvault_commands.py | 6 +++--- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/src/azure-cli/azure/cli/command_modules/keyvault/custom.py b/src/azure-cli/azure/cli/command_modules/keyvault/custom.py index 1a96a6cba35..93a0894ac7b 100644 --- a/src/azure-cli/azure/cli/command_modules/keyvault/custom.py +++ b/src/azure-cli/azure/cli/command_modules/keyvault/custom.py @@ -2574,6 +2574,7 @@ def copy_secret(cmd, client, destination_vault, name=None, all_secrets=False, ov raise CLIError(f"Failed to list secrets from source: {str(e)}") copied_secrets = [] + failed_secrets = [] for secret_name in secrets_to_copy: try: # Check destination @@ -2586,6 +2587,7 @@ def copy_secret(cmd, client, destination_vault, name=None, all_secrets=False, ov pass except HttpResponseError as e: logger.warning(f"Error checking secret '{secret_name}' in destination: {str(e)}") + failed_secrets.append(secret_name) if e.status_code == 403: logger.error(f"Access denied checking secret '{secret_name}' in destination.") continue @@ -2607,10 +2609,21 @@ def copy_secret(cmd, client, destination_vault, name=None, all_secrets=False, ov logger.info(f"Successfully copied secret: {secret_name}") copied_secrets.append({'name': new_secret.name, 'id': new_secret.id}) + except ResourceNotFoundError: + if name: + raise CLIError(f"Secret '{secret_name}' not found in source vault.") + logger.error(f"Secret '{secret_name}' not found in source vault.") + failed_secrets.append(secret_name) except HttpResponseError as e: + if name: + raise CLIError(f"Failed to copy secret '{secret_name}': {str(e)}") + failed_secrets.append(secret_name) if e.status_code == 403: # Forbidden logger.error(f"Access denied (403) for secret '{secret_name}': {str(e)}") else: logger.error(f"Failed to copy secret '{secret_name}': {str(e)}") + if failed_secrets: + logger.warning(f"Operation completed with failures. {len(failed_secrets)} secrets failed to copy: {', '.join(failed_secrets)}") + return copied_secrets diff --git a/src/azure-cli/azure/cli/command_modules/keyvault/tests/latest/test_keyvault_commands.py b/src/azure-cli/azure/cli/command_modules/keyvault/tests/latest/test_keyvault_commands.py index 1340858530c..3ff5baf6908 100644 --- a/src/azure-cli/azure/cli/command_modules/keyvault/tests/latest/test_keyvault_commands.py +++ b/src/azure-cli/azure/cli/command_modules/keyvault/tests/latest/test_keyvault_commands.py @@ -2778,9 +2778,6 @@ def test_keyvault_mhsm_region(self, resource_group, managed_hsm): self.cmd('keyvault region remove -g {rg} --hsm-name {hsm_name} -r uksouth') -if __name__ == '__main__': - unittest.main() - class KeyVaultCopyScenarioTest(ScenarioTest): @ResourceGroupPreparer(name_prefix='cli_test_keyvault_copy') @KeyVaultPreparer(name_prefix='cli-test-kv-src-') @@ -2854,3 +2851,6 @@ def test_keyvault_secret_copy(self, resource_group, key_vault): # 8. Test Non-existent Secret in Source (Should fail) self.cmd('keyvault secret copy --source-vault {kv} --destination-vault {dest_kv} --name non_existent_secret_123', expect_failure=True) + +if __name__ == '__main__': + unittest.main() From de2f2654f08d57111f5c3aa4fae1d0167b42c107 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Cassanji?= <111895926+jcassanji-southworks@users.noreply.github.com> Date: Wed, 4 Feb 2026 23:23:43 -0300 Subject: [PATCH 11/35] Update src/azure-cli/azure/cli/command_modules/keyvault/_params.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .../azure/cli/command_modules/keyvault/_params.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/azure-cli/azure/cli/command_modules/keyvault/_params.py b/src/azure-cli/azure/cli/command_modules/keyvault/_params.py index 06b668d53b5..3cc398dd516 100644 --- a/src/azure-cli/azure/cli/command_modules/keyvault/_params.py +++ b/src/azure-cli/azure/cli/command_modules/keyvault/_params.py @@ -575,11 +575,12 @@ class CLISecurityDomainOperation(str, Enum): c.extra('destination_vault', vault_name_type, type=get_vault_base_url_type(self.cli_ctx), options_list=['--destination-vault'], help='Name of the destination Key Vault.', required=True) c.argument('name', options_list=['--name', '-n'], - help='Name of the secret to copy. Cannot be used with --all. If omitted, you must specify --all.', + help='Name of the secret to copy. Mutually exclusive with --all. If neither --name nor --all is ' + 'specified, all secrets will be copied.', required=False) c.extra('all_secrets', arg_type=get_three_state_flag(), options_list=['--all'], - help='Copy all secrets from the source vault. Cannot be used with --name. If omitted, you must ' - 'specify --name.') + help='Copy all secrets from the source vault. Mutually exclusive with --name. If neither --name nor ' + '--all is specified, all secrets will be copied.') c.extra('overwrite', arg_type=get_three_state_flag(), help='Overwrite existing secrets in destination.') # endregion From 4fc1c567d853a48b46b1e061bab85f97dfaaa2dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Cassanji?= <111895926+jcassanji-southworks@users.noreply.github.com> Date: Wed, 4 Feb 2026 23:25:01 -0300 Subject: [PATCH 12/35] Update src/azure-cli/azure/cli/command_modules/keyvault/custom.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- src/azure-cli/azure/cli/command_modules/keyvault/custom.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/azure-cli/azure/cli/command_modules/keyvault/custom.py b/src/azure-cli/azure/cli/command_modules/keyvault/custom.py index 6479f417a4c..e5e65a707db 100644 --- a/src/azure-cli/azure/cli/command_modules/keyvault/custom.py +++ b/src/azure-cli/azure/cli/command_modules/keyvault/custom.py @@ -2525,8 +2525,6 @@ def copy_secret(cmd, client, destination_vault, name=None, all_secrets=False, ov from azure.cli.core._profile import Profile from azure.cli.core.commands.client_factory import prepare_client_kwargs_track2 - from azure.cli.core.azclierror import MutuallyExclusiveArgumentError - # If neither a specific secret name nor --all is provided, default to copying all secrets. if not name and not all_secrets: all_secrets = True From 429bfcc7ba4ec8fa726d84c9e1b7fd06b9d8858d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Cassanji?= <111895926+jcassanji-southworks@users.noreply.github.com> Date: Wed, 4 Feb 2026 23:25:16 -0300 Subject: [PATCH 13/35] Update src/azure-cli/azure/cli/command_modules/keyvault/custom.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .../azure/cli/command_modules/keyvault/custom.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/azure-cli/azure/cli/command_modules/keyvault/custom.py b/src/azure-cli/azure/cli/command_modules/keyvault/custom.py index e5e65a707db..401d824c5b4 100644 --- a/src/azure-cli/azure/cli/command_modules/keyvault/custom.py +++ b/src/azure-cli/azure/cli/command_modules/keyvault/custom.py @@ -2546,13 +2546,15 @@ def copy_secret(cmd, client, destination_vault, name=None, all_secrets=False, ov # Fail fast if destination vault is not accessible or does not exist try: - # Force a call to the service; if the vault is empty this will raise StopIteration, which is fine - next(dest_client.list_properties_of_secrets()) - except StopIteration: - # Vault is accessible but has no secrets yet - pass + # Perform a lightweight call to validate vault accessibility. + # A 404 for a dummy secret name means the vault is reachable but the secret does not exist. + dest_client.get_secret_properties("azure_cli_destination_vault_validation_dummy") except HttpResponseError as e: - raise CLIError(f"Failed to access destination Key Vault '{destination_vault}': {str(e)}") + if getattr(e, "status_code", None) == 404: + # Vault is accessible but the dummy secret does not exist, which is expected. + pass + else: + raise CLIError(f"Failed to access destination Key Vault '{destination_vault}': {str(e)}") secrets_to_copy = [] if name: From 9b3b66970f6616299a47c02af87fed220831c01c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Cassanji?= <111895926+jcassanji-southworks@users.noreply.github.com> Date: Wed, 4 Feb 2026 23:25:35 -0300 Subject: [PATCH 14/35] Update src/azure-cli/azure/cli/command_modules/keyvault/custom.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- src/azure-cli/azure/cli/command_modules/keyvault/custom.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/azure-cli/azure/cli/command_modules/keyvault/custom.py b/src/azure-cli/azure/cli/command_modules/keyvault/custom.py index 401d824c5b4..9f592cc23e2 100644 --- a/src/azure-cli/azure/cli/command_modules/keyvault/custom.py +++ b/src/azure-cli/azure/cli/command_modules/keyvault/custom.py @@ -2519,7 +2519,7 @@ def set_attributes_certificate(client, certificate_name, version=None, policy=No return client.get_certificate(certificate_name=certificate_name) -def copy_secret(cmd, client, destination_vault, name=None, all_secrets=False, overwrite=False): +def copy_secret(cmd, client, destination_vault, name=None, all_secrets=None, overwrite=False): from azure.core.exceptions import ResourceNotFoundError, HttpResponseError from azure.keyvault.secrets import SecretClient from azure.cli.core._profile import Profile From 69e3d2cbcb4330a5daf893dcce98c6c16c6615f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Cassanji?= Date: Wed, 4 Feb 2026 23:27:32 -0300 Subject: [PATCH 15/35] Enhance keyvault secret copy command: update overwrite flag to store_true and add test for default behavior --- .../azure/cli/command_modules/keyvault/_params.py | 2 +- .../tests/latest/test_keyvault_commands.py | 14 ++++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/src/azure-cli/azure/cli/command_modules/keyvault/_params.py b/src/azure-cli/azure/cli/command_modules/keyvault/_params.py index 06b668d53b5..10eb92485d2 100644 --- a/src/azure-cli/azure/cli/command_modules/keyvault/_params.py +++ b/src/azure-cli/azure/cli/command_modules/keyvault/_params.py @@ -580,7 +580,7 @@ class CLISecurityDomainOperation(str, Enum): c.extra('all_secrets', arg_type=get_three_state_flag(), options_list=['--all'], help='Copy all secrets from the source vault. Cannot be used with --name. If omitted, you must ' 'specify --name.') - c.extra('overwrite', arg_type=get_three_state_flag(), help='Overwrite existing secrets in destination.') + c.extra('overwrite', action='store_true', help='Overwrite existing secrets in destination.') # endregion # region keyvault security-domain diff --git a/src/azure-cli/azure/cli/command_modules/keyvault/tests/latest/test_keyvault_commands.py b/src/azure-cli/azure/cli/command_modules/keyvault/tests/latest/test_keyvault_commands.py index 73eb60e00b7..3b058e2e8e6 100644 --- a/src/azure-cli/azure/cli/command_modules/keyvault/tests/latest/test_keyvault_commands.py +++ b/src/azure-cli/azure/cli/command_modules/keyvault/tests/latest/test_keyvault_commands.py @@ -2852,5 +2852,19 @@ def test_keyvault_secret_copy(self, resource_group, key_vault): # 8. Test Non-existent Secret in Source (Should fail) self.cmd('keyvault secret copy --source-vault {kv} --destination-vault {dest_kv} --name non_existent_secret_123', expect_failure=True) + # 9. Test Default Behavior (Implicit --all) + # Add a unique secret to check implicit copy + secret_name_3 = self.create_random_name('secret3-', 24) + self.kwargs['secret_name_3'] = secret_name_3 + self.cmd('keyvault secret set --vault-name {kv} -n {secret_name_3} --value {secret_value}') + + # Run copy without --name or --all + self.cmd('keyvault secret copy --source-vault {kv} --destination-vault {dest_kv}') + + # Verify it was copied + self.cmd('keyvault secret show --vault-name {dest_kv} -n {secret_name_3}', checks=[ + self.check('value', '{secret_value}') + ]) + if __name__ == '__main__': unittest.main() From 8b11ea15592452a14cd00c3eae38db1769d8685c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Cassanji?= Date: Wed, 4 Feb 2026 23:32:45 -0300 Subject: [PATCH 16/35] Fix copy_secret function: remove http_logging_policy from client kwargs and update dummy secret validation call --- src/azure-cli/azure/cli/command_modules/keyvault/custom.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/azure-cli/azure/cli/command_modules/keyvault/custom.py b/src/azure-cli/azure/cli/command_modules/keyvault/custom.py index 9f592cc23e2..eba9e97d3e3 100644 --- a/src/azure-cli/azure/cli/command_modules/keyvault/custom.py +++ b/src/azure-cli/azure/cli/command_modules/keyvault/custom.py @@ -2541,6 +2541,7 @@ def copy_secret(cmd, client, destination_vault, name=None, all_secrets=None, ove # Use standard client kwargs for consistent logging/telemetry client_kwargs = prepare_client_kwargs_track2(cmd.cli_ctx) + client_kwargs.pop('http_logging_policy', None) # KeyVault clients handle this internally or differently sometimes, mimicking _client_factory dest_client = SecretClient(vault_url=destination_vault, credential=credential, **client_kwargs) @@ -2548,7 +2549,7 @@ def copy_secret(cmd, client, destination_vault, name=None, all_secrets=None, ove try: # Perform a lightweight call to validate vault accessibility. # A 404 for a dummy secret name means the vault is reachable but the secret does not exist. - dest_client.get_secret_properties("azure_cli_destination_vault_validation_dummy") + dest_client.get_secret("azure-cli-validation-dummy") except HttpResponseError as e: if getattr(e, "status_code", None) == 404: # Vault is accessible but the dummy secret does not exist, which is expected. From 3376e7d00e461887d23d4537dfe9b61f8e3de633 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Cassanji?= Date: Thu, 5 Feb 2026 13:00:16 -0300 Subject: [PATCH 17/35] Implement secret copying functionality: add _copy_single_secret function and refactor copy_secret to utilize it --- .../cli/command_modules/keyvault/custom.py | 110 ++++++++++-------- 1 file changed, 61 insertions(+), 49 deletions(-) diff --git a/src/azure-cli/azure/cli/command_modules/keyvault/custom.py b/src/azure-cli/azure/cli/command_modules/keyvault/custom.py index eba9e97d3e3..6618e95f0ea 100644 --- a/src/azure-cli/azure/cli/command_modules/keyvault/custom.py +++ b/src/azure-cli/azure/cli/command_modules/keyvault/custom.py @@ -2519,6 +2519,57 @@ def set_attributes_certificate(client, certificate_name, version=None, policy=No return client.get_certificate(certificate_name=certificate_name) +def _copy_single_secret(source_client, dest_client, secret_name, overwrite, is_single_mode): + from azure.core.exceptions import ResourceNotFoundError, HttpResponseError + + try: + # Check destination + if not overwrite: + try: + dest_client.get_secret(secret_name) + logger.warning("Secret '%s' already exists in destination. Skipping.", secret_name) + return None # Skipped + except ResourceNotFoundError: + pass + except HttpResponseError as e: + logger.warning("Error checking secret '%s' in destination: %s", secret_name, str(e)) + if e.status_code == 403: + logger.error("Access denied checking secret '%s' in destination.", secret_name) + return False # Failed + + # Copy + logger.info("Copying secret: %s", secret_name) + s = source_client.get_secret(secret_name) + + new_secret = dest_client.set_secret( + s.name, + s.value, + content_type=s.properties.content_type, + tags=s.properties.tags, + enabled=s.properties.enabled, + not_before=s.properties.not_before, + expires_on=s.properties.expires_on + ) + + logger.info("Successfully copied secret: %s", secret_name) + return {'name': new_secret.name, 'id': new_secret.id} + + except ResourceNotFoundError: + if is_single_mode: + raise CLIError("Secret '{}' not found in source vault.".format(secret_name)) + logger.error("Secret '%s' not found in source vault.", secret_name) + return False + except HttpResponseError as e: + if is_single_mode: + raise CLIError("Failed to copy secret '{}': {}".format(secret_name, str(e))) + + if e.status_code == 403: # Forbidden + logger.error("Access denied (403) for secret '%s': %s", secret_name, str(e)) + else: + logger.error("Failed to copy secret '%s': %s", secret_name, str(e)) + return False + + def copy_secret(cmd, client, destination_vault, name=None, all_secrets=None, overwrite=False): from azure.core.exceptions import ResourceNotFoundError, HttpResponseError from azure.keyvault.secrets import SecretClient @@ -2541,7 +2592,8 @@ def copy_secret(cmd, client, destination_vault, name=None, all_secrets=None, ove # Use standard client kwargs for consistent logging/telemetry client_kwargs = prepare_client_kwargs_track2(cmd.cli_ctx) - client_kwargs.pop('http_logging_policy', None) # KeyVault clients handle this internally or differently sometimes, mimicking _client_factory + # KeyVault clients handle this internally or differently sometimes, mimicking _client_factory + client_kwargs.pop('http_logging_policy', None) dest_client = SecretClient(vault_url=destination_vault, credential=credential, **client_kwargs) @@ -2566,7 +2618,7 @@ def copy_secret(cmd, client, destination_vault, name=None, all_secrets=None, ove source_secrets = client.list_properties_of_secrets() for s in source_secrets: if s.managed: - logger.warning(f"Skipping managed secret: {s.name}") + logger.warning("Skipping managed secret: %s", s.name) continue secrets_to_copy.append(s.name) except HttpResponseError as e: @@ -2575,54 +2627,14 @@ def copy_secret(cmd, client, destination_vault, name=None, all_secrets=None, ove copied_secrets = [] failed_secrets = [] for secret_name in secrets_to_copy: - try: - # Check destination - if not overwrite: - try: - dest_client.get_secret(secret_name) - logger.warning(f"Secret '{secret_name}' already exists in destination. Skipping.") - continue - except ResourceNotFoundError: - pass - except HttpResponseError as e: - logger.warning(f"Error checking secret '{secret_name}' in destination: {str(e)}") - failed_secrets.append(secret_name) - if e.status_code == 403: - logger.error(f"Access denied checking secret '{secret_name}' in destination.") - continue - - # Copy - logger.info(f"Copying secret: {secret_name}") - s = client.get_secret(secret_name) - - new_secret = dest_client.set_secret( - s.name, - s.value, - content_type=s.properties.content_type, - tags=s.properties.tags, - enabled=s.properties.enabled, - not_before=s.properties.not_before, - expires_on=s.properties.expires_on - ) - - logger.info(f"Successfully copied secret: {secret_name}") - copied_secrets.append({'name': new_secret.name, 'id': new_secret.id}) - - except ResourceNotFoundError: - if name: - raise CLIError(f"Secret '{secret_name}' not found in source vault.") - logger.error(f"Secret '{secret_name}' not found in source vault.") - failed_secrets.append(secret_name) - except HttpResponseError as e: - if name: - raise CLIError(f"Failed to copy secret '{secret_name}': {str(e)}") + result = _copy_single_secret(client, dest_client, secret_name, overwrite, bool(name)) + if result: + copied_secrets.append(result) + elif result is False: failed_secrets.append(secret_name) - if e.status_code == 403: # Forbidden - logger.error(f"Access denied (403) for secret '{secret_name}': {str(e)}") - else: - logger.error(f"Failed to copy secret '{secret_name}': {str(e)}") - + if failed_secrets: - logger.warning(f"Operation completed with failures. {len(failed_secrets)} secrets failed to copy: {', '.join(failed_secrets)}") + logger.warning("Operation completed with failures. %s secrets failed to copy: %s", + len(failed_secrets), ', '.join(failed_secrets)) return copied_secrets From 56bf92accd0cce4380eddf54dc43f6b33d2a6d73 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Cassanji?= Date: Thu, 5 Feb 2026 15:39:21 -0300 Subject: [PATCH 18/35] Add unit tests for keyvault secret copying functionality --- .../tests/latest/test_keyvault_unit.py | 190 ++++++++++++++++++ 1 file changed, 190 insertions(+) create mode 100644 src/azure-cli/azure/cli/command_modules/keyvault/tests/latest/test_keyvault_unit.py diff --git a/src/azure-cli/azure/cli/command_modules/keyvault/tests/latest/test_keyvault_unit.py b/src/azure-cli/azure/cli/command_modules/keyvault/tests/latest/test_keyvault_unit.py new file mode 100644 index 00000000000..f3787734044 --- /dev/null +++ b/src/azure-cli/azure/cli/command_modules/keyvault/tests/latest/test_keyvault_unit.py @@ -0,0 +1,190 @@ +# -------------------------------------------------------------------------------------------- +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. See License.txt in the project root for license information. +# -------------------------------------------------------------------------------------------- + +import unittest +from unittest import mock +from azure.core.exceptions import ResourceNotFoundError, HttpResponseError +from knack.util import CLIError + +# Mock the logger to prevent actual logging during tests +with mock.patch('azure.cli.command_modules.keyvault.custom.logger'): + from azure.cli.command_modules.keyvault.custom import copy_secret + +class KeyVaultCopySecretTest(unittest.TestCase): + def setUp(self): + self.cmd = mock.MagicMock() + self.cmd.cli_ctx = mock.MagicMock() + self.cmd.cli_ctx.data = { + 'subscription_id': 'sub_id', + 'headers': {}, + 'completer_active': False, + 'command': 'keyvault secret copy' + } + + # Patches + self.patcher_profile = mock.patch('azure.cli.core._profile.Profile') + self.mock_profile = self.patcher_profile.start() + self.mock_profile_instance = mock.MagicMock() + self.mock_profile.return_value = self.mock_profile_instance + self.mock_profile_instance.get_login_credentials.return_value = (mock.Mock(), mock.Mock(), mock.Mock()) + + self.patcher_secret_client = mock.patch('azure.keyvault.secrets.SecretClient') + self.mock_secret_client_cls = self.patcher_secret_client.start() + + # Source Client Mock (passed as argument) + self.source_client = mock.MagicMock() + self.source_client.vault_url = "https://source-kv.vault.azure.net/" + + # Dest Client Mock (instantiated inside function) + self.dest_client = mock.MagicMock() + self.mock_secret_client_cls.return_value = self.dest_client + + def tearDown(self): + self.patcher_profile.stop() + self.patcher_secret_client.stop() + + def test_copy_single_secret_success(self): + # Setup + secret_name = "mysecret" + destination_vault = "https://dest-kv.vault.azure.net/" + + # Mocks for verification check + # Dummy check raises 404 which is expected/success path for connectivity check + not_found_error = HttpResponseError(message="Not Found") + not_found_error.status_code = 404 + self.dest_client.get_secret.side_effect = [not_found_error, ResourceNotFoundError] + # First call is dummy check (fails with 404), second is check existence (fails with ResourceNotFoundError -> OK to copy) + + # Source secret + secret_obj = mock.Mock() + secret_obj.name = secret_name + secret_obj.value = "secret_value" + secret_obj.properties.content_type = "text/plain" + secret_obj.properties.tags = {} + secret_obj.properties.enabled = True + secret_obj.properties.not_before = None + secret_obj.properties.expires_on = None + + self.source_client.get_secret.return_value = secret_obj + + # Result of set_secret + new_secret = mock.Mock() + new_secret.name = secret_name + new_secret.id = destination_vault + "/secrets/" + secret_name + self.dest_client.set_secret.return_value = new_secret + + # Act + result = copy_secret(self.cmd, self.source_client, destination_vault, name=secret_name) + + # Assert + self.assertEqual(len(result), 1) + self.assertEqual(result[0]['name'], secret_name) + self.dest_client.set_secret.assert_called_with( + secret_name, "secret_value", content_type="text/plain", tags={}, + enabled=True, not_before=None, expires_on=None + ) + + def test_copy_secret_already_exists_no_overwrite(self): + # Setup + secret_name = "mysecret" + destination_vault = "https://dest-kv.vault.azure.net/" + + # Dummy check 404 + not_found_error = HttpResponseError(message="Not Found") + not_found_error.status_code = 404 + + # Pre-check existence returns Success (means it exists) + self.dest_client.get_secret.side_effect = [not_found_error, mock.Mock()] + + # Act + result = copy_secret(self.cmd, self.source_client, destination_vault, name=secret_name, overwrite=False) + + # Assert + self.assertEqual(len(result), 0) # Should be empty list as it was skipped + self.dest_client.set_secret.assert_not_called() + + def test_copy_secret_already_exists_with_overwrite(self): + # Setup + secret_name = "mysecret" + destination_vault = "https://dest-kv.vault.azure.net/" + + # Dummy check 404 + not_found_error = HttpResponseError(message="Not Found") + not_found_error.status_code = 404 + self.dest_client.get_secret.side_effect = [not_found_error] # No second call because overwrite=True skips check + + # Source secret + secret_obj = mock.Mock() + secret_obj.name = secret_name + secret_obj.value = "val" + secret_obj.properties.content_type = None + secret_obj.properties.tags = None + secret_obj.properties.enabled = True + secret_obj.properties.not_before = None + secret_obj.properties.expires_on = None + self.source_client.get_secret.return_value = secret_obj + + new_secret = mock.Mock() + new_secret.name = secret_name + new_secret.id = destination_vault + "/secrets/" + secret_name + self.dest_client.set_secret.return_value = new_secret + + # Act + result = copy_secret(self.cmd, self.source_client, destination_vault, name=secret_name, overwrite=True) + + # Assert + self.assertEqual(len(result), 1) + self.dest_client.set_secret.assert_called() + + def test_copy_all_secrets(self): + # Setup + destination_vault = "https://dest-kv.vault.azure.net/" + + # Dummy check 404 + not_found_error = HttpResponseError(message="Not Found") + not_found_error.status_code = 404 + # We have 2 secrets. For each, we check existence (fails -> copy). + # Side effect sequence: DummyCheck -> Check(sec1) -> Check(sec2) + self.dest_client.get_secret.side_effect = [ + not_found_error, + ResourceNotFoundError, + ResourceNotFoundError + ] + + # List secrets source + s1 = mock.Mock(); s1.name = "sec1"; s1.managed = False + s2 = mock.Mock(); s2.name = "sec2"; s2.managed = False + s3 = mock.Mock(); s3.name = "mgd1"; s3.managed = True # Should be skipped + self.source_client.list_properties_of_secrets.return_value = [s1, s2, s3] + + # Get secret details + def get_secret_side_effect(name): + m = mock.Mock() + m.name = name + m.value = "val" + m.properties.content_type = None + m.properties.tags = None + m.properties.enabled = True + m.properties.not_before = None + m.properties.expires_on = None + return m + self.source_client.get_secret.side_effect = get_secret_side_effect + + new_secret = mock.Mock() + new_secret.name = "sec" + new_secret.id = "id" + self.dest_client.set_secret.return_value = new_secret + + # Act + result = copy_secret(self.cmd, self.source_client, destination_vault, all_secrets=True) + + # Assert + self.assertEqual(len(result), 2) + call_args = self.dest_client.set_secret.call_args_list + self.assertEqual(call_args[0][0][0], "sec1") + self.assertEqual(call_args[1][0][0], "sec2") + +if __name__ == '__main__': + unittest.main() From e3e8f0e7c8afca99432778179a55f01a82a94156 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Cassanji?= Date: Thu, 5 Feb 2026 17:26:51 -0300 Subject: [PATCH 19/35] Update KeyVault preparation to disable RBAC authorization for copy tests --- .../keyvault/tests/latest/test_keyvault_commands.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/azure-cli/azure/cli/command_modules/keyvault/tests/latest/test_keyvault_commands.py b/src/azure-cli/azure/cli/command_modules/keyvault/tests/latest/test_keyvault_commands.py index 3b058e2e8e6..f8449c91e2c 100644 --- a/src/azure-cli/azure/cli/command_modules/keyvault/tests/latest/test_keyvault_commands.py +++ b/src/azure-cli/azure/cli/command_modules/keyvault/tests/latest/test_keyvault_commands.py @@ -2780,7 +2780,7 @@ def test_keyvault_mhsm_region(self, resource_group, managed_hsm): class KeyVaultCopyScenarioTest(ScenarioTest): @ResourceGroupPreparer(name_prefix='cli_test_keyvault_copy') - @KeyVaultPreparer(name_prefix='cli-test-kv-src-') + @KeyVaultPreparer(name_prefix='cli-test-kv-src-', additional_params='--enable-rbac-authorization false') def test_keyvault_secret_copy(self, resource_group, key_vault): self.kwargs.update({ 'src_kv': key_vault, @@ -2793,7 +2793,7 @@ def test_keyvault_secret_copy(self, resource_group, key_vault): # Create Dest KV # Use simple creation to ensure speed and reliability in playback - self.cmd('keyvault create -g {rg} -n {dest_kv}') + self.cmd('keyvault create -g {rg} -n {dest_kv} --enable-rbac-authorization false') self.addCleanup(self.cmd, 'keyvault delete -g {rg} -n {dest_kv}') self.addCleanup(self.cmd, 'keyvault purge -n {dest_kv}') From 398c73ac8a3a7d27beb31a8aa1d6f871222e5131 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Cassanji?= Date: Thu, 5 Feb 2026 18:02:51 -0300 Subject: [PATCH 20/35] Add User-Agent filter to prevent recording mismatch in KeyVault copy tests --- .../keyvault/tests/latest/test_keyvault_commands.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/azure-cli/azure/cli/command_modules/keyvault/tests/latest/test_keyvault_commands.py b/src/azure-cli/azure/cli/command_modules/keyvault/tests/latest/test_keyvault_commands.py index f8449c91e2c..ec69755c7db 100644 --- a/src/azure-cli/azure/cli/command_modules/keyvault/tests/latest/test_keyvault_commands.py +++ b/src/azure-cli/azure/cli/command_modules/keyvault/tests/latest/test_keyvault_commands.py @@ -2779,6 +2779,9 @@ def test_keyvault_mhsm_region(self, resource_group, managed_hsm): class KeyVaultCopyScenarioTest(ScenarioTest): + # Filter User-Agent to prevent recording mismatch between recording env (Windows) and CI (Linux) + FILTER_HEADERS = ScenarioTest.FILTER_HEADERS + ['user-agent'] + @ResourceGroupPreparer(name_prefix='cli_test_keyvault_copy') @KeyVaultPreparer(name_prefix='cli-test-kv-src-', additional_params='--enable-rbac-authorization false') def test_keyvault_secret_copy(self, resource_group, key_vault): From 2a79899288b8cd711d520dfab822564be1b9227e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Cassanji?= Date: Thu, 5 Feb 2026 18:39:41 -0300 Subject: [PATCH 21/35] Fix unused import in keyvault custom.py --- src/azure-cli/azure/cli/command_modules/keyvault/custom.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/azure-cli/azure/cli/command_modules/keyvault/custom.py b/src/azure-cli/azure/cli/command_modules/keyvault/custom.py index 6618e95f0ea..1d6bb068d7c 100644 --- a/src/azure-cli/azure/cli/command_modules/keyvault/custom.py +++ b/src/azure-cli/azure/cli/command_modules/keyvault/custom.py @@ -2571,7 +2571,7 @@ def _copy_single_secret(source_client, dest_client, secret_name, overwrite, is_s def copy_secret(cmd, client, destination_vault, name=None, all_secrets=None, overwrite=False): - from azure.core.exceptions import ResourceNotFoundError, HttpResponseError + from azure.core.exceptions import HttpResponseError from azure.keyvault.secrets import SecretClient from azure.cli.core._profile import Profile from azure.cli.core.commands.client_factory import prepare_client_kwargs_track2 From db401680751595812842635355784b2973065724 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Cassanji?= Date: Thu, 5 Feb 2026 19:30:04 -0300 Subject: [PATCH 22/35] implement PR feedback --- .../cli/command_modules/keyvault/_params.py | 2 +- .../cli/command_modules/keyvault/custom.py | 61 +++++++++++++------ 2 files changed, 42 insertions(+), 21 deletions(-) diff --git a/src/azure-cli/azure/cli/command_modules/keyvault/_params.py b/src/azure-cli/azure/cli/command_modules/keyvault/_params.py index 3cc398dd516..9b6a05ee807 100644 --- a/src/azure-cli/azure/cli/command_modules/keyvault/_params.py +++ b/src/azure-cli/azure/cli/command_modules/keyvault/_params.py @@ -581,7 +581,7 @@ class CLISecurityDomainOperation(str, Enum): c.extra('all_secrets', arg_type=get_three_state_flag(), options_list=['--all'], help='Copy all secrets from the source vault. Mutually exclusive with --name. If neither --name nor ' '--all is specified, all secrets will be copied.') - c.extra('overwrite', arg_type=get_three_state_flag(), help='Overwrite existing secrets in destination.') + c.extra('overwrite', arg_type=get_three_state_flag(), help='Overwrite secrets in the destination vault if they already exist.') # endregion # region keyvault security-domain diff --git a/src/azure-cli/azure/cli/command_modules/keyvault/custom.py b/src/azure-cli/azure/cli/command_modules/keyvault/custom.py index 1d6bb068d7c..2c923b6496b 100644 --- a/src/azure-cli/azure/cli/command_modules/keyvault/custom.py +++ b/src/azure-cli/azure/cli/command_modules/keyvault/custom.py @@ -2532,24 +2532,40 @@ def _copy_single_secret(source_client, dest_client, secret_name, overwrite, is_s except ResourceNotFoundError: pass except HttpResponseError as e: - logger.warning("Error checking secret '%s' in destination: %s", secret_name, str(e)) - if e.status_code == 403: - logger.error("Access denied checking secret '%s' in destination.", secret_name) + status_code = getattr(e, "status_code", None) + if status_code == 403: + logger.error("Access denied (403) checking secret '%s' in destination: %s", secret_name, str(e)) + elif status_code is not None and 400 <= status_code < 500: + logger.warning("Client error (%s) checking secret '%s' in destination: %s", + status_code, secret_name, str(e)) + elif status_code is not None and status_code >= 500: + logger.error("Server error (%s) checking secret '%s' in destination: %s", + status_code, secret_name, str(e)) + else: + logger.error("Unexpected error checking secret '%s' in destination: %s", secret_name, str(e)) return False # Failed # Copy logger.info("Copying secret: %s", secret_name) s = source_client.get_secret(secret_name) - new_secret = dest_client.set_secret( - s.name, - s.value, - content_type=s.properties.content_type, - tags=s.properties.tags, - enabled=s.properties.enabled, - not_before=s.properties.not_before, - expires_on=s.properties.expires_on - ) + try: + new_secret = dest_client.set_secret( + s.name, + s.value, + content_type=s.properties.content_type, + tags=s.properties.tags, + enabled=s.properties.enabled, + not_before=s.properties.not_before, + expires_on=s.properties.expires_on + ) + except HttpResponseError as e: + from azure.cli.core.azclierror import CLIError + if is_single_mode: + raise CLIError(f"Failed to copy secret '{secret_name}': {str(e)}") + + logger.error("Failed to copy secret '%s': %s", secret_name, str(e)) + return False logger.info("Successfully copied secret: %s", secret_name) return {'name': new_secret.name, 'id': new_secret.id} @@ -2571,7 +2587,7 @@ def _copy_single_secret(source_client, dest_client, secret_name, overwrite, is_s def copy_secret(cmd, client, destination_vault, name=None, all_secrets=None, overwrite=False): - from azure.core.exceptions import HttpResponseError + from azure.core.exceptions import ResourceNotFoundError, HttpResponseError from azure.keyvault.secrets import SecretClient from azure.cli.core._profile import Profile from azure.cli.core.commands.client_factory import prepare_client_kwargs_track2 @@ -2595,25 +2611,30 @@ def copy_secret(cmd, client, destination_vault, name=None, all_secrets=None, ove # KeyVault clients handle this internally or differently sometimes, mimicking _client_factory client_kwargs.pop('http_logging_policy', None) - dest_client = SecretClient(vault_url=destination_vault, credential=credential, **client_kwargs) + dest_client = SecretClient( + vault_url=destination_vault, + credential=credential, + api_version='7.4', + verify_challenge_resource=False, + **client_kwargs + ) # Fail fast if destination vault is not accessible or does not exist try: # Perform a lightweight call to validate vault accessibility. # A 404 for a dummy secret name means the vault is reachable but the secret does not exist. dest_client.get_secret("azure-cli-validation-dummy") + except ResourceNotFoundError: + # Vault is accessible but the dummy secret does not exist, which is expected. + pass except HttpResponseError as e: - if getattr(e, "status_code", None) == 404: - # Vault is accessible but the dummy secret does not exist, which is expected. - pass - else: - raise CLIError(f"Failed to access destination Key Vault '{destination_vault}': {str(e)}") + raise CLIError(f"Failed to access destination Key Vault '{destination_vault}': {str(e)}") secrets_to_copy = [] if name: secrets_to_copy.append(name) else: - logger.warning("Copying all secrets from source...") + logger.info("Copying all secrets from source...") try: source_secrets = client.list_properties_of_secrets() for s in source_secrets: From 6723be1392791ba12a000b48795e5a793476bff4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Cassanji?= Date: Tue, 17 Feb 2026 18:49:46 -0300 Subject: [PATCH 23/35] Refactor logging messages in _copy_single_secret for consistency and clarity --- src/azure-cli/azure/cli/command_modules/keyvault/custom.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/azure-cli/azure/cli/command_modules/keyvault/custom.py b/src/azure-cli/azure/cli/command_modules/keyvault/custom.py index 2c923b6496b..0f79dfd4529 100644 --- a/src/azure-cli/azure/cli/command_modules/keyvault/custom.py +++ b/src/azure-cli/azure/cli/command_modules/keyvault/custom.py @@ -2536,10 +2536,10 @@ def _copy_single_secret(source_client, dest_client, secret_name, overwrite, is_s if status_code == 403: logger.error("Access denied (403) checking secret '%s' in destination: %s", secret_name, str(e)) elif status_code is not None and 400 <= status_code < 500: - logger.warning("Client error (%s) checking secret '%s' in destination: %s", + logger.warning("Client error (%s) checking secret '%s' in destination: %s", status_code, secret_name, str(e)) elif status_code is not None and status_code >= 500: - logger.error("Server error (%s) checking secret '%s' in destination: %s", + logger.error("Server error (%s) checking secret '%s' in destination: %s", status_code, secret_name, str(e)) else: logger.error("Unexpected error checking secret '%s' in destination: %s", secret_name, str(e)) @@ -2560,10 +2560,9 @@ def _copy_single_secret(source_client, dest_client, secret_name, overwrite, is_s expires_on=s.properties.expires_on ) except HttpResponseError as e: - from azure.cli.core.azclierror import CLIError if is_single_mode: raise CLIError(f"Failed to copy secret '{secret_name}': {str(e)}") - + logger.error("Failed to copy secret '%s': %s", secret_name, str(e)) return False From d7bf9476391d777e10bba4bb8eaa3f0bf61b30d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Cassanji?= Date: Tue, 17 Feb 2026 19:47:08 -0300 Subject: [PATCH 24/35] Handle 404 error in copy_secret function to improve error handling for missing secrets --- src/azure-cli/azure/cli/command_modules/keyvault/custom.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/azure-cli/azure/cli/command_modules/keyvault/custom.py b/src/azure-cli/azure/cli/command_modules/keyvault/custom.py index 0f79dfd4529..77f1345f4fc 100644 --- a/src/azure-cli/azure/cli/command_modules/keyvault/custom.py +++ b/src/azure-cli/azure/cli/command_modules/keyvault/custom.py @@ -2627,7 +2627,11 @@ def copy_secret(cmd, client, destination_vault, name=None, all_secrets=None, ove # Vault is accessible but the dummy secret does not exist, which is expected. pass except HttpResponseError as e: - raise CLIError(f"Failed to access destination Key Vault '{destination_vault}': {str(e)}") + if e.status_code == 404: + # Vault is accessible but the dummy secret does not exist, which is expected. + pass + else: + raise CLIError(f"Failed to access destination Key Vault '{destination_vault}': {str(e)}") secrets_to_copy = [] if name: From 97df9a56fa00d1debf5492b6451707747da4d96e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Cassanji?= Date: Tue, 17 Feb 2026 20:41:14 -0300 Subject: [PATCH 25/35] Reuse existing credentials in copy_secret function to optimize authentication process --- .../azure/cli/command_modules/keyvault/custom.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/azure-cli/azure/cli/command_modules/keyvault/custom.py b/src/azure-cli/azure/cli/command_modules/keyvault/custom.py index 77f1345f4fc..cebabda33db 100644 --- a/src/azure-cli/azure/cli/command_modules/keyvault/custom.py +++ b/src/azure-cli/azure/cli/command_modules/keyvault/custom.py @@ -2602,8 +2602,14 @@ def copy_secret(cmd, client, destination_vault, name=None, all_secrets=None, ove if client.vault_url.rstrip('/') == destination_vault.rstrip('/'): raise CLIError("Source and destination Key Vaults cannot be the same.") - profile = Profile(cli_ctx=cmd.cli_ctx) - credential, _, _ = profile.get_login_credentials(subscription_id=cmd.cli_ctx.data.get('subscription_id')) + credential = None + # Try to reuse the credential from the source client if available + if hasattr(client, '_credential'): + credential = client._credential + + if not credential: + profile = Profile(cli_ctx=cmd.cli_ctx) + credential, _, _ = profile.get_login_credentials(subscription_id=cmd.cli_ctx.data.get('subscription_id')) # Use standard client kwargs for consistent logging/telemetry client_kwargs = prepare_client_kwargs_track2(cmd.cli_ctx) From 2f442359cedcdbdf4aeefdf4d65f01dfbef6ea4e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Cassanji?= Date: Tue, 17 Feb 2026 21:01:36 -0300 Subject: [PATCH 26/35] Enhance credential retrieval in copy_secret function to support additional client configurations --- src/azure-cli/azure/cli/command_modules/keyvault/custom.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/azure-cli/azure/cli/command_modules/keyvault/custom.py b/src/azure-cli/azure/cli/command_modules/keyvault/custom.py index cebabda33db..42c6e5d6f8a 100644 --- a/src/azure-cli/azure/cli/command_modules/keyvault/custom.py +++ b/src/azure-cli/azure/cli/command_modules/keyvault/custom.py @@ -2606,8 +2606,14 @@ def copy_secret(cmd, client, destination_vault, name=None, all_secrets=None, ove # Try to reuse the credential from the source client if available if hasattr(client, '_credential'): credential = client._credential + elif hasattr(client, 'credential'): + credential = client.credential + elif hasattr(client, '_config') and hasattr(client._config, 'credential'): + credential = client._config.credential if not credential: + logger.warning("Credential not found on source client. Falling back to Profile.") + from azure.cli.core._profile import Profile profile = Profile(cli_ctx=cmd.cli_ctx) credential, _, _ = profile.get_login_credentials(subscription_id=cmd.cli_ctx.data.get('subscription_id')) From 31a1c7522568473418066fb81bfea8f26e990301 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Cassanji?= Date: Tue, 17 Feb 2026 21:11:44 -0300 Subject: [PATCH 27/35] Add unit tests for copy_secret function and remove obsolete test file --- .../tests/latest/test_keyvault_commands.py | 179 ++++++++++++++++- .../tests/latest/test_keyvault_unit.py | 190 ------------------ 2 files changed, 178 insertions(+), 191 deletions(-) delete mode 100644 src/azure-cli/azure/cli/command_modules/keyvault/tests/latest/test_keyvault_unit.py diff --git a/src/azure-cli/azure/cli/command_modules/keyvault/tests/latest/test_keyvault_commands.py b/src/azure-cli/azure/cli/command_modules/keyvault/tests/latest/test_keyvault_commands.py index ec69755c7db..dbd2f27f0a6 100644 --- a/src/azure-cli/azure/cli/command_modules/keyvault/tests/latest/test_keyvault_commands.py +++ b/src/azure-cli/azure/cli/command_modules/keyvault/tests/latest/test_keyvault_commands.py @@ -18,8 +18,9 @@ from azure.cli.testsdk.scenario_tests import AllowLargeResponse, record_only from azure.cli.testsdk.scenario_tests import RecordingProcessor from azure.cli.testsdk import ResourceGroupPreparer, StorageAccountPreparer, KeyVaultPreparer, ManagedHSMPreparer, ScenarioTest -from azure.core.exceptions import HttpResponseError +from azure.core.exceptions import HttpResponseError, ResourceNotFoundError from knack.util import CLIError +from azure.cli.command_modules.keyvault.custom import copy_secret secret_text_encoding_values = ['utf-8', 'utf-16le', 'utf-16be', 'ascii'] secret_binary_encoding_values = ['base64', 'hex'] @@ -2869,5 +2870,181 @@ def test_keyvault_secret_copy(self, resource_group, key_vault): self.check('value', '{secret_value}') ]) + +class KeyVaultCopySecretTest(unittest.TestCase): + def setUp(self): + self.cmd = mock.MagicMock() + self.cmd.cli_ctx = mock.MagicMock() + self.cmd.cli_ctx.data = { + 'subscription_id': 'sub_id', + 'headers': {}, + 'completer_active': False, + 'command': 'keyvault secret copy' + } + + # Patches + self.patcher_profile = mock.patch('azure.cli.core._profile.Profile') + self.mock_profile = self.patcher_profile.start() + self.mock_profile_instance = mock.MagicMock() + self.mock_profile.return_value = self.mock_profile_instance + self.mock_profile_instance.get_login_credentials.return_value = (mock.Mock(), mock.Mock(), mock.Mock()) + + self.patcher_secret_client = mock.patch('azure.keyvault.secrets.SecretClient') + self.mock_secret_client_cls = self.patcher_secret_client.start() + + # Source Client Mock (passed as argument) + self.source_client = mock.MagicMock() + self.source_client.vault_url = "https://source-kv.vault.azure.net/" + + # Dest Client Mock (instantiated inside function) + self.dest_client = mock.MagicMock() + self.mock_secret_client_cls.return_value = self.dest_client + + def tearDown(self): + self.patcher_profile.stop() + self.patcher_secret_client.stop() + + def test_copy_single_secret_success(self): + # Setup + secret_name = "mysecret" + destination_vault = "https://dest-kv.vault.azure.net/" + + # Mocks for verification check + # Dummy check raises 404 which is expected/success path for connectivity check + not_found_error = HttpResponseError(message="Not Found") + not_found_error.status_code = 404 + self.dest_client.get_secret.side_effect = [not_found_error, ResourceNotFoundError] + # First call is dummy check (fails with 404), second is check existence (fails with ResourceNotFoundError -> OK to copy) + + # Source secret + secret_obj = mock.Mock() + secret_obj.name = secret_name + secret_obj.value = "secret_value" + secret_obj.properties.content_type = "text/plain" + secret_obj.properties.tags = {} + secret_obj.properties.enabled = True + secret_obj.properties.not_before = None + secret_obj.properties.expires_on = None + + self.source_client.get_secret.return_value = secret_obj + + # Result of set_secret + new_secret = mock.Mock() + new_secret.name = secret_name + new_secret.id = destination_vault + "/secrets/" + secret_name + self.dest_client.set_secret.return_value = new_secret + + # Act + result = copy_secret(self.cmd, self.source_client, destination_vault, name=secret_name) + + # Assert + self.assertEqual(len(result), 1) + self.assertEqual(result[0]['name'], secret_name) + self.dest_client.set_secret.assert_called_with( + secret_name, "secret_value", content_type="text/plain", tags={}, + enabled=True, not_before=None, expires_on=None + ) + + def test_copy_secret_already_exists_no_overwrite(self): + # Setup + secret_name = "mysecret" + destination_vault = "https://dest-kv.vault.azure.net/" + + # Dummy check 404 + not_found_error = HttpResponseError(message="Not Found") + not_found_error.status_code = 404 + + # Pre-check existence returns Success (means it exists) + self.dest_client.get_secret.side_effect = [not_found_error, mock.Mock()] + + # Act + result = copy_secret(self.cmd, self.source_client, destination_vault, name=secret_name, overwrite=False) + + # Assert + self.assertEqual(len(result), 0) # Should be empty list as it was skipped + self.dest_client.set_secret.assert_not_called() + + def test_copy_secret_already_exists_with_overwrite(self): + # Setup + secret_name = "mysecret" + destination_vault = "https://dest-kv.vault.azure.net/" + + # Dummy check 404 + not_found_error = HttpResponseError(message="Not Found") + not_found_error.status_code = 404 + self.dest_client.get_secret.side_effect = [not_found_error] # No second call because overwrite=True skips check + + # Source secret + secret_obj = mock.Mock() + secret_obj.name = secret_name + secret_obj.value = "val" + secret_obj.properties.content_type = None + secret_obj.properties.tags = None + secret_obj.properties.enabled = True + secret_obj.properties.not_before = None + secret_obj.properties.expires_on = None + self.source_client.get_secret.return_value = secret_obj + + new_secret = mock.Mock() + new_secret.name = secret_name + new_secret.id = destination_vault + "/secrets/" + secret_name + self.dest_client.set_secret.return_value = new_secret + + # Act + result = copy_secret(self.cmd, self.source_client, destination_vault, name=secret_name, overwrite=True) + + # Assert + self.assertEqual(len(result), 1) + self.dest_client.set_secret.assert_called() + + def test_copy_all_secrets(self): + # Setup + destination_vault = "https://dest-kv.vault.azure.net/" + + # Dummy check 404 + not_found_error = HttpResponseError(message="Not Found") + not_found_error.status_code = 404 + # We have 2 secrets. For each, we check existence (fails -> copy). + # Side effect sequence: DummyCheck -> Check(sec1) -> Check(sec2) + self.dest_client.get_secret.side_effect = [ + not_found_error, + ResourceNotFoundError, + ResourceNotFoundError + ] + + # List secrets source + s1 = mock.Mock(); s1.name = "sec1"; s1.managed = False + s2 = mock.Mock(); s2.name = "sec2"; s2.managed = False + s3 = mock.Mock(); s3.name = "mgd1"; s3.managed = True # Should be skipped + self.source_client.list_properties_of_secrets.return_value = [s1, s2, s3] + + # Get secret details + def get_secret_side_effect(name): + m = mock.Mock() + m.name = name + m.value = "val" + m.properties.content_type = None + m.properties.tags = None + m.properties.enabled = True + m.properties.not_before = None + m.properties.expires_on = None + return m + self.source_client.get_secret.side_effect = get_secret_side_effect + + new_secret = mock.Mock() + new_secret.name = "sec" + new_secret.id = "id" + self.dest_client.set_secret.return_value = new_secret + + # Act + result = copy_secret(self.cmd, self.source_client, destination_vault, all_secrets=True) + + # Assert + self.assertEqual(len(result), 2) + call_args = self.dest_client.set_secret.call_args_list + self.assertEqual(call_args[0][0][0], "sec1") + self.assertEqual(call_args[1][0][0], "sec2") + + if __name__ == '__main__': unittest.main() diff --git a/src/azure-cli/azure/cli/command_modules/keyvault/tests/latest/test_keyvault_unit.py b/src/azure-cli/azure/cli/command_modules/keyvault/tests/latest/test_keyvault_unit.py deleted file mode 100644 index f3787734044..00000000000 --- a/src/azure-cli/azure/cli/command_modules/keyvault/tests/latest/test_keyvault_unit.py +++ /dev/null @@ -1,190 +0,0 @@ -# -------------------------------------------------------------------------------------------- -# Copyright (c) Microsoft Corporation. All rights reserved. -# Licensed under the MIT License. See License.txt in the project root for license information. -# -------------------------------------------------------------------------------------------- - -import unittest -from unittest import mock -from azure.core.exceptions import ResourceNotFoundError, HttpResponseError -from knack.util import CLIError - -# Mock the logger to prevent actual logging during tests -with mock.patch('azure.cli.command_modules.keyvault.custom.logger'): - from azure.cli.command_modules.keyvault.custom import copy_secret - -class KeyVaultCopySecretTest(unittest.TestCase): - def setUp(self): - self.cmd = mock.MagicMock() - self.cmd.cli_ctx = mock.MagicMock() - self.cmd.cli_ctx.data = { - 'subscription_id': 'sub_id', - 'headers': {}, - 'completer_active': False, - 'command': 'keyvault secret copy' - } - - # Patches - self.patcher_profile = mock.patch('azure.cli.core._profile.Profile') - self.mock_profile = self.patcher_profile.start() - self.mock_profile_instance = mock.MagicMock() - self.mock_profile.return_value = self.mock_profile_instance - self.mock_profile_instance.get_login_credentials.return_value = (mock.Mock(), mock.Mock(), mock.Mock()) - - self.patcher_secret_client = mock.patch('azure.keyvault.secrets.SecretClient') - self.mock_secret_client_cls = self.patcher_secret_client.start() - - # Source Client Mock (passed as argument) - self.source_client = mock.MagicMock() - self.source_client.vault_url = "https://source-kv.vault.azure.net/" - - # Dest Client Mock (instantiated inside function) - self.dest_client = mock.MagicMock() - self.mock_secret_client_cls.return_value = self.dest_client - - def tearDown(self): - self.patcher_profile.stop() - self.patcher_secret_client.stop() - - def test_copy_single_secret_success(self): - # Setup - secret_name = "mysecret" - destination_vault = "https://dest-kv.vault.azure.net/" - - # Mocks for verification check - # Dummy check raises 404 which is expected/success path for connectivity check - not_found_error = HttpResponseError(message="Not Found") - not_found_error.status_code = 404 - self.dest_client.get_secret.side_effect = [not_found_error, ResourceNotFoundError] - # First call is dummy check (fails with 404), second is check existence (fails with ResourceNotFoundError -> OK to copy) - - # Source secret - secret_obj = mock.Mock() - secret_obj.name = secret_name - secret_obj.value = "secret_value" - secret_obj.properties.content_type = "text/plain" - secret_obj.properties.tags = {} - secret_obj.properties.enabled = True - secret_obj.properties.not_before = None - secret_obj.properties.expires_on = None - - self.source_client.get_secret.return_value = secret_obj - - # Result of set_secret - new_secret = mock.Mock() - new_secret.name = secret_name - new_secret.id = destination_vault + "/secrets/" + secret_name - self.dest_client.set_secret.return_value = new_secret - - # Act - result = copy_secret(self.cmd, self.source_client, destination_vault, name=secret_name) - - # Assert - self.assertEqual(len(result), 1) - self.assertEqual(result[0]['name'], secret_name) - self.dest_client.set_secret.assert_called_with( - secret_name, "secret_value", content_type="text/plain", tags={}, - enabled=True, not_before=None, expires_on=None - ) - - def test_copy_secret_already_exists_no_overwrite(self): - # Setup - secret_name = "mysecret" - destination_vault = "https://dest-kv.vault.azure.net/" - - # Dummy check 404 - not_found_error = HttpResponseError(message="Not Found") - not_found_error.status_code = 404 - - # Pre-check existence returns Success (means it exists) - self.dest_client.get_secret.side_effect = [not_found_error, mock.Mock()] - - # Act - result = copy_secret(self.cmd, self.source_client, destination_vault, name=secret_name, overwrite=False) - - # Assert - self.assertEqual(len(result), 0) # Should be empty list as it was skipped - self.dest_client.set_secret.assert_not_called() - - def test_copy_secret_already_exists_with_overwrite(self): - # Setup - secret_name = "mysecret" - destination_vault = "https://dest-kv.vault.azure.net/" - - # Dummy check 404 - not_found_error = HttpResponseError(message="Not Found") - not_found_error.status_code = 404 - self.dest_client.get_secret.side_effect = [not_found_error] # No second call because overwrite=True skips check - - # Source secret - secret_obj = mock.Mock() - secret_obj.name = secret_name - secret_obj.value = "val" - secret_obj.properties.content_type = None - secret_obj.properties.tags = None - secret_obj.properties.enabled = True - secret_obj.properties.not_before = None - secret_obj.properties.expires_on = None - self.source_client.get_secret.return_value = secret_obj - - new_secret = mock.Mock() - new_secret.name = secret_name - new_secret.id = destination_vault + "/secrets/" + secret_name - self.dest_client.set_secret.return_value = new_secret - - # Act - result = copy_secret(self.cmd, self.source_client, destination_vault, name=secret_name, overwrite=True) - - # Assert - self.assertEqual(len(result), 1) - self.dest_client.set_secret.assert_called() - - def test_copy_all_secrets(self): - # Setup - destination_vault = "https://dest-kv.vault.azure.net/" - - # Dummy check 404 - not_found_error = HttpResponseError(message="Not Found") - not_found_error.status_code = 404 - # We have 2 secrets. For each, we check existence (fails -> copy). - # Side effect sequence: DummyCheck -> Check(sec1) -> Check(sec2) - self.dest_client.get_secret.side_effect = [ - not_found_error, - ResourceNotFoundError, - ResourceNotFoundError - ] - - # List secrets source - s1 = mock.Mock(); s1.name = "sec1"; s1.managed = False - s2 = mock.Mock(); s2.name = "sec2"; s2.managed = False - s3 = mock.Mock(); s3.name = "mgd1"; s3.managed = True # Should be skipped - self.source_client.list_properties_of_secrets.return_value = [s1, s2, s3] - - # Get secret details - def get_secret_side_effect(name): - m = mock.Mock() - m.name = name - m.value = "val" - m.properties.content_type = None - m.properties.tags = None - m.properties.enabled = True - m.properties.not_before = None - m.properties.expires_on = None - return m - self.source_client.get_secret.side_effect = get_secret_side_effect - - new_secret = mock.Mock() - new_secret.name = "sec" - new_secret.id = "id" - self.dest_client.set_secret.return_value = new_secret - - # Act - result = copy_secret(self.cmd, self.source_client, destination_vault, all_secrets=True) - - # Assert - self.assertEqual(len(result), 2) - call_args = self.dest_client.set_secret.call_args_list - self.assertEqual(call_args[0][0][0], "sec1") - self.assertEqual(call_args[1][0][0], "sec2") - -if __name__ == '__main__': - unittest.main() From 6ebb73e9954b6446d4a574925972ec42b4a21d43 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Cassanji?= Date: Tue, 17 Feb 2026 21:32:12 -0300 Subject: [PATCH 28/35] Refactor KeyVaultCopyScenarioTest to improve test reliability and add location parameter for destination Key Vault creation --- .../keyvault/tests/latest/test_keyvault_commands.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/azure-cli/azure/cli/command_modules/keyvault/tests/latest/test_keyvault_commands.py b/src/azure-cli/azure/cli/command_modules/keyvault/tests/latest/test_keyvault_commands.py index dbd2f27f0a6..a52aed7c82a 100644 --- a/src/azure-cli/azure/cli/command_modules/keyvault/tests/latest/test_keyvault_commands.py +++ b/src/azure-cli/azure/cli/command_modules/keyvault/tests/latest/test_keyvault_commands.py @@ -2780,14 +2780,12 @@ def test_keyvault_mhsm_region(self, resource_group, managed_hsm): class KeyVaultCopyScenarioTest(ScenarioTest): - # Filter User-Agent to prevent recording mismatch between recording env (Windows) and CI (Linux) - FILTER_HEADERS = ScenarioTest.FILTER_HEADERS + ['user-agent'] - + @AllowLargeResponse() @ResourceGroupPreparer(name_prefix='cli_test_keyvault_copy') - @KeyVaultPreparer(name_prefix='cli-test-kv-src-', additional_params='--enable-rbac-authorization false') + @KeyVaultPreparer(name_prefix='cli-test-kv-src-', location='eastus', additional_params='--enable-rbac-authorization false') def test_keyvault_secret_copy(self, resource_group, key_vault): self.kwargs.update({ - 'src_kv': key_vault, + 'kv': key_vault, 'dest_kv': self.create_random_name('cli-test-kv-dest-', 24), 'secret_name': self.create_random_name('secret-', 24), 'secret_value': 'mysecretvalue', @@ -2797,7 +2795,7 @@ def test_keyvault_secret_copy(self, resource_group, key_vault): # Create Dest KV # Use simple creation to ensure speed and reliability in playback - self.cmd('keyvault create -g {rg} -n {dest_kv} --enable-rbac-authorization false') + self.cmd('keyvault create -g {rg} -n {dest_kv} --location eastus --enable-rbac-authorization false') self.addCleanup(self.cmd, 'keyvault delete -g {rg} -n {dest_kv}') self.addCleanup(self.cmd, 'keyvault purge -n {dest_kv}') From 9b8bddb42f723469c30f9ad07e64a97d24fc8d9e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Cassanji?= <111895926+jcassanji-southworks@users.noreply.github.com> Date: Wed, 18 Feb 2026 00:54:43 -0300 Subject: [PATCH 29/35] Add mock profile to test_keyvault_secret_copy test --- .../keyvault/tests/latest/test_keyvault_commands.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/azure-cli/azure/cli/command_modules/keyvault/tests/latest/test_keyvault_commands.py b/src/azure-cli/azure/cli/command_modules/keyvault/tests/latest/test_keyvault_commands.py index a52aed7c82a..58bc481da3e 100644 --- a/src/azure-cli/azure/cli/command_modules/keyvault/tests/latest/test_keyvault_commands.py +++ b/src/azure-cli/azure/cli/command_modules/keyvault/tests/latest/test_keyvault_commands.py @@ -2784,6 +2784,11 @@ class KeyVaultCopyScenarioTest(ScenarioTest): @ResourceGroupPreparer(name_prefix='cli_test_keyvault_copy') @KeyVaultPreparer(name_prefix='cli-test-kv-src-', location='eastus', additional_params='--enable-rbac-authorization false') def test_keyvault_secret_copy(self, resource_group, key_vault): + patcher = mock.patch('azure.cli.core._profile.Profile') + mock_profile = patcher.start() + self.addCleanup(patcher.stop) + mock_profile.return_value.get_login_credentials.return_value = (mock.Mock(), mock.Mock(), mock.Mock()) + self.kwargs.update({ 'kv': key_vault, 'dest_kv': self.create_random_name('cli-test-kv-dest-', 24), From c7aefd1be43637bee628671a1dbf7eb089324469 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Cassanji?= <111895926+jcassanji-southworks@users.noreply.github.com> Date: Wed, 18 Feb 2026 20:52:34 -0300 Subject: [PATCH 30/35] Simplify credential handling in custom.py Refactor credential retrieval logic in Key Vault client. --- src/azure-cli/azure/cli/command_modules/keyvault/custom.py | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/azure-cli/azure/cli/command_modules/keyvault/custom.py b/src/azure-cli/azure/cli/command_modules/keyvault/custom.py index 42c6e5d6f8a..53e106919b8 100644 --- a/src/azure-cli/azure/cli/command_modules/keyvault/custom.py +++ b/src/azure-cli/azure/cli/command_modules/keyvault/custom.py @@ -2603,13 +2603,8 @@ def copy_secret(cmd, client, destination_vault, name=None, all_secrets=None, ove raise CLIError("Source and destination Key Vaults cannot be the same.") credential = None - # Try to reuse the credential from the source client if available - if hasattr(client, '_credential'): - credential = client._credential - elif hasattr(client, 'credential'): + if hasattr(client, 'credential'): credential = client.credential - elif hasattr(client, '_config') and hasattr(client._config, 'credential'): - credential = client._config.credential if not credential: logger.warning("Credential not found on source client. Falling back to Profile.") From 94b3947894e23efb4a98246c8451116b602d39da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Cassanji?= <111895926+jcassanji-southworks@users.noreply.github.com> Date: Wed, 18 Feb 2026 20:52:38 -0300 Subject: [PATCH 31/35] Refactor test setup for KeyVault secret copy test --- .../tests/latest/test_keyvault_commands.py | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/azure-cli/azure/cli/command_modules/keyvault/tests/latest/test_keyvault_commands.py b/src/azure-cli/azure/cli/command_modules/keyvault/tests/latest/test_keyvault_commands.py index 58bc481da3e..e61b333769a 100644 --- a/src/azure-cli/azure/cli/command_modules/keyvault/tests/latest/test_keyvault_commands.py +++ b/src/azure-cli/azure/cli/command_modules/keyvault/tests/latest/test_keyvault_commands.py @@ -2783,12 +2783,17 @@ class KeyVaultCopyScenarioTest(ScenarioTest): @AllowLargeResponse() @ResourceGroupPreparer(name_prefix='cli_test_keyvault_copy') @KeyVaultPreparer(name_prefix='cli-test-kv-src-', location='eastus', additional_params='--enable-rbac-authorization false') - def test_keyvault_secret_copy(self, resource_group, key_vault): - patcher = mock.patch('azure.cli.core._profile.Profile') - mock_profile = patcher.start() - self.addCleanup(patcher.stop) - mock_profile.return_value.get_login_credentials.return_value = (mock.Mock(), mock.Mock(), mock.Mock()) + def setUp(self): + super().setUp() + self.patcher = mock.patch('azure.cli.core._profile.Profile') + self.mock_profile = self.patcher.start() + self.addCleanup(self.patcher.stop) + self.mock_profile.return_value.get_login_credentials.return_value = (mock.Mock(), mock.Mock(), mock.Mock()) + @AllowLargeResponse() + @ResourceGroupPreparer(name_prefix='cli_test_keyvault_copy') + @KeyVaultPreparer(name_prefix='cli-test-kv-src-', location='eastus', additional_params='--enable-rbac-authorization false') + def test_keyvault_secret_copy(self, key_vault): self.kwargs.update({ 'kv': key_vault, 'dest_kv': self.create_random_name('cli-test-kv-dest-', 24), From 8d06e9c98973a09053f78a2205a092fc5615db77 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Cassanji?= <111895926+jcassanji-southworks@users.noreply.github.com> Date: Wed, 18 Feb 2026 21:18:49 -0300 Subject: [PATCH 32/35] Refactor KeyVault secret copy tests for clarity Refactor KeyVault copy tests to improve structure and clarity, including renaming test classes and methods for better readability. Enhance test coverage for secret copy functionality, including overwrite behavior and error cases. --- .../tests/latest/test_keyvault_commands.py | 682 ++++++++++++------ 1 file changed, 467 insertions(+), 215 deletions(-) diff --git a/src/azure-cli/azure/cli/command_modules/keyvault/tests/latest/test_keyvault_commands.py b/src/azure-cli/azure/cli/command_modules/keyvault/tests/latest/test_keyvault_commands.py index e61b333769a..982c17fc599 100644 --- a/src/azure-cli/azure/cli/command_modules/keyvault/tests/latest/test_keyvault_commands.py +++ b/src/azure-cli/azure/cli/command_modules/keyvault/tests/latest/test_keyvault_commands.py @@ -2779,279 +2779,531 @@ def test_keyvault_mhsm_region(self, resource_group, managed_hsm): self.cmd('keyvault region remove -g {rg} --hsm-name {hsm_name} -r uksouth') -class KeyVaultCopyScenarioTest(ScenarioTest): +class KeyVaultSecretCopyScenarioTest(ScenarioTest): + """Test suite for keyvault secret copy functionality. + + This test class validates the secret copy command across different scenarios including + single secret copy, bulk copy, overwrite behavior, and error handling. + """ + @AllowLargeResponse() - @ResourceGroupPreparer(name_prefix='cli_test_keyvault_copy') - @KeyVaultPreparer(name_prefix='cli-test-kv-src-', location='eastus', additional_params='--enable-rbac-authorization false') - def setUp(self): - super().setUp() - self.patcher = mock.patch('azure.cli.core._profile.Profile') - self.mock_profile = self.patcher.start() - self.addCleanup(self.patcher.stop) - self.mock_profile.return_value.get_login_credentials.return_value = (mock.Mock(), mock.Mock(), mock.Mock()) + @ResourceGroupPreparer(name_prefix='cli_test_kv_secret_copy', location='eastus2') + @KeyVaultPreparer(name_prefix='cli-test-kv-src-', location='eastus2', + additional_params='--enable-rbac-authorization false') + def test_keyvault_secret_copy_single(self, resource_group, key_vault): + """Test copying a single secret between key vaults.""" + self.kwargs.update({ + 'src_kv': key_vault, + 'dest_kv': self.create_random_name('cli-test-kv-dst-', 24), + 'secret1': self.create_random_name('secret-', 24), + 'secret_value': 'TestSecretValue123', + 'loc': 'eastus2' + }) + + # Create destination vault + self.cmd('keyvault create -g {rg} -n {dest_kv} -l {loc} --enable-rbac-authorization false', + checks=[ + self.check('name', '{dest_kv}'), + self.check('properties.enableSoftDelete', True) + ]) + + # Create secret in source vault with metadata + self.cmd('keyvault secret set --vault-name {src_kv} -n {secret1} --value {secret_value} ' + '--tags environment=test owner=cli --content-type text/plain', + checks=[ + self.check('value', '{secret_value}'), + self.check('contentType', 'text/plain'), + self.check('tags.environment', 'test'), + self.check('tags.owner', 'cli') + ]) + + # Copy single secret to destination vault + self.cmd('keyvault secret copy --source-vault {src_kv} --destination-vault {dest_kv} --name {secret1}') + + # Verify secret was copied with all properties + self.cmd('keyvault secret show --vault-name {dest_kv} -n {secret1}', + checks=[ + self.check('value', '{secret_value}'), + self.check('contentType', 'text/plain'), + self.check('tags.environment', 'test'), + self.check('tags.owner', 'cli'), + self.check('name', '{secret1}') + ]) + + # Verify source secret still exists + self.cmd('keyvault secret show --vault-name {src_kv} -n {secret1}', + checks=[ + self.check('value', '{secret_value}'), + self.check('name', '{secret1}') + ]) + + # Cleanup + self.cmd('keyvault delete -g {rg} -n {dest_kv}') + self.cmd('keyvault purge -n {dest_kv}') @AllowLargeResponse() - @ResourceGroupPreparer(name_prefix='cli_test_keyvault_copy') - @KeyVaultPreparer(name_prefix='cli-test-kv-src-', location='eastus', additional_params='--enable-rbac-authorization false') - def test_keyvault_secret_copy(self, key_vault): - self.kwargs.update({ - 'kv': key_vault, - 'dest_kv': self.create_random_name('cli-test-kv-dest-', 24), - 'secret_name': self.create_random_name('secret-', 24), - 'secret_value': 'mysecretvalue', - 'new_val': 'newval', - 'secret_name_2': self.create_random_name('secret2-', 24) - }) - - # Create Dest KV - # Use simple creation to ensure speed and reliability in playback - self.cmd('keyvault create -g {rg} -n {dest_kv} --location eastus --enable-rbac-authorization false') - self.addCleanup(self.cmd, 'keyvault delete -g {rg} -n {dest_kv}') - self.addCleanup(self.cmd, 'keyvault purge -n {dest_kv}') - - # Set secret in Source with tags and content-type - self.cmd('keyvault secret set --vault-name {kv} -n {secret_name} --value {secret_value} --tags tag1=value1 --content-type text/plain') - - # 1. Copy specific secret - self.cmd('keyvault secret copy --source-vault {kv} --destination-vault {dest_kv} --name {secret_name}') - self.cmd('keyvault secret show --vault-name {dest_kv} -n {secret_name}', checks=[ - self.check('value', '{secret_value}'), - self.check('tags.tag1', 'value1'), - self.check('contentType', 'text/plain') - ]) + @ResourceGroupPreparer(name_prefix='cli_test_kv_secret_copy', location='eastus2') + @KeyVaultPreparer(name_prefix='cli-test-kv-src-', location='eastus2', + additional_params='--enable-rbac-authorization false') + def test_keyvault_secret_copy_all(self, resource_group, key_vault): + """Test copying all secrets from source to destination vault.""" + self.kwargs.update({ + 'src_kv': key_vault, + 'dest_kv': self.create_random_name('cli-test-kv-dst-', 24), + 'secret1': self.create_random_name('secret1-', 24), + 'secret2': self.create_random_name('secret2-', 24), + 'secret3': self.create_random_name('secret3-', 24), + 'value1': 'Value1', + 'value2': 'Value2', + 'value3': 'Value3', + 'loc': 'eastus2' + }) - # 2. Copy all secrets - # Add another secret to source - self.cmd('keyvault secret set --vault-name {kv} -n {secret_name_2} --value {secret_value}') - - # Run copy --all - self.cmd('keyvault secret copy --source-vault {kv} --destination-vault {dest_kv} --all') - - # Verify both exist in dest - self.cmd('keyvault secret show --vault-name {dest_kv} -n {secret_name_2}', checks=[ - self.check('value', '{secret_value}') - ]) + # Create destination vault + self.cmd('keyvault create -g {rg} -n {dest_kv} -l {loc} --enable-rbac-authorization false') - # 3. Test overwrite protection (default behavior: skip) - # Update source - self.cmd('keyvault secret set --vault-name {kv} -n {secret_name} --value {new_val}') - - # Copy without overwrite (should skip) - self.cmd('keyvault secret copy --source-vault {kv} --destination-vault {dest_kv} --name {secret_name}') - - # Verify destination still has old value - self.cmd('keyvault secret show --vault-name {dest_kv} -n {secret_name}', checks=[ - self.check('value', '{secret_value}') - ]) + # Create multiple secrets in source vault + self.cmd('keyvault secret set --vault-name {src_kv} -n {secret1} --value {value1}', + checks=self.check('value', '{value1}')) + self.cmd('keyvault secret set --vault-name {src_kv} -n {secret2} --value {value2}', + checks=self.check('value', '{value2}')) + self.cmd('keyvault secret set --vault-name {src_kv} -n {secret3} --value {value3}', + checks=self.check('value', '{value3}')) - # 4. Test overwrite - self.cmd('keyvault secret copy --source-vault {kv} --destination-vault {dest_kv} --name {secret_name} --overwrite') - - # Verify destination has new value - self.cmd('keyvault secret show --vault-name {dest_kv} -n {secret_name}', checks=[ - self.check('value', '{new_val}') - ]) + # Copy all secrets to destination + self.cmd('keyvault secret copy --source-vault {src_kv} --destination-vault {dest_kv} --all') - # 5. Test Mutual Exclusivity - self.cmd('keyvault secret copy --source-vault {kv} --destination-vault {dest_kv} --name {secret_name} --all', expect_failure=True) - - # 6. Test Source == Destination (Should fail) - self.cmd('keyvault secret copy --source-vault {kv} --destination-vault {kv} --name {secret_name}', expect_failure=True) - - # 7. Test Non-existent Destination (Should fail fast) - self.cmd('keyvault secret copy --source-vault {kv} --destination-vault non_existent_kv_12345 --name {secret_name}', expect_failure=True) - - # 8. Test Non-existent Secret in Source (Should fail) - self.cmd('keyvault secret copy --source-vault {kv} --destination-vault {dest_kv} --name non_existent_secret_123', expect_failure=True) - - # 9. Test Default Behavior (Implicit --all) - # Add a unique secret to check implicit copy - secret_name_3 = self.create_random_name('secret3-', 24) - self.kwargs['secret_name_3'] = secret_name_3 - self.cmd('keyvault secret set --vault-name {kv} -n {secret_name_3} --value {secret_value}') - - # Run copy without --name or --all - self.cmd('keyvault secret copy --source-vault {kv} --destination-vault {dest_kv}') - - # Verify it was copied - self.cmd('keyvault secret show --vault-name {dest_kv} -n {secret_name_3}', checks=[ - self.check('value', '{secret_value}') - ]) + # Verify all secrets were copied + self.cmd('keyvault secret show --vault-name {dest_kv} -n {secret1}', + checks=self.check('value', '{value1}')) + self.cmd('keyvault secret show --vault-name {dest_kv} -n {secret2}', + checks=self.check('value', '{value2}')) + self.cmd('keyvault secret show --vault-name {dest_kv} -n {secret3}', + checks=self.check('value', '{value3}')) + + # Verify source secrets still exist + self.cmd('keyvault secret list --vault-name {src_kv}', + checks=self.check('length(@)', 3)) + + # Cleanup + self.cmd('keyvault delete -g {rg} -n {dest_kv}') + self.cmd('keyvault purge -n {dest_kv}') + + @AllowLargeResponse() + @ResourceGroupPreparer(name_prefix='cli_test_kv_secret_copy', location='eastus2') + @KeyVaultPreparer(name_prefix='cli-test-kv-src-', location='eastus2', + additional_params='--enable-rbac-authorization false') + def test_keyvault_secret_copy_overwrite_behavior(self, resource_group, key_vault): + """Test overwrite behavior when copying secrets that already exist.""" + self.kwargs.update({ + 'src_kv': key_vault, + 'dest_kv': self.create_random_name('cli-test-kv-dst-', 24), + 'secret': self.create_random_name('secret-', 24), + 'original_value': 'OriginalValue', + 'updated_value': 'UpdatedValue', + 'loc': 'eastus2' + }) + + # Create destination vault + self.cmd('keyvault create -g {rg} -n {dest_kv} -l {loc} --enable-rbac-authorization false') + + # Create secret in source vault + self.cmd('keyvault secret set --vault-name {src_kv} -n {secret} --value {original_value}') + + # Copy secret to destination + self.cmd('keyvault secret copy --source-vault {src_kv} --destination-vault {dest_kv} --name {secret}') + + # Verify secret was copied + self.cmd('keyvault secret show --vault-name {dest_kv} -n {secret}', + checks=self.check('value', '{original_value}')) + + # Update secret in source vault + self.cmd('keyvault secret set --vault-name {src_kv} -n {secret} --value {updated_value}') + + # Copy again without overwrite flag (should skip) + self.cmd('keyvault secret copy --source-vault {src_kv} --destination-vault {dest_kv} --name {secret}') + + # Verify destination still has original value + self.cmd('keyvault secret show --vault-name {dest_kv} -n {secret}', + checks=self.check('value', '{original_value}')) + + # Copy with overwrite flag (should update) + self.cmd('keyvault secret copy --source-vault {src_kv} --destination-vault {dest_kv} ' + '--name {secret} --overwrite') + + # Verify destination now has updated value + self.cmd('keyvault secret show --vault-name {dest_kv} -n {secret}', + checks=self.check('value', '{updated_value}')) + + # Cleanup + self.cmd('keyvault delete -g {rg} -n {dest_kv}') + self.cmd('keyvault purge -n {dest_kv}') + + @AllowLargeResponse() + @ResourceGroupPreparer(name_prefix='cli_test_kv_secret_copy', location='eastus2') + @KeyVaultPreparer(name_prefix='cli-test-kv-src-', location='eastus2', + additional_params='--enable-rbac-authorization false') + def test_keyvault_secret_copy_error_cases(self, resource_group, key_vault): + """Test error handling for invalid copy operations.""" + self.kwargs.update({ + 'src_kv': key_vault, + 'dest_kv': self.create_random_name('cli-test-kv-dst-', 24), + 'nonexistent_kv': 'nonexistent-kv-' + self.create_random_name('', 10), + 'secret': self.create_random_name('secret-', 24), + 'nonexistent_secret': 'nonexistent-secret-' + self.create_random_name('', 10), + 'secret_value': 'TestValue', + 'loc': 'eastus2' + }) + + # Create destination vault and a secret + self.cmd('keyvault create -g {rg} -n {dest_kv} -l {loc} --enable-rbac-authorization false') + self.cmd('keyvault secret set --vault-name {src_kv} -n {secret} --value {secret_value}') + + # Test 1: Copy to non-existent destination vault (should fail) + self.cmd('keyvault secret copy --source-vault {src_kv} --destination-vault {nonexistent_kv} ' + '--name {secret}', + expect_failure=True) + + # Test 2: Copy non-existent secret (should fail) + self.cmd('keyvault secret copy --source-vault {src_kv} --destination-vault {dest_kv} ' + '--name {nonexistent_secret}', + expect_failure=True) + + # Test 3: Source and destination are the same (should fail) + self.cmd('keyvault secret copy --source-vault {src_kv} --destination-vault {src_kv} ' + '--name {secret}', + expect_failure=True) + + # Test 4: Using both --name and --all flags (should fail due to mutual exclusivity) + self.cmd('keyvault secret copy --source-vault {src_kv} --destination-vault {dest_kv} ' + '--name {secret} --all', + expect_failure=True) + + # Cleanup + self.cmd('keyvault delete -g {rg} -n {dest_kv}') + self.cmd('keyvault purge -n {dest_kv}') + + @AllowLargeResponse() + @ResourceGroupPreparer(name_prefix='cli_test_kv_secret_copy', location='eastus2') + @KeyVaultPreparer(name_prefix='cli-test-kv-src-', location='eastus2', + additional_params='--enable-rbac-authorization false') + def test_keyvault_secret_copy_default_behavior(self, resource_group, key_vault): + """Test default behavior when neither --name nor --all is specified.""" + self.kwargs.update({ + 'src_kv': key_vault, + 'dest_kv': self.create_random_name('cli-test-kv-dst-', 24), + 'secret1': self.create_random_name('secret1-', 24), + 'secret2': self.create_random_name('secret2-', 24), + 'value1': 'DefaultValue1', + 'value2': 'DefaultValue2', + 'loc': 'eastus2' + }) + + # Create destination vault + self.cmd('keyvault create -g {rg} -n {dest_kv} -l {loc} --enable-rbac-authorization false') + # Create secrets in source vault + self.cmd('keyvault secret set --vault-name {src_kv} -n {secret1} --value {value1}') + self.cmd('keyvault secret set --vault-name {src_kv} -n {secret2} --value {value2}') + + # Copy without specifying --name or --all (should default to --all) + self.cmd('keyvault secret copy --source-vault {src_kv} --destination-vault {dest_kv}') + + # Verify all secrets were copied + self.cmd('keyvault secret show --vault-name {dest_kv} -n {secret1}', + checks=self.check('value', '{value1}')) + self.cmd('keyvault secret show --vault-name {dest_kv} -n {secret2}', + checks=self.check('value', '{value2}')) + + # Cleanup + self.cmd('keyvault delete -g {rg} -n {dest_kv}') + self.cmd('keyvault purge -n {dest_kv}') + + +class KeyVaultSecretCopyUnitTest(unittest.TestCase): + """Unit tests for the copy_secret function with mocked dependencies.""" -class KeyVaultCopySecretTest(unittest.TestCase): def setUp(self): + """Set up test fixtures and mocks.""" + # Create mock command context self.cmd = mock.MagicMock() self.cmd.cli_ctx = mock.MagicMock() self.cmd.cli_ctx.data = { - 'subscription_id': 'sub_id', + 'subscription_id': 'test-subscription-id', 'headers': {}, 'completer_active': False, 'command': 'keyvault secret copy' } - # Patches + # Mock Profile for authentication self.patcher_profile = mock.patch('azure.cli.core._profile.Profile') - self.mock_profile = self.patcher_profile.start() + self.mock_profile_class = self.patcher_profile.start() self.mock_profile_instance = mock.MagicMock() - self.mock_profile.return_value = self.mock_profile_instance - self.mock_profile_instance.get_login_credentials.return_value = (mock.Mock(), mock.Mock(), mock.Mock()) + self.mock_profile_class.return_value = self.mock_profile_instance + self.mock_profile_instance.get_login_credentials.return_value = ( + mock.Mock(), mock.Mock(), mock.Mock() + ) + # Mock SecretClient self.patcher_secret_client = mock.patch('azure.keyvault.secrets.SecretClient') - self.mock_secret_client_cls = self.patcher_secret_client.start() - - # Source Client Mock (passed as argument) + self.mock_secret_client_class = self.patcher_secret_client.start() + + # Create source and destination client mocks self.source_client = mock.MagicMock() - self.source_client.vault_url = "https://source-kv.vault.azure.net/" - - # Dest Client Mock (instantiated inside function) + self.source_client.vault_url = "https://source-vault.vault.azure.net/" + self.dest_client = mock.MagicMock() - self.mock_secret_client_cls.return_value = self.dest_client + self.dest_client.vault_url = "https://destination-vault.vault.azure.net/" + self.mock_secret_client_class.return_value = self.dest_client def tearDown(self): + """Clean up mocks.""" self.patcher_profile.stop() self.patcher_secret_client.stop() + def _create_mock_secret(self, name, value, content_type=None, tags=None, enabled=True, + not_before=None, expires_on=None): + """Helper method to create a mock secret object.""" + secret = mock.Mock() + secret.name = name + secret.value = value + secret.properties = mock.Mock() + secret.properties.content_type = content_type + secret.properties.tags = tags or {} + secret.properties.enabled = enabled + secret.properties.not_before = not_before + secret.properties.expires_on = expires_on + secret.properties.managed = False + return secret + def test_copy_single_secret_success(self): - # Setup - secret_name = "mysecret" - destination_vault = "https://dest-kv.vault.azure.net/" - - # Mocks for verification check - # Dummy check raises 404 which is expected/success path for connectivity check + """Test successful copy of a single secret.""" + secret_name = "test-secret" + destination_vault = "https://destination-vault.vault.azure.net/" + + # Mock destination vault connectivity check not_found_error = HttpResponseError(message="Not Found") not_found_error.status_code = 404 - self.dest_client.get_secret.side_effect = [not_found_error, ResourceNotFoundError] - # First call is dummy check (fails with 404), second is check existence (fails with ResourceNotFoundError -> OK to copy) - - # Source secret - secret_obj = mock.Mock() - secret_obj.name = secret_name - secret_obj.value = "secret_value" - secret_obj.properties.content_type = "text/plain" - secret_obj.properties.tags = {} - secret_obj.properties.enabled = True - secret_obj.properties.not_before = None - secret_obj.properties.expires_on = None - - self.source_client.get_secret.return_value = secret_obj - - # Result of set_secret - new_secret = mock.Mock() - new_secret.name = secret_name - new_secret.id = destination_vault + "/secrets/" + secret_name - self.dest_client.set_secret.return_value = new_secret - - # Act + + # Mock sequence: connectivity check (404), then existence check (not found) + self.dest_client.get_secret.side_effect = [ + not_found_error, + ResourceNotFoundError("Secret not found") + ] + + # Mock source secret + source_secret = self._create_mock_secret( + name=secret_name, + value="secret-value-123", + content_type="application/json", + tags={"env": "test", "version": "1.0"} + ) + self.source_client.get_secret.return_value = source_secret + + # Mock successful set operation + dest_secret = self._create_mock_secret( + name=secret_name, + value="secret-value-123" + ) + dest_secret.id = f"{destination_vault}/secrets/{secret_name}" + self.dest_client.set_secret.return_value = dest_secret + + # Execute copy operation result = copy_secret(self.cmd, self.source_client, destination_vault, name=secret_name) - # Assert + # Verify results self.assertEqual(len(result), 1) self.assertEqual(result[0]['name'], secret_name) - self.dest_client.set_secret.assert_called_with( - secret_name, "secret_value", content_type="text/plain", tags={}, - enabled=True, not_before=None, expires_on=None - ) - - def test_copy_secret_already_exists_no_overwrite(self): - # Setup - secret_name = "mysecret" - destination_vault = "https://dest-kv.vault.azure.net/" - - # Dummy check 404 + self.assertIn('id', result[0]) + + # Verify set_secret was called with correct parameters + self.dest_client.set_secret.assert_called_once() + call_args = self.dest_client.set_secret.call_args + self.assertEqual(call_args[0][0], secret_name) + self.assertEqual(call_args[0][1], "secret-value-123") + self.assertEqual(call_args[1]['content_type'], "application/json") + self.assertEqual(call_args[1]['tags'], {"env": "test", "version": "1.0"}) + + def test_copy_secret_already_exists_skip(self): + """Test that existing secrets are skipped when overwrite is False.""" + secret_name = "existing-secret" + destination_vault = "https://destination-vault.vault.azure.net/" + + # Mock destination vault connectivity check not_found_error = HttpResponseError(message="Not Found") not_found_error.status_code = 404 - - # Pre-check existence returns Success (means it exists) - self.dest_client.get_secret.side_effect = [not_found_error, mock.Mock()] - # Act - result = copy_secret(self.cmd, self.source_client, destination_vault, name=secret_name, overwrite=False) + # Mock sequence: connectivity check (404), then existence check (found) + existing_secret = self._create_mock_secret(name=secret_name, value="existing-value") + self.dest_client.get_secret.side_effect = [not_found_error, existing_secret] + + # Execute copy operation without overwrite + result = copy_secret( + self.cmd, + self.source_client, + destination_vault, + name=secret_name, + overwrite=False + ) - # Assert - self.assertEqual(len(result), 0) # Should be empty list as it was skipped + # Verify secret was skipped + self.assertEqual(len(result), 0) self.dest_client.set_secret.assert_not_called() - def test_copy_secret_already_exists_with_overwrite(self): - # Setup - secret_name = "mysecret" - destination_vault = "https://dest-kv.vault.azure.net/" - - # Dummy check 404 + def test_copy_secret_overwrite_enabled(self): + """Test that existing secrets are overwritten when overwrite is True.""" + secret_name = "overwrite-secret" + destination_vault = "https://destination-vault.vault.azure.net/" + + # Mock destination vault connectivity check only (skip existence check) not_found_error = HttpResponseError(message="Not Found") not_found_error.status_code = 404 - self.dest_client.get_secret.side_effect = [not_found_error] # No second call because overwrite=True skips check - - # Source secret - secret_obj = mock.Mock() - secret_obj.name = secret_name - secret_obj.value = "val" - secret_obj.properties.content_type = None - secret_obj.properties.tags = None - secret_obj.properties.enabled = True - secret_obj.properties.not_before = None - secret_obj.properties.expires_on = None - self.source_client.get_secret.return_value = secret_obj - - new_secret = mock.Mock() - new_secret.name = secret_name - new_secret.id = destination_vault + "/secrets/" + secret_name - self.dest_client.set_secret.return_value = new_secret - - # Act - result = copy_secret(self.cmd, self.source_client, destination_vault, name=secret_name, overwrite=True) - - # Assert + self.dest_client.get_secret.side_effect = [not_found_error] + + # Mock source secret with new value + source_secret = self._create_mock_secret( + name=secret_name, + value="new-overwrite-value", + tags={"updated": "true"} + ) + self.source_client.get_secret.return_value = source_secret + + # Mock successful set operation + dest_secret = self._create_mock_secret(name=secret_name, value="new-overwrite-value") + dest_secret.id = f"{destination_vault}/secrets/{secret_name}" + self.dest_client.set_secret.return_value = dest_secret + + # Execute copy operation with overwrite + result = copy_secret( + self.cmd, + self.source_client, + destination_vault, + name=secret_name, + overwrite=True + ) + + # Verify secret was overwritten self.assertEqual(len(result), 1) - self.dest_client.set_secret.assert_called() + self.dest_client.set_secret.assert_called_once() - def test_copy_all_secrets(self): - # Setup - destination_vault = "https://dest-kv.vault.azure.net/" - - # Dummy check 404 + def test_copy_all_secrets_success(self): + """Test copying all secrets from source to destination.""" + destination_vault = "https://destination-vault.vault.azure.net/" + + # Mock destination vault connectivity check not_found_error = HttpResponseError(message="Not Found") not_found_error.status_code = 404 - # We have 2 secrets. For each, we check existence (fails -> copy). - # Side effect sequence: DummyCheck -> Check(sec1) -> Check(sec2) - self.dest_client.get_secret.side_effect = [ - not_found_error, - ResourceNotFoundError, - ResourceNotFoundError + + # Mock list of secrets in source vault + secret_props_1 = mock.Mock() + secret_props_1.name = "secret-one" + secret_props_1.managed = False + + secret_props_2 = mock.Mock() + secret_props_2.name = "secret-two" + secret_props_2.managed = False + + secret_props_3 = mock.Mock() + secret_props_3.name = "managed-secret" + secret_props_3.managed = True # This should be skipped + + self.source_client.list_properties_of_secrets.return_value = [ + secret_props_1, + secret_props_2, + secret_props_3 ] - # List secrets source - s1 = mock.Mock(); s1.name = "sec1"; s1.managed = False - s2 = mock.Mock(); s2.name = "sec2"; s2.managed = False - s3 = mock.Mock(); s3.name = "mgd1"; s3.managed = True # Should be skipped - self.source_client.list_properties_of_secrets.return_value = [s1, s2, s3] + # Mock destination vault checks (connectivity + existence for each secret) + self.dest_client.get_secret.side_effect = [ + not_found_error, # Connectivity check + ResourceNotFoundError("Not found"), # secret-one doesn't exist + ResourceNotFoundError("Not found") # secret-two doesn't exist + ] - # Get secret details + # Mock get_secret for source secrets def get_secret_side_effect(name): - m = mock.Mock() - m.name = name - m.value = "val" - m.properties.content_type = None - m.properties.tags = None - m.properties.enabled = True - m.properties.not_before = None - m.properties.expires_on = None - return m + return self._create_mock_secret(name=name, value=f"value-for-{name}") + self.source_client.get_secret.side_effect = get_secret_side_effect - new_secret = mock.Mock() - new_secret.name = "sec" - new_secret.id = "id" - self.dest_client.set_secret.return_value = new_secret + # Mock set_secret responses + def set_secret_side_effect(name, value, **kwargs): + secret = self._create_mock_secret(name=name, value=value) + secret.id = f"{destination_vault}/secrets/{name}" + return secret + + self.dest_client.set_secret.side_effect = set_secret_side_effect - # Act + # Execute copy all operation result = copy_secret(self.cmd, self.source_client, destination_vault, all_secrets=True) - # Assert + # Verify only non-managed secrets were copied self.assertEqual(len(result), 2) - call_args = self.dest_client.set_secret.call_args_list - self.assertEqual(call_args[0][0][0], "sec1") - self.assertEqual(call_args[1][0][0], "sec2") + copied_names = {r['name'] for r in result} + self.assertEqual(copied_names, {"secret-one", "secret-two"}) + + # Verify set_secret was called twice (not for managed secret) + self.assertEqual(self.dest_client.set_secret.call_count, 2) + + def test_copy_all_secrets_with_some_existing(self): + """Test copying all secrets when some already exist in destination.""" + destination_vault = "https://destination-vault.vault.azure.net/" + + # Mock destination vault connectivity check + not_found_error = HttpResponseError(message="Not Found") + not_found_error.status_code = 404 + + # Mock list of secrets in source vault + secret_props_1 = mock.Mock() + secret_props_1.name = "secret-new" + secret_props_1.managed = False + + secret_props_2 = mock.Mock() + secret_props_2.name = "secret-existing" + secret_props_2.managed = False + + self.source_client.list_properties_of_secrets.return_value = [ + secret_props_1, + secret_props_2 + ] + + # Mock destination vault checks + existing_secret = self._create_mock_secret(name="secret-existing", value="old-value") + self.dest_client.get_secret.side_effect = [ + not_found_error, # Connectivity check + ResourceNotFoundError("Not found"), # secret-new doesn't exist + existing_secret # secret-existing exists + ] + + # Mock get_secret for source secrets + def get_secret_side_effect(name): + return self._create_mock_secret(name=name, value=f"value-for-{name}") + + self.source_client.get_secret.side_effect = get_secret_side_effect + + # Mock set_secret response + def set_secret_side_effect(name, value, **kwargs): + secret = self._create_mock_secret(name=name, value=value) + secret.id = f"{destination_vault}/secrets/{name}" + return secret + + self.dest_client.set_secret.side_effect = set_secret_side_effect + + # Execute copy all operation without overwrite + result = copy_secret( + self.cmd, + self.source_client, + destination_vault, + all_secrets=True, + overwrite=False + ) + + # Verify only the new secret was copied + self.assertEqual(len(result), 1) + self.assertEqual(result[0]['name'], "secret-new") + + # Verify set_secret was called only once + self.assertEqual(self.dest_client.set_secret.call_count, 1) if __name__ == '__main__': From 95f26f318d3dfb49726000cd54c76a82a748151e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Cassanji?= <111895926+jcassanji-southworks@users.noreply.github.com> Date: Sat, 28 Feb 2026 00:13:03 -0300 Subject: [PATCH 33/35] Refactor keyvault command tests for better mocking --- .../tests/latest/test_keyvault_commands.py | 57 +++++++++---------- 1 file changed, 27 insertions(+), 30 deletions(-) diff --git a/src/azure-cli/azure/cli/command_modules/keyvault/tests/latest/test_keyvault_commands.py b/src/azure-cli/azure/cli/command_modules/keyvault/tests/latest/test_keyvault_commands.py index 982c17fc599..4c38254fd0c 100644 --- a/src/azure-cli/azure/cli/command_modules/keyvault/tests/latest/test_keyvault_commands.py +++ b/src/azure-cli/azure/cli/command_modules/keyvault/tests/latest/test_keyvault_commands.py @@ -2969,6 +2969,11 @@ def test_keyvault_secret_copy_error_cases(self, resource_group, key_vault): '--name {nonexistent_secret}', expect_failure=True) + # Test 2.5: Copy from non-existent source vault (should fail) + self.cmd('keyvault secret copy --source-vault {nonexistent_kv} --destination-vault {dest_kv} ' + '--name {secret}', + expect_failure=True) + # Test 3: Source and destination are the same (should fail) self.cmd('keyvault secret copy --source-vault {src_kv} --destination-vault {src_kv} ' '--name {secret}', @@ -3035,18 +3040,9 @@ def setUp(self): 'command': 'keyvault secret copy' } - # Mock Profile for authentication - self.patcher_profile = mock.patch('azure.cli.core._profile.Profile') - self.mock_profile_class = self.patcher_profile.start() - self.mock_profile_instance = mock.MagicMock() - self.mock_profile_class.return_value = self.mock_profile_instance - self.mock_profile_instance.get_login_credentials.return_value = ( - mock.Mock(), mock.Mock(), mock.Mock() - ) - - # Mock SecretClient - self.patcher_secret_client = mock.patch('azure.keyvault.secrets.SecretClient') - self.mock_secret_client_class = self.patcher_secret_client.start() + # Mock data_plane_azure_keyvault_secret_client + self.patcher_secret_client = mock.patch('azure.cli.command_modules.keyvault._client_factory.data_plane_azure_keyvault_secret_client') + self.mock_secret_client_factory = self.patcher_secret_client.start() # Create source and destination client mocks self.source_client = mock.MagicMock() @@ -3054,11 +3050,10 @@ def setUp(self): self.dest_client = mock.MagicMock() self.dest_client.vault_url = "https://destination-vault.vault.azure.net/" - self.mock_secret_client_class.return_value = self.dest_client + self.mock_secret_client_factory.return_value = self.dest_client def tearDown(self): """Clean up mocks.""" - self.patcher_profile.stop() self.patcher_secret_client.stop() def _create_mock_secret(self, name, value, content_type=None, tags=None, enabled=True, @@ -3090,15 +3085,15 @@ def test_copy_single_secret_success(self): not_found_error, ResourceNotFoundError("Secret not found") ] - - # Mock source secret - source_secret = self._create_mock_secret( - name=secret_name, - value="secret-value-123", - content_type="application/json", - tags={"env": "test", "version": "1.0"} - ) - self.source_client.get_secret.return_value = source_secret + self.source_client.get_secret.side_effect = [ + not_found_error, + self._create_mock_secret( + name=secret_name, + value="secret-value-123", + content_type="application/json", + tags={"env": "test", "version": "1.0"} + ) + ] # Mock successful set operation dest_secret = self._create_mock_secret( @@ -3136,6 +3131,7 @@ def test_copy_secret_already_exists_skip(self): # Mock sequence: connectivity check (404), then existence check (found) existing_secret = self._create_mock_secret(name=secret_name, value="existing-value") self.dest_client.get_secret.side_effect = [not_found_error, existing_secret] + self.source_client.get_secret.side_effect = [not_found_error, existing_secret] # Execute copy operation without overwrite result = copy_secret( @@ -3160,13 +3156,14 @@ def test_copy_secret_overwrite_enabled(self): not_found_error.status_code = 404 self.dest_client.get_secret.side_effect = [not_found_error] - # Mock source secret with new value - source_secret = self._create_mock_secret( - name=secret_name, - value="new-overwrite-value", - tags={"updated": "true"} - ) - self.source_client.get_secret.return_value = source_secret + self.source_client.get_secret.side_effect = [ + not_found_error, + self._create_mock_secret( + name=secret_name, + value="new-overwrite-value", + tags={"updated": "true"} + ) + ] # Mock successful set operation dest_secret = self._create_mock_secret(name=secret_name, value="new-overwrite-value") From 18a6c23648d65d5f10d06e9c67f42be9bc71cebd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Cassanji?= <111895926+jcassanji-southworks@users.noreply.github.com> Date: Sat, 28 Feb 2026 00:13:21 -0300 Subject: [PATCH 34/35] Update custom.py --- .../cli/command_modules/keyvault/custom.py | 79 ++++++------------- 1 file changed, 22 insertions(+), 57 deletions(-) diff --git a/src/azure-cli/azure/cli/command_modules/keyvault/custom.py b/src/azure-cli/azure/cli/command_modules/keyvault/custom.py index 53e106919b8..f94e5a2ff1e 100644 --- a/src/azure-cli/azure/cli/command_modules/keyvault/custom.py +++ b/src/azure-cli/azure/cli/command_modules/keyvault/custom.py @@ -2532,17 +2532,7 @@ def _copy_single_secret(source_client, dest_client, secret_name, overwrite, is_s except ResourceNotFoundError: pass except HttpResponseError as e: - status_code = getattr(e, "status_code", None) - if status_code == 403: - logger.error("Access denied (403) checking secret '%s' in destination: %s", secret_name, str(e)) - elif status_code is not None and 400 <= status_code < 500: - logger.warning("Client error (%s) checking secret '%s' in destination: %s", - status_code, secret_name, str(e)) - elif status_code is not None and status_code >= 500: - logger.error("Server error (%s) checking secret '%s' in destination: %s", - status_code, secret_name, str(e)) - else: - logger.error("Unexpected error checking secret '%s' in destination: %s", secret_name, str(e)) + logger.warning("Error checking secret '%s' in destination: %s", secret_name, str(e)) return False # Failed # Copy @@ -2561,7 +2551,7 @@ def _copy_single_secret(source_client, dest_client, secret_name, overwrite, is_s ) except HttpResponseError as e: if is_single_mode: - raise CLIError(f"Failed to copy secret '{secret_name}': {str(e)}") + raise e logger.error("Failed to copy secret '%s': %s", secret_name, str(e)) return False @@ -2569,27 +2559,21 @@ def _copy_single_secret(source_client, dest_client, secret_name, overwrite, is_s logger.info("Successfully copied secret: %s", secret_name) return {'name': new_secret.name, 'id': new_secret.id} - except ResourceNotFoundError: + except ResourceNotFoundError as e: if is_single_mode: - raise CLIError("Secret '{}' not found in source vault.".format(secret_name)) + raise e logger.error("Secret '%s' not found in source vault.", secret_name) return False except HttpResponseError as e: if is_single_mode: - raise CLIError("Failed to copy secret '{}': {}".format(secret_name, str(e))) - - if e.status_code == 403: # Forbidden - logger.error("Access denied (403) for secret '%s': %s", secret_name, str(e)) - else: - logger.error("Failed to copy secret '%s': %s", secret_name, str(e)) + raise e + logger.error("Failed to copy secret '%s': %s", secret_name, str(e)) return False def copy_secret(cmd, client, destination_vault, name=None, all_secrets=None, overwrite=False): from azure.core.exceptions import ResourceNotFoundError, HttpResponseError - from azure.keyvault.secrets import SecretClient - from azure.cli.core._profile import Profile - from azure.cli.core.commands.client_factory import prepare_client_kwargs_track2 + from azure.cli.command_modules.keyvault._client_factory import data_plane_azure_keyvault_secret_client # If neither a specific secret name nor --all is provided, default to copying all secrets. if not name and not all_secrets: @@ -2602,43 +2586,24 @@ def copy_secret(cmd, client, destination_vault, name=None, all_secrets=None, ove if client.vault_url.rstrip('/') == destination_vault.rstrip('/'): raise CLIError("Source and destination Key Vaults cannot be the same.") - credential = None - if hasattr(client, 'credential'): - credential = client.credential - - if not credential: - logger.warning("Credential not found on source client. Falling back to Profile.") - from azure.cli.core._profile import Profile - profile = Profile(cli_ctx=cmd.cli_ctx) - credential, _, _ = profile.get_login_credentials(subscription_id=cmd.cli_ctx.data.get('subscription_id')) - - # Use standard client kwargs for consistent logging/telemetry - client_kwargs = prepare_client_kwargs_track2(cmd.cli_ctx) - # KeyVault clients handle this internally or differently sometimes, mimicking _client_factory - client_kwargs.pop('http_logging_policy', None) - - dest_client = SecretClient( - vault_url=destination_vault, - credential=credential, - api_version='7.4', - verify_challenge_resource=False, - **client_kwargs - ) + command_args = {'vault_base_url': destination_vault} + dest_client = data_plane_azure_keyvault_secret_client(cmd.cli_ctx, command_args) - # Fail fast if destination vault is not accessible or does not exist - try: - # Perform a lightweight call to validate vault accessibility. - # A 404 for a dummy secret name means the vault is reachable but the secret does not exist. - dest_client.get_secret("azure-cli-validation-dummy") - except ResourceNotFoundError: - # Vault is accessible but the dummy secret does not exist, which is expected. - pass - except HttpResponseError as e: - if e.status_code == 404: + # Fail fast if source or destination vault is not accessible or does not exist + for c, vault_url in [(client, client.vault_url), (dest_client, destination_vault)]: + try: + # Perform a lightweight call to validate vault accessibility. + # A 404 for a dummy secret name means the vault is reachable but the secret does not exist. + c.get_secret("azure-cli-validation-dummy") + except ResourceNotFoundError: # Vault is accessible but the dummy secret does not exist, which is expected. pass - else: - raise CLIError(f"Failed to access destination Key Vault '{destination_vault}': {str(e)}") + except HttpResponseError as e: + if e.status_code == 404: + # Vault is accessible but the dummy secret does not exist, which is expected. + pass + else: + raise CLIError(f"Failed to access Key Vault '{vault_url}': {str(e)}") secrets_to_copy = [] if name: From aa3494774c997a73da852467cd838bf20e5a2a19 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Cassanji?= Date: Thu, 5 Mar 2026 22:04:10 -0300 Subject: [PATCH 35/35] Refactor KeyVault secret copy tests to use parameterized key vault names --- .../tests/latest/test_keyvault_commands.py | 93 +++++++------------ 1 file changed, 34 insertions(+), 59 deletions(-) diff --git a/src/azure-cli/azure/cli/command_modules/keyvault/tests/latest/test_keyvault_commands.py b/src/azure-cli/azure/cli/command_modules/keyvault/tests/latest/test_keyvault_commands.py index 4c38254fd0c..c0669ebfe27 100644 --- a/src/azure-cli/azure/cli/command_modules/keyvault/tests/latest/test_keyvault_commands.py +++ b/src/azure-cli/azure/cli/command_modules/keyvault/tests/latest/test_keyvault_commands.py @@ -2789,24 +2789,19 @@ class KeyVaultSecretCopyScenarioTest(ScenarioTest): @AllowLargeResponse() @ResourceGroupPreparer(name_prefix='cli_test_kv_secret_copy', location='eastus2') @KeyVaultPreparer(name_prefix='cli-test-kv-src-', location='eastus2', - additional_params='--enable-rbac-authorization false') - def test_keyvault_secret_copy_single(self, resource_group, key_vault): + additional_params='--enable-rbac-authorization false', parameter_name='src_kv') + @KeyVaultPreparer(name_prefix='cli-test-kv-dst-', location='eastus2', + additional_params='--enable-rbac-authorization false', parameter_name='dest_kv') + def test_keyvault_secret_copy_single(self, resource_group, src_kv, dest_kv): """Test copying a single secret between key vaults.""" self.kwargs.update({ - 'src_kv': key_vault, - 'dest_kv': self.create_random_name('cli-test-kv-dst-', 24), + 'src_kv': src_kv, + 'dest_kv': dest_kv, 'secret1': self.create_random_name('secret-', 24), 'secret_value': 'TestSecretValue123', 'loc': 'eastus2' }) - # Create destination vault - self.cmd('keyvault create -g {rg} -n {dest_kv} -l {loc} --enable-rbac-authorization false', - checks=[ - self.check('name', '{dest_kv}'), - self.check('properties.enableSoftDelete', True) - ]) - # Create secret in source vault with metadata self.cmd('keyvault secret set --vault-name {src_kv} -n {secret1} --value {secret_value} ' '--tags environment=test owner=cli --content-type text/plain', @@ -2837,19 +2832,17 @@ def test_keyvault_secret_copy_single(self, resource_group, key_vault): self.check('name', '{secret1}') ]) - # Cleanup - self.cmd('keyvault delete -g {rg} -n {dest_kv}') - self.cmd('keyvault purge -n {dest_kv}') - @AllowLargeResponse() @ResourceGroupPreparer(name_prefix='cli_test_kv_secret_copy', location='eastus2') @KeyVaultPreparer(name_prefix='cli-test-kv-src-', location='eastus2', - additional_params='--enable-rbac-authorization false') - def test_keyvault_secret_copy_all(self, resource_group, key_vault): + additional_params='--enable-rbac-authorization false', parameter_name='src_kv') + @KeyVaultPreparer(name_prefix='cli-test-kv-dst-', location='eastus2', + additional_params='--enable-rbac-authorization false', parameter_name='dest_kv') + def test_keyvault_secret_copy_all(self, resource_group, src_kv, dest_kv): """Test copying all secrets from source to destination vault.""" self.kwargs.update({ - 'src_kv': key_vault, - 'dest_kv': self.create_random_name('cli-test-kv-dst-', 24), + 'src_kv': src_kv, + 'dest_kv': dest_kv, 'secret1': self.create_random_name('secret1-', 24), 'secret2': self.create_random_name('secret2-', 24), 'secret3': self.create_random_name('secret3-', 24), @@ -2859,9 +2852,6 @@ def test_keyvault_secret_copy_all(self, resource_group, key_vault): 'loc': 'eastus2' }) - # Create destination vault - self.cmd('keyvault create -g {rg} -n {dest_kv} -l {loc} --enable-rbac-authorization false') - # Create multiple secrets in source vault self.cmd('keyvault secret set --vault-name {src_kv} -n {secret1} --value {value1}', checks=self.check('value', '{value1}')) @@ -2885,28 +2875,23 @@ def test_keyvault_secret_copy_all(self, resource_group, key_vault): self.cmd('keyvault secret list --vault-name {src_kv}', checks=self.check('length(@)', 3)) - # Cleanup - self.cmd('keyvault delete -g {rg} -n {dest_kv}') - self.cmd('keyvault purge -n {dest_kv}') - @AllowLargeResponse() @ResourceGroupPreparer(name_prefix='cli_test_kv_secret_copy', location='eastus2') @KeyVaultPreparer(name_prefix='cli-test-kv-src-', location='eastus2', - additional_params='--enable-rbac-authorization false') - def test_keyvault_secret_copy_overwrite_behavior(self, resource_group, key_vault): + additional_params='--enable-rbac-authorization false', parameter_name='src_kv') + @KeyVaultPreparer(name_prefix='cli-test-kv-dst-', location='eastus2', + additional_params='--enable-rbac-authorization false', parameter_name='dest_kv') + def test_keyvault_secret_copy_overwrite_behavior(self, resource_group, src_kv, dest_kv): """Test overwrite behavior when copying secrets that already exist.""" self.kwargs.update({ - 'src_kv': key_vault, - 'dest_kv': self.create_random_name('cli-test-kv-dst-', 24), + 'src_kv': src_kv, + 'dest_kv': dest_kv, 'secret': self.create_random_name('secret-', 24), 'original_value': 'OriginalValue', 'updated_value': 'UpdatedValue', 'loc': 'eastus2' }) - # Create destination vault - self.cmd('keyvault create -g {rg} -n {dest_kv} -l {loc} --enable-rbac-authorization false') - # Create secret in source vault self.cmd('keyvault secret set --vault-name {src_kv} -n {secret} --value {original_value}') @@ -2935,19 +2920,17 @@ def test_keyvault_secret_copy_overwrite_behavior(self, resource_group, key_vault self.cmd('keyvault secret show --vault-name {dest_kv} -n {secret}', checks=self.check('value', '{updated_value}')) - # Cleanup - self.cmd('keyvault delete -g {rg} -n {dest_kv}') - self.cmd('keyvault purge -n {dest_kv}') - @AllowLargeResponse() @ResourceGroupPreparer(name_prefix='cli_test_kv_secret_copy', location='eastus2') @KeyVaultPreparer(name_prefix='cli-test-kv-src-', location='eastus2', - additional_params='--enable-rbac-authorization false') - def test_keyvault_secret_copy_error_cases(self, resource_group, key_vault): + additional_params='--enable-rbac-authorization false', parameter_name='src_kv') + @KeyVaultPreparer(name_prefix='cli-test-kv-dst-', location='eastus2', + additional_params='--enable-rbac-authorization false', parameter_name='dest_kv') + def test_keyvault_secret_copy_error_cases(self, resource_group, src_kv, dest_kv): """Test error handling for invalid copy operations.""" self.kwargs.update({ - 'src_kv': key_vault, - 'dest_kv': self.create_random_name('cli-test-kv-dst-', 24), + 'src_kv': src_kv, + 'dest_kv': dest_kv, 'nonexistent_kv': 'nonexistent-kv-' + self.create_random_name('', 10), 'secret': self.create_random_name('secret-', 24), 'nonexistent_secret': 'nonexistent-secret-' + self.create_random_name('', 10), @@ -2955,11 +2938,12 @@ def test_keyvault_secret_copy_error_cases(self, resource_group, key_vault): 'loc': 'eastus2' }) - # Create destination vault and a secret - self.cmd('keyvault create -g {rg} -n {dest_kv} -l {loc} --enable-rbac-authorization false') + # Create a secret self.cmd('keyvault secret set --vault-name {src_kv} -n {secret} --value {secret_value}') - # Test 1: Copy to non-existent destination vault (should fail) + # Test 1: Copy to non-existent destination vault + # In playback mode, accessing a non-existent vault triggers an unknown host or similar network error, + # but the command should still fail handled. We run this to expect failure. self.cmd('keyvault secret copy --source-vault {src_kv} --destination-vault {nonexistent_kv} ' '--name {secret}', expect_failure=True) @@ -2984,19 +2968,17 @@ def test_keyvault_secret_copy_error_cases(self, resource_group, key_vault): '--name {secret} --all', expect_failure=True) - # Cleanup - self.cmd('keyvault delete -g {rg} -n {dest_kv}') - self.cmd('keyvault purge -n {dest_kv}') - @AllowLargeResponse() @ResourceGroupPreparer(name_prefix='cli_test_kv_secret_copy', location='eastus2') @KeyVaultPreparer(name_prefix='cli-test-kv-src-', location='eastus2', - additional_params='--enable-rbac-authorization false') - def test_keyvault_secret_copy_default_behavior(self, resource_group, key_vault): + additional_params='--enable-rbac-authorization false', parameter_name='src_kv') + @KeyVaultPreparer(name_prefix='cli-test-kv-dst-', location='eastus2', + additional_params='--enable-rbac-authorization false', parameter_name='dest_kv') + def test_keyvault_secret_copy_default_behavior(self, resource_group, src_kv, dest_kv): """Test default behavior when neither --name nor --all is specified.""" self.kwargs.update({ - 'src_kv': key_vault, - 'dest_kv': self.create_random_name('cli-test-kv-dst-', 24), + 'src_kv': src_kv, + 'dest_kv': dest_kv, 'secret1': self.create_random_name('secret1-', 24), 'secret2': self.create_random_name('secret2-', 24), 'value1': 'DefaultValue1', @@ -3004,9 +2986,6 @@ def test_keyvault_secret_copy_default_behavior(self, resource_group, key_vault): 'loc': 'eastus2' }) - # Create destination vault - self.cmd('keyvault create -g {rg} -n {dest_kv} -l {loc} --enable-rbac-authorization false') - # Create secrets in source vault self.cmd('keyvault secret set --vault-name {src_kv} -n {secret1} --value {value1}') self.cmd('keyvault secret set --vault-name {src_kv} -n {secret2} --value {value2}') @@ -3020,10 +2999,6 @@ def test_keyvault_secret_copy_default_behavior(self, resource_group, key_vault): self.cmd('keyvault secret show --vault-name {dest_kv} -n {secret2}', checks=self.check('value', '{value2}')) - # Cleanup - self.cmd('keyvault delete -g {rg} -n {dest_kv}') - self.cmd('keyvault purge -n {dest_kv}') - class KeyVaultSecretCopyUnitTest(unittest.TestCase): """Unit tests for the copy_secret function with mocked dependencies."""