fix: Fixed Bucket Policy chain dependency with Public Access Block

[Jean717p]

Description

Following AWS update of April 2023 for S3 Bucket Public Access Block default values for resources created via api calls, buckets are now created with block all public access enabled by default.

Motivation and Context

If a bucket is created with a public read policy, it will incur in 403 Access Denied caused by block all public access enabled at bucket creation, with the previous dependency chain the bucket policy gets created before the block all public access block.

Example of public read bucket policy:

{
    "Version": "2012-10-17",
    "Statement": [
        {
            "Sid": "ReadBucketPolicy",
            "Effect": "Allow",
            "Principal": "*",
            "Action": "s3:GetObject",
            "Resource": "arn:aws:s3:::bucket_name/*"
        }
    ]
}

Inverting the dependency chain between aws_s3_bucket_policy.this and aws_s3_bucket_public_access_block.this will solve this issue.

Example to reproduce the issue

# https://github.com/terraform-aws-modules/terraform-aws-s3-bucket
module "s3-dd6271fee6c7c15a5dfa2041aef5f9e6de89b5ce" {
  source = "terraform-aws-modules/s3-bucket/aws"

  bucket                               = "dd6271fee6c7c15a5dfa2041aef5f9e6de89b5ce-pr227"
  attach_policy                        = true
  attach_require_latest_tls_policy     = true
  block_public_acls                    = true
  block_public_policy                  = false # Not updated before bucket policy due to current chain dependency
  restrict_public_buckets              = false # Not updated before bucket policy due to current chain dependency
  force_destroy                        = true
  ignore_public_acls                   = true
  policy                               = jsonencode({
    "Version": "2012-10-17",
    "Statement": [
      {
        "Sid": "ReadBucketPolicy",
        "Effect": "Allow",
        "Principal": "*",
        "Action": "s3:GetObject",
        "Resource": "arn:aws:s3::dd6271fee6c7c15a5dfa2041aef5f9e6de89b5ce-pr227/*"
      }
    ]
  })
}

Obtained error

module.s3-dd6271fee6c7c15a5dfa2041aef5f9e6de89b5ce.aws_s3_bucket.this[0]: Creating...
module.s3-dd6271fee6c7c15a5dfa2041aef5f9e6de89b5ce.aws_s3_bucket.this[0]: Creation complete after 3s [id=dd6271fee6c7c15a5dfa2041aef5f9e6de89b5ce-pr227]
module.s3-dd6271fee6c7c15a5dfa2041aef5f9e6de89b5ce.data.aws_iam_policy_document.require_latest_tls[0]: Reading...
module.s3-dd6271fee6c7c15a5dfa2041aef5f9e6de89b5ce.data.aws_iam_policy_document.require_latest_tls[0]: Read complete after 0s [id=3386508698]
module.s3-dd6271fee6c7c15a5dfa2041aef5f9e6de89b5ce.data.aws_iam_policy_document.combined[0]: Reading...
module.s3-dd6271fee6c7c15a5dfa2041aef5f9e6de89b5ce.data.aws_iam_policy_document.combined[0]: Read complete after 0s [id=3075998097]
module.s3-dd6271fee6c7c15a5dfa2041aef5f9e6de89b5ce.aws_s3_bucket_policy.this[0]: Creating...
╷
│ Error: Error putting S3 policy: AccessDenied: Access Denied
│       status code: 403, request id: <REDACTED>, host id: <REDACTED>
│
│   with module.s3-dd6271fee6c7c15a5dfa2041aef5f9e6de89b5ce.aws_s3_bucket_policy.this[0],
│   on .terraform/modules/s3-dd6271fee6c7c15a5dfa2041aef5f9e6de89b5ce/main.tf line 512, in resource "aws_s3_bucket_policy" "this":
│  512: resource "aws_s3_bucket_policy" "this" {
│
╵

Breaking Changes

How Has This Been Tested?

• I have provided at least one example in the PR to demonstrate and validate my change(s)
• I have tested and validated these changes using one or more of the provided examples/* projects

• I have tested and validated these changes in aws cloud

• I have executed pre-commit run -a on my pull request

[stefan-matic]

+1

[matheusmazzoni]

Hello Guys, when will it be merged?

[antonbabenko]

Please provide the complete code snippet which reproduces the bug so that I can run it locally and experience the problem.

[Jean717p]

Please provide the complete code snippet which reproduces the bug so that I can run it locally and experience the problem.

@antonbabenko
PR updated with example and error.
Best

[antonbabenko]

Thank you for the PR, @Jean717p !

[antonbabenko]

This PR is included in version 3.10.1 🎉

[martinfarmer]

FYI - With this version, I now see a cycle error

│ Error: Cycle: module.test-basic-vpc.module.dns-query-logs-s3-bucket.module.s3_bucket.aws_s3_bucket_public_access_block.this[0], module.test-basic-vpc.module.dns-query-logs-s3-bucket.module.s3_bucket.aws_s3_bucket_policy.this[0]

it appears that the aws_s3_bucket_public_access_block and the s3_bucket.aws_s3_bucket_policy have a cyclic dependency

[Dr0p42]

Thank you @Jean717p 👏

[github-actions]

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.