r/sns_topic_policy - add disappears + refactor tests

.changelog/14123.txt

@@ -0,0 +1,7 @@
+```release-note:enhancement
+resource/aws_sns_topic_policy: Add plan time validation to `arn`
+```
+
+```release-note:enhancement
+resource/aws_sns_topic_policy: Add `owner` attribute
+```

aws/resource_aws_sns_topic_policy.go

@@ -3,10 +3,8 @@ package aws
 import (
 	"fmt"
 	"log"
-	"regexp"
 
 	"github.com/aws/aws-sdk-go/aws"
-	"github.com/aws/aws-sdk-go/aws/awserr"
 	"github.com/aws/aws-sdk-go/service/sns"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
@@ -25,16 +23,21 @@ func resourceAwsSnsTopicPolicy() *schema.Resource {
 
 		Schema: map[string]*schema.Schema{
 			"arn": {
-				Type:     schema.TypeString,
-				Required: true,
-				ForceNew: true,
+				Type:         schema.TypeString,
+				Required:     true,
+				ForceNew:     true,
+				ValidateFunc: validateArn,
 			},
 			"policy": {
 				Type:             schema.TypeString,
 				Required:         true,
 				ValidateFunc:     validation.StringIsJSON,
 				DiffSuppressFunc: suppressEquivalentAwsPolicyDiffs,
 			},
+			"owner": {
+				Type:     schema.TypeString,
+				Computed: true,
+			},
 		},
 	}
 }
@@ -64,14 +67,14 @@ func resourceAwsSnsTopicPolicyUpsert(d *schema.ResourceData, meta interface{}) e
 }
 
 func resourceAwsSnsTopicPolicyRead(d *schema.ResourceData, meta interface{}) error {
-	snsconn := meta.(*AWSClient).snsconn
+	conn := meta.(*AWSClient).snsconn
 
-	attributeOutput, err := snsconn.GetTopicAttributes(&sns.GetTopicAttributesInput{
+	attributeOutput, err := conn.GetTopicAttributes(&sns.GetTopicAttributesInput{
 		TopicArn: aws.String(d.Id()),
 	})
 	if err != nil {
-		if awsErr, ok := err.(awserr.Error); ok && awsErr.Code() == "NotFound" {
-			log.Printf("[WARN] SNS Topic (%s) not found, error code (404)", d.Id())
+		if isAWSErr(err, sns.ErrCodeNotFoundException, "") {
+			log.Printf("[WARN] SNS Topic (%s) not found, removing from state", d.Id())
 			d.SetId("")
 			return nil
 		}
@@ -80,62 +83,47 @@ func resourceAwsSnsTopicPolicyRead(d *schema.ResourceData, meta interface{}) err
 	}
 
 	if attributeOutput.Attributes == nil {
-		log.Printf("[WARN] SNS Topic (%q) attributes not found (nil)", d.Id())
+		log.Printf("[WARN] SNS Topic (%q) attributes not found (nil), removing from state", d.Id())
 		d.SetId("")
 		return nil
 	}
 	attrmap := attributeOutput.Attributes
 
 	policy, ok := attrmap["Policy"]
 	if !ok {
-		log.Printf("[WARN] SNS Topic (%q) policy not found in attributes", d.Id())
+		log.Printf("[WARN] SNS Topic (%q) policy not found in attributes, removing from state", d.Id())
 		d.SetId("")
 		return nil
 	}
 
 	d.Set("policy", policy)
 	d.Set("arn", attrmap["TopicArn"])
+	d.Set("owner", attrmap["Owner"])
 
 	return nil
 }
 
 func resourceAwsSnsTopicPolicyDelete(d *schema.ResourceData, meta interface{}) error {
-	accountId, err := getAccountIdFromSnsTopicArn(d.Id(), meta.(*AWSClient).partition)
-	if err != nil {
-		return err
-	}
-
 	req := sns.SetTopicAttributesInput{
 		TopicArn:      aws.String(d.Id()),
 		AttributeName: aws.String("Policy"),
 		// It is impossible to delete a policy or set to empty
 		// (confirmed by AWS Support representative)
 		// so we instead set it back to the default one
-		AttributeValue: aws.String(buildDefaultSnsTopicPolicy(d.Id(), accountId)),
+		AttributeValue: aws.String(buildDefaultSnsTopicPolicy(d.Id(), d.Get("owner").(string))),
 	}
 
 	// Retry the update in the event of an eventually consistent style of
 	// error, where say an IAM resource is successfully created but not
 	// actually available. See https://github.com/hashicorp/terraform/issues/3660
 	log.Printf("[DEBUG] Resetting SNS Topic Policy to default: %s", req)
 	conn := meta.(*AWSClient).snsconn
-	_, err = retryOnAwsCode("InvalidParameter", func() (interface{}, error) {
+	_, err := retryOnAwsCode("InvalidParameter", func() (interface{}, error) {
 		return conn.SetTopicAttributes(&req)
 	})
 	return err
 }
 
-func getAccountIdFromSnsTopicArn(arn, partition string) (string, error) {
-	// arn:aws:sns:us-west-2:123456789012:test-new
-	// arn:aws-us-gov:sns:us-west-2:123456789012:test-new
-	re := regexp.MustCompile(fmt.Sprintf("^arn:%s:sns:[^:]+:([0-9]{12}):.+", partition))
-	matches := re.FindStringSubmatch(arn)
-	if len(matches) != 2 {
-		return "", fmt.Errorf("Unable to get account ID from ARN (%q)", arn)
-	}
-	return matches[1], nil
-}
-
 func buildDefaultSnsTopicPolicy(topicArn, accountId string) string {
 	return fmt.Sprintf(`{
   "Version": "2008-10-17",

aws/resource_aws_sns_topic_policy_test.go

@@ -1,29 +1,36 @@
 package aws
 
 import (
+	"fmt"
 	"regexp"
 	"testing"
 
+	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/service/sns"
+	"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"
 )
 
 func TestAccAWSSNSTopicPolicy_basic(t *testing.T) {
 	attributes := make(map[string]string)
-	resourceName := "aws_sns_topic_policy.custom"
+	resourceName := "aws_sns_topic_policy.test"
+	rName := acctest.RandomWithPrefix("tf-acc-test")
 
 	resource.ParallelTest(t, resource.TestCase{
 		PreCheck:     func() { testAccPreCheck(t) },
 		ErrorCheck:   testAccErrorCheck(t, sns.EndpointsID),
 		Providers:    testAccProviders,
-		CheckDestroy: testAccCheckAWSSNSTopicDestroy,
+		CheckDestroy: testAccCheckAWSSNSTopicPolicyDestroy,
 		Steps: []resource.TestStep{
 			{
-				Config: testAccAWSSNSTopicConfig_withPolicy,
+				Config: testAccAWSSNSTopicPolicyBasicConfig(rName),
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckAWSSNSTopicExists("aws_sns_topic.test", attributes),
+					resource.TestCheckResourceAttrPair(resourceName, "arn", "aws_sns_topic.test", "arn"),
 					resource.TestMatchResourceAttr(resourceName, "policy",
-						regexp.MustCompile("^{\"Version\":\"2012-10-17\".+")),
+						regexp.MustCompile(fmt.Sprintf("\"Sid\":\"%[1]s\"", rName))),
+					testAccCheckResourceAttrAccountID(resourceName, "owner"),
 				),
 			},
 			{
@@ -35,36 +42,180 @@ func TestAccAWSSNSTopicPolicy_basic(t *testing.T) {
 	})
 }
 
-const testAccAWSSNSTopicConfig_withPolicy = `
+func TestAccAWSSNSTopicPolicy_updated(t *testing.T) {
+	attributes := make(map[string]string)
+	resourceName := "aws_sns_topic_policy.test"
+	rName := acctest.RandomWithPrefix("tf-acc-test")
+
+	resource.ParallelTest(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		ErrorCheck:   testAccErrorCheck(t, sns.EndpointsID),
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckAWSSNSTopicPolicyDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config: testAccAWSSNSTopicPolicyBasicConfig(rName),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckAWSSNSTopicExists("aws_sns_topic.test", attributes),
+					resource.TestMatchResourceAttr(resourceName, "policy",
+						regexp.MustCompile(fmt.Sprintf("\"Sid\":\"%[1]s\"", rName))),
+				),
+			},
+			{
+				ResourceName:      resourceName,
+				ImportState:       true,
+				ImportStateVerify: true,
+			},
+			{
+				Config: testAccAWSSNSTopicPolicyUpdatedConfig(rName),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckAWSSNSTopicExists("aws_sns_topic.test", attributes),
+					resource.TestMatchResourceAttr(resourceName, "policy",
+						regexp.MustCompile(fmt.Sprintf("\"Sid\":\"%[1]s\"", rName))),
+					resource.TestMatchResourceAttr(resourceName, "policy",
+						regexp.MustCompile("SNS:DeleteTopic")),
+				),
+			},
+		},
+	})
+}
+
+func TestAccAWSSNSTopicPolicy_disappears_topic(t *testing.T) {
+	attributes := make(map[string]string)
+	topicResourceName := "aws_sns_topic.test"
+	rName := acctest.RandomWithPrefix("tf-acc-test")
+
+	resource.ParallelTest(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		ErrorCheck:   testAccErrorCheck(t, sns.EndpointsID),
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckAWSSNSTopicPolicyDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config: testAccAWSSNSTopicPolicyBasicConfig(rName),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckAWSSNSTopicExists(topicResourceName, attributes),
+					testAccCheckResourceDisappears(testAccProvider, resourceAwsSnsTopic(), topicResourceName),
+				),
+				ExpectNonEmptyPlan: true,
+			},
+		},
+	})
+}
+
+func TestAccAWSSNSTopicPolicy_disappears(t *testing.T) {
+	attributes := make(map[string]string)
+	resourceName := "aws_sns_topic_policy.test"
+	rName := acctest.RandomWithPrefix("tf-acc-test")
+
+	resource.ParallelTest(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		ErrorCheck:   testAccErrorCheck(t, sns.EndpointsID),
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckAWSSNSTopicPolicyDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config: testAccAWSSNSTopicPolicyBasicConfig(rName),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckAWSSNSTopicExists("aws_sns_topic.test", attributes),
+					testAccCheckResourceDisappears(testAccProvider, resourceAwsSnsTopicPolicy(), resourceName),
+				),
+				ExpectNonEmptyPlan: true,
+			},
+		},
+	})
+}
+
+func testAccCheckAWSSNSTopicPolicyDestroy(s *terraform.State) error {
+	conn := testAccProvider.Meta().(*AWSClient).snsconn
+
+	for _, rs := range s.RootModule().Resources {
+		if rs.Type != "aws_sns_topic_policy" {
+			continue
+		}
+
+		// Check if the topic policy exists by fetching its attributes
+		params := &sns.GetTopicAttributesInput{
+			TopicArn: aws.String(rs.Primary.ID),
+		}
+		_, err := conn.GetTopicAttributes(params)
+		if err != nil {
+			if isAWSErr(err, sns.ErrCodeNotFoundException, "") {
+				return nil
+			}
+			return err
+		}
+		return fmt.Errorf("SNS Topic Policy (%s) exists when it should be destroyed", rs.Primary.ID)
+	}
+
+	return nil
+}
+
+func testAccAWSSNSTopicPolicyBasicConfig(rName string) string {
+	return fmt.Sprintf(`
 resource "aws_sns_topic" "test" {
-  name = "tf-acc-test-topic-with-policy"
+  name = %[1]q
+}
+
+resource "aws_sns_topic_policy" "test" {
+  arn    = aws_sns_topic.test.arn
+  policy = <<POLICY
+{
+  "Version":"2012-10-17",
+  "Id":"default",
+  "Statement":[
+    {
+      "Sid":"%[1]s",
+      "Effect":"Allow",
+      "Principal":{
+        "AWS":"*"
+      },
+      "Action":[
+        "SNS:GetTopicAttributes",
+        "SNS:SetTopicAttributes",
+        "SNS:AddPermission",
+        "SNS:RemovePermission"
+      ],
+      "Resource":"${aws_sns_topic.test.arn}"
+    }
+  ]
+}
+POLICY
+}
+`, rName)
 }
 
-resource "aws_sns_topic_policy" "custom" {
-  arn = aws_sns_topic.test.arn
+func testAccAWSSNSTopicPolicyUpdatedConfig(rName string) string {
+	return fmt.Sprintf(`
+resource "aws_sns_topic" "test" {
+  name = %[1]q
+}
 
+resource "aws_sns_topic_policy" "test" {
+  arn    = aws_sns_topic.test.arn
   policy = <<POLICY
 {
-  "Version": "2012-10-17",
-  "Id": "default",
-  "Statement": [
+  "Version":"2012-10-17",
+  "Id":"default",
+  "Statement":[
     {
-      "Sid": "default",
-      "Effect": "Allow",
-      "Principal": {
-        "AWS": "*"
+      "Sid":"%[1]s",
+      "Effect":"Allow",
+      "Principal":{
+        "AWS":"*"
       },
-      "Action": [
+      "Action":[
         "SNS:GetTopicAttributes",
         "SNS:SetTopicAttributes",
         "SNS:AddPermission",
         "SNS:RemovePermission",
         "SNS:DeleteTopic"
       ],
-      "Resource": "${aws_sns_topic.test.arn}"
+      "Resource":"${aws_sns_topic.test.arn}"
     }
   ]
 }
 POLICY
 }
-`
+`, rName)
+}

website/docs/r/sns_topic_policy.html.markdown

@@ -75,7 +75,9 @@ The following arguments are supported:
 
 ## Attributes Reference
 
-No additional attributes are exported.
+In addition to all arguments above, the following attributes are exported:
+
+* `owner` - The AWS Account ID of the SNS topic owner
 
 ## Import