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

Split server_side_encryption for s3 bucket into its own resource #125

Merged
merged 1 commit into from
Nov 11, 2022

Conversation

gregeinfrank
Copy link
Contributor

@gregeinfrank gregeinfrank commented Nov 11, 2022

Discussion about this here but tldr is that in later versions of the AWS provider, individual configurations on S3 buckets are split out into their own Terraform resources. This was backported to v3.75.2 (which is what we're currently using) to prep for the breaking change.

I already imported into Asana's remote state so this is a no-op

Pull Request synchronized with Asana task

@gregeinfrank
Copy link
Contributor Author

@skeggse OK if I assign this to you as Terraform AOR? Going to merge though since I already terraform imported the new resources into our state

@gregeinfrank gregeinfrank merged commit 17feed6 into master Nov 11, 2022
@gregeinfrank gregeinfrank deleted the gregeinfrank-split-server-side-encryption branch November 11, 2022 23:08
Copy link
Member

@skeggse skeggse left a comment

Choose a reason for hiding this comment

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

lgtm

I'd love to see us upgrade to v4+ on the aws provider for consistency with our other Terraform code!

Comment on lines +355 to 363
resource "aws_s3_bucket_server_side_encryption_configuration" "api_key_bucket_server_side_encryption_configuration" {
bucket = aws_s3_bucket.api_key_bucket.bucket
rule {
apply_server_side_encryption_by_default {
kms_master_key_id = aws_kms_key.api_encryption_key.arn
sse_algorithm = "aws:kms"
}
}
}
}
Copy link
Member

Choose a reason for hiding this comment

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

tf fmt might be good for this file

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

Successfully merging this pull request may close these issues.

2 participants