fix: cleans up principals lambda logic to separate policy doc (#105)

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Andriy Knysh <aknysh@users.noreply.github.com>
cloudposse · May 26, 2023 · 862fc85 · 862fc85
1 parent d9effd7
commit 862fc85
Show file tree

Hide file tree

Showing 3 changed files with 38 additions and 85 deletions.
diff --git a/README.md b/README.md
@@ -176,6 +176,7 @@ Available targets:
 | [aws_ecr_repository.name](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/ecr_repository) | resource |
 | [aws_ecr_repository_policy.name](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/ecr_repository_policy) | resource |
 | [aws_iam_policy_document.empty](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/iam_policy_document) | data source |
+| [aws_iam_policy_document.lambda_access](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/iam_policy_document) | data source |
 | [aws_iam_policy_document.resource](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/iam_policy_document) | data source |
 | [aws_iam_policy_document.resource_full_access](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/iam_policy_document) | data source |
 | [aws_iam_policy_document.resource_push_access](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/iam_policy_document) | data source |

diff --git a/docs/terraform.md b/docs/terraform.md
@@ -26,6 +26,7 @@
 | [aws_ecr_repository.name](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/ecr_repository) | resource |
 | [aws_ecr_repository_policy.name](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/ecr_repository_policy) | resource |
 | [aws_iam_policy_document.empty](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/iam_policy_document) | data source |
+| [aws_iam_policy_document.lambda_access](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/iam_policy_document) | data source |
 | [aws_iam_policy_document.resource](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/iam_policy_document) | data source |
 | [aws_iam_policy_document.resource_full_access](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/iam_policy_document) | data source |
 | [aws_iam_policy_document.resource_push_access](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/iam_policy_document) | data source |

diff --git a/main.tf b/main.tf
@@ -2,7 +2,8 @@ locals {
   principals_readonly_access_non_empty = length(var.principals_readonly_access) > 0
   principals_push_access_non_empty     = length(var.principals_push_access) > 0
   principals_full_access_non_empty     = length(var.principals_full_access) > 0
-  ecr_need_policy                      = length(var.principals_full_access) + length(var.principals_readonly_access) + length(var.principals_push_access) > 0
+  principals_lambda_non_empty          = length(var.principals_lambda) > 0
+  ecr_need_policy                      = length(var.principals_full_access) + length(var.principals_readonly_access) + length(var.principals_push_access) + length(var.principals_lambda) > 0
 }
 
 locals {
@@ -99,8 +100,7 @@ data "aws_iam_policy_document" "resource_readonly_access" {
     effect = "Allow"
 
     principals {
-      type = "AWS"
-
+      type        = "AWS"
       identifiers = var.principals_readonly_access
     }
 
@@ -118,49 +118,6 @@ data "aws_iam_policy_document" "resource_readonly_access" {
       "ecr:ListTagsForResource",
     ]
   }
-
-  dynamic "statement" {
-    for_each = length(var.principals_lambda) > 0 ? [1] : []
-
-    content {
-      sid    = "LambdaECRImageCrossAccountRetrievalPolicy"
-      effect = "Allow"
-      actions = [
-        "ecr:BatchGetImage",
-        "ecr:GetDownloadUrlForLayer"
-      ]
-
-      principals {
-        type        = "Service"
-        identifiers = ["lambda.amazonaws.com"]
-      }
-
-      condition {
-        test     = "StringLike"
-        values   = formatlist("arn:%s:lambda:*:%s:function:*", data.aws_partition.current.partition, var.principals_lambda)
-        variable = "aws:sourceArn"
-      }
-    }
-  }
-
-  dynamic "statement" {
-    for_each = length(var.principals_lambda) > 0 ? [1] : []
-    content {
-      sid    = "CrossAccountPermission"
-      effect = "Allow"
-
-      principals {
-        type = "AWS"
-
-        identifiers = formatlist("arn:%s:iam::%s:root", data.aws_partition.current.partition, var.principals_lambda)
-      }
-
-      actions = [
-        "ecr:BatchGetImage",
-        "ecr:GetDownloadUrlForLayer"
-      ]
-    }
-  }
 }
 
 data "aws_iam_policy_document" "resource_push_access" {
@@ -171,8 +128,7 @@ data "aws_iam_policy_document" "resource_push_access" {
     effect = "Allow"
 
     principals {
-      type = "AWS"
-
+      type        = "AWS"
       identifiers = var.principals_push_access
     }
 
@@ -195,54 +151,48 @@ data "aws_iam_policy_document" "resource_full_access" {
     effect = "Allow"
 
     principals {
-      type = "AWS"
-
+      type        = "AWS"
       identifiers = var.principals_full_access
     }
 
     actions = ["ecr:*"]
   }
+}
 
-  dynamic "statement" {
-    for_each = length(var.principals_lambda) > 0 ? [1] : []
+data "aws_iam_policy_document" "lambda_access" {
+  count = module.this.enabled && length(var.principals_lambda) > 0 ? 1 : 0
 
-    content {
-      sid    = "LambdaECRImageCrossAccountRetrievalPolicy"
-      effect = "Allow"
-      actions = [
-        "ecr:BatchGetImage",
-        "ecr:GetDownloadUrlForLayer"
-      ]
-
-      principals {
-        type        = "Service"
-        identifiers = ["lambda.amazonaws.com"]
-      }
+  statement {
+    sid    = "LambdaECRImageCrossAccountRetrievalPolicy"
+    effect = "Allow"
+    actions = [
+      "ecr:BatchGetImage",
+      "ecr:GetDownloadUrlForLayer"
+    ]
 
-      condition {
-        test     = "StringLike"
-        values   = formatlist("arn:%s:lambda:*:%s:function:*", data.aws_partition.current.partition, var.principals_lambda)
-        variable = "aws:sourceArn"
-      }
+    principals {
+      type        = "Service"
+      identifiers = ["lambda.amazonaws.com"]
     }
-  }
-
-  dynamic "statement" {
-    for_each = length(var.principals_lambda) > 0 ? [1] : []
-    content {
-      sid    = "CrossAccountPermission"
-      effect = "Allow"
 
-      principals {
-        type = "AWS"
+    condition {
+      test     = "StringLike"
+      values   = local.principals_lambda_non_empty ? formatlist("arn:%s:lambda:*:%s:function:*", data.aws_partition.current.partition, var.principals_lambda) : []
+      variable = "aws:SourceArn"
+    }
+  }
 
-        identifiers = formatlist("arn:%s:iam::%s:root", data.aws_partition.current.partition, var.principals_lambda)
-      }
+  statement {
+    sid    = "CrossAccountPermission"
+    effect = "Allow"
+    actions = [
+      "ecr:BatchGetImage",
+      "ecr:GetDownloadUrlForLayer"
+    ]
 
-      actions = [
-        "ecr:BatchGetImage",
-        "ecr:GetDownloadUrlForLayer"
-      ]
+    principals {
+      type        = "AWS"
+      identifiers = local.principals_lambda_non_empty ? formatlist("arn:%s:iam::%s:root", data.aws_partition.current.partition, var.principals_lambda) : []
     }
   }
 }
@@ -253,11 +203,12 @@ data "aws_iam_policy_document" "resource" {
   override_policy_documents = distinct([
     local.principals_push_access_non_empty ? data.aws_iam_policy_document.resource_push_access[0].json : data.aws_iam_policy_document.empty[0].json,
     local.principals_full_access_non_empty ? data.aws_iam_policy_document.resource_full_access[0].json : data.aws_iam_policy_document.empty[0].json,
+    local.principals_lambda_non_empty ? data.aws_iam_policy_document.lambda_access[0].json : data.aws_iam_policy_document.empty[0].json,
   ])
 }
 
 resource "aws_ecr_repository_policy" "name" {
   for_each   = toset(local.ecr_need_policy && module.this.enabled ? local.image_names : [])
   repository = aws_ecr_repository.name[each.value].name
-  policy     = join("", data.aws_iam_policy_document.resource.*.json)
+  policy     = join("", data.aws_iam_policy_document.resource[*].json)
 }