Skip to content

[AKS] Added support for Private DNS Zone Contributor role assignment in app routing #7284

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Xing Zhou (zhoxing-ms) merged 12 commits into from
Feb 20, 2024
Merged

[AKS] Added support for Private DNS Zone Contributor role assignment in app routing #7284

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
71f366f
moved app routing test under correct method
bosesuneha Feb 12, 2024
dd5eb62
added support for private dns zone
bosesuneha Feb 12, 2024
d5c087e
fixed lint issues
bosesuneha Feb 12, 2024
ca5e32f
added description of changes
bosesuneha Feb 12, 2024
28581dd
updated tests
bosesuneha Feb 12, 2024
3c5e327
updated tests
bosesuneha Feb 12, 2024
de46e26
removed print statements
bosesuneha Feb 20, 2024
fcbc738
added consts for role assignments
bosesuneha Feb 20, 2024
9c801b8
Merge branch 'main' into support-private-dns-zone-in-war-cli
bosesuneha Feb 20, 2024
d741d4d
fixed lint error
bosesuneha Feb 20, 2024
bd029d5
Merge branch 'support-private-dns-zone-in-war-cli' of https://github.…
bosesuneha Feb 20, 2024
1b8f006
Merge branch 'main' into support-private-dns-zone-in-war-cli
FumingZhang Feb 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/aks-preview/HISTORY.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ Pending
* Vendor new SDK and bump API version to 2023-11-02-preview.
* Implicitly enable istio when ingress or egress gateway is enabled for Azure Service Mesh.
* Add `az aks nodepool delete-machines` command.
* Update `az aks approuting zone` command to support private dns zones.

1.0.0b5
+++++++
Expand Down
72 changes: 40 additions & 32 deletions src/aks-preview/azext_aks_preview/managed_cluster_decorator.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@
from knack.prompting import prompt_y_n
from knack.util import CLIError
from msrestazure.tools import is_valid_resource_id
from msrestazure.tools import parse_resource_id


logger = get_logger(__name__)
Expand Down Expand Up @@ -2567,15 +2568,16 @@ def get_dns_zone_resource_ids_from_input(self) -> Union[List[str], None]:

:return: list of str or None
"""
dns_zone_resource_ids = self.raw_param.get("dns_zone_resource_ids")
dns_zone_resource_ids = [
x.strip()
for x in (
dns_zone_resource_ids.split(",")
if dns_zone_resource_ids
else []
)
]
dns_zone_resource_ids_input = self.raw_param.get("dns_zone_resource_ids")
dns_zone_resource_ids = []

if dns_zone_resource_ids_input:
for dns_zone in dns_zone_resource_ids_input.split(","):
dns_zone = dns_zone.strip()
if dns_zone and is_valid_resource_id(dns_zone):
dns_zone_resource_ids.append(dns_zone)
else:
raise CLIError(dns_zone, " is not a valid Azure DNS Zone resource ID.")

return dns_zone_resource_ids

Expand Down Expand Up @@ -4485,24 +4487,30 @@ def _update_dns_zone_resource_ids(self, mc: ManagedCluster, dns_zone_resource_id

if mc.ingress_profile and mc.ingress_profile.web_app_routing and mc.ingress_profile.web_app_routing.enabled:
if add_dns_zone:
if mc.ingress_profile.web_app_routing.dns_zone_resource_ids is None:
mc.ingress_profile.web_app_routing.dns_zone_resource_ids = []
mc.ingress_profile.web_app_routing.dns_zone_resource_ids.extend(dns_zone_resource_ids)
if attach_zones:
try:
for dns_zone in dns_zone_resource_ids:
if not add_role_assignment(
self.cmd,
"DNS Zone Contributor",
mc.ingress_profile.web_app_routing.identity.object_id,
False,
scope=dns_zone
):
logger.warning(
'Could not create a role assignment for App Routing. '
'Are you an Owner on this subscription?')
except Exception as ex:
raise CLIError('Error in granting dns zone permisions to managed identity.\n') from ex
mc.ingress_profile.web_app_routing.dns_zone_resource_ids = (
mc.ingress_profile.web_app_routing.dns_zone_resource_ids or []
)
for dns_zone_id in dns_zone_resource_ids:
if dns_zone_id not in mc.ingress_profile.web_app_routing.dns_zone_resource_ids:
mc.ingress_profile.web_app_routing.dns_zone_resource_ids.append(dns_zone_id)
if attach_zones:
try:
is_private_dns_zone = (
parse_resource_id(dns_zone_id).get("type").lower() == "privatednszones"
)
role = "Private DNS Zone Contributor" if is_private_dns_zone else "DNS Zone Contributor"
Comment thread
bosesuneha marked this conversation as resolved.
Outdated
if not add_role_assignment(
self.cmd,
role,
mc.ingress_profile.web_app_routing.identity.object_id,
False,
scope=dns_zone_id
):
logger.warning(
'Could not create a role assignment for App Routing. '
'Are you an Owner on this subscription?')
except Exception as ex:
raise CLIError('Error in granting dns zone permissions to managed identity.\n') from ex
elif delete_dns_zone:
if mc.ingress_profile.web_app_routing.dns_zone_resource_ids:
dns_zone_resource_ids = [
Expand All @@ -4518,9 +4526,11 @@ def _update_dns_zone_resource_ids(self, mc: ManagedCluster, dns_zone_resource_id
if attach_zones:
try:
for dns_zone in dns_zone_resource_ids:
is_private_dns_zone = parse_resource_id(dns_zone).get("type").lower() == "privatednszones"
role = "Private DNS Zone Contributor" if is_private_dns_zone else "DNS Zone Contributor"
if not add_role_assignment(
self.cmd,
"DNS Zone Contributor",
role,
mc.ingress_profile.web_app_routing.identity.object_id,
False,
scope=dns_zone,
Expand Down Expand Up @@ -4662,6 +4672,8 @@ def postprocessing_after_mc_created(self, cluster: ManagedCluster) -> None:
:return: None
"""
super().postprocessing_after_mc_created(cluster)
print("inside postprocessing_after_mc_created")
print("dns_zone_resource_ids: ", cluster.ingress_profile.web_app_routing.dns_zone_resource_ids)
Comment thread
bosesuneha marked this conversation as resolved.
Outdated
enable_azure_container_storage = self.context.get_intermediate("enable_azure_container_storage")
disable_azure_container_storage = self.context.get_intermediate("disable_azure_container_storage")

Expand Down Expand Up @@ -4718,7 +4730,6 @@ def postprocessing_after_mc_created(self, cluster: ManagedCluster) -> None:
)

# attach keyvault to app routing addon
from msrestazure.tools import parse_resource_id
from azure.cli.command_modules.keyvault.custom import set_policy
from azext_aks_preview._client_factory import get_keyvault_client
keyvault_id = self.context.get_keyvault_id()
Expand All @@ -4743,12 +4754,10 @@ def postprocessing_after_mc_created(self, cluster: ManagedCluster) -> None:
keyvault_client = get_keyvault_client(self.cmd.cli_ctx, subscription_id=keyvault_subscription)
keyvault = keyvault_client.get(resource_group_name=keyvault_rg, vault_name=keyvault_name)
managed_identity_object_id = cluster.ingress_profile.web_app_routing.identity.object_id
print("managed_identity_object_id", managed_identity_object_id)
is_service_principal = False

try:
if keyvault.properties.enable_rbac_authorization:
print("within if block")
if not self.context.external_functions.add_role_assignment(
self.cmd,
"Key Vault Secrets User",
Expand All @@ -4761,7 +4770,6 @@ def postprocessing_after_mc_created(self, cluster: ManagedCluster) -> None:
"Are you an Owner on this subscription?"
)
else:
print("within else block")
keyvault = set_policy(
self.cmd,
keyvault_client,
Expand Down
98 changes: 49 additions & 49 deletions src/aks-preview/azext_aks_preview/tests/latest/test_managed_cluster_decorator.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7693,7 +7693,7 @@ def test_update_app_routing_profile(self):
self.cmd,
self.client,
{
"dns_zone_resource_ids": "test_dns_zone_resource_id_1,test_dns_zone_resource_id_2",
"dns_zone_resource_ids": "/subscriptions/testsub/resourceGroups/testrg/providers/Microsoft.Network/dnsZones/testdnszone_1.com, /subscriptions/testsub/resourceGroups/testrg/providers/Microsoft.Network/dnsZones/testdnszone_2.com",
"add_dns_zone": True,
},
CUSTOM_MGMT_AKS_PREVIEW,
Expand All @@ -7714,8 +7714,8 @@ def test_update_app_routing_profile(self):
web_app_routing=self.models.ManagedClusterIngressProfileWebAppRouting(
enabled=True,
dns_zone_resource_ids=[
"test_dns_zone_resource_id_1",
"test_dns_zone_resource_id_2",
"/subscriptions/testsub/resourceGroups/testrg/providers/Microsoft.Network/dnsZones/testdnszone_1.com",
"/subscriptions/testsub/resourceGroups/testrg/providers/Microsoft.Network/dnsZones/testdnszone_2.com",
],
)
),
Expand All @@ -7728,7 +7728,7 @@ def test_update_app_routing_profile(self):
self.cmd,
self.client,
{
"dns_zone_resource_ids": "test_dns_zone_resource_id_1",
"dns_zone_resource_ids": "/subscriptions/testsub/resourceGroups/testrg/providers/Microsoft.Network/dnsZones/testdnszone_1.com",
"delete_dns_zone": True,
},
CUSTOM_MGMT_AKS_PREVIEW,
Expand All @@ -7739,8 +7739,8 @@ def test_update_app_routing_profile(self):
web_app_routing=self.models.ManagedClusterIngressProfileWebAppRouting(
enabled=True,
dns_zone_resource_ids=[
"test_dns_zone_resource_id_1",
"test_dns_zone_resource_id_2",
"/subscriptions/testsub/resourceGroups/testrg/providers/Microsoft.Network/dnsZones/testdnszone_1.com",
"/subscriptions/testsub/resourceGroups/testrg/providers/Microsoft.Network/dnsZones/testdnszone_2.com",
],
)
),
Expand All @@ -7751,7 +7751,7 @@ def test_update_app_routing_profile(self):
location="test_location",
ingress_profile=self.models.ManagedClusterIngressProfile(
web_app_routing=self.models.ManagedClusterIngressProfileWebAppRouting(
enabled=True, dns_zone_resource_ids=["test_dns_zone_resource_id_2"]
enabled=True, dns_zone_resource_ids=["/subscriptions/testsub/resourceGroups/testrg/providers/Microsoft.Network/dnsZones/testdnszone_2.com"]
)
),
)
Expand All @@ -7762,7 +7762,7 @@ def test_update_app_routing_profile(self):
self.cmd,
self.client,
{
"dns_zone_resource_ids": "test_dns_zone_resource_id_3,test_dns_zone_resource_id_4",
"dns_zone_resource_ids": "/subscriptions/testsub/resourceGroups/testrg/providers/Microsoft.Network/privateDnsZones/testdnszone_3.com,/subscriptions/testsub/resourceGroups/testrg/providers/Microsoft.Network/privateDnsZones/testdnszone_4.com",
"update_dns_zone": True,
},
CUSTOM_MGMT_AKS_PREVIEW,
Expand All @@ -7773,8 +7773,8 @@ def test_update_app_routing_profile(self):
web_app_routing=self.models.ManagedClusterIngressProfileWebAppRouting(
enabled=True,
dns_zone_resource_ids=[
"test_dns_zone_resource_id_1",
"test_dns_zone_resource_id_2",
"/subscriptions/testsub/resourceGroups/testrg/providers/Microsoft.Network/dnsZones/testdnszone_1.com",
"/subscriptions/testsub/resourceGroups/testrg/providers/Microsoft.Network/dnsZones/testdnszone_2.com",
],
)
),
Expand All @@ -7788,8 +7788,8 @@ def test_update_app_routing_profile(self):
web_app_routing=self.models.ManagedClusterIngressProfileWebAppRouting(
enabled=True,
dns_zone_resource_ids=[
"test_dns_zone_resource_id_3",
"test_dns_zone_resource_id_4",
"/subscriptions/testsub/resourceGroups/testrg/providers/Microsoft.Network/privateDnsZones/testdnszone_3.com",
"/subscriptions/testsub/resourceGroups/testrg/providers/Microsoft.Network/privateDnsZones/testdnszone_4.com",
],
)
),
Expand All @@ -7809,8 +7809,8 @@ def test_update_app_routing_profile(self):
web_app_routing=self.models.ManagedClusterIngressProfileWebAppRouting(
enabled=True,
dns_zone_resource_ids=[
"test_dns_zone_resource_id_1",
"test_dns_zone_resource_id_2",
"/subscriptions/testsub/resourceGroups/testrg/providers/Microsoft.Network/dnsZones/testdnszone_1.com",
"/subscriptions/testsub/resourceGroups/testrg/providers/Microsoft.Network/dnsZones/testdnszone_2.com",
],
)
),
Expand All @@ -7823,47 +7823,15 @@ def test_update_app_routing_profile(self):
web_app_routing=self.models.ManagedClusterIngressProfileWebAppRouting(
enabled=True,
dns_zone_resource_ids=[
"test_dns_zone_resource_id_1",
"test_dns_zone_resource_id_2",
"/subscriptions/testsub/resourceGroups/testrg/providers/Microsoft.Network/dnsZones/testdnszone_1.com",
"/subscriptions/testsub/resourceGroups/testrg/providers/Microsoft.Network/dnsZones/testdnszone_2.com",
],
)
),
)
self.assertEqual(dec_mc_7, ground_truth_mc_7)

def test_enable_disable_ai_toolchain_operator(self):
# Should not update mc if unset
dec_0 = AKSPreviewManagedClusterUpdateDecorator(
self.cmd,
self.client,
{},
CUSTOM_MGMT_AKS_PREVIEW,
)
mc_0 = self.models.ManagedCluster(
location="test_location",
)
dec_0.context.attach_mc(mc_0)
dec_mc_0 = dec_0.update_ai_toolchain_operator(mc_0)
ground_truth_mc_0 = self.models.ManagedCluster(
location="test_location",
)
self.assertEqual(dec_mc_0, ground_truth_mc_0)

# Should error if both set
dec_6 = AKSPreviewManagedClusterUpdateDecorator(
self.cmd,
self.client,
{"disable_ai_toolchain_operator": True, "enable_ai_toolchain_operator": True},
CUSTOM_MGMT_AKS_PREVIEW,
)
mc_6 = self.models.ManagedCluster(
location="test_location",
)
dec_6.context.attach_mc(mc_6)
with self.assertRaises(MutuallyExclusiveArgumentError):
dec_6.update_ai_toolchain_operator(mc_6)

# update app routing with key vault
# update app routing with key vault
from azure.cli.core.mock import DummyCli
from azure.cli.core.commands import AzCliCommand
from azure.cli.core import AzCommandsLoader
Expand Down Expand Up @@ -7922,6 +7890,38 @@ def test_enable_disable_ai_toolchain_operator(self):

self.assertEqual(dec_mc_8, ground_truth_mc_8)

def test_enable_disable_ai_toolchain_operator(self):
# Should not update mc if unset
dec_0 = AKSPreviewManagedClusterUpdateDecorator(
self.cmd,
self.client,
{},
CUSTOM_MGMT_AKS_PREVIEW,
)
mc_0 = self.models.ManagedCluster(
location="test_location",
)
dec_0.context.attach_mc(mc_0)
dec_mc_0 = dec_0.update_ai_toolchain_operator(mc_0)
ground_truth_mc_0 = self.models.ManagedCluster(
location="test_location",
)
self.assertEqual(dec_mc_0, ground_truth_mc_0)

# Should error if both set
dec_6 = AKSPreviewManagedClusterUpdateDecorator(
self.cmd,
self.client,
{"disable_ai_toolchain_operator": True, "enable_ai_toolchain_operator": True},
CUSTOM_MGMT_AKS_PREVIEW,
)
mc_6 = self.models.ManagedCluster(
location="test_location",
)
dec_6.context.attach_mc(mc_6)
with self.assertRaises(MutuallyExclusiveArgumentError):
dec_6.update_ai_toolchain_operator(mc_6)

def test_update_mc_profile_preview(self):
import inspect

Expand Down