diff --git a/CHANGELOG.md b/CHANGELOG.md index 437e91e3e..2285c951c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,9 @@ ENHANCEMENTS: * `d/tfe_workspace`: Add an `auto_destroy_at` attribute for reading a scheduled auto-destroy, by @notchairmk [1354](https://github.com/hashicorp/terraform-provider-tfe/pull/1354) * `r/tfe_registry_module`: Add `initial_version` support for Branch Based Modules by @aaabdelgany [#1363](https://github.com/hashicorp/terraform-provider-tfe/pull/1363) +BUG FIXES: +* `r/tfe_registry_module`: Prevents constant diff after a successful apply when `tags` and `tests_enabled` is not set by @Uk1288 [#1357](https://github.com/hashicorp/terraform-provider-tfe/pull/1357) + ## v0.55.0 FEATURES: diff --git a/internal/provider/resource_tfe_registry_module.go b/internal/provider/resource_tfe_registry_module.go index f9b7c1db2..7a1f3805f 100644 --- a/internal/provider/resource_tfe_registry_module.go +++ b/internal/provider/resource_tfe_registry_module.go @@ -32,6 +32,9 @@ func resourceTFERegistryModule() *schema.Resource { StateContext: resourceTFERegistryModuleImporter, }, + CustomizeDiff: func(c context.Context, d *schema.ResourceDiff, meta interface{}) error { + return validateVcsRepo(d) + }, Schema: map[string]*schema.Schema{ "organization": { Type: schema.TypeString, @@ -95,6 +98,7 @@ func resourceTFERegistryModule() *schema.Resource { "tags": { Type: schema.TypeBool, Optional: true, + Computed: true, }, }, }, @@ -125,6 +129,7 @@ func resourceTFERegistryModule() *schema.Resource { "test_config": { Type: schema.TypeList, Optional: true, + Computed: true, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "tests_enabled": { @@ -173,8 +178,8 @@ func resourceTFERegistryModuleCreateWithVCS(v interface{}, meta interface{}, d * branch, branchOk := vcsRepo["branch"].(string) initialVersion, initialVersionOk := d.GetOk("initial_version") - if tagsOk && tags && branchOk && branch != "" { - return nil, fmt.Errorf("tags must be set to false when a branch is provided") + if tagsOk { + options.VCSRepo.Tags = tfe.Bool(tags) } if branchOk && branch != "" { @@ -312,10 +317,6 @@ func resourceTFERegistryModuleUpdate(d *schema.ResourceData, meta interface{}) e tags, tagsOk := vcsRepo["tags"].(bool) branch, branchOk := vcsRepo["branch"].(string) - if tagsOk && tags && branchOk && branch != "" { - return fmt.Errorf("tags must be set to false when a branch is provided") - } - if tagsOk { options.VCSRepo.Tags = tfe.Bool(tags) } @@ -348,7 +349,7 @@ func resourceTFERegistryModuleUpdate(d *schema.ResourceData, meta interface{}) e }) if err != nil { - return fmt.Errorf("Error while waiting for module %s/%s to be updated: %w", registryModule.Organization.Name, registryModule.Name, err) + return fmt.Errorf("Error while waiting for module %s/%s to be updated: %w", rmID.Organization, rmID.Name, err) } d.SetId(registryModule.ID) @@ -413,10 +414,10 @@ func resourceTFERegistryModuleRead(d *schema.ResourceData, meta interface{}) err } testConfig = append(testConfig, testConfigValues) - - d.Set("test_config", testConfig) } + d.Set("test_config", testConfig) + return nil } @@ -481,3 +482,30 @@ func resourceTFERegistryModuleImporter(ctx context.Context, d *schema.ResourceDa d.Id(), ) } + +func validateVcsRepo(d *schema.ResourceDiff) error { + vcsRepo, ok := d.GetRawConfig().AsValueMap()["vcs_repo"] + if !ok || vcsRepo.LengthInt() == 0 { + return nil + } + + branchValue := vcsRepo.AsValueSlice()[0].GetAttr("branch") + tagsValue := vcsRepo.AsValueSlice()[0].GetAttr("tags") + + if !tagsValue.IsNull() && tagsValue.False() && branchValue.IsNull() { + return fmt.Errorf("branch must be provided when tags is set to false") + } + + if !tagsValue.IsNull() && !branchValue.IsNull() { + tags := tagsValue.True() + branch := branchValue.AsString() + // tags must be set to true or branch provided but not both + if tags && branch != "" { + return fmt.Errorf("tags must be set to false when a branch is provided") + } else if !tags && branch == "" { + return fmt.Errorf("tags must be set to true when no branch is provided") + } + } + + return nil +} diff --git a/internal/provider/resource_tfe_registry_module_test.go b/internal/provider/resource_tfe_registry_module_test.go index b5aa7babe..c11e6f462 100644 --- a/internal/provider/resource_tfe_registry_module_test.go +++ b/internal/provider/resource_tfe_registry_module_test.go @@ -17,7 +17,7 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" ) -func TestAccTFERegistryModule_vcs(t *testing.T) { +func TestAccTFERegistryModule_vcsBasic(t *testing.T) { registryModule := &tfe.RegistryModule{} rInt := rand.New(rand.NewSource(time.Now().UnixNano())).Int() orgName := fmt.Sprintf("tst-terraform-%d", rInt) @@ -38,7 +38,7 @@ func TestAccTFERegistryModule_vcs(t *testing.T) { CheckDestroy: testAccCheckTFERegistryModuleDestroy, Steps: []resource.TestStep{ { - Config: testAccTFERegistryModule_vcs(rInt), + Config: testAccTFERegistryModule_vcsBasic(rInt), Check: resource.ComposeTestCheckFunc( testAccCheckTFERegistryModuleExists( "tfe_registry_module.foobar", @@ -300,6 +300,54 @@ func TestAccTFERegistryModule_publicRegistryModule(t *testing.T) { }) } +func TestAccTFERegistryModule_branchOnly(t *testing.T) { + skipUnlessBeta(t) + rInt := rand.New(rand.NewSource(time.Now().UnixNano())).Int() + + resource.Test(t, resource.TestCase{ + PreCheck: func() { + testAccPreCheck(t) + testAccPreCheckTFERegistryModule(t) + }, + Providers: testAccProviders, + Steps: []resource.TestStep{ + { + Config: testAccTFERegistryModule_branchOnly(rInt), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("tfe_registry_module.foobar", "publishing_mechanism", "branch"), + resource.TestCheckResourceAttr("tfe_registry_module.foobar", "test_config.0.tests_enabled", strconv.FormatBool(false)), + resource.TestCheckResourceAttr("tfe_registry_module.foobar", "vcs_repo.0.tags", strconv.FormatBool(false)), + resource.TestCheckResourceAttr("tfe_registry_module.foobar", "vcs_repo.0.branch", "main"), + ), + }, + }, + }) +} + +func TestAccTFERegistryModule_vcsRepoWithTags(t *testing.T) { + skipUnlessBeta(t) + rInt := rand.New(rand.NewSource(time.Now().UnixNano())).Int() + + resource.Test(t, resource.TestCase{ + PreCheck: func() { + testAccPreCheck(t) + testAccPreCheckTFERegistryModule(t) + }, + Providers: testAccProviders, + Steps: []resource.TestStep{ + { + Config: testAccTFERegistryModule_vcsRepoWithFalseTags(rInt), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("tfe_registry_module.foobar", "publishing_mechanism", "branch"), + resource.TestCheckResourceAttr("tfe_registry_module.foobar", "test_config.0.tests_enabled", strconv.FormatBool(false)), + resource.TestCheckResourceAttr("tfe_registry_module.foobar", "vcs_repo.0.tags", strconv.FormatBool(false)), + resource.TestCheckResourceAttr("tfe_registry_module.foobar", "vcs_repo.0.branch", "main"), + ), + }, + }, + }) +} + func TestAccTFERegistryModule_noCodeModule(t *testing.T) { skipIfEnterprise(t) @@ -366,7 +414,7 @@ func TestAccTFERegistryModuleImport_vcsPrivateRMDeprecatedFormat(t *testing.T) { CheckDestroy: testAccCheckTFERegistryModuleDestroy, Steps: []resource.TestStep{ { - Config: testAccTFERegistryModule_vcs(rInt), + Config: testAccTFERegistryModule_vcsWithTagsTrue(rInt), }, { ResourceName: "tfe_registry_module.foobar", @@ -390,7 +438,7 @@ func TestAccTFERegistryModuleImport_vcsPrivateRMRecommendedFormat(t *testing.T) CheckDestroy: testAccCheckTFERegistryModuleDestroy, Steps: []resource.TestStep{ { - Config: testAccTFERegistryModule_vcs(rInt), + Config: testAccTFERegistryModule_vcsWithTagsTrue(rInt), }, { ResourceName: "tfe_registry_module.foobar", @@ -461,6 +509,30 @@ func TestAccTFERegistryModuleImport_vcsPublishingMechanismBranchToTagsToBranch(t }) } +func TestAccTFERegistryModule_branchOnlyEmpty(t *testing.T) { + skipUnlessBeta(t) + rInt := rand.New(rand.NewSource(time.Now().UnixNano())).Int() + + resource.Test(t, resource.TestCase{ + PreCheck: func() { + testAccPreCheck(t) + testAccPreCheckTFERegistryModule(t) + }, + Providers: testAccProviders, + Steps: []resource.TestStep{ + { + Config: testAccTFERegistryModule_branchOnlyEmpty(rInt), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("tfe_registry_module.foobar", "publishing_mechanism", "git_tag"), + resource.TestCheckNoResourceAttr("tfe_registry_module.foobar", "test_config.0"), + resource.TestCheckResourceAttr("tfe_registry_module.foobar", "vcs_repo.0.tags", strconv.FormatBool(true)), + resource.TestCheckResourceAttr("tfe_registry_module.foobar", "vcs_repo.0.branch", ""), + ), + }, + }, + }) +} + func TestAccTFERegistryModuleImport_vcsPublishingMechanismBranchToTagsToBranchWithTests(t *testing.T) { rInt := rand.New(rand.NewSource(time.Now().UnixNano())).Int() @@ -597,6 +669,25 @@ func TestAccTFERegistryModule_invalidTestConfigOnUpdate(t *testing.T) { }) } +func TestAccTFERegistryModule_vcsTagsOnlyFalse(t *testing.T) { + skipUnlessBeta(t) + rInt := rand.New(rand.NewSource(time.Now().UnixNano())).Int() + + resource.Test(t, resource.TestCase{ + PreCheck: func() { + testAccPreCheck(t) + testAccPreCheckTFERegistryModule(t) + }, + Providers: testAccProviders, + Steps: []resource.TestStep{ + { + Config: testAccTFERegistryModule_vcsTagsOnlyFalse(rInt), + ExpectError: regexp.MustCompile(`branch must be provided when tags is set to false`), + }, + }, + }) +} + func TestAccTFERegistryModule_branchAndInvalidTagsOnCreate(t *testing.T) { rInt := rand.New(rand.NewSource(time.Now().UnixNano())).Int() @@ -633,6 +724,25 @@ func TestAccTFERegistryModule_branchAndTagsEnabledOnCreate(t *testing.T) { }) } +func TestAccTFERegistryModule_branchAndTagsDisabledOnCreate(t *testing.T) { + skipUnlessBeta(t) + rInt := rand.New(rand.NewSource(time.Now().UnixNano())).Int() + + resource.Test(t, resource.TestCase{ + PreCheck: func() { + testAccPreCheck(t) + testAccPreCheckTFERegistryModule(t) + }, + Providers: testAccProviders, + Steps: []resource.TestStep{ + { + Config: testAccTFERegistryModule_vcsWithBranchAndTagsDisabled(rInt), + ExpectError: regexp.MustCompile(`tags must be set to true when no branch is provided`), + }, + }, + }) +} + func TestAccTFERegistryModule_branchAndTagsEnabledOnUpdate(t *testing.T) { rInt := rand.New(rand.NewSource(time.Now().UnixNano())).Int() @@ -660,6 +770,34 @@ func TestAccTFERegistryModule_branchAndTagsEnabledOnUpdate(t *testing.T) { }) } +func TestAccTFERegistryModule_branchAndTagsDisabledOnUpdate(t *testing.T) { + skipUnlessBeta(t) + rInt := rand.New(rand.NewSource(time.Now().UnixNano())).Int() + + resource.Test(t, resource.TestCase{ + PreCheck: func() { + testAccPreCheck(t) + testAccPreCheckTFERegistryModule(t) + }, + Providers: testAccProviders, + Steps: []resource.TestStep{ + { + Config: testAccTFERegistryModule_vcsTags(rInt), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("tfe_registry_module.foobar", "publishing_mechanism", "git_tag"), + resource.TestCheckNoResourceAttr("tfe_registry_module.foobar", "test_config.0.tests_enabled"), + resource.TestCheckResourceAttr("tfe_registry_module.foobar", "vcs_repo.0.tags", strconv.FormatBool(true)), + resource.TestCheckResourceAttr("tfe_registry_module.foobar", "vcs_repo.0.branch", ""), + ), + }, + { + Config: testAccTFERegistryModule_vcsWithBranchAndTagsDisabled(rInt), + ExpectError: regexp.MustCompile(`tags must be set to true when no branch is provided`), + }, + }, + }) +} + func TestAccTFERegistryModuleImport_nonVCSPrivateRM(t *testing.T) { rInt := rand.New(rand.NewSource(time.Now().UnixNano())).Int() @@ -964,7 +1102,7 @@ func getRegistryModuleProvider() string { return strings.SplitN(getRegistryModuleRepository(), "-", 3)[1] } -func testAccTFERegistryModule_vcs(rInt int) string { +func testAccTFERegistryModule_vcsWithTagsTrue(rInt int) string { return fmt.Sprintf(` resource "tfe_organization" "foobar" { name = "tst-terraform-%d" @@ -1175,6 +1313,156 @@ resource "tfe_registry_module" "foobar" { envGithubRegistryModuleIdentifer) } +func testAccTFERegistryModule_vcsWithBranchAndTagsDisabled(rInt int) string { + return fmt.Sprintf(` +resource "tfe_organization" "foobar" { + name = "tst-terraform-%d" + email = "admin@company.com" +} + +resource "tfe_oauth_client" "foobar" { + organization = tfe_organization.foobar.name + api_url = "https://api.github.com" + http_url = "https://github.com" + oauth_token = "%s" + service_provider = "github" +} + +resource "tfe_registry_module" "foobar" { + organization = tfe_organization.foobar.name + vcs_repo { + display_identifier = "%s" + identifier = "%s" + oauth_token_id = tfe_oauth_client.foobar.oauth_token_id + branch = "" + tags = false + } +}`, + rInt, + envGithubToken, + envGithubRegistryModuleIdentifer, + envGithubRegistryModuleIdentifer) +} + +func testAccTFERegistryModule_vcsBasic(rInt int) string { + return fmt.Sprintf(` +resource "tfe_organization" "foobar" { + name = "tst-terraform-%d" + email = "admin@company.com" +} + +resource "tfe_oauth_client" "foobar" { + organization = tfe_organization.foobar.name + api_url = "https://api.github.com" + http_url = "https://github.com" + oauth_token = "%s" + service_provider = "github" +} + +resource "tfe_registry_module" "foobar" { + organization = tfe_organization.foobar.name + vcs_repo { + display_identifier = "%s" + identifier = "%s" + oauth_token_id = tfe_oauth_client.foobar.oauth_token_id + } +}`, + rInt, + envGithubToken, + envGithubRegistryModuleIdentifer, + envGithubRegistryModuleIdentifer) +} + +func testAccTFERegistryModule_vcsTagsOnlyFalse(rInt int) string { + return fmt.Sprintf(` +resource "tfe_organization" "foobar" { + name = "tst-terraform-%d" + email = "admin@company.com" +} + +resource "tfe_oauth_client" "foobar" { + organization = tfe_organization.foobar.name + api_url = "https://api.github.com" + http_url = "https://github.com" + oauth_token = "%s" + service_provider = "github" +} + +resource "tfe_registry_module" "foobar" { + organization = tfe_organization.foobar.name + vcs_repo { + display_identifier = "%s" + identifier = "%s" + oauth_token_id = tfe_oauth_client.foobar.oauth_token_id + tags = false + } +}`, + rInt, + envGithubToken, + envGithubRegistryModuleIdentifer, + envGithubRegistryModuleIdentifer) +} + +func testAccTFERegistryModule_branchOnly(rInt int) string { + return fmt.Sprintf(` +resource "tfe_organization" "foobar" { + name = "tst-terraform-%d" + email = "admin@company.com" +} + +resource "tfe_oauth_client" "foobar" { + organization = tfe_organization.foobar.name + api_url = "https://api.github.com" + http_url = "https://github.com" + oauth_token = "%s" + service_provider = "github" +} + +resource "tfe_registry_module" "foobar" { + organization = tfe_organization.foobar.name + vcs_repo { + display_identifier = "%s" + identifier = "%s" + oauth_token_id = tfe_oauth_client.foobar.oauth_token_id + branch = "main" + } +}`, + rInt, + envGithubToken, + envGithubRegistryModuleIdentifer, + envGithubRegistryModuleIdentifer) +} + +func testAccTFERegistryModule_branchOnlyEmpty(rInt int) string { + return fmt.Sprintf(` +resource "tfe_organization" "foobar" { + name = "tst-terraform-%d" + email = "admin@company.com" +} + +resource "tfe_oauth_client" "foobar" { + organization = tfe_organization.foobar.name + api_url = "https://api.github.com" + http_url = "https://github.com" + oauth_token = "%s" + service_provider = "github" +} + +resource "tfe_registry_module" "foobar" { + organization = tfe_organization.foobar.name + vcs_repo { + display_identifier = "%s" + identifier = "%s" + oauth_token_id = tfe_oauth_client.foobar.oauth_token_id + branch = "" + } +}`, + rInt, + envGithubToken, + envGithubRegistryModuleIdentifer, + envGithubRegistryModuleIdentifer) +} + func testAccTFERegistryModule_vcsTags(rInt int) string { return fmt.Sprintf(` resource "tfe_organization" "foobar" { @@ -1205,6 +1493,36 @@ resource "tfe_registry_module" "foobar" { envGithubRegistryModuleIdentifer, envGithubRegistryModuleIdentifer) } +func testAccTFERegistryModule_vcsRepoWithFalseTags(rInt int) string { + return fmt.Sprintf(` +resource "tfe_organization" "foobar" { + name = "tst-terraform-%d" + email = "admin@company.com" +} + +resource "tfe_oauth_client" "foobar" { + organization = tfe_organization.foobar.name + api_url = "https://api.github.com" + http_url = "https://github.com" + oauth_token = "%s" + service_provider = "github" +} + +resource "tfe_registry_module" "foobar" { + organization = tfe_organization.foobar.name + vcs_repo { + display_identifier = "%s" + identifier = "%s" + oauth_token_id = tfe_oauth_client.foobar.oauth_token_id + branch = "main" + tags = false + } +}`, + rInt, + envGithubToken, + envGithubRegistryModuleIdentifer, + envGithubRegistryModuleIdentifer) +} func testAccTFERegistryModule_GitHubApp(rInt int) string { return fmt.Sprintf(`