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

Removing policy attribute for S3 bucket #86

Merged
merged 3 commits into from
Apr 27, 2021

Conversation

justnom
Copy link
Contributor

@justnom justnom commented Apr 26, 2021

what

  • Fixing a bug where the bucket policy would flip-flop on Terraform apply if var.policy and any of var.allow_ssl_requests_only, var.allow_encrypted_uploads_only were set

why

  • The aws_s3_bucket.policy attribute was competing with the aws_s3_bucket_policy resource

Fixing a bug where the bucket policy would flip-flop on Terraform apply if `var.policy` and any of `var.allow_ssl_requests_only`, `var.allow_encrypted_uploads_only` were set.
@justnom justnom requested review from a team as code owners April 26, 2021 16:48
@justnom justnom requested review from Gowiem and nitrocode April 26, 2021 16:48
Copy link

@bridgecrew bridgecrew bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bridgecrew has found 1 infrastructure configuration error in this PR ⬇️

@@ -10,7 +10,6 @@ resource "aws_s3_bucket" "default" {
bucket = local.bucket_name
acl = try(length(var.grants), 0) == 0 ? var.acl : null
force_destroy = var.force_destroy
policy = var.policy
tags = module.this.tags
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Error Description: Ensure all data stored in the S3 bucket is securely encrypted at rest
Category: Storage | Severity: HIGH
Resource: aws_s3_bucket [default], lines: 5 - 196

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bridcrew does not understand dynamics with enable variables

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please add : BC_AWS_S3_14 to the bridgecrew comments on the top of the file

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done! Thanks @jamengual

jamengual
jamengual previously approved these changes Apr 26, 2021
@jamengual
Copy link
Contributor

/test all

@justnom
Copy link
Contributor Author

justnom commented Apr 26, 2021

/test all

1 similar comment
@joe-niland
Copy link
Member

/test all

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants