From 4b918ccce45cb0507021f3ea838e7f5f40f49f2f Mon Sep 17 00:00:00 2001 From: Dominic Robinson Date: Mon, 27 Nov 2023 13:45:18 +0000 Subject: [PATCH 01/10] DSOS-2406: align SecretsManager options with SSM --- locals.tf | 21 +++++++++++++++++++++ main.tf | 40 +++++++++++++++++++++++++++++++++++++--- variables.tf | 12 +++++++++--- 3 files changed, 67 insertions(+), 6 deletions(-) diff --git a/locals.tf b/locals.tf index 6f8aa85..0140fcf 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.ssecretsmanager_secret != 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 3fc4bad..a397969 100644 --- a/main.tf +++ b/main.tf @@ -230,21 +230,55 @@ resource "aws_ssm_parameter" "placeholder" { } } +#------------------------------------------------------------------------------ +# SecretManager Secrets +#------------------------------------------------------------------------------ + +resource "random_password" "secrets" { + for_each = local.secretsmanager_random_passwords + + length = each.value.length + special = each.value.special +} + 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 #checkov:skip=CKV2_AWS_57: Ensure Secrets Manager secrets should have automatic rotation enabled for_each = var.secretsmanager_secrets - 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, + local.secretsmanager_secrets_file + ) + + secret_id = aws_secretsmanager_secret.this[each.key] + secret_string = each.value.value +} + +resource "aws_secretsmanager_secret_version" "placeholder" { + for_each = local.secretsmanager_secrets_default + + secret_id = aws_secretsmanager_secret.this[each.key].id + secret_string = each.value.value + + lifecycle { + ignore_changes = [secret_string] + } +} + #------------------------------------------------------------------------------ # Instance IAM role extra permissions # Allow GetParameter to the EC2 scoped SSM parameter path diff --git a/variables.tf b/variables.tf index a9d352a..0b584e3 100644 --- a/variables.tf +++ b/variables.tf @@ -197,10 +197,16 @@ 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 = {} } From 37ccac7af743613df98ece7861c1f4dde097e729 Mon Sep 17 00:00:00 2001 From: Dominic Robinson Date: Mon, 27 Nov 2023 13:48:24 +0000 Subject: [PATCH 02/10] fix --- main.tf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/main.tf b/main.tf index a397969..8d332bd 100644 --- a/main.tf +++ b/main.tf @@ -247,7 +247,7 @@ resource "aws_secretsmanager_secret" "placeholder" { #checkov:skip=CKV2_AWS_57: Ensure Secrets Manager secrets should have automatic rotation enabled for_each = var.secretsmanager_secrets - name = /${var.secretsmanager_secrets_prefix}${var.name}/${each.key} + 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 From cdf81941b909bd16e4957a48831eef157b8efa17 Mon Sep 17 00:00:00 2001 From: Dominic Robinson Date: Mon, 27 Nov 2023 13:51:19 +0000 Subject: [PATCH 03/10] fix --- locals.tf | 2 +- main.tf | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/locals.tf b/locals.tf index 0140fcf..c03602a 100644 --- a/locals.tf +++ b/locals.tf @@ -43,7 +43,7 @@ locals { }) if value.value == null && value.random != null } secretsmanager_secrets_default = { - for key, value in var.ssecretsmanager_secret != null ? var.secretsmanager_secrets : {} : + for key, value in var.secretsmanager_secret != null ? var.secretsmanager_secrets : {} : key => merge(value, { value = "placeholder, overwrite me outside of terraform" }) if value.value == null && value.random == null diff --git a/main.tf b/main.tf index 8d332bd..96bfaa8 100644 --- a/main.tf +++ b/main.tf @@ -261,7 +261,6 @@ resource "aws_secretsmanager_secret_version" "fixed" { for_each = merge( local.secretsmanager_secrets_value, local.secretsmanager_secrets_random, - local.secretsmanager_secrets_file ) secret_id = aws_secretsmanager_secret.this[each.key] From aa0ad84da89f2f36c4a324c47d4e1182314da663 Mon Sep 17 00:00:00 2001 From: Dominic Robinson Date: Mon, 27 Nov 2023 13:52:50 +0000 Subject: [PATCH 04/10] fix --- locals.tf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/locals.tf b/locals.tf index c03602a..2b1cab7 100644 --- a/locals.tf +++ b/locals.tf @@ -43,7 +43,7 @@ locals { }) if value.value == null && value.random != null } secretsmanager_secrets_default = { - for key, value in var.secretsmanager_secret != null ? var.secretsmanager_secrets : {} : + 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 From 6982188766d533fb5ca5a2e4280daf181d46f7b5 Mon Sep 17 00:00:00 2001 From: Dominic Robinson Date: Mon, 27 Nov 2023 13:57:30 +0000 Subject: [PATCH 05/10] fix --- main.tf | 27 ++++++++++++++++++++++----- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/main.tf b/main.tf index 96bfaa8..52a738e 100644 --- a/main.tf +++ b/main.tf @@ -241,11 +241,28 @@ resource "random_password" "secrets" { 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 @@ -263,14 +280,14 @@ resource "aws_secretsmanager_secret_version" "fixed" { local.secretsmanager_secrets_random, ) - secret_id = aws_secretsmanager_secret.this[each.key] + 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.this[each.key].id + secret_id = aws_secretsmanager_secret.placeholder[each.key].id secret_string = each.value.value lifecycle { From ef78bb346f5362c7cdf0ac1b8568ffc56916b76d Mon Sep 17 00:00:00 2001 From: Dominic Robinson Date: Mon, 27 Nov 2023 14:37:45 +0000 Subject: [PATCH 06/10] fix --- variables.tf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/variables.tf b/variables.tf index 0b584e3..2a34aea 100644 --- a/variables.tf +++ b/variables.tf @@ -208,7 +208,7 @@ variable "secretsmanager_secrets" { })) value = optional(string) })) - default = {} + default = null } variable "cloudwatch_metric_alarms" { From 021e47f2296008054eae817b1c8d700627d43c60 Mon Sep 17 00:00:00 2001 From: Dominic Robinson Date: Mon, 27 Nov 2023 14:56:49 +0000 Subject: [PATCH 07/10] fix --- main.tf | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/main.tf b/main.tf index 52a738e..3bc2b73 100644 --- a/main.tf +++ b/main.tf @@ -251,7 +251,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, { @@ -266,7 +266,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 f1d4b2d727d74aa736156081647fa934d94c0fb6 Mon Sep 17 00:00:00 2001 From: Dominic Robinson Date: Wed, 29 Nov 2023 14:35:22 +0000 Subject: [PATCH 08/10] fix --- main.tf | 31 ++++++++++--------------------- 1 file changed, 10 insertions(+), 21 deletions(-) diff --git a/main.tf b/main.tf index 3bc2b73..35b7024 100644 --- a/main.tf +++ b/main.tf @@ -259,21 +259,6 @@ resource "aws_secretsmanager_secret" "fixed" { }) } -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 - 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 - 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, @@ -284,15 +269,19 @@ resource "aws_secretsmanager_secret_version" "fixed" { secret_string = each.value.value } -resource "aws_secretsmanager_secret_version" "placeholder" { +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 for_each = local.secretsmanager_secrets_default - secret_id = aws_secretsmanager_secret.placeholder[each.key].id - secret_string = each.value.value + name = "/${var.secretsmanager_secrets_prefix}${var.name}/${each.key}" + description = each.value.description + kms_key_id = each.value.kms_key_id + recovery_window_in_days = each.value.recovery_window_in_days - lifecycle { - ignore_changes = [secret_string] - } + tags = merge(local.tags, { + Name = "${var.name}-${each.key}" + }) } #------------------------------------------------------------------------------ From 48df51c9a36108992294134266f12420146f5d09 Mon Sep 17 00:00:00 2001 From: Dominic Robinson Date: Wed, 29 Nov 2023 14:46:40 +0000 Subject: [PATCH 09/10] fix --- locals.tf | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/locals.tf b/locals.tf index 2b1cab7..65f6107 100644 --- a/locals.tf +++ b/locals.tf @@ -44,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 01e245136628b3aece2ed3d0a14bd08e1f8b8a23 Mon Sep 17 00:00:00 2001 From: Dominic Robinson Date: Wed, 29 Nov 2023 16:50:03 +0000 Subject: [PATCH 10/10] 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 cd3b102..9258e8f 100644 --- a/test/unit-test/main.tf +++ b/test/unit-test/main.tf @@ -20,6 +20,8 @@ module "ec2_test_instance" { ebs_volume_config = lookup(each.value, "ebs_volume_config", {}) ebs_volumes = lookup(each.value, "ebs_volumes", {}) ebs_volume_tags = lookup(each.value, "ebs_volume_tags", {}) + 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) route53_records = merge(local.ec2_test.route53_records, lookup(each.value, "route53_records", {}))