diff --git a/src/azure-cli/azure/cli/command_modules/appservice/custom.py b/src/azure-cli/azure/cli/command_modules/appservice/custom.py index 386ba088608..442cb1aa7a6 100644 --- a/src/azure-cli/azure/cli/command_modules/appservice/custom.py +++ b/src/azure-cli/azure/cli/command_modules/appservice/custom.py @@ -3598,34 +3598,64 @@ def _redact_connection_strings(properties): APPSETTINGS_TO_MASK = ['DOCKER_REGISTRY_SERVER_PASSWORD'] +def _is_key_vault_reference(value): + """Check if a setting value is a Key Vault reference.""" + return isinstance(value, str) and value.strip().startswith('@Microsoft.KeyVault(') + + def update_container_settings(cmd, resource_group_name, name, container_registry_url=None, container_image_name=None, container_registry_user=None, websites_enable_app_service_storage=None, container_registry_password=None, multicontainer_config_type=None, multicontainer_config_file=None, slot=None, min_replicas=None, max_replicas=None): - settings = [] - if container_registry_url is not None: - settings.append('DOCKER_REGISTRY_SERVER_URL=' + container_registry_url) + # Read existing app settings so we can preserve non-container settings and Key Vault references + existing_app_settings = _generic_site_operation(cmd.cli_ctx, resource_group_name, name, + 'list_application_settings', slot) + existing_properties = existing_app_settings.properties or {} - if (not container_registry_user and not container_registry_password and - container_registry_url and '.azurecr.io' in container_registry_url): - logger.warning('No credential was provided to access Azure Container Registry. Trying to look up...') - parsed = urlparse(container_registry_url) - registry_name = (parsed.netloc if parsed.scheme else parsed.path).split('.')[0] - try: - container_registry_user, container_registry_password = _get_acr_cred(cmd.cli_ctx, registry_name) - except Exception as ex: # pylint: disable=broad-except - logger.warning("Retrieving credentials failed with an exception:'%s'", ex) # consider throw if needed + if container_registry_url is not None: + if (not container_registry_user and not container_registry_password and + '.azurecr.io' in container_registry_url): + existing_user_val = existing_properties.get('DOCKER_REGISTRY_SERVER_USERNAME', '') + existing_pass_val = existing_properties.get('DOCKER_REGISTRY_SERVER_PASSWORD', '') + if _is_key_vault_reference(existing_user_val) or _is_key_vault_reference(existing_pass_val): + logger.warning('Existing registry credentials use Key Vault references. ' + 'Skipping automatic credential lookup.') + else: + logger.warning('No credential was provided to access Azure Container Registry. ' + 'Trying to look up...') + parsed = urlparse(container_registry_url) + registry_name = (parsed.netloc if parsed.scheme else parsed.path).split('.')[0] + try: + container_registry_user, container_registry_password = _get_acr_cred(cmd.cli_ctx, + registry_name) + except Exception as ex: # pylint: disable=broad-except + logger.warning("Retrieving credentials failed with an exception:'%s'", ex) + # Build dict of only the container-specific settings that were explicitly provided + container_updates = {} + if container_registry_url is not None: + container_updates['DOCKER_REGISTRY_SERVER_URL'] = container_registry_url if container_registry_user is not None: - settings.append('DOCKER_REGISTRY_SERVER_USERNAME=' + container_registry_user) + container_updates['DOCKER_REGISTRY_SERVER_USERNAME'] = container_registry_user if container_registry_password is not None: - settings.append('DOCKER_REGISTRY_SERVER_PASSWORD=' + container_registry_password) + container_updates['DOCKER_REGISTRY_SERVER_PASSWORD'] = container_registry_password if websites_enable_app_service_storage: - settings.append('WEBSITES_ENABLE_APP_SERVICE_STORAGE=' + websites_enable_app_service_storage) + container_updates['WEBSITES_ENABLE_APP_SERVICE_STORAGE'] = websites_enable_app_service_storage - if container_registry_user or container_registry_password or container_registry_url or websites_enable_app_service_storage: # pylint: disable=line-too-long - update_app_settings(cmd, resource_group_name, name, settings, slot) + if container_updates: + # Merge only container-specific keys into the existing settings, + # preserving all other app settings (including Key Vault references) as-is + for key, value in container_updates.items(): + existing_app_settings.properties[key] = value + client = web_client_factory(cmd.cli_ctx) + if is_centauri_functionapp(cmd, resource_group_name, name): + update_application_settings_polling(cmd, resource_group_name, name, + existing_app_settings, slot, client) + else: + _generic_settings_operation(cmd.cli_ctx, resource_group_name, name, + 'update_application_settings', + existing_app_settings, slot, client) settings = get_app_settings(cmd, resource_group_name, name, slot) if container_image_name is not None: _add_fx_version(cmd, resource_group_name, name, container_image_name, slot) diff --git a/src/azure-cli/azure/cli/command_modules/appservice/tests/latest/test_webapp_commands_thru_mock.py b/src/azure-cli/azure/cli/command_modules/appservice/tests/latest/test_webapp_commands_thru_mock.py index 853eadc1edd..45816f80f14 100644 --- a/src/azure-cli/azure/cli/command_modules/appservice/tests/latest/test_webapp_commands_thru_mock.py +++ b/src/azure-cli/azure/cli/command_modules/appservice/tests/latest/test_webapp_commands_thru_mock.py @@ -33,6 +33,8 @@ add_github_actions, update_app_settings, update_application_settings_polling, + update_container_settings, + _is_key_vault_reference, update_webapp) # pylint: disable=line-too-long @@ -639,6 +641,91 @@ def test_update_webapp_platform_release_channel_latest(self): self.assertEqual(result.additional_properties["properties"]["platformReleaseChannel"], "Latest") +class TestUpdateContainerSettingsPreservesKeyVaultRefs(unittest.TestCase): + + @mock.patch('azure.cli.command_modules.appservice.custom._get_fx_version', return_value='DOCKER|myimage:latest') + @mock.patch('azure.cli.command_modules.appservice.custom.get_app_settings') + @mock.patch('azure.cli.command_modules.appservice.custom._generic_settings_operation') + @mock.patch('azure.cli.command_modules.appservice.custom.is_centauri_functionapp', return_value=False) + @mock.patch('azure.cli.command_modules.appservice.custom.web_client_factory') + @mock.patch('azure.cli.command_modules.appservice.custom._generic_site_operation') + def test_container_set_preserves_kv_ref_settings(self, mock_site_op, mock_client_factory, + mock_centauri, mock_settings_op, + mock_get_app_settings, mock_get_fx): + """Key Vault reference app settings must survive az webapp config container set.""" + cmd_mock = _get_test_cmd() + + kv_ref = '@Microsoft.KeyVault(SecretUri=https://myvault.vault.azure.net/secrets/mysecret)' + existing_properties = { + 'MY_KV_SECRET': kv_ref, + 'NORMAL_SETTING': 'normal_value', + 'DOCKER_REGISTRY_SERVER_URL': 'https://old.azurecr.io', + } + mock_app_settings = mock.MagicMock() + mock_app_settings.properties = dict(existing_properties) + mock_site_op.return_value = mock_app_settings + + mock_get_app_settings.return_value = [ + {'name': 'MY_KV_SECRET', 'value': kv_ref, 'slotSetting': False}, + {'name': 'NORMAL_SETTING', 'value': 'normal_value', 'slotSetting': False}, + {'name': 'DOCKER_REGISTRY_SERVER_URL', 'value': 'https://new.example.io', 'slotSetting': False}, + ] + + update_container_settings(cmd_mock, 'test-rg', 'test-app', + container_registry_url='https://new.example.io') + + # The settings written back must still contain the KV ref and normal setting + written_props = mock_app_settings.properties + self.assertEqual(written_props['MY_KV_SECRET'], kv_ref) + self.assertEqual(written_props['NORMAL_SETTING'], 'normal_value') + self.assertEqual(written_props['DOCKER_REGISTRY_SERVER_URL'], 'https://new.example.io') + + @mock.patch('azure.cli.command_modules.appservice.custom._get_fx_version', return_value='DOCKER|myimage:latest') + @mock.patch('azure.cli.command_modules.appservice.custom.get_app_settings') + @mock.patch('azure.cli.command_modules.appservice.custom._generic_settings_operation') + @mock.patch('azure.cli.command_modules.appservice.custom.is_centauri_functionapp', return_value=False) + @mock.patch('azure.cli.command_modules.appservice.custom.web_client_factory') + @mock.patch('azure.cli.command_modules.appservice.custom._generic_site_operation') + def test_container_set_skips_acr_auto_detect_when_kv_refs(self, mock_site_op, mock_client_factory, + mock_centauri, mock_settings_op, + mock_get_app_settings, mock_get_fx): + """ACR credential auto-detection must be skipped when existing creds are KV references.""" + cmd_mock = _get_test_cmd() + + kv_user = '@Microsoft.KeyVault(SecretUri=https://vault.vault.azure.net/secrets/user)' + kv_pass = '@Microsoft.KeyVault(SecretUri=https://vault.vault.azure.net/secrets/pass)' + existing_properties = { + 'DOCKER_REGISTRY_SERVER_URL': 'https://old.azurecr.io', + 'DOCKER_REGISTRY_SERVER_USERNAME': kv_user, + 'DOCKER_REGISTRY_SERVER_PASSWORD': kv_pass, + } + mock_app_settings = mock.MagicMock() + mock_app_settings.properties = dict(existing_properties) + mock_site_op.return_value = mock_app_settings + + mock_get_app_settings.return_value = [ + {'name': 'DOCKER_REGISTRY_SERVER_URL', 'value': 'https://myregistry.azurecr.io', 'slotSetting': False}, + ] + + with mock.patch('azure.cli.command_modules.appservice.custom._get_acr_cred') as mock_acr_cred: + update_container_settings(cmd_mock, 'test-rg', 'test-app', + container_registry_url='https://myregistry.azurecr.io') + # _get_acr_cred should NOT have been called because existing creds are KV refs + mock_acr_cred.assert_not_called() + + # Existing KV references for username/password must be preserved + written_props = mock_app_settings.properties + self.assertEqual(written_props['DOCKER_REGISTRY_SERVER_USERNAME'], kv_user) + self.assertEqual(written_props['DOCKER_REGISTRY_SERVER_PASSWORD'], kv_pass) + + def test_is_key_vault_reference_detects_kv_refs(self): + self.assertTrue(_is_key_vault_reference('@Microsoft.KeyVault(SecretUri=https://v.vault.azure.net/secrets/s)')) + self.assertTrue(_is_key_vault_reference(' @Microsoft.KeyVault(VaultName=v;SecretName=s)')) + self.assertFalse(_is_key_vault_reference('plain_value')) + self.assertFalse(_is_key_vault_reference('')) + self.assertFalse(_is_key_vault_reference(None)) + + class FakedResponse: # pylint: disable=too-few-public-methods def __init__(self, status_code): self.status_code = status_code