SCALRCORE-19995 > Fix a lot of bugs related to array of strings

scalr/resource_scalr_access_policy.go

@@ -117,6 +117,19 @@ func resourceScalrAccessPolicy() *schema.Resource {
 	}
 }
 
+func parseRoleIdDefinitions(d *schema.ResourceData) ([]*scalr.Role, error) {
+	var roles []*scalr.Role
+	for _, roleId := range d.Get("role_ids").([]interface{}) {
+		id, ok := roleId.(string)
+		if !ok || id == "" {
+			return nil, errors.New("Got empty value for role id")
+		}
+		roles = append(roles, &scalr.Role{ID: roleId.(string)})
+	}
+
+	return roles, nil
+}
+
 func resourceScalrAccessPolicyCreate(d *schema.ResourceData, meta interface{}) error {
 	scalrClient := meta.(*scalr.Client)
 
@@ -128,9 +141,9 @@ func resourceScalrAccessPolicyCreate(d *schema.ResourceData, meta interface{}) e
 	scopeType := scope["type"].(string)
 	scopeId := scope["id"].(string)
 
-	var roles []*scalr.Role
-	for _, roleId := range d.Get("role_ids").([]interface{}) {
-		roles = append(roles, &scalr.Role{ID: roleId.(string)})
+	roles, err := parseRoleIdDefinitions(d)
+	if err != nil {
+		return err
 	}
 
 	// Create a new options struct.
@@ -234,16 +247,16 @@ func resourceScalrAccessPolicyUpdate(d *schema.ResourceData, meta interface{}) e
 	id := d.Id()
 
 	if d.HasChange("role_ids") {
-		var roles []*scalr.Role
-		for _, roleId := range d.Get("role_ids").([]interface{}) {
-			roles = append(roles, &scalr.Role{ID: roleId.(string)})
+		roles, err := parseRoleIdDefinitions(d)
+		if err != nil {
+			return err
 		}
 
 		// Create a new options struct.
 		options := scalr.AccessPolicyUpdateOptions{Roles: roles}
 
 		log.Printf("[DEBUG] Update access policy %s", id)
-		_, err := scalrClient.AccessPolicies.Update(ctx, id, options)
+		_, err = scalrClient.AccessPolicies.Update(ctx, id, options)
 		if err != nil {
 			return fmt.Errorf(
 				"Error updating access policy %s: %v", id, err)

scalr/resource_scalr_access_policy_test.go

@@ -145,6 +145,11 @@ func TestAccScalrAccessPolicy_update(t *testing.T) {
 					resource.TestCheckResourceAttr("scalr_access_policy.test", "role_ids.#", "2"),
 				),
 			},
+
+			{
+				Config:      testAccScalrAccessPolicyEmptyRoleId(rInt),
+				ExpectError: regexp.MustCompile("Got empty value for role id"),
+			},
 		},
 	})
 }
@@ -304,6 +309,10 @@ func testAccScalrAccessPolicyBasic(rInt int) string {
 	return fmt.Sprintf(iamPolicyTemplate, rInt, defaultAccount, testUser, readOnlyRole)
 }
 
+func testAccScalrAccessPolicyEmptyRoleId(rInt int) string {
+	return fmt.Sprintf(iamPolicyTemplate, rInt, defaultAccount, testUser, "")
+}
+
 func testAccScalrAccessPolicyChangedOutside(rInt int) string {
 	return fmt.Sprintf(iamPolicyTemplate, rInt, defaultAccount, testUser, userRole)
 }

scalr/resource_scalr_environment.go

@@ -1,6 +1,7 @@
 package scalr
 
 import (
+	"errors"
 	"fmt"
 	"log"
 
@@ -73,20 +74,44 @@ func resourceScalrEnvironment() *schema.Resource {
 	}
 }
 
+func parseCloudCredentialDefinitions(d *schema.ResourceData) ([]*scalr.CloudCredential, error) {
+	var cloudCredentials []*scalr.CloudCredential
+	for _, cloudCredsID := range d.Get("cloud_credentials").([]interface{}) {
+		id, ok := cloudCredsID.(string)
+		if !ok || id == "" {
+			return nil, errors.New("Got empty value for cloud credential")
+		}
+		cloudCredentials = append(cloudCredentials, &scalr.CloudCredential{ID: id})
+	}
+
+	return cloudCredentials, nil
+}
+
+func parsePolicyGroupDefinitions(d *schema.ResourceData) ([]*scalr.PolicyGroup, error) {
+	var policyGroups []*scalr.PolicyGroup
+	for _, cloudCredsID := range d.Get("policy_groups").([]interface{}) {
+		id, ok := cloudCredsID.(string)
+		if !ok || id == "" {
+			return nil, errors.New("Got empty value for policy group")
+		}
+		policyGroups = append(policyGroups, &scalr.PolicyGroup{ID: id})
+	}
+
+	return policyGroups, nil
+}
+
 func resourceScalrEnvironmentCreate(d *schema.ResourceData, meta interface{}) error {
 	scalrClient := meta.(*scalr.Client)
 
 	name := d.Get("name").(string)
 	accountID := d.Get("account_id").(string)
-
-	var cloudCredentials []*scalr.CloudCredential
-	for _, cloudCredsID := range d.Get("cloud_credentials").([]interface{}) {
-		cloudCredentials = append(cloudCredentials, &scalr.CloudCredential{ID: cloudCredsID.(string)})
+	cloudCredentials, err := parseCloudCredentialDefinitions(d)
+	if err != nil {
+		return err
 	}
-
-	var policyGroups []*scalr.PolicyGroup
-	for _, policyGroupID := range d.Get("policy_groups").([]interface{}) {
-		policyGroups = append(policyGroups, &scalr.PolicyGroup{ID: policyGroupID.(string)})
+	policyGroups, err := parsePolicyGroupDefinitions(d)
+	if err != nil {
+		return err
 	}
 
 	options := scalr.EnvironmentCreateOptions{
@@ -161,13 +186,13 @@ func resourceScalrEnvironmentUpdate(d *schema.ResourceData, meta interface{}) er
 	scalrClient := meta.(*scalr.Client)
 
 	var err error
-	var cloudCredentials []*scalr.CloudCredential
-	for _, credsID := range d.Get("cloud_credentials").([]interface{}) {
-		cloudCredentials = append(cloudCredentials, &scalr.CloudCredential{ID: credsID.(string)})
+	cloudCredentials, err := parseCloudCredentialDefinitions(d)
+	if err != nil {
+		return err
 	}
-	var policyGroups []*scalr.PolicyGroup
-	for _, credsID := range d.Get("policy_groups").([]interface{}) {
-		policyGroups = append(policyGroups, &scalr.PolicyGroup{ID: credsID.(string)})
+	policyGroups, err := parsePolicyGroupDefinitions(d)
+	if err != nil {
+		return err
 	}
 
 	// Create a new options struct.

scalr/resource_scalr_environment_test.go

@@ -2,6 +2,7 @@ package scalr
 
 import (
 	"fmt"
+	"regexp"
 	"testing"
 
 	"github.com/hashicorp/terraform/helper/resource"
@@ -75,6 +76,10 @@ func TestAccEnvironment_update(t *testing.T) {
 					resource.TestCheckResourceAttr("scalr_environment.test", "cloud_credentials.%", "0"),
 				),
 			},
+			{
+				Config:      testAccEnvironmentUpdateConfigEmptyString(rInt),
+				ExpectError: regexp.MustCompile("Got empty value for cloud credential"),
+			},
 		},
 	})
 }
@@ -173,3 +178,13 @@ resource "scalr_environment" "test" {
   cloud_credentials = []
 }`, rInt, defaultAccount)
 }
+
+func testAccEnvironmentUpdateConfigEmptyString(rInt int) string {
+	return fmt.Sprintf(`
+resource "scalr_environment" "test" {
+  name       = "test-env-%d-patched"
+  account_id = "%s"
+  cost_estimation_enabled = false
+  cloud_credentials = [""]
+}`, rInt, defaultAccount)
+}

scalr/resource_scalr_role.go

@@ -1,6 +1,7 @@
 package scalr
 
 import (
+	"errors"
 	"fmt"
 	"log"
 	"reflect"
@@ -52,32 +53,41 @@ func resourceScalrRole() *schema.Resource {
 	}
 }
 
+func parsePermissionDefinitions(d *schema.ResourceData) ([]*scalr.Permission, error) {
+	permissionNames := d.Get("permissions").([]interface{})
+	var permissions []*scalr.Permission
+
+	for _, permID := range permissionNames {
+		id, ok := permID.(string)
+		if !ok || id == "" {
+			return nil, errors.New("Got empty value for permission")
+		}
+		permissions = append(permissions, &scalr.Permission{ID: id})
+	}
+
+	return permissions, nil
+}
+
 func resourceScalrRoleCreate(d *schema.ResourceData, meta interface{}) error {
 	scalrClient := meta.(*scalr.Client)
 
 	// Get required options
 	name := d.Get("name").(string)
+	description := d.Get("description").(string)
 	accountID := d.Get("account_id").(string)
 
-	// Create a new options struct.
-	options := scalr.RoleCreateOptions{
-		Name:    scalr.String(name),
-		Account: &scalr.Account{ID: accountID},
-	}
-
-	// Process all optional fields.
-	if value, ok := d.GetOk("permissions"); ok {
-		permissionNames := value.([]interface{})
-		permissions := make([]*scalr.Permission, 0)
-
-		for _, id := range permissionNames {
-			permissions = append(permissions, &scalr.Permission{ID: id.(string)})
-		}
-		options.Permissions = permissions
+	// Get optional attributes
+	permissions, err := parsePermissionDefinitions(d)
+	if err != nil {
+		return err
 	}
 
-	if description, ok := d.GetOk("description"); ok {
-		options.Description = scalr.String(description.(string))
+	// Create a new options struct
+	options := scalr.RoleCreateOptions{
+		Name:        scalr.String(name),
+		Account:     &scalr.Account{ID: accountID},
+		Description: scalr.String(description),
+		Permissions: permissions,
 	}
 
 	log.Printf("[DEBUG] Create role %s for account: %s", name, accountID)
@@ -145,24 +155,20 @@ func resourceScalrRoleUpdate(d *schema.ResourceData, meta interface{}) error {
 	id := d.Id()
 
 	if d.HasChange("name") || d.HasChange("description") || d.HasChange("permissions") {
-		// Create a new options struct.
+		permissions, err := parsePermissionDefinitions(d)
+		if err != nil {
+			return err
+		}
+
+		// Create a new options struct
 		options := scalr.RoleUpdateOptions{
 			Name:        scalr.String(d.Get("name").(string)),
 			Description: scalr.String(d.Get("description").(string)),
+			Permissions: permissions,
 		}
 
-		// Process all configured options.
-		if value, ok := d.GetOk("permissions"); ok {
-			permissionNames := value.([]interface{})
-			permissions := make([]*scalr.Permission, 0)
-
-			for _, id := range permissionNames {
-				permissions = append(permissions, &scalr.Permission{ID: id.(string)})
-			}
-			options.Permissions = permissions
-		}
 		log.Printf("[DEBUG] Update role %s", id)
-		_, err := scalrClient.Roles.Update(ctx, id, options)
+		scalrClient.Roles.Update(ctx, id, options)
 		if err != nil {
 			return fmt.Errorf(
 				"Error updating role %s: %v", id, err)

scalr/resource_scalr_role_test.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 	"log"
+	"regexp"
 	"testing"
 
 	"github.com/hashicorp/terraform/helper/resource"
@@ -104,6 +105,11 @@ func TestAccScalrRole_update(t *testing.T) {
 					resource.TestCheckResourceAttr("scalr_role.test", "permissions.1", "*:delete"),
 				),
 			},
+
+			{
+				Config:      testAccScalrRoleUpdateEmptyPermission(),
+				ExpectError: regexp.MustCompile("Got empty value for permission"),
+			},
 		},
 	})
 }
@@ -237,3 +243,13 @@ resource "scalr_role" "test" {
   ]
 }`, defaultAccount)
 }
+
+func testAccScalrRoleUpdateEmptyPermission() string {
+	return fmt.Sprintf(`
+resource "scalr_role" "test" {
+  name           = "role-updated"
+  account_id     = "%s"
+  description    = "updated"
+  permissions    = [""]
+}`, defaultAccount)
+}

scalr/resource_scalr_webhook.go

@@ -1,27 +1,23 @@
 package scalr
 
 import (
+	"errors"
 	"fmt"
 	"log"
-	"sort"
 	"strings"
 
 	"github.com/hashicorp/terraform/helper/schema"
 	scalr "github.com/scalr/go-scalr"
 )
 
 var (
-	eventDefinitions = []string{
-		"run:completed",
-		"run:errored",
-		"run:needs_attention",
+	eventDefinitions = map[string]bool{
+		"run:completed":       true,
+		"run:errored":         true,
+		"run:needs_attention": true,
 	}
 )
 
-func init() {
-	sort.Strings(eventDefinitions)
-}
-
 func resourceScalrWebhook() *schema.Resource {
 	return &schema.Resource{
 		Create: resourceScalrWebhookCreate,
@@ -113,12 +109,14 @@ func getResourceScope(scalrClient *scalr.Client, workspaceID string, environment
 }
 
 func validateEventDefinitions(eventName string) error {
-	if sort.SearchStrings(eventDefinitions, eventName) < len(eventDefinitions) {
+	if val, ok := eventDefinitions[eventName]; ok && val {
 		return nil
 	}
+	i := 0
 	eventDefinitionsQuoted := make([]string, len(eventDefinitions))
-	for i, eventDefinition := range eventDefinitions {
+	for eventDefinition := range eventDefinitions {
 		eventDefinitionsQuoted[i] = fmt.Sprintf("'%s'", eventDefinition)
+		i++
 	}
 	return fmt.Errorf(
 		"Invalid value for events '%s'. Allowed values: %s", eventName, strings.Join(eventDefinitionsQuoted, ", "))
@@ -128,10 +126,14 @@ func parseEventDefinitions(d *schema.ResourceData) ([]*scalr.EventDefinition, er
 	events := d.Get("events").([]interface{})
 	var eventDefinitions []*scalr.EventDefinition
 	for _, eventID := range events {
-		if err := validateEventDefinitions(eventID.(string)); err != nil {
+		id, ok := eventID.(string)
+		if !ok || id == "" {
+			return nil, errors.New("Got empty value for event")
+		}
+		if err := validateEventDefinitions(id); err != nil {
 			return nil, err
 		}
-		eventDefinitions = append(eventDefinitions, &scalr.EventDefinition{ID: eventID.(string)})
+		eventDefinitions = append(eventDefinitions, &scalr.EventDefinition{ID: id})
 	}
 	return eventDefinitions, nil
 }
@@ -221,7 +223,6 @@ func resourceScalrWebhookRead(d *schema.ResourceData, meta interface{}) error {
 func resourceScalrWebhookUpdate(d *schema.ResourceData, meta interface{}) error {
 	scalrClient := meta.(*scalr.Client)
 
-	var err error
 	eventDefinitions, err := parseEventDefinitions(d)
 	if err != nil {
 		return err