Skip to content

Commit 257c450

Browse files
authored
fix(ecs_fargate): 🐛 fix defect in parsing role name from ARN, avoid data src (#40)
* refactor: parse role names correctly from ARN Need to account for arbitrary role path characters in the ARN's resource component. AWS IAM API enforces that all paths must begin and end with '/', so it is safe to parse by splitting on '/'
1 parent d3f35e6 commit 257c450

File tree

2 files changed

+6
-17
lines changed

2 files changed

+6
-17
lines changed

modules/ecs_fargate/iam.tf

Lines changed: 6 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ locals {
1515
create_dd_secret_perms = var.dd_api_key_secret != null
1616
edit_execution_role = var.execution_role != null && local.create_dd_secret_perms
1717
create_execution_role = var.execution_role == null && local.create_dd_secret_perms
18+
parsed_exec_role_name = var.execution_role == null ? null : split("/", var.execution_role.arn)[length(split("/", var.execution_role.arn)) - 1]
1819
}
1920

2021
# ==============================
@@ -39,14 +40,9 @@ resource "aws_iam_policy" "dd_secret_access" {
3940
# ==============================
4041
# Case 1: User provides existing Task Execution Role
4142
# ==============================
42-
data "aws_iam_role" "ecs_task_exec_role" {
43-
count = local.edit_execution_role ? 1 : 0
44-
name = element(split("/", var.execution_role.arn), 1)
45-
}
46-
4743
resource "aws_iam_role_policy_attachment" "existing_role_dd_secret" {
4844
count = local.edit_execution_role ? 1 : 0
49-
role = data.aws_iam_role.ecs_task_exec_role[0].name
45+
role = local.parsed_exec_role_name
5046
policy_arn = aws_iam_policy.dd_secret_access[0].arn
5147
}
5248

@@ -89,8 +85,9 @@ resource "aws_iam_role_policy_attachment" "new_role_dd_secret" {
8985
# in order to add permissions for the ecs_fargate check
9086

9187
locals {
92-
edit_task_role = var.task_role != null
93-
create_task_role = var.task_role == null
88+
edit_task_role = var.task_role != null
89+
create_task_role = var.task_role == null
90+
parsed_task_role_name = var.task_role == null ? null : split("/", var.task_role.arn)[length(split("/", var.task_role.arn)) - 1]
9491
}
9592

9693
# ==============================
@@ -116,16 +113,10 @@ resource "aws_iam_policy" "dd_ecs_task_permissions" {
116113
# ==============================
117114
# Case 1: User provides existing Task Role
118115
# ==============================
119-
120-
data "aws_iam_role" "ecs_task_role" {
121-
count = local.edit_task_role ? 1 : 0
122-
name = element(split("/", var.task_role.arn), 1)
123-
}
124-
125116
# Always attach `dd_ecs_task_permissions`
126117
resource "aws_iam_role_policy_attachment" "existing_role_ecs_task_permissions" {
127118
count = local.edit_task_role ? 1 : 0
128-
role = data.aws_iam_role.ecs_task_role[0].name
119+
role = local.parsed_task_role_name
129120
policy_arn = aws_iam_policy.dd_ecs_task_permissions.arn
130121
}
131122

modules/ecs_fargate/main.tf

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -161,8 +161,6 @@ resource "aws_ecs_task_definition" "this" {
161161
track_latest = var.track_latest
162162

163163
depends_on = [
164-
data.aws_iam_role.ecs_task_role,
165-
data.aws_iam_role.ecs_task_exec_role,
166164
aws_iam_role.new_ecs_task_role,
167165
aws_iam_role.new_ecs_task_execution_role,
168166
]

0 commit comments

Comments
 (0)