fix(alert_rule): do not track unnecessary fields

This contribution removes unnecessary alert_rule fields from the alert_rules field of all test resources, as they were the source of the problem reported in thousandeyes#73.

Additionally, because we're bumping the thousandeyes-go-sdk dependency, this pull request also fixes thousandeyes#58.

Fixes thousandeyes#73.

go.mod

@@ -5,7 +5,7 @@ go 1.17
 require (
 	github.com/hashicorp/terraform-plugin-docs v0.9.0
 	github.com/hashicorp/terraform-plugin-sdk/v2 v2.17.0
-	github.com/thousandeyes/thousandeyes-sdk-go/v2 v2.0.0-alpha.3
+	github.com/thousandeyes/thousandeyes-sdk-go/v2 v2.0.0-alpha.5
 )
 
 require (

thousandeyes/resource_alert_rule.go

@@ -50,10 +50,16 @@ func resourceAlertRuleUpdate(d *schema.ResourceData, m interface{}) error {
 	// to be retained on update.
 	// Terraform schema validation should guarantee their existence.
 	update.AlertType = thousandeyes.String(d.Get("alert_type").(string))
-	update.Direction = thousandeyes.String(d.Get("direction").(string))
 	update.Expression = thousandeyes.String(d.Get("expression").(string))
-	update.MinimumSources = thousandeyes.Int(d.Get("minimum_sources").(int))
-	update.MinimumSourcesPct = thousandeyes.Int(d.Get("minimum_sources_pct").(int))
+
+	// MinimumSources and MinimumSourcesPct are mutually exclusive
+	minimumSources := d.Get("minimum_sources").(int)
+	if minimumSources > 0 {
+		update.MinimumSources = thousandeyes.Int(minimumSources)
+	} else {
+		update.MinimumSourcesPct = thousandeyes.Int(d.Get("minimum_sources_pct").(int))
+	}
+
 	update.RoundsViolatingRequired = thousandeyes.Int(d.Get("rounds_violating_required").(int))
 	update.RoundsViolatingOutOf = thousandeyes.Int(d.Get("rounds_violating_out_of").(int))
 	update.RuleName = thousandeyes.String(d.Get("rule_name").(string))

thousandeyes/schemas.go

@@ -245,124 +245,14 @@ var schemas = map[string]*schema.Schema{
 		Description: "Get ruleId from /alert-rules endpoint. If alertsEnabled is set to 'true' and alertRules is not included in a creation/update query, applicable defaults will be used",
 		Optional:    true,
 		Required:    false,
-		Type:        schema.TypeList,
+		Type:        schema.TypeSet,
 		Elem: &schema.Resource{
-			// We need to declare all the fields of AlertRule again here,
-			// to accommodate reads of alert rules declarations returned on test reads.
-			// Pulling them from the regular AlertRule schema declaration
-			// would cause conflict due to them being declared with  `Required: true`
-			// elsewhere.
 			Schema: map[string]*schema.Schema{
-				"alert_rule_id": {
-					Type:        schema.TypeInt,
-					Description: "ID of  alert rule",
-					Computed:    true,
-				},
-				"alert_type": {
-					Description: "Acceptable test types, verbose names",
-					Type:        schema.TypeString,
-					Optional:    true,
-				},
-				"default": {
-					Type:        schema.TypeInt,
-					Description: "to set the rule as a default, set this value to 1.",
-					Optional:    true,
-				},
-				"direction": {
-					// See `direction-alert_rule` below rather than `direction`
-					Type: schema.TypeString,
-					Description: "[TO_TARGET, FROM_TARGET, BIDIRECTIONAL]	Direction of the test (affects how results are shown)",
-					Optional: true,
-				},
-				"expression": {
-					Type:        schema.TypeString,
-					Description: "Alert rule evaluation expression",
-					Optional:    true,
-				},
-				"include_covered_prefixes": {
-					Type:        schema.TypeInt,
-					Description: "set to 1 to include queries for subprefixes detected under this prefix",
-					Optional:    true,
-				},
-				"minimum_sources": {
-					Type:        schema.TypeInt,
-					Description: "The minimum number of agents or monitors that must meet the specified criteria in order to trigger an alert",
-					Optional:    true,
-				},
-				"minimum_sources_pct": {
-					Type:        schema.TypeInt,
-					Description: "The minimum percentage of agents or monitors that must meet the specified criteria in order to trigger an alert",
-					Optional:    true,
-				},
-				"notifications": {
-					Type:        schema.TypeSet,
-					Description: "List of notifications for the Alert Rule",
-					Optional:    true,
-					Elem: &schema.Resource{
-						Schema: map[string]*schema.Schema{
-							"email": {
-								Type:        schema.TypeSet,
-								Description: "Email notification",
-								Optional:    true,
-								Elem: &schema.Resource{
-									Schema: map[string]*schema.Schema{
-										"message": {
-											Type:        schema.TypeString,
-											Description: "Email message",
-											Optional:    true,
-										},
-										"recipient": {
-											Type:        schema.TypeList,
-											Description: "Email address",
-											Optional:    true,
-											Elem: &schema.Schema{
-												Type: schema.TypeString,
-											},
-										},
-									},
-								},
-							},
-						},
-					},
-				},
-				"notify_on_clear": {
-					Type:        schema.TypeInt,
-					Description: "Set to 1 to trigger the notification when the alert clears.",
-					Optional:    true,
-				},
-				"rounds_violating_mode": {
-					Type:        schema.TypeString,
-					Description: "ANY or EXACT.  EXACT requires that the same agent(s) meet the threshold in consecutive rounds; default is ANY",
-					Optional:    true,
-				},
-				"rounds_violating_out_of": {
-					Type:        schema.TypeInt,
-					Description: "Specifies the divisor (Y value) of the “X of Y times” condition in an alert rule.  Minimum value is 1, maximum value is 10.",
-					Optional:    true,
-				},
-				"rounds_violating_required": {
-					Type:        schema.TypeInt,
-					Description: "Specifies the numerator (X value) of the “X of Y times” condition in an alert rule.  Minimum value is 1, maximum value is 10. Must be less than or equal to roundsViolatingOutOf",
-					Optional:    true,
-				},
 				"rule_id": {
 					Type:        schema.TypeInt,
 					Description: "Rule ID",
 					Optional:    true,
 				},
-				"rule_name": {
-					Type:        schema.TypeString,
-					Description: "name of the alert rule",
-					Optional:    true,
-				},
-				"test_ids": {
-					Type:        schema.TypeList,
-					Description: "Valid test IDs",
-					Optional:    true,
-					Elem: &schema.Schema{
-						Type: schema.TypeInt,
-					},
-				},
 			},
 		},
 	},
@@ -519,11 +409,10 @@ var schemas = map[string]*schema.Schema{
 		Optional: true,
 	},
 	"default": {
-		Type:         schema.TypeInt,
-		Description:  "to set the rule as a default, set this value to 1.",
-		Optional:     true,
-		Default:      0,
-		ValidateFunc: validation.IntBetween(0, 1),
+		Type:        schema.TypeBool,
+		Description: "set the rule as a default",
+		Optional:    true,
+		Default:     false,
 	},
 	"description": {
 		Type:        schema.TypeString,
@@ -790,14 +679,16 @@ var schemas = map[string]*schema.Schema{
 		},
 	},
 	"minimum_sources": {
-		Type:        schema.TypeInt,
-		Description: "The minimum number of agents or monitors that must meet the specified criteria in order to trigger an alert",
-		Optional:    true,
+		Type:         schema.TypeInt,
+		Description:  "The minimum number of agents or monitors that must meet the specified criteria in order to trigger an alert; mutually exclusive with 'minimum_sources_pct'",
+		Optional:     true,
+		ValidateFunc: validation.IntAtLeast(1),
 	},
 	"minimum_sources_pct": {
-		Type:        schema.TypeInt,
-		Description: "The minimum percentage of agents or monitors that must meet the specified criteria in order to trigger an alert",
-		Optional:    true,
+		Type:         schema.TypeInt,
+		Description:  "The minimum percentage of agents or monitors that must meet the specified criteria in order to trigger an alert; mutually exclusive with 'minimum_sources'",
+		Optional:     true,
+		ValidateFunc: validation.IntBetween(0, 100),
 	},
 	"modified_by": {
 		Type:        schema.TypeString,
@@ -872,10 +763,10 @@ var schemas = map[string]*schema.Schema{
 		},
 	},
 	"notify_on_clear": {
-		Type:         schema.TypeInt,
-		Description:  "set to 1 to trigger the notification when the alert clears.",
-		Optional:     true,
-		ValidateFunc: validation.IntBetween(0, 1),
+		Type:        schema.TypeBool,
+		Description: "set to 'true' to trigger the notification when the alert clears.",
+		Optional:    true,
+		Default:     true,
 	},
 	"num_path_traces": {
 		Type:         schema.TypeInt,
@@ -991,6 +882,7 @@ var schemas = map[string]*schema.Schema{
 	"rounds_violating_mode": {
 		Type:         schema.TypeString,
 		Description:  "ANY or EXACT.  EXACT requires that the same agent(s) meet the threshold in consecutive rounds; default is ANY",
+		Default:      "ANY",
 		Optional:     true,
 		ValidateFunc: validation.StringInSlice([]string{"ANY", "EXACT"}, false),
 	},

thousandeyes/util.go

@@ -25,18 +25,19 @@ func expandAgents(v interface{}) thousandeyes.Agents {
 	return agents
 }
 
-func expandAlertRules(v interface{}) thousandeyes.AlertRules {
-	var alertRules thousandeyes.AlertRules
+func expandAlertRules(alertRules *[]thousandeyes.AlertRule) *[]thousandeyes.AlertRule {
+	if alertRules == nil {
+		return nil
+	}
 
-	for _, er := range v.([]interface{}) {
-		rer := er.(map[string]interface{})
-		alertRule := &thousandeyes.AlertRule{
-			RuleID: thousandeyes.Int(rer["rule_id"].(int)),
-		}
-		alertRules = append(alertRules, *alertRule)
+	ret := &[]thousandeyes.AlertRule{}
+	for _, ar := range *alertRules {
+		*ret = append(*ret, thousandeyes.AlertRule{
+			RuleID: ar.RuleID,
+		})
 	}
 
-	return alertRules
+	return ret
 }
 
 func expandBGPMonitors(v interface{}) thousandeyes.BGPMonitors {
@@ -111,7 +112,7 @@ func ResourceBuildStruct(d *schema.ResourceData, structPtr interface{}) interfac
 	for i := 0; i < v.NumField(); i++ {
 		tag := GetJSONKey(t.Field(i))
 		tfName := CamelCaseToUnderscore(tag)
-		val, ok := d.GetOk(tfName)
+		val, ok := d.GetOkExists(tfName)
 		if ok {
 			newVal := FillValue(val, v.Field(i).Interface())
 			setVal := reflect.ValueOf(newVal)
@@ -168,14 +169,24 @@ func FixReadValues(m interface{}, name string) (interface{}, error) {
 			}
 		}
 
-	// Remove all alert rule fields except for rule ID.
+	// Remove all alert rule fields except for rule ID. Ignore default rules.
 	case "alert_rules":
-		for i, v := range m.([]interface{}) {
-			rule := v.(map[string]interface{})
-			m.([]interface{})[i] = map[string]interface{}{
-				"rule_id": rule["rule_id"],
+		alert_rules := m.([]interface{})
+		// Edit the alert_rules slice in place, to return the same type.
+		i := 0
+		for i < len(alert_rules) {
+			rule := alert_rules[i].(map[string]interface{})
+			if is_default, ok := rule["default"]; ok && is_default != nil && *is_default.(*bool) {
+				// Remove this item from the slice
+				alert_rules = append(alert_rules[:i], alert_rules[i+1:]...)
+			} else {
+				alert_rules[i] = map[string]interface{}{
+					"rule_id": rule["rule_id"],
+				}
+				i = i + 1
 			}
 		}
+		m = alert_rules
 
 	// Remove all public BGP monitors. (ThousandEyes does not allow
 	// specifying individual public BGP monitors, and all available
@@ -283,15 +294,24 @@ func FixReadValues(m interface{}, name string) (interface{}, error) {
 		if err != nil {
 			return nil, err
 		}
-		m.(map[string]interface{})["email"] = e
 
-		m = []interface{}{
-			m.(map[string]interface{}),
+		if e != nil {
+			m.(map[string]interface{})["email"] = e
+
+			m = []interface{}{
+				m.(map[string]interface{}),
+			}
+		} else {
+			m = nil
 		}
 
 	case "email":
-		m = []interface{}{
-			m.(map[string]interface{}),
+		if len(m.(map[string]interface{})["recipient"].([]interface{})) == 0 {
+			m = nil
+		} else {
+			m = []interface{}{
+				m.(map[string]interface{}),
+			}
 		}
 
 	// Remove tests.
@@ -380,14 +400,10 @@ func resourceFixups(d *schema.ResourceData, structPtr interface{}) interface{} {
 		}
 	}
 
-	// If alert_rules is not set, set it explicitly to an empty
-	// slice. We do this to avoid ThousandEyes API setting default
-	// alert rules to Terraform-created tests, as that messes up the
-	// Terraform state.
-	if _, isSet := d.GetOk("alert_rules"); !isSet {
-		if _, hasField := t.FieldByName("AlertRules"); hasField {
-			v.FieldByName("AlertRules").Set(reflect.ValueOf(&[]thousandeyes.AlertRule{}))
-		}
+	_, hasAlertRules := t.FieldByName("AlertRules")
+	if hasAlertRules {
+		scrappedAlertRules := expandAlertRules(v.FieldByName("AlertRules").Interface().(*[]thousandeyes.AlertRule))
+		v.FieldByName("AlertRules").Set(reflect.ValueOf(scrappedAlertRules))
 	}
 
 	return structPtr

thousandeyes/util_test.go

@@ -160,20 +160,27 @@ func TestFixReadValues(t *testing.T) {
 	// alert_rules
 	alertRulesInput := []interface{}{
 		map[string]interface{}{
-			"rule_name": "foo",
-			"rule_id":   1,
+			"rule_name": thousandeyes.String("foo"),
+			"rule_id":   thousandeyes.Int(1),
+			"default":   thousandeyes.Bool(false),
 		},
 		map[string]interface{}{
-			"rule_name": "bar",
-			"rule_id":   2,
+			"rule_name": thousandeyes.String("bar"),
+			"rule_id":   thousandeyes.Int(2),
+			"default":   thousandeyes.Bool(false),
+		},
+		map[string]interface{}{
+			"rule_name": thousandeyes.String("bar"),
+			"rule_id":   thousandeyes.Int(3),
+			"default":   thousandeyes.Bool(true),
 		},
 	}
 	alertRulesTarget := []interface{}{
 		map[string]interface{}{
-			"rule_id": 1,
+			"rule_id": thousandeyes.Int(1),
 		},
 		map[string]interface{}{
-			"rule_id": 2,
+			"rule_id": thousandeyes.Int(2),
 		},
 	}
 	output, err = FixReadValues(alertRulesInput, "alert_rules")