Skip to content

Commit ba0a97c

Browse files
saewonijuanbejdbencardinop
authored
Add LocalDNS Live Tests for valid and invalid scenarios (#9252)
* skip none overrides on localdns profile * update history rst * refactor to process dns overrides func * move overrides function to helper file * apply linter suggestions * add localdnsconfig folder * add more tests * add new json files * add default dns overrides * add more test cases * move tests around, move invalid cases to another file * add back import semver * reorder the existing tests * delete preferred mode only * delete null.json * remove redundant json file * remove redundant json file * fix the mistake at line 3349 * forgot that i put all the configs in data/localconfig folder * remove unused file * spelling error * fix default dnsOverrides check when we create agentpool with required mode only * restore localdnsconfig file * delete extra property case from invalid test file * add extra property cases in src/aks-preview/azext_aks_preview/tests/latest/test_aks_commands.py * add extra property files * check for defaulted *dnsOverrides when making agent pool with mode: required only * comment out cleanup in test_aks_nodepool_add_with_localdns_required_mode * add more logging for debugging * add print statements in src/aks-preview/azext_aks_preview/vendored_sdks/azure_mgmt_preview_aks/operations/_agent_pools_operations.py for debugging * only initialize the dictionaries if dnsoverrides are provided * process dns overrides only when dns overrides are provided * consolidate duplicated build_localdns_profile function * move invalid cases to line 4133 * update test_aks_commands.py * look for vnetDnsOverrides and kubeDNSOverrides keys, case-insensitive * fix test_aks_nodepool_add_with_localdns_required_mode_single_vnetdns * check for dictionary for build_override * update failing test cases * rename from required_mode_extra_property.json -> required_mode_kubedns_extra_property.json * fix azdev style * temporarily add self.fail statements s.t. i can see the error_message * change from assertTrue to assertIn with more specific error msg, delete un-needed test case * change from print to debug * remove logger.debug line to print localdnsprofile * add null config file * fix the tests * fix the tests * update src/aks-preview/HISTORY.rst with a new note under 18.0.0b42 * update src/aks-preview/HISTORY.rst * Revert "add print statements in src/aks-preview/azext_aks_preview/vendored_sdks/azure_mgmt_preview_aks/operations/_agent_pools_operations.py for debugging" This reverts commit b747438. * update src/aks-preview/HISTORY.rst * Revert "add more logging for debugging" This reverts commit 1786254. * mix the casing for *dnsoverrides * throw an exception from cli if the values of kubednsoverrides or vnetdnsoverrides are not type dict * add tests for null and non-dict overrides * make the keys of localdnsprofile mixed-case * add required_mode_null_dnsOverrides.json and required_mode_number_dnsOverrides.json * correct the error message I'm looking for, for non-dict dns overrides * remove print stmt from src/aks-preview/azext_aks_preview/_helpers.py * add check for DNS override settings * update the test with dns override settings check * update assertIn msg for test_aks_nodepool_add_with_localdns_required_mode_partial_invalid * break down InvalidArgumentValueError msg into two lines * update existing cassette files * new cassette files for new tests * add three additional cassette files I did not commit before * expect InvalidArgumentValueError when None is provided for DNS overrides * update AKSPreviewAgentPoolUpdateDecoratorCommonTestCase.common_update_localdns_profile * revert the import statement for from azure.cli.command_modules.acs.tests.latest.mocks import * Add a new line * Revert "Add a new line" This reverts commit 2c347c3. * update version in src/aks-preview/setup.py to align with azure-cli-extensions/src/aks-preview/HISTORY.rst --------- Co-authored-by: juanbe <[email protected]> Co-authored-by: Juan Diego Bencardino <[email protected]>
1 parent 73eb087 commit ba0a97c

File tree

57 files changed

+48605
-1021
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

57 files changed

+48605
-1021
lines changed

‎src/aks-preview/HISTORY.rst‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,12 @@ To release a new version, please select a new version number (usually plus 1 to
1212
Pending
1313
+++++++
1414

15+
18.0.0b43
16+
+++++++
17+
* Fix `--localdns-config` parameter to handle null values and case-insensitive JSON keys in DNS override sections, preventing crashes with malformed localdns configuration files.
18+
* Enhance `build_override` function to validate dictionary types and only initialize DNS overrides when present in localdns configuration (case-insensitive).
19+
* Refactor `build_localdns_profile` function to eliminate code duplication between AgentPool add and update operations.
20+
1521
18.0.0b42
1622
+++++++
1723
* Fix role assignment failure when using azure-cli version >= `2.77.0`.

‎src/aks-preview/azext_aks_preview/_helpers.py‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -460,6 +460,10 @@ def process_dns_overrides(overrides_dict, target_dict, build_override_func):
460460
:param target_dict: Target dictionary to populate with processed overrides
461461
:param build_override_func: Function to build override objects from dict values
462462
"""
463+
if not isinstance(overrides_dict, dict):
464+
raise InvalidArgumentValueError(
465+
f"Expected a dictionary for DNS overrides, but got {type(overrides_dict).__name__}: {overrides_dict}"
466+
)
463467
if overrides_dict is not None:
464468
for key, value in overrides_dict.items():
465469
if value is not None:

‎src/aks-preview/azext_aks_preview/agentpool_decorator.py‎

Lines changed: 74 additions & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -940,6 +940,78 @@ def get_localdns_profile(self):
940940
return profile
941941
return None
942942

943+
def build_localdns_profile(self, agentpool: AgentPool) -> AgentPool:
944+
"""Build local DNS profile for the AgentPool object if provided via --localdns-config."""
945+
localdns_profile = self.get_localdns_profile()
946+
kube_dns_overrides, vnet_dns_overrides = None, None
947+
948+
if localdns_profile is not None:
949+
def find_keys_case_insensitive(dictionary, target_keys):
950+
"""Find multiple keys case-insensitively and return a dict mapping target_key -> actual_key"""
951+
result = {}
952+
lowered_keys = {key.lower(): key for key in dictionary.keys()}
953+
for target_key in target_keys:
954+
lowered_target = target_key.lower()
955+
if lowered_target in lowered_keys:
956+
result[target_key] = lowered_keys[lowered_target]
957+
else:
958+
result[target_key] = None
959+
return result
960+
961+
def build_override(override_dict):
962+
if not isinstance(override_dict, dict):
963+
raise InvalidArgumentValueError(
964+
f"Expected a dictionary for DNS override settings,"
965+
f" but got {type(override_dict).__name__}: {override_dict}"
966+
)
967+
camel_to_snake_case = {
968+
"queryLogging": "query_logging",
969+
"protocol": "protocol",
970+
"forwardDestination": "forward_destination",
971+
"forwardPolicy": "forward_policy",
972+
"maxConcurrent": "max_concurrent",
973+
"cacheDurationInSeconds": "cache_duration_in_seconds",
974+
"serveStaleDurationInSeconds": "serve_stale_duration_in_seconds",
975+
"serveStale": "serve_stale",
976+
}
977+
valid_keys = set(camel_to_snake_case.values())
978+
filtered = {}
979+
for k, v in override_dict.items():
980+
if k in camel_to_snake_case:
981+
filtered[camel_to_snake_case[k]] = v
982+
elif k in valid_keys:
983+
filtered[k] = v
984+
return self.models.LocalDNSOverride(**filtered)
985+
986+
# Build kubeDNSOverrides and vnetDNSOverrides from the localdns_profile
987+
key_mappings = find_keys_case_insensitive(localdns_profile, ["kubeDNSOverrides", "vnetDNSOverrides"])
988+
actual_kube_key = key_mappings["kubeDNSOverrides"]
989+
if actual_kube_key:
990+
logger.debug("Found kubeDNSOverrides key as: %s", actual_kube_key)
991+
kube_dns_overrides = {}
992+
process_dns_overrides(
993+
localdns_profile.get(actual_kube_key),
994+
kube_dns_overrides,
995+
build_override
996+
)
997+
998+
actual_vnet_key = key_mappings["vnetDNSOverrides"]
999+
if actual_vnet_key:
1000+
logger.debug("Found vnetDNSOverrides key as: %s", actual_vnet_key)
1001+
vnet_dns_overrides = {}
1002+
process_dns_overrides(
1003+
localdns_profile.get(actual_vnet_key),
1004+
vnet_dns_overrides,
1005+
build_override
1006+
)
1007+
1008+
agentpool.local_dns_profile = self.models.LocalDNSProfile(
1009+
mode=localdns_profile.get("mode"),
1010+
kube_dns_overrides=kube_dns_overrides,
1011+
vnet_dns_overrides=vnet_dns_overrides,
1012+
)
1013+
return agentpool
1014+
9431015
def get_node_count_and_enable_cluster_autoscaler_min_max_count_vms(
9441016
self,
9451017
) -> Tuple[int, bool, Union[int, None], Union[int, None]]:
@@ -1452,49 +1524,7 @@ def set_up_managed_system_mode(self, agentpool: AgentPool) -> AgentPool:
14521524
def set_up_localdns_profile(self, agentpool: AgentPool) -> AgentPool:
14531525
"""Set up local DNS profile for the AgentPool object if provided via --localdns-config."""
14541526
self._ensure_agentpool(agentpool)
1455-
localdns_profile = self.context.get_localdns_profile()
1456-
if localdns_profile is not None:
1457-
kube_dns_overrides = {}
1458-
vnet_dns_overrides = {}
1459-
1460-
def build_override(override_dict):
1461-
camel_to_snake_case = {
1462-
"queryLogging": "query_logging",
1463-
"protocol": "protocol",
1464-
"forwardDestination": "forward_destination",
1465-
"forwardPolicy": "forward_policy",
1466-
"maxConcurrent": "max_concurrent",
1467-
"cacheDurationInSeconds": "cache_duration_in_seconds",
1468-
"serveStaleDurationInSeconds": "serve_stale_duration_in_seconds",
1469-
"serveStale": "serve_stale",
1470-
}
1471-
valid_keys = set(camel_to_snake_case.values())
1472-
filtered = {}
1473-
for k, v in override_dict.items():
1474-
if k in camel_to_snake_case:
1475-
filtered[camel_to_snake_case[k]] = v
1476-
elif k in valid_keys:
1477-
filtered[k] = v
1478-
return self.models.LocalDNSOverride(**filtered)
1479-
1480-
# Build kubeDNSOverrides and vnetDNSOverrides from the localdns_profile
1481-
process_dns_overrides(
1482-
localdns_profile.get("kubeDNSOverrides"),
1483-
kube_dns_overrides,
1484-
build_override
1485-
)
1486-
process_dns_overrides(
1487-
localdns_profile.get("vnetDNSOverrides"),
1488-
vnet_dns_overrides,
1489-
build_override
1490-
)
1491-
1492-
agentpool.local_dns_profile = self.models.LocalDNSProfile(
1493-
mode=localdns_profile.get("mode"),
1494-
kube_dns_overrides=kube_dns_overrides,
1495-
vnet_dns_overrides=vnet_dns_overrides,
1496-
)
1497-
return agentpool
1527+
return self.context.build_localdns_profile(agentpool)
14981528

14991529
def construct_agentpool_profile_preview(self) -> AgentPool:
15001530
"""The overall controller used to construct the preview AgentPool profile.
@@ -1794,49 +1824,7 @@ def update_fips_image(self, agentpool: AgentPool) -> AgentPool:
17941824
def update_localdns_profile(self, agentpool: AgentPool) -> AgentPool:
17951825
"""Update local DNS profile for the AgentPool object if provided via --localdns-config."""
17961826
self._ensure_agentpool(agentpool)
1797-
localdns_profile = self.context.get_localdns_profile()
1798-
if localdns_profile is not None:
1799-
kube_dns_overrides = {}
1800-
vnet_dns_overrides = {}
1801-
1802-
def build_override(override_dict):
1803-
camel_to_snake_case = {
1804-
"queryLogging": "query_logging",
1805-
"protocol": "protocol",
1806-
"forwardDestination": "forward_destination",
1807-
"forwardPolicy": "forward_policy",
1808-
"maxConcurrent": "max_concurrent",
1809-
"cacheDurationInSeconds": "cache_duration_in_seconds",
1810-
"serveStaleDurationInSeconds": "serve_stale_duration_in_seconds",
1811-
"serveStale": "serve_stale",
1812-
}
1813-
valid_keys = set(camel_to_snake_case.values())
1814-
filtered = {}
1815-
for k, v in override_dict.items():
1816-
if k in camel_to_snake_case:
1817-
filtered[camel_to_snake_case[k]] = v
1818-
elif k in valid_keys:
1819-
filtered[k] = v
1820-
return self.models.LocalDNSOverride(**filtered)
1821-
1822-
# Build kubeDNSOverrides and vnetDNSOverrides from the localdns_profile
1823-
process_dns_overrides(
1824-
localdns_profile.get("kubeDNSOverrides"),
1825-
kube_dns_overrides,
1826-
build_override
1827-
)
1828-
process_dns_overrides(
1829-
localdns_profile.get("vnetDNSOverrides"),
1830-
vnet_dns_overrides,
1831-
build_override
1832-
)
1833-
1834-
agentpool.local_dns_profile = self.models.LocalDNSProfile(
1835-
mode=localdns_profile.get("mode"),
1836-
kube_dns_overrides=kube_dns_overrides,
1837-
vnet_dns_overrides=vnet_dns_overrides,
1838-
)
1839-
return agentpool
1827+
return self.context.build_localdns_profile(agentpool)
18401828

18411829
def update_upgrade_strategy(self, agentpool: AgentPool) -> AgentPool:
18421830
"""Update upgrade strategy for the AgentPool object.

‎src/aks-preview/azext_aks_preview/tests/latest/data/localdnsconfig/disabled_mode_only.json‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
{
2+
"mode": "Disabled"
3+
}

‎src/aks-preview/azext_aks_preview/tests/latest/data/localdnsconfig/empty.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
{}

‎src/aks-preview/azext_aks_preview/tests/latest/data/localdnsconfig/empty_mode.json‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
{
2+
"mode": ""
3+
}

‎src/aks-preview/azext_aks_preview/tests/latest/data/localdnsconfig/invalid_mode.json‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
{
2+
"mode": "InvalidMode"
3+
}

‎src/aks-preview/azext_aks_preview/tests/latest/data/localdnsconfig.json‎ renamed to ‎src/aks-preview/azext_aks_preview/tests/latest/data/localdnsconfig/localdnsconfig.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"mode": "Required",
3-
"kubeDNSOverrides": {
3+
"kubednsoverrides": {
44
".": {
55
"cacheDurationInSeconds": 3600,
66
"forwardDestination": "ClusterCoreDNS",
@@ -22,7 +22,7 @@
2222
"serveStaleDurationInSeconds": 3600
2323
}
2424
},
25-
"vnetDNSOverrides": {
25+
"VNETDNSOVERRIDES": {
2626
".": {
2727
"cacheDurationInSeconds": 3600,
2828
"forwardDestination": "VnetDNS",

‎src/aks-preview/azext_aks_preview/tests/latest/data/localdnsconfig/localdnsconfig_with_extra_property.json‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
{
2+
"mode": "Required",
3+
"extraProperty": "unexpected",
4+
"kubeDNSOverrides": {
5+
".": {
6+
"cacheDurationInSeconds": 3600,
7+
"forwardDestination": "ClusterCoreDNS",
8+
"forwardPolicy": "Sequential",
9+
"maxConcurrent": 1000,
10+
"protocol": "PreferUDP",
11+
"queryLogging": "Error",
12+
"serveStale": "Verify",
13+
"serveStaleDurationInSeconds": 3600
14+
},
15+
"cluster.local": {
16+
"cacheDurationInSeconds": 3600,
17+
"forwardDestination": "ClusterCoreDNS",
18+
"forwardPolicy": "Sequential",
19+
"maxConcurrent": 1000,
20+
"protocol": "ForceTCP",
21+
"queryLogging": "Error",
22+
"serveStale": "Immediate",
23+
"serveStaleDurationInSeconds": 3600
24+
}
25+
},
26+
"vnetDNSOverrides": {
27+
".": {
28+
"cacheDurationInSeconds": 3600,
29+
"forwardDestination": "VnetDNS",
30+
"forwardPolicy": "Sequential",
31+
"maxConcurrent": 1000,
32+
"protocol": "PreferUDP",
33+
"queryLogging": "Error",
34+
"serveStale": "Verify",
35+
"serveStaleDurationInSeconds": 3600
36+
},
37+
"cluster.local": {
38+
"cacheDurationInSeconds": 3600,
39+
"forwardDestination": "ClusterCoreDNS",
40+
"forwardPolicy": "Sequential",
41+
"maxConcurrent": 1000,
42+
"protocol": "ForceTCP",
43+
"queryLogging": "Error",
44+
"serveStale": "Immediate",
45+
"serveStaleDurationInSeconds": 3600
46+
}
47+
}
48+
}

‎src/aks-preview/azext_aks_preview/tests/latest/data/localdnsconfig/localdnsconfig_with_extra_property_in_dnsOverrides.json‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
{
2+
"mode": "Required",
3+
"extraProperty": "unexpected",
4+
"kubeDNSOverrides": {
5+
".": {
6+
"extraProperty": "unexpected",
7+
"cacheDurationInSeconds": 3600,
8+
"forwardDestination": "ClusterCoreDNS",
9+
"forwardPolicy": "Sequential",
10+
"maxConcurrent": 1000,
11+
"protocol": "PreferUDP",
12+
"queryLogging": "Error",
13+
"serveStale": "Verify",
14+
"serveStaleDurationInSeconds": 3600
15+
},
16+
"cluster.local": {
17+
"extraProperty": "unexpected",
18+
"cacheDurationInSeconds": 3600,
19+
"forwardDestination": "ClusterCoreDNS",
20+
"forwardPolicy": "Sequential",
21+
"maxConcurrent": 1000,
22+
"protocol": "ForceTCP",
23+
"queryLogging": "Error",
24+
"serveStale": "Immediate",
25+
"serveStaleDurationInSeconds": 3600
26+
}
27+
},
28+
"vnetDNSOverrides": {
29+
".": {
30+
"extraProperty": "unexpected",
31+
"cacheDurationInSeconds": 3600,
32+
"forwardDestination": "VnetDNS",
33+
"forwardPolicy": "Sequential",
34+
"maxConcurrent": 1000,
35+
"protocol": "PreferUDP",
36+
"queryLogging": "Error",
37+
"serveStale": "Verify",
38+
"serveStaleDurationInSeconds": 3600
39+
},
40+
"cluster.local": {
41+
"extraProperty": "unexpected",
42+
"cacheDurationInSeconds": 3600,
43+
"forwardDestination": "ClusterCoreDNS",
44+
"forwardPolicy": "Sequential",
45+
"maxConcurrent": 1000,
46+
"protocol": "ForceTCP",
47+
"queryLogging": "Error",
48+
"serveStale": "Immediate",
49+
"serveStaleDurationInSeconds": 3600
50+
}
51+
}
52+
}

0 commit comments

Comments
 (0)