provider/aws: Fix aws_ami_launch_permission refresh when AMI disappears (#13469)

Launch permissions are implicitly nuked if an AMI is removed for any
reason - Terraform should not error on refresh in this case, but rather
just see the launch permissions as gone and react appropriately.

builtin/providers/aws/resource_aws_ami_launch_permission.go

@@ -2,7 +2,11 @@ package aws
 
 import (
 	"fmt"
+	"log"
+	"strings"
+
 	"github.com/aws/aws-sdk-go/aws"
+	"github.com/aws/aws-sdk-go/aws/awserr"
 	"github.com/aws/aws-sdk-go/service/ec2"
 	"github.com/hashicorp/terraform/helper/schema"
 )
@@ -92,6 +96,12 @@ func hasLaunchPermission(conn *ec2.EC2, image_id string, account_id string) (boo
 		Attribute: aws.String("launchPermission"),
 	})
 	if err != nil {
+		// When an AMI disappears out from under a launch permission resource, we will
+		// see either InvalidAMIID.NotFound or InvalidAMIID.Unavailable.
+		if ec2err, ok := err.(awserr.Error); ok && strings.HasPrefix(ec2err.Code(), "InvalidAMIID") {
+			log.Printf("[DEBUG] %s no longer exists, so we'll drop launch permission for %s from the state", image_id, account_id)
+			return false, nil
+		}
 		return false, err
 	}
 

builtin/providers/aws/resource_aws_ami_launch_permission_test.go

@@ -2,15 +2,18 @@ package aws
 
 import (
 	"fmt"
-	r "github.com/hashicorp/terraform/helper/resource"
-	"github.com/hashicorp/terraform/terraform"
 	"os"
 	"testing"
+
+	"github.com/aws/aws-sdk-go/aws"
+	"github.com/aws/aws-sdk-go/service/ec2"
+	r "github.com/hashicorp/terraform/helper/resource"
+	"github.com/hashicorp/terraform/terraform"
 )
 
 func TestAccAWSAMILaunchPermission_Basic(t *testing.T) {
-	image_id := ""
-	account_id := os.Getenv("AWS_ACCOUNT_ID")
+	imageID := ""
+	accountID := os.Getenv("AWS_ACCOUNT_ID")
 
 	r.Test(t, r.TestCase{
 		PreCheck: func() {
@@ -23,19 +26,36 @@ func TestAccAWSAMILaunchPermission_Basic(t *testing.T) {
 		Steps: []r.TestStep{
 			// Scaffold everything
 			r.TestStep{
-				Config: testAccAWSAMILaunchPermissionConfig(account_id, true),
+				Config: testAccAWSAMILaunchPermissionConfig(accountID, true),
 				Check: r.ComposeTestCheckFunc(
-					testCheckResourceGetAttr("aws_ami_copy.test", "id", &image_id),
-					testAccAWSAMILaunchPermissionExists(account_id, &image_id),
+					testCheckResourceGetAttr("aws_ami_copy.test", "id", &imageID),
+					testAccAWSAMILaunchPermissionExists(accountID, &imageID),
 				),
 			},
 			// Drop just launch permission to test destruction
 			r.TestStep{
-				Config: testAccAWSAMILaunchPermissionConfig(account_id, false),
+				Config: testAccAWSAMILaunchPermissionConfig(accountID, false),
 				Check: r.ComposeTestCheckFunc(
-					testAccAWSAMILaunchPermissionDestroyed(account_id, &image_id),
+					testAccAWSAMILaunchPermissionDestroyed(accountID, &imageID),
 				),
 			},
+			// Re-add everything so we can test when AMI disappears
+			r.TestStep{
+				Config: testAccAWSAMILaunchPermissionConfig(accountID, true),
+				Check: r.ComposeTestCheckFunc(
+					testCheckResourceGetAttr("aws_ami_copy.test", "id", &imageID),
+					testAccAWSAMILaunchPermissionExists(accountID, &imageID),
+				),
+			},
+			// Here we delete the AMI to verify the follow-on refresh after this step
+			// should not error.
+			r.TestStep{
+				Config: testAccAWSAMILaunchPermissionConfig(accountID, true),
+				Check: r.ComposeTestCheckFunc(
+					testAccAWSAMIDisappears(&imageID),
+				),
+				ExpectNonEmptyPlan: true,
+			},
 		},
 	})
 }
@@ -58,31 +78,53 @@ func testCheckResourceGetAttr(name, key string, value *string) r.TestCheckFunc {
 	}
 }
 
-func testAccAWSAMILaunchPermissionExists(account_id string, image_id *string) r.TestCheckFunc {
+func testAccAWSAMILaunchPermissionExists(accountID string, imageID *string) r.TestCheckFunc {
 	return func(s *terraform.State) error {
 		conn := testAccProvider.Meta().(*AWSClient).ec2conn
-		if has, err := hasLaunchPermission(conn, *image_id, account_id); err != nil {
+		if has, err := hasLaunchPermission(conn, *imageID, accountID); err != nil {
 			return err
 		} else if !has {
-			return fmt.Errorf("launch permission does not exist for '%s' on '%s'", account_id, *image_id)
+			return fmt.Errorf("launch permission does not exist for '%s' on '%s'", accountID, *imageID)
 		}
 		return nil
 	}
 }
 
-func testAccAWSAMILaunchPermissionDestroyed(account_id string, image_id *string) r.TestCheckFunc {
+func testAccAWSAMILaunchPermissionDestroyed(accountID string, imageID *string) r.TestCheckFunc {
 	return func(s *terraform.State) error {
 		conn := testAccProvider.Meta().(*AWSClient).ec2conn
-		if has, err := hasLaunchPermission(conn, *image_id, account_id); err != nil {
+		if has, err := hasLaunchPermission(conn, *imageID, accountID); err != nil {
 			return err
 		} else if has {
-			return fmt.Errorf("launch permission still exists for '%s' on '%s'", account_id, *image_id)
+			return fmt.Errorf("launch permission still exists for '%s' on '%s'", accountID, *imageID)
 		}
 		return nil
 	}
 }
 
-func testAccAWSAMILaunchPermissionConfig(account_id string, includeLaunchPermission bool) string {
+// testAccAWSAMIDisappears is technically a "test check function" but really it
+// exists to perform a side effect of deleting an AMI out from under a resource
+// so we can test that Terraform will react properly
+func testAccAWSAMIDisappears(imageID *string) r.TestCheckFunc {
+	return func(s *terraform.State) error {
+		conn := testAccProvider.Meta().(*AWSClient).ec2conn
+		req := &ec2.DeregisterImageInput{
+			ImageId: aws.String(*imageID),
+		}
+
+		_, err := conn.DeregisterImage(req)
+		if err != nil {
+			return err
+		}
+
+		if err := resourceAwsAmiWaitForDestroy(*imageID, conn); err != nil {
+			return err
+		}
+		return nil
+	}
+}
+
+func testAccAWSAMILaunchPermissionConfig(accountID string, includeLaunchPermission bool) string {
 	base := `
 resource "aws_ami_copy" "test" {
   name = "launch-permission-test"
@@ -101,5 +143,5 @@ resource "aws_ami_launch_permission" "self-test" {
     image_id   = "${aws_ami_copy.test.id}"
     account_id = "%s"
 }
-`, account_id)
+`, accountID)
 }