Skip to content

Commit c1f1bf7

Browse files
authored
Refactor CloudTrail Bucket Management (#2345)
1 parent ec4334b commit c1f1bf7

File tree

6 files changed

+87
-118
lines changed

6 files changed

+87
-118
lines changed

terraform/compliance/s3.feature

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,9 @@ Feature: S3
88
@exclude_aws_s3_bucket.mwaa_etl_scripts_bucket
99
@exclude_module.housing_nec_migration_storage.aws_s3_bucket.bucket
1010
@exclude_module.admin_bucket.aws_s3_bucket.bucket
11+
@exclude_module.cloudtrail_storage.aws_s3_bucket.bucket
1112

12-
# This rule is in place for legacy buckets created with the deprecated block within the aws_s3_bucket resource
13+
# This rule is in place for legacy buckets created with the deprecated block within the aws_s3_bucket resource
1314
Scenario: Data must be encrypted at rest for buckets created using server_side_encryption_configuration property within bucket resource
1415
Given I have aws_s3_bucket defined
1516
Then it must have server_side_encryption_configuration
@@ -30,7 +31,6 @@ Feature: S3
3031
@exclude_module.trusted_zone.aws_s3_bucket.bucket
3132
@exclude_module.kafka_event_streaming\[0\].module.kafka_dependency_storage.aws_s3_bucket.bucket
3233
@exclude_module.db_snapshot_to_s3\[0\].module.rds_export_storage.aws_s3_bucket.bucket
33-
@exclude_module.liberator_dump_to_rds_snapshot\[0\].aws_s3_bucket.cloudtrail
3434
@exclude_module.liberator_db_snapshot_to_s3\[0\].module.rds_export_storage.aws_s3_bucket.bucket
3535
# This rule checks for a separate sse block as supported by the s3 bucket module
3636
Scenario: Data must be encrypted at rest for buckets created using separate server side configuration resource

terraform/core/10-aws-s3-bucket-policies.tf

Lines changed: 34 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -394,7 +394,7 @@ locals {
394394
sid = "Allow S3 Batch Copy to access raw zone KMS key"
395395
effect = "Allow"
396396
principals = {
397-
type = "AWS"
397+
type = "AWS"
398398
identifiers = [
399399
aws_iam_role.batch_s3_copy_role.arn
400400
]
@@ -410,7 +410,7 @@ locals {
410410
sid = "Allow S3 Batch Copy to access refined zone KMS key"
411411
effect = "Allow"
412412
principals = {
413-
type = "AWS"
413+
type = "AWS"
414414
identifiers = [
415415
aws_iam_role.batch_s3_copy_role.arn
416416
]
@@ -421,12 +421,12 @@ locals {
421421
]
422422
resources = ["*"]
423423
}
424-
424+
425425
allow_s3_batch_copy_kms_access_trusted_zone = {
426426
sid = "Allow S3 Batch Copy to access trusted zone KMS key"
427427
effect = "Allow"
428428
principals = {
429-
type = "AWS"
429+
type = "AWS"
430430
identifiers = [
431431
aws_iam_role.batch_s3_copy_role.arn
432432
]
@@ -437,4 +437,33 @@ locals {
437437
]
438438
resources = ["*"]
439439
}
440-
}
440+
441+
#-----------------------------------------------------------------------------
442+
# CloudTrail Policies
443+
#-----------------------------------------------------------------------------
444+
445+
cloudtrail_get_bucket_acl_statement = {
446+
sid = "AllowCloudTrailGetBucketAcl"
447+
effect = "Allow"
448+
actions = ["s3:GetBucketAcl"]
449+
resources = ["arn:aws:s3:::${local.identifier_prefix}-cloudtrail"]
450+
principals = {
451+
type = "Service"
452+
identifiers = ["cloudtrail.amazonaws.com"]
453+
}
454+
}
455+
456+
cloudtrail_put_object_statement = {
457+
sid = "AllowCloudTrailPutObject"
458+
effect = "Allow"
459+
actions = ["s3:PutObject"]
460+
resources = ["arn:aws:s3:::${local.identifier_prefix}-cloudtrail/*/AWSLogs/*"]
461+
# First wildcard (*) matches any custom prefix (e.g., liberator-data-processing)
462+
# AWSLogs path is automatically added by AWS CloudTrail service
463+
# Second wildcard (*) matches AWS-generated path: account-id/CloudTrail/region/date/
464+
principals = {
465+
type = "Service"
466+
identifiers = ["cloudtrail.amazonaws.com"]
467+
}
468+
}
469+
}

terraform/core/10-aws-s3-utility-buckets.tf

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,9 +51,38 @@ module "spark_ui_output_storage" {
5151
}
5252

5353
#===============================================================================
54-
# Application and Storage Buckets
54+
# CloudTrail Storage Bucket
5555
#===============================================================================
5656

57+
module "cloudtrail_storage" {
58+
source = "../modules/s3-bucket"
59+
tags = module.tags.values
60+
project = var.project
61+
environment = var.environment
62+
identifier_prefix = local.identifier_prefix
63+
bucket_name = "CloudTrail"
64+
bucket_identifier = "cloudtrail"
65+
versioning_enabled = true
66+
expire_objects_days = 365
67+
expire_noncurrent_objects_days = 30
68+
abort_multipart_days = 30
69+
include_backup_policy_tags = false
70+
71+
bucket_policy_statements = local.is_live_environment ? [
72+
local.cloudtrail_get_bucket_acl_statement,
73+
local.cloudtrail_put_object_statement
74+
] : []
75+
}
76+
77+
# Move CloudTrail bucket from sql-to-rds-snapshot module to centralized location, keeping the same name
78+
moved {
79+
from = module.liberator_dump_to_rds_snapshot[0].aws_s3_bucket.cloudtrail
80+
to = module.cloudtrail_storage.aws_s3_bucket.bucket
81+
}
82+
83+
#===============================================================================
84+
# Application and Storage Buckets
85+
#===============================================================================
5786
module "lambda_artefact_storage" {
5887
source = "../modules/s3-bucket"
5988
tags = module.tags.values

terraform/core/36-liberator-import.tf

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,9 @@ module "liberator_dump_to_rds_snapshot" {
2525
aws_subnet_ids = data.aws_subnets.network.ids
2626
ecs_cluster_arn = aws_ecs_cluster.workers.arn
2727
vpc_id = data.aws_vpc.network.id
28+
cloudtrail_bucket_id = module.cloudtrail_storage.bucket_id
29+
cloudtrail_bucket_arn = module.cloudtrail_storage.bucket_arn
30+
cloudtrail_kms_key_arn = module.cloudtrail_storage.kms_key_arn
2831
}
2932

3033
resource "aws_glue_workflow" "parking_liberator_data" {

terraform/modules/sql-to-rds-snapshot/01-inputs-required.tf

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,3 +57,18 @@ variable "vpc_id" {
5757
description = "ID of the VPC for the current environment"
5858
type = string
5959
}
60+
61+
variable "cloudtrail_bucket_id" {
62+
description = "ID of the S3 bucket for storing CloudTrail logs"
63+
type = string
64+
}
65+
66+
variable "cloudtrail_bucket_arn" {
67+
description = "ARN of the S3 bucket for storing CloudTrail logs"
68+
type = string
69+
}
70+
71+
variable "cloudtrail_kms_key_arn" {
72+
description = "KMS Key ARN for the CloudTrail bucket"
73+
type = string
74+
}
Lines changed: 3 additions & 110 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
resource "aws_cloudtrail" "events" {
2-
count = var.is_production_environment ? 1 : 0
2+
count = var.is_live_environment ? 1 : 0
33

44
name = var.identifier_prefix
5-
s3_bucket_name = aws_s3_bucket.cloudtrail.id
6-
s3_key_prefix = "prefix"
5+
s3_bucket_name = var.cloudtrail_bucket_id
6+
s3_key_prefix = "liberator-data-processing"
77
include_global_service_events = false
88

99
cloud_watch_logs_group_arn = "${aws_cloudwatch_log_group.cloud_trail_events.arn}:*" # CloudTrail requires the Log Stream wildcard
@@ -67,110 +67,3 @@ data "aws_iam_policy_document" "assume_policy" {
6767
}
6868
}
6969
}
70-
71-
resource "aws_kms_key" "key" {
72-
tags = var.tags
73-
74-
description = "${var.project} ${var.environment} - ${var.identifier_prefix}-cloudtrail Bucket Key"
75-
deletion_window_in_days = 10
76-
enable_key_rotation = true
77-
}
78-
79-
resource "aws_s3_bucket" "cloudtrail" {
80-
bucket = "${var.identifier_prefix}-cloudtrail"
81-
force_destroy = true
82-
}
83-
84-
resource "aws_s3_bucket_server_side_encryption_configuration" "cloudtrail" {
85-
bucket = aws_s3_bucket.cloudtrail.id
86-
87-
rule {
88-
apply_server_side_encryption_by_default {
89-
kms_master_key_id = aws_kms_key.key.arn
90-
sse_algorithm = "aws:kms"
91-
}
92-
bucket_key_enabled = true
93-
}
94-
}
95-
96-
resource "aws_s3_bucket_versioning" "cloudtrail" {
97-
bucket = aws_s3_bucket.cloudtrail.id
98-
99-
versioning_configuration {
100-
status = "Enabled"
101-
}
102-
}
103-
104-
resource "aws_s3_bucket_public_access_block" "block_public_access" {
105-
bucket = aws_s3_bucket.cloudtrail.id
106-
depends_on = [aws_s3_bucket.cloudtrail]
107-
108-
block_public_acls = true
109-
block_public_policy = true
110-
ignore_public_acls = true
111-
restrict_public_buckets = true
112-
}
113-
114-
resource "aws_s3_bucket_policy" "example" {
115-
bucket = aws_s3_bucket.cloudtrail.id
116-
policy = data.aws_iam_policy_document.cloudtrail_bucket_policy.json
117-
}
118-
119-
resource "aws_s3_bucket_lifecycle_configuration" "cloudtrail" {
120-
bucket = aws_s3_bucket.cloudtrail.id
121-
rule {
122-
id = "Keep previous version 30 days"
123-
status = "Enabled"
124-
expiration {
125-
days = 30
126-
}
127-
}
128-
}
129-
130-
data "aws_iam_policy_document" "cloudtrail_bucket_policy" {
131-
statement {
132-
effect = "Allow"
133-
actions = ["s3:GetBucketAcl"]
134-
resources = ["arn:aws:s3:::${var.identifier_prefix}-cloudtrail"]
135-
principals {
136-
type = "Service"
137-
identifiers = ["cloudtrail.amazonaws.com"]
138-
}
139-
}
140-
141-
statement {
142-
effect = "Allow"
143-
actions = ["s3:PutObject"]
144-
resources = ["arn:aws:s3:::${var.identifier_prefix}-cloudtrail/prefix/AWSLogs/*"]
145-
principals {
146-
type = "Service"
147-
identifiers = ["cloudtrail.amazonaws.com"]
148-
}
149-
condition {
150-
test = "StringEquals"
151-
variable = "s3:x-amz-acl"
152-
values = ["bucket-owner-full-control"]
153-
}
154-
}
155-
156-
statement {
157-
sid = "AllowSSLRequestsOnly"
158-
effect = "Deny"
159-
actions = ["s3:*"]
160-
principals {
161-
type = "AWS"
162-
identifiers = ["*"]
163-
}
164-
resources = [
165-
aws_s3_bucket.cloudtrail.arn,
166-
"${aws_s3_bucket.cloudtrail.arn}/*",
167-
]
168-
condition {
169-
test = "Bool"
170-
variable = "aws:SecureTransport"
171-
values = [
172-
"false"
173-
]
174-
}
175-
}
176-
}

0 commit comments

Comments
 (0)