-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Conversation
…ethod for custom download URL's
hc-install
for default downloading - still currently old method for custom download URL'shc-install
for default URL TF downloads
hc-install
for default URL TF downloadshc-install
for default URL TF downloads
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:
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 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?
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! |
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 |
…n't have to do s many conversions
For what it's worth, the warrensbox package now supports opentofu warrensbox/terraform-switcher#435 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 have you looked into tfenv? |
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 The way Atlantis currently uses It uses a Are you talking about fully-implementing Although if the
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. |
Yes that's exactly what I mean. Since the library added support for opentofu, we can use the library to also download the binary.
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. atlantis/server/core/terraform/terraform_client.go Lines 341 to 345 in 854c287
|
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. |
sounds good to me. |
I didn't realize it was still broken, thanks for the comment. My apologies. |
@james0209 its approved and automerged is enabled. Just need to resolve the merge conflicts. |
Head branch was pushed to by a user without write access
Fixing the conflict unfortunately disabled auto-merge because I don't have write access @GenPage |
What
Replace
go-getter
usage withhc-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
'sGetFile
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 intohc-install
instead.Changes
URL Downloads
This refactor uses
hc-install
to:terraform
by default so we rename the binary toterraform{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 2index.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
Removing usage of other packages
Usage of
warrensbox/terraform-switcher
is removedValidVersionFormat
(ref)hc-install
Usage of
go-getter
'sGetFile
is also removed.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.Tests
Seems to pass Unit Tests and Integration Tests.
Local Testing
Ran Atlants + ngrok locally
Test: Remove <=1.8.2 constraint, test that hc-install correctly downloads it.
Notes
hc-install
, although there is aLicenseDir
arg that we can customise.bin
dir in which they are all placed with the nameterraform<version>
.1.8.1
,1.8.2
, and the executables at1.8.1/terraform
,1.8.2/terraform
etc.References