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

[feat] Allow "LicenseDir" field for non-enterprise usage #214

Merged
merged 21 commits into from
May 29, 2024

Conversation

james0209
Copy link
Contributor

@james0209 james0209 commented May 23, 2024

What

High-level overview

Introduces LicenseDir field into the following:

  • build.GitRevision
  • checkpoint.LatestVersion
  • releases.ExactVersion
  • releases.LatestVersion
  • versions.InstallationOptions

Removes LicenseDir field from:

  • EnterpriseOptions

build.GitRevision

  • When copying the license file (with or without .txt) from the repo, keep the name as-is.
    • Currently, the license file created is always "LICENSE.txt".
  • Add destination path for the copied license file to pathsToRemove to match other uses.

checkpoint.LatestVersion

  • Pass LicenseDir to DownloadAndUnpack call
  • Currently an empty string is passed as checkpoint doesn't support Enterprise, and before this PR, it was assumed that only Enterprise versions would contain a License.

versions.InstallationOptions

  • Add LicenseDir to InstallationOptions - same struct that contains InstallDir.

Why

Closes #213

@james0209 james0209 marked this pull request as ready for review May 23, 2024 18:17
@james0209 james0209 requested a review from a team as a code owner May 23, 2024 18:17
@james0209 james0209 changed the title [WIP] Allow "LicenseDir" field for non-enterprise usage [feat] Allow "LicenseDir" field for non-enterprise usage May 24, 2024
Copy link
Member

@radeksimko radeksimko left a comment

Choose a reason for hiding this comment

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

This is looking pretty good aside from my inline comments!

Also thanks for handling the situation with enterprise versions. I did not realise we would have when writing the linked issue.

This does imply breaking change but it seems worth making, we just need to call it out in the Changelog (which I can do once it's merged).

build/git_revision.go Outdated Show resolved Hide resolved
build/git_revision_test.go Outdated Show resolved Hide resolved
build/git_revision_test.go Outdated Show resolved Hide resolved
build/git_revision_test.go Outdated Show resolved Hide resolved
installer_examples_test.go Outdated Show resolved Hide resolved
releases/exact_version.go Show resolved Hide resolved
internal/releasesjson/downloader.go Outdated Show resolved Hide resolved
@james0209 james0209 requested a review from radeksimko May 28, 2024 17:44
Copy link
Member

@radeksimko radeksimko left a comment

Choose a reason for hiding this comment

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

Mostly minor concerns remaining as per my comments inline. Once those are addressed I'm more than happy to merge this.

build/git_revision.go Outdated Show resolved Hide resolved
build/git_revision_test.go Outdated Show resolved Hide resolved
build/git_revision_test.go Outdated Show resolved Hide resolved
build/git_revision.go Outdated Show resolved Hide resolved
checkpoint/latest_version.go Outdated Show resolved Hide resolved
releases/exact_version.go Outdated Show resolved Hide resolved
releases/latest_version.go Outdated Show resolved Hide resolved
@radeksimko radeksimko merged commit e082106 into hashicorp:main May 29, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Introduce LicenseDir for non-enterprise installations (build, checkpoint, releases)
2 participants