terraform-aws-modules / terraform-aws-lambda

Terraform module, which takes care of a lot of AWS Lambda/serverless tasks (build dependencies, packages, updates, deployments) in countless combinations 🇺🇦
https://registry.terraform.io/modules/terraform-aws-modules/lambda/aws
Apache License 2.0
933 stars 690 forks source link

Use Inline Policies instead of Managed #607

Open RafaelWO opened 3 months ago

RafaelWO commented 3 months ago

Is your request related to a new offering from AWS?

No.

Is your request related to a problem? Please describe.

Using customer-managed policies is not directly a problem but since the policies are only used for the Lambda function it would be cleaner to use inline policies, IMO. See also Managed policies and inline policies.

Describe the solution you'd like.

I would suggest replacing aws_iam_policy and aws_iam_role_policy_attachment resources (for additional "JSON" policies) with aws_iam_role_policy.

For example, changing

# iam.if:282
resource "aws_iam_policy" "additional_json" {
  count = local.create_role && var.attach_policy_json ? 1 : 0

  name   = local.policy_name
  path   = var.policy_path
  policy = var.policy_json
  tags   = var.tags
}

resource "aws_iam_role_policy_attachment" "additional_json" {
  count = local.create_role && var.attach_policy_json ? 1 : 0

  role       = aws_iam_role.lambda[0].name
  policy_arn = aws_iam_policy.additional_json[0].arn
}

to

resource "aws_iam_role_policy" "additional_json" {
  count = local.create_role && var.attach_policy_json ? 1 : 0

  name   = local.policy_name
  role   = aws_iam_role.lambda[0].name
  policy = var.policy_json
  tags   = var.tags
}

The same applies to resources related to Additional policies (list of JSON).

Describe alternatives you've considered.

The alternative is keeping it the way it is now :slightly_smiling_face:

Additional context

If you agree with my suggestion, I'm happy to create a PR with the necessary changes :upside_down_face:

antonbabenko commented 3 months ago

This improvement sounds good. Please make a PR, which will be included as a breaking change in the upcoming major release.

RafaelWO commented 3 months ago

Great! I will try to make a PR in the next days :slightly_smiling_face:

github-actions[bot] commented 2 months ago

This issue has been automatically marked as stale because it has been open 30 days with no activity. Remove stale label or comment or this issue will be closed in 10 days

RafaelWO commented 2 months ago

I'm just waiting on a response on my PR :slightly_smiling_face: - no reason for going stale.

github-actions[bot] commented 1 month ago

This issue has been automatically marked as stale because it has been open 30 days with no activity. Remove stale label or comment or this issue will be closed in 10 days

RafaelWO commented 1 month ago

I would very much appreciate a (second) review of my PR, @antonbabenko 😃

github-actions[bot] commented 1 day ago

This issue has been automatically marked as stale because it has been open 30 days with no activity. Remove stale label or comment or this issue will be closed in 10 days