From d09ff5ab698c69a509dbc95d7d0faf991c5c4418 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Fri, 9 Oct 2026 14:35:25 +0200 Subject: [PATCH 1/4] fix(iac): let member-account roles tag purchased reservations - 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 --- .../stacks/CUDly-CrossAccount/template.yaml | 8 +++ .../cloudformation/template.yaml | 8 +++ .../aws-cross-account/terraform/main.tf | 9 +++ .../aws-target/cloudformation/template.yaml | 8 +++ iac/federation/aws-target/terraform/main.tf | 9 +++ .../templates/aws-cross-account-cli.sh.tmpl | 6 +- .../iacfiles/templates/aws-wif-cli.sh.tmpl | 6 +- .../iacfiles/templates_aws_tagging_test.go | 66 +++++++++++++++++++ scripts/test-aws-iam-parity.sh | 18 +++++ 9 files changed, 136 insertions(+), 2 deletions(-) create mode 100644 internal/iacfiles/templates_aws_tagging_test.go diff --git a/cloudformation/stacks/CUDly-CrossAccount/template.yaml b/cloudformation/stacks/CUDly-CrossAccount/template.yaml index 15c88f740..fe9d44a90 100644 --- a/cloudformation/stacks/CUDly-CrossAccount/template.yaml +++ b/cloudformation/stacks/CUDly-CrossAccount/template.yaml @@ -105,6 +105,11 @@ Resources: - ec2:DescribeRegions - ec2:DescribeInstanceTypeOfferings Resource: "*" + - Sid: EC2ReservedInstanceTagging + Effect: Allow + Action: + - ec2:CreateTags + Resource: "arn:aws:ec2:*:*:reserved-instances/*" # RDS Reserved Instances - Sid: RDSReservedInstances @@ -141,6 +146,9 @@ Resources: - redshift:DescribeReservedNodeOfferings - redshift:DescribeReservedNodes - redshift:PurchaseReservedNodeOffering + # No reserved-node resource type exists for the two tag actions (AWS Service Authorization Reference), so "*" is the narrowest scope. + - redshift:CreateTags + - redshift:DescribeTags Resource: "*" # MemoryDB Reserved Nodes diff --git a/iac/federation/aws-cross-account/cloudformation/template.yaml b/iac/federation/aws-cross-account/cloudformation/template.yaml index 750785ef9..5e63158c5 100644 --- a/iac/federation/aws-cross-account/cloudformation/template.yaml +++ b/iac/federation/aws-cross-account/cloudformation/template.yaml @@ -86,6 +86,11 @@ Resources: - ec2:AcceptReservedInstancesExchangeQuote - ec2:DescribeRegions Resource: "*" + - Sid: EC2ReservedInstanceTagging + Effect: Allow + Action: + - ec2:CreateTags + Resource: "arn:aws:ec2:*:*:reserved-instances/*" - Sid: RDSReservations Effect: Allow Action: @@ -106,6 +111,9 @@ Resources: - redshift:PurchaseReservedNodeOffering - redshift:DescribeReservedNodeOfferings - redshift:DescribeReservedNodes + # No reserved-node resource type exists for the two tag actions (AWS Service Authorization Reference), so "*" is the narrowest scope. + - redshift:CreateTags + - redshift:DescribeTags Resource: "*" - Sid: MemoryDBReservations Effect: Allow diff --git a/iac/federation/aws-cross-account/terraform/main.tf b/iac/federation/aws-cross-account/terraform/main.tf index 206c1ed9d..eb574ccd7 100644 --- a/iac/federation/aws-cross-account/terraform/main.tf +++ b/iac/federation/aws-cross-account/terraform/main.tf @@ -86,6 +86,12 @@ resource "aws_iam_policy" "cudly" { ] Resource = "*" }, + { + Sid = "EC2ReservedInstanceTagging" + Effect = "Allow" + Action = ["ec2:CreateTags"] + Resource = "arn:aws:ec2:*:*:reserved-instances/*" + }, { Sid = "RDSReservations" Effect = "Allow" @@ -113,6 +119,9 @@ resource "aws_iam_policy" "cudly" { "redshift:PurchaseReservedNodeOffering", "redshift:DescribeReservedNodeOfferings", "redshift:DescribeReservedNodes", + # No reserved-node resource type exists for the two tag actions (AWS Service Authorization Reference), so "*" is the narrowest scope. + "redshift:CreateTags", + "redshift:DescribeTags", ] Resource = "*" }, diff --git a/iac/federation/aws-target/cloudformation/template.yaml b/iac/federation/aws-target/cloudformation/template.yaml index 7b2546163..5c0094f32 100644 --- a/iac/federation/aws-target/cloudformation/template.yaml +++ b/iac/federation/aws-target/cloudformation/template.yaml @@ -200,6 +200,11 @@ Resources: - ec2:AcceptReservedInstancesExchangeQuote - ec2:DescribeRegions Resource: "*" + - Sid: EC2ReservedInstanceTagging + Effect: Allow + Action: + - ec2:CreateTags + Resource: "arn:aws:ec2:*:*:reserved-instances/*" - Sid: RDSReservations Effect: Allow Action: @@ -220,6 +225,9 @@ Resources: - redshift:PurchaseReservedNodeOffering - redshift:DescribeReservedNodeOfferings - redshift:DescribeReservedNodes + # No reserved-node resource type exists for the two tag actions (AWS Service Authorization Reference), so "*" is the narrowest scope. + - redshift:CreateTags + - redshift:DescribeTags Resource: "*" - Sid: MemoryDBReservations Effect: Allow diff --git a/iac/federation/aws-target/terraform/main.tf b/iac/federation/aws-target/terraform/main.tf index 01f39e81e..4b92bd1a3 100644 --- a/iac/federation/aws-target/terraform/main.tf +++ b/iac/federation/aws-target/terraform/main.tf @@ -73,6 +73,12 @@ resource "aws_iam_policy" "cudly" { ] Resource = "*" }, + { + Sid = "EC2ReservedInstanceTagging" + Effect = "Allow" + Action = ["ec2:CreateTags"] + Resource = "arn:aws:ec2:*:*:reserved-instances/*" + }, { Sid = "RDSReservations" Effect = "Allow" @@ -100,6 +106,9 @@ resource "aws_iam_policy" "cudly" { "redshift:PurchaseReservedNodeOffering", "redshift:DescribeReservedNodeOfferings", "redshift:DescribeReservedNodes", + # No reserved-node resource type exists for the two tag actions (AWS Service Authorization Reference), so "*" is the narrowest scope. + "redshift:CreateTags", + "redshift:DescribeTags", ] Resource = "*" }, diff --git a/internal/iacfiles/templates/aws-cross-account-cli.sh.tmpl b/internal/iacfiles/templates/aws-cross-account-cli.sh.tmpl index e2aad0d7a..4a529613a 100644 --- a/internal/iacfiles/templates/aws-cross-account-cli.sh.tmpl +++ b/internal/iacfiles/templates/aws-cross-account-cli.sh.tmpl @@ -46,7 +46,7 @@ PERMISSIONS_POLICY=$(cat <<'JSON' "elasticache:PurchaseReservedCacheNodesOffering", "elasticache:DescribeReservedCacheNodesOfferings","elasticache:DescribeReservedCacheNodes", "redshift:PurchaseReservedNodeOffering","redshift:DescribeReservedNodeOfferings", - "redshift:DescribeReservedNodes", + "redshift:DescribeReservedNodes","redshift:CreateTags","redshift:DescribeTags", "memorydb:PurchaseReservedNodesOffering","memorydb:DescribeReservedNodesOfferings", "memorydb:DescribeReservedNodes", "savingsplans:CreateSavingsPlan","savingsplans:DescribeSavingsPlans", @@ -55,6 +55,10 @@ PERMISSIONS_POLICY=$(cat <<'JSON' "es:DescribeReservedInstances", "ec2:DescribeRegions" ] + },{ + "Effect": "Allow", + "Resource": "arn:aws:ec2:*:*:reserved-instances/*", + "Action": ["ec2:CreateTags"] }] } JSON diff --git a/internal/iacfiles/templates/aws-wif-cli.sh.tmpl b/internal/iacfiles/templates/aws-wif-cli.sh.tmpl index 0929ae15c..f119a1467 100644 --- a/internal/iacfiles/templates/aws-wif-cli.sh.tmpl +++ b/internal/iacfiles/templates/aws-wif-cli.sh.tmpl @@ -115,7 +115,7 @@ PERMISSIONS_POLICY=$(cat <<'JSON' "elasticache:PurchaseReservedCacheNodesOffering", "elasticache:DescribeReservedCacheNodesOfferings","elasticache:DescribeReservedCacheNodes", "redshift:PurchaseReservedNodeOffering","redshift:DescribeReservedNodeOfferings", - "redshift:DescribeReservedNodes", + "redshift:DescribeReservedNodes","redshift:CreateTags","redshift:DescribeTags", "memorydb:PurchaseReservedNodesOffering","memorydb:DescribeReservedNodesOfferings", "memorydb:DescribeReservedNodes", "savingsplans:CreateSavingsPlan","savingsplans:DescribeSavingsPlans", @@ -124,6 +124,10 @@ PERMISSIONS_POLICY=$(cat <<'JSON' "es:DescribeReservedInstances", "ec2:DescribeRegions" ] + },{ + "Effect": "Allow", + "Resource": "arn:aws:ec2:*:*:reserved-instances/*", + "Action": ["ec2:CreateTags"] }] } JSON diff --git a/internal/iacfiles/templates_aws_tagging_test.go b/internal/iacfiles/templates_aws_tagging_test.go new file mode 100644 index 000000000..917a926e2 --- /dev/null +++ b/internal/iacfiles/templates_aws_tagging_test.go @@ -0,0 +1,66 @@ +package iacfiles + +import ( + "encoding/json" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +const reservedInstanceARN = "arn:aws:ec2:*:*:reserved-instances/*" + +type awsPolicyStatement struct { + Effect string `json:"Effect"` + Action []string `json:"Action"` + Resource string `json:"Resource"` +} + +// permissionsPolicy extracts the PERMISSIONS_POLICY heredoc from a rendered- +// independent copy of an AWS CLI onboarding template (the heredoc is quoted and +// holds no template placeholders). +func permissionsPolicy(t *testing.T, path string) []awsPolicyStatement { + t.Helper() + raw, err := Templates.ReadFile(path) + require.NoError(t, err) + _, rest, found := strings.Cut(string(raw), "PERMISSIONS_POLICY=$(cat <<'JSON'\n") + require.True(t, found, "PERMISSIONS_POLICY heredoc not found in %s", path) + body, _, found := strings.Cut(rest, "\nJSON\n") + require.True(t, found, "PERMISSIONS_POLICY heredoc terminator not found in %s", path) + var doc struct { + Statement []awsPolicyStatement `json:"Statement"` + } + require.NoError(t, json.Unmarshal([]byte(body), &doc)) + return doc.Statement +} + +// TestAWSCLITemplatesTagReservedInstancesOnly pins #702: the member-account +// role can tag the reserved instance it just bought (the purchase idempotency +// tag), but only on reserved-instances resources, never on every EC2 resource. +func TestAWSCLITemplatesTagReservedInstancesOnly(t *testing.T) { + for _, path := range []string{ + "templates/aws-cross-account-cli.sh.tmpl", + "templates/aws-wif-cli.sh.tmpl", + } { + t.Run(path, func(t *testing.T) { + var scoped, wildcardRedshiftTags int + for _, stmt := range permissionsPolicy(t, path) { + for _, action := range stmt.Action { + switch { + case action == "ec2:CreateTags": + assert.Equal(t, reservedInstanceARN, stmt.Resource, + "ec2:CreateTags must be scoped to reserved instances") + scoped++ + case action == "redshift:CreateTags" || action == "redshift:DescribeTags": + assert.Equal(t, "*", stmt.Resource, + "redshift tag actions have no reserved-node resource type, so they are granted on *") + wildcardRedshiftTags++ + } + } + } + assert.Equal(t, 1, scoped, "ec2:CreateTags must appear exactly once") + assert.Equal(t, 2, wildcardRedshiftTags, "redshift:CreateTags and redshift:DescribeTags must each appear once") + }) + } +} diff --git a/scripts/test-aws-iam-parity.sh b/scripts/test-aws-iam-parity.sh index e56b273d1..a57afa5c8 100755 --- a/scripts/test-aws-iam-parity.sh +++ b/scripts/test-aws-iam-parity.sh @@ -92,6 +92,24 @@ run_case "missing compared file exits 2" 2 --root "$FIX4" # unbound variable under set -u. run_case "--root without a value exits 2" 2 --root +# Case 6 (#702): a federation file that loses the post-purchase tagging grant +# must fail, otherwise the purchase idempotency tag silently stops being +# written in member accounts. +FIX6="$TMP_BASE/drift-createtags" +seed_fixture "$FIX6" +grep -v 'ec2:CreateTags' \ + "$REPO_ROOT/iac/federation/aws-target/terraform/main.tf" \ + > "$FIX6/iac/federation/aws-target/terraform/main.tf" +run_case "federation drift (target TF missing ec2:CreateTags) exits 1" 1 --root "$FIX6" + +# Case 7 (#702): same for the Redshift tag actions in a CLI onboarding template. +FIX7="$TMP_BASE/drift-redshift-tags" +seed_fixture "$FIX7" +sed 's/redshift:DescribeTags/redshift-removed/' \ + "$REPO_ROOT/internal/iacfiles/templates/aws-wif-cli.sh.tmpl" \ + > "$FIX7/internal/iacfiles/templates/aws-wif-cli.sh.tmpl" +run_case "federation drift (WIF CLI missing redshift:DescribeTags) exits 1" 1 --root "$FIX7" + echo "" echo "Results: ${pass} passed, ${fail} failed." [[ "$fail" -eq 0 ]] From 40e38a629609246dc0575f347ad366ababacd754 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Fri, 9 Oct 2026 14:51:02 +0200 Subject: [PATCH 2/4] test(iacfiles): use a tagged switch in the tagging scope test - Lint Code reported QF1002 on the untagged switch in TestAWSCLITemplatesTagReservedInstancesOnly; switch on the action string instead. No behavior change. --- internal/iacfiles/templates_aws_tagging_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/iacfiles/templates_aws_tagging_test.go b/internal/iacfiles/templates_aws_tagging_test.go index 917a926e2..70accaff1 100644 --- a/internal/iacfiles/templates_aws_tagging_test.go +++ b/internal/iacfiles/templates_aws_tagging_test.go @@ -47,12 +47,12 @@ func TestAWSCLITemplatesTagReservedInstancesOnly(t *testing.T) { var scoped, wildcardRedshiftTags int for _, stmt := range permissionsPolicy(t, path) { for _, action := range stmt.Action { - switch { - case action == "ec2:CreateTags": + switch action { + case "ec2:CreateTags": assert.Equal(t, reservedInstanceARN, stmt.Resource, "ec2:CreateTags must be scoped to reserved instances") scoped++ - case action == "redshift:CreateTags" || action == "redshift:DescribeTags": + case "redshift:CreateTags", "redshift:DescribeTags": assert.Equal(t, "*", stmt.Resource, "redshift tag actions have no reserved-node resource type, so they are granted on *") wildcardRedshiftTags++ From 9c315d279a14e1d806155ac8b8b7c6ce703b2e3b Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Fri, 9 Oct 2026 15:13:27 +0200 Subject: [PATCH 3/4] fix(iac): drop the Redshift tag grant and pin EC2 tag scoping in all 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 --- .../stacks/CUDly-CrossAccount/template.yaml | 3 - .../cloudformation/template.yaml | 3 - .../aws-cross-account/terraform/main.tf | 3 - .../aws-target/cloudformation/template.yaml | 3 - iac/federation/aws-target/terraform/main.tf | 3 - .../templates/aws-cross-account-cli.sh.tmpl | 2 +- .../iacfiles/templates/aws-wif-cli.sh.tmpl | 2 +- .../iacfiles/templates_aws_tagging_test.go | 155 +++++++++++++++--- scripts/test-aws-iam-parity.sh | 12 +- 9 files changed, 143 insertions(+), 43 deletions(-) diff --git a/cloudformation/stacks/CUDly-CrossAccount/template.yaml b/cloudformation/stacks/CUDly-CrossAccount/template.yaml index fe9d44a90..ee5b9aa72 100644 --- a/cloudformation/stacks/CUDly-CrossAccount/template.yaml +++ b/cloudformation/stacks/CUDly-CrossAccount/template.yaml @@ -146,9 +146,6 @@ Resources: - redshift:DescribeReservedNodeOfferings - redshift:DescribeReservedNodes - redshift:PurchaseReservedNodeOffering - # No reserved-node resource type exists for the two tag actions (AWS Service Authorization Reference), so "*" is the narrowest scope. - - redshift:CreateTags - - redshift:DescribeTags Resource: "*" # MemoryDB Reserved Nodes diff --git a/iac/federation/aws-cross-account/cloudformation/template.yaml b/iac/federation/aws-cross-account/cloudformation/template.yaml index 5e63158c5..c9534b2fd 100644 --- a/iac/federation/aws-cross-account/cloudformation/template.yaml +++ b/iac/federation/aws-cross-account/cloudformation/template.yaml @@ -111,9 +111,6 @@ Resources: - redshift:PurchaseReservedNodeOffering - redshift:DescribeReservedNodeOfferings - redshift:DescribeReservedNodes - # No reserved-node resource type exists for the two tag actions (AWS Service Authorization Reference), so "*" is the narrowest scope. - - redshift:CreateTags - - redshift:DescribeTags Resource: "*" - Sid: MemoryDBReservations Effect: Allow diff --git a/iac/federation/aws-cross-account/terraform/main.tf b/iac/federation/aws-cross-account/terraform/main.tf index eb574ccd7..a97dfb764 100644 --- a/iac/federation/aws-cross-account/terraform/main.tf +++ b/iac/federation/aws-cross-account/terraform/main.tf @@ -119,9 +119,6 @@ resource "aws_iam_policy" "cudly" { "redshift:PurchaseReservedNodeOffering", "redshift:DescribeReservedNodeOfferings", "redshift:DescribeReservedNodes", - # No reserved-node resource type exists for the two tag actions (AWS Service Authorization Reference), so "*" is the narrowest scope. - "redshift:CreateTags", - "redshift:DescribeTags", ] Resource = "*" }, diff --git a/iac/federation/aws-target/cloudformation/template.yaml b/iac/federation/aws-target/cloudformation/template.yaml index 5c0094f32..1014111fa 100644 --- a/iac/federation/aws-target/cloudformation/template.yaml +++ b/iac/federation/aws-target/cloudformation/template.yaml @@ -225,9 +225,6 @@ Resources: - redshift:PurchaseReservedNodeOffering - redshift:DescribeReservedNodeOfferings - redshift:DescribeReservedNodes - # No reserved-node resource type exists for the two tag actions (AWS Service Authorization Reference), so "*" is the narrowest scope. - - redshift:CreateTags - - redshift:DescribeTags Resource: "*" - Sid: MemoryDBReservations Effect: Allow diff --git a/iac/federation/aws-target/terraform/main.tf b/iac/federation/aws-target/terraform/main.tf index 4b92bd1a3..beaab733e 100644 --- a/iac/federation/aws-target/terraform/main.tf +++ b/iac/federation/aws-target/terraform/main.tf @@ -106,9 +106,6 @@ resource "aws_iam_policy" "cudly" { "redshift:PurchaseReservedNodeOffering", "redshift:DescribeReservedNodeOfferings", "redshift:DescribeReservedNodes", - # No reserved-node resource type exists for the two tag actions (AWS Service Authorization Reference), so "*" is the narrowest scope. - "redshift:CreateTags", - "redshift:DescribeTags", ] Resource = "*" }, diff --git a/internal/iacfiles/templates/aws-cross-account-cli.sh.tmpl b/internal/iacfiles/templates/aws-cross-account-cli.sh.tmpl index 4a529613a..43e011542 100644 --- a/internal/iacfiles/templates/aws-cross-account-cli.sh.tmpl +++ b/internal/iacfiles/templates/aws-cross-account-cli.sh.tmpl @@ -46,7 +46,7 @@ PERMISSIONS_POLICY=$(cat <<'JSON' "elasticache:PurchaseReservedCacheNodesOffering", "elasticache:DescribeReservedCacheNodesOfferings","elasticache:DescribeReservedCacheNodes", "redshift:PurchaseReservedNodeOffering","redshift:DescribeReservedNodeOfferings", - "redshift:DescribeReservedNodes","redshift:CreateTags","redshift:DescribeTags", + "redshift:DescribeReservedNodes", "memorydb:PurchaseReservedNodesOffering","memorydb:DescribeReservedNodesOfferings", "memorydb:DescribeReservedNodes", "savingsplans:CreateSavingsPlan","savingsplans:DescribeSavingsPlans", diff --git a/internal/iacfiles/templates/aws-wif-cli.sh.tmpl b/internal/iacfiles/templates/aws-wif-cli.sh.tmpl index f119a1467..b593b6657 100644 --- a/internal/iacfiles/templates/aws-wif-cli.sh.tmpl +++ b/internal/iacfiles/templates/aws-wif-cli.sh.tmpl @@ -115,7 +115,7 @@ PERMISSIONS_POLICY=$(cat <<'JSON' "elasticache:PurchaseReservedCacheNodesOffering", "elasticache:DescribeReservedCacheNodesOfferings","elasticache:DescribeReservedCacheNodes", "redshift:PurchaseReservedNodeOffering","redshift:DescribeReservedNodeOfferings", - "redshift:DescribeReservedNodes","redshift:CreateTags","redshift:DescribeTags", + "redshift:DescribeReservedNodes", "memorydb:PurchaseReservedNodesOffering","memorydb:DescribeReservedNodesOfferings", "memorydb:DescribeReservedNodes", "savingsplans:CreateSavingsPlan","savingsplans:DescribeSavingsPlans", diff --git a/internal/iacfiles/templates_aws_tagging_test.go b/internal/iacfiles/templates_aws_tagging_test.go index 70accaff1..e1a0566b5 100644 --- a/internal/iacfiles/templates_aws_tagging_test.go +++ b/internal/iacfiles/templates_aws_tagging_test.go @@ -2,11 +2,15 @@ package iacfiles import ( "encoding/json" + "os" + "path/filepath" + "regexp" "strings" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "gopkg.in/yaml.v3" ) const reservedInstanceARN = "arn:aws:ec2:*:*:reserved-instances/*" @@ -17,9 +21,9 @@ type awsPolicyStatement struct { Resource string `json:"Resource"` } -// permissionsPolicy extracts the PERMISSIONS_POLICY heredoc from a rendered- -// independent copy of an AWS CLI onboarding template (the heredoc is quoted and -// holds no template placeholders). +// permissionsPolicy extracts the PERMISSIONS_POLICY heredoc from an AWS CLI +// onboarding template (the heredoc is quoted and holds no template +// placeholders). func permissionsPolicy(t *testing.T, path string) []awsPolicyStatement { t.Helper() raw, err := Templates.ReadFile(path) @@ -35,32 +39,143 @@ func permissionsPolicy(t *testing.T, path string) []awsPolicyStatement { return doc.Statement } -// TestAWSCLITemplatesTagReservedInstancesOnly pins #702: the member-account -// role can tag the reserved instance it just bought (the purchase idempotency -// tag), but only on reserved-instances resources, never on every EC2 resource. -func TestAWSCLITemplatesTagReservedInstancesOnly(t *testing.T) { - for _, path := range []string{ - "templates/aws-cross-account-cli.sh.tmpl", - "templates/aws-wif-cli.sh.tmpl", - } { +// cfnStatements returns every mapping that has both an Action list and a +// Resource scalar, which is how a CloudFormation IAM statement looks. +func cfnStatements(t *testing.T, raw []byte) []awsPolicyStatement { + t.Helper() + var root yaml.Node + require.NoError(t, yaml.Unmarshal(raw, &root)) + var out []awsPolicyStatement + var walk func(n *yaml.Node) + walk = func(n *yaml.Node) { + if n.Kind == yaml.MappingNode { + var stmt awsPolicyStatement + var hasAction, hasResource bool + for i := 0; i+1 < len(n.Content); i += 2 { + key, val := n.Content[i].Value, n.Content[i+1] + switch { + case key == "Action" && val.Kind == yaml.SequenceNode: + hasAction = true + for _, item := range val.Content { + stmt.Action = append(stmt.Action, item.Value) + } + case key == "Resource" && val.Kind == yaml.ScalarNode: + hasResource = true + stmt.Resource = val.Value + } + } + if hasAction && hasResource { + out = append(out, stmt) + } + } + for _, c := range n.Content { + walk(c) + } + } + walk(&root) + return out +} + +var ( + tfActionList = regexp.MustCompile(`(?s)Action\s*=\s*\[(.*?)\]`) + tfResource = regexp.MustCompile(`Resource\s*=\s*"([^"]*)"`) + quoted = regexp.MustCompile(`"([^"]+)"`) +) + +// tfStatements returns the statement objects of a Terraform jsonencode policy: +// the brace-balanced object around every `Sid =`. +func tfStatements(t *testing.T, raw []byte) []awsPolicyStatement { + t.Helper() + text := string(raw) + var out []awsPolicyStatement + for _, loc := range regexp.MustCompile(`Sid\s*=`).FindAllStringIndex(text, -1) { + start, depth := loc[0], 0 + for ; start > 0; start-- { + if text[start] == '}' { + depth++ + } + if text[start] == '{' { + if depth == 0 { + break + } + depth-- + } + } + end, depth := loc[1], 0 + for ; end < len(text); end++ { + if text[end] == '{' { + depth++ + } + if text[end] == '}' { + if depth == 0 { + break + } + depth-- + } + } + chunk := text[start:end] + var stmt awsPolicyStatement + if m := tfActionList.FindStringSubmatch(chunk); m != nil { + for _, q := range quoted.FindAllStringSubmatch(m[1], -1) { + stmt.Action = append(stmt.Action, q[1]) + } + } + if m := tfResource.FindStringSubmatch(chunk); m != nil { + stmt.Resource = m[1] + } + out = append(out, stmt) + } + return out +} + +// memberAccountPolicies lists the seven member-account purchase policies: +// the two CLI onboarding templates, the four federation templates and the +// legacy CUDly-CrossAccount stack. +func memberAccountPolicies(t *testing.T) map[string][]awsPolicyStatement { + t.Helper() + root := filepath.Join("..", "..") + read := func(rel string) []byte { + raw, err := os.ReadFile(filepath.Join(root, rel)) + require.NoError(t, err, rel) + return raw + } + return map[string][]awsPolicyStatement{ + "templates/aws-cross-account-cli.sh.tmpl": permissionsPolicy(t, "templates/aws-cross-account-cli.sh.tmpl"), + "templates/aws-wif-cli.sh.tmpl": permissionsPolicy(t, "templates/aws-wif-cli.sh.tmpl"), + + "iac/federation/aws-cross-account/cloudformation/template.yaml": cfnStatements(t, read("iac/federation/aws-cross-account/cloudformation/template.yaml")), + "iac/federation/aws-target/cloudformation/template.yaml": cfnStatements(t, read("iac/federation/aws-target/cloudformation/template.yaml")), + "cloudformation/stacks/CUDly-CrossAccount/template.yaml": cfnStatements(t, read("cloudformation/stacks/CUDly-CrossAccount/template.yaml")), + + "iac/federation/aws-cross-account/terraform/main.tf": tfStatements(t, read("iac/federation/aws-cross-account/terraform/main.tf")), + "iac/federation/aws-target/terraform/main.tf": tfStatements(t, read("iac/federation/aws-target/terraform/main.tf")), + } +} + +// TestMemberAccountPoliciesTagReservedInstancesOnly pins #702 for every +// member-account policy: the role can tag the reserved instance it just bought +// (the purchase idempotency tag), but only on reserved-instances resources, +// never on every EC2 resource. redshift:CreateTags is deliberately absent: it +// cannot be scoped below "*" and whether Redshift can tag reserved nodes at all +// is unverified, so that half of #702 is still open. +func TestMemberAccountPoliciesTagReservedInstancesOnly(t *testing.T) { + for path, statements := range memberAccountPolicies(t) { t.Run(path, func(t *testing.T) { - var scoped, wildcardRedshiftTags int - for _, stmt := range permissionsPolicy(t, path) { + require.NotEmpty(t, statements, "no policy statements parsed") + var createTags int + for _, stmt := range statements { for _, action := range stmt.Action { switch action { case "ec2:CreateTags": assert.Equal(t, reservedInstanceARN, stmt.Resource, "ec2:CreateTags must be scoped to reserved instances") - scoped++ - case "redshift:CreateTags", "redshift:DescribeTags": - assert.Equal(t, "*", stmt.Resource, - "redshift tag actions have no reserved-node resource type, so they are granted on *") - wildcardRedshiftTags++ + createTags++ + case "redshift:CreateTags": + t.Errorf("redshift:CreateTags must not be granted until it is verified and condition-scoped (#702)") } } } - assert.Equal(t, 1, scoped, "ec2:CreateTags must appear exactly once") - assert.Equal(t, 2, wildcardRedshiftTags, "redshift:CreateTags and redshift:DescribeTags must each appear once") + assert.Equal(t, 1, createTags, "ec2:CreateTags must appear exactly once") }) } } diff --git a/scripts/test-aws-iam-parity.sh b/scripts/test-aws-iam-parity.sh index a57afa5c8..8208f6bed 100755 --- a/scripts/test-aws-iam-parity.sh +++ b/scripts/test-aws-iam-parity.sh @@ -102,13 +102,13 @@ grep -v 'ec2:CreateTags' \ > "$FIX6/iac/federation/aws-target/terraform/main.tf" run_case "federation drift (target TF missing ec2:CreateTags) exits 1" 1 --root "$FIX6" -# Case 7 (#702): same for the Redshift tag actions in a CLI onboarding template. -FIX7="$TMP_BASE/drift-redshift-tags" +# Case 7 (#702): same for a CloudFormation federation template. +FIX7="$TMP_BASE/drift-createtags-cfn" seed_fixture "$FIX7" -sed 's/redshift:DescribeTags/redshift-removed/' \ - "$REPO_ROOT/internal/iacfiles/templates/aws-wif-cli.sh.tmpl" \ - > "$FIX7/internal/iacfiles/templates/aws-wif-cli.sh.tmpl" -run_case "federation drift (WIF CLI missing redshift:DescribeTags) exits 1" 1 --root "$FIX7" +grep -v 'ec2:CreateTags' \ + "$REPO_ROOT/iac/federation/aws-target/cloudformation/template.yaml" \ + > "$FIX7/iac/federation/aws-target/cloudformation/template.yaml" +run_case "federation drift (target CFN missing ec2:CreateTags) exits 1" 1 --root "$FIX7" echo "" echo "Results: ${pass} passed, ${fail} failed." From c09d4ad2e473bd444ec8de12af80eb75864bb59c Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Fri, 9 Oct 2026 15:29:15 +0200 Subject: [PATCH 4/4] test(iac): preallocate statement slice in tagging test Co-Authored-By: Claude Opus 5.5 --- internal/iacfiles/templates_aws_tagging_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/internal/iacfiles/templates_aws_tagging_test.go b/internal/iacfiles/templates_aws_tagging_test.go index e1a0566b5..9a363238c 100644 --- a/internal/iacfiles/templates_aws_tagging_test.go +++ b/internal/iacfiles/templates_aws_tagging_test.go @@ -87,8 +87,9 @@ var ( func tfStatements(t *testing.T, raw []byte) []awsPolicyStatement { t.Helper() text := string(raw) - var out []awsPolicyStatement - for _, loc := range regexp.MustCompile(`Sid\s*=`).FindAllStringIndex(text, -1) { + sids := regexp.MustCompile(`Sid\s*=`).FindAllStringIndex(text, -1) + out := make([]awsPolicyStatement, 0, len(sids)) + for _, loc := range sids { start, depth := loc[0], 0 for ; start > 0; start-- { if text[start] == '}' {