Skip to content

chore: Remove duplicate eks-operation-review skill - #113

Merged
ams-thakkar merged 1 commit into
aws:mainfrom
shyamkulkarni:chore/remove-duplicate-eks-operation-review
Sep 29, 2026
Merged

ams-thakkar merged 1 commit into
aws:mainfrom
shyamkulkarni:chore/remove-duplicate-eks-operation-review

Conversation

@shyamkulkarni

Copy link
Copy Markdown
Contributor

Description

Remove the duplicate eks-operation-review skill which is superseded by the more comprehensive aws-eks-operations-review skill.

Why this change?

The repository currently has two EKS operations review skills:

Skill Checks Features
eks-operation-review (removed) Basic 12 sections Simple report
aws-eks-operations-review (kept) 288 core + 81 gated 9 pillars, PASS/FAIL/N/A grading, QA gates, remediation shards

Having both confuses users about which to choose. The comprehensive version (aws-eks-operations-review) is the clear winner and should be the only EKS operations review skill.

Changes

  • Deleted: skills/eks-operation-review/ directory (17 files)
  • Updated: llms.txt to reference aws-eks-operations-review
  • Updated: custom-agents/aws-operation-review/ to use aws-eks-operations-review skill
  • Updated: cloudformation/devops-agent-skill-policies.yaml comment to list aws-eks-operations-review and eks-upgrade-readiness

Migration for users

Users currently using eks-operation-review should switch to aws-eks-operations-review. The new skill:

  • Covers the same 12 best-practices sections plus more
  • Uses the same use_kubectl tool for K8s discovery
  • Produces more detailed reports with PASS/FAIL grading
  • Includes remediation guidance for failures

Type of change

  • New skill
  • New custom agent
  • Update to an existing skill or agent
  • Documentation or infrastructure change

Testing

  • Verified all references to eks-operation-review are updated
  • Verified llms.txt correctly lists aws-eks-operations-review
  • Verified custom agent references are updated

License confirmation

  • By submitting this pull request, I confirm that my contribution is made under the terms of the Apache License 2.0.

Remove the older eks-operation-review skill which is superseded by the more
comprehensive aws-eks-operations-review skill (288 checks vs basic coverage).

Changes:
- Delete skills/eks-operation-review/ directory
- Update llms.txt to reference aws-eks-operations-review
- Update aws-operation-review custom agent to use aws-eks-operations-review
- Update cloudformation/devops-agent-skill-policies.yaml comment

The aws-eks-operations-review skill provides:
- 288 core checks + 81 gated checks
- 9 pillars (Operations, Resilience, Security, Scalability, Performance,
  Observability, Networking, Cost, Control Plane)
- PASS/FAIL/N/A grading with QA gates
- Extensive remediation shards

This consolidation reduces user confusion about which EKS skill to choose.

@ams-thakkar ams-thakkar left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do not merge this before #77. This PR's tree contains no aws-eks-operations-review, but it repoints llms.txt, the agent's SYSTEM_PROMPT.md ("For EKS clusters: use the aws-eks-operations-review skill methodology"), the setup step in README.md, and the CFN summary comment at it. Merged first, main would carry zero EKS operations-review skills and four references to one that doesn't exist. Merge #77, then this.

Otherwise this is exactly the right cleanup, and it settles the question I raised on #77 — supersession is now stated rather than implied. Nice that llms.txt gets a real replacement entry rather than just losing a line; that also closes the missing-entry finding from my #77 review. The remaining eks-operation-review mentions after this land are both in CHANGELOGs, which is correct — those are history.

Verified: 12 deletions all under skills/eks-operation-review/ and 5 modifications, nothing outside the intended set; the CloudFormation edit is a single comment line in SkillPolicySummary with no parameter, condition, or policy resource touched; the agent CHANGELOG is bumped to 1.1.0 with the migration noted; no conflicts against current main, and none between this and #77.

One thing to fix while you're here: this moves aws-eks-operations-review under "Skills covered by AIDevOpsAgentAccessPolicy (no extra policy needed)", but #77's references/docs/minimum-rbac.md:278 asks for iam:SimulatePrincipalPolicy in the agent role policy, and the managed policy does not grant it. The check that uses it offers ListAttachedRolePolicies + GetRolePolicy as the primary path and both are granted, so it degrades rather than breaks — but either that doc should mark it optional or this line should carry the caveat.

Approving as scoped maintainer approval on the mechanics of the removal and the reference updates. Holding the merge on #77 landing first.

@shyamkulkarni

Copy link
Copy Markdown
Contributor Author

Thanks for the thorough review!

Merge order: Understood — will hold this until #77 lands.

SimulatePrincipalPolicy fix: Done. Pushed f76ad80 to #77 which:

  1. Separates iam:SimulatePrincipalPolicy into its own IAMSimulateOptional statement in the minimum-rbac policy JSON
  2. Documents the degradation behavior alongside the existing optional Security Services and Service Quotas statements
  3. Clarifies that the skill falls back to ListAttachedRolePolicies + GetRolePolicy when SimulatePrincipalPolicy isn't granted — same findings, slightly less authoritative

This addresses the caveat: the llms.txt line in this PR listing aws-eks-operations-review under "no extra policy needed" remains accurate because the skill works fully on the managed policy (the optional permission just enhances one check).

@ams-thakkar ams-thakkar left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My approval stands — this is a clean removal and I'm not asking for changes. Two notes for whoever merges.

Merge order: #77 has to land first. This PR's tree contains no aws-eks-operations-review, but it points llms.txt, SYSTEM_PROMPT.md, the setup step in README.md, and the CFN summary comment at it. Merged first, main would carry zero EKS operations-review skills and four references to one that doesn't exist.

Worth a follow-up, not a change here: this repoints aws-operation-review at the new skill, but that agent's setup step provisions only two tools — "Add the use_aws and use_kubectl tools to this custom agent" — while the skill it now points at calls query_cloudwatch_logs for the CP01–CP25 control-plane queries, the artifact tools for its per-pillar output, plus verify_aws_claim, lookup_cloudtrail_events, X-Ray, and Trusted Advisor. Anyone following that README would get a review that grades those rows N/A for missing tooling — degraded rather than broken, since the skill reports unassessable rows with a reason by design.

The cleanest resolution is probably dropping EKS from the multi-service agent altogether and letting the dedicated aws-eks-operations-review agent own it, which also settles the two-agent overlap. That's a judgement about scope rather than a defect in this PR, so I'd leave it out of here unless you'd rather fold it in.

For the record, what I verified: 12 deletions all confined to skills/eks-operation-review/ with five modifications and nothing outside the intended set; the CloudFormation edit is a single comment line in SkillPolicySummary with no parameter, condition, or policy resource touched; the agent CHANGELOG is bumped to 1.1.0 with the migration recorded; the surviving eks-operation-review mentions are both in CHANGELOGs, which is correct as history; and no conflicts against main or #77. Replacing the llms.txt line with a real entry for the successor rather than just deleting it also closed the missing-entry finding from my #77 review.

@ams-thakkar
ams-thakkar merged commit 301ee08 into aws:main Sep 29, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants