Repository navigation
[skills] Add aws-eks-operations-review skill - #77
Conversation
Add a DevOps Agent skill that grades an Amazon EKS cluster against best practices across nine pillars — Operations, Resilience, Security, Scalability, Performance, Observability, Networking, Cost, Control Plane — plus AWS API and Cluster Insights rows, returning evidence-backed PASS/FAIL/N/A scorecards with prioritized remediations. SKILL.md drives an S0-S8 state machine and loads references just-in-time to bound context. The references tree carries the runtime contracts (check manifest, discovery manifest, gates, guards, thresholds, QA checklist, report contract), nine pillar definitions, 52 remediation shards, control-plane Logs Insights query shards, and three decision trees. Only eval definitions and fixtures are included. Generated eval run output under evals/functional, evals/best-practices, and evals/structure is reproducible via evals/run-functional-serial.py, which prunes those same paths when building a workspace, so committing it would add ~8 MB of derived artifacts. The eval helper scripts are excluded by the skills/.gitignore extension allowlist, which permits only the file types DevOps Agent skill uploads accept.
CONTRIBUTING requires each skill's frontmatter to carry a metadata block with author and version, and each skill README to state a non-production disclaimer. Add both to aws-eks-operations-review. Version 1.9.3 matches the top entry in the skill's CHANGELOG.
Add a custom agent that orchestrates the aws-eks-operations-review skill and publishes the review as multiple artifacts — one Summary plus one per graded pillar — rather than a single cumulative artifact. A full review spans 49 discovery areas and roughly 288 graded rows. Assembling that into one artifact means many sequential render calls whose payload grows each time, which is what produces render stalls and half-written reports. Splitting by pillar keeps most artifacts to one or two calls, makes each finished pillar a standalone deliverable, and removes cross-pillar accumulation risk. The system prompt carries the artifact split design, per-artifact render chunking with a character ceiling, a monotonicity guard that treats lost element accumulation as a failed call regardless of reported success, a runtime budget with a degradation ladder that declares skipped scope rather than omitting it silently, the Remediation Depth Contract defining full and compact finding forms, evidence rules requiring an observed result for every verdict, the QA coverage gate, and the artifact element schemas. The README documents the skill, the 21 tools, and the three memory stores to assign, and notes that tools and memory stores are Chat-only. It also covers third-party MCP servers for environments whose telemetry lives outside CloudWatch, including the prompt change required alongside assigning those tools.
A render call was rejected with "Invalid \escape" because remediation prose escaped JMESPath backtick literals as \` — a backslash before a backtick is not a valid JSON escape, so the payload failed to parse and nothing was written. The prompt specified what findings must contain but never how to encode them. The Depth Contract asks for concrete CLI snippets in remediation steps, which is exactly where backticks, quotes, and nested JSON accumulate, so this failure mode was likely rather than incidental. Add an encoding section to Artifacts: only JSON's own escapes are valid, backticks are never escaped, and JMESPath single-quoted raw strings are preferred over backtick literals. Require a parse self-check before each call. Also classify an encoding failure separately from a stall in 4e. Batch halving, finding demotion, and sibling artifacts all address payload size and cannot fix invalid escaping — the correct response is to fix the escape and resend the same content unchanged.
Without an EKS access entry for the Agent Space IAM role, use_kubectl cannot reach the cluster: all 49 discovery areas return n/a with a permission error and the review completes almost entirely unassessed. Neither README covered this, so the most likely first-run failure had no documented cause. Add the setup steps and a link to the DevOps Agent user guide to both READMEs, and state the policy choice this review needs. The documented default, AmazonAIOpsAssistantPolicy, suits incident investigation, but a full review walks the whole cluster object graph — RBAC, admission webhooks, CRDs, StorageClasses, PDBs, NetworkPolicies, quotas, ServiceAccounts — and any object kind the policy misses is recorded N/A for lack of access rather than graded. Recommend AmazonEKSAdminViewPolicy with access scope Cluster, and note that namespace-scoped access hides cluster-scoped objects. Also flag that AmazonEKSAdminViewPolicy grants read access to all objects including Secrets. The skill never fetches Secret values and grades them on existence and metadata only, so the grant is broader than the agent uses and should be an explicit decision, with AmazonAIOpsAssistantPolicy as the narrower fallback.
Drop the sample-code disclaimer from both the skill and custom agent READMEs. For the custom agent this matches convention: no other custom agent in the repo carries a disclaimer. For the skill it diverges from CONTRIBUTING item 7, which asks for a non-production note in a skill's README, and from the 14 of 22 existing skills that carry one.
CONTRIBUTING item 7 requires a non-production note in a skill's README, so restore it. The custom agent README stays without one, matching the other custom agents in the repo, none of which carry a disclaimer.
udid-aws
added
ci-sweep
|
When running review on EKS cluster in us-east-1 with different name than retail-store-demo the agent first look for retail-store-demo and only than try the right cluster (for other region it is not happening). |
…ws#77 review) The published system prompt targeted retail-store-demo in us-east-1, so a run against a differently named cluster looked up the wrong cluster first. Workflow step 1 now ships CLUSTER_NAME and REGION placeholders that must be replaced at creation time, and adds an explicit priority rule: a run request that names a cluster is used directly, without looking up the saved default. Unreplaced placeholders fail fast with a clear cluster-not-found report. README creation step and behavior notes explain the placeholder edit with an example, and the changelog entry matches.
ams-thakkar
left a comment
ams-thakkar
left a comment
There was a problem hiding this comment.
#113 answers the question I had here — supersession is now explicit, and it also supplies the llms.txt entry this PR was missing. That closes two of my earlier concerns. Four things remain.
What needs to change
SKILL.md:21 routes users to a skill that does not exist. "For a quick health snapshot without grading, use aws-eks-healthdashboard instead" — that skill is not on main, not in this PR, and not in #113. It's referenced twice more in the CHANGELOG. This one matters more than the others because it sits in the runtime instruction file the agent reads, so the agent will hand a user a dead end. Either add the boundary without naming an unavailable skill, or land that skill first.
The naming decision is now cheap, and it won't be again. #113 deletes skills/eks-operation-review/, which frees the name that matches the rest of the family — ecs-operation-review, rds-operation-review, bedrock-operation-review. aws-eks-operations-review diverges on both the aws- prefix and the plural, and @shyamkulkarni renamed #42 four days ago for exactly this reason. Directory names are effectively permanent: they're the frontmatter name, the llms.txt path, and the published docs URL.
It also changes the merge order, so it's worth deciding before either lands:
- Keep the name: merge this first, then #113. #113's reference repointing is required, and its tree has no
aws-eks-operations-review, so merging it first would leavemainwith no EKS operations-review skill and four dangling references. - Rename to
eks-operation-review: merge #113 first as a pure deletion, then this one recreates the directory. The agent's existingeks-operation-reviewreference stays correct, so most of #113's repointing becomes unnecessary andllms.txtonly needs its description refreshed.
I'm not going to force the rename — but I'd like it stated as a decision rather than left as drift, and if you keep the name, say why in the CHANGELOG so the next person doesn't read it as an oversight.
Two agents would cover EKS review after both land. #113 repoints custom-agents/aws-operation-review/ at this skill, and this PR adds a dedicated custom-agents/aws-eks-operations-review/. Pick one, or state what distinguishes them. Relatedly, the PR body leaves "New custom agent" unchecked while adding 3 files and 507 lines under custom-agents/ — worth correcting so the description matches the diff.
references/docs/minimum-rbac.md:278 lists iam:SimulatePrincipalPolicy in the agent role policy, and AIDevOpsAgentAccessPolicy does not grant it. I checked against the live policy: the 23 iam: actions it allows do not include it. Check AX9 offers ListAttachedRolePolicies + GetRolePolicy as the primary path and both are granted, so this degrades rather than breaks — but the doc presents it as a flat requirement. Mark it optional, or note that the check falls back. #113 also moves this skill under "no extra policy needed" in the CFN summary, which is only true with that caveat.
Nits
Non-blocking:
- The CHANGELOG uses bare
## 1.9.3headings where the convention and the other skills use## [x.y.z] - YYYY-MM-DD.
Thanks
The safety posture is the strongest part and it's clearly deliberate: use_kubectl restricted to get, describe, logs, version, config current-context, cluster-info, top, and get --raw; Secret checks graded on existence, type, and metadata with values never fetched; remediations as proposals for human approval. Flagging in the README that AmazonEKSAdminViewPolicy grants Secret read access that the skill deliberately doesn't use, and saying it should be approved knowingly, is the right instinct — most contributors would have left that unsaid. Grading unassessable rows as N/A with the reason rather than dropping or guessing them is what makes a 288-check review trustworthy rather than noisy. And the explanation for omitting generated eval output — derived, reproducible, roughly 8 MB — is honest and correct rather than quietly skipped.
Verified myself: merges clean against current main and against #113; mkdocs build --strict passes with zero warnings across 103 markdown files; extensions are limited to md/json/yaml with the run-functional-serial.py referenced in the body correctly not committed; all 4 JSON files parse; name matches the directory; description is 783 characters; version: 1.9.3 matches the CHANGELOG top entry. On IAM, every AWS action the skill actually calls is covered by the managed policy — and the two mutating actions I checked, autoscaling:SetDesiredCapacity and elasticloadbalancing:CreateLoadBalancer, turned out to be check text describing what the cluster's own controller roles need, not agent calls.
Thanks to @yakiratz-aws for the domain review; EKS correctness rests there. My review is scoped to repository mechanics, the IAM surface, and the read-only boundary.
The AIDevOpsAgentAccessPolicy managed policy does not grant iam:SimulatePrincipalPolicy. The skill's AX9 check (Controller IAM permissions) uses ListAttachedRolePolicies + GetRolePolicy as the primary path and falls back gracefully without SimulatePrincipalPolicy. Separated it into its own IAMSimulateOptional statement and documented the degradation behavior alongside the existing optional Security Services and Service Quotas statements.
- Remove all aws-eks-healthdashboard references from SKILL.md and CHANGELOG.md (dead skill that no longer exists in the repository) - Add 'Relationship to aws-operation-review' section to custom-agent README clarifying the distinction between the two agents (different scope, skills, tools, and depth) - Update scope boundary text in SKILL.md to reference incident investigation skills instead of the removed healthdashboard skill Addresses blocking issues from review: 1. Dead skill reference (aws-eks-healthdashboard) - FIXED 2. Two-agents distinction - DOCUMENTED 3. SimulatePrincipalPolicy optional - already fixed in f76ad80 4. CHANGELOG date format - noted as non-blocking nit (repo-wide convention)
|
Thanks for the thorough review! I've addressed all blocking issues: 1. Dead skill reference (
2. Two-agents distinction (
3.
4. CHANGELOG date format — Noted as non-blocking nit
Ready for re-review. |
Add explicit rationale for keeping aws-eks-operations-review name instead of the family pattern (eks-operation-review) as requested by reviewer: 1. Fundamentally different depth (288 checks vs lightweight) 2. aws- prefix and plural -operations- signals distinction to routing This addresses the 'state it as a decision rather than drift' request.
|
Added one more commit (007c7e9): Naming decision documented in CHANGELOG — Added explicit rationale for keeping
This addresses the "state it as a decision rather than drift" request. |
Update all 29 version headings from '## x.y.z' to '## [x.y.z] - YYYY-MM-DD' format to match the repo convention as noted by reviewer.
|
Also fixed the nit (bb04e14): CHANGELOG date format — Updated all 29 version headings from All review comments now addressed. |
ams-thakkar
left a comment
ams-thakkar
left a comment
There was a problem hiding this comment.
Everything from my last review is verified fixed — the dead aws-eks-healthdashboard reference is gone, iam:SimulatePrincipalPolicy is optional with the fallback documented, the naming decision is recorded, and all 29 CHANGELOG headings are converted. Re-ran the checks too: merges clean against main and #113, mkdocs build --strict passes with zero warnings, and version: 1.9.3 matches the CHANGELOG top entry.
One thing to undo, and it's my fault for how I phrased the last round. I wrote "pick one, or state what distinguishes them," which invited the comparison table at custom-agents/aws-eks-operations-review/README.md:11. Please drop it — eks-operation-review is being deprecated in #113, so there's nothing left to compare against, and this repo has no lightweight/heavyweight tier for a table to describe. If you want something in its place, two lines is plenty, and the real reason is that this skill needs tools the shared aws-operation-review agent isn't provisioned with plus the per-pillar artifact split — not depth.
Three small factual fixes worth making while you're in there, all non-blocking but all in docs that will outlive the PR:
- The CHANGELOG naming rationale says the
aws-prefix and plural "signal this distinction to ... the agent's routing logic". Routing keys off thedescriptionfield, not the directory name, so that half of the reasoning doesn't hold. references/docs/minimum-rbac.md:355attributes coverage of the AWS IAM actions toAmazonAIOpsAssistantPolicy, which this same skill uses as an EKS access-entry policy atREADME.md:17. Those govern different layers — Kubernetes RBAC versus AWS IAM — and the policy that actually grants those actions,AIDevOpsAgentAccessPolicy, is never named in the skill.- The agent README lists
get_skill_resource_manifest/get_skill_resourceas "Required — the agent cannot start without these." The oldeks-operation-reviewcarried reference files and loaded them fine under a two-tool agent, and those tools appear nowhere else in the repo, so that looks overstated.
- CHANGELOG: Remove incorrect claim that routing keys off directory name (routing uses the description field, not the directory name) - minimum-rbac.md: Correct policy name from AmazonAIOpsAssistantPolicy to AIDevOpsAgentAccessPolicy (former is EKS access policy for K8s RBAC, latter is the IAM policy granting AWS API actions) - Agent README: Soften 'Required' to 'Recommended' for skill resource tools (agents without reference files work fine without these tools)
|
Fixed the three factual issues:
|
The comparison to aws-operation-review adds clutter without helping users decide which to use — the purpose section already makes the scope clear.
|
Dropped the comparison table at |
ams-thakkar
left a comment
ams-thakkar
left a comment
There was a problem hiding this comment.
Table's gone cleanly — 14 deletions, nothing substituted, and the multi-artifact rationale stays as the agent's stated design decision, which was the real justification anyway. The three factual fixes are all correct too, and your note on the two policy layers is exactly right.
Verified at d1fe78a0: merges clean against main and #113 with zero conflicts, mkdocs build --strict passes with zero warnings across 103 markdown files, extensions limited to md/json/yaml, version: 1.9.3 matches the CHANGELOG top entry, and no lightweight/heavyweight framing or references to the deprecated skill remain anywhere under custom-agents/.
Approving as scoped maintainer approval — repository mechanics, the IAM surface against live AIDevOpsAgentAccessPolicy, and the read-only boundary. EKS correctness rests on @yakiratz-aws's review; their approval was dismissed by the intervening pushes, but those pushes were documentation-only.
Merge this before #113 — that PR's tree has no aws-eks-operations-review yet points llms.txt, SYSTEM_PROMPT.md, and the setup README at it.
4 participants
Description
Adds
skills/aws-eks-operations-review, a read-only DevOps Agent skill that grades an Amazon EKS cluster against best practices and returns evidence-backed scorecards with prioritized remediations.Coverage is nine pillars — Operations, Resilience, Security, Scalability, Performance, Observability, Networking, Cost, Control Plane — plus AWS API and Cluster Insights rows. Every row is graded PASS / FAIL / N/A against an observed result; a row that cannot be assessed stays in the report as N/A with the exact reason rather than being dropped or guessed.
SKILL.mddrives an S0–S8 state machine and loads references just-in-time to bound context. Remediation shards load only after a FAIL verdict exists, and decision trees only after their matching signal.Contents
references/runtime/— discovery manifest, inventory schema, router, cluster gates, grading guards, check manifest, telemetry thresholds, common-check crosswalk, QA gate, report contractreferences/pillars/— nine canonical pillar definitionsreferences/remediations/— FAIL-only dispatch index plus 51 shardsreferences/control-plane-health/— metric sources, four Logs Insights query shards, thresholds, playbooksreferences/decision-trees/— pending pods, OOMKilled, API latency / 429references/docs/— operator and background material, explicitly not runtime authoritySafety posture
All access is read-only.
use_kubectlis limited toget,describe,logs,version,config current-context,cluster-info,top, andget --raw. Kubernetes Secret values are never fetched. Remediations are proposals for human approval, never mutations. Customer-account reads go through audited read-only agent access rather than local CLI or boto3 credentials.Not included
Generated eval run output under
evals/functional,evals/best-practices, andevals/structureis omitted. It is derived output —evals/run-functional-serial.pywrites into those paths and prunes the same three when building a workspace — so committing it would add roughly 8 MB of reproducible artifacts. Eval definitions and fixtures (evals.json,eval_queries.json,evals/files/) are included.Type of change
Testing
Validated read-only against a live EKS cluster (
retail-store-demo, us-east-1) in a non-production account, producing a full review across all nine pillars plus AWS API and Cluster Insights rows, with the QA coverage gate passing before the report was rendered.Structure and consistency suites live in
evals/and are documented inevals/TESTING.md. Per that file, model-matrix and live-validation results should not be claimed until the suites are actually re-run, so this PR states only the live review above.Repo conventions verified before submitting:
SKILL.mdfrontmatter parses as YAML and carries the requiredmetadatablock withauthorandversion; the skill README leads with the non-production disclaimer; the skill directory hasSKILL.md,README.md, andCHANGELOG.md. All internal reference links resolve, and no reference file is orphaned.License confirmation