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.

Sign up for GitHub

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: Breaking Change - default terragrunt_tf_binary to 'opentofu' #980

Merged
merged 6 commits into from
Nov 30, 2024
Merged

Feat: Breaking Change - default terragrunt_tf_binary to 'opentofu' #980

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
7b03946
Feat: Breaking Change - default terragrunt_tf_binary to 'opentofu'
TomerHeber Nov 16, 2024
4692578
add omit empty
TomerHeber Nov 16, 2024
af8e8d7
added tests
TomerHeber Nov 16, 2024
5b13de4
Merge branch 'main' into feat-terragrunt-binary-#979
TomerHeber Nov 17, 2024
6abd865
update bad input use-cases
TomerHeber Nov 17, 2024
37c8525
Merge branch 'main' into feat-terragrunt-binary-#979
TomerHeber Nov 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 20 additions & 7 deletions client/template.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ type TemplateCreatePayload struct {
IsAzureDevOps bool `json:"isAzureDevOps" tfschema:"is_azure_devops"`
IsHelmRepository bool `json:"isHelmRepository"`
HelmChartName string `json:"helmChartName,omitempty"`
TerragruntTfBinary string `json:"terragruntTfBinary,omitempty"`
razbensimon marked this conversation as resolved.
Show resolved Hide resolved
TerragruntTfBinary string `json:"terragruntTfBinary"`
AnsibleVersion string `json:"ansibleVersion,omitempty"`
}

Expand Down Expand Up @@ -131,18 +131,31 @@ func (payload *TemplateCreatePayload) Invalidate() error {
return errors.New("can't define terragrunt version for non-terragrunt template")
}

if payload.Type == TERRAGRUNT && payload.TerragruntVersion == "" {
return errors.New("must supply terragrunt version")
if payload.Type == TERRAGRUNT {
if payload.TerragruntVersion == "" {
return errors.New("must supply terragrunt version")
}

// The provider implicitly defaults to "opentofu".
if payload.TerragruntTfBinary == "" {
payload.TerragruntTfBinary = OPENTOFU
}

if payload.TerragruntTfBinary == OPENTOFU && payload.OpentofuVersion == "" {
razbensimon marked this conversation as resolved.
Show resolved Hide resolved
return errors.New("must supply opentofu version")
}

if payload.TerragruntTfBinary == TERRAFORM && payload.TerraformVersion == "" {
return errors.New("must supply terraform version")
}
} else {
payload.TerragruntTfBinary = ""
razbensimon marked this conversation as resolved.
Show resolved Hide resolved
}

if payload.Type == OPENTOFU && payload.OpentofuVersion == "" {
return errors.New("must supply opentofu version")
}

if payload.TerragruntTfBinary != "" && payload.Type != TERRAGRUNT {
razbensimon marked this conversation as resolved.
Show resolved Hide resolved
return fmt.Errorf("terragrunt_tf_binary should only be used when the template type is 'terragrunt', but type is '%s'", payload.Type)
}

if payload.IsTerragruntRunAll {
if payload.Type != TERRAGRUNT {
return errors.New(`can't set is_terragrunt_run_all to "true" for non-terragrunt template`)
Expand Down
4 changes: 2 additions & 2 deletions env0/resource_environment.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -674,7 +674,7 @@ func resourceEnvironmentRead(ctx context.Context, d *schema.ResourceData, meta i
return diag.Errorf("could not get template: %v", err)
}

if err := templateRead("without_template_settings", template, d, false); err != nil {
if err := templateRead("without_template_settings", template, d); err != nil {
return diag.Errorf("schema resource data serialization failed: %v", err)
}
}
Expand Down Expand Up @@ -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 {
Copy link
Contributor

Choose a reason for hiding this comment

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

can explain this file changes?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

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.

return nil, fmt.Errorf("failed to write template to schema: %w", err)
}
}
Expand Down
8 changes: 0 additions & 8 deletions env0/resource_environment_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2736,11 +2736,6 @@ func TestUnitEnvironmentWithoutTemplateResource(t *testing.T) {
terragruntVersion = "terragrunt_version = \"" + template.TerragruntVersion + "\""
}

terragruntTfBinary := ""
if template.TerragruntTfBinary != "" {
terragruntTfBinary = "terragrunt_tf_binary = \"" + template.TerragruntTfBinary + "\""
}

openTofuVersion := ""
if template.OpentofuVersion != "" {
openTofuVersion = "opentofu_version = \"" + template.OpentofuVersion + "\""
Expand Down Expand Up @@ -2769,7 +2764,6 @@ func TestUnitEnvironmentWithoutTemplateResource(t *testing.T) {
github_installation_id = %d
%s
%s
%s
}
}`,
resourceType, resourceName,
Expand All @@ -2790,7 +2784,6 @@ func TestUnitEnvironmentWithoutTemplateResource(t *testing.T) {
template.Description,
template.GithubInstallationId,
terragruntVersion,
terragruntTfBinary,
openTofuVersion,
)
}
Expand Down Expand Up @@ -2837,7 +2830,6 @@ func TestUnitEnvironmentWithoutTemplateResource(t *testing.T) {
resource.TestCheckResourceAttr(accessor, "without_template_settings.0.type", updatedTemplate.Type),
resource.TestCheckResourceAttr(accessor, "without_template_settings.0.path", updatedTemplate.Path),
resource.TestCheckResourceAttr(accessor, "without_template_settings.0.terragrunt_version", updatedTemplate.TerragruntVersion),
resource.TestCheckResourceAttr(accessor, "without_template_settings.0.terragrunt_tf_binary", updatedTemplate.TerragruntTfBinary),
resource.TestCheckResourceAttr(accessor, "without_template_settings.0.revision", updatedTemplate.Revision),
resource.TestCheckResourceAttr(accessor, "without_template_settings.0.retries_on_deploy", strconv.Itoa(updatedTemplate.Retry.OnDeploy.Times)),
resource.TestCheckResourceAttr(accessor, "without_template_settings.0.retry_on_deploy_only_when_matches_regex", updatedTemplate.Retry.OnDeploy.ErrorRegex),
Expand Down
48 changes: 10 additions & 38 deletions env0/resource_template.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ func getTemplateSchema(prefix string) map[string]*schema.Schema {
"terragrunt_tf_binary": {
Type: schema.TypeString,
Optional: true,
Description: "the binary to use if the template type is 'terragrunt'. Valid values 'opentofu' and 'terraform'. For new templates defaults to 'opentofu'",
Description: "the binary to use if the template type is 'terragrunt'. Valid values 'opentofu' and 'terraform'. Defaults to 'opentofu'",
ValidateDiagFunc: NewStringInValidator([]string{client.OPENTOFU, client.TERRAFORM}),
},
"token_name": {
Expand Down Expand Up @@ -324,7 +324,7 @@ func resourceTemplateRead(ctx context.Context, d *schema.ResourceData, meta inte
return nil
}

if err := templateRead("", template, d, false); err != nil {
if err := templateRead("", template, d); err != nil {
return diag.Errorf("%v", err)
}

Expand Down Expand Up @@ -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()
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this was the code to handle only for new resources... this is the breaking change...


terragruntTfBinaryKey := "terragrunt_tf_binary"
templateTypeKey := "type"

if prefix != "" {
terragruntTfBinaryKey = prefix + "." + terragruntTfBinaryKey
templateTypeKey = prefix + "." + templateTypeKey
}

if templateType, ok := d.GetOk(templateTypeKey); ok {
// If the user has set a value - use it.
if terragruntTfBinary := d.Get(terragruntTfBinaryKey).(string); terragruntTfBinary != "" {
payload.TerragruntTfBinary = terragruntTfBinary
} else if templateType.(string) == client.TERRAGRUNT && isNew {
payload.TerragruntTfBinary = client.OPENTOFU
}
}

templateCreatePayloadRetryOnHelper(prefix, d, "deploy", &payload.Retry.OnDeploy)
templateCreatePayloadRetryOnHelper(prefix, d, "destroy", &payload.Retry.OnDestroy)

Expand All @@ -440,33 +421,24 @@ func templateCreatePayloadFromParameters(prefix string, d *schema.ResourceData)
}

// Reads template and writes to the resource data.
func templateRead(prefix string, template client.Template, d *schema.ResourceData, isImport bool) error {
func templateRead(prefix string, template client.Template, d *schema.ResourceData) error {
pathPrefix := "path"
terragruntTfBinaryPrefix := "terragrunt_tf_binary"
terraformVersionPrefix := "terraform_version"

if prefix != "" {
pathPrefix = prefix + ".0." + pathPrefix
terragruntTfBinaryPrefix = prefix + ".0." + terragruntTfBinaryPrefix
terraformVersionPrefix = prefix + ".0." + terraformVersionPrefix
pathPrefix = prefix + ".0." + pathPrefix
}

path, pathOk := d.GetOk(pathPrefix)
terragruntTfBinary := d.Get(terragruntTfBinaryPrefix).(string)
terraformVersion := d.Get(terraformVersionPrefix).(string)

// If this value isn't set, ignore whatever is returned from the response.
// This helps avoid drifts when defaulting to 'opentofu' for new 'terragrunt' templates, and 'terraform' for existing 'terragrunt' templates.
// 'template.TerragruntTfBinary' field is set to 'omitempty'. Therefore, the state isn't modified if `template.TerragruntTfBinary` is an empty string.
// This is not true for imports - because the shcema is empty irrespective in that case.
if !isImport {
if terragruntTfBinary == "" {

// 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 {
Comment on lines +435 to +437
Copy link
Contributor

Choose a reason for hiding this comment

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

  1. But this PR will do a breaking change on purpose, so why do we need that?
  2. 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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

terraform -> null

isn't it '' -> 'terraform'? I didnt get this part

and thanks for the explanation

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yeah - I think you're right. Just flip the examples.

terragruntTfBinary, terragruntTfBinaryOk := d.GetOk(terragruntTfBinaryPrefix)
if !terragruntTfBinaryOk || terragruntTfBinary.(string) == "" {
template.TerragruntTfBinary = ""
}
// Same explanation as above.
if terraformVersion == "" {
template.TerraformVersion = ""
}
}

if err := writeResourceDataEx(prefix, &template, d); err != nil {
Expand Down
92 changes: 43 additions & 49 deletions env0/resource_template_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func TestUnitTemplateResource(t *testing.T) {
IsGitlabEnterprise: true,
TerraformVersion: "0.12.24",
TerragruntVersion: "0.35.1",
TerragruntTfBinary: "opentofu",
TerragruntTfBinary: "terraform",
}
gleeUpdatedTemplate := client.Template{
Id: gleeTemplate.Id,
Expand All @@ -67,6 +67,7 @@ func TestUnitTemplateResource(t *testing.T) {
IsGitlabEnterprise: true,
TerraformVersion: "0.15.1",
IsTerragruntRunAll: true,
TerragruntTfBinary: "terraform",
}
gitlabTemplate := client.Template{
Id: "id0-gitlab",
Expand Down Expand Up @@ -108,11 +109,12 @@ func TestUnitTemplateResource(t *testing.T) {
ErrorRegex: "NewForDestroy.*",
},
},
Type: "terragrunt",
TerragruntVersion: "0.26.1",
TokenId: "2",
TerraformVersion: "0.15.1",
TokenName: "token_name2",
Type: "terragrunt",
TerragruntVersion: "0.26.1",
TokenId: "2",
TerraformVersion: "0.15.1",
TokenName: "token_name2",
TerragruntTfBinary: "terraform",
}
githubTemplate := client.Template{
Id: "id0",
Expand Down Expand Up @@ -157,6 +159,7 @@ func TestUnitTemplateResource(t *testing.T) {
GithubInstallationId: 2,
TerraformVersion: "0.15.1",
IsTerragruntRunAll: true,
TerragruntTfBinary: "terraform",
}
bitbucketTemplate := client.Template{
Id: "id0",
Expand Down Expand Up @@ -200,6 +203,7 @@ func TestUnitTemplateResource(t *testing.T) {
BitbucketClientKey: "clientkey2",
TerragruntVersion: "0.35.1",
TerraformVersion: "0.15.1",
TerragruntTfBinary: "terraform",
}
gheeTemplate := client.Template{
Id: "id0",
Expand Down Expand Up @@ -366,11 +370,12 @@ func TestUnitTemplateResource(t *testing.T) {
ErrorRegex: "NewForDestroy.*",
},
},
Type: "terragrunt",
TerragruntVersion: "0.26.1",
TokenId: "2",
TerraformVersion: "0.15.1",
IsAzureDevOps: true,
Type: "terragrunt",
TerragruntVersion: "0.26.1",
TokenId: "2",
TerraformVersion: "0.15.1",
TerragruntTfBinary: "terraform",
IsAzureDevOps: true,
}

helmTemplate := client.Template{
Expand Down Expand Up @@ -594,6 +599,10 @@ func TestUnitTemplateResource(t *testing.T) {
templateAsDictionary["ansible_version"] = template.AnsibleVersion
}

if template.TerragruntTfBinary != "" {
templateAsDictionary["terragrunt_tf_binary"] = template.TerragruntTfBinary
}

return resourceConfigCreate(resourceType, resourceName, templateAsDictionary)
}
fullTemplateResourceCheck := func(resourceFullName string, template client.Template) resource.TestCheckFunc {
Expand Down Expand Up @@ -729,31 +738,34 @@ func TestUnitTemplateResource(t *testing.T) {
GithubInstallationId: templateUseCase.updatedTemplate.GithubInstallationId,
IsGitlabEnterprise: templateUseCase.updatedTemplate.IsGitlabEnterprise,
IsGitlab: templateUseCase.updatedTemplate.IsGitlab,

TokenId: templateUseCase.updatedTemplate.TokenId,
Path: templateUseCase.updatedTemplate.Path,
Revision: templateUseCase.updatedTemplate.Revision,
Type: templateUseCase.updatedTemplate.Type,
Retry: templateUseCase.updatedTemplate.Retry,
TerraformVersion: templateUseCase.updatedTemplate.TerraformVersion,
BitbucketClientKey: templateUseCase.updatedTemplate.BitbucketClientKey,
IsGithubEnterprise: templateUseCase.updatedTemplate.IsGithubEnterprise,
IsBitbucketServer: templateUseCase.updatedTemplate.IsBitbucketServer,
FileName: templateUseCase.updatedTemplate.FileName,
TerragruntVersion: templateUseCase.updatedTemplate.TerragruntVersion,
IsTerragruntRunAll: templateUseCase.updatedTemplate.IsTerragruntRunAll,
IsAzureDevOps: templateUseCase.updatedTemplate.IsAzureDevOps,
IsHelmRepository: templateUseCase.updatedTemplate.IsHelmRepository,
HelmChartName: templateUseCase.updatedTemplate.HelmChartName,
OpentofuVersion: templateUseCase.updatedTemplate.OpentofuVersion,
TokenName: templateUseCase.updatedTemplate.TokenName,
AnsibleVersion: templateUseCase.updatedTemplate.AnsibleVersion,
TokenId: templateUseCase.updatedTemplate.TokenId,
Path: templateUseCase.updatedTemplate.Path,
Revision: templateUseCase.updatedTemplate.Revision,
Type: templateUseCase.updatedTemplate.Type,
Retry: templateUseCase.updatedTemplate.Retry,
TerraformVersion: templateUseCase.updatedTemplate.TerraformVersion,
BitbucketClientKey: templateUseCase.updatedTemplate.BitbucketClientKey,
IsGithubEnterprise: templateUseCase.updatedTemplate.IsGithubEnterprise,
IsBitbucketServer: templateUseCase.updatedTemplate.IsBitbucketServer,
FileName: templateUseCase.updatedTemplate.FileName,
TerragruntVersion: templateUseCase.updatedTemplate.TerragruntVersion,
IsTerragruntRunAll: templateUseCase.updatedTemplate.IsTerragruntRunAll,
IsAzureDevOps: templateUseCase.updatedTemplate.IsAzureDevOps,
IsHelmRepository: templateUseCase.updatedTemplate.IsHelmRepository,
HelmChartName: templateUseCase.updatedTemplate.HelmChartName,
OpentofuVersion: templateUseCase.updatedTemplate.OpentofuVersion,
TokenName: templateUseCase.updatedTemplate.TokenName,
AnsibleVersion: templateUseCase.updatedTemplate.AnsibleVersion,
}

if templateUseCase.template.Type == "terragrunt" {
templateCreatePayload.TerragruntTfBinary = templateUseCase.template.TerragruntTfBinary
}

if templateUseCase.updatedTemplate.Type == "terragrunt" {
updateTemplateCreateTemplate.TerragruntTfBinary = templateUseCase.updatedTemplate.TerragruntTfBinary
}

if templateUseCase.template.Type != "terraform" && templateUseCase.template.Type != "terragrunt" {
templateCreatePayload.TerraformVersion = ""
updateTemplateCreateTemplate.TerraformVersion = ""
Expand Down Expand Up @@ -1366,6 +1378,7 @@ func TestUnitTemplateResource(t *testing.T) {
"type": "terragrunt",
"terraform_version": "0.15.1",
"terragrunt_version": "0.27.50",
"terragrunt_tf_binary": "terraform",
"is_terragrunt_run_all": "true",
}),
ExpectError: regexp.MustCompile(`can't set is_terragrunt_run_all to 'true' for terragrunt versions lower than 0.28.1`),
Expand All @@ -1376,25 +1389,6 @@ func TestUnitTemplateResource(t *testing.T) {
runUnitTest(t, testCase, func(mock *client.MockApiClientInterface) {})
})

t.Run("terragrunt_tf_binary set with a non terragrunt template type", func(t *testing.T) {
razbensimon marked this conversation as resolved.
Show resolved Hide resolved
testCase := resource.TestCase{
Steps: []resource.TestStep{
{
Config: resourceConfigCreate(resourceType, resourceName, map[string]interface{}{
"name": "template0",
"repository": "env0/repo",
"type": "terraform",
"terraform_version": "0.15.1",
"terragrunt_tf_binary": "opentofu",
}),
ExpectError: regexp.MustCompile(`terragrunt_tf_binary should only be used when the template type is 'terragrunt', but type is 'terraform'`),
},
},
}

runUnitTest(t, testCase, func(mock *client.MockApiClientInterface) {})
})

t.Run("invalid ansible version", func(t *testing.T) {
testCase := resource.TestCase{
Steps: []resource.TestStep{
Expand Down
Loading