diff --git a/src/azure-cli/azure/cli/command_modules/appservice/commands.py b/src/azure-cli/azure/cli/command_modules/appservice/commands.py index e734d104ff1..8f567a740ba 100644 --- a/src/azure-cli/azure/cli/command_modules/appservice/commands.py +++ b/src/azure-cli/azure/cli/command_modules/appservice/commands.py @@ -40,6 +40,51 @@ def transform_web_list_output(webs): return [transform_web_output(w) for w in webs] +def transform_rename_server_farm_id(web): + """Post-serialization transformer: expose appServicePlanId in output. + + The command-level 'transform' hook runs *before* the pipeline calls todict(), + so the value received here is a raw model object, not yet a plain dict. We + therefore serialise it ourselves first using the same todict() the pipeline + would use, add appServicePlanId to the resulting dict, and return the dict so + that the pipeline's subsequent todict() call passes through it unchanged. + + serverFarmId is preserved in the output for backward compatibility with + existing scripts and tests. appServicePlanId is added alongside it as an + alias so that callers relying on the original field name continue to work. + + In newer azure-mgmt-web SDK versions (ARM-envelope layout) serverFarmId is + nested under 'properties'. In older flat-layout versions it appears at the + top level. Both cases are handled. + """ + from azure.cli.core.util import todict + from azure.cli.core.commands import AzCliCommandInvoker + + if not isinstance(web, dict): + web = todict(web, AzCliCommandInvoker.remove_additional_prop_layer) + + if not isinstance(web, dict): + return web + + # serverFarmId may be at the top level (older/flat SDK serialisation) or + # nested under 'properties' (newer ARM-envelope SDK serialisation). + # Use get (not pop) so that serverFarmId is preserved for backward compat. + server_farm_id = web.get('serverFarmId') + if server_farm_id is None: + properties = web.get('properties') + if isinstance(properties, dict): + server_farm_id = properties.get('serverFarmId') + + if server_farm_id is not None and 'appServicePlanId' not in web: + web['appServicePlanId'] = server_farm_id + + return web + + +def transform_rename_server_farm_id_list(webs): + return [transform_rename_server_farm_id(w) for w in webs] + + def transform_runtime_list_output(result): from collections import OrderedDict return [OrderedDict([ @@ -220,8 +265,10 @@ def load_command_table(self, _): deprecate_info=g.deprecate(redirect='webapp create and webapp deploy')) g.custom_command('ssh', 'ssh_webapp', exception_handler=ex_handler_factory(), is_preview=True) g.custom_command('exec', 'webapp_exec', custom_command_type=webapp_exec_custom, exception_handler=ex_handler_factory(), is_preview=True) - g.custom_command('list', 'list_webapp', table_transformer=transform_web_list_output) - g.custom_show_command('show', 'show_app', table_transformer=transform_web_output) + g.custom_command('list', 'list_webapp', transform=transform_rename_server_farm_id_list, + table_transformer=transform_web_list_output) + g.custom_show_command('show', 'show_app', transform=transform_rename_server_farm_id, + table_transformer=transform_web_output) g.custom_command('delete', 'delete_webapp') g.custom_command('stop', 'stop_webapp') g.custom_command('start', 'start_webapp') @@ -462,8 +509,10 @@ def load_command_table(self, _): validator=validate_functionapp) g.custom_command('list-runtimes', 'list_function_app_runtimes') g.custom_command('list-flexconsumption-runtimes', 'list_flex_function_app_runtimes') - g.custom_command('list', 'list_function_app', table_transformer=transform_web_list_output) - g.custom_show_command('show', 'show_functionapp', table_transformer=transform_web_output) + g.custom_command('list', 'list_function_app', transform=transform_rename_server_farm_id_list, + table_transformer=transform_web_list_output) + g.custom_show_command('show', 'show_functionapp', transform=transform_rename_server_farm_id, + table_transformer=transform_web_output) g.custom_command('delete', 'delete_function_app') g.custom_command('stop', 'stop_webapp') g.custom_command('start', 'start_webapp') @@ -686,8 +735,10 @@ def load_command_table(self, _): with self.command_group('logicapp', custom_command_type=logicapp_custom) as g: g.custom_command('create', 'create_logicapp', exception_handler=ex_handler_factory()) - g.custom_command('list', 'list_logicapp', table_transformer=transform_web_list_output) - g.custom_show_command('show', 'show_logicapp', table_transformer=transform_web_output) + g.custom_command('list', 'list_logicapp', transform=transform_rename_server_farm_id_list, + table_transformer=transform_web_list_output) + g.custom_show_command('show', 'show_logicapp', transform=transform_rename_server_farm_id, + table_transformer=transform_web_output) g.custom_command('scale', 'scale_logicapp', exception_handler=ex_handler_factory()) with self.command_group('logicapp config appsettings', custom_command_type=logicapp_custom) as g: 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 697496f307c..00902217530 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 @@ -42,6 +42,9 @@ show_startup_log, troubleshoot_status, create_webapp) +from azure.cli.command_modules.appservice.utils import _rename_server_farm_props, get_site_server_farm_id +from azure.cli.command_modules.appservice.commands import (transform_rename_server_farm_id, + transform_rename_server_farm_id_list) # pylint: disable=line-too-long from azure.cli.core.profiles import ResourceType @@ -63,6 +66,85 @@ class TestWebappMocked(unittest.TestCase): def setUp(self): self.client = WebSiteManagementClient(mock.MagicMock(), '123455678') + def test_transform_rename_server_farm_id_renames_key(self): + # Verifies the post-serialisation transformer adds appServicePlanId alongside serverFarmId + farm_id = '/subscriptions/sub/resourceGroups/rg/providers/Microsoft.Web/serverfarms/plan' + web = { + 'location': 'westus', + 'serverFarmId': farm_id, + } + + result = transform_rename_server_farm_id(web) + + self.assertEqual(result['appServicePlanId'], farm_id) + # serverFarmId is preserved for backward compatibility + self.assertEqual(result['serverFarmId'], farm_id) + + def test_transform_rename_server_farm_id_nested_under_properties(self): + # New SDK ARM-envelope layout: serverFarmId is under 'properties', not at the top level + farm_id = '/subscriptions/sub/resourceGroups/rg/providers/Microsoft.Web/serverfarms/plan' + web = { + 'location': 'westus', + 'properties': { + 'serverFarmId': farm_id, + 'name': 'myapp', + }, + } + + result = transform_rename_server_farm_id(web) + + self.assertEqual(result['appServicePlanId'], farm_id) + # serverFarmId is preserved in properties for backward compatibility + self.assertEqual(result['properties']['serverFarmId'], farm_id) + + def test_transform_rename_server_farm_id_model_object(self): + # When the transformer receives a raw model object (before todict), it must + # serialise the object first and then add appServicePlanId. + farm_id = '/subscriptions/sub/resourceGroups/rg/providers/Microsoft.Web/serverfarms/plan' + # Simulate a model object with __dict__ that todict() will expand. + # Note: attribute names with underscores are converted to camelCase by todict(), + # so set them as camelCase to match what todict() would produce. + web_obj = types.SimpleNamespace(location='westus') + web_obj.__dict__['serverFarmId'] = farm_id + + with mock.patch('azure.cli.core.util.todict', return_value={'location': 'westus', 'serverFarmId': farm_id}): + result = transform_rename_server_farm_id(web_obj) + + self.assertEqual(result['appServicePlanId'], farm_id) + # serverFarmId is preserved for backward compatibility + self.assertEqual(result['serverFarmId'], farm_id) + + def test_transform_rename_server_farm_id_noop_when_already_renamed(self): + farm_id = '/subscriptions/sub/resourceGroups/rg/providers/Microsoft.Web/serverfarms/plan' + web = {'appServicePlanId': farm_id} + + result = transform_rename_server_farm_id(web) + + self.assertEqual(result['appServicePlanId'], farm_id) + self.assertNotIn('serverFarmId', result) + + def test_transform_rename_server_farm_id_list(self): + farm_id = '/subscriptions/sub/resourceGroups/rg/providers/Microsoft.Web/serverfarms/plan' + webs = [ + {'serverFarmId': farm_id, 'name': 'app1'}, + {'appServicePlanId': farm_id, 'name': 'app2'}, + ] + + results = transform_rename_server_farm_id_list(webs) + + for r in results: + self.assertEqual(r['appServicePlanId'], farm_id) + + def test_rename_server_farm_props_handles_object_attributes(self): + site = types.SimpleNamespace( + server_farm_id='/subscriptions/sub/resourceGroups/rg/providers/Microsoft.Web/serverfarms/plan') + + _rename_server_farm_props(site) + + self.assertEqual(site.app_service_plan_id, + '/subscriptions/sub/resourceGroups/rg/providers/Microsoft.Web/serverfarms/plan') + self.assertFalse(hasattr(site, 'server_farm_id')) + @mock.patch('azure.cli.command_modules.appservice.custom._update_site_source_control_properties_for_gh_action') @mock.patch('azure.cli.command_modules.appservice.custom._add_publish_profile_to_github') @mock.patch('azure.cli.command_modules.appservice.custom.prompt_y_n') @@ -385,8 +467,6 @@ def test_restore_snapshot(self, generic_site_op_mock, client_factory_mock): generic_site_op_mock.return_value = site client_factory_mock.return_value = client - - SnapshotRecoverySource, SnapshotRestoreRequest = \ cmd_mock.get_models('SnapshotRecoverySource', 'SnapshotRestoreRequest') @@ -507,14 +587,13 @@ 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) - def test_update_app_settings_error_handling_no_parameters(self): """Test that MutuallyExclusiveArgumentError is raised when neither settings nor slot_settings are provided.""" cmd_mock = _get_test_cmd() - + # Test missing both parameters - should fail early without calling any services - with self.assertRaisesRegex(MutuallyExclusiveArgumentError, - "Please provide either --settings or --slot-settings parameter"): + with self.assertRaisesRegex(MutuallyExclusiveArgumentError, + "Please provide either --settings or --slot-settings parameter"): update_app_settings(cmd_mock, 'test-rg', 'test-app') @mock.patch('azure.cli.command_modules.appservice.custom._generic_site_operation') @@ -522,19 +601,19 @@ def test_update_app_settings_error_handling_no_parameters(self): def test_update_app_settings_error_handling_invalid_format(self, mock_json_parse, mock_site_op): """Test that InvalidArgumentValueError is raised for invalid setting formats.""" cmd_mock = _get_test_cmd() - + # Setup minimal mocks needed to reach the error handling code mock_app_settings = mock.MagicMock() mock_app_settings.properties = {} mock_site_op.return_value = mock_app_settings - + # Mock shell_safe_json_parse to raise InvalidArgumentValueError (simulating invalid JSON) mock_json_parse.side_effect = InvalidArgumentValueError("Invalid JSON format") - + # Test invalid format that can't be parsed as JSON or key=value invalid_setting = "invalid_format_no_equals_no_json" expected_message = r"Invalid setting format.*Expected 'key=value' format or valid JSON" - + with self.assertRaisesRegex(InvalidArgumentValueError, expected_message): update_app_settings(cmd_mock, 'test-rg', 'test-app', settings=[invalid_setting]) @@ -543,19 +622,19 @@ def test_update_app_settings_error_handling_invalid_format(self, mock_json_parse def test_update_app_settings_error_handling_invalid_format_no_equals(self, mock_json_parse, mock_site_op): """Test ValueError path when shell_safe_json_parse raises InvalidArgumentValueError and string contains no '='.""" cmd_mock = _get_test_cmd() - + # Setup minimal mocks needed to reach the error handling code mock_app_settings = mock.MagicMock() mock_app_settings.properties = {} mock_site_op.return_value = mock_app_settings - + # Mock shell_safe_json_parse to raise InvalidArgumentValueError mock_json_parse.side_effect = InvalidArgumentValueError("Invalid JSON format") - + # Test invalid format with no equals sign - this should trigger ValueError in split('=', 1) invalid_setting_no_equals = "invalidformatthatcontainsnoequalsign" expected_message = r"Invalid setting format.*Expected 'key=value' format or valid JSON" - + with self.assertRaisesRegex(InvalidArgumentValueError, expected_message): update_app_settings(cmd_mock, 'test-rg', 'test-app', settings=[invalid_setting_no_equals]) @@ -564,26 +643,26 @@ def test_update_app_settings_error_handling_invalid_format_no_equals(self, mock_ @mock.patch('azure.cli.command_modules.appservice.custom.is_centauri_functionapp') @mock.patch('azure.cli.command_modules.appservice.custom._generic_settings_operation') @mock.patch('azure.cli.command_modules.appservice.custom._build_app_settings_output') - def test_update_app_settings_success_key_value_format(self, mock_build, mock_settings_op, mock_centauri, - mock_client_factory, mock_site_op): + def test_update_app_settings_success_key_value_format(self, mock_build, mock_settings_op, mock_centauri, + mock_client_factory, mock_site_op): """Test successful processing of key=value format settings.""" cmd_mock = _get_test_cmd() - + # Setup mocks mock_app_settings = mock.MagicMock() mock_app_settings.properties = {} mock_site_op.return_value = mock_app_settings - + mock_client = mock.MagicMock() mock_client_factory.return_value = mock_client mock_centauri.return_value = False mock_settings_op.return_value = mock_app_settings mock_build.return_value = {"KEY1": "value1", "KEY2": "value2"} - + # Test valid key=value format - result = update_app_settings(cmd_mock, 'test-rg', 'test-app', - settings=['KEY1=value1', 'KEY2=value2']) - + result = update_app_settings(cmd_mock, 'test-rg', 'test-app', + settings=['KEY1=value1', 'KEY2=value2']) + # Verify the function completed successfully self.assertEqual(result["KEY1"], "value1") self.assertEqual(result["KEY2"], "value2") @@ -593,20 +672,20 @@ def test_update_app_settings_success_key_value_format(self, mock_build, mock_set def test_update_application_settings_polling_error_handling(self, mock_send_request): """Test that AzureResponseError is raised in polling function when appropriate.""" cmd_mock = _get_test_cmd() - + # Mock an exception that doesn't have the expected structure class MockException(Exception): def __init__(self): self.response = mock.MagicMock() self.response.status_code = 400 # Not 202 self.response.headers = {} - + # Mock _generic_settings_operation to raise the exception with mock.patch('azure.cli.command_modules.appservice.custom._generic_settings_operation') as mock_settings_op, \ self.assertRaisesRegex(AzureResponseError, "Failed to update application settings"): mock_settings_op.side_effect = MockException() - update_application_settings_polling(cmd_mock, 'test-rg', 'test-app', - mock.MagicMock(), None, mock.MagicMock()) + update_application_settings_polling(cmd_mock, 'test-rg', 'test-app', + mock.MagicMock(), None, mock.MagicMock()) @mock.patch('azure.cli.command_modules.appservice.custom._generic_site_operation') @mock.patch('azure.cli.command_modules.appservice.custom.web_client_factory') @@ -614,15 +693,15 @@ def __init__(self): @mock.patch('azure.cli.command_modules.appservice.custom._generic_settings_operation') @mock.patch('azure.cli.command_modules.appservice.custom._build_app_settings_output') def test_update_app_settings_success_with_slot_settings(self, mock_build, mock_settings_op, mock_centauri, - mock_client_factory, mock_site_op): + mock_client_factory, mock_site_op): """Test successful processing with slot settings.""" cmd_mock = _get_test_cmd() - + # Setup mocks mock_app_settings = mock.MagicMock() mock_app_settings.properties = {} mock_site_op.return_value = mock_app_settings - + mock_client = mock.MagicMock() mock_slot_config = mock.MagicMock() mock_slot_config.app_setting_names = [] @@ -631,12 +710,12 @@ def test_update_app_settings_success_with_slot_settings(self, mock_build, mock_s mock_centauri.return_value = False mock_settings_op.return_value = mock_app_settings mock_build.return_value = {"SLOT_KEY": "slot_value"} - + # Test with slot settings - result = update_app_settings(cmd_mock, 'test-rg', 'test-app', - settings=['REGULAR_KEY=regular_value'], - slot_settings=['SLOT_KEY=slot_value']) - + update_app_settings(cmd_mock, 'test-rg', 'test-app', + settings=['REGULAR_KEY=regular_value'], + slot_settings=['SLOT_KEY=slot_value']) + # Verify slot configuration was updated mock_client.web_apps.list_slot_configuration_names.assert_called_once() mock_client.web_apps.update_slot_configuration_names.assert_called_once() @@ -2166,4 +2245,4 @@ def test_get_java_runtimes_from_container_settings_reads_mapping(self): if __name__ == '__main__': - unittest.main() \ No newline at end of file + unittest.main() diff --git a/src/azure-cli/azure/cli/command_modules/appservice/utils.py b/src/azure-cli/azure/cli/command_modules/appservice/utils.py index 1d6e10431bc..1ed29df396c 100644 --- a/src/azure-cli/azure/cli/command_modules/appservice/utils.py +++ b/src/azure-cli/azure/cli/command_modules/appservice/utils.py @@ -8,6 +8,7 @@ import urllib import urllib3 import certifi +from collections.abc import MutableMapping from datetime import datetime from knack.log import get_logger @@ -53,13 +54,21 @@ def get_site_server_farm_id(site): In azure-mgmt-web 11.0.0+, models use hybrid dict/model nature with camelCase keys. This helper provides backward compatibility. """ - # Try new SDK dictionary-style access with camelCase - try: - return site["serverFarmId"] - except (KeyError, TypeError): - pass + # Try new SDK dictionary-style access with camelCase. The generated Site + # model stores flattened properties under "properties". + # For hybrid SDK objects, site.properties is an attribute; for plain dicts, fall back + # to the "properties" key so that unit-test fixtures (plain dicts) also work. + properties_source = getattr(site, 'properties', None) + if properties_source is None and isinstance(site, MutableMapping): + properties_source = site.get('properties') + for source in (site, properties_source): + if not isinstance(source, MutableMapping): + continue + for property_name in ('serverFarmId', 'appServicePlanId'): + if property_name in source: + return source[property_name] # Fall back to old SDK attribute access - return getattr(site, 'server_farm_id', None) + return getattr(site, 'server_farm_id', getattr(site, 'app_service_plan_id', None)) def str2bool(v): @@ -247,15 +256,20 @@ def _list_app(cli_ctx, resource_group_name=None): def _rename_server_farm_props(webapp): - # Should be renamed in SDK in a future release - server_farm_id = get_site_server_farm_id(webapp) - setattr(webapp, 'app_service_plan_id', server_farm_id) - # Remove server_farm_id if it exists as an attribute (for old SDK compatibility) - if hasattr(webapp, 'server_farm_id'): - try: - del webapp.server_farm_id - except (AttributeError, TypeError): - pass + # Newer SDK models (MutableMapping-based) are handled by the command-level + # transform (transform_rename_server_farm_id / _list) which operates on the + # already-serialised dict. Here we only need to cover the legacy attribute-style + # objects so that downstream code (e.g. get_site_server_farm_id) can still read + # app_service_plan_id from older SDK models. + if not isinstance(webapp, MutableMapping): + server_farm_id = get_site_server_farm_id(webapp) + setattr(webapp, 'app_service_plan_id', server_farm_id) + # Remove server_farm_id attribute for old SDK compatibility + if hasattr(webapp, 'server_farm_id'): + try: + del webapp.server_farm_id + except (AttributeError, TypeError): + pass return webapp