Skip to content

Commit 4ef15dd

Browse files
committed
fix tags and branch validations
1 parent 938707a commit 4ef15dd

File tree

2 files changed

+220
-25
lines changed

2 files changed

+220
-25
lines changed

internal/provider/resource_tfe_registry_module.go

Lines changed: 26 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,9 @@ func resourceTFERegistryModule() *schema.Resource {
3232
StateContext: resourceTFERegistryModuleImporter,
3333
},
3434

35+
CustomizeDiff: func(c context.Context, d *schema.ResourceDiff, meta interface{}) error {
36+
return validateVcsRepo(d)
37+
},
3538
Schema: map[string]*schema.Schema{
3639
"organization": {
3740
Type: schema.TypeString,
@@ -175,11 +178,6 @@ func resourceTFERegistryModuleCreateWithVCS(v interface{}, meta interface{}, d *
175178
branch, branchOk := vcsRepo["branch"].(string)
176179
initialVersion, initialVersionOk := d.GetOk("initial_version")
177180

178-
err = validateVcsRepo(tagsOk, tags, branchOk, branch)
179-
if err != nil {
180-
return nil, err
181-
}
182-
183181
if tagsOk {
184182
options.VCSRepo.Tags = tfe.Bool(tags)
185183
}
@@ -319,11 +317,6 @@ func resourceTFERegistryModuleUpdate(d *schema.ResourceData, meta interface{}) e
319317
tags, tagsOk := vcsRepo["tags"].(bool)
320318
branch, branchOk := vcsRepo["branch"].(string)
321319

322-
err = validateVcsRepo(tagsOk, tags, branchOk, branch)
323-
if err != nil {
324-
return err
325-
}
326-
327320
if tagsOk {
328321
options.VCSRepo.Tags = tfe.Bool(tags)
329322
}
@@ -356,7 +349,7 @@ func resourceTFERegistryModuleUpdate(d *schema.ResourceData, meta interface{}) e
356349
})
357350

358351
if err != nil {
359-
return fmt.Errorf("Error while waiting for module %s/%s to be updated: %w", registryModule.Organization.Name, registryModule.Name, err)
352+
return fmt.Errorf("Error while waiting for module %s/%s to be updated: %w", rmID.Organization, rmID.Name, err)
360353
}
361354

362355
d.SetId(registryModule.ID)
@@ -490,12 +483,28 @@ func resourceTFERegistryModuleImporter(ctx context.Context, d *schema.ResourceDa
490483
)
491484
}
492485

493-
func validateVcsRepo(tagsOk bool, tags bool, branchOk bool, branch string) error {
494-
// tags must be set to true or branch provided but not both
495-
if tagsOk && tags && branchOk && branch != "" {
496-
return fmt.Errorf("tags must be set to false when a branch is provided")
497-
} else if tagsOk && !tags && branchOk && branch == "" {
498-
return fmt.Errorf("tags must be set to true when no branch is provided")
486+
func validateVcsRepo(d *schema.ResourceDiff) error {
487+
vcsRepo, ok := d.GetRawConfig().AsValueMap()["vcs_repo"]
488+
if !ok || vcsRepo.LengthInt() == 0 {
489+
return nil
490+
}
491+
492+
branchValue := vcsRepo.AsValueSlice()[0].GetAttr("branch")
493+
tagsValue := vcsRepo.AsValueSlice()[0].GetAttr("tags")
494+
495+
if !tagsValue.IsNull() && tagsValue.False() && branchValue.IsNull() {
496+
return fmt.Errorf("branch must be provided when tags is set to false")
497+
}
498+
499+
if !tagsValue.IsNull() && !branchValue.IsNull() {
500+
tags := tagsValue.True()
501+
branch := branchValue.AsString()
502+
// tags must be set to true or branch provided but not both
503+
if tags && branch != "" {
504+
return fmt.Errorf("tags must be set to false when a branch is provided")
505+
} else if !tags && branch == "" {
506+
return fmt.Errorf("tags must be set to true when no branch is provided")
507+
}
499508
}
500509

501510
return nil

internal/provider/resource_tfe_registry_module_test.go

Lines changed: 194 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ import (
1717
"github.com/hashicorp/terraform-plugin-sdk/v2/terraform"
1818
)
1919

20-
func TestAccTFERegistryModule_vcs(t *testing.T) {
20+
func TestAccTFERegistryModule_vcsBasic(t *testing.T) {
2121
registryModule := &tfe.RegistryModule{}
2222
rInt := rand.New(rand.NewSource(time.Now().UnixNano())).Int()
2323
orgName := fmt.Sprintf("tst-terraform-%d", rInt)
@@ -38,7 +38,7 @@ func TestAccTFERegistryModule_vcs(t *testing.T) {
3838
CheckDestroy: testAccCheckTFERegistryModuleDestroy,
3939
Steps: []resource.TestStep{
4040
{
41-
Config: testAccTFERegistryModule_vcs(rInt),
41+
Config: testAccTFERegistryModule_vcsBasic(rInt),
4242
Check: resource.ComposeTestCheckFunc(
4343
testAccCheckTFERegistryModuleExists(
4444
"tfe_registry_module.foobar",
@@ -300,7 +300,7 @@ func TestAccTFERegistryModule_publicRegistryModule(t *testing.T) {
300300
})
301301
}
302302

303-
func TestAccTFERegistryModule_vcsRepoWithTagField(t *testing.T) {
303+
func TestAccTFERegistryModule_branchOnly(t *testing.T) {
304304
skipUnlessBeta(t)
305305
rInt := rand.New(rand.NewSource(time.Now().UnixNano())).Int()
306306

@@ -312,7 +312,31 @@ func TestAccTFERegistryModule_vcsRepoWithTagField(t *testing.T) {
312312
Providers: testAccProviders,
313313
Steps: []resource.TestStep{
314314
{
315-
Config: testAccTFERegistryModule_vcsRepoWithFalseTagField(rInt),
315+
Config: testAccTFERegistryModule_branchOnly(rInt),
316+
Check: resource.ComposeTestCheckFunc(
317+
resource.TestCheckResourceAttr("tfe_registry_module.foobar", "publishing_mechanism", "branch"),
318+
resource.TestCheckResourceAttr("tfe_registry_module.foobar", "test_config.0.tests_enabled", strconv.FormatBool(false)),
319+
resource.TestCheckResourceAttr("tfe_registry_module.foobar", "vcs_repo.0.tags", strconv.FormatBool(false)),
320+
resource.TestCheckResourceAttr("tfe_registry_module.foobar", "vcs_repo.0.branch", "main"),
321+
),
322+
},
323+
},
324+
})
325+
}
326+
327+
func TestAccTFERegistryModule_vcsRepoWithTags(t *testing.T) {
328+
skipUnlessBeta(t)
329+
rInt := rand.New(rand.NewSource(time.Now().UnixNano())).Int()
330+
331+
resource.Test(t, resource.TestCase{
332+
PreCheck: func() {
333+
testAccPreCheck(t)
334+
testAccPreCheckTFERegistryModule(t)
335+
},
336+
Providers: testAccProviders,
337+
Steps: []resource.TestStep{
338+
{
339+
Config: testAccTFERegistryModule_vcsRepoWithFalseTags(rInt),
316340
Check: resource.ComposeTestCheckFunc(
317341
resource.TestCheckResourceAttr("tfe_registry_module.foobar", "publishing_mechanism", "branch"),
318342
resource.TestCheckResourceAttr("tfe_registry_module.foobar", "test_config.0.tests_enabled", strconv.FormatBool(false)),
@@ -390,7 +414,7 @@ func TestAccTFERegistryModuleImport_vcsPrivateRMDeprecatedFormat(t *testing.T) {
390414
CheckDestroy: testAccCheckTFERegistryModuleDestroy,
391415
Steps: []resource.TestStep{
392416
{
393-
Config: testAccTFERegistryModule_vcs(rInt),
417+
Config: testAccTFERegistryModule_vcsWithTagsTrue(rInt),
394418
},
395419
{
396420
ResourceName: "tfe_registry_module.foobar",
@@ -414,7 +438,7 @@ func TestAccTFERegistryModuleImport_vcsPrivateRMRecommendedFormat(t *testing.T)
414438
CheckDestroy: testAccCheckTFERegistryModuleDestroy,
415439
Steps: []resource.TestStep{
416440
{
417-
Config: testAccTFERegistryModule_vcs(rInt),
441+
Config: testAccTFERegistryModule_vcsWithTagsTrue(rInt),
418442
},
419443
{
420444
ResourceName: "tfe_registry_module.foobar",
@@ -485,6 +509,30 @@ func TestAccTFERegistryModuleImport_vcsPublishingMechanismBranchToTagsToBranch(t
485509
})
486510
}
487511

512+
func TestAccTFERegistryModule_branchOnlyEmpty(t *testing.T) {
513+
skipUnlessBeta(t)
514+
rInt := rand.New(rand.NewSource(time.Now().UnixNano())).Int()
515+
516+
resource.Test(t, resource.TestCase{
517+
PreCheck: func() {
518+
testAccPreCheck(t)
519+
testAccPreCheckTFERegistryModule(t)
520+
},
521+
Providers: testAccProviders,
522+
Steps: []resource.TestStep{
523+
{
524+
Config: testAccTFERegistryModule_branchOnlyEmpty(rInt),
525+
Check: resource.ComposeTestCheckFunc(
526+
resource.TestCheckResourceAttr("tfe_registry_module.foobar", "publishing_mechanism", "git_tag"),
527+
resource.TestCheckNoResourceAttr("tfe_registry_module.foobar", "test_config.0"),
528+
resource.TestCheckResourceAttr("tfe_registry_module.foobar", "vcs_repo.0.tags", strconv.FormatBool(true)),
529+
resource.TestCheckResourceAttr("tfe_registry_module.foobar", "vcs_repo.0.branch", ""),
530+
),
531+
},
532+
},
533+
})
534+
}
535+
488536
func TestAccTFERegistryModuleImport_vcsPublishingMechanismBranchToTagsToBranchWithTests(t *testing.T) {
489537
rInt := rand.New(rand.NewSource(time.Now().UnixNano())).Int()
490538

@@ -621,6 +669,25 @@ func TestAccTFERegistryModule_invalidTestConfigOnUpdate(t *testing.T) {
621669
})
622670
}
623671

672+
func TestAccTFERegistryModule_vcsTagsOnlyFalse(t *testing.T) {
673+
skipUnlessBeta(t)
674+
rInt := rand.New(rand.NewSource(time.Now().UnixNano())).Int()
675+
676+
resource.Test(t, resource.TestCase{
677+
PreCheck: func() {
678+
testAccPreCheck(t)
679+
testAccPreCheckTFERegistryModule(t)
680+
},
681+
Providers: testAccProviders,
682+
Steps: []resource.TestStep{
683+
{
684+
Config: testAccTFERegistryModule_vcsTagsOnlyFalse(rInt),
685+
ExpectError: regexp.MustCompile(`branch must be provided when tags is set to false`),
686+
},
687+
},
688+
})
689+
}
690+
624691
func TestAccTFERegistryModule_branchAndInvalidTagsOnCreate(t *testing.T) {
625692
rInt := rand.New(rand.NewSource(time.Now().UnixNano())).Int()
626693

@@ -1035,7 +1102,7 @@ func getRegistryModuleProvider() string {
10351102
return strings.SplitN(getRegistryModuleRepository(), "-", 3)[1]
10361103
}
10371104

1038-
func testAccTFERegistryModule_vcs(rInt int) string {
1105+
func testAccTFERegistryModule_vcsWithTagsTrue(rInt int) string {
10391106
return fmt.Sprintf(`
10401107
resource "tfe_organization" "foobar" {
10411108
name = "tst-terraform-%d"
@@ -1277,6 +1344,125 @@ resource "tfe_registry_module" "foobar" {
12771344
envGithubRegistryModuleIdentifer)
12781345
}
12791346

1347+
func testAccTFERegistryModule_vcsBasic(rInt int) string {
1348+
return fmt.Sprintf(`
1349+
resource "tfe_organization" "foobar" {
1350+
name = "tst-terraform-%d"
1351+
1352+
}
1353+
1354+
resource "tfe_oauth_client" "foobar" {
1355+
organization = tfe_organization.foobar.name
1356+
api_url = "https://api.github.com"
1357+
http_url = "https://github.com"
1358+
oauth_token = "%s"
1359+
service_provider = "github"
1360+
}
1361+
1362+
resource "tfe_registry_module" "foobar" {
1363+
organization = tfe_organization.foobar.name
1364+
vcs_repo {
1365+
display_identifier = "%s"
1366+
identifier = "%s"
1367+
oauth_token_id = tfe_oauth_client.foobar.oauth_token_id
1368+
}
1369+
}`,
1370+
rInt,
1371+
envGithubToken,
1372+
envGithubRegistryModuleIdentifer,
1373+
envGithubRegistryModuleIdentifer)
1374+
}
1375+
1376+
func testAccTFERegistryModule_vcsTagsOnlyFalse(rInt int) string {
1377+
return fmt.Sprintf(`
1378+
resource "tfe_organization" "foobar" {
1379+
name = "tst-terraform-%d"
1380+
1381+
}
1382+
1383+
resource "tfe_oauth_client" "foobar" {
1384+
organization = tfe_organization.foobar.name
1385+
api_url = "https://api.github.com"
1386+
http_url = "https://github.com"
1387+
oauth_token = "%s"
1388+
service_provider = "github"
1389+
}
1390+
1391+
resource "tfe_registry_module" "foobar" {
1392+
organization = tfe_organization.foobar.name
1393+
vcs_repo {
1394+
display_identifier = "%s"
1395+
identifier = "%s"
1396+
oauth_token_id = tfe_oauth_client.foobar.oauth_token_id
1397+
tags = false
1398+
}
1399+
}`,
1400+
rInt,
1401+
envGithubToken,
1402+
envGithubRegistryModuleIdentifer,
1403+
envGithubRegistryModuleIdentifer)
1404+
}
1405+
1406+
func testAccTFERegistryModule_branchOnly(rInt int) string {
1407+
return fmt.Sprintf(`
1408+
resource "tfe_organization" "foobar" {
1409+
name = "tst-terraform-%d"
1410+
1411+
}
1412+
1413+
resource "tfe_oauth_client" "foobar" {
1414+
organization = tfe_organization.foobar.name
1415+
api_url = "https://api.github.com"
1416+
http_url = "https://github.com"
1417+
oauth_token = "%s"
1418+
service_provider = "github"
1419+
}
1420+
1421+
resource "tfe_registry_module" "foobar" {
1422+
organization = tfe_organization.foobar.name
1423+
vcs_repo {
1424+
display_identifier = "%s"
1425+
identifier = "%s"
1426+
oauth_token_id = tfe_oauth_client.foobar.oauth_token_id
1427+
branch = "main"
1428+
}
1429+
}`,
1430+
rInt,
1431+
envGithubToken,
1432+
envGithubRegistryModuleIdentifer,
1433+
envGithubRegistryModuleIdentifer)
1434+
}
1435+
1436+
func testAccTFERegistryModule_branchOnlyEmpty(rInt int) string {
1437+
return fmt.Sprintf(`
1438+
resource "tfe_organization" "foobar" {
1439+
name = "tst-terraform-%d"
1440+
1441+
}
1442+
1443+
resource "tfe_oauth_client" "foobar" {
1444+
organization = tfe_organization.foobar.name
1445+
api_url = "https://api.github.com"
1446+
http_url = "https://github.com"
1447+
oauth_token = "%s"
1448+
service_provider = "github"
1449+
}
1450+
1451+
resource "tfe_registry_module" "foobar" {
1452+
organization = tfe_organization.foobar.name
1453+
vcs_repo {
1454+
display_identifier = "%s"
1455+
identifier = "%s"
1456+
oauth_token_id = tfe_oauth_client.foobar.oauth_token_id
1457+
branch = ""
1458+
}
1459+
}`,
1460+
rInt,
1461+
envGithubToken,
1462+
envGithubRegistryModuleIdentifer,
1463+
envGithubRegistryModuleIdentifer)
1464+
}
1465+
12801466
func testAccTFERegistryModule_vcsTags(rInt int) string {
12811467
return fmt.Sprintf(`
12821468
resource "tfe_organization" "foobar" {
@@ -1307,7 +1493,7 @@ resource "tfe_registry_module" "foobar" {
13071493
envGithubRegistryModuleIdentifer,
13081494
envGithubRegistryModuleIdentifer)
13091495
}
1310-
func testAccTFERegistryModule_vcsRepoWithFalseTagField(rInt int) string {
1496+
func testAccTFERegistryModule_vcsRepoWithFalseTags(rInt int) string {
13111497
return fmt.Sprintf(`
13121498
resource "tfe_organization" "foobar" {
13131499
name = "tst-terraform-%d"

0 commit comments

Comments
 (0)