From 127b10ba07d57462c73441f3276401c2a27df494 Mon Sep 17 00:00:00 2001 From: Nate McCurdy Date: Wed, 11 Oct 2023 10:42:34 -0700 Subject: [PATCH 1/3] fix: Add context tags to the IAM resources (#45) * fix: Add context tags to the IAM resources Prior to this, the `aws_iam_role` and the `aws_iam_policy` created by this module did not include any of the tags passed via `tags` or via `context`. This fixes that problem by specifying `tags = module.this.tags` on each of those resources so that they use the tags specified determined by the null/label context. * chore: update module boilerplate and docs ``` make init make github/init make readme ``` --- .github/renovate.json | 7 ++++--- .github/workflows/release-branch.yml | 1 + .github/workflows/release-published.yml | 2 +- README.md | 4 ---- iam-role.tf | 6 +++++- 5 files changed, 11 insertions(+), 9 deletions(-) diff --git a/.github/renovate.json b/.github/renovate.json index b61ed24..909df09 100644 --- a/.github/renovate.json +++ b/.github/renovate.json @@ -1,13 +1,14 @@ { "extends": [ "config:base", - ":preserveSemverRanges" + ":preserveSemverRanges", + ":rebaseStalePrs" ], - "baseBranches": ["main", "master", "/^release\\/v\\d{1,2}$/"], + "baseBranches": ["main"], "labels": ["auto-update"], "dependencyDashboardAutoclose": true, "enabledManagers": ["terraform"], "terraform": { - "ignorePaths": ["**/context.tf", "examples/**"] + "ignorePaths": ["**/context.tf"] } } diff --git a/.github/workflows/release-branch.yml b/.github/workflows/release-branch.yml index 3f8fe62..b30901e 100644 --- a/.github/workflows/release-branch.yml +++ b/.github/workflows/release-branch.yml @@ -10,6 +10,7 @@ on: - 'docs/**' - 'examples/**' - 'test/**' + - 'README.*' permissions: contents: write diff --git a/.github/workflows/release-published.yml b/.github/workflows/release-published.yml index f86352b..b31232b 100644 --- a/.github/workflows/release-published.yml +++ b/.github/workflows/release-published.yml @@ -11,4 +11,4 @@ permissions: jobs: terraform-module: - uses: cloudposse/github-actions-workflows-terraform-module/.github/workflows/release.yml@main + uses: cloudposse/github-actions-workflows-terraform-module/.github/workflows/release-published.yml@main diff --git a/README.md b/README.md index 4fd16d6..76c1207 100644 --- a/README.md +++ b/README.md @@ -90,10 +90,6 @@ We highly recommend that in your code you pin the version to the exact version y using so that your infrastructure remains stable, and update versions in a systematic way so that they do not catch you by surprise. -Also, because of a bug in the Terraform registry ([hashicorp/terraform#21417](https://github.com/hashicorp/terraform/issues/21417)), -the registry shows many of our inputs as required when in fact they are optional. -The table below correctly indicates which inputs are required. - For a complete example, see [examples/complete](examples/complete). For automated tests of the complete example using [bats](https://github.com/bats-core/bats-core) and [Terratest](https://github.com/gruntwork-io/terratest) diff --git a/iam-role.tf b/iam-role.tf index 05a7fe2..1fd1d84 100644 --- a/iam-role.tf +++ b/iam-role.tf @@ -2,8 +2,10 @@ resource "aws_iam_role" "this" { count = local.enabled ? 1 : 0 name = "${var.function_name}-${local.region_name}" - assume_role_policy = join("", data.aws_iam_policy_document.assume_role_policy.*.json) + assume_role_policy = join("", data.aws_iam_policy_document.assume_role_policy[*].json) permissions_boundary = var.permissions_boundary + + tags = module.this.tags } data "aws_iam_policy_document" "assume_role_policy" { @@ -68,6 +70,8 @@ resource "aws_iam_policy" "ssm" { name = "${var.function_name}-ssm-policy-${local.region_name}" description = var.iam_policy_description policy = data.aws_iam_policy_document.ssm[count.index].json + + tags = module.this.tags } resource "aws_iam_role_policy_attachment" "ssm" { From 517135d00f2582de89140b6e7feb48864b96eeaf Mon Sep 17 00:00:00 2001 From: Nate McCurdy Date: Wed, 11 Oct 2023 14:33:25 -0700 Subject: [PATCH 2/3] fix: Allow for custom_iam_policy_arns that don't exist yet (#46) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PROBLEM: Prior to this, specifying `custom_iam_policy_arns` for IAM Policies that do not exist yet and would be created in the same Terraform run that creates the Lambda Execution Role would cause the following error: ``` │ Error: Invalid for_each argument │ │ on .terraform/modules/foo.test_lambda/iam-role.tf line 81, in resource "aws_iam_role_policy_attachment" "custom": │ 81: for_each = local.enabled && length(var.custom_iam_policy_arns) > 0 ? var.custom_iam_policy_arns : toset([]) │ ├──────────────── │ │ local.enabled is true │ │ var.custom_iam_policy_arns is set of string with 3 elements │ │ The "for_each" set includes values derived from resource attributes that cannot be determined until apply, and so Terraform cannot determine the full set of keys that will identify the instances of this resource. │ │ When working with unknown values in for_each, it's better to use a map value where the keys are defined statically in your configuration and where only the values contain apply-time results. │ │ Alternatively, you could use the -target planning option to first apply only the resources that the for_each value depends on, and then apply a second time to fully converge. ``` This is due to the ARN's of those policies not being known and the usage of sets in `for_each` for the `aws_iam_role_policy_attachment` resource. As the set's values are unknown at apply time, Terraform can't create a dependency graph. SOLUTION: * Don't use `toset()` for the `for_each` in the `aws_iam_role_policy_attachment` resource. * Build a map of ARNs to attach to the Lambda execution role. Use this map as the value of `for_each`. OUTCOME: * Terraform doesn't error when using `aws_iam_role_policy_attachment` for policies that don't exist yet but will exist after the apply. --- iam-role.tf | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/iam-role.tf b/iam-role.tf index 1fd1d84..397ab7d 100644 --- a/iam-role.tf +++ b/iam-role.tf @@ -1,3 +1,7 @@ +locals { + custom_iam_policy_arns_map = length(var.custom_iam_policy_arns) > 0 ? { for i, arn in var.custom_iam_policy_arns : i => arn } : {} +} + resource "aws_iam_role" "this" { count = local.enabled ? 1 : 0 @@ -82,8 +86,8 @@ resource "aws_iam_role_policy_attachment" "ssm" { } resource "aws_iam_role_policy_attachment" "custom" { - for_each = local.enabled && length(var.custom_iam_policy_arns) > 0 ? var.custom_iam_policy_arns : toset([]) + for_each = local.enabled ? local.custom_iam_policy_arns_map : {} role = aws_iam_role.this[0].name - policy_arn = each.key + policy_arn = each.value } From 18b86208f9aaa899716e15fc712ed54bd14775bc Mon Sep 17 00:00:00 2001 From: Nate McCurdy Date: Wed, 11 Oct 2023 14:37:02 -0700 Subject: [PATCH 3/3] fix: Add null/label context tags to the aws_lambda_function resource (#44) Problem: Prior to this, the `aws_lambda_function` resource was not getting tagged at all when passing just the null/label context into the module. For example, this would end up with a completely untagged Lambda function even though I am passing the context from a standard null/label declaration: ``` module "test" { source = "cloudposse/lambda-function/aws" version = "0.5.1" function_name = "${module.this.id}-test" attributes = ["foo"] description = var.lambda_description s3_bucket = var.lambda_s3_bucket s3_key = var.lambda_s3_key runtime = var.lambda_runtime handler = var.lambda_handler context = module.this.context } ``` To get any tags on the lambda, the `tags` attribute must be used: ``` module "test" { source = "cloudposse/lambda-function/aws" version = "0.5.1" function_name = "${module.this.id}-test" attributes = ["foo"] description = var.lambda_description s3_bucket = var.lambda_s3_bucket s3_key = var.lambda_s3_key runtime = var.lambda_runtime handler = var.lambda_handler context = module.this.context tags = module.this.tags } ``` The requirement of passing an explicit `tags` attribute is not how other CloudPosse modules work. In most other CloudPosse modules, you can just pass around a null/label context and the tags will automatically show up in child resources. Solution: Use `tags = module.this.tags` on the `aws_lambda_function` resource. Outcome: * The `aws_lambda_function` resource is tagged with the implicit tags passed in via `context`. * Tags from the `tags` variable are still present, but are now merged with the tags from `context`. * This module follows the convetion of other CloudPosse modules. * People used to CloudPosse modules will have an easier time using this module. Co-authored-by: Dan Miller --- main.tf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/main.tf b/main.tf index a9ca4d6..b7b9990 100644 --- a/main.tf +++ b/main.tf @@ -39,7 +39,7 @@ resource "aws_lambda_function" "this" { s3_key = var.s3_key s3_object_version = var.s3_object_version source_code_hash = var.source_code_hash - tags = var.tags + tags = module.this.tags timeout = var.timeout dynamic "dead_letter_config" {