-
Notifications
You must be signed in to change notification settings - Fork 3.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
AccountPrinicpal does not support multiple accounts #924
Comments
* feat(IAM): Optimize IAM policy statements for size Attempts to reduce policy sizes by performing a number of optimizations: - `FnConcat` pre-concatenates string literals that are not tokens, to reduce rendered size - Redundant patterns in `Action` and `Resource` policy elements get de-duplicated (aka: `["iam:GetObject", "iam:Get*"]` becomes `["iam:Get*"]`, since this implies `"iam:GetObject"`) - Redundant statements get de-duplicated (if a statement offers strictly broader permissions than another one, then the later will be removed). - Statements that differ only in `Resource` content get merged (adding entries to the `Resource` element). IAM policy statements that have an `Sid` are not merged with anything (this could lead to the `Sid` mis-representing the statement's intention). They are also not de-duplicated (unless the other statement has the exact same `Sid`, which is likely symptom of a programming error). This causes widespread changes to the IAM policy documents (and some occurrences of `Fn::Join` intrinsic all over the place, so this commit is... massive). Generally speaking, this appears to make for smaller templates, which at least is not a bad thing. Fixes #924
When adidng multiple principals, an array of principals was created, but IAM documents expect an object with different keys, for which values may be arrays. This corrects the merging process so it produces valid statements. This restores the fix for #924 that was introduced in #916 and reverted in #958 due to an other feature part of the same commit.
When adidng multiple principals, an array of principals was created, but IAM documents expect an object with different keys, for which values may be arrays. This corrects the merging process so it produces valid statements. This restores the fix for #924 that was introduced in #916 and reverted in #958 due to an other feature part of the same commit.
__IMPORTANT NOTE__: when upgrading to this version of the CDK framework, you must also upgrade your installation the CDK Toolkit to the matching version: ```shell $ npm i -g aws-cdk $ cdk --version 0.14.0 (build ...) ``` Bug Fixes ========= * remove CloudFormation property renames ([#973](#973)) ([3f86603](3f86603)), closes [#852](#852) * **aws-ec2:** fix retention of all egress traffic rule ([#998](#998)) ([b9d5b43](b9d5b43)), closes [#987](#987) * **aws-s3-deployment:** avoid deletion during update using physical ids ([#1006](#1006)) ([bca99c6](bca99c6)), closes [#981](#981) [#981](#981) * **cloudformation-diff:** ignore changes to DependsOn ([#1005](#1005)) ([3605f9c](3605f9c)), closes [#274](#274) * **cloudformation-diff:** track replacements ([#1003](#1003)) ([a83ac5f](a83ac5f)), closes [#1001](#1001) * **docs:** fix EC2 readme for "natgatway" configuration ([#994](#994)) ([0b1e7cc](0b1e7cc)) * **docs:** updates to contribution guide ([#997](#997)) ([b42e742](b42e742)) * **iam:** Merge multiple principals correctly ([#983](#983)) ([3fc5c8c](3fc5c8c)), closes [#924](#924) [#916](#916) [#958](#958) Features ========= * add construct library for Application AutoScaling ([#933](#933)) ([7861c6f](7861c6f)), closes [#856](#856) [#861](#861) [#640](#640) [#644](#644) * add HostedZone context provider ([#823](#823)) ([1626c37](1626c37)) * **assert:** haveResource lists failing properties ([#1016](#1016)) ([7f6f3fd](7f6f3fd)) * **aws-cdk:** add CDK app version negotiation ([#988](#988)) ([db4e718](db4e718)), closes [#891](#891) * **aws-codebuild:** Introduce a CodePipeline test Action. ([#873](#873)) ([770f9aa](770f9aa)) * **aws-sqs:** Add grantXxx() methods ([#1004](#1004)) ([8c90350](8c90350)) * **core:** Pre-concatenate Fn::Join ([#967](#967)) ([33c32a8](33c32a8)), closes [#916](#916) [#958](#958) BREAKING CHANGES ========= * DynamoDB AutoScaling: Instead of `addReadAutoScaling()`, call `autoScaleReadCapacity()`, and similar for write scaling. * CloudFormation resource usage: If you use L1s, you may need to change some `XxxName` properties back into `Name`. These will match the CloudFormation property names. * You must use the matching `aws-cdk` toolkit when upgrading to this version, or context providers will cease to work. All existing cached context values in `cdk.json` will be invalidated and refreshed.
__IMPORTANT NOTE__: when upgrading to this version of the CDK framework, you must also upgrade your installation the CDK Toolkit to the matching version: ```shell $ npm i -g aws-cdk $ cdk --version 0.14.0 (build ...) ``` Bug Fixes ========= * remove CloudFormation property renames ([#973](#973)) ([3f86603](3f86603)), closes [#852](#852) * **aws-ec2:** fix retention of all egress traffic rule ([#998](#998)) ([b9d5b43](b9d5b43)), closes [#987](#987) * **aws-s3-deployment:** avoid deletion during update using physical ids ([#1006](#1006)) ([bca99c6](bca99c6)), closes [#981](#981) [#981](#981) * **cloudformation-diff:** ignore changes to DependsOn ([#1005](#1005)) ([3605f9c](3605f9c)), closes [#274](#274) * **cloudformation-diff:** track replacements ([#1003](#1003)) ([a83ac5f](a83ac5f)), closes [#1001](#1001) * **docs:** fix EC2 readme for "natgatway" configuration ([#994](#994)) ([0b1e7cc](0b1e7cc)) * **docs:** updates to contribution guide ([#997](#997)) ([b42e742](b42e742)) * **iam:** Merge multiple principals correctly ([#983](#983)) ([3fc5c8c](3fc5c8c)), closes [#924](#924) [#916](#916) [#958](#958) Features ========= * add construct library for Application AutoScaling ([#933](#933)) ([7861c6f](7861c6f)), closes [#856](#856) [#861](#861) [#640](#640) [#644](#644) * add HostedZone context provider ([#823](#823)) ([1626c37](1626c37)) * **assert:** haveResource lists failing properties ([#1016](#1016)) ([7f6f3fd](7f6f3fd)) * **aws-cdk:** add CDK app version negotiation ([#988](#988)) ([db4e718](db4e718)), closes [#891](#891) * **aws-codebuild:** Introduce a CodePipeline test Action. ([#873](#873)) ([770f9aa](770f9aa)) * **aws-sqs:** Add grantXxx() methods ([#1004](#1004)) ([8c90350](8c90350)) * **core:** Pre-concatenate Fn::Join ([#967](#967)) ([33c32a8](33c32a8)), closes [#916](#916) [#958](#958) BREAKING CHANGES ========= * DynamoDB AutoScaling: Instead of `addReadAutoScaling()`, call `autoScaleReadCapacity()`, and similar for write scaling. * CloudFormation resource usage: If you use L1s, you may need to change some `XxxName` properties back into `Name`. These will match the CloudFormation property names. * You must use the matching `aws-cdk` toolkit when upgrading to this version, or context providers will cease to work. All existing cached context values in `cdk.json` will be invalidated and refreshed.
__IMPORTANT NOTE__: when upgrading to this version of the CDK framework, you must also upgrade your installation the CDK Toolkit to the matching version: ```shell $ npm i -g aws-cdk $ cdk --version 0.14.0 (build ...) ``` Bug Fixes ========= * remove CloudFormation property renames ([aws#973](aws#973)) ([3f86603](aws@3f86603)), closes [aws#852](aws#852) * **aws-ec2:** fix retention of all egress traffic rule ([aws#998](aws#998)) ([b9d5b43](aws@b9d5b43)), closes [aws#987](aws#987) * **aws-s3-deployment:** avoid deletion during update using physical ids ([aws#1006](aws#1006)) ([bca99c6](aws@bca99c6)), closes [aws#981](aws#981) [aws#981](aws#981) * **cloudformation-diff:** ignore changes to DependsOn ([aws#1005](aws#1005)) ([3605f9c](aws@3605f9c)), closes [aws#274](aws#274) * **cloudformation-diff:** track replacements ([aws#1003](aws#1003)) ([a83ac5f](aws@a83ac5f)), closes [aws#1001](aws#1001) * **docs:** fix EC2 readme for "natgatway" configuration ([aws#994](aws#994)) ([0b1e7cc](aws@0b1e7cc)) * **docs:** updates to contribution guide ([aws#997](aws#997)) ([b42e742](aws@b42e742)) * **iam:** Merge multiple principals correctly ([aws#983](aws#983)) ([3fc5c8c](aws@3fc5c8c)), closes [aws#924](aws#924) [aws#916](aws#916) [aws#958](aws#958) Features ========= * add construct library for Application AutoScaling ([aws#933](aws#933)) ([7861c6f](aws@7861c6f)), closes [aws#856](aws#856) [aws#861](aws#861) [aws#640](aws#640) [aws#644](aws#644) * add HostedZone context provider ([aws#823](aws#823)) ([1626c37](aws@1626c37)) * **assert:** haveResource lists failing properties ([aws#1016](aws#1016)) ([7f6f3fd](aws@7f6f3fd)) * **aws-cdk:** add CDK app version negotiation ([aws#988](aws#988)) ([db4e718](aws@db4e718)), closes [aws#891](aws#891) * **aws-codebuild:** Introduce a CodePipeline test Action. ([aws#873](aws#873)) ([770f9aa](aws@770f9aa)) * **aws-sqs:** Add grantXxx() methods ([aws#1004](aws#1004)) ([8c90350](aws@8c90350)) * **core:** Pre-concatenate Fn::Join ([aws#967](aws#967)) ([33c32a8](aws@33c32a8)), closes [aws#916](aws#916) [aws#958](aws#958) BREAKING CHANGES ========= * DynamoDB AutoScaling: Instead of `addReadAutoScaling()`, call `autoScaleReadCapacity()`, and similar for write scaling. * CloudFormation resource usage: If you use L1s, you may need to change some `XxxName` properties back into `Name`. These will match the CloudFormation property names. * You must use the matching `aws-cdk` toolkit when upgrading to this version, or context providers will cease to work. All existing cached context values in `cdk.json` will be invalidated and refreshed.
I wonder if this has been regressed since this issue was closed? Today, with iam.Role(
self,
"role-name",
assumed_by=iam.AccountPrincipal(["123","456"]),
) Produces AWS:
Fn::Join:
- ""
- - "arn:"
- Ref: AWS::Partition
- :iam::123,456:root I would expect to be able to pass a list of account IDs, as per the initial bug report and fix. (My use case is to take a list from SSM and pass it in, but it looks like there might be other issues there) |
Hi Folks. Any updates about it? My case is very similar to @symroe, I'm receiving a list of accountIDs and a need to pass it to a ECR policy as multiple Principals. I'm using Python 3.9 and CDK 2.8. |
Problem Solved. We used the append function and save the results into a variable. PrincipalList = []
for account in AccountList:
PrincipalList.append(iam.AccountPrincipal(account)) Then we used the variable to forward the principals list to the PolicyStatement. I hope it helps 👍 😁 . |
I ran into a similar problem, but I was able to solve this using spread operator. const accountsToAllowAssumedBy = ['000000000000', '111111111111'];
const role = new iam.Role(this, 'Role', {
roleName: 'AssumeRoleTest',
assumedBy: new iam.CompositePrincipal(
new iam.ServicePrincipal('ecs-tasks.amazonaws.com'),
...accountsToAllowAssumedBy.map((account) => new iam.AccountPrincipal(account)), //Spread Operator (...)
),
}); Output: "Role1ABCC5F0": {
"Type": "AWS::IAM::Role",
"Properties": {
"RoleName": "AssumeRoleTest",
"AssumeRolePolicyDocument": {
"Statement": [
{
"Action": "sts:AssumeRole",
"Effect": "Allow",
"Principal": {
"AWS": [
"arn:aws:iam::000000000000:root",
"arn:aws:iam::111111111111:root"
],
"Service": "ecs-tasks.amazonaws.com"
}
}
],
},
},
}, In your case, you can just do this const accountsToAllowAssumedBy = ['000000000000', '111111111111'];
const role = new iam.Role(this, 'Role', {
roleName: 'AssumeRoleTest',
assumedBy: new iam.CompositePrincipal(
...accountsToAllowAssumedBy.map((account) => new iam.AccountPrincipal(account)), //Spread Operator (...)
),
}); |
If a single role is used to grant access to multiple accounts, the principal does not output correct cloudformation:
generates:
It should instead generate:
The current workaround is to build the role using property overrides.
The text was updated successfully, but these errors were encountered: