Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[v13] Allow typical parsers to optionally support dynamic values #39891

Merged
merged 1 commit into from
Mar 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/services/label_expressions.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
4 changes: 2 additions & 2 deletions lib/utils/parse/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
Expand Down Expand Up @@ -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),
Expand Down
2 changes: 1 addition & 1 deletion lib/utils/typical/cached_parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion lib/utils/typical/cached_parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion lib/utils/typical/example_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,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
Expand Down
80 changes: 69 additions & 11 deletions lib/utils/typical/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -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).
//
Expand Down Expand Up @@ -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
Expand All @@ -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
}
Expand All @@ -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)
Expand All @@ -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{
Expand Down Expand Up @@ -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
}
Expand All @@ -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)
}

Expand All @@ -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")
Expand All @@ -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")
}

Expand All @@ -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
}
Expand Down
133 changes: 125 additions & 8 deletions lib/utils/typical/parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,13 @@ 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"
"golang.org/x/exp/slices"

"github.com/gravitational/teleport"
"github.com/gravitational/teleport/lib/utils"
"github.com/gravitational/teleport/lib/utils/typical"
)
Expand All @@ -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
Expand Down Expand Up @@ -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)
})
}
}
Loading