Merge pull request #27410 from hashicorp/pselle/count-validate

De-duplicate count validate code
hashicorp · Jan 5, 2021 · 45a5a3c · 45a5a3c
2 parents afbde35 + dbde1be
commit 45a5a3c
Show file tree

Hide file tree

Showing 2 changed files with 47 additions and 82 deletions.
diff --git a/terraform/node_resource_validate.go b/terraform/node_resource_validate.go
@@ -11,8 +11,6 @@ import (
 	"github.com/hashicorp/terraform/provisioners"
 	"github.com/hashicorp/terraform/tfdiags"
 	"github.com/zclconf/go-cty/cty"
-	"github.com/zclconf/go-cty/cty/convert"
-	"github.com/zclconf/go-cty/cty/gocty"
 )
 
 // NodeValidatableResource represents a resource that is used for validation
@@ -410,69 +408,16 @@ func (n *NodeValidatableResource) validateResource(ctx EvalContext) tfdiags.Diag
 	return diags
 }
 
-func validateCount(ctx EvalContext, expr hcl.Expression) tfdiags.Diagnostics {
-	if expr == nil {
-		return nil
-	}
-
-	var diags tfdiags.Diagnostics
-
-	countVal, countDiags := ctx.EvaluateExpr(expr, cty.Number, nil)
-	diags = diags.Append(countDiags)
-	if diags.HasErrors() {
-		return diags
-	}
-
-	if countVal.IsNull() {
-		diags = diags.Append(&hcl.Diagnostic{
-			Severity: hcl.DiagError,
-			Summary:  "Invalid count argument",
-			Detail:   `The given "count" argument value is null. An integer is required.`,
-			Subject:  expr.Range().Ptr(),
-		})
-		return diags
-	}
-
-	var err error
-	countVal, err = convert.Convert(countVal, cty.Number)
-	if err != nil {
-		diags = diags.Append(&hcl.Diagnostic{
-			Severity: hcl.DiagError,
-			Summary:  "Invalid count argument",
-			Detail:   fmt.Sprintf(`The given "count" argument value is unsuitable: %s.`, err),
-			Subject:  expr.Range().Ptr(),
-		})
-		return diags
-	}
-
+func validateCount(ctx EvalContext, expr hcl.Expression) (diags tfdiags.Diagnostics) {
+	val, countDiags := evaluateCountExpressionValue(expr, ctx)
 	// If the value isn't known then that's the best we can do for now, but
-	// we'll check more thoroughly during the plan walk.
-	if !countVal.IsKnown() {
-		return diags
-	}
-
-	// If we _do_ know the value, then we can do a few more checks here.
-	var count int
-	err = gocty.FromCtyValue(countVal, &count)
-	if err != nil {
-		// Isn't a whole number, etc.
-		diags = diags.Append(&hcl.Diagnostic{
-			Severity: hcl.DiagError,
-			Summary:  "Invalid count argument",
-			Detail:   fmt.Sprintf(`The given "count" argument value is unsuitable: %s.`, err),
-			Subject:  expr.Range().Ptr(),
-		})
+	// we'll check more thoroughly during the plan walk
+	if !val.IsKnown() {
 		return diags
 	}
 
-	if count < 0 {
-		diags = diags.Append(&hcl.Diagnostic{
-			Severity: hcl.DiagError,
-			Summary:  "Invalid count argument",
-			Detail:   `The given "count" argument value is unsuitable: count cannot be negative.`,
-			Subject:  expr.Range().Ptr(),
-		})
-		return diags
+	if countDiags.HasErrors() {
+		diags = diags.Append(countDiags)
 	}
 
 	return diags

diff --git a/terraform/node_resource_validate_test.go b/terraform/node_resource_validate_test.go
@@ -202,6 +202,7 @@ func TestNodeValidatableResource_ValidateResource_managedResource(t *testing.T)
 }
 
 func TestNodeValidatableResource_ValidateResource_managedResourceCount(t *testing.T) {
+	// Setup
 	mp := simpleMockProvider()
 	mp.ValidateResourceTypeConfigFn = func(req providers.ValidateResourceTypeConfigRequest) providers.ValidateResourceTypeConfigResponse {
 		if got, want := req.TypeName, "test_object"; got != want {
@@ -214,35 +215,54 @@ func TestNodeValidatableResource_ValidateResource_managedResourceCount(t *testin
 	}
 
 	p := providers.Interface(mp)
-	rc := &configs.Resource{
-		Mode:  addrs.ManagedResourceMode,
-		Type:  "test_object",
-		Name:  "foo",
-		Count: hcltest.MockExprLiteral(cty.NumberIntVal(2)),
-		Config: configs.SynthBody("", map[string]cty.Value{
-			"test_string": cty.StringVal("bar"),
-		}),
-	}
-	node := NodeValidatableResource{
-		NodeAbstractResource: &NodeAbstractResource{
-			Addr:             mustConfigResourceAddr("test_foo.bar"),
-			Config:           rc,
-			ResolvedProvider: mustProviderConfig(`provider["registry.terraform.io/hashicorp/aws"]`),
-		},
-	}
 
 	ctx := &MockEvalContext{}
 	ctx.installSimpleEval()
 	ctx.ProviderSchemaSchema = mp.GetSchemaReturn
 	ctx.ProviderProvider = p
 
-	diags := node.validateResource(ctx)
-	if diags.HasErrors() {
-		t.Fatalf("err: %s", diags.Err())
+	tests := []struct {
+		name  string
+		count hcl.Expression
+	}{
+		{
+			"simple count",
+			hcltest.MockExprLiteral(cty.NumberIntVal(2)),
+		},
+		{
+			"marked count value",
+			hcltest.MockExprLiteral(cty.NumberIntVal(3).Mark("marked")),
+		},
 	}
 
-	if !mp.ValidateResourceTypeConfigCalled {
-		t.Fatal("Expected ValidateResourceTypeConfig to be called, but it was not!")
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			rc := &configs.Resource{
+				Mode:  addrs.ManagedResourceMode,
+				Type:  "test_object",
+				Name:  "foo",
+				Count: test.count,
+				Config: configs.SynthBody("", map[string]cty.Value{
+					"test_string": cty.StringVal("bar"),
+				}),
+			}
+			node := NodeValidatableResource{
+				NodeAbstractResource: &NodeAbstractResource{
+					Addr:             mustConfigResourceAddr("test_foo.bar"),
+					Config:           rc,
+					ResolvedProvider: mustProviderConfig(`provider["registry.terraform.io/hashicorp/aws"]`),
+				},
+			}
+
+			diags := node.validateResource(ctx)
+			if diags.HasErrors() {
+				t.Fatalf("err: %s", diags.Err())
+			}
+
+			if !mp.ValidateResourceTypeConfigCalled {
+				t.Fatal("Expected ValidateResourceTypeConfig to be called, but it was not!")
+			}
+		})
 	}
 }