Allows users to provide their own S3 bucket for use in holding LB Access Logs (#37)

* Allows users to provide their own S3 bucket for use in holding LB Access Logs

Our organization has a single bucket designated for LB access logs in
each environment, and we'd like to avoid having a 1:1 ratio between LBs
and log buckets.

This commit enables the module user to provide the ID of a pre-existing
S3 bucket that the LB is configured to use.  This becomes mutually
exclusive with the existing "enable_s3_logs" boolean.

* Keeping enable_s3_logs as the overarching variable, allowing bring-your-own bucket ID variable

* Removing now-unnecessary dynamic access_logs

* Fixing outputs.tf

* Checked for null incorrectly, fixing

* Explicitly setting vars to try to pass the test

* Fixing test error by defining local.bucket_id

Author: tvaughan77

.terraform.lock.hcl

@@ -19,11 +19,11 @@ This terraform module creates an Application Load Balancer and security group ru
 
 ## Install pre commit hooks.
 
-Pleas run this command right after cloning the repository.
+Please run this command right after cloning the repository.
 
         pre-commit install
 
-For that you may need to install the folowwing tools:
+For that you may need to install the following tools:
 * [Pre-commit](https://pre-commit.com/) 
 * [Terraform Docs](https://terraform-docs.io/)
 
@@ -43,14 +43,14 @@ In order to run all checks at any point run the following command:
 
 | Name | Version |
 |------|---------|
-| <a name="provider_aws"></a> [aws](#provider\_aws) | 4.24.0 |
-| <a name="provider_random"></a> [random](#provider\_random) | 3.3.2 |
+| <a name="provider_aws"></a> [aws](#provider\_aws) | 4.48.0 |
+| <a name="provider_random"></a> [random](#provider\_random) | 3.4.3 |
 
 ## Modules
 
 | Name | Source | Version |
 |------|--------|---------|
-| <a name="module_lb_logs_s3"></a> [lb\_logs\_s3](#module\_lb\_logs\_s3) | cn-terraform/logs-s3-bucket/aws | 1.0.3 |
+| <a name="module_lb_logs_s3"></a> [lb\_logs\_s3](#module\_lb\_logs\_s3) | cn-terraform/logs-s3-bucket/aws | 1.0.5 |
 
 ## Resources
 
@@ -73,16 +73,17 @@ In order to run all checks at any point run the following command:
 
 | Name | Description | Type | Default | Required |
 |------|-------------|------|---------|:--------:|
+| <a name="input_access_logs_prefix"></a> [access\_logs\_prefix](#input\_access\_logs\_prefix) | (Optional) if access logging to an S3 bucket, this sets a prefix in the bucket beneath which this LB's logs will be organized. | `string` | `null` | no |
 | <a name="input_additional_certificates_arn_for_https_listeners"></a> [additional\_certificates\_arn\_for\_https\_listeners](#input\_additional\_certificates\_arn\_for\_https\_listeners) | (Optional) List of SSL server certificate ARNs for HTTPS listener. Use it if you need to set additional certificates besides default\_certificate\_arn | `list(any)` | `[]` | no |
-| <a name="input_block_s3_bucket_public_access"></a> [block\_s3\_bucket\_public\_access](#input\_block\_s3\_bucket\_public\_access) | (Optional) If true, public access to the S3 bucket will be blocked. | `bool` | `true` | no |
+| <a name="input_block_s3_bucket_public_access"></a> [block\_s3\_bucket\_public\_access](#input\_block\_s3\_bucket\_public\_access) | (Optional) If true, public access to the S3 bucket will be blocked.  Ignored if log\_bucket\_id is provided. | `bool` | `true` | no |
 | <a name="input_default_certificate_arn"></a> [default\_certificate\_arn](#input\_default\_certificate\_arn) | (Optional) The ARN of the default SSL server certificate. Required if var.https\_ports is set. | `string` | `null` | no |
 | <a name="input_deregistration_delay"></a> [deregistration\_delay](#input\_deregistration\_delay) | (Optional) The amount time for Elastic Load Balancing to wait before changing the state of a deregistering target from draining to unused. The range is 0-3600 seconds. The default value is 300 seconds. | `number` | `300` | no |
 | <a name="input_drop_invalid_header_fields"></a> [drop\_invalid\_header\_fields](#input\_drop\_invalid\_header\_fields) | (Optional) Indicates whether HTTP headers with header fields that are not valid are removed by the load balancer (true) or routed to targets (false). The default is false. Elastic Load Balancing requires that message header names contain only alphanumeric characters and hyphens. | `bool` | `false` | no |
 | <a name="input_enable_cross_zone_load_balancing"></a> [enable\_cross\_zone\_load\_balancing](#input\_enable\_cross\_zone\_load\_balancing) | (Optional) If true, cross-zone load balancing of the load balancer will be enabled. Defaults to false. | `bool` | `false` | no |
 | <a name="input_enable_deletion_protection"></a> [enable\_deletion\_protection](#input\_enable\_deletion\_protection) | (Optional) If true, deletion of the load balancer will be disabled via the AWS API. This will prevent Terraform from deleting the load balancer. Defaults to false. | `bool` | `false` | no |
 | <a name="input_enable_http2"></a> [enable\_http2](#input\_enable\_http2) | (Optional) Indicates whether HTTP/2 is enabled in the load balancer. Defaults to true. | `bool` | `true` | no |
-| <a name="input_enable_s3_bucket_server_side_encryption"></a> [enable\_s3\_bucket\_server\_side\_encryption](#input\_enable\_s3\_bucket\_server\_side\_encryption) | (Optional) If true, server side encryption will be applied. | `bool` | `true` | no |
-| <a name="input_enable_s3_logs"></a> [enable\_s3\_logs](#input\_enable\_s3\_logs) | (Optional) If true, all resources to send LB logs to S3 will be created | `bool` | `true` | no |
+| <a name="input_enable_s3_bucket_server_side_encryption"></a> [enable\_s3\_bucket\_server\_side\_encryption](#input\_enable\_s3\_bucket\_server\_side\_encryption) | (Optional) If true, server side encryption will be applied.  Ignored if log\_bucket\_id is provided. | `bool` | `true` | no |
+| <a name="input_enable_s3_logs"></a> [enable\_s3\_logs](#input\_enable\_s3\_logs) | (Optional) If true, all LoadBalancer logs will be sent to S3.  If true, and log\_bucket\_id is *not* provided, this module will create the bucket with other provided s3 bucket configuration options | `bool` | `true` | no |
 | <a name="input_http_ingress_cidr_blocks"></a> [http\_ingress\_cidr\_blocks](#input\_http\_ingress\_cidr\_blocks) | List of CIDR blocks to allowed to access the Load Balancer through HTTP | `list(string)` | <pre>[<br>  "0.0.0.0/0"<br>]</pre> | no |
 | <a name="input_http_ingress_prefix_list_ids"></a> [http\_ingress\_prefix\_list\_ids](#input\_http\_ingress\_prefix\_list\_ids) | List of prefix list IDs blocks to allowed to access the Load Balancer through HTTP | `list(string)` | `[]` | no |
 | <a name="input_http_ports"></a> [http\_ports](#input\_http\_ports) | Map containing objects to define listeners behaviour based on type field. If type field is `forward`, include listener\_port and the target\_group\_port. For `redirect` type, include listener port, host, path, port, protocol, query and status\_code. For `fixed-response`, include listener\_port, content\_type, message\_body and status\_code | `map(any)` | <pre>{<br>  "default_http": {<br>    "listener_port": 80,<br>    "target_group_port": 80,<br>    "type": "forward"<br>  }<br>}</pre> | no |
@@ -93,11 +94,12 @@ In order to run all checks at any point run the following command:
 | <a name="input_internal"></a> [internal](#input\_internal) | (Optional) If true, the LB will be internal. | `bool` | `false` | no |
 | <a name="input_ip_address_type"></a> [ip\_address\_type](#input\_ip\_address\_type) | (Optional) The type of IP addresses used by the subnets for your load balancer. The possible values are ipv4 and dualstack. Defaults to ipv4 | `string` | `"ipv4"` | no |
 | <a name="input_load_balancing_algorithm_type"></a> [load\_balancing\_algorithm\_type](#input\_load\_balancing\_algorithm\_type) | (Optional) Determines how the load balancer selects targets when routing requests. The value is round\_robin or least\_outstanding\_requests. The default is round\_robin. | `string` | `"round_robin"` | no |
+| <a name="input_log_bucket_id"></a> [log\_bucket\_id](#input\_log\_bucket\_id) | (Optional) if provided, the ID of a previously-defined S3 bucket to send LB logs to. | `string` | `null` | no |
 | <a name="input_name_prefix"></a> [name\_prefix](#input\_name\_prefix) | Name prefix for resources on AWS | `any` | n/a | yes |
 | <a name="input_private_subnets"></a> [private\_subnets](#input\_private\_subnets) | A list of private subnet IDs to attach to the LB if it is INTERNAL. | `list(string)` | n/a | yes |
 | <a name="input_public_subnets"></a> [public\_subnets](#input\_public\_subnets) | A list of public subnet IDs to attach to the LB if it is NOT internal. | `list(string)` | n/a | yes |
-| <a name="input_s3_bucket_server_side_encryption_key"></a> [s3\_bucket\_server\_side\_encryption\_key](#input\_s3\_bucket\_server\_side\_encryption\_key) | (Optional) The AWS KMS master key ID used for the SSE-KMS encryption. This can only be used when you set the value of sse\_algorithm as aws:kms. The default aws/s3 AWS KMS master key is used if this element is absent while the sse\_algorithm is aws:kms. | `string` | `null` | no |
-| <a name="input_s3_bucket_server_side_encryption_sse_algorithm"></a> [s3\_bucket\_server\_side\_encryption\_sse\_algorithm](#input\_s3\_bucket\_server\_side\_encryption\_sse\_algorithm) | (Optional) The server-side encryption algorithm to use. Valid values are AES256 and aws:kms | `string` | `"AES256"` | no |
+| <a name="input_s3_bucket_server_side_encryption_key"></a> [s3\_bucket\_server\_side\_encryption\_key](#input\_s3\_bucket\_server\_side\_encryption\_key) | (Optional) The AWS KMS master key ID used for the SSE-KMS encryption. This can only be used when you set the value of sse\_algorithm as aws:kms. The default aws/s3 AWS KMS master key is used if this element is absent while the sse\_algorithm is aws:kms.  Ignored if log\_bucket\_id is provided. | `string` | `null` | no |
+| <a name="input_s3_bucket_server_side_encryption_sse_algorithm"></a> [s3\_bucket\_server\_side\_encryption\_sse\_algorithm](#input\_s3\_bucket\_server\_side\_encryption\_sse\_algorithm) | (Optional) The server-side encryption algorithm to use. Valid values are AES256 and aws:kms.   Ignored if log\_bucket\_id is provided. | `string` | `"AES256"` | no |
 | <a name="input_security_groups"></a> [security\_groups](#input\_security\_groups) | (Optional) A list of security group IDs to assign to the LB. | `list(string)` | `[]` | no |
 | <a name="input_slow_start"></a> [slow\_start](#input\_slow\_start) | (Optional) The amount time for targets to warm up before the load balancer sends them a full share of requests. The range is 30-900 seconds or 0 to disable. The default value is 0 seconds. | `number` | `0` | no |
 | <a name="input_ssl_policy"></a> [ssl\_policy](#input\_ssl\_policy) | (Optional) The name of the SSL Policy for the listener. . Required if var.https\_ports is set. | `string` | `null` | no |

README.md

@@ -14,4 +14,6 @@ module "load_balancer" {
   vpc_id          = module.base-network.vpc_id
   private_subnets = module.base-network.private_subnets_ids
   public_subnets  = module.base-network.public_subnets_ids
+  enable_s3_logs  = true
+  log_bucket_id   = null
 }

examples/test/.terraform.lock.hcl

@@ -0,0 +1,16 @@
+locals {
+  bucket_id = "my-tf-test-bucket"
+}
+
+resource "aws_s3_bucket" "bucket" {
+  bucket = local.bucket_id
+}
+
+module "load_balancer_bring_your_own_bucket" {
+  source          = "../../"
+  name_prefix     = "test-alb"
+  vpc_id          = module.base-network.vpc_id
+  private_subnets = module.base-network.private_subnets_ids
+  public_subnets  = module.base-network.public_subnets_ids
+  log_bucket_id   = local.bucket_id
+}

examples/test/main.tf

@@ -4,11 +4,11 @@
 data "aws_elb_service_account" "default" {}
 
 module "lb_logs_s3" {
-  count = var.enable_s3_logs ? 1 : 0
+  # If we enable S3 Logs for the ALB, but don't provide our own bucket, create one as part of this module
+  count = var.enable_s3_logs && var.log_bucket_id == null ? 1 : 0
 
   source  = "cn-terraform/logs-s3-bucket/aws"
   version = "1.0.5"
-  # source  = "../terraform-aws-logs-s3-bucket"
 
   name_prefix                                    = "${var.name_prefix}-lb"
   aws_principals_identifiers                     = [data.aws_elb_service_account.default.arn]
@@ -49,8 +49,9 @@ resource "aws_lb" "lb" {
   dynamic "access_logs" {
     for_each = var.enable_s3_logs ? [1] : []
     content {
-      bucket  = module.lb_logs_s3[0].s3_bucket_id
+      bucket  = var.log_bucket_id == null ? module.lb_logs_s3[0].s3_bucket_id : var.log_bucket_id
       enabled = var.enable_s3_logs
+      prefix  = var.access_logs_prefix
     }
   }