Merge pull request #891 from newrelic/fix/nrql-alert-condition

fix(nrql_alert_condition): update validation for nrql conditions

newrelic/resource_newrelic_nrql_alert_condition.go

@@ -72,17 +72,20 @@ func termSchema() *schema.Resource {
 				Description: "The duration of time, in seconds, that the threshold must violate for in order to create a violation. Value must be a multiple of 60 and within 120-3600 seconds for baseline conditions and 120-7200 seconds for static conditions.",
 				ValidateFunc: func(val interface{}, key string) (warns []string, errs []error) {
 					v := val.(int)
+					minVal := 60
+					maxVal := 7200
 
 					// Value must be a factor of 60.
 					if v%60 != 0 {
-						errs = append(errs, fmt.Errorf("%q must be a factor of 60, got: %d", key, v))
+						errs = append(errs, fmt.Errorf("%q must be a factor of %d, got: %d", key, minVal, v))
 					}
 
 					// This validation is a top-level validation check.
+					// Static conditions must be within range [60, 7200].
 					// Baseline conditions must be within range [120, 3600].
-					// Baseline condition validation lives in the "expand" functions.
-					if v < 120 || v > 7200 {
-						errs = append(errs, fmt.Errorf("%q must be between 120 and 7200 inclusive, got: %d", key, v))
+					// Outlier conditions must be within range [120, 3600].
+					if v < minVal || v > maxVal {
+						errs = append(errs, fmt.Errorf("%q must be between %d and %d inclusive, got: %d", key, minVal, maxVal, v))
 					}
 
 					return

newrelic/resource_newrelic_nrql_alert_condition_test.go

@@ -4,6 +4,7 @@ package newrelic
 
 import (
 	"fmt"
+	"regexp"
 	"strconv"
 	"testing"
 
@@ -87,6 +88,73 @@ func TestAccNewRelicNrqlAlertCondition_MissingPolicy(t *testing.T) {
 	})
 }
 
+func TestAccNewRelicNrqlAlertCondition_NerdGraphThresholdDurationValidationErrors(t *testing.T) {
+	rNameBaseline := acctest.RandString(5)
+	rNameOutlier := acctest.RandString(5)
+	conditionalAttrBaseline := `baseline_direction = "lower_only"`
+	conditionalAttrOutlier := `expected_groups = 2
+	open_violation_on_group_overlap = true`
+	facetClause := `FACET host`
+
+	resource.ParallelTest(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckNewRelicNrqlAlertConditionDestroy,
+		Steps: []resource.TestStep{
+			// Test: Baseline condition invalid `threshold_duration`
+			{
+				Config: testAccNewRelicNrqlAlertConditionNerdGraphConfig(
+					rNameBaseline,
+					"baseline",
+					20,
+					7200, // outside of accepted range [120, 3600] to test error handling
+					"static",
+					"0",
+					conditionalAttrBaseline,
+				),
+				ExpectError: regexp.MustCompile("Validation Error"),
+			},
+			// Test: Baseline condition invalid `threshold_duration`
+			{
+				Config: testAccNewRelicNrqlAlertConditionNerdGraphConfig(
+					rNameBaseline,
+					"baseline",
+					20,
+					60, // outside of accepted range [120, 3600] to test error handling
+					"static",
+					"0",
+					conditionalAttrBaseline,
+				),
+				ExpectError: regexp.MustCompile("Validation Error"),
+			},
+			// Test: Outlier condition invalid `threshold_duration`
+			{
+				Config: testAccNewRelicNrqlAlertConditionOutlierNerdGraphConfig(
+					rNameOutlier,
+					"outlier",
+					3,
+					7200, // outside of accepted range [120, 3600] to test error handling
+					conditionalAttrOutlier,
+					facetClause,
+				),
+				ExpectError: regexp.MustCompile("Validation Error"),
+			},
+			// Test: Outlier condition invalid `threshold_duration`
+			{
+				Config: testAccNewRelicNrqlAlertConditionOutlierNerdGraphConfig(
+					rNameOutlier,
+					"outlier",
+					3,
+					60, // outside of accepted range [120, 3600] to test error handling
+					conditionalAttrOutlier,
+					facetClause,
+				),
+				ExpectError: regexp.MustCompile("Validation Error"),
+			},
+		},
+	})
+}
+
 func TestAccNewRelicNrqlAlertCondition_NerdGraphBaseline(t *testing.T) {
 	resourceName := "newrelic_nrql_alert_condition.foo"
 	rName := acctest.RandString(5)

newrelic/structures_newrelic_nrql_alert_condition.go

@@ -202,16 +202,6 @@ func expandNrqlConditionTerm(term map[string]interface{}, conditionType, priorit
 	// required
 	threshold := term["threshold"].(float64)
 
-	if conditionType == "baseline" {
-		if duration < 120 || duration > 3600 {
-			return nil, fmt.Errorf("for baseline conditions duration must be in range %v, got %v", "[2, 60]", duration)
-		}
-
-		if threshold < 1 || threshold > 1000 {
-			return nil, fmt.Errorf("for baseline conditions threshold must be in range %v, got %v", "[1, 1000]", threshold)
-		}
-	}
-
 	thresholdOccurrences, err := expandNrqlThresholdOccurrences(term)
 	if err != nil {
 		return nil, err