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: OpenTofu support #4499

Merged
merged 20 commits into from
Sep 30, 2024
Merged

feat: OpenTofu support #4499

merged 20 commits into from
Sep 30, 2024

Conversation

meringu
Copy link
Contributor

@meringu meringu commented May 2, 2024

what

This is a change to get OpenTofu to work with Atlantis:

  • Introduces a --tf-distribution setting that can be set to terraform or opentofu.
  • OpenTofu is downloaded via tofudl.
  • Added an interface to server/core/terraform package for the Terraform distribution. This matches up with the --tf-distribution setting.
  • I would like to also support configuring Terraform or OpenTofu per project in the server side config or repo level config. This would support an easier path to migrate between the two.

why

#3741

tests

  • Have refactored out a distribution and made changes to the downloaders for both Terraform and Conftest to be more testable
  • I've deployed this branch within my org, and have got successful plan and applies working

references

@meringu meringu requested review from a team as code owners May 2, 2024 10:51
@meringu meringu requested review from jamengual, nitrocode and X-Guardian and removed request for a team May 2, 2024 10:51
@github-actions github-actions bot added build Relating to how we build Atlantis docs Documentation go Pull requests that update Go code labels May 2, 2024
@meringu meringu changed the title [WIP] OpenTofu support feat: [WIP] OpenTofu support May 2, 2024
@james0209
Copy link
Contributor

james0209 commented May 2, 2024

I agree with the premise of this, and I like the solution, but I don't agree that it stops the need for hc-install - I think that is worthy of a discussion.

I do not think your changes inherently conflict with the hc-install PR - it is very easy to do this for example

func (c *DefaultClient) DetectVersion(log logging.SimpleLogging, projectDirectory string) *version.Version {
	return c.distribution.DetectVersion(log, c, projectDirectory)
}

type Distribution interface {
	BinName() string
	SourceURL(v *version.Version, downloadURL string) string
	DetectVersion(log logging.SimpleLogging, c *DefaultClient, projectDirectory string) *version.Version
}

func (*DistributionTerraform) DetectVersion(log logging.SimpleLogging, c DefaultClient, projectDirectory string) *version.Version {
// use hc-install
}

func (*DistributionOpenTofu) ListAvailableVersions(log logging.SimpleLogging, downloadBaseURL string, downloadAllowed bool) ([]string, error) {
// use current implementation - only called by the DetectVersion below
}

func (dt *DistributionOpenTofu) DetectVersion(log logging.SimpleLogging, c DefaultClient, projectDirectory string) *version.Version {
// use current implementation
e.g. tfVersions, err := dt.ListAvailableVersions(log, c.downloadBaseURL, c.downloadAllowed)
}

Especially as the ListAvailableVersions func is only called in the current implementation of DetectVersion - there is no other usage.

I think if you are going to have separate interfaces for OpenTofu vs Terraform implementations, then a discussion on whether to use the same logic vs different logic for each Distribution should be had.

  • An example being how this PR already introduces 2 different logic flows for ListAvailableVersions (due to tagging, pre-relese etc.) - with hc-install, that would only be needed by OpenTofu, as it is handled by hc-install for Terraform

@jamengual jamengual added needs discussion Large change that needs review from community/maintainers waiting-on-review Waiting for a review from a maintainer labels May 2, 2024
@meringu
Copy link
Contributor Author

meringu commented May 2, 2024

Thanks @james0209, that makes sense.

I'd be happy to rebase this over the hc-install change. As you point out, that should be fairly straightforward.

@nitrocode nitrocode marked this pull request as draft May 3, 2024 12:23
@nitrocode nitrocode changed the title feat: [WIP] OpenTofu support feat: OpenTofu support May 3, 2024
@nitrocode
Copy link
Member

Due to wip status, I switched this to a draft as a non draft means that it's ready for review. Please set it as ready to review when ready.

Thank you for the contribution

@chenrui333
Copy link
Member

I'd be happy to rebase this over the hc-install change. As you point out, that should be fairly straightforward.

we have merged hc-install update, might worth do the rebase and pick up this PR again. Thanks for all the efforts!

@GenPage GenPage added waiting-on-response Waiting for a response from the user and removed waiting-on-review Waiting for a review from a maintainer labels Jun 26, 2024
@jamengual
Copy link
Contributor

The OpenTofu team is going to release this downloader to make it easier for us to auto download Opentofu.

https://github.com/janosdebugs/downloader
You can pull it in with a replace statement in go.mod for now until is released today or Monday.

@meringu @nitrocode if any of you want to work on this let us know

@meringu
Copy link
Contributor Author

meringu commented Jul 12, 2024

Awesome, I rebased this from main yesterday and got to the part where we need to download tofu.

I'm keen to keep working on this, but happy to have it taken over if I'm being too slow.

@meringu
Copy link
Contributor Author

meringu commented Jul 16, 2024

Tofu downloader has been moved: https://github.com/opentofu/tofudl

@meringu
Copy link
Contributor Author

meringu commented Aug 2, 2024

Thanks for your patience. I've done the following:

  • Merged from main to get the hc-install changes.
  • Updated to use tofu downloader.
  • Refactored out a distribution and downloader.
  • Made a separate downloader for conftest to decouple it from the TF downloaders.

I've done some testing on one of my Atlantis servers. I've been able to do successful plan and applies. Have also tested resolving different version from constraints from the required_version setting.

@X-Guardian
Copy link
Contributor

Hi @meringu, it looks there are a lot of spurious changes included in this PR, .gitignore, dockerfile, a bunch of mock files. Can you remove the ones that are not relevant to this PR. Thanks!

@meringu meringu force-pushed the opentofu branch 2 times, most recently from ab4be23 to f0f91d9 Compare August 4, 2024 22:23
@meringu
Copy link
Contributor Author

meringu commented Aug 4, 2024

Thanks @X-Guardian, I have cherry picked out a new commit, and only run go generate on the packages I modified.

@meringu
Copy link
Contributor Author

meringu commented Aug 6, 2024

Have updated the PR description to reflect the changes. The PR is labeled as needs-discussion and waiting-on-response, so please let me know how I can help.

@meringu
Copy link
Contributor Author

meringu commented Sep 23, 2024

Looks like tofudl is failing to compile on 32bit as this constant is too large: https://github.com/opentofu/tofudl/blob/76c6857d6eac665824b29da4e893e06e0b1896b2/branding/branding.go#L36

I'll look at raising a PR.

Copy link

@abstractionfactory abstractionfactory left a comment

Choose a reason for hiding this comment

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

Just a few comments from the Tofu side, thank you for your work!

server/core/terraform/distribution.go Outdated Show resolved Hide resolved
@meringu
Copy link
Contributor Author

meringu commented Sep 23, 2024

I believe this is ready for re-review (I don't have permissions to update the tags). I've left the OpenTofu mirror discussion open, If possible I would be keen to leave as is, and add extra flags to Atlantis once we have mirrors in use in the wild or someone requests this feature. This should give us a little more flexibility while the ecosystem matures.

My guess is the security vulnerabilities aren't related to my PR, as main is failing too. If a maintainer feels comfortable sharing the findings, I'd be happy to take a look in this PR or another.

@jamengual jamengual added waiting-on-review Waiting for a review from a maintainer and removed waiting-on-response Waiting for a response from the user labels Sep 23, 2024
@abstractionfactory
Copy link

Hey folks, anything we can do from the OpenTofu side to help move this one along?

Copy link
Contributor

@lukemassa lukemassa left a comment

Choose a reason for hiding this comment

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

I tested locally and it seems to work fine, and the code looks good to me!

Thanks for the contribution!

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Sep 30, 2024
@jamengual
Copy link
Contributor

Thanks @meringu and everyone that helped on this.

@jamengual jamengual merged commit 9c7e5fc into runatlantis:main Sep 30, 2024
31 of 32 checks passed
a1k0u pushed a commit to a1k0u/atlantis that referenced this pull request Nov 1, 2024
Co-authored-by: PePe Amengual <[email protected]>
Signed-off-by: a1k0u <[email protected]>
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 docs Documentation feature New functionality/enhancement go Pull requests that update Go code lgtm This PR has been approved by a maintainer waiting-on-review Waiting for a review from a maintainer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support OpenTofu (epic)
10 participants