BugFix: Add bypass_file_system_lockout_safety_check to efs efs_file_system_policy

.changelog/20838.txt

@@ -0,0 +1,3 @@
+```release-note:enhancement
+resource/aws_efs_file_system_policy: Add `bypass_policy_lockout_safety_check` argument
+```

aws/internal/service/efs/finder/finder.go

@@ -5,6 +5,7 @@ import (
 	"github.com/aws/aws-sdk-go/service/efs"
 	"github.com/hashicorp/aws-sdk-go-base/tfawserr"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
+	"github.com/terraform-providers/terraform-provider-aws/aws/internal/tfresource"
 )
 
 func BackupPolicyByID(conn *efs.EFS, id string) (*efs.BackupPolicy, error) {
@@ -26,11 +27,33 @@ func BackupPolicyByID(conn *efs.EFS, id string) (*efs.BackupPolicy, error) {
 	}
 
 	if output == nil || output.BackupPolicy == nil {
+		return nil, tfresource.NewEmptyResultError(input)
+	}
+
+	return output.BackupPolicy, nil
+}
+
+func FileSystemPolicyByID(conn *efs.EFS, id string) (*efs.DescribeFileSystemPolicyOutput, error) {
+	input := &efs.DescribeFileSystemPolicyInput{
+		FileSystemId: aws.String(id),
+	}
+
+	output, err := conn.DescribeFileSystemPolicy(input)
+
+	if tfawserr.ErrCodeEquals(err, efs.ErrCodeFileSystemNotFound) || tfawserr.ErrCodeEquals(err, efs.ErrCodePolicyNotFound) {
 		return nil, &resource.NotFoundError{
-			Message:     "Empty result",
+			LastError:   err,
 			LastRequest: input,
 		}
 	}
 
-	return output.BackupPolicy, nil
+	if err != nil {
+		return nil, err
+	}
+
+	if output == nil {
+		return nil, tfresource.NewEmptyResultError(input)
+	}
+
+	return output, nil
 }

aws/resource_aws_efs_file_system_policy.go

@@ -6,8 +6,11 @@ import (
 
 	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/service/efs"
+	"github.com/hashicorp/aws-sdk-go-base/tfawserr"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
+	"github.com/terraform-providers/terraform-provider-aws/aws/internal/service/efs/finder"
+	"github.com/terraform-providers/terraform-provider-aws/aws/internal/tfresource"
 )
 
 func resourceAwsEfsFileSystemPolicy() *schema.Resource {
@@ -16,12 +19,16 @@ func resourceAwsEfsFileSystemPolicy() *schema.Resource {
 		Read:   resourceAwsEfsFileSystemPolicyRead,
 		Update: resourceAwsEfsFileSystemPolicyPut,
 		Delete: resourceAwsEfsFileSystemPolicyDelete,
-
 		Importer: &schema.ResourceImporter{
 			State: schema.ImportStatePassthrough,
 		},
 
 		Schema: map[string]*schema.Schema{
+			"bypass_policy_lockout_safety_check": {
+				Type:     schema.TypeBool,
+				Optional: true,
+				Default:  false,
+			},
 			"file_system_id": {
 				Type:     schema.TypeString,
 				Required: true,
@@ -40,45 +47,42 @@ func resourceAwsEfsFileSystemPolicy() *schema.Resource {
 func resourceAwsEfsFileSystemPolicyPut(d *schema.ResourceData, meta interface{}) error {
 	conn := meta.(*AWSClient).efsconn
 
-	fsId := d.Get("file_system_id").(string)
+	fsID := d.Get("file_system_id").(string)
 	input := &efs.PutFileSystemPolicyInput{
-		FileSystemId: aws.String(fsId),
-		Policy:       aws.String(d.Get("policy").(string)),
+		BypassPolicyLockoutSafetyCheck: aws.Bool(d.Get("bypass_policy_lockout_safety_check").(bool)),
+		FileSystemId:                   aws.String(fsID),
+		Policy:                         aws.String(d.Get("policy").(string)),
 	}
-	log.Printf("[DEBUG] Adding EFS File System Policy: %#v", input)
+
+	log.Printf("[DEBUG] Putting EFS File System Policy: %s", input)
 	_, err := conn.PutFileSystemPolicy(input)
+
 	if err != nil {
-		return fmt.Errorf("error creating EFS File System Policy %q: %s", d.Id(), err.Error())
+		return fmt.Errorf("error putting EFS File System Policy (%s): %w", fsID, err)
 	}
 
-	d.SetId(fsId)
+	d.SetId(fsID)
 
 	return resourceAwsEfsFileSystemPolicyRead(d, meta)
 }
 
 func resourceAwsEfsFileSystemPolicyRead(d *schema.ResourceData, meta interface{}) error {
 	conn := meta.(*AWSClient).efsconn
 
-	var policyRes *efs.DescribeFileSystemPolicyOutput
-	policyRes, err := conn.DescribeFileSystemPolicy(&efs.DescribeFileSystemPolicyInput{
-		FileSystemId: aws.String(d.Id()),
-	})
+	output, err := finder.FileSystemPolicyByID(conn, d.Id())
+
+	if !d.IsNewResource() && tfresource.NotFound(err) {
+		log.Printf("[WARN] EFS File System Policy (%s) not found, removing from state", d.Id())
+		d.SetId("")
+		return nil
+	}
+
 	if err != nil {
-		if isAWSErr(err, efs.ErrCodeFileSystemNotFound, "") {
-			log.Printf("[WARN] EFS File System (%s) not found, removing from state", d.Id())
-			d.SetId("")
-			return nil
-		}
-		if isAWSErr(err, efs.ErrCodePolicyNotFound, "") {
-			log.Printf("[WARN] EFS File System Policy (%s) not found, removing from state", d.Id())
-			d.SetId("")
-			return nil
-		}
-		return fmt.Errorf("error describing policy for EFS file system (%s): %s", d.Id(), err)
+		return fmt.Errorf("error reading EFS File System Policy (%s): %w", d.Id(), err)
 	}
 
-	d.Set("file_system_id", policyRes.FileSystemId)
-	d.Set("policy", policyRes.Policy)
+	d.Set("file_system_id", output.FileSystemId)
+	d.Set("policy", output.Policy)
 
 	return nil
 }
@@ -90,11 +94,14 @@ func resourceAwsEfsFileSystemPolicyDelete(d *schema.ResourceData, meta interface
 	_, err := conn.DeleteFileSystemPolicy(&efs.DeleteFileSystemPolicyInput{
 		FileSystemId: aws.String(d.Id()),
 	})
-	if err != nil {
-		return fmt.Errorf("error deleting EFS File System Policy: %s with err %s", d.Id(), err.Error())
+
+	if tfawserr.ErrCodeEquals(err, efs.ErrCodeFileSystemNotFound) {
+		return nil
 	}
 
-	log.Printf("[DEBUG] EFS File System Policy %q deleted.", d.Id())
+	if err != nil {
+		return fmt.Errorf("error deleting EFS File System Policy (%s): %w", d.Id(), err)
+	}
 
 	return nil
 }

aws/resource_aws_efs_file_system_policy_test.go

@@ -4,11 +4,12 @@ import (
 	"fmt"
 	"testing"
 
-	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/service/efs"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/terraform"
+	"github.com/terraform-providers/terraform-provider-aws/aws/internal/service/efs/finder"
+	"github.com/terraform-providers/terraform-provider-aws/aws/internal/tfresource"
 )
 
 func TestAccAWSEFSFileSystemPolicy_basic(t *testing.T) {
@@ -20,24 +21,25 @@ func TestAccAWSEFSFileSystemPolicy_basic(t *testing.T) {
 		PreCheck:     func() { testAccPreCheck(t) },
 		ErrorCheck:   testAccErrorCheck(t, efs.EndpointsID),
 		Providers:    testAccProviders,
-		CheckDestroy: testAccCheckEfsFileSystemDestroy,
+		CheckDestroy: testAccCheckEfsFileSystemPolicyDestroy,
 		Steps: []resource.TestStep{
 			{
 				Config: testAccAWSEFSFileSystemPolicyConfig(rName),
 				Check: resource.ComposeTestCheckFunc(
-					testAccCheckEfsFileSystemPolicy(resourceName, &desc),
+					testAccCheckEfsFileSystemPolicyExists(resourceName, &desc),
 					resource.TestCheckResourceAttrSet(resourceName, "policy"),
 				),
 			},
 			{
-				ResourceName:      resourceName,
-				ImportState:       true,
-				ImportStateVerify: true,
+				ResourceName:            resourceName,
+				ImportState:             true,
+				ImportStateVerify:       true,
+				ImportStateVerifyIgnore: []string{"bypass_policy_lockout_safety_check"},
 			},
 			{
 				Config: testAccAWSEFSFileSystemPolicyConfigUpdated(rName),
 				Check: resource.ComposeTestCheckFunc(
-					testAccCheckEfsFileSystemPolicy(resourceName, &desc),
+					testAccCheckEfsFileSystemPolicyExists(resourceName, &desc),
 					resource.TestCheckResourceAttrSet(resourceName, "policy"),
 				),
 			},
@@ -59,7 +61,7 @@ func TestAccAWSEFSFileSystemPolicy_disappears(t *testing.T) {
 			{
 				Config: testAccAWSEFSFileSystemPolicyConfig(rName),
 				Check: resource.ComposeTestCheckFunc(
-					testAccCheckEfsFileSystemPolicy(resourceName, &desc),
+					testAccCheckEfsFileSystemPolicyExists(resourceName, &desc),
 					testAccCheckResourceDisappears(testAccProvider, resourceAwsEfsFileSystemPolicy(), resourceName),
 				),
 				ExpectNonEmptyPlan: true,
@@ -68,52 +70,85 @@ func TestAccAWSEFSFileSystemPolicy_disappears(t *testing.T) {
 	})
 }
 
+func TestAccAWSEFSFileSystemPolicy_PolicyBypass(t *testing.T) {
+	var desc efs.DescribeFileSystemPolicyOutput
+	resourceName := "aws_efs_file_system_policy.test"
+	rName := acctest.RandomWithPrefix("tf-acc-test")
+
+	resource.ParallelTest(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		ErrorCheck:   testAccErrorCheck(t, efs.EndpointsID),
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckEfsFileSystemPolicyDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config: testAccAWSEFSFileSystemPolicyConfig(rName),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckEfsFileSystemPolicyExists(resourceName, &desc),
+					resource.TestCheckResourceAttr(resourceName, "bypass_policy_lockout_safety_check", "false"),
+				),
+			},
+			{
+				ResourceName:            resourceName,
+				ImportState:             true,
+				ImportStateVerify:       true,
+				ImportStateVerifyIgnore: []string{"bypass_policy_lockout_safety_check"},
+			},
+			{
+				Config: testAccAWSEFSFileSystemPolicyBypassConfig(rName, true),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckEfsFileSystemPolicyExists(resourceName, &desc),
+					resource.TestCheckResourceAttr(resourceName, "bypass_policy_lockout_safety_check", "true"),
+				),
+			},
+		},
+	})
+}
+
 func testAccCheckEfsFileSystemPolicyDestroy(s *terraform.State) error {
 	conn := testAccProvider.Meta().(*AWSClient).efsconn
+
 	for _, rs := range s.RootModule().Resources {
 		if rs.Type != "aws_efs_file_system_policy" {
 			continue
 		}
 
-		resp, err := conn.DescribeFileSystemPolicy(&efs.DescribeFileSystemPolicyInput{
-			FileSystemId: aws.String(rs.Primary.ID),
-		})
-		if err != nil {
-			if isAWSErr(err, efs.ErrCodeFileSystemNotFound, "") ||
-				isAWSErr(err, efs.ErrCodePolicyNotFound, "") {
-				return nil
-			}
-			return fmt.Errorf("error describing EFS file system policy in tests: %s", err)
+		_, err := finder.FileSystemPolicyByID(conn, rs.Primary.ID)
+
+		if tfresource.NotFound(err) {
+			continue
 		}
-		if resp != nil {
-			return fmt.Errorf("EFS file system policy %q still exists", rs.Primary.ID)
+
+		if err != nil {
+			return err
 		}
+
+		return fmt.Errorf("EFS File System Policy %s still exists", rs.Primary.ID)
 	}
 
 	return nil
 }
 
-func testAccCheckEfsFileSystemPolicy(resourceID string, efsFsPolicy *efs.DescribeFileSystemPolicyOutput) resource.TestCheckFunc {
+func testAccCheckEfsFileSystemPolicyExists(n string, v *efs.DescribeFileSystemPolicyOutput) resource.TestCheckFunc {
 	return func(s *terraform.State) error {
-		rs, ok := s.RootModule().Resources[resourceID]
+		rs, ok := s.RootModule().Resources[n]
 		if !ok {
-			return fmt.Errorf("Not found: %s", resourceID)
+			return fmt.Errorf("Not found: %s", n)
 		}
 
 		if rs.Primary.ID == "" {
-			return fmt.Errorf("No ID is set")
+			return fmt.Errorf("No EFS File System Policy ID is set")
 		}
 
 		conn := testAccProvider.Meta().(*AWSClient).efsconn
-		fs, err := conn.DescribeFileSystemPolicy(&efs.DescribeFileSystemPolicyInput{
-			FileSystemId: aws.String(rs.Primary.ID),
-		})
+
+		output, err := finder.FileSystemPolicyByID(conn, rs.Primary.ID)
 
 		if err != nil {
 			return err
 		}
 
-		*efsFsPolicy = *fs
+		*v = *output
 
 		return nil
 	}
@@ -122,7 +157,7 @@ func testAccCheckEfsFileSystemPolicy(resourceID string, efsFsPolicy *efs.Describ
 func testAccAWSEFSFileSystemPolicyConfig(rName string) string {
 	return fmt.Sprintf(`
 resource "aws_efs_file_system" "test" {
-  creation_token = %q
+  creation_token = %[1]q
 }
 
 resource "aws_efs_file_system_policy" "test" {
@@ -160,7 +195,7 @@ POLICY
 func testAccAWSEFSFileSystemPolicyConfigUpdated(rName string) string {
 	return fmt.Sprintf(`
 resource "aws_efs_file_system" "test" {
-  creation_token = %q
+  creation_token = %[1]q
 }
 
 resource "aws_efs_file_system_policy" "test" {
@@ -191,3 +226,43 @@ POLICY
 }
 `, rName)
 }
+
+func testAccAWSEFSFileSystemPolicyBypassConfig(rName string, bypass bool) string {
+	return fmt.Sprintf(`
+resource "aws_efs_file_system" "test" {
+  creation_token = %[1]q
+}
+
+resource "aws_efs_file_system_policy" "test" {
+  file_system_id = aws_efs_file_system.test.id
+
+  bypass_policy_lockout_safety_check = %[2]t
+
+  policy = <<POLICY
+{
+    "Version": "2012-10-17",
+    "Id": "ExamplePolicy01",
+    "Statement": [
+        {
+            "Sid": "ExampleSatement01",
+            "Effect": "Allow",
+            "Principal": {
+                "AWS": "*"
+            },
+            "Resource": "${aws_efs_file_system.test.arn}",
+            "Action": [
+                "elasticfilesystem:ClientMount",
+                "elasticfilesystem:ClientWrite"
+            ],
+            "Condition": {
+                "Bool": {
+                    "aws:SecureTransport": "true"
+                }
+            }
+        }
+    ]
+}
+POLICY
+}
+`, rName, bypass)
+}