From 6c452a01e18d7a46e8200eaf5bd24fb40450d0b5 Mon Sep 17 00:00:00 2001 From: Daniel Couture Date: Thu, 7 May 2020 02:03:14 -0400 Subject: [PATCH 1/5] add ValidateFunc to resourceTFEPolicySet to catch syntax errors at tf plan rather than error on apply --- tfe/resource_tfe_policy_set.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tfe/resource_tfe_policy_set.go b/tfe/resource_tfe_policy_set.go index 0ae4b33a2..0284e9e67 100644 --- a/tfe/resource_tfe_policy_set.go +++ b/tfe/resource_tfe_policy_set.go @@ -3,6 +3,7 @@ package tfe import ( "fmt" "log" + "regexp" tfe "github.com/hashicorp/go-tfe" "github.com/hashicorp/terraform-plugin-sdk/helper/schema" @@ -22,6 +23,17 @@ func resourceTFEPolicySet() *schema.Resource { "name": { Type: schema.TypeString, Required: true, + ValidateFunc: func(v interface{}, k string) (ws []string, errs []error) { + value := v.(string) + matched, err := regexp.Match(`^[A-Za-z\d-_]+$`, []byte(value)) + if err != nil { + errs = append(errs, err) + } + if !matched { + errs = append(errs, fmt.Errorf("policy set name %s should only contain letters, numbers, dashes (-) and underscores (_)", value)) + } + return + }, }, "description": { From 1ceb22cd011b9835618be050573756590ccb5a42 Mon Sep 17 00:00:00 2001 From: Daniel Couture Date: Thu, 7 May 2020 02:25:47 -0400 Subject: [PATCH 2/5] adding acceptance test --- tfe/resource_tfe_policy_set_test.go | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/tfe/resource_tfe_policy_set_test.go b/tfe/resource_tfe_policy_set_test.go index 791ac1aa8..80c815624 100644 --- a/tfe/resource_tfe_policy_set_test.go +++ b/tfe/resource_tfe_policy_set_test.go @@ -2,6 +2,7 @@ package tfe import ( "fmt" + "regexp" "testing" tfe "github.com/hashicorp/go-tfe" @@ -445,6 +446,20 @@ func testAccCheckTFEPolicySetDestroy(s *terraform.State) error { return nil } +func TestAccTFEPolicySet_invalidname(t *testing.T) { + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckTFEPolicySetDestroy, + Steps: []resource.TestStep{ + { + Config: testAccTFEPolicySet_invalidname, + ExpectError: regexp.MustCompile(`policy set name not the right format should only contain`), + }, + }, + }) +} + const testAccTFEPolicySet_basic = ` resource "tfe_organization" "foobar" { name = "tst-terraform" @@ -591,3 +606,11 @@ resource "tfe_policy_set" "foobar" { GITHUB_POLICY_SET_BRANCH, GITHUB_POLICY_SET_PATH, ) + +const testAccTFEPolicySet_invalidname = ` +resource "tfe_policy_set" "foobar" { + name = "not the right format" + description = "Policy Set" + organization = "${tfe_organization.foobar.id}" + policy_ids = ["${tfe_sentinel_policy.foo.id}"] +}` From 928c623a052d3c2a4742c8f71af8d571f9bf6d65 Mon Sep 17 00:00:00 2001 From: Daniel Couture Date: Thu, 7 May 2020 02:30:50 -0400 Subject: [PATCH 3/5] add back required reference resources to test --- tfe/resource_tfe_policy_set_test.go | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tfe/resource_tfe_policy_set_test.go b/tfe/resource_tfe_policy_set_test.go index 80c815624..592257f2b 100644 --- a/tfe/resource_tfe_policy_set_test.go +++ b/tfe/resource_tfe_policy_set_test.go @@ -608,6 +608,17 @@ resource "tfe_policy_set" "foobar" { ) const testAccTFEPolicySet_invalidname = ` +resource "tfe_organization" "foobar" { + name = "tst-terraform" + email = "admin@company.com" +} + +resource "tfe_sentinel_policy" "foo" { + name = "policy-foo" + policy = "main = rule { true }" + organization = "${tfe_organization.foobar.id}" +} + resource "tfe_policy_set" "foobar" { name = "not the right format" description = "Policy Set" From d614b60b74c5d40e4ddc424a54dfc3bb27442fb7 Mon Sep 17 00:00:00 2001 From: Daniel Couture Date: Thu, 30 Jul 2020 08:48:58 -0400 Subject: [PATCH 4/5] update resourceTFEPolicySet.ValidateFunc to native validation.StringMatch and update tests to match new error message --- tfe/resource_tfe_policy_set.go | 17 ++++------------- tfe/resource_tfe_policy_set_test.go | 2 +- 2 files changed, 5 insertions(+), 14 deletions(-) diff --git a/tfe/resource_tfe_policy_set.go b/tfe/resource_tfe_policy_set.go index 0284e9e67..95297be9f 100644 --- a/tfe/resource_tfe_policy_set.go +++ b/tfe/resource_tfe_policy_set.go @@ -7,6 +7,7 @@ import ( tfe "github.com/hashicorp/go-tfe" "github.com/hashicorp/terraform-plugin-sdk/helper/schema" + "github.com/hashicorp/terraform-plugin-sdk/helper/validation" ) func resourceTFEPolicySet() *schema.Resource { @@ -21,19 +22,9 @@ func resourceTFEPolicySet() *schema.Resource { Schema: map[string]*schema.Schema{ "name": { - Type: schema.TypeString, - Required: true, - ValidateFunc: func(v interface{}, k string) (ws []string, errs []error) { - value := v.(string) - matched, err := regexp.Match(`^[A-Za-z\d-_]+$`, []byte(value)) - if err != nil { - errs = append(errs, err) - } - if !matched { - errs = append(errs, fmt.Errorf("policy set name %s should only contain letters, numbers, dashes (-) and underscores (_)", value)) - } - return - }, + Type: schema.TypeString, + Required: true, + ValidateFunc: validation.StringMatch(regexp.MustCompile(`^[\w\.\-]+$`), "The name of the policy set. Can only include letters, numbers, -, and _."), }, "description": { diff --git a/tfe/resource_tfe_policy_set_test.go b/tfe/resource_tfe_policy_set_test.go index 592257f2b..183d2f78f 100644 --- a/tfe/resource_tfe_policy_set_test.go +++ b/tfe/resource_tfe_policy_set_test.go @@ -454,7 +454,7 @@ func TestAccTFEPolicySet_invalidname(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccTFEPolicySet_invalidname, - ExpectError: regexp.MustCompile(`policy set name not the right format should only contain`), + ExpectError: regexp.MustCompile(`The name of the policy set. Can only include letters, numbers, -, and _.`), }, }, }) From 6699f88fe5ddcc83dfd64ea5672a3a90e7f6d8b1 Mon Sep 17 00:00:00 2001 From: Daniel Couture Date: Thu, 13 Aug 2020 21:23:35 -0400 Subject: [PATCH 5/5] pr feedback on regex match and naming consistency --- tfe/resource_tfe_policy_set.go | 2 +- tfe/resource_tfe_policy_set_test.go | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/tfe/resource_tfe_policy_set.go b/tfe/resource_tfe_policy_set.go index 95297be9f..f55398bb7 100644 --- a/tfe/resource_tfe_policy_set.go +++ b/tfe/resource_tfe_policy_set.go @@ -24,7 +24,7 @@ func resourceTFEPolicySet() *schema.Resource { "name": { Type: schema.TypeString, Required: true, - ValidateFunc: validation.StringMatch(regexp.MustCompile(`^[\w\.\-]+$`), "The name of the policy set. Can only include letters, numbers, -, and _."), + ValidateFunc: validation.StringMatch(regexp.MustCompile(`\A[\w\_\-]+\z`), "can only include letters, numbers, -, and _."), }, "description": { diff --git a/tfe/resource_tfe_policy_set_test.go b/tfe/resource_tfe_policy_set_test.go index 183d2f78f..d4617bc25 100644 --- a/tfe/resource_tfe_policy_set_test.go +++ b/tfe/resource_tfe_policy_set_test.go @@ -446,15 +446,15 @@ func testAccCheckTFEPolicySetDestroy(s *terraform.State) error { return nil } -func TestAccTFEPolicySet_invalidname(t *testing.T) { +func TestAccTFEPolicySet_invalidName(t *testing.T) { resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, Providers: testAccProviders, CheckDestroy: testAccCheckTFEPolicySetDestroy, Steps: []resource.TestStep{ { - Config: testAccTFEPolicySet_invalidname, - ExpectError: regexp.MustCompile(`The name of the policy set. Can only include letters, numbers, -, and _.`), + Config: testAccTFEPolicySet_invalidName, + ExpectError: regexp.MustCompile(`can only include letters, numbers, -, and _.`), }, }, }) @@ -607,7 +607,7 @@ resource "tfe_policy_set" "foobar" { GITHUB_POLICY_SET_PATH, ) -const testAccTFEPolicySet_invalidname = ` +const testAccTFEPolicySet_invalidName = ` resource "tfe_organization" "foobar" { name = "tst-terraform" email = "admin@company.com"