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: Use own hash to track object changes (move away from the etag md5 value) #6668

Closed
frittentheke opened this issue Nov 30, 2018 · 10 comments · Fixed by #11522
Assignees
Labels
proposal Proposes new design or functionality. service/s3 Issues and PRs that pertain to the s3 service.
Milestone

Comments

@frittentheke
Copy link

frittentheke commented Nov 30, 2018

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

Description

While discussing an issue with the etag that s3 objects receive from AWS
(#5033) and how that is not working when using encryption I came to wonder why the etag value of an S3 object is so naturally used to track local changes of a file compared to the object on S3.

Using the etag is unsafe in multiple cases - https://docs.aws.amazon.com/AmazonS3/latest/API/RESTCommonResponseHeaders.html !

My proposition is to move away from having the user provide the file md5 hash as an
etag value at all to recognize that the file has changed and instead to use a (md5 or rather sha256) hash of the local file and to store that 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.

Or am I missing anything / a certain use case of the etag to recognize file changes here?
Certainly the etag as a value provided by Amazon S3 allows to determine if the file has changed without terraform somehow, but that is not really an issue, as resources managed by terraform and their state must no be manipulated independently anyways. And certainly a refresh of the hash is always possible by simply downloading the file.

New or Affected Resource(s)

  • aws_s3_bucket_object

Potential Terraform Configuration

Adding a hash / changing the way a changed file is recognized should be transparent to the existing code. The referenced issue about incomplete documentation of the etag when using service side encryption is still valid though.

References

@bflad bflad added the service/s3 Issues and PRs that pertain to the s3 service. label Dec 1, 2018
@aeschright aeschright added needs-triage Waiting for first response or review from a maintainer. enhancement Requests to existing resources that expand the functionality or scope. and removed needs-triage Waiting for first response or review from a maintainer. labels Jun 24, 2019
@andrewfraley
Copy link

What is the current best workaround? I'm still using content_base64 as a workaround, which has the drawback of my files' content being base64 encoded into my state files.

@ztane
Copy link

ztane commented Sep 17, 2019

I got headache trying to find any other possible workaround for this... but couldn't think of any that wouldn't require broken state in between.(i.e. delete object before running terraform plan)... Please fix this... it would be sufficient if there were any way of triggering recreation with any attribute value but it seems that there is none.

@theophilechevalier
Copy link
Contributor

theophilechevalier commented Jan 7, 2020

Hi, I got stuck in this issue and can't use etag because of aws:kms SSE.
I'll propose an implementation based on @frittentheke 's idea of adding a hash of the local file to the state soon.

@Hermain
Copy link

Hermain commented Jun 17, 2020

Is there any other workaround than content_base64? I have glue code in s3 encrypted with kms and really don't want to have all that code in the terraform resource.

@pasalkarsachin1
Copy link
Contributor

I have a case where we download the jars from the artifactory & it gets uploaded to S3 bucket. When I deploy this twice, the first time it uploads the jar & second time when I update the jar version in URL it downloads the jars but fails to upload the object as etag evaluated as same

provider "aws" {
  region = "us-east-1"
}

locals {
  downLoadFolder = "${path.module}/tf-download/"
  jarLocation    = "${local.downLoadFolder}${var.s3_filename}"
  s3_key         = "${var.s3_prefix}${var.s3_filename}"
}

resource "null_resource" "download_object" {
  triggers {
    redo = var.object_version
  }

  provisioner "local-exec" {
    command = "mkdir -p ${local.downLoadFolder} && curl -o ${local.jarLocation} ${var.url}"
  }
}

resource "aws_s3_bucket_object" "put_object" {
  key                    = local.s3_key
  bucket                 = var.bucket
  source                 = local.jarLocation
  server_side_encryption = "AES256"
  depends_on = [null_resource.download_object]

  tags {
    version = var.object_version
  }
}

variable "object_version" {
  type        = "string"
  description = "Version of the object to put in S3 bucket.  New versions trigger uploads"
}

variable "s3_prefix" {
  type        = "string"
  description = "Path for the object"
}

variable "s3_filename" {
  type        = "string"
  description = "Last part of S3 key (i.e. filename).  Used for also local download"
}

variable "url" {
  type        = "string"
  description = "What to download"
}

variable "bucket" {
  type        = "string"
  description = "Bucket for the object"
}

@breathingdust breathingdust added proposal Proposes new design or functionality. and removed enhancement Requests to existing resources that expand the functionality or scope. labels Oct 27, 2020
@T00mm
Copy link

T00mm commented Nov 20, 2020

Same error, files are too big for base64 so need to taint them manually. Any progress on this issue?

@frittentheke
Copy link
Author

@T00mm there is a PR pending ... #11522

@jbdelavoix
Copy link
Contributor

jbdelavoix commented Dec 9, 2020

Waiting for PR #11522, a functional workaround is to use metadata:

  metadata = {
    sha256 = filesha256(local.jarLocation)
  }

@github-actions
Copy link

This functionality has been released in v3.50.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. Thank you!

@github-actions
Copy link

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 have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
proposal Proposes new design or functionality. service/s3 Issues and PRs that pertain to the s3 service.
Projects
None yet
Development

Successfully merging a pull request may close this issue.