From d47e3f0b60f9b0f8eec31aa9ecf3d9dde2d14eb3 Mon Sep 17 00:00:00 2001 From: Jeremy Collins Date: Thu, 12 Oct 2023 08:25:45 +0100 Subject: [PATCH 1/7] Refactor S3 and add AP landing bucket (no IAM yet) --- terraform/environments/performance-hub/s3.tf | 250 +++++++++++++++++++ 1 file changed, 250 insertions(+) create mode 100644 terraform/environments/performance-hub/s3.tf diff --git a/terraform/environments/performance-hub/s3.tf b/terraform/environments/performance-hub/s3.tf new file mode 100644 index 00000000000..c47197fe8e2 --- /dev/null +++ b/terraform/environments/performance-hub/s3.tf @@ -0,0 +1,250 @@ +#------------------------------------------------------------------------------ +# S3 Bucket for file uploads +#------------------------------------------------------------------------------ +#tfsec:ignore:AWS002 tfsec:ignore:AWS098 +resource "aws_s3_bucket" "upload_files" { + #checkov:skip=CKV_AWS_18 + #checkov:skip=CKV_AWS_144 + #checkov:skip=CKV2_AWS_6 + bucket = "${local.application_name}-uploads-${local.environment}" + + lifecycle { + prevent_destroy = true + } + + tags = merge( + local.tags, + { + Name = "${local.application_name}-uploads" + } + ) +} + +resource "aws_s3_bucket_acl" "upload_files" { + bucket = aws_s3_bucket.upload_files.id + acl = "private" +} + +resource "aws_s3_bucket_lifecycle_configuration" "upload_files" { + bucket = aws_s3_bucket.upload_files.id + rule { + id = "tf-s3-lifecycle" + status = "Enabled" + noncurrent_version_transition { + noncurrent_days = 30 + storage_class = "STANDARD_IA" + } + + transition { + days = 60 + storage_class = "STANDARD_IA" + } + } +} + +resource "aws_s3_bucket_server_side_encryption_configuration" "upload_files" { + bucket = aws_s3_bucket.upload_files.id + rule { + apply_server_side_encryption_by_default { + sse_algorithm = "aws:kms" + kms_master_key_id = aws_kms_key.s3.arn + } + } +} + +resource "aws_s3_bucket_versioning" "upload_files" { + bucket = aws_s3_bucket.upload_files.id + versioning_configuration { + status = "Enabled" + } +} + +resource "aws_s3_bucket_policy" "upload_files_policy" { + bucket = aws_s3_bucket.upload_files.id + policy = jsonencode({ + Version = "2012-10-17" + Id = "upload_bucket_policy" + Statement = [ + { + Effect = "Allow" + Principal = { AWS = ["arn:aws:iam::${data.aws_caller_identity.current.account_id}:user/cicd-member-user"] } + Action = "s3:*" + Resource = [ + aws_s3_bucket.upload_files.arn, + "${aws_s3_bucket.upload_files.arn}/*", + ] + }, + ] + }) +} + +resource "aws_iam_role" "s3_uploads_role" { + name = "${local.application_name}-s3-uploads-role" + assume_role_policy = data.aws_iam_policy_document.s3-access-policy.json + tags = merge( + local.tags, + { + Name = "${local.application_name}-s3-uploads-role" + } + ) +} + +data "aws_iam_policy_document" "s3-access-policy" { + version = "2012-10-17" + statement { + sid = "" + effect = "Allow" + actions = [ + "sts:AssumeRole", + ] + principals { + type = "Service" + identifiers = [ + "rds.amazonaws.com", + "ec2.amazonaws.com", + ] + } + } +} + +resource "aws_iam_policy" "s3-uploads-policy" { + name = "${local.application_name}-s3-uploads-policy" + policy = < Date: Thu, 12 Oct 2023 15:28:50 +0100 Subject: [PATCH 2/7] Use mod platform S3 module --- terraform/environments/performance-hub/s3.tf | 78 ++++++++++---------- 1 file changed, 37 insertions(+), 41 deletions(-) diff --git a/terraform/environments/performance-hub/s3.tf b/terraform/environments/performance-hub/s3.tf index c47197fe8e2..411f933db80 100644 --- a/terraform/environments/performance-hub/s3.tf +++ b/terraform/environments/performance-hub/s3.tf @@ -1,5 +1,5 @@ #------------------------------------------------------------------------------ -# S3 Bucket for file uploads +# S3 Bucket for file uploads and other user-generated content #------------------------------------------------------------------------------ #tfsec:ignore:AWS002 tfsec:ignore:AWS098 resource "aws_s3_bucket" "upload_files" { @@ -12,7 +12,7 @@ resource "aws_s3_bucket" "upload_files" { prevent_destroy = true } - tags = merge( + tags = merge ( local.tags, { Name = "${local.application_name}-uploads" @@ -158,59 +158,55 @@ resource "aws_iam_role_policy_attachment" "s3_uploads_attachment" { #------------------------------------------------------------------------------------------------- # S3 "landing" bucket for AP data transfer # AP pipelines write to this bucket and the Performance Hub reads files from here. It doesn't -# need complex retention since files are removed from this bucket once imported. +# need complex retention or versioning since files are removed from this bucket once imported. #------------------------------------------------------------------------------------------------- -#tfsec:ignore:AWS002 tfsec:ignore:AWS098 -resource "aws_s3_bucket" "ap_landing_bucket" { - #checkov:skip=CKV_AWS_18 - #checkov:skip=CKV_AWS_144 - #checkov:skip=CKV2_AWS_6 - bucket = "${local.application_name}-land-${local.environment}" - lifecycle { - prevent_destroy = true +module "s3-bucket" "ap_landing_bucket" { + source = "github.com/ministryofjustice/modernisation-platform-terraform-s3-bucket?ref=v7.0.0" + + bucket_name = "${local.application_name}-land-${local.environment}" + ownership_controls = "BucketOwnerEnforced" + + versioning_enabled = false + replication_enabled = false + + providers = { + # Leave this provider block in even if you are not using replication + aws.bucket-replication = aws } - tags = merge( + custom_kms_key = aws_kms_key.s3.arn + + lifecycle_rule = [ + { + id = "tf-s3-lifecycle-landing" + enabled = "Enabled" + + expiration = { + days = 30 + } + } + ] + + tags = merge ( local.tags, { - Name = "${local.application_name}-ap-landing-bucket" + Name = "${local.application_name}-ap-landing-bucket" } ) } -resource "aws_s3_bucket_acl" "ap_landing_bucket" { - bucket = aws_s3_bucket.ap_landing_bucket.id - acl = "private" -} - -resource "aws_s3_bucket_lifecycle_configuration" "ap_landing_bucket" { - bucket = aws_s3_bucket.ap_landing_bucket.id - rule { - id = "tf-s3-lifecycle-landing" - status = "Enabled" - expiration { - days = 30 - } - } +# AP Airflow jobs are expecting certain folders to exists +resource "aws_s3_object" "prison_incidents" { + bucket = module.s3_bucket.ap_landing_bucket.id + key = "prison_incidents/" } -resource "aws_s3_bucket_server_side_encryption_configuration" "ap_landing_bucket" { - bucket = aws_s3_bucket.ap_landing_bucket.id - rule { - apply_server_side_encryption_by_default { - sse_algorithm = "aws:kms" - kms_master_key_id = aws_kms_key.s3.arn - } - } +resource "aws_s3_object" "prison_performance" { + bucket = module.s3_bucket.ap_landing_bucket.id + key = "prison_performance/" } -resource "aws_s3_bucket_versioning" "ap_landing_bucket" { - bucket = aws_s3_bucket.ap_landing_bucket.id - versioning_configuration { - status = "Disabled" - } -} #------------------------------------------------------------------------------ # KMS setup for S3 From d5e10a2ac9b83236933ee526f7555a7f6dd6bab7 Mon Sep 17 00:00:00 2001 From: Jeremy Collins Date: Fri, 13 Oct 2023 07:27:58 +0100 Subject: [PATCH 3/7] Add comment re inconsisent use of S3 module --- terraform/environments/performance-hub/s3.tf | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/terraform/environments/performance-hub/s3.tf b/terraform/environments/performance-hub/s3.tf index 411f933db80..013dbfd21f8 100644 --- a/terraform/environments/performance-hub/s3.tf +++ b/terraform/environments/performance-hub/s3.tf @@ -1,5 +1,7 @@ #------------------------------------------------------------------------------ # S3 Bucket for file uploads and other user-generated content +# (note this code predates the Modernisation Platform S3 module used below +# for "ap_landing_bucket") #------------------------------------------------------------------------------ #tfsec:ignore:AWS002 tfsec:ignore:AWS098 resource "aws_s3_bucket" "upload_files" { @@ -157,8 +159,8 @@ resource "aws_iam_role_policy_attachment" "s3_uploads_attachment" { #------------------------------------------------------------------------------------------------- # S3 "landing" bucket for AP data transfer -# AP pipelines write to this bucket and the Performance Hub reads files from here. It doesn't -# need complex retention or versioning since files are removed from this bucket once imported. +# AP pipelines write to this bucket and the Performance Hub reads files from here. It doesn't need +# complex retention versioning or replication since files are removed from this bucket once imported. #------------------------------------------------------------------------------------------------- module "s3-bucket" "ap_landing_bucket" { @@ -191,12 +193,13 @@ module "s3-bucket" "ap_landing_bucket" { tags = merge ( local.tags, { - Name = "${local.application_name}-ap-landing-bucket" + Name = "${local.application_name}-ap-landing-bucket", + Desciption = "Data being imported from the Analytical Platform gets dropped here, ingested and removed" } ) } -# AP Airflow jobs are expecting certain folders to exists +# AP Airflow jobs are expecting certain folders to exist resource "aws_s3_object" "prison_incidents" { bucket = module.s3_bucket.ap_landing_bucket.id key = "prison_incidents/" From a734932f73bc107d61e630f438d13a82e4d4cefb Mon Sep 17 00:00:00 2001 From: Jeremy Collins Date: Fri, 13 Oct 2023 07:31:23 +0100 Subject: [PATCH 4/7] Moved upload bucket from database.tf --- .../environments/performance-hub/database.tf | 191 ------------------ 1 file changed, 191 deletions(-) diff --git a/terraform/environments/performance-hub/database.tf b/terraform/environments/performance-hub/database.tf index 9e73fb60520..3cf0bc1f074 100644 --- a/terraform/environments/performance-hub/database.tf +++ b/terraform/environments/performance-hub/database.tf @@ -280,198 +280,7 @@ resource "aws_iam_role_policy_attachment" "s3_database_backups_attachment" { role = aws_iam_role.s3_database_backups_role.name policy_arn = aws_iam_policy.s3_database_backups_policy.arn } -#------------------------------------------------------------------------------ -# S3 Bucket for Uploads -#------------------------------------------------------------------------------ -#tfsec:ignore:AWS002 tfsec:ignore:AWS098 -resource "aws_s3_bucket" "upload_files" { - #checkov:skip=CKV_AWS_18 - #checkov:skip=CKV_AWS_144 - #checkov:skip=CKV2_AWS_6 - bucket = "${local.application_name}-uploads-${local.environment}" - - lifecycle { - prevent_destroy = true - } - - tags = merge( - local.tags, - { - Name = "${local.application_name}-uploads" - } - ) -} - -resource "aws_s3_bucket_acl" "upload_files" { - bucket = aws_s3_bucket.upload_files.id - acl = "private" -} - -resource "aws_s3_bucket_lifecycle_configuration" "upload_files" { - bucket = aws_s3_bucket.upload_files.id - rule { - id = "tf-s3-lifecycle" - status = "Enabled" - noncurrent_version_transition { - noncurrent_days = 30 - storage_class = "STANDARD_IA" - } - - transition { - days = 60 - storage_class = "STANDARD_IA" - } - } -} - -resource "aws_s3_bucket_server_side_encryption_configuration" "upload_files" { - bucket = aws_s3_bucket.upload_files.id - rule { - apply_server_side_encryption_by_default { - sse_algorithm = "aws:kms" - kms_master_key_id = aws_kms_key.s3.arn - } - } -} - -resource "aws_s3_bucket_versioning" "upload_files" { - bucket = aws_s3_bucket.upload_files.id - versioning_configuration { - status = "Enabled" - } -} - -resource "aws_s3_bucket_policy" "upload_files_policy" { - bucket = aws_s3_bucket.upload_files.id - policy = jsonencode({ - Version = "2012-10-17" - Id = "upload_bucket_policy" - Statement = [ - { - Effect = "Allow" - Principal = { AWS = ["arn:aws:iam::${data.aws_caller_identity.current.account_id}:user/cicd-member-user"] } - Action = "s3:*" - Resource = [ - aws_s3_bucket.upload_files.arn, - "${aws_s3_bucket.upload_files.arn}/*", - ] - }, - ] - }) -} - -resource "aws_iam_role" "s3_uploads_role" { - name = "${local.application_name}-s3-uploads-role" - assume_role_policy = data.aws_iam_policy_document.s3-access-policy.json - tags = merge( - local.tags, - { - Name = "${local.application_name}-s3-uploads-role" - } - ) -} - -data "aws_iam_policy_document" "s3-access-policy" { - version = "2012-10-17" - statement { - sid = "" - effect = "Allow" - actions = [ - "sts:AssumeRole", - ] - principals { - type = "Service" - identifiers = [ - "rds.amazonaws.com", - "ec2.amazonaws.com", - ] - } - } -} -resource "aws_iam_policy" "s3-uploads-policy" { - name = "${local.application_name}-s3-uploads-policy" - policy = < Date: Fri, 13 Oct 2023 07:34:50 +0100 Subject: [PATCH 5/7] Correct module syntax --- terraform/environments/performance-hub/s3.tf | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/terraform/environments/performance-hub/s3.tf b/terraform/environments/performance-hub/s3.tf index 013dbfd21f8..d1bf35a332b 100644 --- a/terraform/environments/performance-hub/s3.tf +++ b/terraform/environments/performance-hub/s3.tf @@ -163,7 +163,7 @@ resource "aws_iam_role_policy_attachment" "s3_uploads_attachment" { # complex retention versioning or replication since files are removed from this bucket once imported. #------------------------------------------------------------------------------------------------- -module "s3-bucket" "ap_landing_bucket" { +module "ap_landing_bucket" { source = "github.com/ministryofjustice/modernisation-platform-terraform-s3-bucket?ref=v7.0.0" bucket_name = "${local.application_name}-land-${local.environment}" @@ -201,12 +201,12 @@ module "s3-bucket" "ap_landing_bucket" { # AP Airflow jobs are expecting certain folders to exist resource "aws_s3_object" "prison_incidents" { - bucket = module.s3_bucket.ap_landing_bucket.id + bucket = module.ap_landing_bucket.id key = "prison_incidents/" } resource "aws_s3_object" "prison_performance" { - bucket = module.s3_bucket.ap_landing_bucket.id + bucket = module.ap_landing_bucket.id key = "prison_performance/" } From 1595cfa57168a21227bae29eabe38c09130a3e8b Mon Sep 17 00:00:00 2001 From: Jeremy Collins Date: Fri, 13 Oct 2023 07:41:11 +0100 Subject: [PATCH 6/7] Correct S3 module return object syntax --- terraform/environments/performance-hub/s3.tf | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/terraform/environments/performance-hub/s3.tf b/terraform/environments/performance-hub/s3.tf index d1bf35a332b..812a1d38832 100644 --- a/terraform/environments/performance-hub/s3.tf +++ b/terraform/environments/performance-hub/s3.tf @@ -201,12 +201,12 @@ module "ap_landing_bucket" { # AP Airflow jobs are expecting certain folders to exist resource "aws_s3_object" "prison_incidents" { - bucket = module.ap_landing_bucket.id + bucket = module.ap_landing_bucket.bucket.id key = "prison_incidents/" } resource "aws_s3_object" "prison_performance" { - bucket = module.ap_landing_bucket.id + bucket = module.ap_landing_bucket.bucket.id key = "prison_performance/" } From fb1db0b4d3f458e04bccb9fc84de243589ab4ccb Mon Sep 17 00:00:00 2001 From: Jeremy Collins Date: Fri, 13 Oct 2023 07:49:42 +0100 Subject: [PATCH 7/7] Remove description tag (invalid tag value?) --- terraform/environments/performance-hub/s3.tf | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/terraform/environments/performance-hub/s3.tf b/terraform/environments/performance-hub/s3.tf index 812a1d38832..60414fc61ea 100644 --- a/terraform/environments/performance-hub/s3.tf +++ b/terraform/environments/performance-hub/s3.tf @@ -193,8 +193,7 @@ module "ap_landing_bucket" { tags = merge ( local.tags, { - Name = "${local.application_name}-ap-landing-bucket", - Desciption = "Data being imported from the Analytical Platform gets dropped here, ingested and removed" + Name = "${local.application_name}-ap-landing-bucket" } ) }