Prevent importing Organization policy

.gitignore

@@ -36,3 +36,5 @@ markdown-link-check*.txt
 *.winfile eol=crlf
 /.vs
 node_modules
+
+tests/resource_aws_organizations_policy/plugins

aws/resource_aws_organizations_policy.go

@@ -1,12 +1,14 @@
 package aws
 
 import (
+	"context"
 	"fmt"
 	"log"
 	"time"
 
 	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/service/organizations"
+	"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
@@ -15,12 +17,12 @@ import (
 
 func resourceAwsOrganizationsPolicy() *schema.Resource {
 	return &schema.Resource{
-		Create: resourceAwsOrganizationsPolicyCreate,
-		Read:   resourceAwsOrganizationsPolicyRead,
-		Update: resourceAwsOrganizationsPolicyUpdate,
-		Delete: resourceAwsOrganizationsPolicyDelete,
+		CreateContext: resourceAwsOrganizationsPolicyCreate,
+		ReadContext:   resourceAwsOrganizationsPolicyRead,
+		UpdateContext: resourceAwsOrganizationsPolicyUpdate,
+		DeleteContext: resourceAwsOrganizationsPolicyDelete,
 		Importer: &schema.ResourceImporter{
-			State: schema.ImportStatePassthrough,
+			StateContext: resourceAwsOrganizationsPolicyImport,
 		},
 
 		Schema: map[string]*schema.Schema{
@@ -43,37 +45,31 @@ func resourceAwsOrganizationsPolicy() *schema.Resource {
 				Required: true,
 			},
 			"type": {
-				Type:     schema.TypeString,
-				Optional: true,
-				ForceNew: true,
-				Default:  organizations.PolicyTypeServiceControlPolicy,
-				ValidateFunc: validation.StringInSlice([]string{
-					organizations.PolicyTypeAiservicesOptOutPolicy,
-					organizations.PolicyTypeBackupPolicy,
-					organizations.PolicyTypeServiceControlPolicy,
-					organizations.PolicyTypeTagPolicy,
-				}, false),
+				Type:         schema.TypeString,
+				Optional:     true,
+				ForceNew:     true,
+				Default:      organizations.PolicyTypeServiceControlPolicy,
+				ValidateFunc: validation.StringInSlice(organizations.PolicyType_Values(), false),
 			},
 			"tags": tagsSchema(),
 		},
 	}
 }
 
-func resourceAwsOrganizationsPolicyCreate(d *schema.ResourceData, meta interface{}) error {
+func resourceAwsOrganizationsPolicyCreate(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
 	conn := meta.(*AWSClient).organizationsconn
 
-	// Description is required:
-	// InvalidParameter: 1 validation error(s) found.
-	// - missing required field, CreatePolicyInput.Description.
+	name := d.Get("name").(string)
+
 	input := &organizations.CreatePolicyInput{
 		Content:     aws.String(d.Get("content").(string)),
 		Description: aws.String(d.Get("description").(string)),
-		Name:        aws.String(d.Get("name").(string)),
+		Name:        aws.String(name),
 		Type:        aws.String(d.Get("type").(string)),
 		Tags:        keyvaluetags.New(d.Get("tags").(map[string]interface{})).IgnoreAws().OrganizationsTags(),
 	}
 
-	log.Printf("[DEBUG] Creating Organizations Policy: %s", input)
+	log.Printf("[DEBUG] Creating Organizations Policy (%s): %v", name, input)
 
 	var err error
 	var resp *organizations.CreatePolicyOutput
@@ -82,7 +78,7 @@ func resourceAwsOrganizationsPolicyCreate(d *schema.ResourceData, meta interface
 
 		if err != nil {
 			if isAWSErr(err, organizations.ErrCodeFinalizingOrganizationException, "") {
-				log.Printf("[DEBUG] Trying to create policy again: %q", err.Error())
+				log.Printf("[DEBUG] Retrying creating Organizations Policy (%s): %s", name, err)
 				return resource.RetryableError(err)
 			}
 
@@ -96,35 +92,35 @@ func resourceAwsOrganizationsPolicyCreate(d *schema.ResourceData, meta interface
 	}
 
 	if err != nil {
-		return fmt.Errorf("error creating Organizations Policy: %s", err)
+		return diag.FromErr(fmt.Errorf("error creating Organizations Policy (%s): %w", name, err))
 	}
 
-	d.SetId(*resp.Policy.PolicySummary.Id)
+	d.SetId(aws.StringValue(resp.Policy.PolicySummary.Id))
 
-	return resourceAwsOrganizationsPolicyRead(d, meta)
+	return resourceAwsOrganizationsPolicyRead(ctx, d, meta)
 }
 
-func resourceAwsOrganizationsPolicyRead(d *schema.ResourceData, meta interface{}) error {
+func resourceAwsOrganizationsPolicyRead(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
 	conn := meta.(*AWSClient).organizationsconn
 	ignoreTagsConfig := meta.(*AWSClient).IgnoreTagsConfig
 
 	input := &organizations.DescribePolicyInput{
 		PolicyId: aws.String(d.Id()),
 	}
 
-	log.Printf("[DEBUG] Reading Organizations Policy: %s", input)
+	log.Printf("[DEBUG] Reading Organizations policy: %s", input)
 	resp, err := conn.DescribePolicy(input)
 	if err != nil {
 		if isAWSErr(err, organizations.ErrCodePolicyNotFoundException, "") {
-			log.Printf("[WARN] Policy does not exist, removing from state: %s", d.Id())
+			log.Printf("[WARN] Organizations policy does not exist, removing from state: %s", d.Id())
 			d.SetId("")
 			return nil
 		}
-		return err
+		return diag.FromErr(fmt.Errorf("error reading Organizations Policy (%s): %w", d.Id(), err))
 	}
 
 	if resp.Policy == nil || resp.Policy.PolicySummary == nil {
-		log.Printf("[WARN] Policy does not exist, removing from state: %s", d.Id())
+		log.Printf("[WARN] Organizations policy does not exist, removing from state: %s", d.Id())
 		d.SetId("")
 		return nil
 	}
@@ -135,19 +131,29 @@ func resourceAwsOrganizationsPolicyRead(d *schema.ResourceData, meta interface{}
 	d.Set("name", resp.Policy.PolicySummary.Name)
 	d.Set("type", resp.Policy.PolicySummary.Type)
 
+	if aws.BoolValue(resp.Policy.PolicySummary.AwsManaged) {
+		return diag.Diagnostics{
+			diag.Diagnostic{
+				Severity: diag.Warning,
+				Summary:  "AWS-managed Organizations policies cannot be imported",
+				Detail:   fmt.Sprintf("This resource should be removed from your Terraform state using `terraform state rm` (https://www.terraform.io/docs/commands/state/rm.html) and references should use the ID (%s) directly.", d.Id()),
+			},
+		}
+	}
+
 	tags, err := keyvaluetags.OrganizationsListTags(conn, d.Id())
 	if err != nil {
-		return fmt.Errorf("error listing tags: %s", err)
+		return diag.FromErr(fmt.Errorf("error listing tags for Organizations policy (%s): %w", d.Id(), err))
 	}
 
 	if err := d.Set("tags", tags.IgnoreAws().IgnoreConfig(ignoreTagsConfig).Map()); err != nil {
-		return fmt.Errorf("error setting tags: %s", err)
+		return diag.FromErr(fmt.Errorf("error setting tags for Organizations policy (%s): %w", d.Id(), err))
 	}
 
 	return nil
 }
 
-func resourceAwsOrganizationsPolicyUpdate(d *schema.ResourceData, meta interface{}) error {
+func resourceAwsOrganizationsPolicyUpdate(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
 	conn := meta.(*AWSClient).organizationsconn
 
 	input := &organizations.UpdatePolicyInput{
@@ -169,33 +175,51 @@ func resourceAwsOrganizationsPolicyUpdate(d *schema.ResourceData, meta interface
 	log.Printf("[DEBUG] Updating Organizations Policy: %s", input)
 	_, err := conn.UpdatePolicy(input)
 	if err != nil {
-		return fmt.Errorf("error updating Organizations Policy: %s", err)
+		return diag.FromErr(fmt.Errorf("error updating Organizations policy (%s): %w", d.Id(), err))
 	}
 
 	if d.HasChange("tags") {
 		o, n := d.GetChange("tags")
 		if err := keyvaluetags.OrganizationsUpdateTags(conn, d.Id(), o, n); err != nil {
-			return fmt.Errorf("error updating tags: %s", err)
+			return diag.FromErr(fmt.Errorf("error updating tags for Organizations policy (%s): %w", d.Id(), err))
 		}
 	}
 
-	return resourceAwsOrganizationsPolicyRead(d, meta)
+	return resourceAwsOrganizationsPolicyRead(ctx, d, meta)
 }
 
-func resourceAwsOrganizationsPolicyDelete(d *schema.ResourceData, meta interface{}) error {
+func resourceAwsOrganizationsPolicyDelete(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
 	conn := meta.(*AWSClient).organizationsconn
 
 	input := &organizations.DeletePolicyInput{
 		PolicyId: aws.String(d.Id()),
 	}
 
-	log.Printf("[DEBUG] Deletion Organizations Policy: %s", input)
+	log.Printf("[DEBUG] Deleting Organizations Policy: %s", input)
 	_, err := conn.DeletePolicy(input)
 	if err != nil {
 		if isAWSErr(err, organizations.ErrCodePolicyNotFoundException, "") {
 			return nil
 		}
-		return err
+		return diag.FromErr(fmt.Errorf("error deleting Organizations policy (%s): %w", d.Id(), err))
 	}
 	return nil
 }
+
+func resourceAwsOrganizationsPolicyImport(ctx context.Context, d *schema.ResourceData, meta interface{}) ([]*schema.ResourceData, error) {
+	conn := meta.(*AWSClient).organizationsconn
+
+	input := &organizations.DescribePolicyInput{
+		PolicyId: aws.String(d.Id()),
+	}
+	resp, err := conn.DescribePolicyWithContext(ctx, input)
+	if err != nil {
+		return nil, err
+	}
+
+	if aws.BoolValue(resp.Policy.PolicySummary.AwsManaged) {
+		return nil, fmt.Errorf("AWS-managed Organizations policy (%s) cannot be imported. Use the policy ID directly in your configuration.", d.Id())
+	}
+
+	return []*schema.ResourceData{d}, nil
+}

aws/resource_aws_organizations_policy_attachment_test.go

@@ -96,7 +96,7 @@ func testAccAwsOrganizationsPolicyAttachment_Root(t *testing.T) {
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckAwsOrganizationsPolicyAttachmentExists(resourceName),
 					resource.TestCheckResourceAttrPair(resourceName, "policy_id", policyIdResourceName, "id"),
-					resource.TestCheckResourceAttrPair(resourceName, "target_id", targetIdResourceName, "roots[0].id"),
+					resource.TestCheckResourceAttrPair(resourceName, "target_id", targetIdResourceName, "roots.0.id"),
 				),
 			},
 			{

aws/resource_aws_organizations_policy_test.go

@@ -165,6 +165,28 @@ func testAccAwsOrganizationsPolicy_tags(t *testing.T) {
 	})
 }
 
+func testAccAwsOrganizationsPolicy_disappears(t *testing.T) {
+	var p organizations.Policy
+	rName := acctest.RandomWithPrefix("tf-acc-test")
+	resourceName := "aws_organizations_policy.test"
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t); testAccOrganizationsAccountPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckAwsOrganizationsPolicyDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config: testAccAwsOrganizationsPolicyConfig_Description(rName, ""),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckAwsOrganizationsPolicyExists(resourceName, &p),
+					testAccCheckResourceDisappears(testAccProvider, resourceAwsOrganizationsPolicy(), resourceName),
+				),
+				ExpectNonEmptyPlan: true,
+			},
+		},
+	})
+}
+
 func testAccAwsOrganizationsPolicy_type_AI_OPT_OUT(t *testing.T) {
 	var policy organizations.Policy
 	rName := acctest.RandomWithPrefix("tf-acc-test")
@@ -351,6 +373,31 @@ func testAccAwsOrganizationsPolicy_type_Tag(t *testing.T) {
 	})
 }
 
+func testAccAwsOrganizationsPolicy_ImportAwsManagedPolicy(t *testing.T) {
+	resourceName := "aws_organizations_policy.test"
+
+	resourceID := "p-FullAWSAccess"
+
+	t.Skip("This test requires SDK 2.0.4 or higher for `ExpectError` with `ImportState`")
+	resource.Test(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t); testAccOrganizationsAccountPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckAwsOrganizationsPolicyDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config: testAccAwsOrganizationsPolicyConfig_AwsManagedPolicySetup,
+			},
+			{
+				Config:        testAccAwsOrganizationsPolicyConfig_AwsManagedPolicy,
+				ResourceName:  resourceName,
+				ImportStateId: resourceID,
+				ImportState:   true,
+				ExpectError:   regexp.MustCompile(fmt.Sprintf("AWS-managed Organizations policy (%s) cannot be imported.", resourceID)),
+			},
+		},
+	})
+}
+
 func testAccCheckAwsOrganizationsPolicyDestroy(s *terraform.State) error {
 	conn := testAccProvider.Meta().(*AWSClient).organizationsconn
 
@@ -661,3 +708,19 @@ resource "aws_organizations_policy" "test" {
 }
 `, strconv.Quote(content), rName, policyType)
 }
+
+const testAccAwsOrganizationsPolicyConfig_AwsManagedPolicySetup = `
+resource "aws_organizations_organization" "test" {
+  enabled_policy_types = ["SERVICE_CONTROL_POLICY"]
+}
+`
+
+const testAccAwsOrganizationsPolicyConfig_AwsManagedPolicy = `
+resource "aws_organizations_organization" "test" {
+  enabled_policy_types = ["SERVICE_CONTROL_POLICY"]
+}
+
+resource "aws_organizations_policy" "test" {
+  name = "FullAWSAccess"
+}
+`

aws/resource_aws_organizations_test.go

@@ -26,14 +26,16 @@ func TestAccAWSOrganizations_serial(t *testing.T) {
 			"DataSource": testAccDataSourceAwsOrganizationsOrganizationalUnits_basic,
 		},
 		"Policy": {
-			"basic":           testAccAwsOrganizationsPolicy_basic,
-			"concurrent":      testAccAwsOrganizationsPolicy_concurrent,
-			"Description":     testAccAwsOrganizationsPolicy_description,
-			"Tags":            testAccAwsOrganizationsPolicy_tags,
-			"Type_AI_OPT_OUT": testAccAwsOrganizationsPolicy_type_AI_OPT_OUT,
-			"Type_Backup":     testAccAwsOrganizationsPolicy_type_Backup,
-			"Type_SCP":        testAccAwsOrganizationsPolicy_type_SCP,
-			"Type_Tag":        testAccAwsOrganizationsPolicy_type_Tag,
+			"basic":                  testAccAwsOrganizationsPolicy_basic,
+			"concurrent":             testAccAwsOrganizationsPolicy_concurrent,
+			"Description":            testAccAwsOrganizationsPolicy_description,
+			"Tags":                   testAccAwsOrganizationsPolicy_tags,
+			"disappears":             testAccAwsOrganizationsPolicy_disappears,
+			"Type_AI_OPT_OUT":        testAccAwsOrganizationsPolicy_type_AI_OPT_OUT,
+			"Type_Backup":            testAccAwsOrganizationsPolicy_type_Backup,
+			"Type_SCP":               testAccAwsOrganizationsPolicy_type_SCP,
+			"Type_Tag":               testAccAwsOrganizationsPolicy_type_Tag,
+			"ImportAwsManagedPolicy": testAccAwsOrganizationsPolicy_ImportAwsManagedPolicy,
 		},
 		"PolicyAttachment": {
 			"Account":            testAccAwsOrganizationsPolicyAttachment_Account,