BAU: reduce lamba policies #5604
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
whi-tw
force-pushed
the
BAU/reduce-lambda-policies
branch
from
4aedca9 to
be0a2cc
Compare
Previously, we created many policies, and then attached them all to the
individual roles.
This meant that we often bumped up against the 20 policy limit for a
role. It also took a long time to run the terraform apply, as it had to
check MANY policies for changes.
This change introduces a replacement for the lambda-role module, which
takes in a map of policy documents lists and combines them into a
reduced number of policies specific to the role.
eg.
```hcl
module "lambda-role-policy-reduction" {
source = "./modules/lambda-role-policy-reduction"
role_name = "my-role"
environment = "dev"
default_tags = var.default_tags
vpc_arn = var.vpc_arn
policy_documents_to_attach = {
"dynamodb" = [
data.aws_iam_policy_document.dynamodb_write_policy.json,
data.aws_iam_policy_document.dynamodb_read_policy.json,
],
"s3" = [
data.aws_iam_policy_document.s3_list_policy.json,
data.aws_iam_policy_document.s3_read_policy.json,
data.aws_iam_policy_document.s3_write_policy.json,
],
"sqs" = [
data.aws_iam_policy_document.sqs_policy.json,
],
"redis" = [
data.aws_iam_policy_document.redis_read_policy.json,
data.aws_iam_policy_document.redis_write_policy.json,
],
}
}
```
This would create and attach 5 policies:
1. base: Contains the logging, networking and xray policies that are
common to all roles.
2. dynamodb: Contains the dynamodb policies
3. s3: Contains the s3 policy
4. sqs: Contains the sqs policy
5. redis: Contains the redis policy
previously, this would have created and attached 11 policies to the role
ie. 1 more than the limit.
This is a proof of concept to show how the new lambda role module is used.
whi-tw
force-pushed
the
BAU/reduce-lambda-policies
branch
from
be0a2cc to
5a85b5b
Compare
| error_message = "policy_documents_to_attach cannot contain a key named 'base'" | ||
| } | ||
| validation { | ||
| condition = length(keys(var.policy_documents_to_attach)) <= 20 |
There was a problem hiding this comment.
Off by 1 here. We already have 1 policy defined, base
| base = concat([ | ||
| data.aws_iam_policy_document.logging_policy_document.json, | ||
| data.aws_iam_policy_document.endpoint_xray_policy.json, | ||
| ], | ||
| var.vpc_arn == "" ? [] : [data.aws_iam_policy_document.networking_policy_document[0].json]), | ||
| }, var.policy_documents_to_attach) |
There was a problem hiding this comment.
Do the order of these policies matter? i.e. if the json object keys are not returned in the same order, will that cause AWS/terraform to fail to deploy the change (see previous conversations about changes to policy ordering messing up a deployment)?
|
|
||
| data "aws_iam_policy_document" "iam-policy-document" { | ||
| for_each = local.policy_documents_to_attach | ||
| source_policy_documents = each.value |
There was a problem hiding this comment.
Do we need to worry about this?
Statements defined in source_policy_documents must have unique
sids
There was a problem hiding this comment.
@ethanmills thanks for taking a look - I was going to reach out for input.
Yeah, that's a super valid point I hadn't considered. If we go ahead with this approach, I'll add a validation step in to ensure uniqueness.
|
What does the plan look like if I add a policy doc to e.g. the |
What
Previously, we created many policies, and then attached them all to the
individual roles.
This meant that we often bumped up against the 20 policy limit for a
role. It also took a long time to run the terraform apply, as it had to
check MANY policies for changes.
This change introduces a replacement for the lambda-role module, which
takes in a map of policy documents lists and combines them into a
reduced number of policies specific to the role.
eg.
This would create and attach 5 policies:
common to all roles.
previously, this would have created and attached 11 policies to the role
ie. 1 more than the limit.
How to review
Before Merging