trussworks / terraform-aws-logs Goto Github PK

Add an input to the module to enable s3_bucket_name random generator, which creates s3_bucket_name using a prefix through a variable and a random id. For example:

s3_bucket_name = "${var.perfix}-${random_id.bucket_id.dec}"

There are a number of things that changed between the 1.7.1 and 2.1.0 release of this module. A migration guide would be very helpful and should include a couple things:

• CloudTrail management disappeared between versions. Previously it was managed but users now need to provide their own implementation.
• Template Provider 1.0.0 used to work with the module and now you need Template Provider 2.X or better
• Several of the resources names were changed, which means you need to use terraform mv to manage the transition in your own state.
• The bucket policy was moved out of the aws_s3_bucket resource and into a aws_s3_bucket_policy. While this is potentially a no-op it has the effect of making it look like your policy is being removed completely unless you specify default_allow=true as a variable. And then you've got to compare your new and old policy to make sure it has what you want in it.
• It adds a aws_s3_bucket_public_access_block with sane defaults but there's no way to change the behavior if you didn't have these set before.

A transition guide would be really helpful for folks using this module to help them move forward. If I have time I'll try to make one but I want the issue open here to remind me.

In the project description there's a statement that the terraform v12 users should pin to module v8, however the latest tagged release is specifying tf13 as requirement in versions.tf?
Is there some reason for it or was that just oversight during backporting?

If you're passing AWS FTR, mfa_delete is required for CloudTrail buckets.

From AWS SecurityHub:

[CIS.2.6] S3 Bucket Access Logging generates a log that contains access records for each request made to your S3 bucket. An access log record contains details about the request, such as the request type, the resources specified in the request worked, and the time and date the request was processed. It is recommended that bucket access logging be enabled on the CloudTrail S3 bucket.

Having an option to send logs from a bucket created by this module to another bucket could be used to remediate this issue.

This issue lists Renovate updates and detected dependencies. Read the Dependency Dashboard docs to learn more.

This repository currently has no open or pending branches.

Detected dependencies

• Check this box to trigger a request for Renovate to run again on this repository

Normal log path for cloudtrail for account 123456789012 is:

my-logs-bucket/cloudtrail/AWSLogs/123456789012

When is_organization_trail=true on the aws_cloudtrail resource the path becomes

my-logs-bucket/cloudtrail/AWSLogs/o-somehash/123456789012

and so

Not sure what to suggest as a fix.

Is it possible to get a TF 0.12 version released which supports aws provider 3.x?

I think this would basically just be removing the region value like was done in https://github.com/trussworks/terraform-aws-logs/releases/tag/v10.0.0.

I'd be happy to open a PR if this would be welcome.

This is an old module and as such, uses a JSON template for the policy. Instead, it should be defined in the Terraform itself via an "aws_iam_policy_document" data source. It's far more readable that way.

It would be great if this modules enables support for logging vpc_flow_logs. Would be happy to PR but new to terratest and I maynot be able to address the test part though.

From main.tf I believe we'd have to adjust that section.

  server_side_encryption_configuration {
    rule {
      apply_server_side_encryption_by_default {
        sse_algorithm = "AES256"
      }
    }
  }

Would this codebase be receptive to a PR to handle either default or CMK encryption?

I think the title is quite clear ;-)

Hi there, trying out the module for the first time, and ran into the following error:

module "s3_logs" {
  source         = "trussworks/logs/aws"
  s3_bucket_name = "${var.corp}-${var.env}-logs"
  region         = "${var.aws_region}"
}

Terraform apply:

Error: Error running plan: 1 error(s) occurred:

* module.s3_logs.aws_s3_bucket_policy.bucket_policy: "policy" contains an invalid JSON: invalid character '{' after array element

Any ideas? Thanks!

EDIT: Testing on Terraform 0.11.x

Dependabot couldn't parse the go.mod found at /go.mod.

The error Dependabot encountered was:

go: github.com/gruntwork-io/[email protected] requires
	github.com/google/[email protected] requires
	github.com/vdemeester/[email protected] requires
	k8s.io/[email protected] requires
	google.golang.org/[email protected]: invalid version: git fetch -f origin refs/heads/*:refs/heads/* refs/tags/*:refs/tags/* in /opt/go/gopath/pkg/mod/cache/vcs/30a5dbaa452c7ca9354df264080379bbcf24496036c60968495fa0ec4a41888c: exit status 128:
	error: RPC failed; HTTP 502 curl 22 The requested URL returned error: 502 Bad Gateway
	fatal: The remote end hung up unexpectedly

View the update logs.

This module doesn't work with the template provider at 1.0.0. It's not clear from the error message why this is:

Releasing state lock. This may take a few moments...

Error: Error running plan: 2 errors occurred:
        * module.logs.aws_s3_bucket_policy.bucket_policy: "policy" contains an invalid JSON: invalid character '%' looking for beginning of value
        * module.logs_us_east_1.aws_s3_bucket_policy.bucket_policy: "policy" contains an invalid JSON: invalid character '%' looking for beginning of value

Can we pin the provider by including a providers.tf file?

Describe the bug
A clear and concise description of what the bug is.

To Reproduce
Steps to reproduce the behavior:

1. Clone https://github.com/trussworks/terraform-aws-logs
2. cd to terraform-aws-logs/examples/s3
3. terraform apply

Expected behavior
Able to create bucket successfully.

Screenshots
n/a

Code Snippet

 ❯ terraform apply
var.force_destroy
  Enter a value: 1

var.region
  Enter a value: us-east-2

var.s3_logs_prefix
  Enter a value: pref

var.test_name
  Enter a value: tf-aws-logs-test-bucket


An execution plan has been generated and is shown below.
Resource actions are indicated with the following symbols:
  + create

module.aws_logs.aws_s3_bucket_lifecycle_configuration.aws_logs: Creation complete after 32s [id=tf-aws-logs-test-bucket]

Error: error creating S3 bucket ACL for tf-aws-logs-test-bucket-source: AccessControlListNotSupported: The bucket does not allow ACLs
	status code: 400, request id: 6X4TZRP5EQ2YPDQ3, host id: 9mHxqkGpu11rF2zIiAn99oeV3SMHQG+zMjmnl7CK1eG6f9nQBtF7MgrAsXfpwYWCZwGRp9bBD7k=

  on main.tf line 15, in resource "aws_s3_bucket_acl" "log_source_bucket":
  15: resource "aws_s3_bucket_acl" "log_source_bucket" {



Error: error creating S3 bucket ACL for tf-aws-logs-test-bucket: AccessControlListNotSupported: The bucket does not allow ACLs
	status code: 400, request id: 3G4V9C00DNN2ZCAT, host id: OPOKoxFPHI+fhxMTT3M2kh25bWr7gOH02pY7Fp4aMjoHbdTh9Ud1+RAh1V7hvi6CIvsigoVVlX8=

  on ../../main.tf line 424, in resource "aws_s3_bucket_acl" "aws_logs":
 424: resource "aws_s3_bucket_acl" "aws_logs" {

Additional context
Possibly related to https://aws.amazon.com/about-aws/whats-new/2022/12/amazon-s3-automatically-enable-block-public-access-disable-access-control-lists-buckets-april-2023/

Is your feature request related to a problem? Please describe.
This module creates a bucket without the option to provide a kms key to encrypt objects. This produces a finding in AWS Config "Checks if the S3 buckets are encrypted with AWS Key Management Service (AWS KMS). The rule is NON_COMPLIANT if the S3 bucket is not encrypted with an AWS KMS key."

Describe the solution you'd like
Id like for a kms key to be an input to this module which would be used instead of default encryption
Describe alternatives you've considered
I have explored no alternatives
Additional context
Add any other context or screenshots about the feature request here.

The variable allow_s3 is used in two places in the module:

https://github.com/trussworks/terraform-aws-logs/blob/master/main.tf#L83

and

https://github.com/trussworks/terraform-aws-logs/blob/master/main.tf#L111

In the first instance it appears to change the acl between "log-delivery-write" and "private" and in the second instance it seems to manage the existence of the s3 bucket policy. Which would mean:

True = "private" + no policy
False = "log-delivery-write" + policy

There is also no associated policy for S3 in the template file as there is for the other allow_* variables. I'm not sure that its doing what is intended then based on this.

For my use case I'd love to set this as my module:

module "logs" {
  source  = "trussworks/logs/aws"
  version = "~> 2.1.0"

  allow_alb        = true
  allow_cloudtrail = true
  allow_config     = true
  allow_cloudwatch = true
  allow_s3         = true

  default_allow = false

  s3_log_bucket_retention = "${local.log_retention_days}"
  region                  = "${local.region}"
  s3_bucket_name          = "${local.aws_logs_bucket}"
}

But I get no attached policy, which doesn't seem to be what I want. Instead I have to use this to get the template policy:

module "logs" {
  source  = "trussworks/logs/aws"
  version = "~> 2.1.0"

  default_allow = true

  s3_log_bucket_retention = "${local.log_retention_days}"
  region                  = "${local.region}"
  s3_bucket_name          = "${local.aws_logs_bucket}"
}

But then I have no control over the individual statements inside the template.

Your module do not support new AWS regions.

Describe the solution you'd like
Need to modify module to support new AWS regions(https://docs.aws.amazon.com/elasticloadbalancing/latest/classic/enable-access-logs.html#attach-bucket-policy)

Is your feature request related to a problem? Please describe.
It's nice to be able to also use this module for VPC Flow Logs.

Describe the solution you'd like
Add allow_vpc_flow_logs parameter and possibly extra ones related to VPC Flow Logs

Describe alternatives you've considered
Using your private bucket module.

Additional context
Most of these are security-related logs so it would be nice if they could be put under one umbrella.