fix: set computed_optional on various fields since these fields are always returned to prevent update in place

Author: Sui Mak

internal/services/zone_subscription/model.go

@@ -15,7 +15,7 @@ type ZoneSubscriptionResultEnvelope struct {
 type ZoneSubscriptionModel struct {
 	ID                 types.String                   `tfsdk:"id" json:"-,computed"`
 	ZoneID             types.String                   `tfsdk:"zone_id" path:"zone_id,required"`
-	Frequency          types.String                   `tfsdk:"frequency" json:"frequency,optional"`
+	Frequency          types.String                   `tfsdk:"frequency" json:"frequency, computed_optional"`
 	RatePlan           *ZoneSubscriptionRatePlanModel `tfsdk:"rate_plan" json:"rate_plan,optional"`
 	Currency           types.String                   `tfsdk:"currency" json:"currency,computed"`
 	CurrentPeriodEnd   timetypes.RFC3339              `tfsdk:"current_period_end" json:"current_period_end,computed" format:"date-time"`
@@ -34,10 +34,10 @@ func (m ZoneSubscriptionModel) MarshalJSONForUpdate(state ZoneSubscriptionModel)
 
 type ZoneSubscriptionRatePlanModel struct {
 	ID                types.String    `tfsdk:"id" json:"id,optional"`
-	Currency          types.String    `tfsdk:"currency" json:"currency,optional"`
-	ExternallyManaged types.Bool      `tfsdk:"externally_managed" json:"externally_managed,optional"`
-	IsContract        types.Bool      `tfsdk:"is_contract" json:"is_contract,optional"`
-	PublicName        types.String    `tfsdk:"public_name" json:"public_name,optional"`
-	Scope             types.String    `tfsdk:"scope" json:"scope,optional"`
+	Currency          types.String    `tfsdk:"currency" json:"currency,computed_optional"`
+	ExternallyManaged types.Bool      `tfsdk:"externally_managed" json:"externally_managed,computed_optional"`
+	IsContract        types.Bool      `tfsdk:"is_contract" json:"is_contract,computed_optional"`
+	PublicName        types.String    `tfsdk:"public_name" json:"public_name,computed_optional"`
+	Scope             types.String    `tfsdk:"scope" json:"scope,computed_optional"`
 	Sets              *[]types.String `tfsdk:"sets" json:"sets,optional"`
 }

internal/services/zone_subscription/resource_test.go

@@ -2,40 +2,75 @@ package zone_subscription_test
 
 import (
 	"os"
+	"regexp"
 	"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/knownvalue"
+	"github.com/hashicorp/terraform-plugin-testing/plancheck"
 	"github.com/hashicorp/terraform-plugin-testing/statecheck"
 	"github.com/hashicorp/terraform-plugin-testing/tfjsonpath"
 )
 
-// NOTE: No sweeper is needed for zone_subscription as the resource cannot be deleted
+// https://github.com/cloudflare/terraform-provider-cloudflare/issues/5971
+func TestAccCloudflareZoneSubscription_ImportNoChanges(t *testing.T) {
+	rnd := utils.GenerateRandomResourceName()
+	zoneID := os.Getenv("CLOUDFLARE_ZONE_ID")
+	resourceName := "cloudflare_zone_subscription." + rnd
+
+	resource.Test(t, resource.TestCase{
+		PreCheck: func() {
+			acctest.TestAccPreCheck(t)
+			acctest.TestAccPreCheck_ZoneID(t)
+		},
+		ProtoV6ProviderFactories: acctest.TestAccProtoV6ProviderFactories,
+		Steps: []resource.TestStep{
+			// First create a basic configuration
+			{
+				Config: testAccCloudflareZoneSubscriptionImportConfig(rnd, zoneID),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr(resourceName, "zone_id", zoneID),
+				),
+			},
+			// Then import the resource and verify it
+			{
+				ResourceName:            resourceName,
+				ImportState:             true,
+				ImportStateVerify:       true,
+				ImportStateVerifyIgnore: []string{"current_period_end", "current_period_start"},
+			},
+		},
+	})
+}
 
 func TestAccCloudflareZoneSubscription_Basic(t *testing.T) {
-	t.Skip("Step 1/3 error: After applying this test step, the refresh plan was not empty.")
 	rnd := utils.GenerateRandomResourceName()
 	zoneID := os.Getenv("CLOUDFLARE_ZONE_ID")
 	resourceName := "cloudflare_zone_subscription." + rnd
 
 	resource.Test(t, resource.TestCase{
-		PreCheck:                 func() { acctest.TestAccPreCheck(t) },
+		PreCheck: func() {
+			acctest.TestAccPreCheck(t)
+			acctest.TestAccPreCheck_ZoneID(t)
+		},
 		ProtoV6ProviderFactories: acctest.TestAccProtoV6ProviderFactories,
-		// No CheckDestroy as zone_subscription cannot be deleted
 		Steps: []resource.TestStep{
 			{
 				Config: testAccCloudflareZoneSubscriptionConfig(rnd, zoneID),
 				ConfigStateChecks: []statecheck.StateCheck{
 					statecheck.ExpectKnownValue(resourceName, tfjsonpath.New("zone_id"), knownvalue.StringExact(zoneID)),
 					statecheck.ExpectKnownValue(resourceName, tfjsonpath.New("id"), knownvalue.StringExact(zoneID)),
-					// Note: rate_plan might be nested differently, skip for now
-					// Verify computed attributes exist
 					statecheck.ExpectKnownValue(resourceName, tfjsonpath.New("currency"), knownvalue.NotNull()),
 					statecheck.ExpectKnownValue(resourceName, tfjsonpath.New("state"), knownvalue.NotNull()),
 					statecheck.ExpectKnownValue(resourceName, tfjsonpath.New("price"), knownvalue.NotNull()),
-					// current_period_end and current_period_start might not be set for enterprise plans
+					statecheck.ExpectKnownValue(resourceName, tfjsonpath.New("frequency"), knownvalue.StringExact("not-applicable")),
+				},
+				ConfigPlanChecks: resource.ConfigPlanChecks{
+					PostApplyPostRefresh: []plancheck.PlanCheck{
+						plancheck.ExpectEmptyPlan(), // Should show no changes
+					},
 				},
 			},
 			{
@@ -45,6 +80,12 @@ func TestAccCloudflareZoneSubscription_Basic(t *testing.T) {
 					// Verify computed attributes still exist
 					statecheck.ExpectKnownValue(resourceName, tfjsonpath.New("currency"), knownvalue.NotNull()),
 					statecheck.ExpectKnownValue(resourceName, tfjsonpath.New("state"), knownvalue.NotNull()),
+					statecheck.ExpectKnownValue(resourceName, tfjsonpath.New("frequency"), knownvalue.StringExact("not-applicable")),
+				},
+				ConfigPlanChecks: resource.ConfigPlanChecks{
+					PostApplyPostRefresh: []plancheck.PlanCheck{
+						plancheck.ExpectEmptyPlan(), // Should show no changes
+					},
 				},
 			},
 			{
@@ -57,35 +98,113 @@ func TestAccCloudflareZoneSubscription_Basic(t *testing.T) {
 }
 
 func TestAccCloudflareZoneSubscription_WithPlanChange(t *testing.T) {
-	t.Skip("Step 1/4 error: After applying this test step, the refresh plan was not empty.")
 	rnd := utils.GenerateRandomResourceName()
 	zoneID := os.Getenv("CLOUDFLARE_ZONE_ID")
 	resourceName := "cloudflare_zone_subscription." + rnd
 
 	resource.Test(t, resource.TestCase{
-		PreCheck:                 func() { acctest.TestAccPreCheck(t) },
+		PreCheck: func() {
+			acctest.TestAccPreCheck(t)
+			acctest.TestAccPreCheck_ZoneID(t)
+		},
 		ProtoV6ProviderFactories: acctest.TestAccProtoV6ProviderFactories,
 		Steps: []resource.TestStep{
 			{
 				Config: testAccCloudflareZoneSubscriptionWithPlan(rnd, zoneID, "enterprise"),
 				Check: resource.ComposeTestCheckFunc(
 					resource.TestCheckResourceAttr(resourceName, "zone_id", zoneID),
 					resource.TestCheckResourceAttr(resourceName, "rate_plan.id", "enterprise"),
+					resource.TestCheckResourceAttr(resourceName, "frequency", "not-applicable"),
 				),
+				ConfigPlanChecks: resource.ConfigPlanChecks{
+					PostApplyPostRefresh: []plancheck.PlanCheck{
+						plancheck.ExpectEmptyPlan(), // Should show no changes
+					},
+				},
 			},
 			{
 				Config: testAccCloudflareZoneSubscriptionWithPlan(rnd, zoneID, "free"),
 				Check: resource.ComposeTestCheckFunc(
 					resource.TestCheckResourceAttr(resourceName, "zone_id", zoneID),
 					resource.TestCheckResourceAttr(resourceName, "rate_plan.id", "free"),
+					resource.TestCheckResourceAttr(resourceName, "frequency", "not-applicable"),
 				),
+				ConfigPlanChecks: resource.ConfigPlanChecks{
+					PostApplyPostRefresh: []plancheck.PlanCheck{
+						plancheck.ExpectEmptyPlan(), // Should show no changes
+					},
+				},
 			},
 			{
-				Config: testAccCloudflareZoneSubscriptionWithPlan(rnd, zoneID, "business"),
+				ResourceName:      resourceName,
+				ImportState:       true,
+				ImportStateVerify: true,
+			},
+		},
+	})
+}
+
+func TestAccCloudflareZoneSubscription_WithPlanAndComputedOptionalRatePlanFields(t *testing.T) {
+	rnd := utils.GenerateRandomResourceName()
+	zoneID := os.Getenv("CLOUDFLARE_ZONE_ID")
+	resourceName := "cloudflare_zone_subscription." + rnd
+
+	resource.Test(t, resource.TestCase{
+		PreCheck: func() {
+			acctest.TestAccPreCheck(t)
+			acctest.TestAccPreCheck_ZoneID(t)
+		},
+		ProtoV6ProviderFactories: acctest.TestAccProtoV6ProviderFactories,
+		Steps: []resource.TestStep{
+			{
+				Config: testAccCloudflareZoneSubscriptionWithPlanAndComputedOptionalRatePlanFields(rnd, zoneID),
 				Check: resource.ComposeTestCheckFunc(
 					resource.TestCheckResourceAttr(resourceName, "zone_id", zoneID),
-					resource.TestCheckResourceAttr(resourceName, "rate_plan.id", "business"),
+					resource.TestCheckResourceAttr(resourceName, "frequency", "not-applicable"),
+					resource.TestCheckResourceAttr(resourceName, "rate_plan.id", "enterprise"),
+					resource.TestCheckResourceAttr(resourceName, "rate_plan.currency", "USD"),
+					resource.TestCheckResourceAttr(resourceName, "rate_plan.is_contract", "true"),
+					resource.TestCheckResourceAttr(resourceName, "rate_plan.public_name", "Enterprise"),
+					resource.TestCheckResourceAttr(resourceName, "rate_plan.externally_managed", "false"),
+					resource.TestCheckResourceAttr(resourceName, "rate_plan.scope", "zone"),
 				),
+				ConfigPlanChecks: resource.ConfigPlanChecks{
+					PostApplyPostRefresh: []plancheck.PlanCheck{
+						plancheck.ExpectEmptyPlan(),
+					},
+				},
+			},
+			{
+				ResourceName:      resourceName,
+				ImportState:       true,
+				ImportStateVerify: true,
+			},
+		},
+	})
+}
+
+// Free and Enterprise plans and whenfrequency is provided
+// will return  Error: Provider produced inconsistent result after apply
+
+func TestAccCloudflareZoneSubscription_WithPlanThatReturnsNotApplicableFrequencyResultingInError(t *testing.T) {
+	rnd := utils.GenerateRandomResourceName()
+	zoneID := os.Getenv("CLOUDFLARE_ZONE_ID")
+	resourceName := "cloudflare_zone_subscription." + rnd
+
+	resource.Test(t, resource.TestCase{
+		PreCheck: func() {
+			acctest.TestAccPreCheck(t)
+			acctest.TestAccPreCheck_ZoneID(t)
+		},
+		ProtoV6ProviderFactories: acctest.TestAccProtoV6ProviderFactories,
+		Steps: []resource.TestStep{
+			{
+				Config:      testAccCloudflareZoneSubscriptionWithPlanAndFrequency(rnd, zoneID, "free", "monthly"),
+				ExpectError: regexp.MustCompile(regexp.QuoteMeta("Error: Provider produced inconsistent result after apply")),
+			},
+			{
+				Config:      testAccCloudflareZoneSubscriptionWithPlanAndFrequency(rnd, zoneID, "enterprise", "monthly"),
+				ExpectError: regexp.MustCompile(regexp.QuoteMeta("Error: Provider produced inconsistent result after apply")),
 			},
 			{
 				ResourceName:      resourceName,
@@ -107,3 +226,15 @@ func testAccCloudflareZoneSubscriptionConfigUpdate(rnd, zoneID string) string {
 func testAccCloudflareZoneSubscriptionWithPlan(rnd, zoneID, plan string) string {
 	return acctest.LoadTestCase("with_plan.tf", rnd, zoneID, plan)
 }
+
+func testAccCloudflareZoneSubscriptionWithPlanAndFrequency(rnd, zoneID, plan, frequency string) string {
+	return acctest.LoadTestCase("with_plan_and_frequency.tf", rnd, zoneID, plan, frequency)
+}
+
+func testAccCloudflareZoneSubscriptionImportConfig(rnd, zoneID string) string {
+	return acctest.LoadTestCase("import_config.tf", rnd, zoneID)
+}
+
+func testAccCloudflareZoneSubscriptionWithPlanAndComputedOptionalRatePlanFields(rnd, zoneID string) string {
+	return acctest.LoadTestCase("with_plan_and_computed_optional_rate_plan_fields.tf", rnd, zoneID)
+}

internal/services/zone_subscription/schema.go

@@ -33,12 +33,17 @@ func ResourceSchema(ctx context.Context) schema.Schema {
 			"frequency": schema.StringAttribute{
 				Description: "How often the subscription is renewed automatically.\nAvailable values: \"weekly\", \"monthly\", \"quarterly\", \"yearly\".",
 				Optional:    true,
+				Computed:    true,
+				PlanModifiers: []planmodifier.String{
+					stringplanmodifier.UseStateForUnknown(),
+				},
 				Validators: []validator.String{
 					stringvalidator.OneOfCaseInsensitive(
 						"weekly",
 						"monthly",
 						"quarterly",
 						"yearly",
+						"not-applicable",
 					),
 				},
 			},
@@ -66,22 +71,27 @@ func ResourceSchema(ctx context.Context) schema.Schema {
 					},
 					"currency": schema.StringAttribute{
 						Description: "The currency applied to the rate plan subscription.",
+						Computed:    true,
 						Optional:    true,
 					},
 					"externally_managed": schema.BoolAttribute{
 						Description: "Whether this rate plan is managed externally from Cloudflare.",
+						Computed:    true,
 						Optional:    true,
 					},
 					"is_contract": schema.BoolAttribute{
 						Description: "Whether a rate plan is enterprise-based (or newly adopted term contract).",
+						Computed:    true,
 						Optional:    true,
 					},
 					"public_name": schema.StringAttribute{
 						Description: "The full name of the rate plan.",
+						Computed:    true,
 						Optional:    true,
 					},
 					"scope": schema.StringAttribute{
 						Description: "The scope that this rate plan applies to.",
+						Computed:    true,
 						Optional:    true,
 					},
 					"sets": schema.ListAttribute{

internal/services/zone_subscription/testdata/basic_update.tf

@@ -1,11 +1,11 @@
 resource "cloudflare_zone_subscription" "%[1]s" {
   zone_id = "%[2]s"
-  
+
   # Keep enterprise plan but test that we can manage the resource
   rate_plan = {
     id = "enterprise"
   }
-  
+
   # For enterprise plans, frequency might not be changeable
   # so we're just verifying the resource can be managed
 }

internal/services/zone_subscription/testdata/import.tf

@@ -0,0 +1,3 @@
+resource "cloudflare_zone_subscription" "%[1]s" {
+  zone_id = "%[2]s"
+}

internal/services/zone_subscription/testdata/import_config.tf

@@ -0,0 +1,6 @@
+resource "cloudflare_zone_subscription" "%[1]s" {
+  zone_id = "%[2]s"
+  rate_plan = {
+    id = "free"
+  }
+}

internal/services/zone_subscription/testdata/with_plan_and_computed_optional_rate_plan_fields.tf

@@ -0,0 +1,12 @@
+resource "cloudflare_zone_subscription" "%[1]s" {
+  zone_id = "%[2]s"
+
+  rate_plan = {
+    id = "enterprise"
+    currency = "USD"
+    externally_managed = false
+    is_contract = true
+    public_name = "Enterprise"
+    scope = "zone"
+  }
+}

internal/services/zone_subscription/testdata/with_plan_and_frequency.tf

@@ -0,0 +1,8 @@
+resource "cloudflare_zone_subscription" "%[1]s" {
+  zone_id = "%[2]s"
+
+  rate_plan = {
+    id = "%[3]s"
+  }
+  frequency = "%[4]s"
+}