-
Notifications
You must be signed in to change notification settings - Fork 44
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: support OpenTofu #185
Conversation
TomerHeber
commented
Feb 1, 2024
•
edited
Loading
edited
- Removed version check. (Not required).
- Defaults to opentofu if "tofu" executable is found.
- Wording changes.
- Integration test for opentofu latest.
@@ -23,7 +23,6 @@ type TaggingArgs struct { | |||
IsSkipTerratagFiles bool | |||
Rename bool | |||
IACType IACType | |||
TFVersion Version |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
version was required for 0.11 - terratag no longer support 0.11
newTagsValue = "merge( " + existingTagsExpression + ", " + terratagAddedKey + ")" | ||
} | ||
|
||
if args.TfVersion.Major == 0 && args.TfVersion.Minor == 11 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
0.11
internal/tfschema/tfschema.go
Outdated
cmd := exec.Command("terraform", "providers", "schema", "-json") | ||
// Use tofu by default (if it exists). | ||
name := "terraform" | ||
if _, err := exec.LookPath("tofu"); err == nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this was broken for cases where only tofu was installed.
currently always defaults to tofo is exists.
May want to make it configurable (?) - not sure...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you elaborate? broken how? make what configurable?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you mean that as long as tofu
is installed - this would resolve to it, even if someone specifies defaultToTerraform
? or did you mean something else
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
broken if only tofu was installed and not terraform. (Terratag would not work).
The comment is outdated. If someone uses defaultToTerraform it will default to terraform. I had written this comment before adding 'defaultToTerraform'.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and what happens if only tofu is installed, and defaultToTerraform
isn't provided?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it will use tofu.
- If only terraform is installed - use terraform.
- if only tofu is installed - use tofu.
- If both terraform and opentofu are installed - use tofu.
- If both terraform and opentofu are installed and default terraform is provided - use terraform.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so it sounds like there's no bug?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
before the fix it will always use "terraform" which is a bug if only opentofu is installed.
I don't think there are any bugs after the fix.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks
@@ -71,6 +71,31 @@ func TestTerraformlatest(t *testing.T) { | |||
testTerraform(t, "latest") | |||
} | |||
|
|||
func TestOpenTofu(t *testing.T) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
integration test for latest opentofu.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good
Just wanna make sure i fully understand this