Skip to content

[AKS PREVIEW]aks create: Change the default value of --enable-msi-auth-for-monitoring to true and add check for airgap clouds #6311

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
zhoxing-ms merged 9 commits into from
May 26, 2023
Merged

[AKS PREVIEW]aks create: Change the default value of --enable-msi-auth-for-monitoring to true and add check for airgap clouds #6311

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
0c3d90f
Change the default value of `--enable-msi-auth-for-monitoring` to true
wanlonghenry May 22, 2023
ce356dd
remove default value for addon update
wanlonghenry May 22, 2023
ac190eb
default need to be true
wanlonghenry May 22, 2023
49d883d
update wording
wanlonghenry May 22, 2023
fd76f3e
add default value for aks_addon_update
wanlonghenry May 22, 2023
edf1def
fix style
wanlonghenry May 23, 2023
4b226eb
release new version per comment
wanlonghenry May 23, 2023
0840717
address live tests
wanlonghenry May 24, 2023
a38b47a
update
wanlonghenry May 25, 2023
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 @@ -15,6 +15,7 @@ Pending

0.5.139
* `az aks create` and `az aks nodepool add`: Add warning message when specifying `--os-sku` to `Mariner` or `CBLMariner`.
* `az aks create` and `az aks enable-addons`: Change the default value of `--enable-msi-auth-for-monitoring` to `true` and add check for airgap clouds
Comment thread
wanlonghenry marked this conversation as resolved.
Outdated

0.5.138
+++++++
Expand Down
57 changes: 10 additions & 47 deletions src/aks-preview/azext_aks_preview/addonconfiguration.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ def enable_addons(cmd,
rotation_poll_interval=None,
no_wait=False,
dns_zone_resource_id=None,
enable_msi_auth_for_monitoring=False,
enable_msi_auth_for_monitoring=True,
enable_syslog=False,
data_collection_settings=None):
instance = client.get(resource_group_name, name)
Expand Down Expand Up @@ -139,17 +139,7 @@ def enable_addons(cmd,
# adding a wait here since we rely on the result for role assignment
result = LongRunningOperation(cmd.cli_ctx)(
client.begin_create_or_update(resource_group_name, name, instance))
cloud_name = cmd.cli_ctx.cloud.name
# mdm metrics supported only in Azure Public cloud so add the role assignment only in this cloud
if monitoring_addon_enabled and cloud_name.lower() == 'azurecloud':
from msrestazure.tools import resource_id
cluster_resource_id = resource_id(
subscription=subscription_id,
resource_group=resource_group_name,
namespace='Microsoft.ContainerService', type='managedClusters',
name=name
)
add_monitoring_role_assignment(result, cluster_resource_id, cmd)

if ingress_appgw_addon_enabled:
add_ingress_appgw_addon_role_assignment(result, cmd)
if enable_virtual_node:
Expand Down Expand Up @@ -177,7 +167,7 @@ def update_addons(cmd, # pylint: disable=too-many-branches,too-many-statements
enable,
check_enabled=True,
workspace_resource_id=None,
enable_msi_auth_for_monitoring=False,
enable_msi_auth_for_monitoring=True,
subnet_name=None,
appgw_name=None,
appgw_subnet_prefix=None,
Expand Down Expand Up @@ -262,9 +252,15 @@ def update_addons(cmd, # pylint: disable=too-many-branches,too-many-statements
workspace_resource_id = sanitize_loganalytics_ws_resource_id(
workspace_resource_id)

cloud_name = cmd.cli_ctx.cloud.name
if enable_msi_auth_for_monitoring and (cloud_name.lower() == 'ussec' or cloud_name.lower() == 'usnat'):
if instance.identity is not None and instance.identity.type is not None and instance.identity.type == "userassigned":
logger.warning("--enable_msi_auth_for_monitoring is not supported in %s cloud and continuing monitoring enablement without this flag.", cloud_name)
enable_msi_auth_for_monitoring = False

addon_profile.config = {
logAnalyticsConstName: workspace_resource_id}
addon_profile.config[CONST_MONITORING_USING_AAD_MSI_AUTH] = enable_msi_auth_for_monitoring
addon_profile.config[CONST_MONITORING_USING_AAD_MSI_AUTH] = "true" if enable_msi_auth_for_monitoring else "false"
elif addon == (CONST_VIRTUAL_NODE_ADDON_NAME + os_type):
if addon_profile.enabled and check_enabled:
raise CLIError('The virtual-node addon is already enabled for this managed cluster.\n'
Expand Down Expand Up @@ -348,39 +344,6 @@ def update_addons(cmd, # pylint: disable=too-many-branches,too-many-statements
return instance


def add_monitoring_role_assignment(result, cluster_resource_id, cmd):
service_principal_msi_id = None
# Check if service principal exists, if it does, assign permissions to service principal
# Else, provide permissions to MSI
if (
hasattr(result, 'service_principal_profile') and
hasattr(result.service_principal_profile, 'client_id') and
result.service_principal_profile.client_id != 'msi'
):
logger.info('valid service principal exists, using it')
service_principal_msi_id = result.service_principal_profile.client_id
is_service_principal = True
elif (
(hasattr(result, 'addon_profiles')) and
(CONST_MONITORING_ADDON_NAME in result.addon_profiles) and
(hasattr(result.addon_profiles[CONST_MONITORING_ADDON_NAME], 'identity')) and
(hasattr(
result.addon_profiles[CONST_MONITORING_ADDON_NAME].identity, 'object_id'))
):
logger.info('omsagent MSI exists, using it')
service_principal_msi_id = result.addon_profiles[CONST_MONITORING_ADDON_NAME].identity.object_id
is_service_principal = False

if service_principal_msi_id is not None:
if not add_role_assignment(cmd, 'Monitoring Metrics Publisher',
service_principal_msi_id, is_service_principal, scope=cluster_resource_id):
logger.warning('Could not create a role assignment for Monitoring addon. '
'Are you an Owner on this subscription?')
else:
logger.warning('Could not find service principal or user assigned MSI for role'
'assignment')


def add_ingress_appgw_addon_role_assignment(result, cmd):
service_principal_msi_id = None
# Check if service principal exists, if it does, assign permissions to service principal
Expand Down
31 changes: 13 additions & 18 deletions src/aks-preview/azext_aks_preview/custom.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@
from azext_aks_preview._resourcegroup import get_rg_location
from azext_aks_preview.addonconfiguration import (
add_ingress_appgw_addon_role_assignment,
add_monitoring_role_assignment,
add_virtual_node_role_assignment,
enable_addons,
)
Expand Down Expand Up @@ -500,7 +499,7 @@ def aks_create(
# addons
enable_addons=None,
workspace_resource_id=None,
enable_msi_auth_for_monitoring=False,
enable_msi_auth_for_monitoring=True,
enable_syslog=False,
data_collection_settings=None,
aci_subnet_name=None,
Expand Down Expand Up @@ -1455,7 +1454,7 @@ def aks_addon_enable(cmd, client, resource_group_name, name, addon, workspace_re
subnet_name=None, appgw_name=None, appgw_subnet_prefix=None, appgw_subnet_cidr=None, appgw_id=None,
appgw_subnet_id=None,
appgw_watch_namespace=None, enable_sgxquotehelper=False, enable_secret_rotation=False, rotation_poll_interval=None,
no_wait=False, enable_msi_auth_for_monitoring=False,
no_wait=False, enable_msi_auth_for_monitoring=True,
dns_zone_resource_id=None, enable_syslog=False, data_collection_settings=None):
return enable_addons(cmd, client, resource_group_name, name, addon, workspace_resource_id=workspace_resource_id,
subnet_name=subnet_name, appgw_name=appgw_name, appgw_subnet_prefix=appgw_subnet_prefix,
Expand All @@ -1475,7 +1474,7 @@ def aks_addon_update(cmd, client, resource_group_name, name, addon, workspace_re
subnet_name=None, appgw_name=None, appgw_subnet_prefix=None, appgw_subnet_cidr=None, appgw_id=None,
appgw_subnet_id=None,
appgw_watch_namespace=None, enable_sgxquotehelper=False, enable_secret_rotation=False, rotation_poll_interval=None,
no_wait=False, enable_msi_auth_for_monitoring=False,
no_wait=False, enable_msi_auth_for_monitoring=None,
dns_zone_resource_id=None, enable_syslog=False, data_collection_settings=None):
instance = client.get(resource_group_name, name)
addon_profiles = instance.addon_profiles
Expand Down Expand Up @@ -1543,7 +1542,7 @@ def aks_disable_addons(cmd, client, resource_group_name, name, addons, no_wait=F

def aks_enable_addons(cmd, client, resource_group_name, name, addons, workspace_resource_id=None,
subnet_name=None, appgw_name=None, appgw_subnet_prefix=None, appgw_subnet_cidr=None, appgw_id=None, appgw_subnet_id=None,
appgw_watch_namespace=None, enable_sgxquotehelper=False, enable_secret_rotation=False, rotation_poll_interval=None, no_wait=False, enable_msi_auth_for_monitoring=False,
appgw_watch_namespace=None, enable_sgxquotehelper=False, enable_secret_rotation=False, rotation_poll_interval=None, no_wait=False, enable_msi_auth_for_monitoring=True,
dns_zone_resource_id=None, enable_syslog=False, data_collection_settings=None):

instance = client.get(resource_group_name, name)
Expand Down Expand Up @@ -1610,17 +1609,7 @@ def aks_enable_addons(cmd, client, resource_group_name, name, addons, workspace_
# adding a wait here since we rely on the result for role assignment
result = LongRunningOperation(cmd.cli_ctx)(
client.begin_create_or_update(resource_group_name, name, instance))
cloud_name = cmd.cli_ctx.cloud.name
# mdm metrics supported only in Azure Public cloud so add the role assignment only in this cloud
if monitoring and cloud_name.lower() == 'azurecloud':
from msrestazure.tools import resource_id
cluster_resource_id = resource_id(
subscription=subscription_id,
resource_group=resource_group_name,
namespace='Microsoft.ContainerService', type='managedClusters',
name=name
)
add_monitoring_role_assignment(result, cluster_resource_id, cmd)

if ingress_appgw_addon_enabled:
add_ingress_appgw_addon_role_assignment(result, cmd)
if enable_virtual_node:
Expand Down Expand Up @@ -1651,7 +1640,7 @@ def _update_addons(cmd, # pylint: disable=too-many-branches,too-many-statements
addons,
enable,
workspace_resource_id=None,
enable_msi_auth_for_monitoring=False,
enable_msi_auth_for_monitoring=True,
subnet_name=None,
appgw_name=None,
appgw_subnet_prefix=None,
Expand Down Expand Up @@ -1736,9 +1725,15 @@ def _update_addons(cmd, # pylint: disable=too-many-branches,too-many-statements
workspace_resource_id = sanitize_loganalytics_ws_resource_id(
workspace_resource_id)

cloud_name = cmd.cli_ctx.cloud.name
if enable_msi_auth_for_monitoring and (cloud_name.lower() == 'ussec' or cloud_name.lower() == 'usnat'):
if instance.identity is not None and instance.identity.type is not None and instance.identity.type == "userassigned":
logger.warning("--enable_msi_auth_for_monitoring is not supported in %s cloud and continuing monitoring enablement without this flag.", cloud_name)
enable_msi_auth_for_monitoring = False

addon_profile.config = {
logAnalyticsConstName: workspace_resource_id}
addon_profile.config[CONST_MONITORING_USING_AAD_MSI_AUTH] = enable_msi_auth_for_monitoring
addon_profile.config[CONST_MONITORING_USING_AAD_MSI_AUTH] = "true" if enable_msi_auth_for_monitoring else "false"
elif addon == (CONST_VIRTUAL_NODE_ADDON_NAME + os_type):
if addon_profile.enabled:
raise CLIError('The virtual-node addon is already enabled for this managed cluster.\n'
Expand Down