From 1403d947f3fe28deeed35f1c61676fbfc619068a Mon Sep 17 00:00:00 2001 From: Kate Sugden Date: Wed, 3 Jul 2024 12:08:19 +0100 Subject: [PATCH 1/8] filter_prefix is optional --- s3/variables.tf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/s3/variables.tf b/s3/variables.tf index 2f4e5964f..81a85060e 100644 --- a/s3/variables.tf +++ b/s3/variables.tf @@ -30,7 +30,7 @@ variable "config" { years = optional(number) })) lifecycle_rules = optional(list(object({ - filter_prefix = string + filter_prefix = optional(string) expiration_days = number enabled = bool }) From 8494bd7d790d5cb617bebc36d32c0f16f8f9c932 Mon Sep 17 00:00:00 2001 From: Kate Sugden Date: Wed, 3 Jul 2024 12:08:52 +0100 Subject: [PATCH 2/8] Filter should be an empty block if no filter_prefix is defined --- s3/main.tf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/s3/main.tf b/s3/main.tf index 7b878f79e..299f61b2d 100644 --- a/s3/main.tf +++ b/s3/main.tf @@ -60,7 +60,7 @@ resource "aws_s3_bucket_lifecycle_configuration" "lifecycle-configuration" { days_after_initiation = 7 } filter { - prefix = rule.value.filter_prefix + prefix = rule.value.filter_prefix != null ? rule.value.filter_prefix : null } expiration { days = rule.value.expiration_days From 0e19bd76be2baf8e134fb3127d680ed9cc963ca8 Mon Sep 17 00:00:00 2001 From: Kate Sugden Date: Wed, 3 Jul 2024 12:09:33 +0100 Subject: [PATCH 3/8] try unskipping checkov 300 --- s3/main.tf | 1 - 1 file changed, 1 deletion(-) diff --git a/s3/main.tf b/s3/main.tf index 299f61b2d..4f8d785cc 100644 --- a/s3/main.tf +++ b/s3/main.tf @@ -51,7 +51,6 @@ resource "aws_s3_bucket_lifecycle_configuration" "lifecycle-configuration" { bucket = aws_s3_bucket.this.id - # checkov:skip=CKV_AWS_300: Ensure S3 lifecycle configuration sets period for aborting failed uploads dynamic "rule" { for_each = var.config.lifecycle_rules content { From 05562c8bc010898bed126412b1b864e8a2a5e106 Mon Sep 17 00:00:00 2001 From: Kate Sugden Date: Wed, 3 Jul 2024 12:25:41 +0100 Subject: [PATCH 4/8] Clarify optional usage of filter_prefix --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 7c6b0b8fd..ee9db95e6 100644 --- a/README.md +++ b/README.md @@ -195,7 +195,7 @@ my-s3-bucket: days: 10 # Integer value. Alternatively years may be specified. versioning: true # Optional lifecycle_rules: # Optional. If present, contains a list of rules. - - filter_prefix: "bananas/" # Optional + - filter_prefix: "bananas/" # Optional. If none, the rule applies to all objects. Use an empty string for a catch-all rule. expiration_days: 10 # Integer value enabled: true # Mandatory flag objects: # Optional. If present, contains a list of objects From bcafe566efe22464331b87acbe63dbc90951dc15 Mon Sep 17 00:00:00 2001 From: Kate Sugden Date: Wed, 3 Jul 2024 12:34:37 +0100 Subject: [PATCH 5/8] Revert "try unskipping checkov 300" This reverts commit 0e19bd76be2baf8e134fb3127d680ed9cc963ca8. --- s3/main.tf | 1 + 1 file changed, 1 insertion(+) diff --git a/s3/main.tf b/s3/main.tf index 4f8d785cc..299f61b2d 100644 --- a/s3/main.tf +++ b/s3/main.tf @@ -51,6 +51,7 @@ resource "aws_s3_bucket_lifecycle_configuration" "lifecycle-configuration" { bucket = aws_s3_bucket.this.id + # checkov:skip=CKV_AWS_300: Ensure S3 lifecycle configuration sets period for aborting failed uploads dynamic "rule" { for_each = var.config.lifecycle_rules content { From 9456eb67b3a1bf2ca5a561898e541c3020e3c540 Mon Sep 17 00:00:00 2001 From: Kate Sugden Date: Wed, 3 Jul 2024 14:43:21 +0100 Subject: [PATCH 6/8] Add test for no filter prefix --- s3/tests/unit.tftest.hcl | 36 +++++++++++++++++++++++++++++++++++- 1 file changed, 35 insertions(+), 1 deletion(-) diff --git a/s3/tests/unit.tftest.hcl b/s3/tests/unit.tftest.hcl index 6b3c64c22..c73620b56 100644 --- a/s3/tests/unit.tftest.hcl +++ b/s3/tests/unit.tftest.hcl @@ -185,7 +185,41 @@ run "aws_s3_bucket_lifecycle_configuration_unit_test" { assert { condition = aws_s3_bucket_lifecycle_configuration.lifecycle-configuration[0].rule[0].expiration[0].days == 90 - error_message = "Should be: /foo" + error_message = "Should be: 90" + } + + assert { + condition = aws_s3_bucket_lifecycle_configuration.lifecycle-configuration[0].rule[0].abort_incomplete_multipart_upload[0].days_after_initiation == 7 + error_message = "Should be: 7" + } +} + +run "aws_s3_bucket_lifecycle_configuration_no_prefix_unit_test" { + command = plan + + variables { + config = { + "bucket_name" = "dbt-terraform-test-s3-module", + "type" = "string", + "lifecycle_rules" = [{ "expiration_days" = 90, "enabled" = true }], + "objects" = [], + } + } + + ### Test aws_s3_bucket_lifecycle_configuration resource when no prefix is used ### + assert { + condition = aws_s3_bucket_lifecycle_configuration.lifecycle-configuration[0].rule[0].filter[0] != null + error_message = "Should be: {}" + } + + assert { + condition = aws_s3_bucket_lifecycle_configuration.lifecycle-configuration[0].rule[0].filter[0].prefix == null + error_message = "Should be: null" + } + + assert { + condition = aws_s3_bucket_lifecycle_configuration.lifecycle-configuration[0].rule[0].expiration[0].days == 90 + error_message = "Should be: 90" } assert { From 7239aa71e2bee92a04eb7c194cefe554896506e7 Mon Sep 17 00:00:00 2001 From: Kate Sugden Date: Wed, 3 Jul 2024 14:51:50 +0100 Subject: [PATCH 7/8] Remove pointless ternary --- s3/main.tf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/s3/main.tf b/s3/main.tf index 299f61b2d..7b878f79e 100644 --- a/s3/main.tf +++ b/s3/main.tf @@ -60,7 +60,7 @@ resource "aws_s3_bucket_lifecycle_configuration" "lifecycle-configuration" { days_after_initiation = 7 } filter { - prefix = rule.value.filter_prefix != null ? rule.value.filter_prefix : null + prefix = rule.value.filter_prefix } expiration { days = rule.value.expiration_days From 76a32dcad56181d19607407d0f346a2cda2fbfc8 Mon Sep 17 00:00:00 2001 From: Kate Sugden Date: Wed, 3 Jul 2024 15:30:55 +0100 Subject: [PATCH 8/8] Remove lifecycle policy assertions covered in previous test --- s3/tests/unit.tftest.hcl | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/s3/tests/unit.tftest.hcl b/s3/tests/unit.tftest.hcl index c73620b56..a6265e458 100644 --- a/s3/tests/unit.tftest.hcl +++ b/s3/tests/unit.tftest.hcl @@ -192,6 +192,11 @@ run "aws_s3_bucket_lifecycle_configuration_unit_test" { condition = aws_s3_bucket_lifecycle_configuration.lifecycle-configuration[0].rule[0].abort_incomplete_multipart_upload[0].days_after_initiation == 7 error_message = "Should be: 7" } + + assert { + condition = aws_s3_bucket_lifecycle_configuration.lifecycle-configuration[0].rule[0].status == "Enabled" + error_message = "Should be: Enabled" + } } run "aws_s3_bucket_lifecycle_configuration_no_prefix_unit_test" { @@ -216,16 +221,6 @@ run "aws_s3_bucket_lifecycle_configuration_no_prefix_unit_test" { condition = aws_s3_bucket_lifecycle_configuration.lifecycle-configuration[0].rule[0].filter[0].prefix == null error_message = "Should be: null" } - - assert { - condition = aws_s3_bucket_lifecycle_configuration.lifecycle-configuration[0].rule[0].expiration[0].days == 90 - error_message = "Should be: 90" - } - - assert { - condition = aws_s3_bucket_lifecycle_configuration.lifecycle-configuration[0].rule[0].abort_incomplete_multipart_upload[0].days_after_initiation == 7 - error_message = "Should be: 7" - } } run "aws_s3_bucket_object_lock_configuration_governance_unit_test" {