Skip to content
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

Minimized IAM Statements produced are inconsistent #20631

Closed
joekeilty-oub opened this issue Jun 6, 2022 · 7 comments
Closed

Minimized IAM Statements produced are inconsistent #20631

joekeilty-oub opened this issue Jun 6, 2022 · 7 comments
Labels
@aws-cdk/aws-iam Related to AWS Identity and Access Management documentation This is a problem with documentation. effort/small Small work item – less than a day of effort p1

Comments

@joekeilty-oub
Copy link

Describe the bug

aws-cdk version: 2.27.0 (build 8e89048)
aws-cdk-lib version: 2.27.0

I have a Policy attached to a User which has been granted S3 access as such:

        bucket.grant_delete(iam_user)
        bucket.grant_put_acl(iam_user)

When doing cdk synth this results in the following statement block being added onto an IAM policy tied to the user:

      {
       "Action": [
        "s3:DeleteObject*",
        "s3:PutObjectAcl",
        "s3:PutObjectVersionAcl"
       ],
       "Effect": "Allow",
       "Resource": {
        "Fn::Join": [
         "",
         [
          {
           "Fn::GetAtt": [
            "bucket43879C71",
            "Arn"
           ]
          },
          "/*"
         ]
        ]
       }
      }

However, when I try to assert that this statement is on the policy in the template it fails because in the template generated by the test it has split it into 2 statements.

Expected Behavior

I expect that the output of cdk synth would be the same as what aws_cdk.assertions.Template.from_stack(stack) produces when it comes to IAM policy statements.

Current Behavior

What actually happens is the aws_cdk.assertions.Template.from_stack(stack) produces 2 individual statements like so:

E                     {
E                       "Action": "s3:DeleteObject*",
E                       "Effect": "Allow",
E                       "Resource": {
E                         "Fn::Join": [
E                           "",
E                           [
E                             {
E                               "Fn::GetAtt": [
E                                 "bucket43879C71",
E                                 "Arn"
E                               ]
E                             },
E                             "/*"
E                           ]
E                         ]
E                       }
E                     },
E                     {
E                       "Action": [
E                         "s3:PutObjectAcl",
E                         "s3:PutObjectVersionAcl"
E                       ],
E                       "Effect": "Allow",
E                       "Resource": {
E                         "Fn::Join": [
E                           "",
E                           [
E                             {
E                               "Fn::GetAtt": [
E                                 "bucket43879C71",
E                                 "Arn"
E                               ]
E                             },
E                             "/*"
E                           ]
E                         ]
E                       }
E                     },

(pardon the E's, taken straight from my CLI)

Reproduction Steps

import aws_cdk
from constructs import Construct

class TestStack(aws_cdk.Stack):
    def __init__(
        self,
        scope: Construct,
        construct_id: str,
        **kwargs
    ) -> None:
        super().__init__(scope, construct_id, **kwargs)

        bucket = aws_cdk.aws_s3.Bucket(self, 'bucket')
        iam_user = aws_cdk.aws_iam.User(self, 'iam-user')
        bucket.grant_delete(iam_user)
        bucket.grant_put_acl(iam_user)

app = aws_cdk.App()
stack = TestStack(app, 'test')
template = aws_cdk.assertions.Template.from_stack(stack)

user = template.find_resources("AWS::IAM::User")
bucket = template.find_resources("AWS::S3::Bucket")
template.has_resource_properties(
    "AWS::IAM::Policy",
    aws_cdk.assertions.Match.object_like(
        {
            "PolicyDocument": {
                "Statement": aws_cdk.assertions.Match.array_with(
                    [
                        aws_cdk.assertions.Match.object_like(
                            {
                                "Action": [
                                    "s3:DeleteObject*",
                                    "s3:PutObjectAcl",
                                    "s3:PutObjectVersionAcl",
                                ],
                                "Effect": "Allow",
                                "Resource": {
                                    "Fn::Join": [
                                        "",
                                        [
                                            {
                                                "Fn::GetAtt": [
                                                    aws_cdk.assertions.Match.any_value(),
                                                    "Arn",
                                                ]
                                            },
                                            "/*",
                                        ],
                                    ]
                                },
                            }
                        ),
                    ]
                )
            },
        }
    ),
)

Possible Solution

It seems that the synth is doing some extra smarts to merge the 2 statements for grant_delete and grant_put_acl into 1 statement?

It would be great for the output of synth and aws_cdk.assertions.Template.from_stack() to be as similar as possible so that a developer can write their tests based on the output of synth and what ends up getting deployed into CF

Additional Information/Context

No response

CDK CLI Version

2.27.0 (build 8e89048)

Framework Version

No response

Node.js Version

v18.0.0

OS

Mac

Language

Python

Language Version

3.8.5

Other information

No response

@joekeilty-oub joekeilty-oub added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jun 6, 2022
@github-actions github-actions bot added the @aws-cdk/aws-iam Related to AWS Identity and Access Management label Jun 6, 2022
@peterwoodworth
Copy link
Contributor

interesting @joekeilty-oub, thanks for the report. When I try to reproduce this, my template produced by cdk synth ends up containing two separate policies 🤔 I would expect these policies to be minimized into one with the work we've been doing on this recently.

We've been working on reducing our policy sizes #20400 #19764 - I suspect that this is related somehow. cc @rix0rrr

@peterwoodworth peterwoodworth added p1 effort/small Small work item – less than a day of effort effort/medium Medium work item – several days of effort and removed needs-triage This issue or PR still needs to be triaged. labels Jun 6, 2022
@peterwoodworth peterwoodworth changed the title IAM Statements produced are different when running tests vs cdk synth Minimized IAM Statements produced are inconsistent Jun 6, 2022
@polothy
Copy link
Contributor

polothy commented Jun 6, 2022

Likely your problem is that your test doesn't have the feature flag enabled.

app = aws_cdk.App(context={"@aws-cdk/aws-iam:minimizePolicies": True})

Or load it from cdk.json:

    def new_app(self) -> core.App:
        if not self.context:
            path = os.path.join(os.path.dirname(os.path.dirname(__file__)), "cdk.json")
            with open(path) as f:
                self.context = json.load(f)["context"]
        return core.App(context=self.context)

@peterwoodworth
Copy link
Contributor

ahhhhh that'd be it, you're right @polothy. I think the feature flag should be documented in the IAM overview rather than just the PolicyDocument.minimize prop - which personally I think is easy to be overlooked

@peterwoodworth peterwoodworth added documentation This is a problem with documentation. and removed bug This issue is a bug. effort/medium Medium work item – several days of effort labels Jun 6, 2022
@joekeilty-oub
Copy link
Author

Aha, thank you both - I will give that a go!

@kaizencc
Copy link
Contributor

kaizencc commented Jun 8, 2022

Duplicate of #20573, @joekeilty-oub, you're still welcome to give it a go.

@kaizencc kaizencc closed this as completed Jun 8, 2022
@github-actions
Copy link

github-actions bot commented Jun 8, 2022

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

@joekeilty-oub
Copy link
Author

@polothy That worked perfectly, thank you! ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-iam Related to AWS Identity and Access Management documentation This is a problem with documentation. effort/small Small work item – less than a day of effort p1
Projects
None yet
Development

No branches or pull requests

5 participants