legal: Include license in release zip, Docker image and linux packages by xiehan · Pull Request #34977 · hashicorp/terraform

xiehan · 2024-04-11T09:35:35Z

We have a new requirement from Legal to include a copy of the license with all release .zip files, Docker images, and linux packages. This PR is based on guidance from this doc as well as the work the Nomad team did over in hashicorp/nomad#20345.

Target Release

1.8.2

Draft CHANGELOG entry

UPGRADE NOTES

Starting with this release, we are including a copy of our license file in all packaged versions of our releases, such as the release .zip files. If you are consuming these files directly and would prefer to extract the one terraform file instead of extracting everything, you need to add an extra argument specifying the file to extract, like this:

unzip terraform_1.8.2_linux_amd64.zip terraform

xiehan · 2024-04-11T09:38:24Z

.github/workflows/build-Dockerfile

@@ -29,8 +29,23 @@ LABEL version=$PRODUCT_VERSION
 # Historical Terraform-specific label preserved for backward compatibility.
 LABEL "com.hashicorp.terraform.version"="${PRODUCT_VERSION}"



Question for RelEng: Is this the actual Dockerfile that is in use today? Because there is also https://github.com/hashicorp/terraform-releases/blob/master/build-support/docker-release/Dockerfile-release but that has been updated a lot less recently (last update was 4 years ago) so my assumption is that is no longer used.

Yes afaik the one in terraform-releases is a legacy one from before y'all onboarded to our Common Release Tooling - you have the correct one here (build-Dockerfile)

xiehan · 2024-04-11T09:39:48Z

.github/workflows/build-terraform-cli.yml

          ACTIONSOS: ${{ inputs.runson }}
          CGO_ENABLED: ${{ inputs.cgo-enabled }}
-        uses: hashicorp/[email protected].7
+        uses: hashicorp/actions-go-build@e20c6be7bf010e40e930dab20e6da63176725ec1 # v0.1.9


I noticed that we were a couple of versions behind of this action so it seemed like a good idea to upgrade, but I do worry a little whether I'm introducing more change in this PR than I need to.

this should be totally safe to do - other product repos are using 0.1.9 successfully

I agree upgrading should be fine, but why is this a specific commit sha and not @v0.1.9?

Using SHAs is generally considered a best practice for security -- while in this case it's perhaps less of an issue since HashiCorp owns actions-go-build, it's still technically an open-source project and so it's a good idea to reference an immutable release as described here. I've also generally found it easiest to default to using purely SHAs these days.

xiehan · 2024-04-11T09:45:09Z

.github/workflows/build-terraform-cli.yml

+          zip_name: ${{ env.ARTIFACT_BASENAME }}
          instructions: |-
-            mkdir dist out
-            set -x
-            go build -ldflags "${{ inputs.ld-flags }}" -o dist/ .
-            zip -r -j out/${{ env.ARTIFACT_BASENAME }} dist/
+            go build -ldflags "${{ inputs.ld-flags }}" -o "$BIN_PATH" -trimpath -buildvcs=false
+            cp LICENSE "$TARGET_DIR/LICENSE.txt"


This is the part that I'm least confident about; as I was going through https://github.com/hashicorp/actions-go-build/blob/main/README.md I realized our implementation is wildly different from all the examples there. Notably, we currently run the zip command manually while the action seems to be intended to abstract that away. It seems like a good idea to me to be more consistent with how the action is intended to be used, but I worry a little that there might be unintended side effects.

Specific questions:

Is it okay to remove the set -x in our instructions? Can I safely assume that the action is taking care of that?

We don't currently use the -trimpath -buildvcs=false flags but all of the examples use them, should we be using them as well?

Where does the zip file actually get created as part of this? (See also question on line 69 below.)

Is it okay to remove the set -x in our instructions? Can I safely assume that the action is taking care of that?

Yes this should be fine as set -x enables a mode of the shell where all executed commands are printed to the terminal for debugging purposes. If you specifically want to leave this in then that's also not a problem!

We don't currently use the -trimpath -buildvcs=false flags but all of the examples use them, should we be using them as well?

You don't have to, this is used to help ensure that builds are reproducible (see https://github.com/hashicorp/actions-go-build?tab=readme-ov-file#build-path and https://github.com/hashicorp/actions-go-build?tab=readme-ov-file#vcs-information

Where does the zip file actually get created as part of this? (See also question on line 69 below.)

the Zip file will automatically get uploaded to the workflow run so you shouldn't need to do this in a separate step

xiehan · 2024-04-11T09:56:04Z

.github/workflows/build-terraform-cli.yml

        with:
          name: ${{ env.ARTIFACT_BASENAME }}
-          path: out/${{ env.ARTIFACT_BASENAME }}
+          path: ${{ env.ARTIFACT_BASENAME }}


Related to the above, if we're no longer manually creating the out dir, where does the zip file get created? One thing that surprised me as I was navigating the actions-go-build docs is that I couldn't find a single example of any workflows that do something with the created zip file in subsequent steps, which seems like it'd be a common use case.

xiehan · 2024-04-11T12:05:35Z

.github/workflows/build-terraform-cli.yml

-      - name: Determine artifact basename
-        run: echo "ARTIFACT_BASENAME=${{ inputs.package-name }}_${{ inputs.product-version }}_${{ inputs.goos }}_${{ inputs.goarch }}.zip" >> $GITHUB_ENV


@sarahethompson One more question (tysm for your help!) -- based on what you said about actions-go-build taking care of uploading the zip, from what I can tell in that case, it should no longer be necessary for us to determine the artifact name here and so I removed this step as well, but please let me know if you think this will cause any issues!

Yes as far as I can tell you are totally right here!

apparentlymart · 2024-04-12T01:00:09Z

Regarding the changelog:

I'd bet that at least some Terraform CLI users have automation that fetches the zip archives and extracts them directly into a bin directory, since until now there's never been a Terraform release zip file containing anything other than executables, and historically (a long time ago now) the releases consisted of multiple executables and so extracting just the terraform executable would not have been sufficient for a working installation.

While extracting a non-executable LICENSE file into such a directory would likely not be a huge problem -- it's presumably just clutter -- this probably is worth at least an upgrade note in the changelog for the first release that does it, so that teams that care about it might change their automation to just extract the one terraform file from the zip, instead of extracting everything.

For those who are using the typical unzip command I suppose that would just be a matter of adding an extra argument specifying the file to extract, like this:

unzip terraform_1.8.1_linux_amd64.zip terraform

apparentlymart · 2024-04-12T01:17:38Z

.github/workflows/build-terraform-cli.yml

+        env:
+          LICENSE_DIR: ".release/linux/package/usr/share/doc/${{ inputs.package-name }}"
+        run: |
+          mkdir -p "$LICENSE_DIR" && cp LICENSE "$LICENSE_DIR/LICENSE.txt"


For Debian-based distributions in particular there's a convention for this directory to contain a file named copyright which follows a machine-readable format.

While of course we aren't compelled to follow that convention since we aren't intending these packages to go into the main Debian archive, following the distribution conventions might make it easier for folks to use existing tools to catalog the licenses of software they have installed, to aid with verifying license compliance.

As far as I know the specified format for these files is a relatively new thing and there are various existing official Debian packages that instead just place the license text verbatim into the file without any special formatting, and so a pragmatic compromise might be to just change the name of the file from LICENSE.txt to copyright but leave its contents otherwise unchanged, rather than trying to maintain or generate a separate file that uses the Debian-specific file format. 🤔

(I expect there's probably an established convention for RPM-based distributions too, but I'm not familiar with that ecosystem.)

FYI I've flagged this for the RelEng team to consider and they'll be looking into this suggestion this week.

RelEng has decided they'd like to stick with the original (current) approach, keeping the licenses as .txt files.

…erted" This reverts commit d8a17fc.

…rted)" This reverts commit 178861d.

kmoe · 2024-04-23T14:34:52Z

Approved following successful manual test

github-actions · 2024-04-23T14:36:35Z

Reminder for the merging maintainer: if this is a user-visible change, please update the changelog on the appropriate release branch.

github-actions · 2024-05-25T02:04:25Z

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.

xiehan added 3 commits April 10, 2024 15:57

legal: Add license file and metadata to published Docker image

a132ff9

legal: update the path to license in Dockerfile

289bc3b

legal: Copy license into build zip and linux packages

66641d3

xiehan commented Apr 11, 2024

View reviewed changes

build: Remove unnecessary steps

95e0e93

xiehan commented Apr 11, 2024

View reviewed changes

xiehan marked this pull request as ready for review April 11, 2024 12:55

apparentlymart reviewed Apr 12, 2024

View reviewed changes

kmoe added the 1.8-backport If you add this label to a PR before merging, backport-assistant will open a new PR once merged label Apr 16, 2024

xiehan and others added 4 commits April 23, 2024 15:14

legal: Test building the license change on branch (to be reverted)

178861d

legal: Test releasing the license change on branch, to be reverted

d8a17fc

Revert "legal: Test releasing the license change on branch, to be rev…

ed5ea2e

…erted" This reverts commit d8a17fc.

Revert "legal: Test building the license change on branch (to be reve…

d57dc2e

…rted)" This reverts commit 178861d.

kmoe approved these changes Apr 23, 2024

View reviewed changes

xiehan merged commit d0fcb21 into main Apr 23, 2024

xiehan deleted the xiehan-patch-1 branch April 23, 2024 14:36

ghost mentioned this pull request Apr 23, 2024

Backport of legal: Include license in release zip, Docker image and linux packages into v1.8 #35064

Merged

radeksimko mentioned this pull request May 7, 2024

Ensure license file gets packaged along w/ the CLI binary hashicorp/hc-install#205

Merged

github-actions bot locked as resolved and limited conversation to collaborators May 25, 2024

		@@ -29,8 +29,23 @@ LABEL version=$PRODUCT_VERSION
		# Historical Terraform-specific label preserved for backward compatibility.
		LABEL "com.hashicorp.terraform.version"="${PRODUCT_VERSION}"

		- name: Determine artifact basename
		run: echo "ARTIFACT_BASENAME=${{ inputs.package-name }}_${{ inputs.product-version }}_${{ inputs.goos }}_${{ inputs.goarch }}.zip" >> $GITHUB_ENV

Conversation

xiehan commented Apr 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Target Release

Draft CHANGELOG entry

UPGRADE NOTES

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sarahethompson Apr 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

apparentlymart commented Apr 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kmoe commented Apr 23, 2024

Uh oh!

github-actions bot commented Apr 23, 2024

Uh oh!

github-actions bot commented May 25, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

xiehan commented Apr 11, 2024 •

edited

Loading

sarahethompson Apr 11, 2024 •

edited

Loading

apparentlymart commented Apr 12, 2024 •

edited

Loading