Skip to content

Commit d2f1104

Browse files
committed
Include required bucket policy in S3 module
1 parent e649c08 commit d2f1104

File tree

4 files changed

+80
-66
lines changed

4 files changed

+80
-66
lines changed

terraform/backup/destination-bootstrap/main.tf

Lines changed: 0 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -17,33 +17,6 @@ module "terraform_state_bucket" {
1717
bucket_name = "mavisbackup-terraform-state"
1818
}
1919

20-
resource "aws_s3_bucket_policy" "backend_bucket_block_http" {
21-
bucket = module.terraform_state_bucket.bucket_id
22-
policy = jsonencode({
23-
Version = "2012-10-17"
24-
Id = "block-backend-bucket-http-access"
25-
Statement = [
26-
{
27-
Sid = "HTTPSOnly"
28-
Effect = "Deny"
29-
Principal = {
30-
"AWS" : "*"
31-
}
32-
Action = "s3:*"
33-
Resource = [
34-
module.terraform_state_bucket.arn,
35-
"${module.terraform_state_bucket.arn}/*",
36-
]
37-
Condition = {
38-
Bool = {
39-
"aws:SecureTransport" = "false"
40-
}
41-
}
42-
},
43-
]
44-
})
45-
}
46-
4720
#### Dynamo DB table for terraform state locking
4821
resource "aws_dynamodb_table" "dynamodb_lock_table" {
4922
name = "mavisbackup-terraform-state-lock"

terraform/backup/source/main.tf

Lines changed: 19 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -42,46 +42,23 @@ module "s3_reports_bucket" {
4242
bucket_name = "${local.project_name}-backup-reports"
4343
logging_target_bucket_id = data.aws_s3_bucket.mavis_logs.id
4444
logging_target_prefix = "backup-reports/"
45-
}
46-
47-
resource "aws_s3_bucket_policy" "backup_reports_bucket_policy" {
48-
bucket = module.s3_reports_bucket.bucket_id
49-
policy = jsonencode({
50-
Version = "2012-10-17"
51-
Id = "backup-reports-bucket-policy"
52-
Statement = [
53-
{
54-
Sid = "HTTPSOnly"
55-
Effect = "Deny"
56-
Principal = {
57-
"AWS" : "*"
58-
}
59-
Action = "s3:*"
60-
Resource = [
61-
module.s3_reports_bucket.arn,
62-
"${module.s3_reports_bucket.arn}/*",
63-
]
64-
Condition = {
65-
Bool = {
66-
"aws:SecureTransport" = "false"
67-
}
68-
}
69-
}, {
70-
Sid = "AllowBackupReports"
71-
Effect = "Allow"
72-
Principal = {
73-
"AWS" : "arn:aws:iam::${local.source_account_id}:role/aws-service-role/reports.backup.amazonaws.com/AWSServiceRoleForBackupReports"
74-
}
75-
Action = "s3:PutObject"
76-
Resource = ["${module.s3_reports_bucket.arn}/*"]
77-
Condition = {
78-
StringEquals = {
79-
"s3:x-amz-acl" = "bucket-owner-full-control"
80-
}
81-
}
45+
additional_policy_statements = [
46+
{
47+
sid = "AllowBackupReports"
48+
effect = "Allow"
49+
principals = [{
50+
type = "AWS"
51+
identifiers = ["arn:aws:iam::${local.source_account_id}:role/aws-service-role/reports.backup.amazonaws.com/AWSServiceRoleForBackupReports"]
52+
}]
53+
actions = ["s3:PutObject"]
54+
resources = ["arn:aws:s3:::${local.project_name}-backup-reports/*"]
55+
condition = {
56+
test = "StringEquals"
57+
variable = "s3:x-amz-acl"
58+
values = ["bucket-owner-full-control"]
8259
}
83-
]
84-
})
60+
}
61+
]
8562
}
8663

8764
# Now we can define the key itself
@@ -117,6 +94,9 @@ resource "aws_kms_key" "backup_notifications" {
11794

11895
module "source" {
11996
source = "github.com/NHSDigital/terraform-aws-backup.git//modules/aws-backup-source?ref=v1.1.0"
97+
# Use SSH to fetch sources locally
98+
# source = "git@github.com:NHSDigital/terraform-aws-backup.git//modules/aws-backup-source?ref=v1.1.0"
99+
120100

121101
backup_copy_vault_account_id = local.destination_account_id
122102
backup_copy_vault_arn = var.destination_vault_arn

terraform/modules/s3/main.tf

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,3 +34,58 @@ resource "aws_s3_bucket_public_access_block" "s3_bucket_access" {
3434
ignore_public_acls = true
3535
restrict_public_buckets = true
3636
}
37+
38+
data "aws_iam_policy_document" "block_http_access" {
39+
statement {
40+
sid = "HTTPSOnly"
41+
effect = "Deny"
42+
principals {
43+
type = "AWS"
44+
identifiers = ["*"]
45+
}
46+
actions = ["s3:*"]
47+
resources = [
48+
aws_s3_bucket.this.arn,
49+
"${aws_s3_bucket.this.arn}/*",
50+
]
51+
condition {
52+
test = "Bool"
53+
variable = "aws:SecureTransport"
54+
values = ["false"]
55+
}
56+
}
57+
}
58+
59+
data "aws_iam_policy_document" "additional_policy" {
60+
count = length(var.additional_policy_statements)
61+
dynamic "statement" {
62+
for_each = var.additional_policy_statements
63+
content {
64+
sid = statement.value.sid
65+
effect = statement.value.effect
66+
actions = statement.value.actions
67+
resources = statement.value.resources
68+
dynamic "principals" {
69+
for_each = statement.value.principals
70+
content {
71+
type = principals.value.type
72+
identifiers = principals.value.identifiers
73+
}
74+
}
75+
condition {
76+
test = statement.value.condition.test
77+
variable = statement.value.condition.variable
78+
values = statement.value.condition.values
79+
}
80+
}
81+
}
82+
}
83+
84+
data "aws_iam_policy_document" "combined" {
85+
source_policy_documents = concat([data.aws_iam_policy_document.block_http_access.json], length(data.aws_iam_policy_document.additional_policy) > 0 ? [data.aws_iam_policy_document.additional_policy[0].json] : [])
86+
}
87+
88+
resource "aws_s3_bucket_policy" "this" {
89+
bucket = aws_s3_bucket.this.id
90+
policy = data.aws_iam_policy_document.combined.json
91+
}

terraform/modules/s3/variables.tf

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,3 +18,9 @@ variable "logging_target_prefix" {
1818
nullable = false
1919
}
2020

21+
variable "additional_policy_statements" {
22+
description = "The JSON policy to apply to the bucket"
23+
type = list(any)
24+
default = []
25+
}
26+

0 commit comments

Comments
 (0)