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

Add hash outputs of file content #142

Merged
merged 17 commits into from
Feb 9, 2023
Merged

Add hash outputs of file content #142

merged 17 commits into from
Feb 9, 2023

Conversation

zachwhaley
Copy link
Contributor

@zachwhaley zachwhaley commented Sep 9, 2022

Per #137 this brings the provider up to feature parity with all of Terraform's built-in file-reading functions

The following outputs are added to local_file, local_sensitive_file, data_local_file, and data_local_sensitive_file:

  • content_md5 - MD5 checksum of file content.
  • content_sha1 - SHA1 checksum of file content.
  • content_sha256 - SHA256 checksum of file content.
  • content_base64sha256 - Base64 encoded SHA256 checksum of file content.
  • content_sha512 - SHA512 checksum of file content.
  • content_base64sha512 - Base64 encoded SHA512 checksum of file content.

Closes #137

@zachwhaley zachwhaley changed the title Add more hash outputs of file content Add hash outputs of file content Sep 9, 2022
@github-actions github-actions bot added size/L and removed size/M labels Sep 9, 2022
@zachwhaley zachwhaley marked this pull request as ready for review September 9, 2022 18:40
@zachwhaley zachwhaley requested a review from a team as a code owner September 9, 2022 18:40
@bookshelfdave
Copy link
Contributor

(copied from hashicorp/terraform-provider-archive#165 (comment))

Hello @zachwhaley - my apologies for the delay in getting these reviewed. Our team synced up with the Terraform Core engineers and discussed this PR. As currently implemented, users will need to apply changes twice on updates before they are realized in downstream resources/outputs/etc.

This issue can be solved by using a CustomizeDiff, so the plan also sees the expected value changes, which in turn any references can also see those value changes at the same time.

However, both local and archive providers are scheduled to be migrated to the Terraform Plugin Framework this quarter, and we have a strong preference to implement this feature using attribute plan modifiers with the framework.

For now, we are going to hold off on bringing these changes in. Our plan is to reevaluate this once the migration to the plugin framework is complete.

@bendbennett
Copy link
Contributor

@zachwhaley the issue for tracking migration to the Terraform Plugin Framework is #84

@zachwhaley
Copy link
Contributor Author

@bendbennett I see that #84 has been closed 🎉

Is there another issue/PR to add the additional hash outputs that I can follow?

@bendbennett
Copy link
Contributor

@zachwhaley 👋

Would you be happy to update this PR to use the latest changes in main for the additional hash outputs?

@zachwhaley
Copy link
Contributor Author

@zachwhaley 👋

Would you be happy to update this PR to use the latest changes in main for the additional hash outputs?

I can take a look, but all of my changes have conflicts now so it may take me some time to work out the new way to do things. Y'all feel free to make the changes without me :)

@zachwhaley
Copy link
Contributor Author

zachwhaley commented Jan 13, 2023

@bendbennett I've pushed my first attempt at resolving the conflicts, although I'm not sure if I'm taking advantage of the (edit) attribute plan modifiers @bookshelfdave mentioned.

@bendbennett
Copy link
Contributor

bendbennett commented Jan 16, 2023

@zachwhaley thank you for updating the PR.

Could you run make generate to update the docs and commit the changes?

Will ask the team to review this shortly. Thanks again.

Copy link
Contributor

@SBGoods SBGoods left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution! I left a few suggestions about changing the description for the id attribute to add clarity. After you make those changes, run make generate and commit it to Git so that those changes are reflected in the documentation.

Can you also add similar changes that you made to the tests in data_source_local_file_test to the other resources/data sources?

internal/provider/data_source_local_file.go Outdated Show resolved Hide resolved
internal/provider/data_source_local_sensitive_file.go Outdated Show resolved Hide resolved
internal/provider/resource_local_sensitive_file.go Outdated Show resolved Hide resolved
internal/provider/resource_local_file.go Outdated Show resolved Hide resolved
@zachwhaley
Copy link
Contributor Author

@SBGoods Friendly poke :)

Copy link
Contributor

@SBGoods SBGoods left a comment

Choose a reason for hiding this comment

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

The implementation here looks good to me! Two things:

  1. Can you add acceptance tests for the local_sensitive_file data source and both resources?
  2. Can you add a Changelog entry for this change? Our CONTRIBUTING.md describes how to do this, but please let me know if the instructions are not clear. A potential changelog entry for this change can look like this under the FEATURES category:

provider: added support for MD5, SHA1, SHA256, and SHA512 checksum outputs.

As always, please reach out if there are any questions, Thank you!

@zachwhaley
Copy link
Contributor Author

Can you add acceptance tests for the local_sensitive_file data source and both resources?

Yep 👍🏻

Can you add a Changelog entry for this change? Our CONTRIBUTING.md describes how to do this, but please let me know if the instructions are not clear. A potential changelog entry for this change can look like this under the FEATURES category:

That seems like something y'all would want to do as opposed to external contributors? Would I put the entry under 2.3? Or add a new 2.4 release? Ultimately, I'd feel more comfortable if y'all updated that file :)

Copy link
Contributor

@SBGoods SBGoods left a comment

Choose a reason for hiding this comment

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

Looks good to me. Thanks again for the contribution!

Copy link
Contributor

@bendbennett bendbennett left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@SBGoods SBGoods merged commit 1c122b1 into hashicorp:main Feb 9, 2023
@zachwhaley zachwhaley deleted the shas branch February 9, 2023 23:32
@zachwhaley
Copy link
Contributor Author

Hi! @bendbennett Don't mean to rush y'all, but I was wondering how often this provider ships a release? I tried looking at the repo's documentation but didn't find anything.

@bookshelfdave
Copy link
Contributor

hi @zachwhaley - we don't have a defined cadence for releasing the utility providers. This release is queued behind a few other things, but it should be out in the next few weeks. My apologies for the delay!

@SBGoods
Copy link
Contributor

SBGoods commented Mar 8, 2023

@zachwhaley We just released v2.4.0 which includes this change. Thank you again for the contribution!

dduportal pushed a commit to jenkins-infra/azure that referenced this pull request Sep 23, 2023
<Actions>
<action
id="433396668ffbe69847f17a09a6cf8e8e490b1f5912247cfc582e78aa0c952904">
        <h3>Bump Terraform `local` provider version</h3>
<details
id="1d9343c012f5434ac9fe8a98135bae3667b399259be16d9b14302ea3bd424a24">
            <summary>Update Terraform lock file</summary>
<p>&#34;hashicorp/local&#34; updated from &#34;2.4.0&#34; to
&#34;2.4.0&#34; in file &#34;.terraform.lock.hcl&#34;</p>
            <details>
                <summary>2.4.0</summary>
<pre>Changelog retrieved
from:&#xA;&#x9;https://github.com/hashicorp/terraform-provider-local/releases/tag/v2.4.0&#xA;NOTES:&#xA;&#xA;*
This Go module has been updated to Go 1.19 per the [Go support
policy](https://golang.org/doc/devel/release.html#policy). Any consumers
building on earlier Go versions may experience errors.
([#184](https://github.com/hashicorp/terraform-provider-local/issues/184))&#xA;&#xA;FEATURES:&#xA;&#xA;*
resource/local_file: added support for `MD5`, `SHA1`, `SHA256`, and
`SHA512` checksum outputs.
([#142](hashicorp/terraform-provider-local#142
resource/local_sensitive_file: added support for `MD5`, `SHA1`,
`SHA256`, and `SHA512` checksum outputs.
([#142](hashicorp/terraform-provider-local#142
data-source/local_file: added support for `MD5`, `SHA1`, `SHA256`, and
`SHA512` checksum outputs.
([#142](hashicorp/terraform-provider-local#142
data-source/local_sensitive-file: added support for `MD5`, `SHA1`,
`SHA256`, and `SHA512` checksum outputs.
([#142](https://github.com/hashicorp/terraform-provider-local/issues/142))&#xA;&#xA;&#xA;</pre>
            </details>
        </details>
    </action>
</Actions>

---

<table>
  <tr>
    <td width="77">
<img src="https://www.updatecli.io/images/updatecli.png" alt="Updatecli
logo" width="50" height="50">
    </td>
    <td>
      <p>
Created automatically by <a
href="https://www.updatecli.io/">Updatecli</a>
      </p>
      <details><summary>Options:</summary>
        <br />
<p>Most of Updatecli configuration is done via <a
href="https://www.updatecli.io/docs/prologue/quick-start/">its
manifest(s)</a>.</p>
        <ul>
<li>If you close this pull request, Updatecli will automatically reopen
it, the next time it runs.</li>
<li>If you close this pull request and delete the base branch, Updatecli
will automatically recreate it, erasing all previous commits made.</li>
        </ul>
        <p>
Feel free to report any issues at <a
href="https://github.com/updatecli/updatecli/issues">github.com/updatecli/updatecli</a>.<br
/>
If you find this tool useful, do not hesitate to star <a
href="https://github.com/updatecli/updatecli/stargazers">our GitHub
repository</a> as a sign of appreciation, and/or to tell us directly on
our <a
href="https://matrix.to/#/#Updatecli_community:gitter.im">chat</a>!
        </p>
      </details>
    </td>
  </tr>
</table>

Co-authored-by: Jenkins Infra Bot (updatecli) <60776566+jenkins-infra-bot@users.noreply.github.com>
Copy link

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, 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 May 25, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature parity with all of Terraform's built-in file-reading functions
4 participants