diff --git a/helper/test/helper.go b/helper/test/helper.go new file mode 100644 index 00000000..5aa7cf83 --- /dev/null +++ b/helper/test/helper.go @@ -0,0 +1,104 @@ +package test + +import ( + "fmt" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" + "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" + "strings" +) + +const ( + sentinelIndex = "*" +) + +// The below few functions were copied from the AWS provider. + +// TestCheckTypeSetElemAttr is a resource.TestCheckFunc that accepts a resource +// name, an attribute path, which should use the sentinel value '*' for indexing +// into a TypeSet. The function verifies that an element matches the provided +// value. +// +// Use this function over SDK provided TestCheckFunctions when validating a +// TypeSet where its elements are a simple value +func TestCheckTypeSetElemAttr(name, attr, value string) resource.TestCheckFunc { + return func(s *terraform.State) error { + is, err := instanceState(s, name) + if err != nil { + return err + } + + err = testCheckTypeSetElem(is, attr, value) + if err != nil { + return fmt.Errorf("%q error: %s", name, err) + } + + return nil + } +} + +// TestCheckTypeSetElemAttrPair is a TestCheckFunc that verifies a pair of name/key +// combinations are equal where the first uses the sentinel value to index into a +// TypeSet. +// +// E.g., tfawsresource.TestCheckTypeSetElemAttrPair("aws_autoscaling_group.bar", "availability_zones.*", "data.aws_availability_zones.available", "names.0") +func TestCheckTypeSetElemAttrPair(nameFirst, keyFirst, nameSecond, keySecond string) resource.TestCheckFunc { + return func(s *terraform.State) error { + isFirst, err := instanceState(s, nameFirst) + if err != nil { + return err + } + + isSecond, err := instanceState(s, nameSecond) + if err != nil { + return err + } + + vSecond, okSecond := isSecond.Attributes[keySecond] + if !okSecond { + return fmt.Errorf("%s: Attribute %q not set, cannot be checked against TypeSet", nameSecond, keySecond) + } + + return testCheckTypeSetElem(isFirst, keyFirst, vSecond) + } +} + +// instanceState returns the primary instance state for the given +// resource name in the root module. +func instanceState(s *terraform.State, name string) (*terraform.InstanceState, error) { + ms := s.RootModule() + rs, ok := ms.Resources[name] + if !ok { + return nil, fmt.Errorf("Not found: %s in %s", name, ms.Path) + } + + is := rs.Primary + if is == nil { + return nil, fmt.Errorf("No primary instance: %s in %s", name, ms.Path) + } + + return is, nil +} + +func testCheckTypeSetElem(is *terraform.InstanceState, attr, value string) error { + attrParts := strings.Split(attr, ".") + if attrParts[len(attrParts)-1] != sentinelIndex { + return fmt.Errorf("%q does not end with the special value %q", attr, sentinelIndex) + } + for stateKey, stateValue := range is.Attributes { + if stateValue == value { + stateKeyParts := strings.Split(stateKey, ".") + if len(stateKeyParts) == len(attrParts) { + for i := range attrParts { + if attrParts[i] != stateKeyParts[i] && attrParts[i] != sentinelIndex { + break + } + if i == len(attrParts)-1 { + return nil + } + } + } + } + } + + return fmt.Errorf("no TypeSet element %q, with value %q in state: %#v", attr, value, is.Attributes) +} diff --git a/heroku/resource_heroku_addon_test.go b/heroku/resource_heroku_addon_test.go index 35805088..946c463f 100644 --- a/heroku/resource_heroku_addon_test.go +++ b/heroku/resource_heroku_addon_test.go @@ -111,7 +111,7 @@ func TestAccHerokuAddon_CustomName_Invalid(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccCheckHerokuAddonConfig_CustomName(appName, customName), - ExpectError: regexp.MustCompile(`config is invalid: invalid value for name`), + ExpectError: regexp.MustCompile(`Invalid custom addon name.*`), }, }, }) @@ -128,7 +128,7 @@ func TestAccHerokuAddon_CustomName_EmptyString(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccCheckHerokuAddonConfig_CustomName(appName, customName), - ExpectError: regexp.MustCompile(`config is invalid: 2 problems:.*`), + ExpectError: regexp.MustCompile(`Invalid custom addon name.*`), }, }, }) @@ -145,7 +145,7 @@ func TestAccHerokuAddon_CustomName_FirstCharNum(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccCheckHerokuAddonConfig_CustomName(appName, customName), - ExpectError: regexp.MustCompile(`config is invalid: invalid value for name`), + ExpectError: regexp.MustCompile(`Invalid custom addon name.*`), }, }, }) diff --git a/heroku/resource_heroku_app.go b/heroku/resource_heroku_app.go index 398d98ef..0e34c395 100644 --- a/heroku/resource_heroku_app.go +++ b/heroku/resource_heroku_app.go @@ -344,15 +344,15 @@ func setTeamDetails(d *schema.ResourceData, app *application) (err error) { } func setAppDetails(d *schema.ResourceData, app *application) (err error) { - err = d.Set("name", app.App.Name) - err = d.Set("stack", app.App.Stack) - err = d.Set("internal_routing", app.App.InternalRouting) - err = d.Set("region", app.App.Region) - err = d.Set("git_url", app.App.GitURL) - err = d.Set("web_url", app.App.WebURL) - err = d.Set("acm", app.App.Acm) - err = d.Set("uuid", app.App.ID) - err = d.Set("heroku_hostname", fmt.Sprintf("%s.herokuapp.com", app.App.Name)) + d.Set("name", app.App.Name) + d.Set("stack", app.App.Stack) + d.Set("internal_routing", app.App.InternalRouting) + d.Set("region", app.App.Region) + d.Set("git_url", app.App.GitURL) + d.Set("web_url", app.App.WebURL) + d.Set("acm", app.App.Acm) + d.Set("uuid", app.App.ID) + d.Set("heroku_hostname", fmt.Sprintf("%s.herokuapp.com", app.App.Name)) return err } diff --git a/heroku/resource_heroku_app_test.go b/heroku/resource_heroku_app_test.go index 04675c9c..fa908e99 100644 --- a/heroku/resource_heroku_app_test.go +++ b/heroku/resource_heroku_app_test.go @@ -595,7 +595,7 @@ func testAccCheckHerokuAppAttributesOrg(app *heroku.TeamApp, appName, space, org } // This needs to be updated whenever heroku bumps the stack number - if app.BuildStack.Name != "heroku-18" { + if app.BuildStack.Name != "heroku-20" { return fmt.Errorf("Bad stack: %s", app.BuildStack.Name) } diff --git a/heroku/resource_heroku_build.go b/heroku/resource_heroku_build.go index c714a523..397d0604 100644 --- a/heroku/resource_heroku_build.go +++ b/heroku/resource_heroku_build.go @@ -64,11 +64,10 @@ func resourceHerokuBuild() *schema.Resource { }, "source": { - Type: schema.TypeList, - Required: true, - ForceNew: true, - MaxItems: 1, - ConfigMode: schema.SchemaConfigModeAttr, + Type: schema.TypeList, + Required: true, + ForceNew: true, + MaxItems: 1, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "checksum": { @@ -194,18 +193,20 @@ func resourceHerokuBuildCreate(d *schema.ResourceData, meta interface{}) error { for _, s := range vL { sourceArg := s.(map[string]interface{}) - if v := sourceArg["checksum"]; v != nil { + if v, ok := sourceArg["checksum"]; ok && v != "" { s := v.(string) - if v = sourceArg["path"]; v != nil { + if vv, okok := sourceArg["path"]; okok && vv != "" { return fmt.Errorf("source.checksum should be empty when source.path is set (checksum is auto-generated)") } opts.SourceBlob.Checksum = &s } - if v = sourceArg["version"]; v != nil { + + if v, ok := sourceArg["version"]; ok && v != "" { s := v.(string) opts.SourceBlob.Version = &s } - if v = sourceArg["path"]; v != nil { + + if v, ok := sourceArg["path"]; ok && v != "" { path := v.(string) var tarballPath string fileInfo, err := os.Stat(path) @@ -240,7 +241,7 @@ func resourceHerokuBuildCreate(d *schema.ResourceData, meta interface{}) error { } opts.SourceBlob.URL = &newSource.SourceBlob.GetURL opts.SourceBlob.Checksum = &checksum - } else if v = sourceArg["url"]; v != nil { + } else if v, ok = sourceArg["url"]; ok && v != "" { s := v.(string) opts.SourceBlob.URL = &s } else { @@ -313,7 +314,7 @@ func resourceHerokuBuildCustomizeDiff(ctx context.Context, diff *schema.Resource if v, ok := diff.GetOk("source"); ok { vL := v.([]interface{}) source := vL[0].(map[string]interface{}) - if vv := source["path"]; vv != nil { + if vv, okok := source["path"]; okok && vv != "" { path := vv.(string) var tarballPath string fileInfo, err := os.Stat(path) diff --git a/heroku/resource_heroku_space_app_access_test.go b/heroku/resource_heroku_space_app_access_test.go index 7de2237d..0def1de2 100644 --- a/heroku/resource_heroku_space_app_access_test.go +++ b/heroku/resource_heroku_space_app_access_test.go @@ -3,6 +3,7 @@ package heroku import ( "context" "fmt" + "github.com/heroku/terraform-provider-heroku/v4/helper/test" "strings" "testing" @@ -29,7 +30,7 @@ func TestAccHerokuSpaceAppAccess_Basic(t *testing.T) { Config: testAccCheckHerokuSpaceAppAccessConfig_basic(spaceName, org, testUser, []string{"create_apps"}), Check: resource.ComposeTestCheckFunc( testAccCheckHerokuSpaceExists("heroku_space.foobar", &space), - resource.TestCheckResourceAttr("heroku_space_app_access.foobar", "permissions.3695762012", "create_apps"), + test.TestCheckTypeSetElemAttr("heroku_space_app_access.foobar", "permissions.*", "create_apps"), ), }, }, diff --git a/heroku/resource_heroku_space_vpn_connection.go b/heroku/resource_heroku_space_vpn_connection.go index 5224d00d..1bc1c28a 100644 --- a/heroku/resource_heroku_space_vpn_connection.go +++ b/heroku/resource_heroku_space_vpn_connection.go @@ -134,33 +134,21 @@ func resourceHerokuSpaceVPNConnectionCreate(d *schema.ResourceData, meta interfa if err != nil { return fmt.Errorf("Error creating VPN: %v", err) } - id := conn.ID - log.Printf("[DEBUG] Waiting for VPN (%s) to be allocated", id) - retryErr := resource.Retry(d.Timeout(schema.TimeoutCreate), func() *resource.RetryError { - vpn, vpnGetErr := client.VPNConnectionInfo(context.TODO(), space, id) - - // Retry on "not found" - if vpnGetErr != nil && strings.Contains(vpnGetErr.Error(), "VPN is not found") { - return resource.RetryableError(fmt.Errorf("Waiting for new VPN connection")) - } - - // Fail for any remaining error - if vpnGetErr != nil { - return resource.NonRetryableError(fmt.Errorf("Error fetching VPN connection status: %s", vpnGetErr)) - } - - if vpn.Status != "active" { - return resource.RetryableError(fmt.Errorf("Want VPN connection status 'active', instead got '%s'", vpn.Status)) - } + log.Printf("[DEBUG] Waiting for VPN (%s) to be allocated", conn.ID) + stateConf := &resource.StateChangeConf{ + Pending: []string{"pending", "provisioning"}, + Target: []string{"active"}, + Refresh: spaceVPNConnectionStateRefreshFunc(client, space, conn.ID), + Timeout: 30 * time.Minute, + PollInterval: 20 * time.Second, + } - return resource.NonRetryableError(nil) - }) - if retryErr != nil { - return fmt.Errorf("Error waiting for VPN to become available: %v", retryErr) + if _, err := stateConf.WaitForState(); err != nil { + return fmt.Errorf("error waiting for VPN to become available: %s", err) } - d.SetId(buildCompositeID(space, id)) + d.SetId(buildCompositeID(space, conn.ID)) return resourceHerokuSpaceVPNConnectionRead(d, meta) } @@ -180,3 +168,25 @@ func resourceHerokuSpaceVPNConnectionDelete(d *schema.ResourceData, meta interfa d.SetId("") return nil } + +func spaceVPNConnectionStateRefreshFunc(client *heroku.Service, space, connectionID string) resource.StateRefreshFunc { + return func() (interface{}, string, error) { + vpn, vpnGetErr := client.VPNConnectionInfo(context.TODO(), space, connectionID) + + // Retry on "not found" + if vpnGetErr != nil && strings.Contains(vpnGetErr.Error(), "VPN is not found") { + return vpn, "pending", nil + } + + // Fail for any remaining error + if vpnGetErr != nil { + return nil, "failed", fmt.Errorf("error fetching VPN connection status: %s", vpnGetErr) + } + + if vpn.Status != "active" { + return vpn, vpn.Status, nil + } + + return vpn, vpn.Status, nil + } +} diff --git a/heroku/resource_heroku_team_collaborator.go b/heroku/resource_heroku_team_collaborator.go index 2a192340..2d1054d8 100644 --- a/heroku/resource_heroku_team_collaborator.go +++ b/heroku/resource_heroku_team_collaborator.go @@ -272,7 +272,13 @@ func resourceHerokuTeamCollaboratorImport(d *schema.ResourceData, meta interface d.SetId(collaborator.ID) d.Set("app", collaborator.App.Name) d.Set("email", collaborator.User.Email) - d.Set("permissions", collaborator.Permissions) + + var perms []string + for _, p := range collaborator.Permissions { + perms = append(perms, p.Name) + } + + d.Set("permissions", perms) return []*schema.ResourceData{d}, nil } diff --git a/heroku/resource_heroku_team_collaborator_test.go b/heroku/resource_heroku_team_collaborator_test.go index b782fd97..604c4a08 100644 --- a/heroku/resource_heroku_team_collaborator_test.go +++ b/heroku/resource_heroku_team_collaborator_test.go @@ -3,6 +3,7 @@ package heroku import ( "context" "fmt" + "github.com/heroku/terraform-provider-heroku/v4/helper/test" "testing" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest" @@ -30,8 +31,7 @@ func TestAccHerokuTeamCollaborator_Org(t *testing.T) { Check: resource.ComposeTestCheckFunc( testAccCheckHerokuTeamCollaboratorExists("heroku_team_collaborator.foobar-collaborator", &teamCollaborator), testAccCheckHerokuTeamCollaboratorEmailAttribute(&teamCollaborator, testUser), - resource.TestCheckResourceAttr( - "heroku_team_collaborator.foobar-collaborator", "permissions.1056122515", "deploy"), + test.TestCheckTypeSetElemAttr("heroku_team_collaborator.foobar-collaborator", "permissions.*", "deploy"), ), }, }, @@ -57,8 +57,7 @@ func TestAccHerokuTeamCollaboratorPermsOutOfOrder_Org(t *testing.T) { Check: resource.ComposeTestCheckFunc( testAccCheckHerokuTeamCollaboratorExists("heroku_team_collaborator.foobar-collaborator", &teamCollaborator), testAccCheckHerokuTeamCollaboratorEmailAttribute(&teamCollaborator, testUser), - resource.TestCheckResourceAttr( - "heroku_team_collaborator.foobar-collaborator", "permissions.1056122515", "deploy"), + test.TestCheckTypeSetElemAttr("heroku_team_collaborator.foobar-collaborator", "permissions.*", "deploy"), ), }, },