Merge pull request #4 from DataDog/gabedos/iam-task-role

fix: Create task execution role

Author: gabedos

modules/ecs_fargate/iam.tf

@@ -1,7 +1,15 @@
+# ==============================
+# Task Execution Role
+# ==============================
+
+# Will create or edit the *task execution role*
+# only if the user provides a Datadog API key secret ARN
+# in order to provide permissions to access the secret
+
 locals {
-  edit_execution_role    = var.execution_role_arn != null
-  create_execution_role  = !local.edit_execution_role
   create_dd_secret_perms = var.dd_api_key_secret_arn != null
+  edit_execution_role    = var.execution_role_arn != null && local.create_dd_secret_perms
+  create_execution_role  = var.execution_role_arn == null && local.create_dd_secret_perms
 }
 
 # ==============================
@@ -23,6 +31,63 @@ resource "aws_iam_policy" "dd_secret_access" {
   policy = data.aws_iam_policy_document.dd_secret_access[0].json
 }
 
+# ==============================
+# Case 1: User provides existing Task Execution Role
+# ==============================
+data "aws_iam_role" "ecs_task_exec_role" {
+  count = local.edit_execution_role ? 1 : 0
+  name  = element(split("/", var.execution_role_arn), 1)
+}
+
+resource "aws_iam_role_policy_attachment" "existing_role_dd_secret" {
+  count      = local.edit_execution_role ? 1 : 0
+  role       = data.aws_iam_role.ecs_task_exec_role[0].name
+  policy_arn = aws_iam_policy.dd_secret_access[0].arn
+}
+
+# ==============================
+# Case 2: Create a Task Execution Role
+# ==============================
+resource "aws_iam_role" "new_ecs_task_execution_role" {
+  count = local.create_execution_role ? 1 : 0
+  name  = "${var.family}-ecs-task-exec-role"
+
+  assume_role_policy = jsonencode({
+    Version = "2012-10-17"
+    Statement = [{
+      Effect = "Allow"
+      Principal = {
+        Service = "ecs-tasks.amazonaws.com"
+      }
+      Action = "sts:AssumeRole"
+    }]
+  })
+}
+
+resource "aws_iam_role_policy_attachment" "new_ecs_task_execution_role_policy" {
+  count      = local.create_execution_role ? 1 : 0
+  role       = aws_iam_role.new_ecs_task_execution_role[0].name
+  policy_arn = "arn:aws:iam::aws:policy/service-role/AmazonECSTaskExecutionRolePolicy"
+}
+
+resource "aws_iam_role_policy_attachment" "new_role_dd_secret" {
+  count      = local.create_execution_role ? 1 : 0
+  role       = aws_iam_role.new_ecs_task_execution_role[0].name
+  policy_arn = aws_iam_policy.dd_secret_access[0].arn
+}
+
+# ==============================
+# Task Role
+# ==============================
+
+# Will create or edit the *task role* always
+# in order to add permissions for the ecs_fargate check
+
+locals {
+  edit_task_role    = var.task_role_arn != null
+  create_task_role  = var.task_role_arn == null
+}
+
 # ==============================
 # ECS Task Permissions Policy
 # ==============================
@@ -44,11 +109,12 @@ resource "aws_iam_policy" "dd_ecs_task_permissions" {
 }
 
 # ==============================
-# Case 1: User provides existing Task Execution Role
+# Case 1: User provides existing Task Role
 # ==============================
+
 data "aws_iam_role" "ecs_task_role" {
-  count = local.edit_execution_role ? 1 : 0
-  name  = element(split("/", var.execution_role_arn), 1)
+  count = local.edit_task_role ? 1 : 0
+  name  = element(split("/", var.task_role_arn), 1)
 }
 
 # Always attach `dd_ecs_task_permissions`
@@ -58,19 +124,14 @@ resource "aws_iam_role_policy_attachment" "existing_role_ecs_task_permissions" {
   policy_arn = aws_iam_policy.dd_ecs_task_permissions.arn
 }
 
-# Conditionally attach `dd_secret_access` only if required
-resource "aws_iam_role_policy_attachment" "existing_role_dd_secret" {
-  count      = local.edit_execution_role && local.create_dd_secret_perms ? 1 : 0
-  role       = data.aws_iam_role.ecs_task_role[0].name
-  policy_arn = aws_iam_policy.dd_secret_access[0].arn
-}
 
 # ==============================
-# Case 2: Create a Task Execution Role
+# Case 2: Create a Task Role
 # ==============================
-resource "aws_iam_role" "ecs_task_execution" {
-  count = local.create_execution_role ? 1 : 0
-  name  = "${var.family}-ecs-task-exec-role"
+
+resource "aws_iam_role" "new_ecs_task_role" {
+  count = local.create_task_role ? 1 : 0
+  name  = "${var.family}-ecs-task-role"
 
   assume_role_policy = jsonencode({
     Version = "2012-10-17"
@@ -84,22 +145,9 @@ resource "aws_iam_role" "ecs_task_execution" {
   })
 }
 
-resource "aws_iam_role_policy_attachment" "ecs_task_execution_policy" {
-  count      = local.create_execution_role ? 1 : 0
-  role       = aws_iam_role.ecs_task_execution[0].name
-  policy_arn = "arn:aws:iam::aws:policy/service-role/AmazonECSTaskExecutionRolePolicy"
-}
-
 # Always attach `dd_ecs_task_permissions`
 resource "aws_iam_role_policy_attachment" "new_role_ecs_task_permissions" {
-  count      = local.create_execution_role ? 1 : 0
-  role       = aws_iam_role.ecs_task_execution[0].name
+  count      = local.create_task_role ? 1 : 0
+  role       = aws_iam_role.new_ecs_task_role[0].name
   policy_arn = aws_iam_policy.dd_ecs_task_permissions.arn
-}
-
-# Conditionally attach `dd_secret_access` only if required
-resource "aws_iam_role_policy_attachment" "new_role_dd_secret" {
-  count      = local.create_execution_role && local.create_dd_secret_perms ? 1 : 0
-  role       = aws_iam_role.ecs_task_execution[0].name
-  policy_arn = aws_iam_policy.dd_secret_access[0].arn
-}
+}

modules/ecs_fargate/main.tf

@@ -23,8 +23,9 @@ resource "aws_ecs_task_definition" "this" {
 
   enable_fault_injection = var.enable_fault_injection
 
-  # If we editted the provided execution role, use that, otherwise use the one we created
-  execution_role_arn = local.edit_execution_role ? var.execution_role_arn : aws_iam_role.ecs_task_execution[0].arn
+  # Prioritize the user-provided task execution role over the one created by the module
+  execution_role_arn = var.execution_role_arn != null ? var.execution_role_arn : (length(aws_iam_role.new_ecs_task_execution_role) > 0 ? aws_iam_role.new_ecs_task_execution_role[0].arn : null)
+
   family             = var.family
 
   # Fargate incompatible parameter
@@ -74,7 +75,8 @@ resource "aws_ecs_task_definition" "this" {
   }
 
   skip_destroy  = var.skip_destroy
-  task_role_arn = var.task_role_arn
+  # Prioritize the user-provided task role over the one created by the module
+  task_role_arn = var.task_role_arn != null ? var.task_role_arn : (length(aws_iam_role.new_ecs_task_role) > 0 ? aws_iam_role.new_ecs_task_role[0].arn : null)
 
   dynamic "volume" {
     for_each = var.volume
@@ -139,11 +141,8 @@ resource "aws_ecs_task_definition" "this" {
   tags = var.tags
 
   depends_on = [
-    aws_iam_role_policy_attachment.ecs_task_execution_policy,
-    aws_iam_role_policy_attachment.existing_role_dd_secret,
-    aws_iam_role_policy_attachment.new_role_ecs_task_permissions,
-    aws_iam_role_policy_attachment.new_role_dd_secret,
-    aws_iam_role_policy_attachment.new_role_ecs_task_permissions
+    aws_iam_role.new_ecs_task_role,
+    aws_iam_role.new_ecs_task_execution_role,
   ]
 
   lifecycle {