feat: Extend AKS skill to perform cluster reviews - #2076
feat: Extend AKS skill to perform cluster reviews #2076johnbilliris (johnbilliris) wants to merge 27 commits into
Conversation
Co-authored-by: Copilot <copilot@github.com>
There was a problem hiding this comment.
Pull request overview
Introduces a new azure-kubernetes-review skill under plugin/skills/ to guide an evidence-based AKS posture review and generate a standardized Markdown audit report.
Changes:
- Adds the
azure-kubernetes-reviewskill with an end-to-end workflow and guardrails. - Adds reference documents (checklist matrix, container checks, diagnostics detector mapping) plus a report template.
- Updates
.gitignoreto ignore*.crtfiles.
Reviewed changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| plugin/skills/azure-kubernetes-review/SKILL.md | Skill entrypoint, workflow outline, inputs/outputs, guardrails |
| plugin/skills/azure-kubernetes-review/assets/aks-audit-report-template.md | Report template for audit output |
| plugin/skills/azure-kubernetes-review/references/aks-checklist-matrix.md | Canonical checklist matrix + validation commands |
| plugin/skills/azure-kubernetes-review/references/aks-container-best-practices.md | Container-level best-practice checks and scoring rules |
| plugin/skills/azure-kubernetes-review/references/aks-diagnostics-detectors.md | Diagnostics detector extraction + mapping guidance |
| plugin/skills/azure-kubernetes-review/references/quality-gates.md | Completeness checks and decision logic |
| plugin/skills/azure-kubernetes-review/references/workflow-scope-and-checklist.md | Scope definition + checklist loading workflow |
| plugin/skills/azure-kubernetes-review/references/workflow-diagnostics.md | Diagnostics execution workflow |
| plugin/skills/azure-kubernetes-review/references/workflow-warning-events.md | Warning event collection + correlation workflow |
| plugin/skills/azure-kubernetes-review/references/workflow-container-assessment.md | Container assessment workflow |
| plugin/skills/azure-kubernetes-review/references/workflow-validation-and-findings.md | Validation command execution + findings workflow |
| plugin/skills/azure-kubernetes-review/references/workflow-report.md | Report generation workflow |
| .gitignore | Ignores *.crt files |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Jon Gallant (jongio)
left a comment
Jon Gallant (jongio)
left a comment
There was a problem hiding this comment.
Solid contribution with deep AKS domain knowledge. The progressive disclosure structure (SKILL.md + 11 reference files) is well-suited for a complex audit workflow.
Main concern - echoing kvenkatrajan's feedback: the existing azure-kubernetes skill handles Day-0 provisioning while this targets Day-2 audit. There's a reasonable case for separation given the 1200+ lines of audit-specific content, but the repo's direction is to minimize skill count. Consider whether this could be a mode or workflow within the existing skill (e.g., triggered by "review AKS", "audit AKS cluster") with the reference files living alongside the existing skill's references.
Additional items below focus on frontmatter compliance and missing required sections per repo conventions.
Jon Gallant (jongio)
left a comment
Jon Gallant (jongio)
left a comment
There was a problem hiding this comment.
Consolidation into the existing azure-kubernetes skill addresses my previous concerns and kvenkatrajan's request. All four of my earlier comments (metadata.author, argument-hint, missing Quick Reference, unrelated .gitignore change) are resolved - the separate skill was removed and the .gitignore hunk dropped.
The Day-2 section integrates cleanly alongside the Day-0 content. Progressive disclosure is solid - 10 reference files handle the detail, keeping the workflow steps in SKILL.md as pointers.
One thing worth watching: SKILL.md went from ~2600 to ~4000 tokens. It's under the 5000-token limit from skill-files.instructions.md, but the Day-2 "Required Inputs" table and "Review Decision Logic" subsections could move to a reference file to keep the main file leaner - see Alex Okonechnikov (@okonech)'s thread.
Trim 26 chars from the description so total char count is exactly 20000 (was 20026). Regenerate snapshot to reflect the resulting trigger-keyword changes.
Jon Gallant (jongio)
left a comment
Jon Gallant (jongio)
left a comment
There was a problem hiding this comment.
Three issues to fix:
-
MCP tool names in �ks-diagnostics-detectors.md and �ks-review-diagnostics.md use mcp_aks_mcp_* naming. The rest of this skill and the repo uses mcp_azure_mcp_aks as the entry point with discovery. The agent won't find these tools at runtime.
-
Two �z aks nodepool list commands are missing required -g and --cluster-name params (see inline comments). Other instances in the checklist matrix correctly include them.
-
15 validation commands in �ks-checklist-matrix.md use indstr (Windows-only). Existing AKS skill files use grep. The �ks-container-best-practices.md already notes the platform difference, but the checklist defaults should match. Consider grep -i as primary with a Windows note for indstr.
…view # Conflicts: # tests/azure-kubernetes/__snapshots__/triggers.test.ts.snap # tests/azure-kubernetes/integration.test.ts # tests/azure-kubernetes/triggers.test.ts
johnbilliris (johnbilliris)
left a comment
johnbilliris (johnbilliris)
left a comment
There was a problem hiding this comment.
SKILL modified to reduce token usage
|
Besides the comment about ignoring the false positive vally lint error. Could you please also update the title so it has a well known prefix (e.g. |
|
Alex Thompson (@alex-thompson) Please review the changes in the azure-kubernetes skill. Please approve the PR once you are happy about it. |
johnbilliris (johnbilliris)
changed the title
Title changed as requested. |
Jon Gallant (jongio)
left a comment
Jon Gallant (jongio)
left a comment
There was a problem hiding this comment.
Incremental review since my 2026-05-29 feedback. The three issues I previously raised are addressed:
- MCP tool names now use
mcp_azure_mcp_akswith discovery (no hardcodedmcp_aks_mcp_*) az aks nodepool listcommands now include-gand--cluster-nameaks-checklist-matrix.mdnow usesgrepas primary withfindstras Windows fallback
One consistency gap remains in aks-container-best-practices.md, and the azure-compute/SKILL.md change looks out of scope for this PR (see inline comments).
Jon Gallant (jongio)
left a comment
Jon Gallant (jongio)
left a comment
There was a problem hiding this comment.
Incremental review since my 2026-05-29 feedback. The three issues I previously raised are addressed:
- MCP tool names now use
mcp_azure_mcp_akswith discovery (no hardcodedmcp_aks_mcp_*) az aks nodepool listcommands now include-gand--cluster-nameaks-checklist-matrix.mdnow usesgrepas primary withfindstras Windows fallback
One consistency gap remains in aks-container-best-practices.md, and the azure-compute/SKILL.md change looks out of scope for this PR (see inline comments).
Jon Gallant (jongio)
left a comment
Jon Gallant (jongio)
left a comment
There was a problem hiding this comment.
The Day-2 AKS cluster review workflow is well-structured: proper 9-step workflow, quality gates, conservative assessment rules, and graceful fallback chains. The content itself is solid AKS operations guidance.
One blocking concern: the frontmatter description removes several existing routing keywords (pod rightsizing, over-provisioned AKS pod, pod resource requests and limits, Vertical Pod Autoscaler, VPA recommendations) without verifying routing still works. The PR checklist item for routing verification is unchecked. These keywords represent distinct user intents that may no longer route to this skill.
Please either restore the removed keywords (the description is still under the 1024-char limit with them) or run npm run test:skills:integration -- azure-kubernetes confirming VPA/pod-rightsizing queries still route correctly.
Merge main into branch
|
The PR description is outdated. |
Description
Introduce an Azure Kubernetes Review skill
Checklist
cd tests && npm test)npm run test:skills:integration -- <skill>)USE FOR/DO NOT USE FOR/PREFER OVERclauses: confirmed no routing regressions for competing skillsRelated Issues