cloudposse / terraform-aws-config-storage

This module creates an S3 bucket suitable for storing AWS Config data
https://cloudposse.com/accelerate
Apache License 2.0
6 stars 8 forks source link

module does not properly support the policy variable #31

Open gpetras opened 1 year ago

gpetras commented 1 year ago

Describe the Bug

The module has a policy variable, but it doesn't seem to work properly

Expected Behavior

The default S3 bucket policy can be overridden by using the policy variable, and that policy will be applied to the S3 bucket that this module creates.

Steps to Reproduce

Here's the Terraform code to reproduce:

data "aws_caller_identity" "current" {}

locals {
  account_id = data.aws_caller_identity.current.account_id
}

module "aws_config_storage" {
  source                   = "cloudposse/config-storage/aws"
  version                  = "1.0.0"
  name                     = null
  stage                    = "prod"
  namespace                = "test"
  policy                   = data.aws_iam_policy_document.aws_config_bucket_policy.json
  standard_transition_days = 30
  glacier_transition_days  = 60
}

# Generate a bucket policy for use in the logging account. You won't see this reflected in the plan
data "aws_iam_policy_document" "aws_config_bucket_policy" {
  statement {
    sid = "ExampleCustomPolicy"
    principals {
      type        = "Service"
      identifiers = ["config.amazonaws.com"]
    }
    actions   = ["s3:GetBucketAcl"]
    resources = ["arn:aws:s3:::test-prod-aws-config"]
    condition {
      test     = "StringEquals"
      variable = "AWS:SourceAccount"
      values   = [local.account_id]
    }
  }
}

Screenshots

$ terraform plan
module.aws_config_storage.module.storage[0].module.aws_s3_bucket.data.aws_partition.current[0]: Reading...
module.aws_config_storage.module.storage[0].module.aws_s3_bucket.data.aws_canonical_user_id.default[0]: Reading...
data.aws_caller_identity.current: Reading...
module.aws_config_storage.module.storage[0].data.aws_caller_identity.current[0]: Reading...
module.aws_config_storage.module.storage[0].data.aws_partition.current[0]: Reading...
module.aws_config_storage.data.aws_caller_identity.current: Reading...
module.aws_config_storage.data.aws_partition.current: Reading...
module.aws_config_storage.module.storage[0].module.aws_s3_bucket.data.aws_partition.current[0]: Read complete after 0s [id=aws]
module.aws_config_storage.module.storage[0].data.aws_partition.current[0]: Read complete after 0s [id=aws]
module.aws_config_storage.data.aws_partition.current: Read complete after 0s [id=aws]
module.aws_config_storage.data.aws_iam_policy_document.aws_config_bucket_policy[0]: Reading...
module.aws_config_storage.data.aws_iam_policy_document.aws_config_bucket_policy[0]: Read complete after 0s [id=1660148360]
module.aws_config_storage.module.storage[0].data.aws_caller_identity.current[0]: Read complete after 0s [id=123456789123]
data.aws_caller_identity.current: Read complete after 0s [id=123456789123]
module.aws_config_storage.data.aws_caller_identity.current: Read complete after 0s [id=123456789123]
data.aws_iam_policy_document.aws_config_bucket_policy: Reading...
data.aws_iam_policy_document.aws_config_bucket_policy: Read complete after 0s [id=838523877]
module.aws_config_storage.module.storage[0].module.aws_s3_bucket.data.aws_canonical_user_id.default[0]: Read complete after 0s [id=XXXX]

Terraform used the selected providers to generate the following execution plan. Resource actions are indicated with the following symbols:
  + create
 <= read (data resources)

Terraform will perform the following actions:

  # module.aws_config_storage.module.storage[0].module.aws_s3_bucket.data.aws_iam_policy_document.aggregated_policy[0] will be read during apply
  # (config refers to values not yet known)
 <= data "aws_iam_policy_document" "aggregated_policy" {
      + id                        = (known after apply)
      + json                      = (known after apply)
      + override_policy_documents = [
          + jsonencode(
                {
                  + Statement = [
                      + {
                          + Action    = "s3:GetBucketAcl"
                          + Effect    = "Allow"
                          + Principal = {
                              + Service = "config.amazonaws.com"
                            }
                          + Resource  = "arn:aws:s3:::test-prod-aws-config"
                          + Sid       = "AWSConfigBucketPermissionsCheck"
                        },
                      + {
                          + Action    = "s3:ListBucket"
                          + Effect    = "Allow"
                          + Principal = {
                              + Service = "config.amazonaws.com"
                            }
                          + Resource  = "arn:aws:s3:::test-prod-aws-config"
                          + Sid       = "AWSConfigBucketExistenceCheck"
                        },
                      + {
                          + Action    = "s3:PutObject"
                          + Condition = {
                              + StringLike = {
                                  + "s3:x-amz-acl" = "bucket-owner-full-control"
                                }
                            }
                          + Effect    = "Allow"
                          + Principal = {
                              + Service = "config.amazonaws.com"
                            }
                          + Resource  = "arn:aws:s3:::test-prod-aws-config/AWSLogs/*"
                          + Sid       = "AWSConfigBucketDelivery"
                        },
                    ]
                  + Version   = "2012-10-17"
                }
            ),
        ]
      + source_policy_documents   = [
          + (known after apply),
        ]
    }

  # module.aws_config_storage.module.storage[0].module.aws_s3_bucket.data.aws_iam_policy_document.bucket_policy[0] will be read during apply
  # (depends on a resource or a module with changes pending)
 <= data "aws_iam_policy_document" "bucket_policy" {
      + id   = (known after apply)
      + json = (known after apply)
    }

  # module.aws_config_storage.module.storage[0].module.aws_s3_bucket.aws_s3_bucket.default[0] will be created
  + resource "aws_s3_bucket" "default" {
      + acceleration_status         = (known after apply)
      + acl                         = (known after apply)
      + arn                         = (known after apply)
      + bucket                      = "test-prod-aws-config"
      + bucket_domain_name          = (known after apply)
      + bucket_prefix               = (known after apply)
      + bucket_regional_domain_name = (known after apply)
      + force_destroy               = false
      + hosted_zone_id              = (known after apply)
      + id                          = (known after apply)
      + object_lock_enabled         = false
      + policy                      = (known after apply)
      + region                      = (known after apply)
      + request_payer               = (known after apply)
      + tags                        = {
          + "Attributes" = "aws-config"
          + "Name"       = "test-prod"
          + "Namespace"  = "test"
          + "Stage"      = "prod"
        }
      + tags_all                    = {
          + "Attributes" = "aws-config"
          + "Name"       = "test-prod"
          + "Namespace"  = "test"
          + "Stage"      = "prod"
        }
      + website_domain              = (known after apply)
      + website_endpoint            = (known after apply)
    }

  # module.aws_config_storage.module.storage[0].module.aws_s3_bucket.aws_s3_bucket_acl.default[0] will be created
  + resource "aws_s3_bucket_acl" "default" {
      + acl    = "private"
      + bucket = (known after apply)
      + id     = (known after apply)
    }

  # module.aws_config_storage.module.storage[0].module.aws_s3_bucket.aws_s3_bucket_lifecycle_configuration.default[0] will be created
  + resource "aws_s3_bucket_lifecycle_configuration" "default" {
      + bucket = (known after apply)
      + id     = (known after apply)

      + rule {
          + id     = "test-prod-aws-config"
          + status = "Enabled"

          + abort_incomplete_multipart_upload {
              + days_after_initiation = 5
            }

          + expiration {
              + days                         = 90
              + expired_object_delete_marker = (known after apply)
            }

          + filter {
            }

          + noncurrent_version_expiration {
              + noncurrent_days = 90
            }

          + noncurrent_version_transition {
              + noncurrent_days = 30
              + storage_class   = "GLACIER"
            }

          + transition {
              + days          = 30
              + storage_class = "STANDARD_IA"
            }
          + transition {
              + days          = 60
              + storage_class = "GLACIER"
            }
        }
    }

  # module.aws_config_storage.module.storage[0].module.aws_s3_bucket.aws_s3_bucket_ownership_controls.default[0] will be created
  + resource "aws_s3_bucket_ownership_controls" "default" {
      + bucket = (known after apply)
      + id     = (known after apply)

      + rule {
          + object_ownership = "BucketOwnerPreferred"
        }
    }

  # module.aws_config_storage.module.storage[0].module.aws_s3_bucket.aws_s3_bucket_public_access_block.default[0] will be created
  + resource "aws_s3_bucket_public_access_block" "default" {
      + block_public_acls       = true
      + block_public_policy     = true
      + bucket                  = (known after apply)
      + id                      = (known after apply)
      + ignore_public_acls      = true
      + restrict_public_buckets = true
    }

  # module.aws_config_storage.module.storage[0].module.aws_s3_bucket.aws_s3_bucket_server_side_encryption_configuration.default[0] will be created
  + resource "aws_s3_bucket_server_side_encryption_configuration" "default" {
      + bucket = (known after apply)
      + id     = (known after apply)

      + rule {
          + bucket_key_enabled = false

          + apply_server_side_encryption_by_default {
              + sse_algorithm = "AES256"
            }
        }
    }

  # module.aws_config_storage.module.storage[0].module.aws_s3_bucket.aws_s3_bucket_versioning.default[0] will be created
  + resource "aws_s3_bucket_versioning" "default" {
      + bucket = (known after apply)
      + id     = (known after apply)

      + versioning_configuration {
          + mfa_delete = (known after apply)
          + status     = "Enabled"
        }
    }

  # module.aws_config_storage.module.storage[0].module.aws_s3_bucket.time_sleep.wait_for_aws_s3_bucket_settings[0] will be created
  + resource "time_sleep" "wait_for_aws_s3_bucket_settings" {
      + create_duration  = "30s"
      + destroy_duration = "30s"
      + id               = (known after apply)
    }

Plan: 8 to add, 0 to change, 0 to destroy.

────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────

Note: You didn't use the -out option to save this plan, so Terraform can't guarantee to take exactly these actions if you run "terraform apply" now.

Environment

Additional Context

Terraform plan above should show a policy with SID = ExampleCustomPolicy but it does not.

gpetras commented 1 year ago

This was also pointed out in this comment on another issue: https://github.com/cloudposse/terraform-aws-config-storage/issues/14#issuecomment-1222337802