feat: exposing cataloging IAM Role as a prop parameter & limiting default permissions #357
Conversation
There was a problem hiding this comment.
I think this will work. Only main question is RE: glue:UntagResource.
Also, as a side note, this PR was slightly difficult to review without "hide whitespace" changes toggled on. If the code needs to be formatted, it might be nice to do that as a separate PR to make future reviews simpler 👀
| }); | ||
| return { role, policy }; | ||
| role.addToPolicy( | ||
| new PolicyStatement({ |
There was a problem hiding this comment.
| new PolicyStatement({ | |
| // https://docs.aws.amazon.com/appflow/latest/userguide/security_iam_id-based-policy-examples.html#security_iam_id-based-policy-examples-access-gdc | |
| new PolicyStatement({ |
test/s3/s3.test.ts
Outdated
| template.resourceCountIs('AWS::S3::Bucket', 0); | ||
| template.resourceCountIs('AWS::AppFlow::Flow', 1); | ||
| template.resourceCountIs('AWS::IAM::Role', 1); |
There was a problem hiding this comment.
It would be nice to add a test on the role properties to ensure it's scoped to the database, schema, and tables.
There was a problem hiding this comment.
Please take a look at the updated test. Now there is a strict assertion on the actions and resources for the IAM Policy.
| 'glue:BatchDeleteTable', | ||
| 'glue:DeletePartition', | ||
| ], | ||
| resources: ['*'], |
There was a problem hiding this comment.
The only action I'm unsure about is glue:UntagResource, which operates on a bunch of different resources: https://docs.aws.amazon.com/service-authorization/latest/reference/list_awsglue.html#awsglue-connection
My guess is that it only deals with database and schemas, but I'm not positive.
There was a problem hiding this comment.
Please take a look here. One can tag/untag databases so you're right. I don't think we need to break down this policy into smaller as the combination of actions and resources gives us least privilege
There was a problem hiding this comment.
My concern here is if we need to allow glue:UntagResource on * to retain good behavior. But, like you said, it might just need to work with catalog/database/table, so we could start there.
Just noting that if something breaks we might need a separate statement that allows Tag/Untag resource on a wider swath of resources.
| 'glue:BatchDeleteTable', | ||
| 'glue:DeletePartition', | ||
| ], | ||
| resources: ['*'], |
There was a problem hiding this comment.
My concern here is if we need to allow glue:UntagResource on * to retain good behavior. But, like you said, it might just need to work with catalog/database/table, so we could start there.
Just noting that if something breaks we might need a separate statement that allows Tag/Untag resource on a wider swath of resources.
Fixes #336
This PR includes: