cdk-stacksets icon indicating copy to clipboard operation
cdk-stacksets copied to clipboard

fix: create assume role policy inline

Open tabrezm opened this issue 1 year ago • 2 comments

Fixes #438

This PR moves the assume role policy inline to avoid a race condition of the stack set resource deployment before the policy is attached. As a consequence, an existing admin role won't be modified with a potentially duplicate policy.

tabrezm avatar May 05 '24 08:05 tabrezm

Looks good, just please add or modify an integ test for this one.

I think the existing integration tests are sufficient but the snapshot needs to be updated because this PR removes the duplicate policy that gets added to the admin role. Unfortunately I'm a bit stumped on how to make this change on my end. I tried to run yarn integ:update but I'm not sure what values to set for INTEG_DEPLOYMENT_ACCOUNT or INTEG_TARGET_ACCOUNT. If I use a personal account, won't the checked-in snapshot point to it?

For context, this is the snapshot change that's causing the integration test to fail:

Verifying integration test snapshots...

  CHANGED    integ.stack-set 3.496s
      IAM Statement Changes
┌───┬───────────────────────────────────────────────────────────────────────┬────────┬────────────────┬──────────────────────────┬───────────┐
│   │ Resource                                                              │ Effect │ Action         │ Principal                │ Condition │
├───┼───────────────────────────────────────────────────────────────────────┼────────┼────────────────┼──────────────────────────┼───────────┤
│ - │ arn:aws:iam::*:role/AWSCloudFormationStackSetExecutionRole-integ-test │ Allow  │ sts:AssumeRole │ AWS:${AdminRole38563C57} │           │
└───┴───────────────────────────────────────────────────────────────────────┴────────┴────────────────┴──────────────────────────┴───────────┘
(NOTE: There may be security-related changes not in this list. See https://github.com/aws/aws-cdk/issues/1299)

Resources
[-] AWS::IAM::Policy AdminRoleDefaultPolicy1C2AB961 destroy



Snapshot Results: 

Tests:    1 failed, 1 total
Failed: /Users/tabrezm/Code/cdk-stacksets/test/integ.stack-set.ts
!!! This test contains destructive changes !!!
    Stack: integ-stackset-test - Resource: AdminRoleDefaultPolicy1C2AB961 - Impact: WILL_DESTROY
!!! If these destructive changes are necessary, please indicate this on the PR !!!

Please advise, thanks!

tabrezm avatar Jun 26 '24 16:06 tabrezm

I'm running into this issue as well. I want to have a pre-provisioned Admin role (due to the race condition) for the StackSet, but it tries to attach a duplicate inline permission to the pre-provisioned Admin role. This occurs when deploying multiple self managed stacksets with this same Admin role. Please fix the issue.

robertjan-b avatar Jul 22 '24 09:07 robertjan-b

+1

I too hope this issue is resolved.

go-to-k avatar Nov 18 '24 11:11 go-to-k

#678 is the right thing to do here so im going to close this in favor of that.

kaizencc avatar Dec 24 '24 20:12 kaizencc