Skip to content

Commit

Permalink
Merge pull request #1547 from cloudflare/dont-get-stuck-in-loop
Browse files Browse the repository at this point in the history 
resource/cloudflare_zone: don't get stuck in a loop for partner plans
  • Loading branch information
@jacobbednarz
jacobbednarz authored Apr 6, 2022
2 parents 1c8bb9f + 0055847 commit d1fbd09
Show file tree
Hide file tree
Showing 4 changed files with 67 additions and 110 deletions.
3 changes: 3 additions & 0 deletions .changelog/1547.txt
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
resource/cloudflare_zone: don't get stuck in endless loop for partner zone rate plans
```
122 changes: 64 additions & 58 deletions cloudflare/resource_cloudflare_zone.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
Expand All @@ -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
Expand Down Expand Up @@ -239,44 +253,36 @@ 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)
}
}

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"))
}

return nil
})
}

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
Expand Down
50 changes: 0 additions & 50 deletions cloudflare/resource_cloudflare_zone_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 0 additions & 2 deletions cloudflare/schema_cloudflare_zone.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,6 @@ func resourceCloudflareZoneSchema() map[string]*schema.Schema {
planIDPartnerPro,
planIDPartnerBusiness,
planIDPartnerEnterprise,
planIDPartnerWorkers,
planIDPartnerImageEnterprise,
}, false),
},
"meta": {
Expand Down

0 comments on commit d1fbd09

Please sign in to comment.