diff --git a/src/aks-preview/HISTORY.rst b/src/aks-preview/HISTORY.rst index 39c93b3c34f..3ef5b6a14cb 100644 --- a/src/aks-preview/HISTORY.rst +++ b/src/aks-preview/HISTORY.rst @@ -11,6 +11,7 @@ To release a new version, please select a new version number (usually plus 1 to Pending +++++++ +* Fix `--localdns-config` parameter to handle null values in JSON configuration files gracefully, preventing crashes when DNS override sections are null. 18.0.0b40 +++++++ diff --git a/src/aks-preview/azext_aks_preview/_helpers.py b/src/aks-preview/azext_aks_preview/_helpers.py index 0e791b4aecf..9b3fdd3ed01 100644 --- a/src/aks-preview/azext_aks_preview/_helpers.py +++ b/src/aks-preview/azext_aks_preview/_helpers.py @@ -448,3 +448,19 @@ def get_extension_in_allow_list(result): if _check_if_extension_type_is_in_allow_list(result.extension_type.lower()): return result return None + + +def process_dns_overrides(overrides_dict, target_dict, build_override_func): + """Helper function to safely process DNS overrides with null checks. + + Processes DNS override dictionaries from LocalDNS configuration, + filtering out null values and applying the build function to valid entries. + + :param overrides_dict: Dictionary containing DNS overrides (can be None) + :param target_dict: Target dictionary to populate with processed overrides + :param build_override_func: Function to build override objects from dict values + """ + if overrides_dict is not None: + for key, value in overrides_dict.items(): + if value is not None: + target_dict[key] = build_override_func(value) diff --git a/src/aks-preview/azext_aks_preview/agentpool_decorator.py b/src/aks-preview/azext_aks_preview/agentpool_decorator.py index 6ec1fa1f3ea..2f627f13fac 100644 --- a/src/aks-preview/azext_aks_preview/agentpool_decorator.py +++ b/src/aks-preview/azext_aks_preview/agentpool_decorator.py @@ -50,6 +50,7 @@ from azext_aks_preview._helpers import ( get_nodepool_snapshot_by_snapshot_id, filter_hard_taints, + process_dns_overrides, ) logger = get_logger(__name__) @@ -1477,13 +1478,16 @@ def build_override(override_dict): return self.models.LocalDNSOverride(**filtered) # Build kubeDNSOverrides and vnetDNSOverrides from the localdns_profile - kube_overrides = localdns_profile.get("kubeDNSOverrides") - for key, value in kube_overrides.items(): - kube_dns_overrides[key] = build_override(value) - - vnet_overrides = localdns_profile.get("vnetDNSOverrides") - for key, value in vnet_overrides.items(): - vnet_dns_overrides[key] = build_override(value) + process_dns_overrides( + localdns_profile.get("kubeDNSOverrides"), + kube_dns_overrides, + build_override + ) + process_dns_overrides( + localdns_profile.get("vnetDNSOverrides"), + vnet_dns_overrides, + build_override + ) agentpool.local_dns_profile = self.models.LocalDNSProfile( mode=localdns_profile.get("mode"), @@ -1816,13 +1820,16 @@ def build_override(override_dict): return self.models.LocalDNSOverride(**filtered) # Build kubeDNSOverrides and vnetDNSOverrides from the localdns_profile - kube_overrides = localdns_profile.get("kubeDNSOverrides") - for key, value in kube_overrides.items(): - kube_dns_overrides[key] = build_override(value) - - vnet_overrides = localdns_profile.get("vnetDNSOverrides") - for key, value in vnet_overrides.items(): - vnet_dns_overrides[key] = build_override(value) + process_dns_overrides( + localdns_profile.get("kubeDNSOverrides"), + kube_dns_overrides, + build_override + ) + process_dns_overrides( + localdns_profile.get("vnetDNSOverrides"), + vnet_dns_overrides, + build_override + ) agentpool.local_dns_profile = self.models.LocalDNSProfile( mode=localdns_profile.get("mode"), diff --git a/src/aks-preview/azext_aks_preview/tests/latest/test_agentpool_decorator.py b/src/aks-preview/azext_aks_preview/tests/latest/test_agentpool_decorator.py index 08b4de4d0a7..9591c00d1b6 100644 --- a/src/aks-preview/azext_aks_preview/tests/latest/test_agentpool_decorator.py +++ b/src/aks-preview/azext_aks_preview/tests/latest/test_agentpool_decorator.py @@ -2684,6 +2684,151 @@ def common_update_blue_green_upgrade_settings(self): ) self.assertEqual(dec_agentpool_3, ground_truth_agentpool_3) + def common_update_localdns_profile(self): + import tempfile + import json + import os + + # Test case 1: LocalDNS config provided - verify method is called + localdns_config = { + "mode": "Required", + "kubeDNSOverrides": { + ".": { + "cacheDurationInSeconds": 3600, + "protocol": "PreferUDP" + } + } + } + + with tempfile.NamedTemporaryFile(mode="w+", delete=False, suffix=".json") as f: + json.dump(localdns_config, f) + f.flush() + config_file_path = f.name + + try: + dec_1 = AKSPreviewAgentPoolUpdateDecorator( + self.cmd, + self.client, + {"localdns_config": config_file_path}, + self.resource_type, + self.agentpool_decorator_mode, + ) + + agentpool_1 = self.create_initialized_agentpool_instance() + dec_1.context.attach_agentpool(agentpool_1) + dec_agentpool_1 = dec_1.update_localdns_profile(agentpool_1) + + # Verify that LocalDNS profile was created and assigned + self.assertIsNotNone(dec_agentpool_1.local_dns_profile) + self.assertEqual(dec_agentpool_1.local_dns_profile.mode, "Required") + + finally: + os.unlink(config_file_path) + + # Test case 2: No LocalDNS config provided - no change + dec_2 = AKSPreviewAgentPoolUpdateDecorator( + self.cmd, + self.client, + {}, + self.resource_type, + self.agentpool_decorator_mode, + ) + + agentpool_2 = self.create_initialized_agentpool_instance() + original_local_dns_profile = agentpool_2.local_dns_profile + dec_2.context.attach_agentpool(agentpool_2) + dec_agentpool_2 = dec_2.update_localdns_profile(agentpool_2) + + # Verify LocalDNS profile wasn't changed + self.assertEqual(dec_agentpool_2.local_dns_profile, original_local_dns_profile) + + # Test case 3: LocalDNS config with null values + localdns_config_with_nulls = { + "mode": "Required", + "kubeDNSOverrides": None, + "vnetDNSOverrides": { + ".": { + "cacheDurationInSeconds": 1800, + "protocol": "ForceTCP" + } + } + } + + with tempfile.NamedTemporaryFile(mode="w+", delete=False, suffix=".json") as f: + json.dump(localdns_config_with_nulls, f) + f.flush() + config_file_path = f.name + + try: + dec_3 = AKSPreviewAgentPoolUpdateDecorator( + self.cmd, + self.client, + {"localdns_config": config_file_path}, + self.resource_type, + self.agentpool_decorator_mode, + ) + + agentpool_3 = self.create_initialized_agentpool_instance() + dec_3.context.attach_agentpool(agentpool_3) + dec_agentpool_3 = dec_3.update_localdns_profile(agentpool_3) + + # Verify that LocalDNS profile was created with null handling + self.assertIsNotNone(dec_agentpool_3.local_dns_profile) + self.assertEqual(dec_agentpool_3.local_dns_profile.mode, "Required") + # kubeDNSOverrides should be empty dict due to null input + self.assertEqual(len(dec_agentpool_3.local_dns_profile.kube_dns_overrides), 0) + # vnetDNSOverrides should have one entry + self.assertEqual(len(dec_agentpool_3.local_dns_profile.vnet_dns_overrides), 1) + + finally: + os.unlink(config_file_path) + + def common_test_process_dns_overrides_helper(self): + from azext_aks_preview._helpers import process_dns_overrides + + # Test the process_dns_overrides utility function functionality + + # Test case 1: Valid DNS overrides without nulls + dns_overrides = { + ".": { + "cacheDurationInSeconds": 3600, + "protocol": "PreferUDP" + } + } + target_dict = {} + + def mock_build_override(override_dict): + return self.models.LocalDNSOverride( + cache_duration_in_seconds=override_dict.get("cacheDurationInSeconds"), + protocol=override_dict.get("protocol") + ) + + process_dns_overrides(dns_overrides, target_dict, mock_build_override) + self.assertEqual(len(target_dict), 1) + self.assertIn(".", target_dict) + + # Test case 2: DNS overrides with null values (should handle gracefully) + dns_overrides_with_nulls = { + ".": { + "cacheDurationInSeconds": 1800, + "protocol": None + } + } + target_dict_2 = {} + + process_dns_overrides(dns_overrides_with_nulls, target_dict_2, mock_build_override) + self.assertEqual(len(target_dict_2), 1) + + # Test case 3: None input (should handle gracefully) + target_dict_3 = {} + process_dns_overrides(None, target_dict_3, mock_build_override) + self.assertEqual(len(target_dict_3), 0) + + # Test case 4: Empty input (should handle gracefully) + target_dict_4 = {} + process_dns_overrides({}, target_dict_4, mock_build_override) + self.assertEqual(len(target_dict_4), 0) + class AKSPreviewAgentPoolUpdateDecoratorStandaloneModeTestCase( AKSPreviewAgentPoolUpdateDecoratorCommonTestCase @@ -2721,6 +2866,12 @@ def test_update_upgrade_strategy(self): def test_update_blue_green_upgrade_settings(self): self.common_update_blue_green_upgrade_settings() + def test_update_localdns_profile(self): + self.common_update_localdns_profile() + + def test_process_dns_overrides_helper(self): + self.common_test_process_dns_overrides_helper() + def test_update_agentpool_profile_preview(self): import inspect @@ -2808,6 +2959,12 @@ def test_update_upgrade_strategy(self): def test_update_blue_green_upgrade_settings(self): self.common_update_blue_green_upgrade_settings() + def test_update_localdns_profile(self): + self.common_update_localdns_profile() + + def test_process_dns_overrides_helper(self): + self.common_test_process_dns_overrides_helper() + def test_update_agentpool_profile_preview(self): import inspect