Repository navigation
fix(aws): record the pending endpoint update id and count every EKS tag - #87
Closed
tim-thacker-nullify wants to merge 2 commits into
Closed
tim-thacker-nullify wants to merge 2 commits into
tim-thacker-nullify wants to merge 2 commits into
Conversation
Three round-4 review lows on the managed EKS scan script. - apply records the id update-cluster-config returns in nullify-pending-update, and the in-progress guard describes that one update instead of trusting list-updates to already show an update accepted seconds earlier - a describe-update that fails with ResourceNotFoundException is skipped rather than aborting the run; other failures still die - only a record written without an update id falls back to describing every update list-updates returns - the tag-slot precheck counts every tag key, aws: ones included, through the same keys() query load_cidr_records uses instead of an untested JMESPath filter; the records now need three free slots Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…lcheck SC2178/SC2128: keys is an array elsewhere in the script. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Member
Author
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.
Round-4 review fixes for #79: R4-79-1, R4-79-2 and R4-79-3 on
setup-eks-managed-scan.sh.applyrecords the idupdate-cluster-configreturns in anullify-pending-updatecluster tag, straight after the call. The in-progress guard describes that one update instead of trustingListUpdates, whose consistency AWS does not document. The tag is dropped with the pending CIDR record.ListUpdatesreturns, so the common path is one call, not one per historical update. Adescribe-updatefailing withResourceNotFoundExceptionis skipped with a warning; every other failure still dies.aws:ones included, and reads them through the samekeys(cluster.tags)queryload_cidr_recordsuses. That drops thelength(keys(...)[?!starts_with(@, 'aws:')])expression no test executed, and the assumption thataws:tags are exempt from the 50-tag quota. The records now need three free slots.Tests: four new bats cases (the id is recorded between the update and the wait; a recorded InProgress update stops
applywithout callinglist-updates; a forgotten id does not;removedrops a stale id tag), plus one pinning thataws:tags count. The two tag-limit cases move from two free slots to three. The fake CLI now describesupdate-1, the id it hands out, so the recovery cases take the real path.Still open from round 3: R3-74-3, the fallback RBAC manifest is still fetched from the mutable tag
nullify-k8s-readonly-access-v0.1.0with no commit pin and noRBAC_MANIFEST_SHA256, so a moved tag makesremovedelete a different object set thanapplycreated. The tag still does not exist; pinning needs it created, which needs PR #84.🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.