resource/cloudflare_ruleset: improve dashboard collisions

One of the earliest niggles with customers coming from the dashboard to
Terraform was the collision caused by a Ruleset phase being created in
the UI but the Terraform provider also needing to create the same
phase. This was problematic for a few reasons:

- The first was that when you deleted Ruleset rules in the UI, it didn't
  remove the phase. This was intentional but hidden behind the UI and
  only accessible via the API.
- Secondly, when customers wanted to use Terraform, there was an
  assumption they would be starting from scratch and many were not.
- Finally, in the event of a collision, we didn't know which Ruleset the
  customer wanted to use so we error'd out with a link to manually
  resolve which isn't a great solution but made the issue more
  prominent.

I had a chance to rethink through this issue and managed to find a way
that we improve all three points above and remove majority of the pain
points. With the proposed changes, the only time a customer needs to
manually resolve the Ruleset rules is if there are existing rules in the
UI which requires them to be deleted or migrated.

Achieving this requires the assumption that if the Ruleset has no rules,
it is ok to modify. Unfortunately, it's not that simple in practice. If
the phase already exists, we cannot just update it as the `name`
attribute is not writable after creation. Along with this, the `ref` and
`version` values will be automatically incremented causing a permadiff
in Terraform as the customer hasn't actually modified these values but
the backing service (and API) has. To work around this, if we find a
phase with no rules, we recreate it with the provided values which is
essentially the same the same thing as the "happy path" for a new
Terraform resource would be anyway.

.changelog/1393.txt

@@ -0,0 +1,3 @@
+```release-note:enhancement
+resource/cloudflare_ruleset: smoother handling of UI/API collisions during migrations
+```

cloudflare/resource_cloudflare_ruleset.go

@@ -17,7 +17,7 @@ import (
 const (
 	accountLevelRulesetDeleteURL = "https://api.cloudflare.com/#account-rulesets-delete-account-ruleset"
 	zoneLevelRulesetDeleteURL    = "https://api.cloudflare.com/#zone-rulesets-delete-zone-ruleset"
-	duplicateRulesetError        = "failed to create ruleset %q as a similar configuration already exists. If you are migrating from the Dashboard, you will need to first manually remove it using the API (%s) before you can configure it in Terraform. Otherwise, you have hit the entitlements quota and should contact your account team."
+	duplicateRulesetError        = "failed to create ruleset %q as a similar configuration with rules already exists and overwriting will have unintended consequences. If you are migrating from the Dashboard, you will need to first remove the existing rules otherwise you can remove the existing phase yourself using the API (%s)."
 )
 
 func resourceCloudflareRuleset() *schema.Resource {
@@ -37,6 +37,23 @@ func resourceCloudflareRulesetCreate(d *schema.ResourceData, meta interface{}) e
 	client := meta.(*cloudflare.API)
 	accountID := d.Get("account_id").(string)
 	zoneID := d.Get("zone_id").(string)
+	rulesetPhase := d.Get("phase").(string)
+
+	var ruleset cloudflare.Ruleset
+	var sempahoreErr error
+	if accountID != "" {
+		ruleset, sempahoreErr = client.GetAccountRulesetPhase(context.Background(), accountID, rulesetPhase)
+	} else {
+		ruleset, sempahoreErr = client.GetZoneRulesetPhase(context.Background(), zoneID, rulesetPhase)
+	}
+
+	if len(ruleset.Rules) > 0 {
+		deleteRulesetURL := accountLevelRulesetDeleteURL
+		if accountID == "" {
+			deleteRulesetURL = zoneLevelRulesetDeleteURL
+		}
+		return fmt.Errorf(duplicateRulesetError, rulesetPhase, deleteRulesetURL)
+	}
 
 	rulesetName := d.Get("name").(string)
 	rulesetDescription := d.Get("description").(string)
@@ -45,35 +62,41 @@ func resourceCloudflareRulesetCreate(d *schema.ResourceData, meta interface{}) e
 		Name:        rulesetName,
 		Description: rulesetDescription,
 		Kind:        rulesetKind,
-		Phase:       d.Get("phase").(string),
+		Phase:       rulesetPhase,
 	}
 
 	rules, err := buildRulesetRulesFromResource(d)
 	if err != nil {
-		return errors.Wrap(err, fmt.Sprintf("error building ruleset from resource"))
+		return fmt.Errorf("error building ruleset rules from resource: %w", err)
 	}
 
 	if len(rules) > 0 {
 		rs.Rules = rules
 	}
 
-	var ruleset cloudflare.Ruleset
+	if sempahoreErr == nil && len(ruleset.Rules) == 0 && ruleset.Description == "" {
+		log.Print("[DEBUG] default ruleset created by the UI with empty rules found, recreating from scratch")
+		var deleteRulesetErr error
+		if accountID != "" {
+			deleteRulesetErr = client.DeleteAccountRuleset(context.Background(), accountID, ruleset.ID)
+		} else {
+			deleteRulesetErr = client.DeleteZoneRuleset(context.Background(), zoneID, ruleset.ID)
+		}
+
+		if deleteRulesetErr != nil {
+			return fmt.Errorf("failed to delete ruleset: %w", deleteRulesetErr)
+		}
+	}
+
+	var rulesetCreateErr error
 	if accountID != "" {
-		ruleset, err = client.CreateAccountRuleset(context.Background(), accountID, rs)
+		ruleset, rulesetCreateErr = client.CreateAccountRuleset(context.Background(), accountID, rs)
 	} else {
-		ruleset, err = client.CreateZoneRuleset(context.Background(), zoneID, rs)
+		ruleset, rulesetCreateErr = client.CreateZoneRuleset(context.Background(), zoneID, rs)
 	}
 
-	if err != nil {
-		if strings.Contains(err.Error(), "exceeded maximum number") {
-			deleteRulesetURL := accountLevelRulesetDeleteURL
-			if accountID == "" {
-				deleteRulesetURL = zoneLevelRulesetDeleteURL
-			}
-			return fmt.Errorf(duplicateRulesetError, rulesetName, deleteRulesetURL)
-		}
-
-		return errors.Wrap(err, fmt.Sprintf("error creating ruleset %s", rulesetName))
+	if rulesetCreateErr != nil {
+		return fmt.Errorf("error creating ruleset %s: %w", rulesetName, rulesetCreateErr)
 	}
 
 	rulesetEntryPoint := cloudflare.Ruleset{
@@ -85,13 +108,13 @@ func resourceCloudflareRulesetCreate(d *schema.ResourceData, meta interface{}) e
 	// endpoint.
 	if rulesetKind != string(cloudflare.RulesetKindCustom) {
 		if accountID != "" {
-			_, err = client.UpdateAccountRulesetPhase(context.Background(), accountID, rs.Phase, rulesetEntryPoint)
+			_, err = client.UpdateAccountRulesetPhase(context.Background(), accountID, rulesetPhase, rulesetEntryPoint)
 		} else {
-			_, err = client.UpdateZoneRulesetPhase(context.Background(), zoneID, rs.Phase, rulesetEntryPoint)
+			_, err = client.UpdateZoneRulesetPhase(context.Background(), zoneID, rulesetPhase, rulesetEntryPoint)
 		}
 
 		if err != nil {
-			return errors.Wrap(err, fmt.Sprintf("error updating ruleset phase entrypoint %s", rulesetName))
+			return fmt.Errorf("error updating ruleset phase entrypoint %s: %w", rulesetName, err)
 		}
 	}
 
@@ -124,7 +147,7 @@ func resourceCloudflareRulesetRead(d *schema.ResourceData, meta interface{}) err
 			d.SetId("")
 			return nil
 		}
-		return errors.Wrap(err, fmt.Sprintf("error reading ruleset ID: %s", d.Id()))
+		return fmt.Errorf("error reading ruleset ID %q: %w", d.Id(), err)
 	}
 
 	d.Set("name", ruleset.Name)
@@ -144,7 +167,7 @@ func resourceCloudflareRulesetUpdate(d *schema.ResourceData, meta interface{}) e
 
 	rules, err := buildRulesetRulesFromResource(d)
 	if err != nil {
-		return errors.Wrap(err, fmt.Sprintf("error building ruleset from resource"))
+		return fmt.Errorf("error building ruleset from resource: %w", err)
 	}
 
 	description := d.Get("description").(string)
@@ -155,7 +178,7 @@ func resourceCloudflareRulesetUpdate(d *schema.ResourceData, meta interface{}) e
 	}
 
 	if err != nil {
-		return errors.Wrap(err, fmt.Sprintf("error updating ruleset with ID %q", d.Id()))
+		return fmt.Errorf("error updating ruleset with ID %q: %w", d.Id(), err)
 	}
 
 	return resourceCloudflareRulesetRead(d, meta)
@@ -174,7 +197,7 @@ func resourceCloudflareRulesetDelete(d *schema.ResourceData, meta interface{}) e
 	}
 
 	if err != nil {
-		return errors.Wrap(err, fmt.Sprintf("error deleting ruleset with ID %q", d.Id()))
+		return fmt.Errorf("error deleting ruleset with ID %q: %w", d.Id(), err)
 	}
 
 	return nil