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

refactor: use hc-install for TF downloads + constraints #4494

Open
wants to merge 26 commits into
base: main
Choose a base branch
from

Conversation

james0209
Copy link

@james0209 james0209 commented Apr 30, 2024

What

Replace go-getter usage with hc-install usage for both default + custom URL TF downloads.

Why

Currently, a URL based on system architecture etc is put together to produce a download link. go-getter's GetFile is then used to get the binary. This is broken with TF >= 1.8.2 as a License.txt file is also included. TF maintainer suggested looking into hc-install instead.

Changes

URL Downloads

This refactor uses hc-install to:

  • Get a list of Versions based on a constraint
  • Install the binary (installs as terraform by default so we rename the binary to terraform{version} to match current convention)

Custom URL's

For custom URL usage, the root URL must have the same structure as the default releases.hashicorp.com layout. e.g. it must have 2 index.json files; 1 in the project root and 1 in the version root. e.g.
https://releases.hashicorp.com/terraform/index.json
https://releases.hashicorp.com/terraform/1.8.2/index.json

This is because hc-install parses these json files behind the scenes for it's logic.

Example setup can be seen in this commit: 39443b3

The URL's in the json can be kept as the default Hashicorp releases url's - if a custom ApiBaseUrl is set, hc-install will use that instead as the base URL - the URL in the JSON is ignored.

Can be done via mirroring the website, or an approach similar to this to make one locally

# URL of the root directory
root_url="https://releases.hashicorp.com/terraform"

# Download the root index.json file
curl -s "${root_url}/index.json" -o "index.json"


# Download the version-specific index.json file into the version directory
curl -s "${root_url}/${version}/index.json" -o "${version}/index.json"

# Extract the file names from the version-specific index.json file and download each file
jq -r '.builds[].url' index.json | while read file_url; do
  curl -O "${file_url}"
done

Removing usage of other packages

Usage of warrensbox/terraform-switcher is removed

  • Was used to scrape the API to get a list of TF versions (ref) and to ensure ValidVersionFormat (ref)
  • Both of these use-cases are covered by hc-install

Usage of go-getter's GetFile is also removed.

  • The usage has been replaced by hc-install, and it will not be required by eventual addition of OpemTofu support, as "GetAny" will be used for that to accommodate extra files in the ZIP.
  • e.g. The current WIP PR to add Tofu support uses GetAny.

Future support for OpenTofu

Whilst hc-install cannot be used to install OpenTofu, I personally don't see that as a good reason not to use it for Terraform.

Due to possible divergences in the future (look at the License File in the zip change, unexpected), the same method of go-getter etc. may not be viable for both anyway - there may end up being a lot of conditionals for vendor-specific changes.

I think a better approach is to have different "retrievers" for the different softwares. hc-install for Terraform, go-getter or some other approach for OpenTofu etc.

Notes

  • This PR currently uses a branch of my fork of hc-install which adds support for Custom Download URL's. There is a PR I opened today for hc-install upstream (here) to add the support. Once that goes through, then the go.mod can be changed back to the normal upstream version.

Tests

Seems to pass Unit Tests and Integration Tests.

Local Testing

Ran Atlants + ngrok locally

terraform {
  required_providers {
  }

  required_version = ">= 1.5.0"
}

{"level":"info","ts":"2024-05-03T15:21:26.186+0100","caller":"terraform/terraform_client.go:571","msg":"could not find terraform version 1.8.1 in PATH or /Users/jamesbrookes/.atlantis/bin","json":{"repo":"james0209/test-terraform","pull":"1"}}
{"level":"info","ts":"2024-05-03T15:21:26.186+0100","caller":"terraform/terraform_client.go:583","msg":"using Hashicorp's 'hc-install' to download Terraform version 1.8.1","json":{"repo":"james0209/test-terraform","pull":"1"}}
{"level":"info","ts":"2024-05-03T15:21:27.967+0100","caller":"terraform/terraform_client.go:591","msg":"Downloaded terraform 1.8.1 to /Users/jamesbrookes/.atlantis/bin/terraform1.8.1","json":{"repo":"james0209/test-terraform","pull":"1"}}
{"level":"debug","ts":"2024-05-03T15:21:27.967+0100","caller":"models/shell_command_runner.go:95","msg":"starting \"/Users/jamesbrookes/.atlantis/bin/terraform1.8.1 init -input=false -upgrade\" in \"/Users/jamesbrookes/.atlantis/repos/james0209/test-terraform/1/default\"","json":{"repo":"james0209/test-terraform","pull":"1"}}
{"level":"info","ts":"2024-05-03T15:21:28.878+0100","caller":"models/shell_command_runner.go:161","msg":"successfully ran \"/Users/jamesbrookes/.atlantis/bin/terraform1.8.1 init -input=false -upgrade\" in \"/Users/jamesbrookes/.atlantis/repos/james0209/test-terraform/1/default\"","json":{"repo":"james0209/test-terraform","pull":"1","duration":0.910314042}}
{"level":"info","ts":"2024-05-03T15:21:29.167+0100","caller":"terraform/terraform_client.go:418","msg":"successfully ran \"/Users/jamesbrookes/.atlantis/bin/terraform1.8.1 workspace show\" in \"/Users/jamesbrookes/.atlantis/repos/james0209/test-terraform/1/default\"","json":{"repo":"james0209/test-terraform","pull":"1","duration":0.289030458}}
terraform {
  required_providers {
  }

  required_version = "= 1.7.5"
}

{"level":"debug","ts":"2024-05-03T16:31:25.759+0100","caller":"terraform/terraform_client.go:311","msg":"Found required_version setting of \"= 1.7.5\"","json":{"repo":"james0209/test-terraform","pull":"1"}}
{"level":"info","ts":"2024-05-03T16:31:26.609+0100","caller":"terraform/terraform_client.go:571","msg":"could not find terraform version 1.7.5 in PATH or /Users/jamesbrookes/.atlantis/bin","json":{"repo":"james0209/test-terraform","pull":"1"}}
{"level":"info","ts":"2024-05-03T16:31:26.609+0100","caller":"terraform/terraform_client.go:583","msg":"using Hashicorp's 'hc-install' to download Terraform version 1.7.5","json":{"repo":"james0209/test-terraform","pull":"1"}}
{"level":"info","ts":"2024-05-03T16:31:28.011+0100","caller":"terraform/terraform_client.go:591","msg":"Downloaded terraform 1.7.5 to /Users/jamesbrookes/.atlantis/bin/terraform1.7.5","json":{"repo":"james0209/test-terraform","pull":"1"}}
{"level":"debug","ts":"2024-05-03T16:31:28.011+0100","caller":"models/shell_command_runner.go:95","msg":"starting \"/Users/jamesbrookes/.atlantis/bin/terraform1.7.5 init -input=false -upgrade\" in \"/Users/jamesbrookes/.atlantis/repos/james0209/test-terraform/1/default\"","json":{"repo":"james0209/test-terraform","pull":"1"}}
{"level":"info","ts":"2024-05-03T16:31:29.020+0100","caller":"models/shell_command_runner.go:161","msg":"successfully ran \"/Users/jamesbrookes/.atlantis/bin/terraform1.7.5 init -input=false -upgrade\" in \"/Users/jamesbrookes/.atlantis/repos/james0209/test-terraform/1/default\"","json":{"repo":"james0209/test-terraform","pull":"1","duration":1.008430792}}

Test: Remove <=1.8.2 constraint, test that hc-install correctly downloads it.

terraform {
  required_providers {
  }

  required_version = "= 1.8.2"
}

{"level":"info","ts":"2024-05-03T16:43:32.180+0100","caller":"terraform/terraform_client.go:571","msg":"could not find terraform version 1.8.2 in PATH or /Users/jamesbrookes/.atlantis/bin","json":{"repo":"james0209/test-terraform","pull":"1"}}
{"level":"info","ts":"2024-05-03T16:43:32.180+0100","caller":"terraform/terraform_client.go:583","msg":"using Hashicorp's 'hc-install' to download Terraform version 1.8.2","json":{"repo":"james0209/test-terraform","pull":"1"}}
{"level":"info","ts":"2024-05-03T16:43:33.544+0100","caller":"terraform/terraform_client.go:591","msg":"Downloaded terraform 1.8.2 to /Users/jamesbrookes/.atlantis/bin/terraform1.8.2","json":{"repo":"james0209/test-terraform","pull":"1"}}
{"level":"debug","ts":"2024-05-03T16:43:33.544+0100","caller":"models/shell_command_runner.go:95","msg":"starting \"/Users/jamesbrookes/.atlantis/bin/terraform1.8.2 init -input=false -upgrade\" in \"/Users/jamesbrookes/.atlantis/repos/james0209/test-terraform/1/default\"","json":{"repo":"james0209/test-terraform","pull":"1"}}
{"level":"info","ts":"2024-05-03T16:43:34.447+0100","caller":"models/shell_command_runner.go:161","msg":"successfully ran \"/Users/jamesbrookes/.atlantis/bin/terraform1.8.2 init -input=false -upgrade\" in \"/Users/jamesbrookes/.atlantis/repos/james0209/test-terraform/1/default\"","json":{"repo":"james0209/test-terraform","pull":"1","duration":0.902799416}}

References

@james0209 james0209 requested review from a team as code owners April 30, 2024 02:15
@james0209 james0209 requested review from jamengual, lukemassa and X-Guardian and removed request for a team April 30, 2024 02:15
@github-actions github-actions bot added dependencies PRs that update a dependency file go Pull requests that update Go code labels Apr 30, 2024
@james0209 james0209 changed the title WIP: Use hc-install for default downloading - still currently old method for custom download URL's feat: use hc-install for default URL TF downloads Apr 30, 2024
@james0209 james0209 changed the title feat: use hc-install for default URL TF downloads refactor: use hc-install for default URL TF downloads Apr 30, 2024
@nitrocode
Copy link
Member

One downside of swapping out warrensbox for hc-install is that we lose the potential to reuse the same library to download opentofu versions.

Is opentofu possible with hc-install or would we need to add additional code to support it? If additional code, would swapping to hc-install be worth it in the long run?

@jamengual jamengual added the waiting-on-response Waiting for a response from the user label Apr 30, 2024
@james0209
Copy link
Author

james0209 commented Apr 30, 2024

One downside of swapping out warrensbox for hc-install is that we lose the potential to reuse the same library to download opentofu versions.

Is opentofu possible with hc-install or would we need to add additional code to support it? If additional code, would swapping to hc-install be worth it in the long run?

@nitrocode I do not think installing OpenTofu with hc-install is possible unfortunately. There are a couple of things that come to my mind:

  • Atlantis got caught out by the Terraform license file change as there wasn't much forewarning; the current method is brittle; no thought that it would break anything etc. I think using their first-party tooling which they also use internally would be the best bet to not run into any other issues regarding this?
  • I don't see any issue with using hc-install for Terraform and a different approach for OpenTofu? i.e. when it comes to supporting OpenTofu, we could have a terraformretriever.go and a tofuretriever.go for example. Similar to how tenv has different retrievers for Terraform here and Tofu here

I believe hc-install is also open license, so from my understanding it's not out of the realm of possibility for OpenTofu to fork/adapt a verson for themselves.

It also looks as though warrensbox/terraform-switcher doesn't yet support OpenTofu and there isn't a timeline yet for it's support - based on the open issue I see over there anyway - I haven't dived into it's code yet.

I don't think Atlantis should pigeon-hole itself into using the same methods for obtaining Terraform and it's forks given the divergences that could come in the future - if there's a tool designed for obtaining Terraform and it is the best way forward, why not use it, and use a more suited approach for OpenTofu?

  • From what I can gather, Atlantis doesn't even use terraform-switcher libs that much. It's used to scrape the API to get a list of TF versions (ref) and to ensure ValidVersionFormat (ref), but in-house Atlantis code deals with constraining the list, selecting the version, and downloading the binary via go-getter

There could be a discussion about using tenv, tfenv, terraform-switcher etc. to actually download Terraform?

This is my first PR for Atlantis and I don't know the in's and out's too well yet so I'm more than happy to hear other opinions/views!

@jamengual
Copy link
Contributor

  • Atlantis got caught out by the Terraform license file change as there wasn't much forewarning; the current method is brittle; no thought that it would break anything etc. I think using their first-party tooling which they also use internally would be the best bet to not run into any other issues regarding this?

no, we got Hashicorp approval to use terraform with Atlantis, we do not have that problem.

@james0209
Copy link
Author

james0209 commented Apr 30, 2024

  • Atlantis got caught out by the Terraform license file change as there wasn't much forewarning; the current method is brittle; no thought that it would break anything etc. I think using their first-party tooling which they also use internally would be the best bet to not run into any other issues regarding this?

no, we got Hashicorp approval to use terraform with Atlantis, we do not have that problem.

Sorry if I wasn't clear - I don't mean the Hashicorp License change - I mean the inclusion of the license file in the packaged TF that made Atlantis have to constrain below TF 1.8.2. I'm not questioning Atlantis' perms to use TF, merely saying that the current system broke because of the inclusion of a License File. If anyones tooling is most likely to not break from changes to TF packaging, it's Hashicorps own installer that they use internally

@meringu meringu mentioned this pull request May 2, 2024
@jamengual
Copy link
Contributor

  • Atlantis got caught out by the Terraform license file change as there wasn't much forewarning; the current method is brittle; no thought that it would break anything etc. I think using their first-party tooling which they also use internally would be the best bet to not run into any other issues regarding this?

no, we got Hashicorp approval to use terraform with Atlantis, we do not have that problem.

Sorry if I wasn't clear - I don't mean the Hashicorp License change - I mean the inclusion of the license file in the packaged TF that made Atlantis have to constrain below TF 1.8.2. I'm not questioning Atlantis' perms to use TF, merely saying that the current system broke because of the inclusion of a License File. If anyones tooling is most likely to not break from changes to TF packaging, it's Hashicorps own installer that they use internally

this is a bloody nightmare.... ( runt is over)

I guess we will not be opposed to using the tool if it makes Atlantis maintenance easier, but we have been talking about removing this functionality entirely at some point so that we do not have to maintain this type of feature, which can be very brittle.

Now, we are a package manager for Terraform and Opentofu installs, which adds complexity.

@james0209
Copy link
Author

james0209 commented May 3, 2024

  • Atlantis got caught out by the Terraform license file change as there wasn't much forewarning; the current method is brittle; no thought that it would break anything etc. I think using their first-party tooling which they also use internally would be the best bet to not run into any other issues regarding this?

no, we got Hashicorp approval to use terraform with Atlantis, we do not have that problem.

Sorry if I wasn't clear - I don't mean the Hashicorp License change - I mean the inclusion of the license file in the packaged TF that made Atlantis have to constrain below TF 1.8.2. I'm not questioning Atlantis' perms to use TF, merely saying that the current system broke because of the inclusion of a License File. If anyones tooling is most likely to not break from changes to TF packaging, it's Hashicorps own installer that they use internally

this is a bloody nightmare.... ( runt is over)

I guess we will not be opposed to using the tool if it makes Atlantis maintenance easier, but we have been talking about removing this functionality entirely at some point so that we do not have to maintain this type of feature, which can be very brittle.

Now, we are a package manager for Terraform and Opentofu installs, which adds complexity.

@jamengual I would argue that this PR is one step closer to removing the package manager aspect from Atlantis - it shifts the constraints management and downloading over to hc-install rather than doing it in-house. I agree about removing the functionality - but for that you would need a third-party package-manager / downloader - hc-install may not be a package-manager but it is a downloader.

e.g. Currently, Atlantis calls terraform-switcher to fetch all available TF versions. Atlantis then puts these into a slice, sorts it, iterates though it checking constraints.

hc-install removes the need to fetch TF versions, and decides on the best version given constraints by itself.

@jippi
Copy link
Contributor

jippi commented May 3, 2024

go-getter's GetFile is then used to get the binary. This is broken with TF >= 1.8.2 as a License.txt file is also included

Any reason why we can't tweak go-getter usage to deal with multiple files?

I like hc-install and use it a lot of our internal tooling; just curious if tweaking existing go-getter usage to not care about extra files (of any kind beyond the expected binary) and keeping the code usable between TF and Tofu wouldn't be a bigger net benefit?

  1. Grab zip
  2. Extract the zip to tmp dir
  3. Expect a file named terraform or tofu in the tmp dir
  4. Copy that binary file into place
  5. Wipe the tmp directory

Any future zip file changes or additional files will be ignored.

For the grab+extract, we could either let go-getter deal with it, or grab the file and use archive/zip to pull out the single file we care about.

We would need to pay the maintenance cost of most (everything?) hc-install improves on the TF side for Tofu anyway; so it yields two different behaviors and code paths to maintain and debug, compared to one (assuming it is one today, not familiar with this part of Atlantis yet).

And if HC breaks changes something in the future, we would still be in the same situation, now we would just need to bump hc-install instead; which is less work, but would force upgrades in usage anyway.

@james0209 james0209 changed the title refactor: use hc-install for default URL TF downloads refactor: use hc-install for TF downloads + constraints May 4, 2024
@GenPage GenPage added refactoring Code refactoring that doesn't add additional functionality needs discussion Large change that needs review from community/maintainers labels May 4, 2024
@meringu
Copy link
Contributor

meringu commented May 6, 2024

+1 on this approach.

We'll need a difference in implementation for OpenTofu anyway. As far as I can tell there isn't a compatible endpoint for resolving versions like there is with https://releases.hashicorp.com/terraform. The install script provided with OpenTofu uses the GitHub releases, so should be relatively stable.

I struggled to get go-getter to work with the archive with multiple files in it. It also looks like it wont support the verifying signing methods used by OpenTofu. As far as I can tell it only has support for checksums.

@meringu
Copy link
Contributor

meringu commented May 7, 2024

Awesome, thanks @nitrocode.

I think my observation about the checksums may be more accurate. We don't seem to (I may have missed this too) get shasum files with the tofu releases.

@GenPage GenPage added this to the v0.28.0 milestone May 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies PRs that update a dependency file go Pull requests that update Go code needs discussion Large change that needs review from community/maintainers refactoring Code refactoring that doesn't add additional functionality waiting-on-response Waiting for a response from the user
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Consider using hc-install instead of go-getter to install terraform binary
7 participants