From 37187d7f98ebdd77cf71289fc653a6137c24a9fe Mon Sep 17 00:00:00 2001 From: Jacob Bednarz Date: Mon, 27 Apr 2020 09:21:47 +1000 Subject: [PATCH 1/3] resource/cloudflare_zone: No need to translate the ID to Name back to ID Within the `Update` we set the rate plan which requires the subscription ID mapped value[1]. We already have the legacy ID[2] so we don't need to pass it through the `planIDForName` method which we previously. I suspect this was a copy-paste issue from the `Read` method as we were also setting the `plan` value to the plan name which would haven't have been accepted in the schema anyway. This commit trims all of that out and just passes the ID as it should have always done. [1]: https://github.com/terraform-providers/terraform-provider-cloudflare/blob/master/cloudflare/resource_cloudflare_zone.go#L35-L40 [2]: https://github.com/terraform-providers/terraform-provider-cloudflare/blob/master/cloudflare/resource_cloudflare_zone.go#L18-L23 [3]: https://github.com/terraform-providers/terraform-provider-cloudflare/blob/master/cloudflare/resource_cloudflare_zone.go#L81 --- cloudflare/resource_cloudflare_zone.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/cloudflare/resource_cloudflare_zone.go b/cloudflare/resource_cloudflare_zone.go index 2b937dcd65..cbb1ca9617 100644 --- a/cloudflare/resource_cloudflare_zone.go +++ b/cloudflare/resource_cloudflare_zone.go @@ -217,15 +217,15 @@ func resourceCloudflareZoneUpdate(d *schema.ResourceData, meta interface{}) erro } // In the cases where the zone isn't completely setup yet, we need to - // check the `status` field and should it be pending, use the `Name` + // check the `status` field and should it be pending, use the `LegacyID` // from `zone.PlanPending` instead to account for paid plans. if zone.Status == "pending" && zone.PlanPending.Name != "" { - d.Set("plan", zone.PlanPending.Name) + d.Set("plan", zone.PlanPending.LegacyID) } if plan, ok := d.GetOk("plan"); ok { - planName := planIDForName(plan.(string)) - if err := setRatePlan(client, zoneID, planName, false); err != nil { + planID := plan.(string) + if err := setRatePlan(client, zoneID, planID, false); err != nil { return err } } From f83a47c91a975bca3e1d52158a5a807de0a25a3f Mon Sep 17 00:00:00 2001 From: Jacob Bednarz Date: Tue, 28 Apr 2020 07:14:34 +1000 Subject: [PATCH 2/3] Handle upgrading from "free" to "enterprise" Turns out if you need to upgrade from a free plan to an enterprise plan, you need to create the subscription (using POST) not modify an existing one (PUT). Updates the call the `setRatePlan` to work out which HTTP verb to use based on the incoming plan ID change. --- cloudflare/resource_cloudflare_zone.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/cloudflare/resource_cloudflare_zone.go b/cloudflare/resource_cloudflare_zone.go index cbb1ca9617..745dfb939c 100644 --- a/cloudflare/resource_cloudflare_zone.go +++ b/cloudflare/resource_cloudflare_zone.go @@ -224,8 +224,14 @@ func resourceCloudflareZoneUpdate(d *schema.ResourceData, meta interface{}) erro } if plan, ok := d.GetOk("plan"); ok { + // If we're upgrading from a free plan, we need to use POST (not PUT) as the + // the subscription needs to be created, not modified despite the resource + // already existing. + existingPlan, _ := d.GetChange("plan") + wasFreePlan := existingPlan.(string) == "free" planID := plan.(string) - if err := setRatePlan(client, zoneID, planID, false); err != nil { + + if err := setRatePlan(client, zoneID, planID, wasFreePlan); err != nil { return err } } From 885ae0b44bdd728c7e94c147907b52763df66f60 Mon Sep 17 00:00:00 2001 From: Jacob Bednarz Date: Tue, 28 Apr 2020 07:23:57 +1000 Subject: [PATCH 3/3] Only make rate plan calls if we have changes This isn't a huge deal but seeing how I'm touching this code, I can make it more efficient by only making the `setRatePlan` calls if we have changed the plan. Otherwise, pass over it. --- cloudflare/resource_cloudflare_zone.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cloudflare/resource_cloudflare_zone.go b/cloudflare/resource_cloudflare_zone.go index 745dfb939c..832c5f9e91 100644 --- a/cloudflare/resource_cloudflare_zone.go +++ b/cloudflare/resource_cloudflare_zone.go @@ -223,13 +223,13 @@ func resourceCloudflareZoneUpdate(d *schema.ResourceData, meta interface{}) erro d.Set("plan", zone.PlanPending.LegacyID) } - if plan, ok := d.GetOk("plan"); ok { + if change := d.HasChange("plan"); change { // If we're upgrading from a free plan, we need to use POST (not PUT) as the // the subscription needs to be created, not modified despite the resource // already existing. - existingPlan, _ := d.GetChange("plan") + existingPlan, newPlan := d.GetChange("plan") wasFreePlan := existingPlan.(string) == "free" - planID := plan.(string) + planID := newPlan.(string) if err := setRatePlan(client, zoneID, planID, wasFreePlan); err != nil { return err