atlantis icon indicating copy to clipboard operation
atlantis copied to clipboard

refactor: use `hc-install` for TF downloads + constraints

Open james0209 opened this issue 1 year ago • 10 comments

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: https://github.com/runatlantis/atlantis/pull/4494/commits/39443b3f5a8e08e4eecacc456f1faf6db0fe1d32

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

  • closes https://github.com/runatlantis/atlantis/issues/4483

james0209 avatar Apr 30 '24 02:04 james0209

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 avatar Apr 30 '24 04:04 nitrocode

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!

james0209 avatar Apr 30 '24 17:04 james0209

  • 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.

jamengual avatar Apr 30 '24 18:04 jamengual

  • 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

james0209 avatar Apr 30 '24 18:04 james0209

  • 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 avatar May 03 '24 15:05 jamengual

  • 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.

james0209 avatar May 03 '24 15:05 james0209

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.

jippi avatar May 03 '24 16:05 jippi

+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 avatar May 06 '24 22:05 meringu

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.

See this page https://releases-page.opentofu-get.pages.dev/releases/

Ref

  • https://github.com/opentofu/opentofu/issues/928
  • https://github.com/opentofu/get.opentofu.org/pull/16
  • https://github.com/runatlantis/atlantis/issues/4339
  • https://github.com/opentofu/get.opentofu.org/pull/22

nitrocode avatar May 07 '24 13:05 nitrocode

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.

meringu avatar May 07 '24 20:05 meringu

Any ETA for this feature to be implemented ? this is still broken for version 1.8.5

atlantis-atlantis-1  | {"level":"error","ts":"2024-06-12T16:50:29.616Z","caller":"terraform/terraform_client.go:164","msg":"could not download terraform 1.8.5: downloading terraform version 1.8.5 at \"https://releases.hashicorp.com/terraform/1.8.5/terraform_1.8.5_linux_amd64.zip?checksum=file:https://releases.hashicorp.com/terraform/1.8.5/terraform_1.8.5_SHA256SUMS\": expected a single file: /tmp/getter302320555/archive","json":{},"stacktrace":"github.com/runatlantis/atlantis/server/core/terraform.NewClientWithDefaultVersion.func1\n\tgithub.com/runatlantis/atlantis/server/core/terraform/terraform_client.go:164"}

caiodelgadonew avatar Jun 12 '24 16:06 caiodelgadonew

Any ETA for this feature to be implemented ? this is still broken for version 1.8.5

atlantis-atlantis-1  | {"level":"error","ts":"2024-06-12T16:50:29.616Z","caller":"terraform/terraform_client.go:164","msg":"could not download terraform 1.8.5: downloading terraform version 1.8.5 at \"https://releases.hashicorp.com/terraform/1.8.5/terraform_1.8.5_linux_amd64.zip?checksum=file:https://releases.hashicorp.com/terraform/1.8.5/terraform_1.8.5_SHA256SUMS\": expected a single file: /tmp/getter302320555/archive","json":{},"stacktrace":"github.com/runatlantis/atlantis/server/core/terraform.NewClientWithDefaultVersion.func1\n\tgithub.com/runatlantis/atlantis/server/core/terraform/terraform_client.go:164"}

🤷 I've been waiting on a review for a lil' while - not sure why there is still a "waiting-on-response" label on this PR.

james0209 avatar Jun 12 '24 18:06 james0209

it requires @GenPage @Luay-Sol @chenrui333 @nitrocode to get to a consensus on this approach.

jamengual avatar Jun 12 '24 19:06 jamengual

For what it's worth, the warrensbox package now supports opentofu

https://github.com/warrensbox/terraform-switcher/pull/435 https://github.com/warrensbox/terraform-switcher/issues/315

It's pending an official release.


The downside of supporting a tf specific package (hc-getter) is that we need to reimplement it for opentofu.

Since the upstream package is planning to support it, then we might as well wait, upgrade the dependency, and expose the inputs. This would reduce maintenance for us.

nitrocode avatar Jun 13 '24 05:06 nitrocode

@nitrocode have you looked into tfenv? I use it and I think it's amazing..

https://github.com/tfutils/tfenv

caiodelgadonew avatar Jun 13 '24 10:06 caiodelgadonew

I think youre suggesting to use a different library that supports both opentofu and terraform which would be additional work to fit in into atlantis. I haven't evaluated that dependency.

If a single dependency is chosen, it supports both opentofu and terraform, it's well maintained, easy to implement, then that can be used.

Either way (warrensbox or tfenv), using hc-install would be more effort to maintain when it may be easier to defer that to a separate library (warrensbox or tfenv).

nitrocode avatar Jun 13 '24 11:06 nitrocode

For what it's worth, the warrensbox package now supports opentofu

warrensbox/terraform-switcher#435 warrensbox/terraform-switcher#315

It's pending an official release.

The downside of supporting a tf specific package (hc-getter) is that we need to reimplement it for opentofu.

Since the upstream package is planning to support it, then we might as well wait, upgrade the dependency, and expose the inputs. This would reduce maintenance for us.

@nitrocode The way Atlantis currently uses terraform-switcher lib currently is not for the actual downloading part of the process.

It uses a GetTFList method from the lib to return all available versions of TF (ref). That is currently the only implementation of terraform-switcher in Atlantis (that function has since been made private upstream (ref).

Are you talking about fully-implementing terraform-switcher to do the downloading of Terraform? I think it would be possible - looks like you could make use GetSemver and InstallProductVersion.

Although if the tfswitch route is to be taken, you should be mindful of:

  • https://github.com/warrensbox/terraform-switcher/pull/457
  • https://github.com/warrensbox/terraform-switcher/discussions/452
  • There is a PR as you can see - but tfswitch is only just maybe moving to "officially" supporting other libs importing it (e.g. until that PR is merged, no errors are thrown from public funcs, just os.Exit()

I don't mind which route is chosen; I made this PR a month and a half ago to try and fix something that is still broken.

james0209 avatar Jun 13 '24 11:06 james0209

Are you talking about fully-implementing terraform-switcher to do the downloading of Terraform?

Yes that's exactly what I mean.

Since the library added support for opentofu, we can use the library to also download the binary.

I made this PR a month and a half ago to try and fix something that is still broken.

What is broken? I thought this was originally fixing the issue where 1.8.2+ could not be installed but that has since been resolved.

After that was fixed, i thought this pr was being pushed to insulate against future terraform zip changes.

nitrocode avatar Jun 13 '24 11:06 nitrocode

Are you talking about fully-implementing terraform-switcher to do the downloading of Terraform?

Yes that's exactly what I mean.

Since the library added support for opentofu, we can use the library to also download the binary.

I made this PR a month and a half ago to try and fix something that is still broken.

What is broken? I thought this was originally fixing the issue where 1.8.2+ could not be installed but that has since been resolved.

After that was fixed, i thought this pr was being pushed to insulate against future terraform zip changes.

Nope. >= 1.8.2 download is still not possible; the temporary fix implemented was a constraint preventing Atlantis from downloading greater than that. The only version greater than that available to Atlantis is the one that is pre-packaged into the Docker image as the default (1.8.4 or 1.8.5 now I think?). Which is why the lack of reviews/discussion around this PR has been a bit annoying.

https://github.com/runatlantis/atlantis/blob/854c287986b2f52efccd72cd2776a2b26081dd6d/server/core/terraform/terraform_client.go#L341-L345

@nitrocode

james0209 avatar Jun 13 '24 12:06 james0209

I will go ahead and weigh in here. @james0209, I apologize for the lack of review from the team. We are stretched very thin, and my focus has been on growing the community and our team so that we can adequately handle reviews promptly. @nitrocode I appreciate your driving the discussion thus far.

I want us all to align on a simple solution agnostic to the workloads users wish to run, whether it be OpenTofu, Terraform, etc. However, given the nature of the broken user experience, I think it would be prudent to proceed with this PR that has a solution ready and revisit this in a future refactor once we've decided on an approach for our long-term goals to support Terraform and OpenTofu.

@nitrocode @jamengual @lukemassa

GenPage avatar Jun 13 '24 17:06 GenPage

sounds good to me.

jamengual avatar Jun 13 '24 19:06 jamengual

I didn't realize it was still broken, thanks for the comment. My apologies.

nitrocode avatar Jun 14 '24 06:06 nitrocode

@james0209 its approved and automerged is enabled. Just need to resolve the merge conflicts.

GenPage avatar Jun 15 '24 23:06 GenPage

@james0209 its approved and automerged is enabled. Just need to resolve the merge conflicts.

Fixing the conflict unfortunately disabled auto-merge because I don't have write access @GenPage

james0209 avatar Jun 15 '24 23:06 james0209