Allow typical parsers to optionally support dynamic values (#39765) (#…

…39890)

Provides an opt-in mechanism to allow parsers to support dynamically
resolving variables at runtime. While this defeats some of the
static compile type benefits that typical guarantees, it is required
to allow existing parsers to be converted to use typical.

The existing resource parsers allow searching for variabls by their
fully qualified path. For example, the hostname of a server may
be quiered via `resource.spec.hostname`. In order to support this
typical could use some reflection to automatically add variables
to every nested field but that would be complicated and expensive.

Instead the ParserSpec can now be configured with
`GetUnknownIdentifier`, which allows creators of the parsers to
have full control over how a dynamic variable is resolved. The
parser prevents any dynamic resolution if the value is not configured
to limit the unsafe behvior to only parsers that explicitly opt in.

lib/services/label_expressions.go

@@ -52,7 +52,7 @@ func mustNewLabelExpressionParser() *typical.CachedParser[labelExpressionEnv, bo
 }
 
 func newLabelExpressionParser() (*typical.CachedParser[labelExpressionEnv, bool], error) {
-	parser, err := typical.NewCachedParser[labelExpressionEnv, bool](typical.ParserSpec{
+	parser, err := typical.NewCachedParser[labelExpressionEnv, bool](typical.ParserSpec[labelExpressionEnv]{
 		Variables: map[string]typical.Variable{
 			"user.spec.traits": typical.DynamicVariable(
 				func(env labelExpressionEnv) (map[string][]string, error) {

lib/utils/parse/parse.go

@@ -161,7 +161,7 @@ func newTraitsTemplateParser() (*typical.CachedParser[traitsTemplateEnv, []strin
 		})
 	}
 
-	parser, err := typical.NewCachedParser[traitsTemplateEnv, []string](typical.ParserSpec{
+	parser, err := typical.NewCachedParser[traitsTemplateEnv, []string](typical.ParserSpec[traitsTemplateEnv]{
 		Variables: map[string]typical.Variable{
 			"external": traitsVariable("external"),
 			"internal": traitsVariable("internal"),
@@ -340,7 +340,7 @@ func mustNewMatcherParser() *typical.CachedParser[matcherEnv, Matcher] {
 }
 
 func newMatcherParser() (*typical.CachedParser[matcherEnv, Matcher], error) {
-	parser, err := typical.NewCachedParser[matcherEnv, Matcher](typical.ParserSpec{
+	parser, err := typical.NewCachedParser[matcherEnv, Matcher](typical.ParserSpec[matcherEnv]{
 		Functions: map[string]typical.Function{
 			RegexpMatchFnName:    typical.UnaryFunction[matcherEnv](regexpMatch),
 			RegexpNotMatchFnName: typical.UnaryFunction[matcherEnv](regexpNotMatch),

lib/utils/typical/cached_parser.go

@@ -56,7 +56,7 @@ type CachedParser[TEnv, TResult any] struct {
 }
 
 // NewCachedParser creates a cached predicate expression parser with the given specification.
-func NewCachedParser[TEnv, TResult any](spec ParserSpec, opts ...ParserOption) (*CachedParser[TEnv, TResult], error) {
+func NewCachedParser[TEnv, TResult any](spec ParserSpec[TEnv], opts ...ParserOption) (*CachedParser[TEnv, TResult], error) {
 	parser, err := NewParser[TEnv, TResult](spec, opts...)
 	if err != nil {
 		return nil, trace.Wrap(err)

lib/utils/typical/cached_parser_test.go

@@ -31,7 +31,7 @@ func (t *testLogger) Infof(msg string, args ...any) {
 
 func TestCachedParser(t *testing.T) {
 	// A simple cached parser with no environment that always returns an int.
-	p, err := NewCachedParser[struct{}, int](ParserSpec{
+	p, err := NewCachedParser[struct{}, int](ParserSpec[struct{}]{
 		Functions: map[string]Function{
 			"inc": UnaryFunction[struct{}](func(i int) (int, error) {
 				return i + 1, nil

lib/utils/typical/example_test.go

@@ -27,7 +27,7 @@ type expressionEnv struct {
 }
 
 func Example() {
-	parser, err := typical.NewParser[expressionEnv, bool](typical.ParserSpec{
+	parser, err := typical.NewParser[expressionEnv, bool](typical.ParserSpec[expressionEnv]{
 		Variables: map[string]typical.Variable{
 			"traits": typical.DynamicVariable(func(e expressionEnv) (map[string][]string, error) {
 				return e.traits, nil

lib/utils/typical/parser.go

@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-// typical (TYPed predICAte Library) is a library for building better predicate
+// Package typical (TYPed predICAte Library) is a library for building better predicate
 // expression parsers faster. It is built on top of
 // [predicate](github.com/gravitational/predicate).
 //
@@ -52,20 +52,31 @@ type Expression[TEnv, TResult any] interface {
 }
 
 // ParserSpec defines a predicate language.
-type ParserSpec struct {
-	// Variables defines all of the literals and variables available to
+type ParserSpec[TEnv any] struct {
+	// Variables define the literals and variables available to
 	// expressions in the predicate language. It is a map from variable name to
 	// definition.
 	Variables map[string]Variable
 
-	// Functions defines all of the functions available to expressions in the
+	// Functions define the functions available to expressions in the
 	// predicate language.
 	Functions map[string]Function
 
-	// Methods defines all of the methods available to expressions in the
+	// Methods define the methods available to expressions in the
 	// predicate language. Methods are just functions that take their receiver
 	// as their first argument.
 	Methods map[string]Function
+
+	// GetUnknownIdentifier is used to retrieve any identifiers that cannot
+	// be determined statically. If not defined, any unknown identifiers result
+	// in a [UnknownIdentifierError].
+	//
+	// Useful in situations where a parser may allow specifying nested paths
+	// to variables without requiring users to enumerate all paths in [ParserSpec.Variables].
+	// Caution should be used when using this method as it shifts type safety
+	// guarantees from parse time to evaluation time.
+	// Typos in identifier names will also be caught at evaluation time instead of parse time.
+	GetUnknownIdentifier func(env TEnv, fields []string) (any, error)
 }
 
 // Variable holds the definition of a literal or variable. It is expected to be
@@ -82,7 +93,7 @@ type Function interface {
 // Parser is a predicate expression parser configured to parse expressions of a
 // specific expression language.
 type Parser[TEnv, TResult any] struct {
-	spec    ParserSpec
+	spec    ParserSpec[TEnv]
 	pred    predicate.Parser
 	options parserOptions
 }
@@ -105,7 +116,7 @@ func WithInvalidNamespaceHack() ParserOption {
 }
 
 // NewParser creates a predicate expression parser with the given specification.
-func NewParser[TEnv, TResult any](spec ParserSpec, opts ...ParserOption) (*Parser[TEnv, TResult], error) {
+func NewParser[TEnv, TResult any](spec ParserSpec[TEnv], opts ...ParserOption) (*Parser[TEnv, TResult], error) {
 	var options parserOptions
 	for _, opt := range opts {
 		opt(&options)
@@ -117,7 +128,7 @@ func NewParser[TEnv, TResult any](spec ParserSpec, opts ...ParserOption) (*Parse
 	}
 	def := predicate.Def{
 		GetIdentifier: p.getIdentifier,
-		GetProperty:   getProperty[TEnv],
+		GetProperty:   p.getProperty,
 		Functions:     make(map[string]any, len(spec.Functions)),
 		Methods:       make(map[string]any, len(spec.Methods)),
 		Operators: predicate.Operators{
@@ -187,7 +198,7 @@ func (p *Parser[TEnv, TResult]) getIdentifier(selector []string) (any, error) {
 		}
 		// Allow map lookups with map.key instead of map["key"]
 		if len(remaining) == 1 {
-			expr, err := getProperty[TEnv](v, remaining[0])
+			expr, err := p.getProperty(v, remaining[0])
 			if err == nil {
 				return expr, nil
 			}
@@ -208,11 +219,21 @@ func (p *Parser[TEnv, TResult]) getIdentifier(selector []string) (any, error) {
 		case 1:
 			return external, nil
 		case 2:
-			expr, err := getProperty[TEnv](external, selector[1])
+			expr, err := p.getProperty(external, selector[1])
 			return expr, trace.Wrap(err)
 		}
 	}
 
+	// Return a dynamic variable if and only if the parser was
+	// constructed to opt in to the dangerous behavior.
+	if p.spec.GetUnknownIdentifier != nil {
+		return dynamicVariable[TEnv, any]{
+			accessor: func(env TEnv) (any, error) {
+				return p.spec.GetUnknownIdentifier(env, selector)
+			},
+		}, nil
+	}
+
 	return nil, UnknownIdentifierError(joined)
 }
 
@@ -230,7 +251,7 @@ func (u UnknownIdentifierError) Identifier() string {
 
 // getProperty is a helper for parsing map[key] expressions and returns either a
 // propertyExpr or a dynamicMapExpr.
-func getProperty[TEnv any](mapVal, keyVal any) (any, error) {
+func (p *Parser[TEnv, TResult]) getProperty(mapVal, keyVal any) (any, error) {
 	keyExpr, err := coerce[TEnv, string](keyVal)
 	if err != nil {
 		return nil, trace.Wrap(err, "parsing key of index expression")
@@ -245,6 +266,17 @@ func getProperty[TEnv any](mapVal, keyVal any) (any, error) {
 	if dynamicMap, ok := mapVal.(indexExpressionBuilder[TEnv]); ok {
 		return dynamicMap.buildIndexExpression(keyExpr), nil
 	}
+
+	// Only allow falling back to an untyped expression if the parser was constructed
+	// to allow unknown identifiers. This ensures compile time type safety for all
+	// parsers that don't explicitly opt in to the more dangerous behavior required to
+	// support dynamic fields.
+	if p.spec.GetUnknownIdentifier != nil {
+		if mapExpr, ok := mapVal.(Expression[TEnv, any]); ok {
+			return untypedPropertyExpr[TEnv]{mapExpr, keyExpr}, nil
+		}
+	}
+
 	return nil, trace.Wrap(unexpectedTypeError[map[string]string](mapVal), "cannot take index of unexpected type")
 }
 
@@ -266,6 +298,32 @@ func (p propertyExpr[TEnv, TValues]) Evaluate(env TEnv) (TValues, error) {
 	return m[k], nil
 }
 
+type untypedPropertyExpr[TEnv any] struct {
+	mapExpr Expression[TEnv, any]
+	keyExpr Expression[TEnv, string]
+}
+
+func (u untypedPropertyExpr[TEnv]) Evaluate(env TEnv) (any, error) {
+	k, err := u.keyExpr.Evaluate(env)
+	if err != nil {
+		return nil, trace.Wrap(err, "evaluating key of index expression")
+	}
+	m, err := u.mapExpr.Evaluate(env)
+	if err != nil {
+		return nil, trace.Wrap(err, "evaluating base of index expression")
+	}
+	switch typedMap := m.(type) {
+	case map[string]string:
+		return typedMap[k], nil
+	case map[string][]string:
+		return typedMap[k], nil
+	case map[string]any:
+		return typedMap[k], nil
+	default:
+		return nil, trace.Wrap(unexpectedTypeError[map[string]any](u.mapExpr), "cannot take index of unexpected type")
+	}
+}
+
 type indexExpressionBuilder[TEnv any] interface {
 	buildIndexExpression(keyExpr Expression[TEnv, string]) any
 }

lib/utils/typical/parser_test.go

@@ -21,9 +21,12 @@ import (
 	"testing"
 
 	"github.com/gravitational/trace"
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"github.com/vulcand/predicate"
 	"golang.org/x/exp/maps"
 
+	"github.com/gravitational/teleport"
 	"github.com/gravitational/teleport/lib/utils"
 	"github.com/gravitational/teleport/lib/utils/typical"
 )
@@ -36,7 +39,7 @@ func TestParser(t *testing.T) {
 		traits map[string][]string
 	}
 
-	parser, err := typical.NewParser[env, bool](typical.ParserSpec{
+	parser, err := typical.NewParser[env, bool](typical.ParserSpec[env]{
 		Variables: map[string]typical.Variable{
 			"labels": typical.DynamicVariable(func(e env) (map[string]string, error) {
 				return e.labels, nil
@@ -421,13 +424,127 @@ func TestParser(t *testing.T) {
 func TestUnknownIdentifier(t *testing.T) {
 	t.Parallel()
 
-	parser, err := typical.NewParser[struct{}, bool](typical.ParserSpec{})
-	require.NoError(t, err)
+	type metadata struct {
+		Name   string            `json:"name"`
+		Labels map[string]string `json:"labels"`
+	}
 
-	_, err = parser.Parse("unknown")
+	type spec struct {
+		Hostname string `json:"hostname"`
+	}
+
+	type resource struct {
+		Metadata metadata `json:"metadata"`
+		Spec     spec     `json:"spec"`
+	}
 
-	var u typical.UnknownIdentifierError
-	require.True(t, errors.As(err, &u))
-	require.True(t, errors.As(trace.Wrap(err), &u))
-	require.Equal(t, "unknown", u.Identifier())
+	cases := []struct {
+		name               string
+		expression         string
+		knownVariablesOnly bool
+		parseAssertion     require.ErrorAssertionFunc
+		evalAssertion      func(t *testing.T, ok bool, err error)
+	}{
+		{
+			name:           "dynamic variable indexed and passed to function",
+			expression:     `hasPrefix(resource.metadata.labels["foo"], "b")`,
+			parseAssertion: require.NoError,
+			evalAssertion: func(t *testing.T, ok bool, err error) {
+				assert.True(t, ok)
+				require.NoError(t, err)
+			},
+		},
+		{
+			name:           "dynamic variable found via path not found",
+			expression:     `hasPrefix(resource.metadata.labels.foo, "b")`,
+			parseAssertion: require.NoError,
+			evalAssertion: func(t *testing.T, ok bool, err error) {
+				assert.False(t, ok)
+				require.Error(t, err)
+			},
+		},
+		{
+			name:           "incorrect type from dynamic variable passed to function",
+			expression:     `hasPrefix(resource.metadata.labels, "b")`,
+			parseAssertion: require.NoError,
+			evalAssertion: func(t *testing.T, ok bool, err error) {
+				assert.False(t, ok)
+				require.Error(t, err)
+			},
+		},
+		{
+			name:           "dynamic map",
+			expression:     `exists(resource.metadata.labels, "foo")`,
+			parseAssertion: require.NoError,
+			evalAssertion: func(t *testing.T, ok bool, err error) {
+				assert.True(t, ok)
+				require.NoError(t, err)
+			},
+		},
+		{
+			name:           "unknown dynamic variable",
+			expression:     "unknown",
+			parseAssertion: require.NoError,
+			evalAssertion: func(t *testing.T, ok bool, err error) {
+				assert.False(t, ok)
+				require.ErrorContains(t, err, "missing field names")
+			},
+		},
+		{
+			name:               "unknown variable",
+			expression:         "unknown",
+			knownVariablesOnly: true,
+			parseAssertion: func(t require.TestingT, err error, i ...interface{}) {
+				var u typical.UnknownIdentifierError
+				require.ErrorAs(t, err, &u, i...)
+				require.ErrorAs(t, trace.Wrap(err), &u, i...)
+				require.Equal(t, "unknown", u.Identifier(), i...)
+			},
+		},
+	}
+
+	srv := resource{
+		Metadata: metadata{
+			Name:   "test",
+			Labels: map[string]string{"foo": "bar", "animal": "llama"},
+		},
+		Spec: spec{Hostname: "server01"},
+	}
+
+	for _, test := range cases {
+		test := test
+		t.Run(test.name, func(t *testing.T) {
+			spec := typical.ParserSpec[resource]{
+				Functions: map[string]typical.Function{
+					"hasPrefix": typical.BinaryFunction[resource, string, string, bool](func(s, suffix string) (bool, error) {
+						return strings.HasPrefix(s, suffix), nil
+					}),
+					"exists": typical.BinaryFunction[resource, map[string]string, string, bool](func(m map[string]string, key string) (bool, error) {
+						_, ok := m[key]
+						return ok, nil
+					}),
+				},
+				GetUnknownIdentifier: func(env resource, fields []string) (any, error) {
+					f, err := predicate.GetFieldByTag(env, teleport.JSON, fields[1:])
+					return f, trace.Wrap(err)
+				},
+			}
+
+			if test.knownVariablesOnly {
+				spec.GetUnknownIdentifier = nil
+			}
+
+			parser, err := typical.NewParser[resource, bool](spec)
+			require.NoError(t, err, "creating parser")
+
+			expression, err := parser.Parse(test.expression)
+			test.parseAssertion(t, err)
+			if err != nil {
+				return
+			}
+
+			ok, err := expression.Evaluate(srv)
+			test.evalAssertion(t, ok, err)
+		})
+	}
 }