Skip to content

[AKS] aks create: Add condition to disable --enable-msi-auth-for-monitoring for service principle #26528

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 14 commits into from
May 25, 2023
Merged

[AKS] aks create: Add condition to disable --enable-msi-auth-for-monitoring for service principle #26528

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 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
bb8bfc9
Merge branch 'Azure:dev' into longw/ci-MSI-default
wanlonghenry May 24, 2023
4b6b36a
address create wiht service principle and fix tests
wanlonghenry May 24, 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
2 changes: 2 additions & 0 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 @@ -875,6 +875,8 @@ def aks_enable_addons(cmd, client, resource_group_name, name, addons,
msi_auth = False
if instance.service_principal_profile.client_id == "msi":
msi_auth = True
else:
enable_msi_auth_for_monitoring = False
subscription_id = get_subscription_id(cmd.cli_ctx)

instance = _update_addons(cmd, instance, subscription_id, resource_group_name, name, addons, enable=True,
Expand Down
10 changes: 8 additions & 2 deletions src/azure-cli/azure/cli/command_modules/acs/managed_cluster_decorator.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2608,6 +2608,12 @@ def get_enable_msi_auth_for_monitoring(self) -> Union[bool, None]:

# read the original value passed by the command
enable_msi_auth_for_monitoring = self.raw_param.get("enable_msi_auth_for_monitoring")
if (
self.mc and
self.mc.service_principal_profile and
self.mc.service_principal_profile.client_id is not None
):
return False
# try to read the property value corresponding to the parameter from the `mc` object
if (
self.mc and
Expand Down Expand Up @@ -5373,9 +5379,9 @@ def build_monitoring_addon_profile(self) -> ManagedClusterAddonProfile:
enabled=True,
config={
CONST_MONITORING_LOG_ANALYTICS_WORKSPACE_RESOURCE_ID: self.context.get_workspace_resource_id(),
CONST_MONITORING_USING_AAD_MSI_AUTH: "True"
CONST_MONITORING_USING_AAD_MSI_AUTH: "true"
if self.context.get_enable_msi_auth_for_monitoring()
else "False",
else "false",
},
)
# post-process, create a deployment
Expand Down
3 changes: 1 addition & 2 deletions src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_aks_commands.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7494,8 +7494,7 @@ def test_aks_create_with_monitoring_legacy_auth(self, resource_group, resource_g
'--ssh-key-value={ssh_key_value} '
response = self.cmd(create_cmd, checks=[
self.check('addonProfiles.omsagent.enabled', True),
self.exists('addonProfiles.omsagent.config.logAnalyticsWorkspaceResourceID'),
self.check('addonProfiles.omsagent.config.useAADAuth', 'false')
self.exists('addonProfiles.omsagent.config.logAnalyticsWorkspaceResourceID')
]).get_output_in_json()

# make sure a DCR was not created
Expand Down
6 changes: 3 additions & 3 deletions src/azure-cli/azure/cli/command_modules/acs/tests/latest/test_managed_cluster_decorator.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2312,7 +2312,7 @@ def test_get_enable_msi_auth_for_monitoring(self):
addon_profiles_1 = {
CONST_MONITORING_ADDON_NAME: self.models.ManagedClusterAddonProfile(
enabled=True,
config={CONST_MONITORING_USING_AAD_MSI_AUTH: "True"},
config={CONST_MONITORING_USING_AAD_MSI_AUTH: "true"},
)
}
mc = self.models.ManagedCluster(location="test_location", addon_profiles=addon_profiles_1)
Expand Down Expand Up @@ -6016,7 +6016,7 @@ def test_build_monitoring_addon_profile(self):
enabled=True,
config={
CONST_MONITORING_LOG_ANALYTICS_WORKSPACE_RESOURCE_ID: "/test_workspace_resource_id",
CONST_MONITORING_USING_AAD_MSI_AUTH: "False",
CONST_MONITORING_USING_AAD_MSI_AUTH: "false",
},
)
self.assertEqual(monitoring_addon_profile, ground_truth_monitoring_addon_profile)
Expand Down Expand Up @@ -6291,7 +6291,7 @@ def test_set_up_addon_profiles(self):
enabled=True,
config={
CONST_MONITORING_LOG_ANALYTICS_WORKSPACE_RESOURCE_ID: "/test_workspace_resource_id",
CONST_MONITORING_USING_AAD_MSI_AUTH: "False",
CONST_MONITORING_USING_AAD_MSI_AUTH: "false",
},
),
CONST_VIRTUAL_NODE_ADDON_NAME
Expand Down