From 0055847ccf227e85a85dbb1957a5eef6bc18fd55 Mon Sep 17 00:00:00 2001 From: Jacob Bednarz Date: Mon, 4 Apr 2022 15:00:59 +1000 Subject: [PATCH] resource/cloudflare_zone: don't get stuck in a loop for partner plans In #1532 we added support for partner rate plans based on the _subscription_ endpoints however, within zone creation we get the _zone_ endpoint (same thing, different "view of the world"). This caused an issue where we were waiting for a plan ID to return that never would as the partners rate plans in the zone endpoint just display the regular rate plan instead of the partner specific one. Closes #1532 --- .changelog/1547.txt | 3 + cloudflare/resource_cloudflare_zone.go | 122 ++++++++++---------- cloudflare/resource_cloudflare_zone_test.go | 50 -------- cloudflare/schema_cloudflare_zone.go | 2 - 4 files changed, 67 insertions(+), 110 deletions(-) create mode 100644 .changelog/1547.txt diff --git a/.changelog/1547.txt b/.changelog/1547.txt new file mode 100644 index 0000000000..f43157ed0b --- /dev/null +++ b/.changelog/1547.txt @@ -0,0 +1,3 @@ +```release-note:bug +resource/cloudflare_zone: don't get stuck in endless loop for partner zone rate plans +``` diff --git a/cloudflare/resource_cloudflare_zone.go b/cloudflare/resource_cloudflare_zone.go index fa27a9ac9d..ca684eebd2 100644 --- a/cloudflare/resource_cloudflare_zone.go +++ b/cloudflare/resource_cloudflare_zone.go @@ -20,43 +20,57 @@ const ( planIDBusiness = "business" planIDEnterprise = "enterprise" - planIDPartnerFree = "partners_free" - planIDPartnerPro = "partners_pro" - planIDPartnerBusiness = "partners_business" - planIDPartnerEnterprise = "partners_enterprise" - planIDPartnerWorkers = "partners_workers_ss" - planIDPartnerImageEnterprise = "image_resizing_enterprise" + planIDPartnerFree = "partners_free" + planIDPartnerPro = "partners_pro" + planIDPartnerBusiness = "partners_business" + planIDPartnerEnterprise = "partners_enterprise" ) -// we keep a private map and we will have a function to check and validate the descriptive name from the RatePlan API with the legacy_id -var idForName = map[string]string{ - "Free Website": planIDFree, - "Pro Website": planIDPro, - "Business Website": planIDBusiness, - "Enterprise Website": planIDEnterprise, - - "Partners Free Plan": planIDPartnerFree, - "Partners Professional Plan": planIDPartnerPro, - "Partners Business Plan": planIDPartnerBusiness, - "Partners Enterprise Plan": planIDPartnerEnterprise, - "Cloudflare Workers": planIDPartnerWorkers, - "Image Resizing Ent": planIDPartnerImageEnterprise, +type subscriptionData struct { + ID, Name, Description string } -// maintain a mapping for the subscription API term for rate plans to -// the one we are expecting end users to use. -var subscriptionIDOfRatePlans = map[string]string{ - planIDFree: "CF_FREE", - planIDPro: "CF_PRO", - planIDBusiness: "CF_BIZ", - planIDEnterprise: "CF_ENT", - - planIDPartnerFree: "PARTNERS_FREE", - planIDPartnerPro: "PARTNERS_PRO", - planIDPartnerBusiness: "PARTNERS_BIZ", - planIDPartnerEnterprise: "PARTNERS_ENT", - planIDPartnerWorkers: "PARTNERS_WORKERS_SS", - planIDPartnerImageEnterprise: "IMAGE_RESIZING_ENT", +var ratePlans = map[string]subscriptionData{ + planIDFree: { + Name: "CF_FREE", + ID: planIDFree, + Description: "Free Website", + }, + planIDPro: { + Name: "CF_PRO", + ID: planIDPro, + Description: "Pro Website", + }, + planIDBusiness: { + Name: "CF_BIZ", + ID: planIDBusiness, + Description: "Business Website", + }, + planIDEnterprise: { + Name: "CF_ENT", + ID: planIDEnterprise, + Description: "Enterprise Website", + }, + planIDPartnerFree: { + Name: "PARTNERS_FREE", + ID: planIDPartnerFree, + Description: "Free Website", + }, + planIDPartnerPro: { + Name: "PARTNERS_PRO", + ID: planIDPartnerPro, + Description: "Pro Website", + }, + planIDPartnerBusiness: { + Name: "PARTNERS_BIZ", + ID: planIDPartnerBusiness, + Description: "Business Website", + }, + planIDPartnerEnterprise: { + Name: "PARTNERS_ENT", + ID: planIDPartnerEnterprise, + Description: "Enterprise Website", + }, } func resourceCloudflareZone() *schema.Resource { @@ -137,13 +151,13 @@ func resourceCloudflareZoneRead(d *schema.ResourceData, meta interface{}) error } // 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. var plan string - if zone.Status == "pending" && zone.PlanPending.Name != "" { - plan = zone.PlanPending.Name + if zone.Status == "pending" && zone.PlanPending.LegacyID != "" { + plan = zone.PlanPending.LegacyID } else { - plan = zone.Plan.Name + plan = zone.Plan.LegacyID } d.Set("paused", zone.Paused) @@ -153,7 +167,7 @@ func resourceCloudflareZoneRead(d *schema.ResourceData, meta interface{}) error d.Set("name_servers", zone.NameServers) d.Set("meta", flattenMeta(d, zone.Meta)) d.Set("zone", zone.Name) - d.Set("plan", planIDForName(plan)) + d.Set("plan", plan) d.Set("verification_key", zone.VerificationKey) return nil @@ -239,13 +253,13 @@ func setRatePlan(client *cloudflare.API, zoneID, planID string, isNewPlan bool, if isNewPlan { // A free rate plan is the default so no need to explicitly make another // HTTP call to set it. - if subscriptionIDOfRatePlans[planID] != planIDFree { - if err := client.ZoneSetPlan(context.Background(), zoneID, subscriptionIDOfRatePlans[planID]); err != nil { + if ratePlans[planID].ID != planIDFree { + if err := client.ZoneSetPlan(context.Background(), zoneID, ratePlans[planID].Name); err != nil { return fmt.Errorf("error setting plan %s for zone %q: %s", planID, zoneID, err) } } } else { - if err := client.ZoneUpdatePlan(context.Background(), zoneID, subscriptionIDOfRatePlans[planID]); err != nil { + if err := client.ZoneUpdatePlan(context.Background(), zoneID, ratePlans[planID].Name); err != nil { return fmt.Errorf("error updating plan %s for zone %q: %s", planID, zoneID, err) } } @@ -253,7 +267,15 @@ func setRatePlan(client *cloudflare.API, zoneID, planID string, isNewPlan bool, return resource.Retry(d.Timeout(schema.TimeoutCreate), func() *resource.RetryError { zone, _ := client.ZoneDetails(context.Background(), zoneID) - if zone.Plan.LegacyID != planID { + // This is a little confusing but due to the multiple views of + // subscriptions, partner plans actually end up "appearing" like regular + // rate plans to end users. To ensure we don't get stuck in an endless + // loop, we need to compare the "name" of the plan to the "description" + // of the rate plan. That way, even if we send `PARTNERS_ENT` to the + // subscriptions service, we will compare it in Terraform as + // "Enterprise Website" and know that we made the swap and just trust + // that the rate plan identifier did the right thing. + if zone.Plan.Name != ratePlans[planID].Description { return resource.RetryableError(fmt.Errorf("plan ID change has not yet propagated")) } @@ -261,22 +283,6 @@ func setRatePlan(client *cloudflare.API, zoneID, planID string, isNewPlan bool, }) } -func planIDForName(name string) string { - if p, ok := idForName[name]; ok { - return p - } - return "" -} - -func planNameForID(id string) string { - for k, v := range idForName { - if strings.EqualFold(id, v) { - return k - } - } - return "" -} - // zoneDiffFunc is a DiffSuppressFunc that accepts two strings and then converts // them to unicode before performing the comparison whether or not the value has // changed. This ensures that zones which could be either are evaluated diff --git a/cloudflare/resource_cloudflare_zone_test.go b/cloudflare/resource_cloudflare_zone_test.go index 24ecadaf12..f3a030081a 100644 --- a/cloudflare/resource_cloudflare_zone_test.go +++ b/cloudflare/resource_cloudflare_zone_test.go @@ -236,56 +236,6 @@ func testZoneConfigWithPlan(resourceID, zoneName, paused, jumpStart, plan string }`, resourceID, zoneName, paused, jumpStart, plan) } -func TestPlanNameFallsBackToEmptyIfUnknown(t *testing.T) { - type args struct { - planName string - } - tests := []struct { - name string - args args - want string - }{ - { - "free website", args{"Free Website"}, planIDFree, - }, - { - "enterprise", args{"Enterprise Website"}, planIDEnterprise, - }, - { - "undefined or new", args{"New Awesome Plan Website"}, "", - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - if got := planIDForName(tt.args.planName); got != tt.want { - t.Errorf("planIDForName() = %v, want %v", got, tt.want) - } - }) - } -} - -func TestPlanIDFallsBackToEmptyIfUnknown(t *testing.T) { - type args struct { - id string - } - tests := []struct { - name string - args args - want string - }{ - {"free id", args{planIDFree}, "Free Website"}, - {"enterprise id", args{planIDEnterprise}, "Enterprise Website"}, - {"unknonw id", args{"bogus"}, ""}, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - if got := planNameForID(tt.args.id); got != tt.want { - t.Errorf("planNameForID() = %v, want %v", got, tt.want) - } - }) - } -} - func TestAccCloudflareZone_SetType(t *testing.T) { rnd := generateRandomResourceName() name := "cloudflare_zone." + rnd diff --git a/cloudflare/schema_cloudflare_zone.go b/cloudflare/schema_cloudflare_zone.go index 76d898e961..15aae166fd 100644 --- a/cloudflare/schema_cloudflare_zone.go +++ b/cloudflare/schema_cloudflare_zone.go @@ -41,8 +41,6 @@ func resourceCloudflareZoneSchema() map[string]*schema.Schema { planIDPartnerPro, planIDPartnerBusiness, planIDPartnerEnterprise, - planIDPartnerWorkers, - planIDPartnerImageEnterprise, }, false), }, "meta": {