From 7b03946f1352825bf8223e0a37d959c24a7a3106 Mon Sep 17 00:00:00 2001 From: Tomer Heber Date: Sat, 16 Nov 2024 10:11:22 -0600 Subject: [PATCH 1/4] Feat: Breaking Change - default terragrunt_tf_binary to 'opentofu' --- client/template.go | 27 ++++++--- env0/resource_environment.go | 4 +- env0/resource_environment_test.go | 8 --- env0/resource_template.go | 48 ++++------------ env0/resource_template_test.go | 92 +++++++++++++++---------------- 5 files changed, 75 insertions(+), 104 deletions(-) diff --git a/client/template.go b/client/template.go index 704d0202..49aceb20 100644 --- a/client/template.go +++ b/client/template.go @@ -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"` + TerragruntTfBinary string `json:"terragruntTfBinary"` AnsibleVersion string `json:"ansibleVersion,omitempty"` } @@ -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 == "" { + return errors.New("must supply opentofu version") + } + + if payload.TerragruntTfBinary == TERRAFORM && payload.TerraformVersion == "" { + return errors.New("must supply terraform version") + } + } else { + payload.TerragruntTfBinary = "" } if payload.Type == OPENTOFU && payload.OpentofuVersion == "" { return errors.New("must supply opentofu version") } - if payload.TerragruntTfBinary != "" && payload.Type != TERRAGRUNT { - 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`) diff --git a/env0/resource_environment.go b/env0/resource_environment.go index 60511e9a..dc33e545 100644 --- a/env0/resource_environment.go +++ b/env0/resource_environment.go @@ -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) } } @@ -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 { return nil, fmt.Errorf("failed to write template to schema: %w", err) } } diff --git a/env0/resource_environment_test.go b/env0/resource_environment_test.go index 7a71a1e0..3c6122fe 100644 --- a/env0/resource_environment_test.go +++ b/env0/resource_environment_test.go @@ -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 + "\"" @@ -2769,7 +2764,6 @@ func TestUnitEnvironmentWithoutTemplateResource(t *testing.T) { github_installation_id = %d %s %s - %s } }`, resourceType, resourceName, @@ -2790,7 +2784,6 @@ func TestUnitEnvironmentWithoutTemplateResource(t *testing.T) { template.Description, template.GithubInstallationId, terragruntVersion, - terragruntTfBinary, openTofuVersion, ) } @@ -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), diff --git a/env0/resource_template.go b/env0/resource_template.go index e60e9aa3..775ebb26 100644 --- a/env0/resource_template.go +++ b/env0/resource_template.go @@ -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": { @@ -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) } @@ -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() - - 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) @@ -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 { + 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 { diff --git a/env0/resource_template_test.go b/env0/resource_template_test.go index 7afaa198..c6d584bc 100644 --- a/env0/resource_template_test.go +++ b/env0/resource_template_test.go @@ -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, @@ -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", @@ -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", @@ -157,6 +159,7 @@ func TestUnitTemplateResource(t *testing.T) { GithubInstallationId: 2, TerraformVersion: "0.15.1", IsTerragruntRunAll: true, + TerragruntTfBinary: "terraform", } bitbucketTemplate := client.Template{ Id: "id0", @@ -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", @@ -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{ @@ -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 { @@ -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 = "" @@ -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`), @@ -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) { - 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{ From 4692578dd48455819113c8f1822e745f922190c1 Mon Sep 17 00:00:00 2001 From: Tomer Heber Date: Sat, 16 Nov 2024 10:45:03 -0600 Subject: [PATCH 2/4] add omit empty --- client/template.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/template.go b/client/template.go index 49aceb20..2dbcd4c3 100644 --- a/client/template.go +++ b/client/template.go @@ -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"` + TerragruntTfBinary string `json:"terragruntTfBinary,omitempty"` AnsibleVersion string `json:"ansibleVersion,omitempty"` } From af8e8d77b9d5a6bb226a21a40f6af00d0dd234e0 Mon Sep 17 00:00:00 2001 From: Tomer Heber Date: Sat, 16 Nov 2024 11:01:11 -0600 Subject: [PATCH 3/4] added tests --- env0/resource_template_test.go | 39 ++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/env0/resource_template_test.go b/env0/resource_template_test.go index c6d584bc..a1e662f3 100644 --- a/env0/resource_template_test.go +++ b/env0/resource_template_test.go @@ -1389,6 +1389,45 @@ func TestUnitTemplateResource(t *testing.T) { runUnitTest(t, testCase, func(mock *client.MockApiClientInterface) {}) }) + t.Run("run with terragrunt without an opentofu version", func(t *testing.T) { + testCase := resource.TestCase{ + Steps: []resource.TestStep{ + { + Config: resourceConfigCreate(resourceType, resourceName, map[string]interface{}{ + "name": "template0", + "repository": "env0/repo", + "type": "terragrunt", + "terragrunt_version": "0.56.50", + "is_terragrunt_run_all": "true", + }), + ExpectError: regexp.MustCompile("must supply opentofu version"), + }, + }, + } + + runUnitTest(t, testCase, func(mock *client.MockApiClientInterface) {}) + }) + + t.Run("run terragrunt with terraform binary and no terraform version", func(t *testing.T) { + testCase := resource.TestCase{ + Steps: []resource.TestStep{ + { + Config: resourceConfigCreate(resourceType, resourceName, map[string]interface{}{ + "name": "template0", + "repository": "env0/repo", + "type": "terragrunt", + "terragrunt_version": "0.56.50", + "is_terragrunt_run_all": "true", + "terragrunt_tf_binary": "terraform", + }), + ExpectError: regexp.MustCompile("must supply terraform version"), + }, + }, + } + + runUnitTest(t, testCase, func(mock *client.MockApiClientInterface) {}) + }) + t.Run("invalid ansible version", func(t *testing.T) { testCase := resource.TestCase{ Steps: []resource.TestStep{ From 6abd865082dad28d3a9787dd791a3a9e4747cbfb Mon Sep 17 00:00:00 2001 From: Tomer Heber Date: Sun, 17 Nov 2024 09:23:56 -0600 Subject: [PATCH 4/4] update bad input use-cases --- client/template.go | 12 +++++++----- env0/resource_template_test.go | 19 +++++++++++++++++++ 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/client/template.go b/client/template.go index 2dbcd4c3..c6d20a0a 100644 --- a/client/template.go +++ b/client/template.go @@ -127,10 +127,6 @@ func (payload *TemplateCreatePayload) Invalidate() error { return errors.New("must not specify organizationId") } - if payload.Type != TERRAGRUNT && payload.TerragruntVersion != "" { - return errors.New("can't define terragrunt version for non-terragrunt template") - } - if payload.Type == TERRAGRUNT { if payload.TerragruntVersion == "" { return errors.New("must supply terragrunt version") @@ -149,7 +145,13 @@ func (payload *TemplateCreatePayload) Invalidate() error { return errors.New("must supply terraform version") } } else { - payload.TerragruntTfBinary = "" + if payload.TerragruntVersion != "" { + return errors.New("can't define terragrunt version for non-terragrunt template") + } + + if payload.TerragruntTfBinary != "" { + return errors.New("can't define terragrunt_tf_binary for non-terragrunt template") + } } if payload.Type == OPENTOFU && payload.OpentofuVersion == "" { diff --git a/env0/resource_template_test.go b/env0/resource_template_test.go index a1e662f3..fbe84ee4 100644 --- a/env0/resource_template_test.go +++ b/env0/resource_template_test.go @@ -1389,6 +1389,25 @@ 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) { + 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(`can't define terragrunt_tf_binary for non-terragrunt template`), + }, + }, + } + + runUnitTest(t, testCase, func(mock *client.MockApiClientInterface) {}) + }) + t.Run("run with terragrunt without an opentofu version", func(t *testing.T) { testCase := resource.TestCase{ Steps: []resource.TestStep{