Update AKS tool to use BaseAzureResourceService#2852
Open
msalaman wants to merge 3 commits into
Open
Conversation
msalaman
requested review from
KarishmaGhiya,
RickWinter,
alzimmermsft,
anannya03,
Copilot,
fanyang-mono and
g2vinay
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the AKS tool’s cluster enumeration path to use BaseAzureResourceService (Azure Resource Graph) instead of ARM SDK enumeration, enabling Resource Graph-backed querying and shifting cluster modeling to JSON-based projection.
Changes:
- Switched
AksServiceinheritance toBaseAzureResourceServiceand migratedGetClusters/single-cluster retrieval to Resource Graph queries. - Replaced ARM SDK-based
Clustermapping with a JSON (JsonElement) projection/mapping pipeline and supporting parsing helpers.
Show a summary per file
| File | Description |
|---|---|
| tools/Azure.Mcp.Tools.Aks/src/Services/AksService.cs | Migrates cluster retrieval to Resource Graph and introduces JSON-based mapping for Cluster (and nested profiles). |
Copilot's findings
- Files reviewed: 1/1 changed files
- Comments generated: 2
Comment on lines
+60
to
+64
| var result = await ExecuteResourceQueryAsync( | ||
| "Microsoft.ContainerService/managedClusters", | ||
| resourceGroup, | ||
| subscription, | ||
| retryPolicy, |
Comment on lines
+281
to
+287
| if (props.TryGetProperty("oidcIssuerProfile", out var oidcEl)) | ||
| { | ||
| cluster.OidcIssuerProfile = new() | ||
| { | ||
| NetworkPlugin = data.NetworkProfile?.NetworkPlugin?.ToString(), | ||
| NetworkPluginMode = data.NetworkProfile?.NetworkPluginMode?.ToString(), | ||
| NetworkPolicy = data.NetworkProfile?.NetworkPolicy?.ToString(), | ||
| NetworkDataplane = data.NetworkProfile?.NetworkDataplane?.ToString(), | ||
| LoadBalancerSku = data.NetworkProfile?.LoadBalancerSku?.ToString(), | ||
| LoadBalancerProfile = data.NetworkProfile?.LoadBalancerProfile is null ? null : new() | ||
| { | ||
| ManagedOutboundIPCount = data.NetworkProfile?.LoadBalancerProfile?.ManagedOutboundIPs?.Count, | ||
| EffectiveOutboundIPs = data.NetworkProfile?.LoadBalancerProfile?.EffectiveOutboundIPs?.Select(e => new EffectiveOutboundIPReference() { Id = e.Id?.ToString() }).ToList(), | ||
| BackendPoolType = data.NetworkProfile?.LoadBalancerProfile?.BackendPoolType?.ToString() | ||
| }, | ||
| PodCidr = data.NetworkProfile?.PodCidr, | ||
| ServiceCidr = data.NetworkProfile?.ServiceCidr, | ||
| DnsServiceIP = data.NetworkProfile?.DnsServiceIP?.ToString(), | ||
| OutboundType = data.NetworkProfile?.OutboundType?.ToString(), | ||
| PodCidrs = data.NetworkProfile?.PodCidrs?.ToList(), | ||
| ServiceCidrs = data.NetworkProfile?.ServiceCidrs?.ToList(), | ||
| IpFamilies = data.NetworkProfile?.IPFamilies?.Select(f => f.ToString()).ToList() | ||
| }, | ||
| WindowsProfile = data.WindowsProfile is null ? null : new() { AdminUsername = data.WindowsProfile.AdminUsername }, | ||
| ServicePrincipalProfile = data.ServicePrincipalProfile is null ? null : new() { ClientId = data.ServicePrincipalProfile.ClientId }, | ||
| AutoUpgradeProfile = data.AutoUpgradeProfile is null ? null : new() | ||
| Enabled = TryGetNullableBool(oidcEl, "enabled"), | ||
| IssuerUrl = oidcEl.TryGetProperty("issuerURL", out var iuEl) ? iuEl.GetString() : null, | ||
| }; |
alzimmermsft
reviewed
Jun 9, 2026
| } | ||
|
|
||
| private static Cluster ConvertToClusterModel(ContainerServiceManagedClusterResource clusterResource) | ||
| private static Cluster ConvertToClusterFromJson(JsonElement item) |
Contributor
There was a problem hiding this comment.
Would it be possible for this method to leverage converting the JsonElement using JsonSerializer.Deserialize if we get the property names and types properly aligned? There is a lot of logic going on here, which makes understanding what's going on more complicated and likely to be error prone.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Update AKS tool to use BaseAzureResourceService to make use of Resource Graph functionality
GitHub issue number?
467
Pre-merge Checklist
servers/Azure.Mcp.Server/README.mdand/orservers/Fabric.Mcp.Server/README.mddocumentationREADME.mdchanges running the script./eng/scripts/Process-PackageReadMe.ps1. See Package READMEToolDescriptionEvaluatorand obtained a score of0.4or more and a top 3 ranking for all related test promptsconsolidated-tools.jsonbreaking-changelabelservers/Azure.Mcp.Server/docs/azmcp-commands.md./eng/scripts/Update-AzCommandsMetadata.ps1to update tool metadata inazmcp-commands.md(required for CI)servers/Azure.Mcp.Server/docs/e2eTestPrompts.mdcrypto mining, spam, data exfiltration, etc.)/azp run mcp - pullrequest - liveto run Live Test Pipeline