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

r/aws_s3_bucket_object: etag can’t be used with any server encryption, but docs mention a different thing #5033

Closed
iamakulov opened this issue Jun 29, 2018 · 8 comments · Fixed by #9442
Labels
bug Addresses a defect in current functionality. documentation Introduces or discusses updates to documentation. service/s3 Issues and PRs that pertain to the s3 service.
Milestone

Comments

@iamakulov
Copy link

iamakulov commented Jun 29, 2018

Currently, docs state that etag in aws_s3_bucket_object can’t be used only with KMS encryption:

etag - (Optional) Used to trigger updates. The only meaningful value is ${md5(file("path/to/file"))}. This attribute is not compatible with KMS encryption, kms_key_id or server_side_encryption = "aws:kms".

However, when I try to use it with AES256 encryption, I receive the

Error: module.chorus-backend.aws_s3_bucket_object.env-file: "etag": conflicts with server_side_encryption

error.

#2244 brought this issue about docs. #4928 closed it – however, the docs still talk only about KMS encryption. Looks like it’s either the issue in docs again (Terraform is right, docs are wrong) or in the Terraform itself (docs list correct expectations, but Terraform implements them wrong).

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • Please do not leave "+1" or "me too" comments, they generate extra noise for issue followers and do not help prioritize the request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment

Terraform Version

v0.11.7

Affected Resource(s)

  • aws_s3_bucket_object

Terraform Configuration Files

resource "aws_s3_bucket_object" "env-file" {
  key                    = "${var.env-name}/.env"
  bucket                 = "${var.chorus-configs-bucket-name}"
  content                = "${data.template_file.env-file.rendered}"
  server_side_encryption = "AES256"
  etag                   = "${md5("${data.template_file.env-file.rendered}")}"
}

data "template_file" "env-file" {
  template = "${file("${path.module}/.env")}"

  vars {
    app-domain = "${var.app-domain}"
    aws-region = "${var.aws-region}"
  }
}

Debug Output

https://gist.github.com/iamakulov/7525920a9ebeaa0ca5af2b465a4be878

Steps to Reproduce

  1. terraform apply

Important Factoids

I’m using Windows.

References

@bflad bflad added bug Addresses a defect in current functionality. documentation Introduces or discusses updates to documentation. service/s3 Issues and PRs that pertain to the s3 service. labels Jun 29, 2018
@marek-obuchowicz
Copy link

Same issue happens on mac os x

@nwalke
Copy link
Contributor

nwalke commented Sep 20, 2018

If you look at the AWS docs, I think the Terraform docs are correct about what could happen (more on that in a bit).
https://docs.aws.amazon.com/AmazonS3/latest/API/RESTObjectPUT.html#RESTObjectPUT-requests
Look for the section titled "Server-Side-Encryption-Specific Request Headers". Under "Use customer-provided encryption keys" there's a note saying:

If you use this feature, the ETag value that Amazon S3 returns in the response is not the MD5 of the object.

There is no such note under the section talking about using an AWS managed key.

I think this is a limitation of Terraform. If we look at how etag is defined, it becomes obvious what's going on:

"etag": {
	Type: schema.TypeString,
	// This will conflict with SSE-C and SSE-KMS encryption and multi-part upload
	// if/when it's actually implemented. The Etag then won't match raw-file MD5.
	// See http://docs.aws.amazon.com/AmazonS3/latest/API/RESTCommonResponseHeaders.html
	Optional:      true,
	Computed:      true,
	ConflictsWith: []string{"kms_key_id", "server_side_encryption"},
},

Notice how the author was unable to specify any arguments under server_side_encryption, they had to just say the entire thing conflicts.

The documentation should probably be updated to reflect this limitation.

nwalke added a commit to nwalke/terraform-provider-aws that referenced this issue Sep 20, 2018
Update doc to explain etag inconsistency
@frittentheke
Copy link

frittentheke commented Oct 9, 2018

While this is not an immediate response to the actual issue ... how about moving away from having the user provide the file md5 hash as an etag value at all to recognize that the file has changed and use an arbitrary checksum of the local file and store it in the terraform state after a successful upload? This way doing the checksum on the source and comparing it to the terraform state should be enough to recognize a changed file and the requirement to update the S3 object. Using the etag is unsafe in multiple cases - https://docs.aws.amazon.com/AmazonS3/latest/API/RESTCommonResponseHeaders.html

Or am I missing anything / a certain use case there?

@eedwards-sk
Copy link

any updates on this? @frittentheke makes a great suggestion

I'm currently bit by needing server side encryption but then being unable to have terraform manage the object due to inability do recognize changes.

@frittentheke
Copy link

@nwalke @marek-obuchowicz @eedwards-sk @akkessler @mikemac42 as you see referenced, I opened a new issue just about the independently done hash approach of tracking file changes - see #6668

@bflad
Copy link
Contributor

bflad commented Aug 20, 2019

Support for specifying the aws_s3_bucket_object resource etag argument with server_side_encryption (only supporting SSE-S3) has been merged and will release with version 2.25.0 of the Terraform AWS Provider later this week. The resource will currently not provide any error if you attempt to use other forms of SSE with etag, but that can be implemented in a separate change request.

The feature request for a separate hash/checksum argument in #6668 is certainly still valid to cover the usage of other forms of SSE. Please follow that issue for updates there. Thanks.

@ghost
Copy link

ghost commented Aug 23, 2019

This has been released in version 2.25.0 of the Terraform AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template for triage. Thanks!

@ghost
Copy link

ghost commented Nov 1, 2019

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!

@ghost ghost locked and limited conversation to collaborators Nov 1, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Addresses a defect in current functionality. documentation Introduces or discusses updates to documentation. service/s3 Issues and PRs that pertain to the s3 service.
Projects
None yet
6 participants