Merge pull request #5844 from steve-thousand/sconrad/ACCT-10424_policy-state-drift

policy state drift

Author: vaishakdinesh

internal/services/api_token/resource_test.go

@@ -1,32 +1,50 @@
 package api_token_test
 
 import (
+	"fmt"
 	"testing"
 
 	"github.com/cloudflare/terraform-provider-cloudflare/internal/acctest"
 	"github.com/cloudflare/terraform-provider-cloudflare/internal/utils"
 	"github.com/hashicorp/terraform-plugin-testing/helper/resource"
+	"github.com/hashicorp/terraform-plugin-testing/plancheck"
 )
 
 func TestAccAPIToken_Basic(t *testing.T) {
 	rnd := utils.GenerateRandomResourceName()
 	resourceID := "cloudflare_api_token." + rnd
 	permissionID := "82e64a83756745bbbb1c9c2701bf816b" // DNS read
 
+	var policyId string
+
 	resource.Test(t, resource.TestCase{
-		PreCheck:                 func() { acctest.TestAccPreCheck(t) },
+		PreCheck:                 func() { acctest.TestAccPreCheck_APIToken(t) },
 		ProtoV6ProviderFactories: acctest.TestAccProtoV6ProviderFactories,
 		Steps: []resource.TestStep{
 			{
 				Config: testAccCloudflareAPITokenWithoutCondition(rnd, rnd, permissionID),
 				Check: resource.ComposeTestCheckFunc(
 					resource.TestCheckResourceAttr(resourceID, "name", rnd),
+					resource.TestCheckResourceAttrSet(resourceID, "policies.0.id"),
+					resource.TestCheckResourceAttrWith(resourceID, "policies.0.id", func(value string) error {
+						policyId = value
+						return nil
+					}),
+					resource.TestCheckResourceAttr(resourceID, "policies.0.permission_groups.0.id", permissionID),
 				),
 			},
 			{
 				Config: testAccCloudflareAPITokenWithoutCondition(rnd, rnd+"-updated", permissionID),
 				Check: resource.ComposeTestCheckFunc(
 					resource.TestCheckResourceAttr(resourceID, "name", rnd+"-updated"),
+					resource.TestCheckResourceAttrSet(resourceID, "policies.0.id"),
+					resource.TestCheckResourceAttrWith(resourceID, "policies.0.id", func(value string) error {
+						if value != policyId {
+							return fmt.Errorf("policy ID changed from %s to %s", policyId, value)
+						}
+						return nil
+					}),
+					resource.TestCheckResourceAttr(resourceID, "policies.0.permission_groups.0.id", permissionID),
 				),
 			},
 		},
@@ -39,7 +57,7 @@ func TestAccAPIToken_DoesNotSetConditions(t *testing.T) {
 	permissionID := "82e64a83756745bbbb1c9c2701bf816b" // DNS read
 
 	resource.Test(t, resource.TestCase{
-		PreCheck:                 func() { acctest.TestAccPreCheck(t) },
+		PreCheck:                 func() { acctest.TestAccPreCheck_APIToken(t) },
 		ProtoV6ProviderFactories: acctest.TestAccProtoV6ProviderFactories,
 		Steps: []resource.TestStep{
 			{
@@ -64,7 +82,7 @@ func TestAccAPIToken_SetIndividualCondition(t *testing.T) {
 	permissionID := "82e64a83756745bbbb1c9c2701bf816b" // DNS read
 
 	resource.Test(t, resource.TestCase{
-		PreCheck:                 func() { acctest.TestAccPreCheck(t) },
+		PreCheck:                 func() { acctest.TestAccPreCheck_APIToken(t) },
 		ProtoV6ProviderFactories: acctest.TestAccProtoV6ProviderFactories,
 		Steps: []resource.TestStep{
 			{
@@ -89,7 +107,7 @@ func TestAccAPIToken_SetAllCondition(t *testing.T) {
 	permissionID := "82e64a83756745bbbb1c9c2701bf816b" // DNS read
 
 	resource.Test(t, resource.TestCase{
-		PreCheck:                 func() { acctest.TestAccPreCheck(t) },
+		PreCheck:                 func() { acctest.TestAccPreCheck_APIToken(t) },
 		ProtoV6ProviderFactories: acctest.TestAccProtoV6ProviderFactories,
 		Steps: []resource.TestStep{
 			{
@@ -114,7 +132,7 @@ func TestAccAPIToken_TokenTTL(t *testing.T) {
 	permissionID := "82e64a83756745bbbb1c9c2701bf816b" // DNS read
 
 	resource.Test(t, resource.TestCase{
-		PreCheck:                 func() { acctest.TestAccPreCheck(t) },
+		PreCheck:                 func() { acctest.TestAccPreCheck_APIToken(t) },
 		ProtoV6ProviderFactories: acctest.TestAccProtoV6ProviderFactories,
 		Steps: []resource.TestStep{
 			{
@@ -132,3 +150,67 @@ func TestAccAPIToken_TokenTTL(t *testing.T) {
 func testAccCloudflareAPITokenWithTTL(rnd string, permissionID string) string {
 	return acctest.LoadTestCase("apitokenwithttl.tf", rnd, permissionID)
 }
+
+func TestAccAPIToken_PermissionGroupOrder(t *testing.T) {
+	rnd := utils.GenerateRandomResourceName()
+	name := "cloudflare_api_token." + rnd
+	permissionID1 := "82e64a83756745bbbb1c9c2701bf816b" // DNS read
+	permissionID2 := "e199d584e69344eba202452019deafe3" // Disable ESC read
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:                 func() { acctest.TestAccPreCheck_APIToken(t) },
+		ProtoV6ProviderFactories: acctest.TestAccProtoV6ProviderFactories,
+		Steps: []resource.TestStep{
+			{
+				Config: acctest.LoadTestCase("api_token-permissiongroup-order.tf", rnd, permissionID1, permissionID2),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr(name, "name", rnd),
+					resource.TestCheckResourceAttr(name, "policies.0.permission_groups.0.id", permissionID1),
+					resource.TestCheckResourceAttr(name, "policies.0.permission_groups.1.id", permissionID2),
+				),
+			},
+			{
+				Config: acctest.LoadTestCase("api_token-permissiongroup-order.tf", rnd, permissionID2, permissionID1),
+				// changing the order of permission groups should not affect plan
+				ConfigPlanChecks: resource.ConfigPlanChecks{
+					PreApply: []plancheck.PlanCheck{
+						plancheck.ExpectEmptyPlan(),
+					},
+				},
+			},
+		},
+	})
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:                 func() { acctest.TestAccPreCheck_APIToken(t) },
+		ProtoV6ProviderFactories: acctest.TestAccProtoV6ProviderFactories,
+		Steps: []resource.TestStep{
+			{
+				Config: acctest.LoadTestCase("api_token-permissiongroup-order.tf", rnd, permissionID2, permissionID1),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr(name, "name", rnd),
+					resource.TestCheckResourceAttr(name, "policies.0.permission_groups.0.id", permissionID1),
+					resource.TestCheckResourceAttr(name, "policies.0.permission_groups.1.id", permissionID2),
+				),
+			},
+			{
+				Config: acctest.LoadTestCase("api_token-permissiongroup-order.tf", rnd, permissionID2, permissionID1),
+				// re-applying same change does not produce drift
+				ConfigPlanChecks: resource.ConfigPlanChecks{
+					PreApply: []plancheck.PlanCheck{
+						plancheck.ExpectEmptyPlan(),
+					},
+				},
+			},
+			{
+				Config: acctest.LoadTestCase("api_token-permissiongroup-order.tf", rnd, permissionID1, permissionID2),
+				// changing the order of permission groups should not affect plan
+				ConfigPlanChecks: resource.ConfigPlanChecks{
+					PreApply: []plancheck.PlanCheck{
+						plancheck.ExpectEmptyPlan(),
+					},
+				},
+			},
+		},
+	})
+}

internal/services/api_token/schema.go

@@ -37,6 +37,9 @@ func ResourceSchema(ctx context.Context) schema.Schema {
 						"id": schema.StringAttribute{
 							Description: "Policy identifier.",
 							Computed:    true,
+							PlanModifiers: []planmodifier.String{
+								stringplanmodifier.UseStateForUnknown(),
+							},
 						},
 						"effect": schema.StringAttribute{
 							Description: "Allow or deny operations against the resources.\nAvailable values: \"allow\", \"deny\".",

internal/services/api_token/testdata/api_token-permissiongroup-order.tf

@@ -0,0 +1,21 @@
+resource "cloudflare_api_token" "%[1]s" {
+  name = "%[1]s"
+  status = "active"
+
+  policies = [{
+    effect = "allow"
+    permission_groups = [{
+      id = "%[2]s"
+    },{
+      id = "%[3]s"
+    }]
+    resources = {
+      "com.cloudflare.api.account.zone.*" = "*"
+    }
+  }]
+}
+
+data "cloudflare_api_token" "%[1]s" {
+  token_id = cloudflare_api_token.%[1]s.id
+  depends_on = [cloudflare_api_token.%[1]s]
+}