Repository navigation
chore: Remove duplicate eks-operation-review skill - #113
ams-thakkar merged 1 commit into
Conversation
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
left a comment
There was a problem hiding this comment.
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.
|
Thanks for the thorough review! Merge order: Understood — will hold this until #77 lands. SimulatePrincipalPolicy fix: Done. Pushed f76ad80 to #77 which:
This addresses the caveat: the llms.txt line in this PR listing |
ams-thakkar
left a comment
There was a problem hiding this comment.
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.
Description
Remove the duplicate
eks-operation-reviewskill which is superseded by the more comprehensiveaws-eks-operations-reviewskill.Why this change?
The repository currently has two EKS operations review skills:
eks-operation-review(removed)aws-eks-operations-review(kept)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
skills/eks-operation-review/directory (17 files)llms.txtto referenceaws-eks-operations-reviewcustom-agents/aws-operation-review/to useaws-eks-operations-reviewskillcloudformation/devops-agent-skill-policies.yamlcomment to listaws-eks-operations-reviewandeks-upgrade-readinessMigration for users
Users currently using
eks-operation-reviewshould switch toaws-eks-operations-review. The new skill:use_kubectltool for K8s discoveryType of change
Testing
eks-operation-revieware updatedllms.txtcorrectly listsaws-eks-operations-reviewLicense confirmation