pre-commit-terraform icon indicating copy to clipboard operation
pre-commit-terraform copied to clipboard

Support for OpenTofu

Open RobertKosten opened this issue 1 year ago β€’ 34 comments

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 avatar Sep 15 '23 07:09 RobertKosten

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

yermulnik avatar Sep 15 '23 11:09 yermulnik

Opentofu 1.6.0 exists now in alpha 2.

mcantinqc avatar Oct 10 '23 20:10 mcantinqc

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[bot] avatar Nov 10 '23 00:11 github-actions[bot]

@RobertKosten Did you make any progress on a PR?

rjhenry avatar Nov 12 '23 22:11 rjhenry

+1. Supporting OpenTofu is really appreciated.

worawatwi avatar Nov 17 '23 04:11 worawatwi

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[bot] avatar Dec 18 '23 00:12 github-actions[bot]

This issue was automatically closed because of stale in 10 days

github-actions[bot] avatar Dec 29 '23 00:12 github-actions[bot]

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

kvendingoldo avatar Jan 17 '24 00:01 kvendingoldo

@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! πŸ‡ΊπŸ‡¦

antonbabenko avatar Jan 17 '24 07:01 antonbabenko

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

egarbi avatar Jan 17 '24 17:01 egarbi

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

yermulnik avatar Jan 17 '24 17:01 yermulnik

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?

adarobin avatar Jan 17 '24 20:01 adarobin

@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

MaxymVlasov avatar Jan 17 '24 20:01 MaxymVlasov

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? πŸ˜‚

yermulnik avatar Jan 17 '24 21:01 yermulnik

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

adarobin avatar Jan 17 '24 21:01 adarobin

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)

MaxymVlasov avatar Jan 17 '24 21:01 MaxymVlasov

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 avatar Jan 17 '24 21:01 yermulnik

By the way, should we re-open this issue as of revived interest and effort? πŸ€”

yermulnik avatar Jan 17 '24 21:01 yermulnik

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

kvendingoldo avatar Jan 17 '24 22:01 kvendingoldo

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: https://github.com/antonbabenko/pre-commit-terraform/issues/570#issuecomment-1895277309 β€” 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).

yermulnik avatar Jan 17 '24 22:01 yermulnik

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

adarobin avatar Jan 17 '24 22:01 adarobin

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

MaxymVlasov avatar Jan 17 '24 22:01 MaxymVlasov

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

yermulnik avatar Jan 17 '24 22:01 yermulnik

"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 :)

kvendingoldo avatar Jan 17 '24 23:01 kvendingoldo

@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 🀝

yermulnik avatar Jan 17 '24 23:01 yermulnik

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

kvendingoldo avatar Jan 18 '24 00:01 kvendingoldo

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

egarbi avatar Jan 18 '24 12:01 egarbi

My company has recently switched to OpenTofu. terraform_fmt hook has hardcoded terraform binary https://github.com/antonbabenko/pre-commit-terraform/blob/0340c8d00fe3ba39829b66fd7890d828697a7878/hooks/terraform_fmt.sh#L50

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)

den-is avatar May 02 '24 09:05 den-is

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 🀷🏻)

yermulnik avatar May 02 '24 13:05 yermulnik

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 avatar May 02 '24 13:05 den-is