Merge pull request #94 from trussworks/mr337_add_alb_nlb_accounts

Adding alb_account and nlb_account vars

README.md

@@ -109,6 +109,7 @@ module "aws_logs" {
 
 | Name | Description | Type | Default | Required |
 |------|-------------|------|---------|:--------:|
+| alb\_account | Account for ALB logs.  By default limits to the current account. | `string` | `""` | no |
 | alb\_logs\_prefixes | S3 key prefixes for ALB logs. | `list(string)` | <pre>[<br>  "alb"<br>]</pre> | no |
 | allow\_alb | Allow ALB service to log to bucket. | `bool` | `false` | no |
 | allow\_cloudtrail | Allow Cloudtrail service to log to bucket. | `bool` | `false` | no |
@@ -127,6 +128,7 @@ module "aws_logs" {
 | elb\_accounts | List of accounts for ELB logs.  By default limits to the current account. | `list(string)` | `[]` | no |
 | elb\_logs\_prefix | S3 prefix for ELB logs. | `string` | `"elb"` | no |
 | force\_destroy | A bool that indicates all objects (including any locked objects) should be deleted from the bucket so the bucket can be destroyed without error. | `bool` | `false` | no |
+| nlb\_account | Account for NLB logs.  By default limits to the current account. | `string` | `""` | no |
 | nlb\_logs\_prefixes | S3 key prefixes for NLB logs. | `list(string)` | <pre>[<br>  "nlb"<br>]</pre> | no |
 | redshift\_logs\_prefix | S3 prefix for RedShift logs. | `string` | `"redshift"` | no |
 | region | Region where the AWS S3 bucket will be created. | `string` | n/a | yes |

examples/alb_remote/main.tf

@@ -0,0 +1,16 @@
+# Testing the log bucket has a certain policy. Spinning up an ALB won't work as the
+# s3 prefix is different since the ALB will be using local Account ID, not the
+# external_account
+module "aws_logs" {
+  source = "../../"
+
+  s3_bucket_name    = var.test_name
+  alb_logs_prefixes = var.alb_logs_prefixes
+  region            = var.region
+  allow_alb         = true
+  default_allow     = false
+
+  alb_account = var.alb_external_account
+
+  force_destroy = var.force_destroy
+}

examples/alb_remote/providers.tf

@@ -0,0 +1,3 @@
+provider "aws" {
+  version = "~> 2.70"
+}

examples/alb_remote/variables.tf

@@ -0,0 +1,23 @@
+variable "test_name" {
+  type = string
+}
+
+variable "region" {
+  type = string
+}
+
+variable "vpc_azs" {
+  type = list(string)
+}
+
+variable "force_destroy" {
+  type = bool
+}
+
+variable "alb_external_account" {
+  type = string
+}
+
+variable "alb_logs_prefixes" {
+  type = list(string)
+}

examples/nlb_remote/main.tf

@@ -0,0 +1,16 @@
+# Testing the log bucket has a certain policy. Spinning up an ALB won't work as the
+# s3 prefix is different since the ALB will be using local Account ID, not the
+# external_account
+module "aws_logs" {
+  source = "../../"
+
+  s3_bucket_name    = var.test_name
+  nlb_logs_prefixes = var.nlb_logs_prefixes
+  region            = var.region
+  allow_nlb         = true
+  default_allow     = false
+
+  nlb_account = var.nlb_external_account
+
+  force_destroy = var.force_destroy
+}

examples/nlb_remote/providers.tf

@@ -0,0 +1,3 @@
+provider "aws" {
+  version = "~> 2.70"
+}

examples/nlb_remote/variables.tf

@@ -0,0 +1,23 @@
+variable "test_name" {
+  type = string
+}
+
+variable "region" {
+  type = string
+}
+
+variable "vpc_azs" {
+  type = list(string)
+}
+
+variable "force_destroy" {
+  type = bool
+}
+
+variable "nlb_external_account" {
+  type = string
+}
+
+variable "nlb_logs_prefixes" {
+  type = list(string)
+}

main.tf

@@ -85,6 +85,8 @@ locals {
   #
 
   # doesn't support logging to multiple accounts
+  alb_account = var.alb_account != "" ? var.alb_account : data.aws_caller_identity.current.account_id
+
   # supports logging to multiple prefixes
   alb_effect = var.default_allow || var.allow_alb ? "Allow" : "Deny"
 
@@ -98,13 +100,15 @@ locals {
   alb_logs_path = local.alb_include_root_prefix ? concat(local.alb_logs_path_temp, ["AWSLogs"]) : local.alb_logs_path_temp
 
   # finally, format the full final resources ARN list
-  alb_resources = sort(formatlist("${local.bucket_arn}/%s/${data.aws_caller_identity.current.account_id}/*", local.alb_logs_path))
+  alb_resources = sort(formatlist("${local.bucket_arn}/%s/${local.alb_account}/*", local.alb_logs_path))
 
   #
   # NLB locals
   #
 
   # doesn't support logging to multiple accounts
+  nlb_account = var.nlb_account != "" ? var.nlb_account : data.aws_caller_identity.current.account_id
+
   # supports logging to multiple prefixes
   nlb_effect = var.default_allow || var.allow_nlb ? "Allow" : "Deny"
 
@@ -114,7 +118,7 @@ locals {
 
   nlb_logs_path = local.nlb_include_root_prefix ? concat(local.nlb_logs_path_temp, ["AWSLogs"]) : local.nlb_logs_path_temp
 
-  nlb_resources = sort(formatlist("${local.bucket_arn}/%s/${data.aws_caller_identity.current.account_id}/*", local.nlb_logs_path))
+  nlb_resources = sort(formatlist("${local.bucket_arn}/%s/${local.nlb_account}/*", local.nlb_logs_path))
 
   #
   # Redshift locals

test/terraform_aws_logs_alb_test.go

@@ -9,8 +9,27 @@ import (
 	"github.com/gruntwork-io/terratest/modules/random"
 	"github.com/gruntwork-io/terratest/modules/terraform"
 	test_structure "github.com/gruntwork-io/terratest/modules/test-structure"
+	"github.com/stretchr/testify/require"
 )
 
+func AssertS3BucketPolicyAllowExternalALB(t *testing.T, region string, bucketName string, prefix string, externalAccount string) {
+	pattern := fmt.Sprintf(`"Action":"s3:PutObject","Resource":"arn:aws:s3:::%s/%s/AWSLogs/%s/*"`, bucketName, prefix, externalAccount)
+	err := AssertS3BucketPolicyContains(t, region, bucketName, pattern)
+	require.NoError(t, err)
+
+}
+
+func AssertS3BucketPolicyContains(t *testing.T, region string, bucketName string, pattern string) error {
+	policy, err := aws.GetS3BucketPolicyE(t, region, bucketName)
+	require.NoError(t, err)
+
+	if !strings.Contains(policy, pattern) {
+		return fmt.Errorf("could not find pattern: %s in policy: %s", pattern, policy)
+	}
+
+	return nil
+}
+
 func TestTerraformAwsLogsAlb(t *testing.T) {
 	t.Parallel()
 
@@ -62,3 +81,35 @@ func TestTerraformAwsLogsAlbRootPrefix(t *testing.T) {
 	defer terraform.Destroy(t, terraformOptions)
 	terraform.InitAndApply(t, terraformOptions)
 }
+
+func TestTerraformAwsLogsAlbAccount(t *testing.T) {
+	t.Parallel()
+
+	tempTestFolder := test_structure.CopyTerraformFolderToTemp(t, "../", "examples/alb_remote")
+	testName := fmt.Sprintf("terratest-aws-logs-%s", strings.ToLower(random.UniqueId()))
+	awsRegion := "us-west-2"
+	externalAlbAccount := "222222222222"
+	prefix := "alb"
+	vpcAzs := aws.GetAvailabilityZones(t, awsRegion)[:3]
+
+	terraformOptions := &terraform.Options{
+		TerraformDir: tempTestFolder,
+		Vars: map[string]interface{}{
+			"region":               awsRegion,
+			"vpc_azs":              vpcAzs,
+			"alb_external_account": externalAlbAccount,
+			"test_name":            testName,
+			"force_destroy":        true,
+			"alb_logs_prefixes":    []string{prefix},
+		},
+		EnvVars: map[string]string{
+			"AWS_DEFAULT_REGION": awsRegion,
+		},
+	}
+
+	defer terraform.Destroy(t, terraformOptions)
+	terraform.InitAndApply(t, terraformOptions)
+
+	// let us check to make sure the resource contains the alb_account
+	AssertS3BucketPolicyAllowExternalALB(t, awsRegion, testName, prefix, externalAlbAccount)
+}

test/terraform_aws_logs_nlb_test.go

@@ -9,8 +9,16 @@ import (
 	"github.com/gruntwork-io/terratest/modules/random"
 	"github.com/gruntwork-io/terratest/modules/terraform"
 	test_structure "github.com/gruntwork-io/terratest/modules/test-structure"
+	"github.com/stretchr/testify/require"
 )
 
+func AssertS3BucketPolicyAllowExternalNLB(t *testing.T, region string, bucketName string, prefix string, externalAccount string) {
+	pattern := fmt.Sprintf(`"Action":"s3:PutObject","Resource":"arn:aws:s3:::%s/%s/AWSLogs/%s/*"`, bucketName, prefix, externalAccount)
+	err := AssertS3BucketPolicyContains(t, region, bucketName, pattern)
+	require.NoError(t, err)
+
+}
+
 func TestTerraformAwsLogsNlb(t *testing.T) {
 	t.Parallel()
 
@@ -62,3 +70,35 @@ func TestTerraformAwsLogsNlbRootPrefix(t *testing.T) {
 	defer terraform.Destroy(t, terraformOptions)
 	terraform.InitAndApply(t, terraformOptions)
 }
+
+func TestTerraformAwsLogsNlbAccount(t *testing.T) {
+	t.Parallel()
+
+	tempTestFolder := test_structure.CopyTerraformFolderToTemp(t, "../", "examples/nlb_remote")
+	testName := fmt.Sprintf("terratest-aws-logs-%s", strings.ToLower(random.UniqueId()))
+	awsRegion := "us-west-2"
+	externalAlbAccount := "222222222222"
+	prefix := "nlb"
+	vpcAzs := aws.GetAvailabilityZones(t, awsRegion)[:3]
+
+	terraformOptions := &terraform.Options{
+		TerraformDir: tempTestFolder,
+		Vars: map[string]interface{}{
+			"region":               awsRegion,
+			"vpc_azs":              vpcAzs,
+			"nlb_external_account": externalAlbAccount,
+			"test_name":            testName,
+			"force_destroy":        true,
+			"nlb_logs_prefixes":    []string{prefix},
+		},
+		EnvVars: map[string]string{
+			"AWS_DEFAULT_REGION": awsRegion,
+		},
+	}
+
+	defer terraform.Destroy(t, terraformOptions)
+	terraform.InitAndApply(t, terraformOptions)
+
+	// let us check to make sure the resource contains the alb_account
+	AssertS3BucketPolicyAllowExternalALB(t, awsRegion, testName, prefix, externalAlbAccount)
+}

variables.tf

@@ -123,12 +123,24 @@ variable "config_accounts" {
   type        = list(string)
 }
 
+variable "alb_account" {
+  description = "Account for ALB logs.  By default limits to the current account."
+  default     = ""
+  type        = string
+}
+
 variable "elb_accounts" {
   description = "List of accounts for ELB logs.  By default limits to the current account."
   default     = []
   type        = list(string)
 }
 
+variable "nlb_account" {
+  description = "Account for NLB logs.  By default limits to the current account."
+  default     = ""
+  type        = string
+}
+
 variable "force_destroy" {
   description = "A bool that indicates all objects (including any locked objects) should be deleted from the bucket so the bucket can be destroyed without error."
   default     = false