From d997164b6a46bc321821aed97879a5d77e53dc07 Mon Sep 17 00:00:00 2001 From: Dominic Robinson Date: Mon, 27 Nov 2023 14:42:32 +0000 Subject: [PATCH 1/7] DSOS-2406: align secrets functionality with ssm --- locals.tf | 21 +++++++++++++++++++ main.tf | 58 ++++++++++++++++++++++++++++++++++++++++++++++------ variables.tf | 14 +++++++++---- 3 files changed, 83 insertions(+), 10 deletions(-) diff --git a/locals.tf b/locals.tf index 8904a49..38a66b7 100644 --- a/locals.tf +++ b/locals.tf @@ -28,6 +28,27 @@ locals { }) if value.value == null && value.random == null } + secretsmanager_random_passwords = { + for key, value in var.secretsmanager_secrets != null ? var.secretsmanager_secrets : {} : + key => value.random if value.random != null + } + secretsmanager_secrets_value = { + for key, value in var.secretsmanager_secrets != null ? var.secretsmanager_secrets : {} : + key => value if value.value != null + } + secretsmanager_secrets_random = { + for key, value in var.secretsmanager_secrets != null ? var.secretsmanager_secrets : {} : + key => merge(value, { + value = random_password.secrets[key].result + }) if value.value == null && value.random != null + } + secretsmanager_secrets_default = { + for key, value in var.secretsmanager_secrets != null ? var.secretsmanager_secrets : {} : + key => merge(value, { + value = "placeholder, overwrite me outside of terraform" + }) if value.value == null && value.random == null + } + ami_block_device_mappings = { for bdm in data.aws_ami.this.block_device_mappings : bdm.device_name => bdm } diff --git a/main.tf b/main.tf index 1962ed5..13f4bb0 100644 --- a/main.tf +++ b/main.tf @@ -228,21 +228,67 @@ resource "aws_ssm_parameter" "placeholder" { } } +resource "random_password" "secrets" { + for_each = local.secretsmanager_random_passwords + + length = each.value.length + special = each.value.special +} + +resource "aws_secretsmanager_secret" "fixed" { + # skipped check as the secret value is defined by terraform so cannot be rotated by AWS + #checkov:skip=CKV2_AWS_57: Ensure Secrets Manager secrets should have automatic rotation enabled + for_each = merge( + local.secretsmanager_secrets_value, + local.secretsmanager_secrets_random, + ) + + name = "/${var.secretsmanager_secrets_prefix}${var.name}/${each.key}" + description = each.value.description + kms_key_id = each.value.kms_key_id != null ? try(var.environment.kms_keys[each.value.kms_key_id].arn, each.value.kms_key_id) : null + recovery_window_in_days = each.value.recovery_window_in_days + + tags = merge(local.tags, { + Name = "${var.name}-${each.key}" + }) +} + resource "aws_secretsmanager_secret" "placeholder" { - # skipped check to keep consistent behaviour between ssm params and secrets - # Rotation can be added later as a configurable option. Some will want it, for some it will break things + # Rotation can be added later as a configurable option #checkov:skip=CKV2_AWS_57: Ensure Secrets Manager secrets should have automatic rotation enabled - for_each = var.secretsmanager_secrets + for_each = local.secretsmanager_secrets_default - name = "/${var.secretsmanager_secrets_prefix}${var.name}/${each.key}" - description = each.value.description - kms_key_id = each.value.kms_key_id + name = "/${var.secretsmanager_secrets_prefix}${var.name}/${each.key}" + description = each.value.description + kms_key_id = each.value.kms_key_id != null ? try(var.environment.kms_keys[each.value.kms_key_id].arn, each.value.kms_key_id) : null + recovery_window_in_days = each.value.recovery_window_in_days tags = merge(local.tags, { Name = "${var.name}-${each.key}" }) } +resource "aws_secretsmanager_secret_version" "fixed" { + for_each = merge( + local.secretsmanager_secrets_value, + local.secretsmanager_secrets_random, + ) + + secret_id = aws_secretsmanager_secret.fixed[each.key].id + secret_string = each.value.value +} + +resource "aws_secretsmanager_secret_version" "placeholder" { + for_each = local.secretsmanager_secrets_default + + secret_id = aws_secretsmanager_secret.placeholder[each.key].id + secret_string = each.value.value + + lifecycle { + ignore_changes = [secret_string] + } +} + resource "aws_iam_role" "this" { name = "${var.iam_resource_names_prefix}-role-${var.name}" path = "/" diff --git a/variables.tf b/variables.tf index 6e3e91a..5b2c91e 100644 --- a/variables.tf +++ b/variables.tf @@ -222,12 +222,18 @@ variable "ssm_parameters" { } variable "secretsmanager_secrets" { - description = "A map of secretsmanager secrets to create. No value is created, add a value outside of terraform" + description = "A map of secretsmanager secrets to create. Set a specific value or a randomly generated value. If neither random or value are set, a placeholder value is created which can be updated outside of terraform" type = map(object({ - description = optional(string) - kms_key_id = optional(string) + description = optional(string) + kms_key_id = optional(string) + recovery_window_in_days = optional(number) + random = optional(object({ + length = number + special = optional(bool) + })) + value = optional(string) })) - default = {} + default = null } variable "lb_target_groups" { From 1234c1f32ef484c287a84e6f7dae01c71b9866df Mon Sep 17 00:00:00 2001 From: Dominic Robinson Date: Mon, 27 Nov 2023 14:57:03 +0000 Subject: [PATCH 2/7] fix --- main.tf | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/main.tf b/main.tf index 13f4bb0..12c7a56 100644 --- a/main.tf +++ b/main.tf @@ -245,7 +245,7 @@ resource "aws_secretsmanager_secret" "fixed" { name = "/${var.secretsmanager_secrets_prefix}${var.name}/${each.key}" description = each.value.description - kms_key_id = each.value.kms_key_id != null ? try(var.environment.kms_keys[each.value.kms_key_id].arn, each.value.kms_key_id) : null + kms_key_id = each.value.kms_key_id recovery_window_in_days = each.value.recovery_window_in_days tags = merge(local.tags, { @@ -260,7 +260,7 @@ resource "aws_secretsmanager_secret" "placeholder" { name = "/${var.secretsmanager_secrets_prefix}${var.name}/${each.key}" description = each.value.description - kms_key_id = each.value.kms_key_id != null ? try(var.environment.kms_keys[each.value.kms_key_id].arn, each.value.kms_key_id) : null + kms_key_id = each.value.kms_key_id recovery_window_in_days = each.value.recovery_window_in_days tags = merge(local.tags, { From 6ab408107bfcf1e1ff9676f15728cfe07229725c Mon Sep 17 00:00:00 2001 From: Dominic Robinson Date: Mon, 27 Nov 2023 15:25:36 +0000 Subject: [PATCH 3/7] test --- main.tf | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/main.tf b/main.tf index 12c7a56..0463420 100644 --- a/main.tf +++ b/main.tf @@ -278,17 +278,6 @@ resource "aws_secretsmanager_secret_version" "fixed" { secret_string = each.value.value } -resource "aws_secretsmanager_secret_version" "placeholder" { - for_each = local.secretsmanager_secrets_default - - secret_id = aws_secretsmanager_secret.placeholder[each.key].id - secret_string = each.value.value - - lifecycle { - ignore_changes = [secret_string] - } -} - resource "aws_iam_role" "this" { name = "${var.iam_resource_names_prefix}-role-${var.name}" path = "/" From 8b083494f154c3e253290471fe018410918b8b21 Mon Sep 17 00:00:00 2001 From: Dominic Robinson Date: Wed, 29 Nov 2023 14:38:09 +0000 Subject: [PATCH 4/7] fix --- main.tf | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/main.tf b/main.tf index 0463420..cb32412 100644 --- a/main.tf +++ b/main.tf @@ -253,6 +253,16 @@ resource "aws_secretsmanager_secret" "fixed" { }) } +resource "aws_secretsmanager_secret_version" "fixed" { + for_each = merge( + local.secretsmanager_secrets_value, + local.secretsmanager_secrets_random, + ) + + secret_id = aws_secretsmanager_secret.fixed[each.key].id + secret_string = each.value.value +} + resource "aws_secretsmanager_secret" "placeholder" { # Rotation can be added later as a configurable option #checkov:skip=CKV2_AWS_57: Ensure Secrets Manager secrets should have automatic rotation enabled @@ -268,16 +278,6 @@ resource "aws_secretsmanager_secret" "placeholder" { }) } -resource "aws_secretsmanager_secret_version" "fixed" { - for_each = merge( - local.secretsmanager_secrets_value, - local.secretsmanager_secrets_random, - ) - - secret_id = aws_secretsmanager_secret.fixed[each.key].id - secret_string = each.value.value -} - resource "aws_iam_role" "this" { name = "${var.iam_resource_names_prefix}-role-${var.name}" path = "/" From 568685b807f7703e78fd750562a5c2a87c154f24 Mon Sep 17 00:00:00 2001 From: Dominic Robinson Date: Wed, 29 Nov 2023 14:45:11 +0000 Subject: [PATCH 5/7] fix --- locals.tf | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/locals.tf b/locals.tf index 38a66b7..6f4a9b0 100644 --- a/locals.tf +++ b/locals.tf @@ -23,9 +23,7 @@ locals { } ssm_parameters_default = { for key, value in var.ssm_parameters != null ? var.ssm_parameters : {} : - key => merge(value, { - value = "placeholder, overwrite me outside of terraform" - }) if value.value == null && value.random == null + key => value if value.value == null && value.random == null } secretsmanager_random_passwords = { From 3912d202ba07c259da1f36cd0870e148383394b2 Mon Sep 17 00:00:00 2001 From: Dominic Robinson Date: Wed, 29 Nov 2023 14:48:20 +0000 Subject: [PATCH 6/7] fix --- locals.tf | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/locals.tf b/locals.tf index 6f4a9b0..a51f299 100644 --- a/locals.tf +++ b/locals.tf @@ -23,7 +23,9 @@ locals { } ssm_parameters_default = { for key, value in var.ssm_parameters != null ? var.ssm_parameters : {} : - key => value if value.value == null && value.random == null + key => merge(value, { + value = "placeholder, overwrite me outside of terraform" + }) if value.value == null && value.random == null } secretsmanager_random_passwords = { @@ -42,9 +44,7 @@ locals { } secretsmanager_secrets_default = { for key, value in var.secretsmanager_secrets != null ? var.secretsmanager_secrets : {} : - key => merge(value, { - value = "placeholder, overwrite me outside of terraform" - }) if value.value == null && value.random == null + key => value if value.value == null && value.random == null } ami_block_device_mappings = { From 8a2419196ce99d39367e2b90b97763331fd8ac45 Mon Sep 17 00:00:00 2001 From: Dominic Robinson Date: Wed, 29 Nov 2023 16:49:09 +0000 Subject: [PATCH 7/7] add unit test --- test/unit-test/main.tf | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/unit-test/main.tf b/test/unit-test/main.tf index 5f6a65b..76aa021 100644 --- a/test/unit-test/main.tf +++ b/test/unit-test/main.tf @@ -15,6 +15,8 @@ module "ec2_test_autoscaling_group" { ebs_volumes_copy_all_from_ami = try(each.value.ebs_volumes_copy_all_from_ami, true) ebs_volume_config = lookup(each.value, "ebs_volume_config", {}) ebs_volumes = lookup(each.value, "ebs_volumes", {}) + secretsmanager_secrets_prefix = lookup(each.value, "secretsmanager_secrets_prefix", "test/") + secretsmanager_secrets = lookup(each.value, "secretsmanager_secrets", null) ssm_parameters_prefix = lookup(each.value, "ssm_parameters_prefix", "test/") ssm_parameters = lookup(each.value, "ssm_parameters", null) autoscaling_group = merge(local.ec2_test.autoscaling_group, lookup(each.value, "autoscaling_group", {}))