Skip to content

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

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 12 commits into from
May 18, 2023
Merged

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

Changes from 2 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
142dc0d
MSI default change to true and add check for airgap
wanlonghenry May 6, 2023
5aa1eb9
fix style
wanlonghenry May 8, 2023
33f3368
address comments: remove Metrics role assignment and change airgap co…
wanlonghenry May 8, 2023
566509d
Merge branch 'dev' of https://github.com/wanlonghenry/azure-cli into …
wanlonghenry May 9, 2023
078924e
address comments use the instance object to check the identity type
wanlonghenry May 10, 2023
e29fb52
Merge branch 'Azure:dev' into longw/ci-MSI-default
wanlonghenry May 11, 2023
c8869c9
regenerate recordings for tests
wanlonghenry May 11, 2023
f59171c
update recording
wanlonghenry May 16, 2023
cff97cb
update warning message
wanlonghenry May 17, 2023
221f58f
update style
wanlonghenry May 17, 2023
9afcb74
update test and move check places
wanlonghenry May 18, 2023
421ad75
merge address conflict
wanlonghenry May 18, 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
39 changes: 25 additions & 14 deletions src/azure-cli/azure/cli/command_modules/acs/custom.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,9 @@
CONST_VIRTUAL_NODE_SUBNET_NAME,
DecoratorEarlyExitException,
)
from azure.cli.command_modules.acs._helpers import (
get_user_assigned_identity_by_resource_id,
)
from azure.cli.command_modules.acs._helpers import get_snapshot_by_snapshot_id
from azure.cli.command_modules.acs._resourcegroup import get_rg_location
from azure.cli.command_modules.acs._validators import extract_comma_separated_string
Expand Down Expand Up @@ -452,7 +455,7 @@ def aks_create(
# addons
enable_addons=None,
workspace_resource_id=None,
enable_msi_auth_for_monitoring=False,
enable_msi_auth_for_monitoring=True,
Comment thread
wanlonghenry marked this conversation as resolved.
enable_syslog=False,
data_collection_settings=None,
aci_subnet_name=None,
Expand Down Expand Up @@ -854,7 +857,7 @@ def aks_enable_addons(cmd, client, resource_group_name, name, addons,
enable_secret_rotation=False,
rotation_poll_interval=None,
no_wait=False,
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 @@ -925,21 +928,29 @@ def aks_enable_addons(cmd, client, resource_group_name, name, addons,
result = LongRunningOperation(cmd.cli_ctx)(
client.begin_create_or_update(resource_group_name, name, instance))

# For monitoring addon, Metrics role assignement doesnt require in case of MSI auth
if enable_monitoring and not enable_msi_auth_for_monitoring:
if enable_monitoring:
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
)
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 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
)
# For monitoring addon, Metrics role assignement doesnt require in case of MSI auth
if not enable_msi_auth_for_monitoring and cloud_name.lower() == 'azurecloud':
Comment thread
wanlonghenry marked this conversation as resolved.
Outdated
# mdm metrics supported only in Azure Public cloud so add the role assignment only in this cloud
add_monitoring_role_assignment(
result, cluster_resource_id, cmd)

if enable_msi_auth_for_monitoring and cloud_name.lower() != 'azurecloud':
Comment thread
wanlonghenry marked this conversation as resolved.
Outdated
try:
get_user_assigned_identity_by_resource_id(cluster_resource_id)
Comment thread
wanlonghenry marked this conversation as resolved.
Outdated
Comment thread
wanlonghenry marked this conversation as resolved.
Outdated
except InvalidArgumentValueError:
enable_msi_auth_for_monitoring = False
# raise ArgumentUsageError("--enable_msi_auth_for_monitoring are only support in public cloud.")
# logger.warning("--enable_msi_auth_for_monitoring are only support in public cloud.")
Comment thread
wanlonghenry marked this conversation as resolved.
Outdated

if ingress_appgw_addon_enabled:
add_ingress_appgw_addon_role_assignment(result, cmd)

Expand All @@ -960,7 +971,7 @@ def aks_enable_addons(cmd, client, resource_group_name, name, addons,

def _update_addons(cmd, instance, subscription_id, resource_group_name, name, addons, enable,
workspace_resource_id=None,
enable_msi_auth_for_monitoring=False,
enable_msi_auth_for_monitoring=True,
Comment thread
wanlonghenry marked this conversation as resolved.
subnet_name=None,
appgw_name=None,
appgw_subnet_cidr=None,
Expand Down