Skip to content

Commit

Permalink
feat: DBTP-1568 - Add s3 support for cross environment service access (
Browse files Browse the repository at this point in the history
…#293)

Co-authored-by: Anthony Roy <[email protected]>
  • Loading branch information
ksugden and antroy-madetech authored Jan 6, 2025
1 parent 795dbae commit e4252ad
Show file tree
Hide file tree
Showing 6 changed files with 219 additions and 7 deletions.
1 change: 1 addition & 0 deletions .trufflehogignore
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
.git
.playwright-browsers
poetry.lock
.terraform
5 changes: 2 additions & 3 deletions postgres/database-load/locals.tf
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,8 @@ locals {

task_name = "${var.application}-${var.environment}-${var.database_name}-load"

dump_task_name = "${var.application}-${var.task.from}-${var.database_name}-dump"
dump_kms_key_alias = "alias/${local.dump_task_name}"
dump_bucket_name = local.dump_task_name
dump_task_name = "${var.application}-${var.task.from}-${var.database_name}-dump"
dump_bucket_name = local.dump_task_name

pipeline_task = lookup(var.task, "pipeline", null) != null
region_account = "${data.aws_region.current.name}:${data.aws_caller_identity.current.account_id}"
Expand Down
13 changes: 9 additions & 4 deletions postgres/database-load/tests/unit.tftest.hcl
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,18 @@ run "data_load_unit_test" {
command = plan

assert {
condition = local.dump_kms_key_alias == "alias/test-app-some-other-env-test-db-dump"
error_message = "Dump Kms key alias should be: alias/test-app-some-other-env-test-db-dump"
condition = local.dump_bucket_name == "test-app-some-other-env-test-db-dump"
error_message = "Dump bucket name should be: test-app-some-other-env-test-db-dump"
}

assert {
condition = local.dump_bucket_name == "test-app-some-other-env-test-db-dump"
error_message = "Dump bucket name should be: test-app-some-other-env-test-db-dump"
condition = local.task_name == "test-app-test-env-test-db-load"
error_message = "Task name incorrect"
}

assert {
condition = local.dump_task_name == "test-app-some-other-env-test-db-dump"
error_message = "Dump task name incorrect"
}

assert {
Expand Down
42 changes: 42 additions & 0 deletions s3/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,27 @@ data "aws_iam_policy_document" "bucket-policy" {
resources = [aws_s3_bucket.this.arn, "${aws_s3_bucket.this.arn}/*"]
}
}

dynamic "statement" {
for_each = coalesce(var.config.cross_environment_service_access, {})
content {
effect = "Allow"
actions = flatten([
statement.value.read ? ["s3:Get*", "s3:ListBucket"] : [],
statement.value.write ? ["s3:Put*"] : [],
])
principals {
identifiers = ["*"]
type = "AWS"
}
condition {
test = "StringLike"
values = ["arn:aws:iam::${statement.value.account}:role/${statement.value.application}-${statement.value.environment}-${statement.value.service}-TaskRole-*"]
variable = "aws:PrincipalArn"
}
resources = [aws_s3_bucket.this.arn, "${aws_s3_bucket.this.arn}/*"]
}
}
}

resource "aws_s3_bucket_policy" "bucket-policy" {
Expand Down Expand Up @@ -126,6 +147,27 @@ data "aws_iam_policy_document" "key-policy" {
resources = [aws_kms_key.kms-key[0].arn]
}
}

dynamic "statement" {
for_each = coalesce(var.config.cross_environment_service_access, {})
content {
effect = "Allow"
actions = flatten([
statement.value.read ? ["kms:Decrypt"] : [],
statement.value.write ? ["kms:GenerateDataKey"] : [],
])
principals {
identifiers = ["*"]
type = "AWS"
}
condition {
test = "StringLike"
values = ["arn:aws:iam::${statement.value.account}:role/${statement.value.application}-${statement.value.environment}-${statement.value.service}-TaskRole-*"]
variable = "aws:PrincipalArn"
}
resources = [aws_kms_key.kms-key[0].arn]
}
}
}

resource "aws_kms_key_policy" "key-policy" {
Expand Down
147 changes: 147 additions & 0 deletions s3/tests/unit.tftest.hcl
Original file line number Diff line number Diff line change
Expand Up @@ -442,6 +442,153 @@ run "aws_s3_bucket_external_role_access_invalid_cyber_sign_off" {
expect_failures = [var.config.external_role_access.cyber_sign_off_by]
}

run "aws_s3_bucket_cross_environment_service_access_read_write_unit_test" {
command = plan

variables {
config = {
"bucket_name" = "dbt-terraform-test-s3-cross-env-service-access",
"type" = "s3",
"cross_environment_service_access" = {
"test-access" = {
"application" = "app",
"environment" = "test",
"account" = "123456789012",
"service" = "service",
"read" = true,
"write" = true,
"cyber_sign_off_by" = "[email protected]"
}
}
}
}

assert {
condition = data.aws_iam_policy_document.bucket-policy.statement[1].effect == "Allow"
error_message = "Should be: Allow"
}

assert {
condition = length([for item in data.aws_iam_policy_document.bucket-policy.statement[1].condition : item if item.test == "StringLike"]) == 1
error_message = "condition should have a test: StringLike attribute"
}

assert {
condition = length([for item in data.aws_iam_policy_document.bucket-policy.statement[1].condition : item if item.variable == "aws:PrincipalArn"]) == 1
error_message = "condition should have a variable: aws:PrincipalArn attribute"
}

assert {
condition = length([for item in data.aws_iam_policy_document.bucket-policy.statement[1].condition :
item if item.values == tolist(["arn:aws:iam::123456789012:role/app-test-service-TaskRole-*"])]) == 1
error_message = "condition should have a values: [bucket arn] attribute"
}

assert {
condition = alltrue([
contains(data.aws_iam_policy_document.bucket-policy.statement[1].actions, "s3:Get*"),
contains(data.aws_iam_policy_document.bucket-policy.statement[1].actions, "s3:Put*"),
contains(data.aws_iam_policy_document.bucket-policy.statement[1].actions, "s3:ListBucket"),
])
error_message = "Should be: s3:Get*, s3:Put*, s3:ListBucket"
}
}

run "aws_s3_bucket_cross_environment_service_access_read_only_unit_test" {
command = plan

variables {
config = {
"bucket_name" = "dbt-terraform-test-s3-cross-env-service-access",
"type" = "s3",
"cross_environment_service_access" = {
"test-access" = {
"application" = "app",
"environment" = "test",
"account" = "123456789012",
"service" = "service",
"read" = true,
"write" = false,
"cyber_sign_off_by" = "[email protected]"
}
}
}
}

assert {
condition = data.aws_iam_policy_document.bucket-policy.statement[1].effect == "Allow"
error_message = "Should be: Allow"
}

assert {
condition = alltrue([
contains(data.aws_iam_policy_document.bucket-policy.statement[1].actions, "s3:Get*"),
contains(data.aws_iam_policy_document.bucket-policy.statement[1].actions, "s3:ListBucket"),
!contains(data.aws_iam_policy_document.bucket-policy.statement[1].actions, "s3:Put*"),
])
error_message = "Should be: s3:Get*, s3:ListBucket"
}
}

run "aws_s3_bucket_cross_environment_service_access_write_only_unit_test" {
command = plan

variables {
config = {
"bucket_name" = "dbt-terraform-test-s3-cross-env-service-access",
"type" = "s3",
"cross_environment_service_access" = {
"test-access" = {
"application" = "app",
"environment" = "test",
"account" = "123456789012",
"service" = "service",
"read" = false,
"write" = true,
"cyber_sign_off_by" = "[email protected]"
}
}
}
}

assert {
condition = data.aws_iam_policy_document.bucket-policy.statement[1].effect == "Allow"
error_message = "Should be: Allow"
}

assert {
condition = alltrue([
!contains(data.aws_iam_policy_document.bucket-policy.statement[1].actions, "s3:Get*"),
!contains(data.aws_iam_policy_document.bucket-policy.statement[1].actions, "s3:ListBucket"),
contains(data.aws_iam_policy_document.bucket-policy.statement[1].actions, "s3:Put*"),
])
error_message = "Should be: s3:Put*"
}
}

run "aws_s3_bucket_cross_environment_service_access_invalid_cyber_sign_off" {
command = plan

variables {
config = {
"bucket_name" = "dbt-terraform-test-s3-cross-env-service-access",
"type" = "s3",
"cross_environment_service_access" = {
"test-access" = {
"application" = "app",
"environment" = "test",
"account" = "123456789012",
"service" = "service",
"read" = true,
"write" = true,
"cyber_sign_off_by" = "no-one"
}
}
}
}
expect_failures = [var.config.cross_environment_service_access.cyber_sign_off_by]
}

run "aws_s3_bucket_object_lock_configuration_governance_unit_test" {
command = plan

Expand Down
18 changes: 18 additions & 0 deletions s3/variables.tf
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,16 @@ variable "config" {
write = bool
cyber_sign_off_by = string
})))
# NOTE: allows access to S3 bucket from DBT Platform managed service roles, also generates Copilot addon for service access
cross_environment_service_access = optional(map(object({
application = string
account = string
environment = string
service = string
read = bool
write = bool
cyber_sign_off_by = string
})))
# NOTE: readonly access is managed by Copilot server addon s3 policy.
readonly = optional(bool)
serve_static_content = optional(bool, false)
Expand Down Expand Up @@ -69,4 +79,12 @@ variable "config" {
])
error_message = "All instances of external_role_access must be approved by cyber, and a cyber rep's email address entered."
}

validation {
condition = var.config.cross_environment_service_access == null ? true : alltrue([
for k, v in var.config.cross_environment_service_access : (can(regex("^[\\w\\-\\.]+@(businessandtrade.gov.uk|digital.trade.gov.uk)$", v.cyber_sign_off_by)))
])
error_message = "All instances of cross_environment_service_access must be approved by cyber, and a cyber rep's email address entered."
}

}

0 comments on commit e4252ad

Please sign in to comment.