Repository navigation
fix(iac): let member-account roles tag purchased EC2 reservations - #773
Conversation
- Outside the host account the purchase idempotency tag was never written: the member-account purchase roles had no ec2:CreateTags and no Redshift tag actions, the failure is only logged, and the pre-buy tag guard then finds no tag and can buy again on a retry. - Add ec2:CreateTags scoped to arn:aws:ec2:*:*:reserved-instances/* as its own statement, and redshift:CreateTags and redshift:DescribeTags on "*" (no reserved-node resource type exists for those actions), to all six federation templates (cross-account and target, Terraform and CloudFormation, plus both CLI onboarding scripts) and to the legacy CUDly-CrossAccount stack. The CLI heredocs get a second statement so ec2:CreateTags is not widened to every EC2 resource. - Tests: two parity-script drift cases (dropping ec2:CreateTags from target Terraform, dropping redshift:DescribeTags from the WIF CLI template) and a Go test that pins the scoping in both CLI templates; the scoping test fails if ec2:CreateTags is added to the flat list. - Existing member accounts keep their deployed role until the template is re-applied. Refs #702
|
Warning Review limit reached
This review includes 9 billable files and costs up to $2.25.
Reviews can continue after your included limit without a manual trigger. An admin must approve usage-based billing. Or wait 20 minutes for your next included review. View limit detailsLimit details: You’ve used the included review currently available. Your 86 included PR review attempts over the past 7 days set your current allowance at 1 review per hour. Review configuration: ⚙️ Run configuration
📒 Files selected for processing (9)
Comment |
|
Subject: PR 773 (#702 IaC half) @ d09ff5a - REQUEST CHANGES (1 CI blocker), design verified BLOCKER L1: CI "Lint Code" FAILED at this SHA: internal/iacfiles/templates_aws_tagging_test.go:50 QF1002 "could use tagged switch on action" (staticcheck). Your new test file. Fix and push; I re-review the delta. (Also mergeStateStatus UNKNOWN, other checks still running when I looked.) Verified at the SHA (scratch copy of the PR tree via git archive; no worktree, no cloud calls): |
- Lint Code reported QF1002 on the untagged switch in TestAWSCLITemplatesTagReservedInstancesOnly; switch on the action string instead. No behavior change.
|
Subject: PR 773 delta d09ff5a..40e38a6 - L1 CLOSED, APPROVE conditional on green CI Delta is exactly one file (internal/iacfiles/templates_aws_tagging_test.go, +3/-3): |
…policies - Coordinator least-privilege review of PR 773: redshift:CreateTags on "*" has no condition and Redshift may not support tagging reserved nodes at all, so it could add risk and fix nothing. Remove redshift:CreateTags and redshift:DescribeTags from the six member account policies and the legacy CUDly-CrossAccount stack. The Redshift part of #702 stays open until it is verified against a real account. - The EC2 grant is unchanged: ec2:CreateTags on arn:aws:ec2:*:*:reserved-instances/*, no CreateAction condition. - The scoping test now reads all seven policy files (both CLI heredocs, the four federation templates and the legacy stack) and asserts ec2:CreateTags appears once, scoped to reserved instances, and that redshift:CreateTags is absent. It fails when ec2:CreateTags is widened to "*" in a Terraform file, a CloudFormation file or the legacy stack, and when redshift:CreateTags is reintroduced. - Parity drift case 7 now drops ec2:CreateTags from the target CloudFormation template. Refs #702
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Merge note: squash-merged at 7e2a3d8, matched head c09d4ad. Review: Opus least-privilege review APPROVE at 9c315d2; delta 9c315d2..c09d4ad is a test-only prealloc lint fix, checked by the coordinator. CI: 25/25 pass, state CLEAN, no unresolved defect threads. Local verification on the archived head (real test runs, not mocks of the IAM files): go test ./internal/iacfiles/... exit 0 (187 passed); scripts/test-aws-iam-parity.sh exit 0 (7 passed, 0 failed). |
Summary
Part of #702, EC2 half only. The member-account purchase roles had no
ec2:CreateTags. The Go client tags the reserved instance it just bought with the purchase idempotency tag; in a member account that call was denied, the failure is only logged, and the pre-buy tag guard then finds no tag and can buy again on a retry.Added to all six federation templates (cross-account and target, Terraform and CloudFormation, plus
aws-cross-account-cli.sh.tmplandaws-wif-cli.sh.tmpl) and to the legacycloudformation/stacks/CUDly-CrossAccount/template.yaml:ec2:CreateTagsin its own statement,Resource: arn:aws:ec2:*:*:reserved-instances/*, noec2:CreateActioncondition (PurchaseReservedInstancesOfferingtakes noTagSpecifications, so a standaloneCreateTagscarries noCreateActionand the call would be denied with one). In the CLI templates, which had one flatResource: "*"statement, this is a second statement so the grant is not widened to every EC2 resource.Redshift is NOT part of this PR
An earlier version of this PR also granted
redshift:CreateTagsandredshift:DescribeTagson*. The coordinator's least-privilege review (Opus, at 40e38a6) found that grant unconditioned, and that Redshift may not support tagging reserved nodes at all (the AWS Service Authorization Reference lists noreservednoderesource type), so it could add risk and fix nothing. Those actions are removed from all seven policy files. The Redshift part of #702 stays open until it is verified against a real account (a sandboxCreateTags/DescribeTagscall against a reserved-node ARN, owner decision 21). #779 (host runtime role Redshift tags) depends on the same answer.What is verified
Run locally (macOS), no applies and no cloud calls:
scripts/check-aws-iam-parity.shpasses;scripts/test-aws-iam-parity.sh7/7 (two drift cases: droppingec2:CreateTagsfrom the target Terraform and from the target CloudFormation template).TestMemberAccountPoliciesTagReservedInstancesOnlyreads all seven policy files (both CLI heredocs, the four federation templates, the legacy stack) and assertsec2:CreateTagsappears once, scoped toreserved-instances/*, andredshift:CreateTagsis absent. Mutants, each run: wideningec2:CreateTagsto*in a Terraform file, in a CloudFormation file and in the legacy stack all fail it; reintroducingredshift:CreateTagsfails it.go test ./internal/iacfilespasses;go test ./internal/api -run Federationpasses;terraform fmt -checkclean.Known limits
ec2:CreateTagsis scoped by resource type only; it can still retag reserved instances the customer did not buy through CUDly. A tag-key condition (aws:TagKeysplusaws:RequestTag/Tool) was left out by decision of the coordinator review.ec2/client.go:203-205, plus platform handling); OpenSearchAddTagsis granted by no template (INF-10: es:AddTags granted by no IaC template although code performs post-purchase OpenSearch RI tagging #42).-race, no full-repo test run locally; CI does the full run.Review history
deployment-investigation-kimi: design approved at d09ff5a, lint blocker QF1002 fixed at 40e38a6, delta approved. Coordinator Opus least-privilege review at 40e38a6: request changes (remove the Redshift half, extend the scoping test to all seven files), done at 9c315d2 as a new commit.
Refs #702