helper/validation: Prevented panics with ToDiagFunc() function when used inside Schema type Elem field (#915)

Reference: hashicorp/terraform-plugin-sdk#734

Author: bflad

.changelog/915.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+helper/validation: Prevented panics with `ToDiagFunc()` function when used inside `Schema` type `Elem` field, such as validating `TypeList` elements
+```

helper/validation/meta.go

@@ -66,8 +66,21 @@ func ToDiagFunc(validator schema.SchemaValidateFunc) schema.SchemaValidateDiagFu
 	return func(i interface{}, p cty.Path) diag.Diagnostics {
 		var diags diag.Diagnostics
 
-		attr := p[len(p)-1].(cty.GetAttrStep)
-		ws, es := validator(i, attr.Name)
+		// A practitioner-friendly key for any SchemaValidateFunc output.
+		// Generally this should be the last attribute name on the path.
+		// If not found for some unexpected reason, an empty string is fine
+		// as the diagnostic will have the full attribute path anyways.
+		var key string
+
+		// Reverse search for last cty.GetAttrStep
+		for i := len(p) - 1; i >= 0; i-- {
+			if pathStep, ok := p[i].(cty.GetAttrStep); ok {
+				key = pathStep.Name
+				break
+			}
+		}
+
+		ws, es := validator(i, key)
 
 		for _, w := range ws {
 			diags = append(diags, diag.Diagnostic{

helper/validation/meta_test.go

@@ -3,6 +3,9 @@ package validation
 import (
 	"regexp"
 	"testing"
+
+	"github.com/hashicorp/go-cty/cty"
+	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 )
 
 func TestValidationNoZeroValues(t *testing.T) {
@@ -101,36 +104,94 @@ func TestValidationAny(t *testing.T) {
 }
 
 func TestToDiagFunc(t *testing.T) {
-	runDiagTestCases(t, []diagTestCase{
-		{
+	t.Parallel()
+
+	testCases := map[string]struct {
+		path        cty.Path
+		val         interface{}
+		f           schema.SchemaValidateDiagFunc
+		expectedErr *regexp.Regexp
+	}{
+		"success-GetAttrStep-int": {
+			path: cty.Path{
+				cty.GetAttrStep{Name: "test_property"},
+			},
 			val: 43,
 			f: ToDiagFunc(Any(
 				IntAtLeast(42),
 				IntAtMost(5),
 			)),
 		},
-		{
+		"success-GetAttrStep-string": {
+			path: cty.Path{
+				cty.GetAttrStep{Name: "test_property"},
+			},
 			val: "foo",
 			f: ToDiagFunc(All(
 				StringLenBetween(1, 10),
 				StringIsNotWhiteSpace,
 			)),
 		},
-		{
+		"success-IndexStep-string": {
+			path: cty.Path{
+				cty.GetAttrStep{Name: "test_property"},
+				cty.IndexStep{Key: cty.NumberIntVal(0)},
+			},
+			val: "foo",
+			f:   ToDiagFunc(StringLenBetween(1, 10)),
+		},
+		"error-GetAttrStep-int-first": {
+			path: cty.Path{
+				cty.GetAttrStep{Name: "test_property"},
+			},
 			val: 7,
 			f: ToDiagFunc(Any(
 				IntAtLeast(42),
 				IntAtMost(5),
 			)),
-			expectedErr: regexp.MustCompile(`expected [\w]+ to be at least \(42\), got 7`),
+			expectedErr: regexp.MustCompile(`expected test_property to be at least \(42\), got 7`),
 		},
-		{
+		"error-GetAttrStep-int-second": {
+			path: cty.Path{
+				cty.GetAttrStep{Name: "test_property"},
+			},
 			val: 7,
 			f: ToDiagFunc(Any(
 				IntAtLeast(42),
 				IntAtMost(5),
 			)),
-			expectedErr: regexp.MustCompile(`expected [\w]+ to be at most \(5\), got 7`),
-		},
-	})
+			expectedErr: regexp.MustCompile(`expected test_property to be at most \(5\), got 7`),
+		},
+		"error-IndexStep-int": {
+			path: cty.Path{
+				cty.GetAttrStep{Name: "test_property"},
+				cty.IndexStep{Key: cty.NumberIntVal(0)},
+			},
+			val:         7,
+			f:           ToDiagFunc(IntAtLeast(42)),
+			expectedErr: regexp.MustCompile(`expected test_property to be at least \(42\), got 7`),
+		},
+	}
+
+	for name, testCase := range testCases {
+		name, testCase := name, testCase
+
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+
+			diags := testCase.f(testCase.val, testCase.path)
+
+			if !diags.HasError() && testCase.expectedErr == nil {
+				return
+			}
+
+			if diags.HasError() && testCase.expectedErr == nil {
+				t.Fatalf("expected to produce no errors, got %v", diags)
+			}
+
+			if !matchAnyDiagSummary(diags, testCase.expectedErr) {
+				t.Fatalf("expected to produce error matching %q, got %v", testCase.expectedErr, diags)
+			}
+		})
+	}
 }

helper/validation/testing.go

@@ -5,7 +5,6 @@ import (
 
 	testing "github.com/mitchellh/go-testing-interface"
 
-	"github.com/hashicorp/go-cty/cty"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 )
@@ -16,12 +15,6 @@ type testCase struct {
 	expectedErr *regexp.Regexp
 }
 
-type diagTestCase struct {
-	val         interface{}
-	f           schema.SchemaValidateDiagFunc
-	expectedErr *regexp.Regexp
-}
-
 func runTestCases(t testing.T, cases []testCase) {
 	t.Helper()
 
@@ -52,29 +45,6 @@ func matchAnyError(errs []error, r *regexp.Regexp) bool {
 	return false
 }
 
-func runDiagTestCases(t testing.T, cases []diagTestCase) {
-	t.Helper()
-
-	for i, tc := range cases {
-		p := cty.Path{
-			cty.GetAttrStep{Name: "test_property"},
-		}
-		diags := tc.f(tc.val, p)
-
-		if !diags.HasError() && tc.expectedErr == nil {
-			continue
-		}
-
-		if diags.HasError() && tc.expectedErr == nil {
-			t.Fatalf("expected test case %d to produce no errors, got %v", i, diags)
-		}
-
-		if !matchAnyDiagSummary(diags, tc.expectedErr) {
-			t.Fatalf("expected test case %d to produce error matching \"%s\", got %v", i, tc.expectedErr, diags)
-		}
-	}
-}
-
 func matchAnyDiagSummary(ds diag.Diagnostics, r *regexp.Regexp) bool {
 	for _, d := range ds {
 		if r.MatchString(d.Summary) {