Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: DBTP-1568 - Add s3 support for cross environment service access #293

Merged
merged 10 commits into from
Jan 6, 2025
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."
}

}
Loading