Added flexibility around logging

Author: brandonjbjelland

CHANGELOG.md

@@ -4,14 +4,22 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](http://keepachangelog.com/)
 and this project adheres to [Semantic Versioning](http://semver.org/).
 
+## [1.0.4] - 2017-11-06
+#### Added
+* added `create_log_bucket` and `enable_logging` to help control logging more granularly.
+
+#### Changed
+* existing related variables made more descriptive
+* S3 policy related test made more explicit (⭐ @antonbabenko)
+
 ## [1.0.3] - 2017-10-19
-## Added
+#### Added
 * TravisCI configuration added and now passing.
 * badge added to docs.
 * permissions section now in the example readme.
 * placeholder shell script added for CI deployment. Eventually this should conditionally release to the registry when those APIs become available.
 
-## Changed
+#### Changed
 * altered tf variable `aws_region` to `region`.
 * replaced hardcoding the region to instead use a random region as retrieved by an awscli docker container within CI.
 * example cert is now a regionally-specific resource enabling tests to run in various regions at once and not collide.

README.md

@@ -35,16 +35,18 @@ For an example of using ALB with ECS look no further than the [hashicorp example
 A full example leveraging other community modules is contained in the [examples/test_fixtures directory](examples/test_fixtures). Here's the gist of using it via the Terraform registry:
 ```
 module "alb" {
-  source              = "terraform-aws-modules/alb/aws"
-  alb_name            = "my-alb"
-  region              = "us-east-2"
-  alb_security_groups = ["sg-edcd9784", "sg-edcd9785"]
-  vpc_id              = "vpc-abcde012"
-  subnets             = ["subnet-abcde012", "subnet-bcde012a"]
-  certificate_arn     = "arn:aws:iam::123456789012:server-certificate/test_cert-123456789012"
-  log_bucket          = "logs-us-east-2-123456789012"
-  log_prefix          = "my-alb-logs"
-  health_check_path   = "/"
+  source                        = "terraform-aws-modules/alb/aws"
+  alb_name                      = "my-alb"
+  region                        = "us-east-2"
+  alb_security_groups           = ["sg-edcd9784", "sg-edcd9785"]
+  vpc_id                        = "vpc-abcde012"
+  subnets                       = ["subnet-abcde012", "subnet-bcde012a"]
+  certificate_arn               = "arn:aws:iam::123456789012:server-certificate/test_cert-123456789012"
+  create_log_bucket             = true
+  enable_logging                = true
+  log_bucket_name               = "logs-us-east-2-123456789012"
+  log_location_prefix           = "my-alb-logs"
+  health_check_path             = "/"
 
   tags {
     "Terraform" = "true"

examples/test_fixtures/main.tf

@@ -48,8 +48,10 @@ module "alb" {
   subnets                  = "${module.vpc.public_subnets}"
   certificate_arn          = "${aws_iam_server_certificate.fixture_cert.arn}"
   health_check_path        = "/"
-  log_bucket               = "logs-${var.region}-${data.aws_caller_identity.fixtures.account_id}"
-  log_prefix               = "${var.log_prefix}"
+  create_log_bucket        = true
+  enable_logging           = true
+  log_bucket_name          = "logs-${var.region}-${data.aws_caller_identity.fixtures.account_id}"
+  log_location_prefix      = "${var.log_location_prefix}"
   force_destroy_log_bucket = true
 
   tags {

examples/test_fixtures/variables.tf

@@ -1,4 +1,4 @@
-variable "log_prefix" {
+variable "log_location_prefix" {
   default = "my-alb-logs"
 }
 

main.tf

@@ -13,9 +13,9 @@ resource "aws_alb" "main" {
   tags            = "${merge(var.tags, map("Name", format("%s", var.alb_name)))}"
 
   access_logs {
-    bucket  = "${var.log_bucket}"
-    prefix  = "${var.log_prefix}"
-    enabled = "${var.log_bucket != ""}"
+    bucket  = "${var.log_bucket_name}"
+    prefix  = "${var.log_location_prefix}"
+    enabled = "${var.enable_logging}"
   }
 }
 
@@ -28,7 +28,7 @@ data "aws_iam_policy_document" "bucket_policy" {
     ]
 
     resources = [
-      "arn:aws:s3:::${var.log_bucket}/${var.log_prefix}/AWSLogs/${data.aws_caller_identity.current.account_id}/*",
+      "arn:aws:s3:::${var.log_bucket_name}/${var.log_location_prefix}/AWSLogs/${data.aws_caller_identity.current.account_id}/*",
     ]
 
     principals {
@@ -39,11 +39,11 @@ data "aws_iam_policy_document" "bucket_policy" {
 }
 
 resource "aws_s3_bucket" "log_bucket" {
-  bucket        = "${var.log_bucket}"
+  bucket        = "${var.log_bucket_name}"
   policy        = "${var.bucket_policy == "" ? data.aws_iam_policy_document.bucket_policy.json : var.bucket_policy}"
   force_destroy = "${var.force_destroy_log_bucket}"
-  count         = "${var.log_bucket != "" ? 1 : 0}"
-  tags          = "${merge(var.tags, map("Name", format("%s", var.log_bucket)))}"
+  count         = "${var.create_log_bucket ? 1 : 0}"
+  tags          = "${merge(var.tags, map("Name", format("%s", var.log_bucket_name)))}"
 }
 
 resource "aws_alb_target_group" "target_group" {
@@ -71,7 +71,7 @@ resource "aws_alb_target_group" "target_group" {
   tags = "${merge(var.tags, map("Name", format("%s-tg", var.alb_name)))}"
 }
 
-resource "aws_alb_listener" "front_end_http" {
+resource "aws_alb_listener" "frontend_http" {
   load_balancer_arn = "${aws_alb.main.arn}"
   port              = "80"
   protocol          = "HTTP"
@@ -83,7 +83,7 @@ resource "aws_alb_listener" "front_end_http" {
   }
 }
 
-resource "aws_alb_listener" "front_end_https" {
+resource "aws_alb_listener" "frontend_https" {
   load_balancer_arn = "${aws_alb.main.arn}"
   port              = "443"
   protocol          = "HTTPS"

test/integration/default/local_alb.rb

@@ -2,15 +2,15 @@
 require 'rhcl'
 
 module_vars = Rhcl.parse(File.open('examples/test_fixtures/variables.tf'))
-log_prefix = module_vars['variable']['log_prefix']['default']
+log_location_prefix = module_vars['variable']['log_location_prefix']['default']
 tf_state = JSON.parse(File.open('.kitchen/kitchen-terraform/default-aws/terraform.tfstate').read)
 principal_account_id = tf_state['modules'][0]['outputs']['principal_account_id']['value']
 account_id = tf_state['modules'][0]['outputs']['account_id']['value']
 vpc_id = tf_state['modules'][0]['outputs']['vpc_id']['value']
 security_group_id = tf_state['modules'][0]['outputs']['sg_id']['value']
 account_id = tf_state['modules'][0]['outputs']['account_id']['value']
 # this must match the format in examples/test_fixtures/locals.tf
-log_bucket = 'logs-' + ENV['AWS_REGION'] + '-' + account_id
+log_bucket_name = 'logs-' + ENV['AWS_REGION'] + '-' + account_id
 # subnet_ids = tf_state['modules'][0]['outputs']['subnet_ids']['value']
 
 describe alb('my-alb') do
@@ -34,9 +34,9 @@
     it { should belong_to_vpc('my-vpc') }
  end
 
-describe s3_bucket(log_bucket) do
+describe s3_bucket(log_bucket_name) do
   it { should exist }
-  it { should have_object("#{log_prefix}/AWSLogs/#{account_id}/ELBAccessLogTestFile") }
+  it { should have_object("#{log_location_prefix}/AWSLogs/#{account_id}/ELBAccessLogTestFile") }
     it do
     should have_policy <<-POLICY
 {
@@ -49,7 +49,7 @@
                 "AWS": "arn:aws:iam::#{principal_account_id}:root"
             },
             "Action": "s3:PutObject",
-            "Resource": "arn:aws:s3:::#{log_bucket}/#{log_prefix}/AWSLogs/#{account_id}/*"
+            "Resource": "arn:aws:s3:::#{log_bucket_name}/#{log_location_prefix}/AWSLogs/#{account_id}/*"
         }
     ]
 }

variables.tf

@@ -33,7 +33,7 @@ variable "backend_protocol" {
 }
 
 variable "bucket_policy" {
-  description = "A custom S3 bucket policy to apply to the log bucket. If not provided, a minimal policy will be generated from other variables."
+  description = "An S3 bucket policy to apply to the log bucket. If not provided, a minimal policy will be generated from other variables."
   default     = ""
 }
 
@@ -80,13 +80,23 @@ variable "health_check_unhealthy_threshold" {
   default     = 3
 }
 
-variable "log_bucket" {
+variable "create_log_bucket" {
+  default     = false
+  description = "Create the S3 bucket (named with the log_bucket_name var) and attach a policy to allow ALB logging."
+}
+
+variable "enable_logging" {
+  default     = false
+  description = "Enable the ALB to write log entries to S3."
+}
+
+variable "log_bucket_name" {
   description = "S3 bucket for storing ALB access logs. Setting this means the module will try to create the bucket."
   default     = ""
 }
 
-variable "log_prefix" {
-  description = "S3 prefix within the log_bucket under which logs are stored."
+variable "log_location_prefix" {
+  description = "S3 prefix within the log_bucket_name under which logs are stored."
   default     = ""
 }