From 1bdf982a93d523479f3c42789fdd617104546828 Mon Sep 17 00:00:00 2001 From: Jordan Selig Date: Thu, 26 Mar 2026 10:49:21 -0400 Subject: [PATCH 1/4] [App Service] Fix #30597, #29495, #26334: Connection string replace, SSL cert list pagination, slot ID parsing Bug fixes: - #30597: az webapp config connection-string set with JSON file now replaces the full set of connection strings instead of only merging. When settings are provided as JSON (e.g. --settings @file.json), existing connection strings not in the JSON are removed. Key=value format retains merge behavior. - #29495: list_ssl_certs now wraps the SDK pager with list() to ensure full pagination, matching the pattern used by other list commands in the module. - #26334: The slot parameter in the webapp argument context now includes id_part="child_name_1", so --ids with a slot resource ID (e.g. .../sites/myApp/slots/staging) correctly populates the slot argument instead of falling back to the production slot. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../cli/command_modules/appservice/_params.py | 2 +- .../cli/command_modules/appservice/custom.py | 19 ++- .../latest/test_webapp_commands_thru_mock.py | 155 ++++++++++++++++++ 3 files changed, 174 insertions(+), 2 deletions(-) diff --git a/src/azure-cli/azure/cli/command_modules/appservice/_params.py b/src/azure-cli/azure/cli/command_modules/appservice/_params.py index bd4dc0b7ea5..f7b3ed006b7 100644 --- a/src/azure-cli/azure/cli/command_modules/appservice/_params.py +++ b/src/azure-cli/azure/cli/command_modules/appservice/_params.py @@ -90,7 +90,7 @@ def load_arguments(self, _): c.ignore('app_instance') c.argument('resource_group_name', arg_type=resource_group_name_type) c.argument('location', arg_type=get_location_type(self.cli_ctx)) - c.argument('slot', options_list=['--slot', '-s'], + c.argument('slot', options_list=['--slot', '-s'], id_part='child_name_1', help="the name of the slot. Default to the productions slot if not specified") c.argument('name', arg_type=webapp_name_arg_type) 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..411de41f930 100644 --- a/src/azure-cli/azure/cli/command_modules/appservice/custom.py +++ b/src/azure-cli/azure/cli/command_modules/appservice/custom.py @@ -3494,6 +3494,18 @@ def _redact_appsettings(settings): return settings +def _is_json_settings(settings): + """Check if settings input is in JSON format (e.g. from @file.json).""" + if not settings: + return False + try: + settings_str = ''.join([i.rstrip() for i in settings]) + json.loads(settings_str) + return True + except (json.decoder.JSONDecodeError, ValueError): + return False + + def _build_app_settings_input(settings, connection_string_type): if not settings: return [] @@ -3527,6 +3539,8 @@ def update_connection_strings(cmd, resource_group_name, name, connection_string_ from azure.mgmt.web.models import ConnStringValueTypePair if not settings and not slot_settings: raise ArgumentUsageError('Usage Error: --settings |--slot-settings') + # Detect JSON input before parsing — JSON means replace-all semantics + replace_all = _is_json_settings(settings) settings = _build_app_settings_input(settings, connection_string_type) sticky_slot_settings = _build_app_settings_input(slot_settings, connection_string_type) rm_sticky_slot_settings = set() @@ -3534,6 +3548,9 @@ def update_connection_strings(cmd, resource_group_name, name, connection_string_ conn_strings = _generic_site_operation(cmd.cli_ctx, resource_group_name, name, 'list_connection_strings', slot) + if replace_all: + conn_strings.properties = {} + for name_value_type in settings + sticky_slot_settings: # split at the first '=', connection string should not have '=' in the name conn_strings.properties[name_value_type['name']] = ConnStringValueTypePair(value=name_value_type['value'], @@ -5860,7 +5877,7 @@ def _get_cert(certificate_password, certificate_file): def list_ssl_certs(cmd, resource_group_name): client = web_client_factory(cmd.cli_ctx) - return client.certificates.list_by_resource_group(resource_group_name) + return list(client.certificates.list_by_resource_group(resource_group_name)) def show_ssl_cert(cmd, resource_group_name, certificate_name): 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..352085a01cf 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 @@ -20,8 +20,10 @@ view_in_browser, sync_site_repo, _match_host_names_from_cert, + _is_json_settings, bind_ssl_cert, list_publish_profiles, + list_ssl_certs, show_app, get_streaming_log, download_historical_logs, @@ -32,6 +34,7 @@ create_managed_ssl_cert, add_github_actions, update_app_settings, + update_connection_strings, update_application_settings_polling, update_webapp) @@ -639,6 +642,158 @@ def test_update_webapp_platform_release_channel_latest(self): self.assertEqual(result.additional_properties["properties"]["platformReleaseChannel"], "Latest") +class TestIsJsonSettings(unittest.TestCase): + """Tests for _is_json_settings helper (Issue #30597).""" + + def test_json_array_input_detected(self): + settings = ['[{"name":"conn1","value":"val1","type":"SQLAzure"}]'] + self.assertTrue(_is_json_settings(settings)) + + def test_json_object_input_detected(self): + settings = ['{"name":"conn1","value":"val1","type":"SQLAzure"}'] + self.assertTrue(_is_json_settings(settings)) + + def test_key_value_input_not_json(self): + settings = ['conn1=Server=tcp:myserver;Database=mydb'] + self.assertFalse(_is_json_settings(settings)) + + def test_empty_input(self): + self.assertFalse(_is_json_settings(None)) + self.assertFalse(_is_json_settings([])) + + def test_multi_key_value_not_json(self): + settings = ['key1=value1', 'key2=value2'] + self.assertFalse(_is_json_settings(settings)) + + +class TestUpdateConnectionStringsReplace(unittest.TestCase): + """Tests for update_connection_strings JSON replace-all semantics (Issue #30597).""" + + @mock.patch('azure.cli.command_modules.appservice.custom._generic_settings_operation') + @mock.patch('azure.cli.command_modules.appservice.custom._generic_site_operation') + @mock.patch('azure.cli.command_modules.appservice.custom.web_client_factory') + @mock.patch('azure.cli.command_modules.appservice.custom._redact_connection_strings') + def test_json_settings_replaces_all(self, mock_redact, mock_client_factory, mock_site_op, mock_settings_op): + """When settings are JSON, existing connection strings not in the JSON should be removed.""" + from azure.mgmt.web.models import ConnStringValueTypePair + cmd_mock = _get_test_cmd() + + existing_conn_strings = mock.MagicMock() + existing_conn_strings.properties = { + 'OldConn': ConnStringValueTypePair(value='old_val', type='SQLAzure'), + 'KeepConn': ConnStringValueTypePair(value='keep_val', type='Custom'), + } + mock_site_op.return_value = existing_conn_strings + + result_mock = mock.MagicMock() + result_mock.properties = {} + mock_settings_op.return_value = result_mock + mock_redact.return_value = {} + + client_mock = mock.MagicMock() + mock_client_factory.return_value = client_mock + + json_settings = ['[{"name":"NewConn","value":"new_val","type":"SQLAzure"}]'] + update_connection_strings(cmd_mock, 'rg', 'app', settings=json_settings) + + # OldConn and KeepConn should have been cleared; only NewConn should remain + self.assertNotIn('OldConn', existing_conn_strings.properties) + self.assertNotIn('KeepConn', existing_conn_strings.properties) + self.assertIn('NewConn', existing_conn_strings.properties) + + @mock.patch('azure.cli.command_modules.appservice.custom._generic_settings_operation') + @mock.patch('azure.cli.command_modules.appservice.custom._generic_site_operation') + @mock.patch('azure.cli.command_modules.appservice.custom.web_client_factory') + @mock.patch('azure.cli.command_modules.appservice.custom._redact_connection_strings') + def test_key_value_settings_merges(self, mock_redact, mock_client_factory, mock_site_op, mock_settings_op): + """When settings are key=value, existing connection strings should be preserved (merge).""" + from azure.mgmt.web.models import ConnStringValueTypePair + cmd_mock = _get_test_cmd() + + existing_conn_strings = mock.MagicMock() + existing_conn_strings.properties = { + 'OldConn': ConnStringValueTypePair(value='old_val', type='SQLAzure'), + } + mock_site_op.return_value = existing_conn_strings + + result_mock = mock.MagicMock() + result_mock.properties = {} + mock_settings_op.return_value = result_mock + mock_redact.return_value = {} + + client_mock = mock.MagicMock() + mock_client_factory.return_value = client_mock + + kv_settings = ['NewConn=new_val'] + update_connection_strings(cmd_mock, 'rg', 'app', connection_string_type='SQLAzure', settings=kv_settings) + + # OldConn should still be present (merge behavior) + self.assertIn('OldConn', existing_conn_strings.properties) + self.assertIn('NewConn', existing_conn_strings.properties) + + +class TestListSslCertsPagination(unittest.TestCase): + """Tests for list_ssl_certs pagination fix (Issue #29495).""" + + @mock.patch('azure.cli.command_modules.appservice.custom.web_client_factory') + def test_list_ssl_certs_returns_list(self, mock_client_factory): + """list_ssl_certs should return a list, not a pager object.""" + cmd_mock = _get_test_cmd() + mock_client = mock.MagicMock() + mock_client_factory.return_value = mock_client + + mock_pager = mock.MagicMock() + mock_pager.__iter__ = mock.MagicMock(return_value=iter(['cert1', 'cert2', 'cert3'])) + mock_client.certificates.list_by_resource_group.return_value = mock_pager + + result = list_ssl_certs(cmd_mock, 'test-rg') + self.assertIsInstance(result, list) + self.assertEqual(len(result), 3) + + +class TestSlotIdParsing(unittest.TestCase): + """Tests for --ids slot resource ID parsing (Issue #26334).""" + + def test_parse_resource_id_extracts_slot(self): + """Verify parse_resource_id returns slot name as child_name_1 from a slot resource ID.""" + from azure.mgmt.core.tools import parse_resource_id + + slot_rid = ('/subscriptions/00000000-0000-0000-0000-000000000000' + '/resourceGroups/myRG/providers/Microsoft.Web' + '/sites/mySite/slots/staging') + parsed = parse_resource_id(slot_rid) + + self.assertEqual(parsed.get('name'), 'mySite') + self.assertEqual(parsed.get('child_name_1'), 'staging') + self.assertEqual(parsed.get('child_type_1'), 'slots') + self.assertEqual(parsed.get('resource_group'), 'myRG') + + def test_parse_resource_id_no_slot(self): + """Verify parse_resource_id returns no child_name_1 for a production site resource ID.""" + from azure.mgmt.core.tools import parse_resource_id + + prod_rid = ('/subscriptions/00000000-0000-0000-0000-000000000000' + '/resourceGroups/myRG/providers/Microsoft.Web' + '/sites/mySite') + parsed = parse_resource_id(prod_rid) + + self.assertEqual(parsed.get('name'), 'mySite') + self.assertIsNone(parsed.get('child_name_1')) + + def test_slot_param_configured_with_id_part(self): + """Verify the slot argument in _params.py includes id_part='child_name_1'. + + This ensures the --ids parameter correctly populates the slot argument + when a slot resource ID is provided.""" + import inspect + from azure.cli.command_modules.appservice import _params + + source = inspect.getsource(_params.load_arguments) + # The webapp context should have slot configured with id_part='child_name_1' + self.assertIn("id_part='child_name_1'", source, + "slot argument must have id_part='child_name_1' for --ids slot parsing") + + class FakedResponse: # pylint: disable=too-few-public-methods def __init__(self, status_code): self.status_code = status_code From 1f6dd7b3ac5fc75fb567f4d80dd2a0204250bfb1 Mon Sep 17 00:00:00 2001 From: Jordan Selig Date: Thu, 26 Mar 2026 09:55:39 -0400 Subject: [PATCH 2/4] [App Service] Fix #28836, #30100: Wire through `--protocol` and `--domain-validation-method` params Add --protocol (Smb/Nfs) parameter to `az webapp config storage-account add/update` to support NFS protocol for Azure Files storage mounts (fixes #28836). Add --domain-validation-method parameter to `az webapp config ssl create` to support managed certificate creation for child DNS zones (fixes #30100). Both parameters were already supported by the underlying SDK models but were not wired through the CLI commands. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../cli/command_modules/appservice/_help.py | 17 ++++ .../cli/command_modules/appservice/_params.py | 6 ++ .../cli/command_modules/appservice/custom.py | 21 +++-- .../latest/test_webapp_commands_thru_mock.py | 88 ++++++++++++++++++- 4 files changed, 124 insertions(+), 8 deletions(-) diff --git a/src/azure-cli/azure/cli/command_modules/appservice/_help.py b/src/azure-cli/azure/cli/command_modules/appservice/_help.py index e0dad92e98c..91c75125106 100644 --- a/src/azure-cli/azure/cli/command_modules/appservice/_help.py +++ b/src/azure-cli/azure/cli/command_modules/appservice/_help.py @@ -1776,6 +1776,8 @@ examples: - name: Create a Managed Certificate for cname.mycustomdomain.com. text: az webapp config ssl create --resource-group MyResourceGroup --name MyWebapp --hostname cname.mycustomdomain.com + - name: Create a Managed Certificate for a child DNS zone using domain validation method. + text: az webapp config ssl create --resource-group MyResourceGroup --name MyWebapp --hostname child.mycustomdomain.com --domain-validation-method TXT """ helps['webapp config storage-account'] = """ @@ -1797,6 +1799,16 @@ --share-name MyShare \\ --access-key MyAccessKey \\ --mount-path /path/to/mount + - name: Add an NFS Azure Files connection with Nfs protocol. + text: > + az webapp config storage-account add -g MyResourceGroup -n MyUniqueApp \\ + --custom-id NfsId \\ + --storage-type AzureFiles \\ + --account-name MyStorageAccount \\ + --share-name MyNfsShare \\ + --access-key MyAccessKey \\ + --mount-path /path/to/mount \\ + --protocol Nfs """ helps['webapp config storage-account delete'] = """ @@ -1828,6 +1840,11 @@ az webapp config storage-account update -g MyResourceGroup -n MyUniqueApp \\ --custom-id CustomId \\ --mount-path /path/to/new/mount + - name: Update the protocol for an existing Azure storage account configuration. + text: > + az webapp config storage-account update -g MyResourceGroup -n MyUniqueApp \\ + --custom-id CustomId \\ + --protocol Nfs - name: Update an existing Azure storage account configuration on a web app. text: az webapp config storage-account update --access-key MyAccessKey --account-name MyAccount --custom-id CustomId --mount-path /path/to/new/mount --name MyUniqueApp --resource-group MyResourceGroup --share-name MyShare --storage-type AzureFiles crafted: true diff --git a/src/azure-cli/azure/cli/command_modules/appservice/_params.py b/src/azure-cli/azure/cli/command_modules/appservice/_params.py index f7b3ed006b7..81cf2866d50 100644 --- a/src/azure-cli/azure/cli/command_modules/appservice/_params.py +++ b/src/azure-cli/azure/cli/command_modules/appservice/_params.py @@ -521,6 +521,9 @@ def load_arguments(self, _): c.argument('hostname', help='The custom domain name') c.argument('name', options_list=['--name', '-n'], help='Name of the web app.') c.argument('resource-group', options_list=['--resource-group', '-g'], help='Name of resource group.') + c.argument('domain_validation_method', options_list=['--domain-validation-method'], + help='Method used for domain validation. Use this when the certificate needs to validate a ' + 'child DNS zone, e.g. "TXT" for TXT record validation.') with self.argument_context(scope + ' config hostname') as c: c.argument('hostname', completer=get_hostname_completion_list, help="hostname assigned to the site, such as custom domains", id_part='child_name_1') @@ -816,6 +819,9 @@ def load_arguments(self, _): help='the path which the web app uses to read-write data ex: /share1 or /share2') c.argument('slot', options_list=['--slot', '-s'], help="the name of the slot. Default to the productions slot if not specified") + c.argument('protocol', options_list=['--protocol'], + arg_type=get_enum_type(['Smb', 'Nfs']), + help='the protocol used to mount the storage account, e.g. Smb or Nfs') with self.argument_context('webapp config storage-account add') as c: c.argument('slot_setting', options_list=['--slot-setting'], help="With slot setting you can decide to make BYOS configuration sticky to a slot, meaning that when that slot is swapped, the storage account stays with that slot.") with self.argument_context('webapp config storage-account update') as c: 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 411de41f930..50b08cfbb1c 100644 --- a/src/azure-cli/azure/cli/command_modules/appservice/custom.py +++ b/src/azure-cli/azure/cli/command_modules/appservice/custom.py @@ -653,7 +653,8 @@ def update_application_settings_polling(cmd, resource_group_name, name, app_sett def add_azure_storage_account(cmd, resource_group_name, name, custom_id, storage_type, account_name, - share_name, access_key, mount_path=None, slot=None, slot_setting=False): + share_name, access_key, mount_path=None, slot=None, slot_setting=False, + protocol=None): AzureStorageInfoValue = cmd.get_models('AzureStorageInfoValue') azure_storage_accounts = _generic_site_operation(cmd.cli_ctx, resource_group_name, name, 'list_azure_storage_accounts', slot) @@ -665,7 +666,7 @@ def add_azure_storage_account(cmd, resource_group_name, name, custom_id, storage azure_storage_accounts.properties[custom_id] = AzureStorageInfoValue(type=storage_type, account_name=account_name, share_name=share_name, access_key=access_key, - mount_path=mount_path) + mount_path=mount_path, protocol=protocol) client = web_client_factory(cmd.cli_ctx) result = _generic_settings_operation(cmd.cli_ctx, resource_group_name, name, @@ -684,7 +685,8 @@ def add_azure_storage_account(cmd, resource_group_name, name, custom_id, storage def update_azure_storage_account(cmd, resource_group_name, name, custom_id, storage_type=None, account_name=None, - share_name=None, access_key=None, mount_path=None, slot=None, slot_setting=False): + share_name=None, access_key=None, mount_path=None, slot=None, slot_setting=False, + protocol=None): AzureStorageInfoValue = cmd.get_models('AzureStorageInfoValue') azure_storage_accounts = _generic_site_operation(cmd.cli_ctx, resource_group_name, name, @@ -702,7 +704,8 @@ def update_azure_storage_account(cmd, resource_group_name, name, custom_id, stor account_name=account_name or existing_account_config.account_name, share_name=share_name or existing_account_config.share_name, access_key=access_key or existing_account_config.access_key, - mount_path=mount_path or existing_account_config.mount_path + mount_path=mount_path or existing_account_config.mount_path, + protocol=protocol or existing_account_config.protocol ) azure_storage_accounts.properties[custom_id] = new_account_config @@ -5977,7 +5980,8 @@ def import_ssl_cert(cmd, resource_group_name, key_vault, key_vault_certificate_n certificate_envelope=kv_cert_def) -def create_managed_ssl_cert(cmd, resource_group_name, name, hostname, slot=None, certificate_name=None): +def create_managed_ssl_cert(cmd, resource_group_name, name, hostname, slot=None, certificate_name=None, + domain_validation_method=None): Certificate = cmd.get_models('Certificate') hostname = hostname.lower() client = web_client_factory(cmd.cli_ctx) @@ -6002,8 +6006,11 @@ def create_managed_ssl_cert(cmd, resource_group_name, name, hostname, slot=None, server_farm_id = webapp.server_farm_id location = webapp.location - easy_cert_def = Certificate(location=location, canonical_name=hostname, - server_farm_id=server_farm_id, password='') + cert_kwargs = dict(location=location, canonical_name=hostname, + server_farm_id=server_farm_id, password='') + if domain_validation_method: + cert_kwargs['domain_validation_method'] = domain_validation_method + easy_cert_def = Certificate(**cert_kwargs) # TODO: Update manual polling to use LongRunningOperation once backend API & new SDK supports polling try: 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 352085a01cf..66b7662f7b7 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 @@ -36,7 +36,9 @@ update_app_settings, update_connection_strings, update_application_settings_polling, - update_webapp) + update_webapp, + add_azure_storage_account, + update_azure_storage_account) # pylint: disable=line-too-long from azure.cli.core.profiles import ResourceType @@ -471,6 +473,90 @@ def test_create_managed_ssl_cert(self, generic_site_op_mock, client_factory_mock client.certificates.create_or_update.assert_called_once_with(name=host_name, resource_group_name=rg_name, certificate_envelope=cert_def) + @mock.patch('azure.cli.command_modules.appservice.custom._verify_hostname_binding', autospec=True) + @mock.patch('azure.cli.command_modules.appservice.custom.web_client_factory', autospec=True) + @mock.patch('azure.cli.command_modules.appservice.custom._generic_site_operation', autospec=True) + def test_create_managed_ssl_cert_with_domain_validation_method(self, generic_site_op_mock, client_factory_mock, verify_binding_mock): + webapp_name = 'someWebAppName' + rg_name = 'someRgName' + farm_id = '/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/rg1/providers/Microsoft.Web/serverfarms/farm1' + host_name = 'child.mycustomdomain.com' + + client = mock.MagicMock() + client_factory_mock.return_value = client + cmd_mock = _get_test_cmd() + cli_ctx_mock = mock.MagicMock() + cli_ctx_mock.data = {'subscription_id': 'sub1'} + cmd_mock.cli_ctx = cli_ctx_mock + Site, Certificate = cmd_mock.get_models('Site', 'Certificate') + site = Site(name=webapp_name, location='westeurope') + site.server_farm_id = farm_id + generic_site_op_mock.return_value = site + + verify_binding_mock.return_value = True + create_managed_ssl_cert(cmd_mock, rg_name, webapp_name, host_name, None, + domain_validation_method='TXT') + + cert_def = Certificate(location='westeurope', canonical_name=host_name, + server_farm_id=farm_id, password='', domain_validation_method='TXT') + client.certificates.create_or_update.assert_called_once_with(name=host_name, resource_group_name=rg_name, + certificate_envelope=cert_def) + + @mock.patch('azure.cli.command_modules.appservice.custom._generic_settings_operation', autospec=True) + @mock.patch('azure.cli.command_modules.appservice.custom.web_client_factory', autospec=True) + @mock.patch('azure.cli.command_modules.appservice.custom._generic_site_operation', autospec=True) + def test_add_azure_storage_account_with_protocol(self, generic_site_op_mock, client_factory_mock, settings_op_mock): + rg_name = 'someRgName' + webapp_name = 'someWebAppName' + + cmd_mock = _get_test_cmd() + AzureStorageInfoValue = cmd_mock.get_models('AzureStorageInfoValue') + + storage_accounts = mock.MagicMock() + storage_accounts.properties = {} + generic_site_op_mock.return_value = storage_accounts + + result_mock = mock.MagicMock() + result_mock.properties = {} + settings_op_mock.return_value = result_mock + + add_azure_storage_account(cmd_mock, rg_name, webapp_name, custom_id='myId', + storage_type='AzureFiles', account_name='myAccount', + share_name='myShare', access_key='myKey', + mount_path='/mnt/share', protocol='Nfs') + + expected = AzureStorageInfoValue(type='AzureFiles', account_name='myAccount', + share_name='myShare', access_key='myKey', + mount_path='/mnt/share', protocol='Nfs') + self.assertEqual(storage_accounts.properties['myId'].protocol, expected.protocol) + + @mock.patch('azure.cli.command_modules.appservice.custom._generic_settings_operation', autospec=True) + @mock.patch('azure.cli.command_modules.appservice.custom.web_client_factory', autospec=True) + @mock.patch('azure.cli.command_modules.appservice.custom._generic_site_operation', autospec=True) + def test_update_azure_storage_account_with_protocol(self, generic_site_op_mock, client_factory_mock, settings_op_mock): + rg_name = 'someRgName' + webapp_name = 'someWebAppName' + + cmd_mock = _get_test_cmd() + AzureStorageInfoValue = cmd_mock.get_models('AzureStorageInfoValue') + + existing_config = AzureStorageInfoValue(type='AzureFiles', account_name='myAccount', + share_name='myShare', access_key='myKey', + mount_path='/mnt/share', protocol='Smb') + storage_accounts = mock.MagicMock() + storage_accounts.properties = {'myId': existing_config} + generic_site_op_mock.return_value = storage_accounts + + result_mock = mock.MagicMock() + result_mock.properties = {} + settings_op_mock.return_value = result_mock + + update_azure_storage_account(cmd_mock, rg_name, webapp_name, custom_id='myId', + protocol='Nfs') + + new_config = storage_accounts.properties['myId'] + self.assertEqual(new_config.protocol, 'Nfs') + def test_update_app_settings_error_handling_no_parameters(self): """Test that MutuallyExclusiveArgumentError is raised when neither settings nor slot_settings are provided.""" From 9f725defe012ca982772f979f3345d2cafe6300f Mon Sep 17 00:00:00 2001 From: Jordan Selig Date: Thu, 26 Mar 2026 10:20:13 -0400 Subject: [PATCH 3/4] Fix linter: add --validation-method alias and test coverage exclusions - Add --validation-method as shorter alias for --domain-validation-method to satisfy option_length_too_long (HIGH severity) linter rule - Add missing_parameter_test_coverage exclusions for new params: domain_validation_method (webapp/functionapp config ssl create) protocol (webapp config storage-account add/update) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- linter_exclusions.yml | 20 +++++++++++++++++++ .../cli/command_modules/appservice/_params.py | 2 +- 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/linter_exclusions.yml b/linter_exclusions.yml index 444667c011e..baaaff4cb89 100644 --- a/linter_exclusions.yml +++ b/linter_exclusions.yml @@ -2005,6 +2005,11 @@ functionapp config ssl delete: certificate_thumbprint: rule_exclusions: - option_length_too_long +functionapp config ssl create: + parameters: + domain_validation_method: + rule_exclusions: + - missing_parameter_test_coverage functionapp config ssl import: parameters: key_vault_certificate_name: @@ -3922,6 +3927,11 @@ webapp config ssl delete: certificate_thumbprint: rule_exclusions: - option_length_too_long +webapp config ssl create: + parameters: + domain_validation_method: + rule_exclusions: + - missing_parameter_test_coverage webapp config ssl import: parameters: key_vault_certificate_name: @@ -3932,6 +3942,16 @@ webapp config ssl unbind: certificate_thumbprint: rule_exclusions: - option_length_too_long +webapp config storage-account add: + parameters: + protocol: + rule_exclusions: + - missing_parameter_test_coverage +webapp config storage-account update: + parameters: + protocol: + rule_exclusions: + - missing_parameter_test_coverage webapp create: parameters: multicontainer_config_file: diff --git a/src/azure-cli/azure/cli/command_modules/appservice/_params.py b/src/azure-cli/azure/cli/command_modules/appservice/_params.py index 81cf2866d50..d70a70e4366 100644 --- a/src/azure-cli/azure/cli/command_modules/appservice/_params.py +++ b/src/azure-cli/azure/cli/command_modules/appservice/_params.py @@ -521,7 +521,7 @@ def load_arguments(self, _): c.argument('hostname', help='The custom domain name') c.argument('name', options_list=['--name', '-n'], help='Name of the web app.') c.argument('resource-group', options_list=['--resource-group', '-g'], help='Name of resource group.') - c.argument('domain_validation_method', options_list=['--domain-validation-method'], + c.argument('domain_validation_method', options_list=['--domain-validation-method', '--validation-method'], help='Method used for domain validation. Use this when the certificate needs to validate a ' 'child DNS zone, e.g. "TXT" for TXT record validation.') with self.argument_context(scope + ' config hostname') as c: From c2e72e4d4da2ed1521528c6b04fbf0db6b923058 Mon Sep 17 00:00:00 2001 From: Jordan Selig Date: Thu, 26 Mar 2026 15:21:37 -0400 Subject: [PATCH 4/4] Address review comments and fix style: slot_cfg reconciliation, stronger test assertions - Fix pylint R1735: use dict literal instead of dict() call (line 6009) - In replace_all mode, reconcile slot_cfg_names.connection_string_names to exactly match new slotSetting=true entries, removing stale names - Strengthen test_slot_param_configured_with_id_part to check that the slot argument specifically (not just any argument) has id_part - Fix TestListSslCertsPagination docstring: remove incorrect Issue #29495 reference since that issue is about az webapp list, not SSL cert pagination Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../cli/command_modules/appservice/custom.py | 12 +++++++--- .../latest/test_webapp_commands_thru_mock.py | 22 +++++++++++++++---- 2 files changed, 27 insertions(+), 7 deletions(-) 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 50b08cfbb1c..5dbc65f6cc7 100644 --- a/src/azure-cli/azure/cli/command_modules/appservice/custom.py +++ b/src/azure-cli/azure/cli/command_modules/appservice/custom.py @@ -3569,7 +3569,13 @@ def update_connection_strings(cmd, resource_group_name, name, connection_string_ 'update_connection_strings', conn_strings, slot, client) - if sticky_slot_settings or rm_sticky_slot_settings: + if replace_all: + # Replace-all: slot config names must exactly match new slotSetting=true entries + new_slot_setting_names = set(n['name'] for n in sticky_slot_settings) + slot_cfg_names = client.web_apps.list_slot_configuration_names(resource_group_name, name) + slot_cfg_names.connection_string_names = list(new_slot_setting_names) + client.web_apps.update_slot_configuration_names(resource_group_name, name, slot_cfg_names) + elif sticky_slot_settings or rm_sticky_slot_settings: new_slot_setting_names = set(n['name'] for n in sticky_slot_settings) # add setting name slot_cfg_names = client.web_apps.list_slot_configuration_names(resource_group_name, name) slot_cfg_names.connection_string_names = set(slot_cfg_names.connection_string_names or []) @@ -6006,8 +6012,8 @@ def create_managed_ssl_cert(cmd, resource_group_name, name, hostname, slot=None, server_farm_id = webapp.server_farm_id location = webapp.location - cert_kwargs = dict(location=location, canonical_name=hostname, - server_farm_id=server_farm_id, password='') + cert_kwargs = {"location": location, "canonical_name": hostname, + "server_farm_id": server_farm_id, "password": ''} if domain_validation_method: cert_kwargs['domain_validation_method'] = domain_validation_method easy_cert_def = Certificate(**cert_kwargs) 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 66b7662f7b7..89ae5203c02 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 @@ -779,6 +779,11 @@ def test_json_settings_replaces_all(self, mock_redact, mock_client_factory, mock client_mock = mock.MagicMock() mock_client_factory.return_value = client_mock + # Simulate stale slot config names for connection strings that will be removed + slot_cfg_mock = mock.MagicMock() + slot_cfg_mock.connection_string_names = ['OldConn', 'KeepConn'] + client_mock.web_apps.list_slot_configuration_names.return_value = slot_cfg_mock + json_settings = ['[{"name":"NewConn","value":"new_val","type":"SQLAzure"}]'] update_connection_strings(cmd_mock, 'rg', 'app', settings=json_settings) @@ -787,6 +792,11 @@ def test_json_settings_replaces_all(self, mock_redact, mock_client_factory, mock self.assertNotIn('KeepConn', existing_conn_strings.properties) self.assertIn('NewConn', existing_conn_strings.properties) + # Stale slot config names should be cleared in replace-all mode + client_mock.web_apps.update_slot_configuration_names.assert_called_once() + updated_cfg = client_mock.web_apps.update_slot_configuration_names.call_args[0][2] + self.assertEqual(updated_cfg.connection_string_names, []) + @mock.patch('azure.cli.command_modules.appservice.custom._generic_settings_operation') @mock.patch('azure.cli.command_modules.appservice.custom._generic_site_operation') @mock.patch('azure.cli.command_modules.appservice.custom.web_client_factory') @@ -819,7 +829,7 @@ def test_key_value_settings_merges(self, mock_redact, mock_client_factory, mock_ class TestListSslCertsPagination(unittest.TestCase): - """Tests for list_ssl_certs pagination fix (Issue #29495).""" + """Tests for list_ssl_certs pagination behavior.""" @mock.patch('azure.cli.command_modules.appservice.custom.web_client_factory') def test_list_ssl_certs_returns_list(self, mock_client_factory): @@ -875,9 +885,13 @@ def test_slot_param_configured_with_id_part(self): from azure.cli.command_modules.appservice import _params source = inspect.getsource(_params.load_arguments) - # The webapp context should have slot configured with id_part='child_name_1' - self.assertIn("id_part='child_name_1'", source, - "slot argument must have id_part='child_name_1' for --ids slot parsing") + # Find the specific line that configures the slot argument with id_part + slot_lines = [line.strip() for line in source.splitlines() + if "argument('slot'" in line and "id_part=" in line] + self.assertTrue(len(slot_lines) > 0, + "Expected c.argument('slot', ..., id_part=...) in load_arguments") + self.assertTrue(any("child_name_1" in line for line in slot_lines), + "slot argument must have id_part='child_name_1' for --ids slot parsing") class FakedResponse: # pylint: disable=too-few-public-methods