Merge pull request #8641 from kwilczynski/feature/update-aws_efs_file_system

provider/aws: Maintenance and clean-up for aws_efs_file_system.

builtin/providers/aws/resource_aws_efs_file_system.go

@@ -24,14 +24,14 @@ func resourceAwsEfsFileSystem() *schema.Resource {
 		},
 
 		Schema: map[string]*schema.Schema{
-			"creation_token": &schema.Schema{
+			"creation_token": {
 				Type:         schema.TypeString,
 				Optional:     true,
 				ForceNew:     true,
 				ValidateFunc: validateMaxLength(64),
 			},
 
-			"reference_name": &schema.Schema{
+			"reference_name": {
 				Type:          schema.TypeString,
 				Optional:      true,
 				ForceNew:      true,
@@ -40,7 +40,7 @@ func resourceAwsEfsFileSystem() *schema.Resource {
 				ValidateFunc:  validateReferenceName,
 			},
 
-			"performance_mode": &schema.Schema{
+			"performance_mode": {
 				Type:         schema.TypeString,
 				Optional:     true,
 				ForceNew:     true,
@@ -95,8 +95,8 @@ func resourceAwsEfsFileSystemCreate(d *schema.ResourceData, meta interface{}) er
 				return nil, "error", err
 			}
 
-			if len(resp.FileSystems) < 1 {
+			if hasEmptyFileSystems(resp) {
-				return nil, "not-found", fmt.Errorf("EFS file system %q not found", d.Id())
+				return nil, "not-found", fmt.Errorf("EFS file system %q could not be found.", d.Id())
 			}
 
 			fs := resp.FileSystems[0]
@@ -110,7 +110,7 @@ func resourceAwsEfsFileSystemCreate(d *schema.ResourceData, meta interface{}) er
 
 	_, err = stateConf.WaitForState()
 	if err != nil {
-		return fmt.Errorf("Error waiting for EFS file system (%q) to create: %q",
+		return fmt.Errorf("Error waiting for EFS file system (%q) to create: %s",
 			d.Id(), err.Error())
 	}
 	log.Printf("[DEBUG] EFS file system %q created.", d.Id())
@@ -122,7 +122,7 @@ func resourceAwsEfsFileSystemUpdate(d *schema.ResourceData, meta interface{}) er
 	conn := meta.(*AWSClient).efsconn
 	err := setTagsEFS(conn, d)
 	if err != nil {
-		return fmt.Errorf("Error setting EC2 tags for EFS file system (%q): %q",
+		return fmt.Errorf("Error setting EC2 tags for EFS file system (%q): %s",
 			d.Id(), err.Error())
 	}
 
@@ -137,25 +137,29 @@ func resourceAwsEfsFileSystemRead(d *schema.ResourceData, meta interface{}) erro
 	})
 	if err != nil {
 		if awsErr, ok := err.(awserr.Error); ok && awsErr.Code() == "FileSystemNotFound" {
-			log.Printf("[WARN] EFS File System (%s) not found, error code (404)", d.Id())
+			log.Printf("[WARN] EFS file system (%s) could not be found.", d.Id())
 			d.SetId("")
 			return nil
 		}
 		return err
 	}
-	if len(resp.FileSystems) < 1 {
+
-		return fmt.Errorf("EFS file system %q not found", d.Id())
+	if hasEmptyFileSystems(resp) {
+		return fmt.Errorf("EFS file system %q could not be found.", d.Id())
 	}
 
 	tagsResp, err := conn.DescribeTags(&efs.DescribeTagsInput{
 		FileSystemId: aws.String(d.Id()),
 	})
 	if err != nil {
-		return fmt.Errorf("Error retrieving EC2 tags for EFS file system (%q): %q",
+		return fmt.Errorf("Error retrieving EC2 tags for EFS file system (%q): %s",
 			d.Id(), err.Error())
 	}
 
-	d.Set("tags", tagsToMapEFS(tagsResp.Tags))
+	err = d.Set("tags", tagsToMapEFS(tagsResp.Tags))
+	if err != nil {
+		return err
+	}
 
 	return nil
 }
@@ -182,7 +186,7 @@ func resourceAwsEfsFileSystemDelete(d *schema.ResourceData, meta interface{}) er
 				return nil, "error", err
 			}
 
-			if len(resp.FileSystems) < 1 {
+			if hasEmptyFileSystems(resp) {
 				return nil, "", nil
 			}
 
@@ -197,7 +201,7 @@ func resourceAwsEfsFileSystemDelete(d *schema.ResourceData, meta interface{}) er
 
 	_, err = stateConf.WaitForState()
 	if err != nil {
-		return fmt.Errorf("Error waiting for EFS file system (%q) to delete: %q",
+		return fmt.Errorf("Error waiting for EFS file system (%q) to delete: %s",
 			d.Id(), err.Error())
 	}
 
@@ -218,10 +222,17 @@ func validateReferenceName(v interface{}, k string) (ws []string, errors []error
 
 func validatePerformanceModeType(v interface{}, k string) (ws []string, errors []error) {
 	value := v.(string)
-	if value != "generalPurpose" && value != "maxIO" {
+	if value != efs.PerformanceModeGeneralPurpose && value != efs.PerformanceModeMaxIo {
 		errors = append(errors, fmt.Errorf(
-			"%q contains an invalid Performance Mode %q. Valid modes are either %q or %q",
+			"%q contains an invalid Performance Mode %q. Valid modes are either %q or %q.",
-			k, value, "generalPurpose", "maxIO"))
+			k, value, efs.PerformanceModeGeneralPurpose, efs.PerformanceModeMaxIo))
 	}
 	return
 }
+
+func hasEmptyFileSystems(fs *efs.DescribeFileSystemsOutput) bool {
+	if fs != nil && len(fs.FileSystems) > 0 {
+		return false
+	}
+	return true
+}

builtin/providers/aws/resource_aws_efs_file_system_test.go

@@ -14,7 +14,7 @@ import (
 	"github.com/hashicorp/terraform/terraform"
 )
 
-func TestResourceAWSEFSReferenceName_validation(t *testing.T) {
+func TestResourceAWSEFSFileSystem_validateReferenceName(t *testing.T) {
 	var value string
 	var errors []error
 
@@ -27,35 +27,20 @@ func TestResourceAWSEFSReferenceName_validation(t *testing.T) {
 	value = acctest.RandString(32)
 	_, errors = validateReferenceName(value, "reference_name")
 	if len(errors) != 0 {
-		t.Fatalf("Expected to trigger a validation error")
+		t.Fatalf("Expected not to trigger a validation error")
 	}
 }
 
-func TestResourceAWSEFSPerformanceMode_validation(t *testing.T) {
+func TestResourceAWSEFSFileSystem_validatePerformanceModeType(t *testing.T) {
-	type testCase struct {
+	_, errors := validatePerformanceModeType("incorrect", "performance_mode")
+	if len(errors) == 0 {
+		t.Fatalf("Expected to trigger a validation error")
+	}
+
+	var testCases = []struct {
 		Value    string
 		ErrCount int
-	}
+	}{
-
-	invalidCases := []testCase{
-		{
-			Value:    "garrusVakarian",
-			ErrCount: 1,
-		},
-		{
-			Value:    acctest.RandString(80),
-			ErrCount: 1,
-		},
-	}
-
-	for _, tc := range invalidCases {
-		_, errors := validatePerformanceModeType(tc.Value, "performance_mode")
-		if len(errors) != tc.ErrCount {
-			t.Fatalf("Expected to trigger a validation error")
-		}
-	}
-
-	validCases := []testCase{
 		{
 			Value:    "generalPurpose",
 			ErrCount: 0,
@@ -66,14 +51,36 @@ func TestResourceAWSEFSPerformanceMode_validation(t *testing.T) {
 		},
 	}
 
-	for _, tc := range validCases {
+	for _, tc := range testCases {
-		_, errors := validatePerformanceModeType(tc.Value, "aws_efs_file_system")
+		_, errors := validatePerformanceModeType(tc.Value, "performance_mode")
 		if len(errors) != tc.ErrCount {
 			t.Fatalf("Expected not to trigger a validation error")
 		}
 	}
 }
 
+func TestResourceAWSEFSFileSystem_hasEmptyFileSystems(t *testing.T) {
+	fs := &efs.DescribeFileSystemsOutput{
+		FileSystems: []*efs.FileSystemDescription{},
+	}
+
+	var actual bool
+
+	actual = hasEmptyFileSystems(fs)
+	if !actual {
+		t.Fatalf("Expected return value to be true, got %t", actual)
+	}
+
+	// Add an empty file system.
+	fs.FileSystems = append(fs.FileSystems, &efs.FileSystemDescription{})
+
+	actual = hasEmptyFileSystems(fs)
+	if actual {
+		t.Fatalf("Expected return value to be false, got %t", actual)
+	}
+
+}
+
 func TestAccAWSEFSFileSystem_basic(t *testing.T) {
 	resource.Test(t, resource.TestCase{
 		PreCheck:     func() { testAccPreCheck(t) },