From 0e93c54ef4a742956ded0084719d725fb5a2e45c Mon Sep 17 00:00:00 2001 From: David Elliott Date: Wed, 21 Sep 2022 17:53:34 +0100 Subject: [PATCH] Use the default provider This causes issues when running a local plan as the local user does not have access to the current provider given. By passing in the default provider to the S3 bucket module we solve this. This provider is not needed as the S3 bucket does not use replication, but because of the way that providers work it's not possible to just to have a default. Add encryption to database, move to template function and sort interpolation warnings. --- README.md | 8 +++++--- main.tf | 35 +++++++++++++++++---------------- providers.tf | 7 ------- test/unit-test/main.tf | 6 +++++- test/unit-test/role.tf | 44 ------------------------------------------ versions.tf | 5 +++-- 6 files changed, 31 insertions(+), 74 deletions(-) delete mode 100644 providers.tf delete mode 100644 test/unit-test/role.tf diff --git a/README.md b/README.md index 46953ae6..226045c3 100644 --- a/README.md +++ b/README.md @@ -58,6 +58,11 @@ Try a query like `select * from lb_logs limit 100;` module "lb-access-logs-enabled" { source = "github.com/ministryofjustice/modernisation-platform-terraform-loadbalancer" + providers = { + # Here we use the default provider for the S3 bucket module, buck replication is disabled but we still + # Need to pass the provider to the S3 bucket module + aws.bucket-replication = aws + } vpc_all = "${local.vpc_name}-${local.environment}" #existing_bucket_name = "my-bucket-name" application_name = local.application_name @@ -150,7 +155,6 @@ If you're looking to raise an issue with this module, please create a new issue | Name | Version | |------|---------| | [aws](#provider\_aws) | ~> 4.0 | -| [template](#provider\_template) | n/a | ## Modules @@ -169,9 +173,7 @@ If you're looking to raise an issue with this module, please create a new issue | [aws_security_group.lb](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/security_group) | resource | | [aws_elb_service_account.default](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/elb_service_account) | data source | | [aws_iam_policy_document.bucket_policy](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/iam_policy_document) | data source | -| [aws_region.current](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/region) | data source | | [aws_vpc.shared](https://registry.terraform.io/providers/hashicorp/aws/latest/docs/data-sources/vpc) | data source | -| [template_file.lb-access-logs](https://registry.terraform.io/providers/hashicorp/template/latest/docs/data-sources/file) | data source | ## Inputs diff --git a/main.tf b/main.tf index 4de24b60..24b64c72 100644 --- a/main.tf +++ b/main.tf @@ -111,18 +111,17 @@ data "aws_iam_policy_document" "bucket_policy" { ] resources = [ - var.existing_bucket_name != "" ? "arn:aws:s3:::${var.existing_bucket_name}" : "${module.s3-bucket[0].bucket.arn}" + var.existing_bucket_name != "" ? "arn:aws:s3:::${var.existing_bucket_name}" : module.s3-bucket[0].bucket.arn ] } } data "aws_elb_service_account" "default" {} -# https://www.terraform.io/docs/providers/aws/d/region.html -# Get the region of the callee -data "aws_region" "current" {} - +#tfsec:ignore:aws-elb-alb-not-public resource "aws_lb" "loadbalancer" { + #checkov:skip=CKV_AWS_150:preventing destroy can be controlled outside of the module + #checkov:skip=CKV2_AWS_28:WAF is configured outside of the module for more flexibility name = "${var.application_name}-lb" internal = false load_balancer_type = "application" @@ -130,9 +129,10 @@ resource "aws_lb" "loadbalancer" { subnets = [var.public_subnets[0], var.public_subnets[1], var.public_subnets[2]] enable_deletion_protection = var.enable_deletion_protection idle_timeout = var.idle_timeout + drop_invalid_header_fields = true access_logs { - bucket = var.existing_bucket_name != "" ? var.existing_bucket_name : "${module.s3-bucket[0].bucket.id}" + bucket = var.existing_bucket_name != "" ? var.existing_bucket_name : module.s3-bucket[0].bucket.id prefix = var.application_name enabled = true } @@ -175,25 +175,26 @@ resource "aws_security_group" "lb" { } } -data "template_file" "lb-access-logs" { - template = file("${path.module}/templates/create_table.sql") - - vars = { - bucket = var.existing_bucket_name != "" ? var.existing_bucket_name : "${module.s3-bucket[0].bucket.id}" - account_id = var.account_number - region = var.region - } -} resource "aws_athena_database" "lb-access-logs" { name = "loadbalancer_access_logs" - bucket = var.existing_bucket_name != "" ? var.existing_bucket_name : "${module.s3-bucket[0].bucket.id}" + bucket = var.existing_bucket_name != "" ? var.existing_bucket_name : module.s3-bucket[0].bucket.id + encryption_configuration { + encryption_option = "SSE_S3" + } } resource "aws_athena_named_query" "main" { name = "${var.application_name}-create-table" database = aws_athena_database.lb-access-logs.name - query = data.template_file.lb-access-logs.rendered + query = templatefile( + "${path.module}/templates/create_table.sql", + { + bucket = var.existing_bucket_name != "" ? var.existing_bucket_name : module.s3-bucket[0].bucket.id + account_id = var.account_number + region = var.region + } + ) } resource "aws_athena_workgroup" "lb-access-logs" { diff --git a/providers.tf b/providers.tf deleted file mode 100644 index 4c0b4f7b..00000000 --- a/providers.tf +++ /dev/null @@ -1,7 +0,0 @@ -provider "aws" { - alias = "bucket-replication" - region = "eu-west-2" - assume_role { - role_arn = "arn:aws:iam::${var.account_number}:role/MemberInfrastructureAccess" - } -} diff --git a/test/unit-test/main.tf b/test/unit-test/main.tf index 71067ce2..c8655f76 100644 --- a/test/unit-test/main.tf +++ b/test/unit-test/main.tf @@ -58,7 +58,11 @@ data "aws_subnet" "public_subnets_c" { module "lb_access_logs_enabled" { source = "../.." - + providers = { + # Here we use the default provider for the S3 bucket module, buck replication is disabled but we still + # Need to pass the provider to the S3 bucket module + aws.bucket-replication = aws + } vpc_all = "${local.vpc_name}-${local.environment}" application_name = local.application_name public_subnets = [data.aws_subnet.public_subnets_a.id, data.aws_subnet.public_subnets_b.id, data.aws_subnet.public_subnets_c.id] diff --git a/test/unit-test/role.tf b/test/unit-test/role.tf deleted file mode 100644 index a520f216..00000000 --- a/test/unit-test/role.tf +++ /dev/null @@ -1,44 +0,0 @@ -# S3 bucket replication: role -resource "aws_iam_role" "default" { - name = "AWSS3BucketReplication" - assume_role_policy = data.aws_iam_policy_document.s3-assume-role-policy.json -} - -# S3 bucket replication: assume role policy -data "aws_iam_policy_document" "s3-assume-role-policy" { - version = "2012-10-17" - statement { - effect = "Allow" - actions = ["sts:AssumeRole"] - - principals { - type = "Service" - identifiers = ["s3.amazonaws.com"] - } - } -} - -resource "aws_iam_policy" "default" { - name = "AWSS3BucketReplicationPolicy" - policy = data.aws_iam_policy_document.default-policy.json -} - -# S3 bucket replication: role policy -data "aws_iam_policy_document" "default-policy" { - version = "2012-10-17" - statement { - effect = "Allow" - actions = [ - "s3:GetObjectVersion", - "s3:GetObjectVersionAcl", - "s3:ReplicateObject", - "s3:ReplicateDelete" - ] - resources = ["*"] - } -} - -resource "aws_iam_role_policy_attachment" "default" { - role = aws_iam_role.default.name - policy_arn = aws_iam_policy.default.arn -} diff --git a/versions.tf b/versions.tf index 901dbcca..3b4ff026 100644 --- a/versions.tf +++ b/versions.tf @@ -1,8 +1,9 @@ terraform { required_providers { aws = { - version = "~> 4.0" - source = "hashicorp/aws" + source = "hashicorp/aws" + version = "~> 4.0" + configuration_aliases = [aws.bucket-replication] } } required_version = ">= 1.0.1"