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

New File Hashing Errors In Terraform 0.12 #20098

Closed
bflad opened this issue Jan 24, 2019 · 2 comments · Fixed by #20115
Closed

New File Hashing Errors In Terraform 0.12 #20098

bflad opened this issue Jan 24, 2019 · 2 comments · Fixed by #20115
Assignees
Labels
Milestone

Comments

@bflad
Copy link
Contributor

bflad commented Jan 24, 2019

Terraform Version

terraform 0.12-alpha4

Terraform Configuration Files

# https://www.terraform.io/docs/providers/aws/r/lambda_function.html
resource "aws_lambda_function" "example" {
  # ... other configuration ...

  filename         = "somefile.zip"
  source_code_hash = "${base64sha256(file("somefile.zip"))}"
}

# https://www.terraform.io/docs/providers/aws/r/s3_bucket_object.html
resource "aws_s3_bucket_object" "example" {
  # ... other configuration ...

  source = "somefile.zip"
  etag   = "${md5(file("somefile.zip"))}"
}

Expected Behavior

No differences between upgrading Terraform 0.11 and Terraform 0.12 or the breaking changes are documented in the Terraform 0.12 upgrade guide with a recommended alternative to achieve the equivalent file hash value. If new file hashing functions are introduced, it would be nice if this was handled automatically by terraform 0.12upgrade or if these new functions could be backported to Terraform 0.11 to lower the Terraform 0.12 barrier to entry.

Actual Behavior

When the files have binary content:

config is invalid: Error in function call: Call to function "file" failed: contents of somefile.zip are not valid UTF-8; to read arbitrary bytes, use the filebase64 function instead.

Using filebase64() would not generate an equivalent file hash.

References

@bflad bflad added the config label Jan 24, 2019
@bflad bflad added this to the v0.12.0 milestone Jan 24, 2019
bflad added a commit to hashicorp/terraform-provider-aws that referenced this issue Jan 24, 2019
These changes are backwards compatible with Terraform 0.11. The remaining configuration error requires this upstream issue: hashicorp/terraform#20098

Previous output from Terraform 0.12 acceptance testing:

```
--- FAIL: TestAccAWSSNSTopicSubscription_autoConfirmingEndpoint (0.44s)
    testing.go:568: Step 0 error: /opt/teamcity-agent/temp/buildTmp/tf-test250105636/main.tf:25,5-6: Invalid argument name; Argument names must not be quoted.

--- FAIL: TestAccAWSSNSTopicSubscription_autoConfirmingSecuredEndpoint (0.60s)
    testing.go:568: Step 0 error: /opt/teamcity-agent/temp/buildTmp/tf-test092280804/main.tf:26,5-6: Invalid argument name; Argument names must not be quoted.
```

Output from Terraform 0.12 acceptance testing (additional fixes required upstream in Terraform Provider SDK):

```
--- FAIL: TestAccAWSSNSTopicSubscription_autoConfirmingEndpoint (1.70s)
    testing.go:568: Step 0 error: config is invalid: Error in function call: Call to function "file" failed: contents of test-fixtures/lambda_confirm_sns.zip are not valid UTF-8; to read arbitrary bytes, use the filebase64 function instead.
--- FAIL: TestAccAWSSNSTopicSubscription_autoConfirmingSecuredEndpoint (1.70s)
    testing.go:568: Step 0 error: config is invalid: 2 problems:

        - Error in function call: Call to function "file" failed: contents of test-fixtures/lambda_confirm_sns.zip are not valid UTF-8; to read arbitrary bytes, use the filebase64 function instead.
        - Error in function call: Call to function "file" failed: contents of test-fixtures/lambda_basic_authorizer.zip are not valid UTF-8; to read arbitrary bytes, use the filebase64 function instead.
```
@apparentlymart apparentlymart self-assigned this Jan 25, 2019
@apparentlymart
Copy link
Contributor

After weighing a few different options here, I decided to add some specialized functions for hashing the contents of files, in #20115. That PR also includes a new rule for the 0.12upgrade tool which should fix use of the documented idiom in existing configurations:

base64sha256(file("somefile.zip")) -> filebase64sha256("somefile.zip")

(I wrote a little about the other options I considered in the PR description, if you're interested.)

This of course doesn't fully address the problem because the existing examples in the provider docs (and, I assume, also in some provider tests) will need to be updated to reflect the new pattern.

I'm sensitive to the fact that this makes it awkward to write documentation that works for both 0.12 and 0.11. In principle we could backport these functions into a 0.11 release, but since functions work very differently in prior versions that would amount to repeating all of the work from that PR again in a different package and against a different API, so I'd like to first see if we can get away with a documentation-only solution where we update the examples to show the new usage but then put a ~> callout under the example noting that versions prior to 0.12 require a different approach.

(Part of my rationale for that is that even if we did backport, we'd still need that callout explaining that versions of Terraform prior to 0.11.x need a different usage -- since not everyone will be on the latest minor -- and so it's perhaps simpler just to make the crease for that change be on the major release which is hopefully easier for users to remember.)

@ghost
Copy link

ghost commented Mar 30, 2020

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.

@ghost ghost locked and limited conversation to collaborators Mar 30, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants