Fix addon and msi auth bug

Author: carlotaarvela

src/aks-preview/azext_aks_preview/managed_cluster_decorator.py

@@ -389,7 +389,24 @@ def get_enable_msi_auth_for_monitoring(self) -> Union[bool, None]:
         elif enable_azure_monitor_logs:
             result = True
         elif enable_msi_auth_for_monitoring is False:
-            result = False
+            # The base class returns False when service_principal_profile.client_id is not None,
+            # but MSI-based clusters set client_id to "msi". Check if the monitoring addon
+            # already has useAADAuth=true, which indicates MSI auth is actually in use.
+            addon_consts = self.get_addon_consts()
+            CONST_MONITORING_ADDON_NAME = addon_consts.get("CONST_MONITORING_ADDON_NAME")
+            CONST_MONITORING_USING_AAD_MSI_AUTH = addon_consts.get("CONST_MONITORING_USING_AAD_MSI_AUTH")
+            if self.mc and self.mc.addon_profiles:
+                monitoring_profile = (
+                    self.mc.addon_profiles.get(CONST_MONITORING_ADDON_NAME) or
+                    self.mc.addon_profiles.get(CONST_MONITORING_ADDON_NAME_CAMELCASE)
+                )
+                if (monitoring_profile and monitoring_profile.config and
+                    str(monitoring_profile.config.get(CONST_MONITORING_USING_AAD_MSI_AUTH, "")).lower() == "true"):
+                    result = True
+                else:
+                    result = False
+            else:
+                result = False
         elif enable_msi_auth_for_monitoring is None and not disable_msi_auth and not enable_msi_auth:
             result = True
         else:
@@ -5109,9 +5126,14 @@ def postprocessing_after_mc_created(self, cluster: ManagedCluster) -> None:
                 )
                 addon_consts = self.context.get_addon_consts()
                 CONST_MONITORING_ADDON_NAME = addon_consts.get("CONST_MONITORING_ADDON_NAME")
+                # The API response may return the addon key as either "omsagent" or "omsAgent"
+                monitoring_addon_key = CONST_MONITORING_ADDON_NAME
+                if CONST_MONITORING_ADDON_NAME not in cluster.addon_profiles and \
+                   CONST_MONITORING_ADDON_NAME_CAMELCASE in cluster.addon_profiles:
+                    monitoring_addon_key = CONST_MONITORING_ADDON_NAME_CAMELCASE
                 self.context.external_functions.ensure_container_insights_for_monitoring(
                     self.cmd,
-                    cluster.addon_profiles[CONST_MONITORING_ADDON_NAME],
+                    cluster.addon_profiles[monitoring_addon_key],
                     self.context.get_subscription_id(),
                     self.context.get_resource_group_name(),
                     self.context.get_name(),
@@ -7733,14 +7755,21 @@ def postprocessing_after_mc_created(self, cluster: ManagedCluster) -> None:
             CONST_MONITORING_ADDON_NAME = addon_consts.get("CONST_MONITORING_ADDON_NAME")
             CONST_MONITORING_USING_AAD_MSI_AUTH = addon_consts.get("CONST_MONITORING_USING_AAD_MSI_AUTH")
 
-            if (cluster.addon_profiles and
-                    CONST_MONITORING_ADDON_NAME in cluster.addon_profiles and
-                    cluster.addon_profiles[CONST_MONITORING_ADDON_NAME].enabled):
+            # The API response may return the addon key as either "omsagent" or "omsAgent"
+            monitoring_addon_key = None
+            if cluster.addon_profiles:
+                if CONST_MONITORING_ADDON_NAME in cluster.addon_profiles:
+                    monitoring_addon_key = CONST_MONITORING_ADDON_NAME
+                elif CONST_MONITORING_ADDON_NAME_CAMELCASE in cluster.addon_profiles:
+                    monitoring_addon_key = CONST_MONITORING_ADDON_NAME_CAMELCASE
+
+            if (monitoring_addon_key and
+                    cluster.addon_profiles[monitoring_addon_key].enabled):
 
                 # Check if MSI auth is enabled
                 if (CONST_MONITORING_USING_AAD_MSI_AUTH in
-                    cluster.addon_profiles[CONST_MONITORING_ADDON_NAME].config and
-                    str(cluster.addon_profiles[CONST_MONITORING_ADDON_NAME].config[
+                    cluster.addon_profiles[monitoring_addon_key].config and
+                    str(cluster.addon_profiles[monitoring_addon_key].config[
                         CONST_MONITORING_USING_AAD_MSI_AUTH]).lower() == "true"):
 
                     # Check parameter sizes to identify what might be causing large headers
@@ -7755,7 +7784,7 @@ def postprocessing_after_mc_created(self, cluster: ManagedCluster) -> None:
 
                     self.context.external_functions.ensure_container_insights_for_monitoring(
                         self.cmd,
-                        cluster.addon_profiles[CONST_MONITORING_ADDON_NAME],
+                        cluster.addon_profiles[monitoring_addon_key],
                         self.context.get_subscription_id(),
                         self.context.get_resource_group_name(),
                         self.context.get_name(),

src/aks-preview/azext_aks_preview/tests/latest/test_managed_cluster_decorator.py

@@ -41,6 +41,7 @@
     CONST_MANAGED_GATEWAY_INSTALLATION_DISABLED,
     CONST_MANAGED_GATEWAY_INSTALLATION_STANDARD,
     CONST_MONITORING_ADDON_NAME,
+    CONST_MONITORING_ADDON_NAME_CAMELCASE,
     CONST_MONITORING_LOG_ANALYTICS_WORKSPACE_RESOURCE_ID,
     CONST_MONITORING_USING_AAD_MSI_AUTH,
     CONST_NODEPOOL_MODE_SYSTEM,
@@ -5137,6 +5138,36 @@ def test_get_enable_high_log_scale_mode_update_monitoring_camelcase_key(self):
         result = ctx.get_enable_high_log_scale_mode()
         self.assertTrue(result)
 
+    def test_get_enable_msi_auth_for_monitoring_with_msi_service_principal(self):
+        """Test that MSI auth is correctly detected when service_principal_profile.client_id='msi'.
+
+        The base class returns False when client_id is not None, but MSI-based clusters set
+        client_id to 'msi'. The preview override should check the addon config for useAADAuth.
+        """
+        ctx = AKSPreviewManagedClusterContext(
+            self.cmd,
+            AKSManagedClusterParamDict({}),
+            self.models,
+            decorator_mode=DecoratorMode.UPDATE,
+        )
+        mc = self.models.ManagedCluster(
+            location="test_location",
+            service_principal_profile=self.models.ManagedClusterServicePrincipalProfile(
+                client_id="msi",
+            ),
+            addon_profiles={
+                "omsagent": self.models.ManagedClusterAddonProfile(
+                    enabled=True,
+                    config={
+                        CONST_MONITORING_USING_AAD_MSI_AUTH: "true",
+                    }
+                )
+            },
+        )
+        ctx.attach_mc(mc)
+        result = ctx.get_enable_msi_auth_for_monitoring()
+        self.assertTrue(result)
+
     def test_get_enable_default_domain(self):
         # default value
         ctx_1 = AKSPreviewManagedClusterContext(
@@ -7967,6 +7998,33 @@ def test_postprocessing_create_dcr_true_when_cnl_and_hlsm_both_set(self):
         _, kwargs = mock_ecifm.call_args
         self.assertTrue(kwargs["create_dcr"])
 
+    def test_postprocessing_create_dcr_true_with_camelcase_addon_key(self):
+        """create_dcr=True when the API response uses the camelCase 'omsAgent' addon key.
+
+        The API may return 'omsAgent' instead of 'omsagent'. The postprocessing must
+        handle both key variants to ensure the DCR is updated.
+        """
+        dec = self._make_postprocessing_decorator(
+            {"enable_container_network_logs": True, "enable_high_log_scale_mode": True}
+        )
+        # Build cluster with camelCase addon key (as the API might return)
+        cluster = self.models.ManagedCluster(
+            location="test_location",
+            addon_profiles={
+                CONST_MONITORING_ADDON_NAME_CAMELCASE: self.models.ManagedClusterAddonProfile(enabled=True)
+            },
+        )
+        external_functions = dec.context.external_functions
+        with patch.object(
+            external_functions, "ensure_container_insights_for_monitoring", return_value=None
+        ) as mock_ecifm, patch.object(
+            dec.context, "get_enable_high_log_scale_mode", return_value=True
+        ):
+            dec.postprocessing_after_mc_created(cluster)
+        mock_ecifm.assert_called_once()
+        _, kwargs = mock_ecifm.call_args
+        self.assertTrue(kwargs["create_dcr"])
+
 
     def test_set_up_health_monitor_profile(self):
         # no flag - no change
@@ -13738,6 +13796,71 @@ def test_update_disable_hlsm_standalone_no_postprocessing(self):
             dec.context.get_intermediate("monitoring_addon_postprocessing_required", default_value=False)
         )
 
+    def test_update_postprocessing_with_camelcase_addon_key(self):
+        """Test that update postprocessing works when the API response uses 'omsAgent' (camelCase).
+
+        The API may return the monitoring addon as 'omsAgent' instead of 'omsagent'.
+        The postprocessing must handle both key variants so the DCR gets updated.
+        """
+        dec = AKSPreviewManagedClusterUpdateDecorator(
+            self.cmd,
+            self.client,
+            {
+                "enable_container_network_logs": True,
+                "enable_high_log_scale_mode": True,
+                "name": "test_name",
+                "resource_group_name": "test_rg_name",
+                "location": "test_location",
+                "enable_msi_auth_for_monitoring": True,
+                "enable_syslog": False,
+                "data_collection_settings": None,
+            },
+            CUSTOM_MGMT_AKS_PREVIEW,
+        )
+        mc = self.models.ManagedCluster(
+            location="test_location",
+            network_profile=self.models.ContainerServiceNetworkProfile(
+                advanced_networking=self.models.AdvancedNetworking(
+                    enabled=True,
+                ),
+            ),
+            addon_profiles={
+                "omsagent": self.models.ManagedClusterAddonProfile(
+                    enabled=True,
+                    config={
+                        CONST_MONITORING_USING_AAD_MSI_AUTH: "true",
+                    }
+                )
+            },
+        )
+        dec.context.attach_mc(mc)
+        dec.context.set_intermediate("subscription_id", "test_subscription_id")
+        # Simulate profile update setting the postprocessing flag
+        dec.update_monitoring_profile_flow_logs(mc)
+        self.assertTrue(dec.context.get_intermediate("monitoring_addon_postprocessing_required"))
+
+        # Build API response cluster with camelCase addon key
+        cluster = self.models.ManagedCluster(
+            location="test_location",
+            addon_profiles={
+                CONST_MONITORING_ADDON_NAME_CAMELCASE: self.models.ManagedClusterAddonProfile(
+                    enabled=True,
+                    config={
+                        CONST_MONITORING_USING_AAD_MSI_AUTH: "true",
+                    }
+                )
+            },
+        )
+        external_functions = dec.context.external_functions
+        with patch.object(
+            external_functions, "ensure_container_insights_for_monitoring", return_value=None
+        ) as mock_ecifm:
+            dec.postprocessing_after_mc_created(cluster)
+        mock_ecifm.assert_called_once()
+        _, kwargs = mock_ecifm.call_args
+        self.assertTrue(kwargs["create_dcr"])
+        self.assertTrue(kwargs["enable_high_log_scale_mode"])
+
     def test_update_node_provisioning_profile(self):
         dec_0 = AKSPreviewManagedClusterUpdateDecorator(
             self.cmd,