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

Support for OpenTofu #570

Closed
RobertKosten opened this issue Sep 15, 2023 · 35 comments · Fixed by #670
Closed

Support for OpenTofu #570

RobertKosten opened this issue Sep 15, 2023 · 35 comments · Fixed by #670
Assignees
Labels
feature New feature or request

Comments

@RobertKosten
Copy link

It would be nice if the hooks could support using opentf in addition to terraform. I'm thinking a simple change to using _BIN variables and setting those depending on what can be found in the PATH. I'd be willing to author a PR for that and help maintain that functionality in the future, if it is welcome :-)

@RobertKosten RobertKosten added the feature New feature or request label Sep 15, 2023
@yermulnik
Copy link
Collaborator

It would be nice if the hooks could support using opentf in addition to terraform.

Not sure it makes sense to support non-existent (at least as of yet) tool.

I'd be willing to author a PR for that and help maintain that functionality in the future, if it is welcome :-)

Yep, contributions are defo welcome: https://github.com/antonbabenko/pre-commit-terraform/blob/master/.github/CONTRIBUTING.md

@mcantinqc
Copy link

Opentofu 1.6.0 exists now in alpha 2.

Copy link

This issue has been automatically marked as stale because it has been open 30 days
with no activity. Remove stale label or comment or this issue will be closed in 10 days

@github-actions github-actions bot added the stale Denotes an issue or PR has remained open with no activity and has become stale. label Nov 10, 2023
@rjhenry
Copy link

rjhenry commented Nov 12, 2023

@RobertKosten Did you make any progress on a PR?

@github-actions github-actions bot removed the stale Denotes an issue or PR has remained open with no activity and has become stale. label Nov 13, 2023
@worawatwi
Copy link

+1.
Supporting OpenTofu is really appreciated.

This comment has been minimized.

@github-actions github-actions bot added the stale Denotes an issue or PR has remained open with no activity and has become stale. label Dec 18, 2023

This comment has been minimized.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Dec 29, 2023
@kvendingoldo
Copy link

Hello everyone. Out team forked the project and plan to adopt it for OpenTofu by the end of this week.
Welcome to contribute: https://github.com/tofuutils/pre-commit-opentofu

cc @RobertKosten @mcantinqc @rjhenry @worawatwi

@antonbabenko
Copy link
Owner

@kvendingoldo Please stop spamming this repository (5 comments and PR during 24 hours)!

You've made a fork, but you brag about it so many times like it contains something SO BIG... it is just a fork :)

Slava Ukraini! 🇺🇦

@egarbi
Copy link

egarbi commented Jan 17, 2024

@antonbabenko are you planning to add the support for Opentofu? there is a stable version now.

@yermulnik
Copy link
Collaborator

@egarbi An excerpt from what I replied to topicstarter in this regards in the very beginning of this thread:
image

@adarobin
Copy link

Would the preferred route be to adapt terraform_fmt and terraform_validate to work with both or would it make more sense to basically duplicate the existing hooks and make a separate opentofu_fmt and opentofu_validate hooks?

@MaxymVlasov
Copy link
Collaborator

@adarobin Adapt.
I think that we can make something like

--hook-config=binary=<path_to_binary_or_binary_name> 

Which will add much more flexibility than just adding support for tofu

@yermulnik
Copy link
Collaborator

Which will add much more flexibility than just adding support for tofu

While I support the approach, just out of curiosity (and for fun): how many more TF derivatives do you expect to appear in future? 😂

@adarobin
Copy link

@MaxymVlasov as the projects start to diverge over time there might be functionality in one that does not exist in the other so that might start to make a shared hook interesting.

That said, I'm willing to make an attempt at it, but I won't have time to start on it for a month or so.

@MaxymVlasov
Copy link
Collaborator

While I support the approach, just out of curiosity (and for fun): how many more TF derivatives do you expect to appear in future?

Good one :D
I more thinking about providing a kind of "CI pseudo-matrix" for cases when someone will try to test t validate not in their current tf, but in lower supported, like /usr/bin/terraform0.13 which can be useful for module maintainers. Or test both tf and tofu etc.

@adarobin roger that. Please let us know when you'll start (in case nobody else will not start before)

@yermulnik
Copy link
Collaborator

which can be useful for module maintainers. Or test both tf and tofu etc.

Yep-yep, I fully support the approach. Would've been great if there existed an option to provide such a var or parameter globally to pre-commit-terraform (so that people can change the value in on single place), though I'm not sure such an option exists 🤷🏻

@yermulnik
Copy link
Collaborator

By the way, should we re-open this issue as of revived interest and effort? 🤔

@kvendingoldo
Copy link

@adarobin

as the projects start to diverge over time there might be functionality in one that does not exist in the other so that might start to make a shared hook interesting.

For that reason it will make more sense to develop the forked version of hook only for opentofu, that won't have any legacy terraform stuff. We kicked off the fork, but haven't deleted old terraform legacy code that is not required for opentofu.

@yermulnik
Copy link
Collaborator

yermulnik commented Jan 17, 2024

it will make more sense to develop the forked version of hook only for opentofu

Right, double the effort instead of contributing 👍🏻 Defo it's the way to go...
Please give this comment a read once again and mindfully: #570 (comment) — it seems like you're not adding any value by posting comments apart from making your best to have people start using your forked repo.

legacy terraform stuff

"legacy"... okay. Could you please explain what's the modern thing about opentofu at the moment apart from it using another license (which is good in general and I'm not pushing against opentofu).

@MaxymVlasov MaxymVlasov reopened this Jan 17, 2024
@MaxymVlasov MaxymVlasov removed the stale Denotes an issue or PR has remained open with no activity and has become stale. label Jan 17, 2024
@adarobin
Copy link

@kvendingoldo many of the hooks in this repo don't even rely on the terraform binary directly. In other words, there is nothing to even fork. For example, you aren't going to make a theoretical tofu_tflint unless you also forked tflint.

@MaxymVlasov
Copy link
Collaborator

MaxymVlasov commented Jan 17, 2024

Yeah, we definitely can change hooks names in 2.0.0 [BREAKING CHANGES] milestone (whenever it will come) to reduce wrong perceptions. If you have any suggestions - please open a separate issue as this one is already overloaded

@yermulnik
Copy link
Collaborator

we definitely can change hooks names

Given the terraform name being de-facto a very well known and very widely used term for one of the most popular IaaC implementations and opentofu being a fork of terraform in general (at this very current moment as a very least and obviously until opentofu hits its own road by introducing its own "different-from-terraform" features), I'd refrain from sudden changes and fluctuations (the tags on the repo will help us promote pre-commit-terraform support for opentofu hopefully).
Support for opentofu within terraform-specific hooks is great expansion of what pre-commit-terraform provides to users, hence let's try and implement the best we can do (and I hope people will join by contributing to this repo 🤞🏻 hopefully including @kvendingoldo et alii, that would reduce common effort and help end-users avoid vagueness on why the difference and what to choose for where they still use terraform and/or where they've already switched to opentofu for whichever reason).

@kvendingoldo
Copy link

"legacy"... okay.

Thank you for sharing your perspective, and I truly value feedback. My primary aim is to contribute positively to the Terraform/OpenTofu community by sharing improvements that I believe can be beneficial for everyone.

If you perceive my engagement in this discussion as promotion of something else, it's ok, you can have this opinion. My original intent was to provide OpenTofu support by git pre-commit hooks. I forked the repository yesterday evening and began adapting hooks for that purpose, and while the differences may currently be minimal, it addressed the specific issue I wanted to resolve collaboratively with others.

If you hold reservations about the concept of forks tailored exclusively for OpenTofu without legacy code support (Terraform 0.12/0.13 and so on), I completely understand. Everyone is entitled to their opinions, and I appreciate the diverse perspectives within the community. From my point of view, I think that OpenTofu will have differences and will be better tool with real open source license. From this point of view the fork of hooks at the early stage is a good idea, because I'm not sure that it will make sense to use Terraform for new projects. Also, as I said before, this repo contains a lot of legacy code for old versions of Terraform that is useless for OpenTofu.

I'll continue to work on OpenTofu hooks because first of all I'm reaching my goal at project that requires Tofu as a primary IAC tool. Welcome to contribute to OpenTofu hooks without legacy code :)

@yermulnik
Copy link
Collaborator

@kvendingoldo I see and understand your perspective. It's all fine. It's opensource. Hopefully someday you'll get to the point of giving a proper respect to the source of what your repo is effectively based off of by adding sort of "courtesy of" or "forked from" to the README 😏 Cheers and good luck with your own endeavor in this area. Feel free to contribute and collaborate when you feel necessary. We're open for cooperation 🤝

@kvendingoldo
Copy link

@yermulnik no worries, all links to the original project will be saved. GitHub deleted the link to it after detach.

@egarbi
Copy link

egarbi commented Jan 18, 2024

Can you perhaps fix the title to avoid misleading (or not leading) people s/OpenTF/OpenTofu/ many thanks

@den-is
Copy link
Contributor

den-is commented May 2, 2024

My company has recently switched to OpenTofu.
terraform_fmt hook has hardcoded terraform binary

terraform fmt "${args[@]}"

Only because of this hood I have to keep terraform binary in $PATH.
Yeah, I'm aware about symlinks, and possible renames :) , but the fact is that thing is hardcoded in this hook)

@yermulnik
Copy link
Collaborator

It's hardcoded in other hooks too. And I think in fact it's because hooks are tool-specific to some extent.

I think it should be feasible to add hook config knob to all hooks where opentofu is a drop-in replacement for terraform to allow user to provide TF-binary name (with optional path for when it is not in PATH). @MaxymVlasov What do you think? Does this sound meaningful at all?

Else, @den-is, it should be easy to PR the Opentofu fmt hook by simply copying terraform_fmt.sh into opentofu_fmt.sh and adding aux bits to .pre-commit-hooks.yaml and README (anything else? 🤔). Please refer to https://github.com/antonbabenko/pre-commit-terraform/blob/master/.github/CONTRIBUTING.md and we'll be happy to review and accept the contribution (not sure about other collaborators, though I personally don't use opentofu at the moment, hence it would a bit challenging for me to test this new hooks if I'd contribute it myself 🤷🏻)

@den-is
Copy link
Contributor

den-is commented May 2, 2024

Else, @den-is, it should be easy to PR the Opentofu fmt hook by simply copying terraform_fmt.sh into opentofu_fmt.s

damn.. my bad.. :) I forgot to mention/foresee this suggestion to include in my list of future suggestions.

Exactly! Because the binary name is hardcoded in other tools it is not optimal to copy-paste files to accommodate just one tool. At least while they quite similar at this moment in time.

My potential imagined solution sounds like this: These pre-commit hooks have to calculate/determine and then set the correct binary for hooks... apparently, this should be done in _common.sh. (using arguments, or env, or ...)
This is similar to Terragrunt's behavior from one of the latest releases.

Or another terragrunt's feature of setting TF binary export TERRAGRUNT_TFPATH="tofu"

@den-is
Copy link
Contributor

den-is commented May 2, 2024

Because there are already clones of this project that target specifically opentofu binary. But I really want to support this project 💛💙

@yermulnik
Copy link
Collaborator

Yep, the option for a user to redefine TF binary and path looks best. I can see @antonbabenko already liked this approach by adding his reaction above. Though let's see what @MaxymVlasov says as he's the main dev doing dev work on this project.

@MaxymVlasov
Copy link
Collaborator

As I wrote above in #570 (comment) :

I think that we can make something like

--hook-config=binary=<path_to_binary_or_binary_name> 

Which will add much more flexibility than just adding support for tofu

Also, it can be done as env var too, to set 1 path for all hooks at once.

@robinbowes
Copy link
Contributor

As a general comment, in some circumstances it's useful for the user to be able to specify the explicit binary to use when invoking, eg. terraform.

One way to do this is to have a default setting (in this case terraform) and allow it to be over-ridden in various ways, with a clearly defined precedence, eg. command-line option > env var > user config file > global config file.

@antonbabenko
Copy link
Owner

This issue has been resolved in version 1.90.0 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment