Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unstable json in s3 bucket policy #53

Closed
booi opened this issue Dec 14, 2021 · 4 comments
Closed

Unstable json in s3 bucket policy #53

booi opened this issue Dec 14, 2021 · 4 comments
Labels
invalid This doesn't seem right

Comments

@booi
Copy link

booi commented Dec 14, 2021

Describe the Bug

Applying this terraform module appears to work fine but the s3 bucket policy json is unstable and terraform wants to update it every run.

Expected Behavior

Subsequent terraform runs should not trigger updates to the provisioned resources

Steps to Reproduce

  1. Provision
module "cloudtrail_s3_bucket" {
  source = "cloudposse/cloudtrail-s3-bucket/aws"
  version = "0.23.1"

  name                     = "cloudtrail_logs"
  stage                    = var.env_name
  namespace                = var.namespace
  standard_transition_days = 30
  glacier_transition_days  = 60
  expiration_days          = 365
  force_destroy = var.force_destroy
}

module "cloudtrail" {
  source = "cloudposse/cloudtrail/aws"
  version = "0.21.0"

  namespace = var.namespace
  stage = var.env_name
  name = "cloudtrail"
  enable_log_file_validation    = true
  include_global_service_events = true
  is_multi_region_trail         = true
  s3_bucket_name                = module.cloudtrail_s3_bucket.bucket_id
}
  1. terraform apply
    This correctly creates all resources.

  2. terraform apply

Note: Objects have changed outside of Terraform

Terraform detected the following changes made outside of Terraform since the last "terraform apply":

  # module.cloudtrail.module.cloudtrail_s3_bucket.module.s3_bucket.aws_s3_bucket.default[0] has been changed
  ~ resource "aws_s3_bucket" "default" {
        id                          = "REDACTED-prod-cloudtraillogs"
      ~ policy                      = jsonencode(
          ~ {
              ~ Statement = [
                    {
                        Action    = "s3:GetBucketAcl"
                        Effect    = "Allow"
                        Principal = {
                            Service = "cloudtrail.amazonaws.com"
                        }
                        Resource  = "arn:aws:s3:::REDACTED-prod-cloudtraillogs"
                        Sid       = "AWSCloudTrailAclCheck"
                    },
                  ~ {
                      ~ Principal = {
                          ~ Service = [
                              - "cloudtrail.amazonaws.com",
                                "config.amazonaws.com",
                              + "cloudtrail.amazonaws.com",
                            ]
                        }
                        # (5 unchanged elements hidden)
                    },
                  + {
                      + Action    = "s3:*"
                      + Condition = {
                          + Bool = {
                              + aws:SecureTransport = "false"
                            }
                        }
                      + Effect    = "Deny"
                      + Principal = "*"
                      + Resource  = [
                          + "arn:aws:s3:::REDACTED-prod-cloudtraillogs/*",
                          + "arn:aws:s3:::REDACTED-prod-cloudtraillogs",
                        ]
                      + Sid       = "ForceSSLOnlyAccess"
                    },
                ]
                # (1 unchanged element hidden)
            }
        )
        tags                        = {
            "Name"      = "REDACTED-prod-cloudtraillogs"
            "Namespace" = "REDACTED"
            "Stage"     = "prod"
        }
        # (10 unchanged attributes hidden)



        # (3 unchanged blocks hidden)
    }
  # module.cloudtrail.module.cloudtrail_s3_bucket.module.s3_bucket.aws_s3_bucket_policy.default[0] has been changed
  ~ resource "aws_s3_bucket_policy" "default" {
        id     = "REDACTED-prod-cloudtraillogs"
      ~ policy = jsonencode(
          ~ {
              ~ Statement = [
                    {
                        Action    = "s3:GetBucketAcl"
                        Effect    = "Allow"
                        Principal = {
                            Service = "cloudtrail.amazonaws.com"
                        }
                        Resource  = "arn:aws:s3:::REDACTED-prod-cloudtraillogs"
                        Sid       = "AWSCloudTrailAclCheck"
                    },
                  ~ {
                      ~ Condition = {
                          ~ StringEquals = {
                              ~ s3:x-amz-acl = [
                                  - "bucket-owner-full-control",
                                ] -> "bucket-owner-full-control"
                            }
                        }
                      ~ Principal = {
                          ~ Service = [
                              - "config.amazonaws.com",
                                "cloudtrail.amazonaws.com",
                              + "config.amazonaws.com",
                            ]
                        }
                        # (4 unchanged elements hidden)
                    },
                  ~ {
                      ~ Condition = {
                          ~ Bool = {
                              ~ aws:SecureTransport = [
                                  - "false",
                                ] -> "false"
                            }
                        }
                        # (5 unchanged elements hidden)
                    },
                ]
                # (1 unchanged element hidden)
            }
        )
        # (1 unchanged attribute hidden)
    }

The changes seem to be essentially no-ops but it would be great if they were consistent. It appears that amazon reformats inputted JSON and this collides with what is generated by terraform.

@booi booi added the bug 🐛 An issue with the system label Dec 14, 2021
@ivanmartos
Copy link

any update pls?

@leb4r
Copy link

leb4r commented Jan 14, 2022

Is this issue actually associated with https://github.com/cloudposse/terraform-aws-cloudtrail-s3-bucket instead ?

@enver-multibank
Copy link

enver-multibank commented Nov 16, 2022

Problem is indeed in https://github.com/cloudposse/terraform-aws-cloudtrail-s3-bucket. Latest version of this module uses s3-log-storage module version 0.26 (https://github.com/cloudposse/terraform-aws-cloudtrail-s3-bucket/blob/master/main.tf#L13) which is managing bucket policy in 2 different places:
https://github.com/cloudposse/terraform-aws-s3-log-storage/blob/0.26.0/main.tf#L8 and https://github.com/cloudposse/terraform-aws-s3-log-storage/blob/0.26.0/main.tf#L162

Solution is to upgrade version of terraform-aws-s3-log-storage to 0.27 (at least) in terraform-aws-cloudtrail-s3-bucket module. However 0.27 is preparation release for upgrade of aws provider to version 4, so it will require some manual state changes :(

Update: There is a pull request: cloudposse/terraform-aws-cloudtrail-s3-bucket#59 which seems to be stalled.

@Nuru Nuru added invalid This doesn't seem right and removed bug 🐛 An issue with the system labels Apr 15, 2024
@Nuru
Copy link
Contributor

Nuru commented Apr 15, 2024

This issue relates to terraform-aws-s3-log-storage, not this module, and has been fixed.

@Nuru Nuru closed this as completed Apr 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
invalid This doesn't seem right
Projects
None yet
Development

No branches or pull requests

5 participants