Feat: Breaking Change - default terragrunt_tf_binary to 'opentofu' #980
Conversation
| @@ -410,25 +410,6 @@ func templateCreatePayloadFromParameters(prefix string, d *schema.ResourceData) | |||
| return payload, diag.Errorf("schema resource data serialization failed: %v", err) | |||
| } | |||
|
|
|||
| isNew := d.IsNewResource() | |||
There was a problem hiding this comment.
this was the code to handle only for new resources... this is the breaking change...
|
/review |
| @@ -1443,7 +1443,7 @@ func resourceEnvironmentImport(ctx context.Context, d *schema.ResourceData, meta | |||
| return nil, fmt.Errorf("failed to get template with id %s: %w", templateId, err) | |||
| } | |||
|
|
|||
| if err := templateRead("without_template_settings", template, d, true); err != nil { | |||
| if err := templateRead("without_template_settings", template, d); err != nil { | |||
There was a problem hiding this comment.
can explain this file changes?
There was a problem hiding this comment.
There was special handling related to import and TerragruntTfBinary - it's no longer needed since we will implicitly default to "opentofu".
If there is a value returned from the http response (when askign for the template) it will be assigned to the schema.
In other words, the code that was handling a special use case related to terraform import and TerragruntTfBinary was removed. So it's no longer required to pass a true/false flag.
| // This is done to avoid drifts in case the backend returns "opentofu", but non is configured in the provider. | ||
| // (The provider implicitly defaults to "opentofu"). | ||
| if template.TerragruntTfBinary == client.OPENTOFU { |
There was a problem hiding this comment.
- But this PR will do a breaking change on purpose, so why do we need that?
- if BE returns "opentofu" and non is configured in the provider, it means this template was "terraform" before. So we should actually set "terraform", or actually show the drift. Am I missing something? i
There was a problem hiding this comment.
If the BE returns opentofu - then it's opentofu.
The backend is the source of truth.
However, if nothing is set in the provider - then - the defaul is "opentofu".
For various hashicorp reasons, I can't set default as "opentofu" in the terraform schema. But still want avoid the drift for the "opentofu" case. '' -> 'opentofu'.
If in the backend the value is "terraform" - and nothing was set in the provider - the user will get a drift message terraform -> null. To avoid the drift they will have to explicitly add terragrunt_tf_binary = 'terraform'
Now this is assuming that the backend (source of truth) will always return the correct value.
There was a problem hiding this comment.
terraform -> null
isn't it '' -> 'terraform'? I didnt get this part
and thanks for the explanation
There was a problem hiding this comment.
yeah - I think you're right. Just flip the examples.
github-actions
bot
added
ready to merge
Yes. That's the plan. |
|
yea lets |
|
Maybe release it after other open PRs, an isolated version of a single change @TomerHeber |
Issue & Steps to Reproduce / Feature Request
resolves #979
Solution
--- breaking change for terraform + terragrunt_tf_binary (must excplicitly state 'terraform' value).
Discussion thread:
https://env0.slack.com/archives/C01QBBEGBEH/p1731500435184849