From d62ef2943fe93a92e485f4fc0d97656823b7c9e7 Mon Sep 17 00:00:00 2001 From: TristonianJones Date: Thu, 1 Aug 2024 16:37:30 -0700 Subject: [PATCH 1/2] Support for typename import aliases --- policy/compiler.go | 34 +++++++++++++---- policy/helper_test.go | 28 +++++++++----- policy/parser.go | 61 ++++++++++++++++++++++++++++++ policy/testdata/errors/policy.yaml | 4 ++ policy/testdata/pb/policy.yaml | 13 ++++++- 5 files changed, 123 insertions(+), 17 deletions(-) diff --git a/policy/compiler.go b/policy/compiler.go index a8a67128..d3fd42bb 100644 --- a/policy/compiler.go +++ b/policy/compiler.go @@ -211,17 +211,36 @@ func CompileRule(env *cel.Env, p *Policy, opts ...CompilerOption) (*CompiledRule src: p.Source(), maxNestedExpressions: defaultMaxNestedExpressions, } + var err error errs := common.NewErrors(c.src) iss := cel.NewIssuesWithSourceInfo(errs, c.info) for _, o := range opts { - if err := o(c); err != nil { + if err = o(c); err != nil { iss.ReportErrorAtID(p.Name().ID, "error configuring compiler option: %s", err) return nil, iss } } - rule, ruleIss := c.compileRule(p.Rule(), c.env, iss) - iss = iss.Append(ruleIss) - return rule, iss + c.env, err = c.env.Extend(cel.EagerlyValidateDeclarations(true)) + if err != nil { + iss.ReportErrorAtID(p.Name().ID, "error configuring environment: %s", err) + return nil, iss + } + + importCount := len(p.Imports()) + if importCount > 0 { + importNames := make([]string, importCount) + for i, imp := range p.Imports() { + typeName := imp.Name().Value + importNames[i] = typeName + } + env, err := c.env.Extend(cel.Abbrevs(importNames...)) + if err != nil { + iss.ReportErrorAtID(p.Imports()[0].Name().ID, "error configuring imports: %s", err) + } else { + c.env = env + } + } + return c.compileRule(p.Rule(), c.env, iss) } type compiler struct { @@ -234,7 +253,6 @@ type compiler struct { } func (c *compiler) compileRule(r *Rule, ruleEnv *cel.Env, iss *cel.Issues) (*CompiledRule, *cel.Issues) { - var err error compiledVars := make([]*CompiledVariable, len(r.Variables())) for i, v := range r.Variables() { exprSrc := c.relSource(v.Expression()) @@ -254,9 +272,11 @@ func (c *compiler) compileRule(r *Rule, ruleEnv *cel.Env, iss *cel.Issues) (*Com // Introduce the variable into the environment. By extending the environment, the variables // are effectively scoped such that they must be declared before use. varDecl := decls.NewVariable(fmt.Sprintf("%s.%s", variablePrefix, varName), varType) - ruleEnv, err = ruleEnv.Extend(cel.Variable(varDecl.Name(), varDecl.Type())) + varEnv, err := ruleEnv.Extend(cel.Variable(varDecl.Name(), varDecl.Type())) if err != nil { - iss.ReportErrorAtID(v.Expression().ID, "invalid variable declaration") + iss.ReportErrorAtID(v.exprID, "invalid variable declaration: %s", err.Error()) + } else { + ruleEnv = varEnv } compiledVar := &CompiledVariable{ exprID: v.name.ID, diff --git a/policy/helper_test.go b/policy/helper_test.go index 29924e66..91a12fff 100644 --- a/policy/helper_test.go +++ b/policy/helper_test.go @@ -87,9 +87,13 @@ var ( }, { name: "pb", - expr: `(spec.single_int32 > 10) - ? optional.of("invalid spec, got single_int32=%d, wanted <= 10".format([spec.single_int32])) - : optional.none()`, + expr: ` + (spec.single_int32 > google.expr.proto3.test.TestAllTypes{single_int64: 10}.single_int64) + ? optional.of("invalid spec, got single_int32=%d, wanted <= 10".format([spec.single_int32])) + : ((spec.standalone_enum == google.expr.proto3.test.TestAllTypes.NestedEnum.BAR || + google.expr.proto3.test.ImportedGlobalEnum.IMPORT_BAR in spec.imported_enums) + ? optional.of("invalid spec, neither nested nor imported enums may refer to BAR or IMPORT_BAR") + : optional.none())`, envOpts: []cel.EnvOption{ cel.Types(&proto3pb.TestAllTypes{}), }, @@ -168,22 +172,28 @@ var ( }{ { name: "errors", - err: `ERROR: testdata/errors/policy.yaml:19:19: undeclared reference to 'spec' (in container '') + err: `ERROR: testdata/errors/policy.yaml:17:12: error configuring imports: invalid qualified name: bad import, wanted name of the form 'qualified.name' + | - name: "bad import" + | ...........^ +ERROR: testdata/errors/policy.yaml:21:19: undeclared reference to 'spec' (in container '') | expression: spec.labels | ..................^ -ERROR: testdata/errors/policy.yaml:21:50: Syntax error: mismatched input 'resource' expecting ')' +ERROR: testdata/errors/policy.yaml:22:7: invalid variable declaration: overlapping identifier for name 'variables.want' + | - name: want + | ......^ +ERROR: testdata/errors/policy.yaml:25:50: Syntax error: mismatched input 'resource' expecting ')' | expression: variables.want.filter(l, !(lin resource.labels)) | .................................................^ -ERROR: testdata/errors/policy.yaml:21:66: Syntax error: extraneous input ')' expecting +ERROR: testdata/errors/policy.yaml:25:66: Syntax error: extraneous input ')' expecting | expression: variables.want.filter(l, !(lin resource.labels)) | .................................................................^ -ERROR: testdata/errors/policy.yaml:23:27: Syntax error: mismatched input '2' expecting {'}', ','} +ERROR: testdata/errors/policy.yaml:27:27: Syntax error: mismatched input '2' expecting {'}', ','} | expression: "{1:305 2:569}" | ..........................^ -ERROR: testdata/errors/policy.yaml:31:75: Syntax error: extraneous input ']' expecting ')' +ERROR: testdata/errors/policy.yaml:35:75: Syntax error: extraneous input ']' expecting ')' | "missing one or more required labels: %s".format(variables.missing]) | ..........................................................................^ -ERROR: testdata/errors/policy.yaml:34:67: undeclared reference to 'format' (in container '') +ERROR: testdata/errors/policy.yaml:38:67: undeclared reference to 'format' (in container '') | "invalid values provided on one or more labels: %s".format([variables.invalid]) | ..................................................................^ ERROR: testdata/errors/policy.yaml:38:16: incompatible output types: bool not assignable to string diff --git a/policy/parser.go b/policy/parser.go index 5dc57e21..e2035abc 100644 --- a/policy/parser.go +++ b/policy/parser.go @@ -39,12 +39,14 @@ func NewPolicy(src *Source, info *ast.SourceInfo) *Policy { source: src, info: info, semantic: firstMatch, + imports: []*Import{}, } } // Policy declares a name, rule, and evaluation semantic for a given expression graph. type Policy struct { name ValueString + imports []*Import rule *Rule semantic semanticType info *ast.SourceInfo @@ -63,6 +65,10 @@ func (p *Policy) SourceInfo() *ast.SourceInfo { return p.info } +func (p *Policy) Imports() []*Import { + return p.imports +} + // Name returns the name of the policy. func (p *Policy) Name() ValueString { return p.name @@ -88,6 +94,10 @@ func (p *Policy) MetadataKeys() []string { return keys } +func (p *Policy) AddImport(name ValueString) { + p.imports = append(p.imports, &Import{name: name}) +} + // SetName configures the policy name. func (p *Policy) SetName(name ValueString) { p.name = name @@ -119,6 +129,22 @@ func (p *Policy) GetExplanationOutputPolicy() *Policy { return &ep } +func NewImport() *Import { + return &Import{} +} + +type Import struct { + name ValueString +} + +func (i *Import) Name() ValueString { + return i.name +} + +func (i *Import) SetName(name ValueString) { + i.name = name +} + // NewRule creates a Rule instance. func NewRule(exprID int64) *Rule { return &Rule{ @@ -582,6 +608,8 @@ func (p *parserImpl) ParsePolicy(ctx ParserContext, node *yaml.Node) *Policy { fieldName := key.Value val := node.Content[i+1] switch fieldName { + case "imports": + p.parseImports(ctx, policy, val) case "name": policy.SetName(ctx.NewString(val)) case "rule": @@ -593,6 +621,39 @@ func (p *parserImpl) ParsePolicy(ctx ParserContext, node *yaml.Node) *Policy { return policy } +func (p *parserImpl) parseImports(ctx ParserContext, policy *Policy, node *yaml.Node) { + id := ctx.CollectMetadata(node) + if p.assertYamlType(id, node, yamlList) == nil { + return + } + for _, val := range node.Content { + policy.AddImport(p.parseImport(ctx, policy, val).Name()) + } +} + +func (p *parserImpl) parseImport(ctx ParserContext, _ *Policy, node *yaml.Node) *Import { + id := ctx.CollectMetadata(node) + imp := NewImport() + if p.assertYamlType(id, node, yamlMap) == nil || !p.checkMapValid(ctx, id, node) { + return imp + } + for i := 0; i < len(node.Content); i += 2 { + key := node.Content[i] + ctx.CollectMetadata(key) + fieldName := key.Value + val := node.Content[i+1] + if val.Style == yaml.FoldedStyle || val.Style == yaml.LiteralStyle { + val.Line++ + val.Column = key.Column + 1 + } + switch fieldName { + case "name": + imp.SetName(ctx.NewString(val)) + } + } + return imp +} + // ParseRule will parse the current yaml node as though it is the entry point to a rule. func (p *parserImpl) ParseRule(ctx ParserContext, policy *Policy, node *yaml.Node) *Rule { r, id := ctx.NewRule(node) diff --git a/policy/testdata/errors/policy.yaml b/policy/testdata/errors/policy.yaml index e43c9453..02775801 100644 --- a/policy/testdata/errors/policy.yaml +++ b/policy/testdata/errors/policy.yaml @@ -13,10 +13,14 @@ # limitations under the License. name: "errors" +imports: + - name: "bad import" rule: variables: - name: want expression: spec.labels + - name: want + expression: "2" - name: missing expression: variables.want.filter(l, !(lin resource.labels)) - name: bad_data diff --git a/policy/testdata/pb/policy.yaml b/policy/testdata/pb/policy.yaml index 8a2723b5..f16d5fbe 100644 --- a/policy/testdata/pb/policy.yaml +++ b/policy/testdata/pb/policy.yaml @@ -13,8 +13,19 @@ # limitations under the License. name: "pb" + +imports: + - name: google.expr.proto3.test.TestAllTypes.NestedEnum + - name: google.expr.proto3.test.ImportedGlobalEnum + rule: match: - - condition: spec.single_int32 > 10 + - condition: > + spec.single_int32 > TestAllTypes{single_int64: 10}.single_int64 output: | "invalid spec, got single_int32=%d, wanted <= 10".format([spec.single_int32]) + - condition: > + spec.standalone_enum == NestedEnum.BAR || + ImportedGlobalEnum.IMPORT_BAR in spec.imported_enums + output: | + "invalid spec, neither nested nor imported enums may refer to BAR or IMPORT_BAR" From a311bffd1ff387cd3ec07652bf993ae92a604da5 Mon Sep 17 00:00:00 2001 From: TristonianJones Date: Thu, 1 Aug 2024 16:37:30 -0700 Subject: [PATCH 2/2] Support for typename import aliases --- common/containers/container.go | 12 +++ common/containers/container_test.go | 122 +++++++++++++++++----------- policy/BUILD.bazel | 1 + policy/compiler.go | 15 +++- policy/compiler_test.go | 14 ++++ policy/helper_test.go | 43 +++++----- policy/parser.go | 26 ++++-- policy/parser_test.go | 59 ++++++++++++++ policy/testdata/errors/policy.yaml | 3 + policy/testdata/pb/config.yaml | 2 +- policy/testdata/pb/policy.yaml | 4 +- policy/testdata/pb/tests.yaml | 9 +- 12 files changed, 224 insertions(+), 86 deletions(-) diff --git a/common/containers/container.go b/common/containers/container.go index 52153d4c..da174a4e 100644 --- a/common/containers/container.go +++ b/common/containers/container.go @@ -19,6 +19,7 @@ package containers import ( "fmt" "strings" + "unicode" "github.com/google/cel-go/common/ast" ) @@ -212,6 +213,13 @@ type ContainerOption func(*Container) (*Container, error) func Abbrevs(qualifiedNames ...string) ContainerOption { return func(c *Container) (*Container, error) { for _, qn := range qualifiedNames { + qn = strings.TrimSpace(qn) + for _, r := range []rune(qn) { + if !isIdentifierChar(r) { + return nil, fmt.Errorf( + "invalid qualified name: %s, wanted name of the form 'qualified.name'", qn) + } + } ind := strings.LastIndex(qn, ".") if ind <= 0 || ind >= len(qn)-1 { return nil, fmt.Errorf( @@ -278,6 +286,10 @@ func aliasAs(kind, qualifiedName, alias string) ContainerOption { } } +func isIdentifierChar(r rune) bool { + return r <= unicode.MaxASCII && (r == '.' || r == '_' || unicode.IsLetter(r) || unicode.IsNumber(r)) +} + // Name sets the fully-qualified name of the Container. func Name(name string) ContainerOption { return func(c *Container) (*Container, error) { diff --git a/common/containers/container_test.go b/common/containers/container_test.go index 224490e9..06efd419 100644 --- a/common/containers/container_test.go +++ b/common/containers/container_test.go @@ -15,6 +15,7 @@ package containers import ( + "fmt" "reflect" "testing" @@ -104,54 +105,79 @@ func TestContainers_Abbrevs(t *testing.T) { } func TestContainers_Aliasing_Errors(t *testing.T) { - _, err := NewContainer(Abbrevs("my.alias.R", "yer.other.R")) - wantErr := "abbreviation collides with existing reference: " + - "name=yer.other.R, abbreviation=R, existing=my.alias.R" - if err == nil || err.Error() != wantErr { - t.Errorf("got error %v, expected %s.", err, wantErr) - } - - _, err = NewContainer(Name("a.b.c.M.N"), Abbrevs("my.alias.a", "yer.other.b")) - wantErr = "abbreviation collides with container name: name=my.alias.a, " + - "abbreviation=a, container=a.b.c.M.N" - if err == nil || err.Error() != wantErr { - t.Errorf("got error %v, expected %s.", err, wantErr) - } - - _, err = NewContainer(Abbrevs(".bad")) - wantErr = "invalid qualified name: .bad, wanted name of the form 'qualified.name'" - if err == nil || err.Error() != wantErr { - t.Errorf("got error %v, expected %s.", err, wantErr) - } - - _, err = NewContainer(Abbrevs("bad.alias.")) - wantErr = "invalid qualified name: bad.alias., wanted name of the form 'qualified.name'" - if err == nil || err.Error() != wantErr { - t.Errorf("got error %v, expected %s.", err, wantErr) - } - - _, err = NewContainer(Alias("a", "b")) - wantErr = "alias must refer to a valid qualified name: a" - if err == nil || err.Error() != wantErr { - t.Errorf("got error %v, expected %s.", err, wantErr) - } - - _, err = NewContainer(Alias("my.alias", "b.c")) - wantErr = "alias must be non-empty and simple (not qualified): alias=b.c" - if err == nil || err.Error() != wantErr { - t.Errorf("got error %v, expected %s.", err, wantErr) - } - - _, err = NewContainer(Alias(".my.qual.name", "a")) - wantErr = "qualified name must not begin with a leading '.': .my.qual.name" - if err == nil || err.Error() != wantErr { - t.Errorf("got error %v, expected %s.", err, wantErr) - } - - _, err = NewContainer(Alias(".my.qual.name", "a")) - wantErr = "qualified name must not begin with a leading '.': .my.qual.name" - if err == nil || err.Error() != wantErr { - t.Errorf("got error %v, expected %s.", err, wantErr) + type aliasDef struct { + name string + alias string + } + tests := []struct { + container string + abbrevs []string + aliases []aliasDef + err string + }{ + { + abbrevs: []string{"my.alias.R", "yer.other.R"}, + err: "abbreviation collides with existing reference: " + + "name=yer.other.R, abbreviation=R, existing=my.alias.R", + }, + { + container: "a.b.c.M.N", + abbrevs: []string{"my.alias.a", "yer.other.b"}, + err: "abbreviation collides with container name: name=my.alias.a, " + + "abbreviation=a, container=a.b.c.M.N", + }, + { + abbrevs: []string{".bad"}, + err: "invalid qualified name: .bad, wanted name of the form 'qualified.name'", + }, + { + abbrevs: []string{"bad.alias."}, + err: "invalid qualified name: bad.alias., wanted name of the form 'qualified.name'", + }, + { + abbrevs: []string{" bad_alias1"}, + err: "invalid qualified name: bad_alias1, wanted name of the form 'qualified.name'", + }, + { + abbrevs: []string{" bad.alias! "}, + err: "invalid qualified name: bad.alias!, wanted name of the form 'qualified.name'", + }, + { + aliases: []aliasDef{{name: "a", alias: "b"}}, + err: "alias must refer to a valid qualified name: a", + }, + { + aliases: []aliasDef{{name: "my.alias", alias: "b.c"}}, + err: "alias must be non-empty and simple (not qualified): alias=b.c", + }, + { + aliases: []aliasDef{{name: ".my.qual.name", alias: "a'"}}, + err: "qualified name must not begin with a leading '.': .my.qual.name", + }, + } + for i, tst := range tests { + tc := tst + t.Run(fmt.Sprintf("[%d]", i), func(t *testing.T) { + opts := []ContainerOption{} + if tc.container != "" { + opts = append(opts, Name(tc.container)) + } + if len(tc.abbrevs) != 0 { + opts = append(opts, Abbrevs(tc.abbrevs...)) + } + if len(tc.aliases) != 0 { + for _, a := range tc.aliases { + opts = append(opts, Alias(a.name, a.alias)) + } + } + _, err := NewContainer(opts...) + if err == nil { + t.Fatalf("NewContainer() succeeded, wanted err %s", tc.err) + } + if err.Error() != tc.err { + t.Errorf("NewContainer() got error %s, wanted error %s", err.Error(), tc.err) + } + }) } } diff --git a/policy/BUILD.bazel b/policy/BUILD.bazel index 1cc2b3c0..c35f46ce 100644 --- a/policy/BUILD.bazel +++ b/policy/BUILD.bazel @@ -34,6 +34,7 @@ go_library( "//cel:go_default_library", "//common:go_default_library", "//common/ast:go_default_library", + "//common/containers:go_default_library", "//common/decls:go_default_library", "//common/operators:go_default_library", "//common/types:go_default_library", diff --git a/policy/compiler.go b/policy/compiler.go index d3fd42bb..6a362328 100644 --- a/policy/compiler.go +++ b/policy/compiler.go @@ -22,6 +22,7 @@ import ( "github.com/google/cel-go/cel" "github.com/google/cel-go/common" "github.com/google/cel-go/common/ast" + "github.com/google/cel-go/common/containers" "github.com/google/cel-go/common/decls" "github.com/google/cel-go/common/types" "github.com/google/cel-go/common/types/ref" @@ -228,14 +229,20 @@ func CompileRule(env *cel.Env, p *Policy, opts ...CompilerOption) (*CompiledRule importCount := len(p.Imports()) if importCount > 0 { - importNames := make([]string, importCount) - for i, imp := range p.Imports() { + importNames := make([]string, 0, importCount) + for _, imp := range p.Imports() { typeName := imp.Name().Value - importNames[i] = typeName + _, err := containers.NewContainer(containers.Abbrevs(typeName)) + if err != nil { + iss.ReportErrorAtID(imp.Name().ID, "error configuring import: %s", err) + } else { + importNames = append(importNames, typeName) + } } env, err := c.env.Extend(cel.Abbrevs(importNames...)) if err != nil { - iss.ReportErrorAtID(p.Imports()[0].Name().ID, "error configuring imports: %s", err) + // validation happens earlier in the sequence, so this should be unreachable. + iss.ReportErrorAtID(p.Imports()[0].SourceID(), "error configuring imports: %s", err) } else { c.env = env } diff --git a/policy/compiler_test.go b/policy/compiler_test.go index 9323dce9..f8dd7348 100644 --- a/policy/compiler_test.go +++ b/policy/compiler_test.go @@ -86,6 +86,20 @@ func TestCompiledRuleHasOptionalOutput(t *testing.T) { } } +func TestMaxNestedExpressions_Error(t *testing.T) { + policyName := "required_labels" + wantError := `ERROR: testdata/required_labels/policy.yaml:15:8: error configuring compiler option: nested expression limit must be non-negative, non-zero value: -1 + | name: "required_labels" + | .......^` + _, _, iss := compile(t, policyName, []ParserOption{}, []cel.EnvOption{}, []CompilerOption{MaxNestedExpressions(-1)}) + if iss.Err() == nil { + t.Fatalf("compile(%s) did not error, wanted %s", policyName, wantError) + } + if iss.Err().Error() != wantError { + t.Errorf("compile(%s) got error %s, wanted %s", policyName, iss.Err().Error(), wantError) + } +} + func BenchmarkCompile(b *testing.B) { for _, tst := range policyTests { r := newRunner(b, tst.name, tst.expr, tst.parseOpts, tst.envOpts...) diff --git a/policy/helper_test.go b/policy/helper_test.go index 91a12fff..79b205cb 100644 --- a/policy/helper_test.go +++ b/policy/helper_test.go @@ -63,26 +63,26 @@ var ( { name: "nested_rule2", expr: ` - cel.bind(variables.permitted_regions, ["us", "uk", "es"], - resource.?user.orValue("").startsWith("bad") - ? cel.bind(variables.banned_regions, {"us": false, "ru": false, "ir": false}, + cel.bind(variables.permitted_regions, ["us", "uk", "es"], + resource.?user.orValue("").startsWith("bad") + ? cel.bind(variables.banned_regions, {"us": false, "ru": false, "ir": false}, (resource.origin in variables.banned_regions && - !(resource.origin in variables.permitted_regions)) - ? {"banned": "restricted_region"} : {"banned": "bad_actor"}) - : (!(resource.origin in variables.permitted_regions) + !(resource.origin in variables.permitted_regions)) + ? {"banned": "restricted_region"} : {"banned": "bad_actor"}) + : (!(resource.origin in variables.permitted_regions) ? {"banned": "unconfigured_region"} : {}))`, }, { name: "nested_rule3", expr: ` - cel.bind(variables.permitted_regions, ["us", "uk", "es"], - resource.?user.orValue("").startsWith("bad") + cel.bind(variables.permitted_regions, ["us", "uk", "es"], + resource.?user.orValue("").startsWith("bad") ? optional.of( cel.bind(variables.banned_regions, {"us": false, "ru": false, "ir": false}, (resource.origin in variables.banned_regions && - !(resource.origin in variables.permitted_regions)) - ? {"banned": "restricted_region"} : {"banned": "bad_actor"})) - : (!(resource.origin in variables.permitted_regions) + !(resource.origin in variables.permitted_regions)) + ? {"banned": "restricted_region"} : {"banned": "bad_actor"})) + : (!(resource.origin in variables.permitted_regions) ? optional.of({"banned": "unconfigured_region"}) : optional.none()))`, }, { @@ -172,31 +172,34 @@ var ( }{ { name: "errors", - err: `ERROR: testdata/errors/policy.yaml:17:12: error configuring imports: invalid qualified name: bad import, wanted name of the form 'qualified.name' + err: `ERROR: testdata/errors/policy.yaml:19:1: error configuring import: invalid qualified name: punc.Import!, wanted name of the form 'qualified.name' + | punc.Import! + | ^ +ERROR: testdata/errors/policy.yaml:20:12: error configuring import: invalid qualified name: bad import, wanted name of the form 'qualified.name' | - name: "bad import" | ...........^ -ERROR: testdata/errors/policy.yaml:21:19: undeclared reference to 'spec' (in container '') +ERROR: testdata/errors/policy.yaml:24:19: undeclared reference to 'spec' (in container '') | expression: spec.labels | ..................^ -ERROR: testdata/errors/policy.yaml:22:7: invalid variable declaration: overlapping identifier for name 'variables.want' +ERROR: testdata/errors/policy.yaml:25:7: invalid variable declaration: overlapping identifier for name 'variables.want' | - name: want | ......^ -ERROR: testdata/errors/policy.yaml:25:50: Syntax error: mismatched input 'resource' expecting ')' +ERROR: testdata/errors/policy.yaml:28:50: Syntax error: mismatched input 'resource' expecting ')' | expression: variables.want.filter(l, !(lin resource.labels)) | .................................................^ -ERROR: testdata/errors/policy.yaml:25:66: Syntax error: extraneous input ')' expecting +ERROR: testdata/errors/policy.yaml:28:66: Syntax error: extraneous input ')' expecting | expression: variables.want.filter(l, !(lin resource.labels)) | .................................................................^ -ERROR: testdata/errors/policy.yaml:27:27: Syntax error: mismatched input '2' expecting {'}', ','} +ERROR: testdata/errors/policy.yaml:30:27: Syntax error: mismatched input '2' expecting {'}', ','} | expression: "{1:305 2:569}" | ..........................^ -ERROR: testdata/errors/policy.yaml:35:75: Syntax error: extraneous input ']' expecting ')' +ERROR: testdata/errors/policy.yaml:38:75: Syntax error: extraneous input ']' expecting ')' | "missing one or more required labels: %s".format(variables.missing]) | ..........................................................................^ -ERROR: testdata/errors/policy.yaml:38:67: undeclared reference to 'format' (in container '') +ERROR: testdata/errors/policy.yaml:41:67: undeclared reference to 'format' (in container '') | "invalid values provided on one or more labels: %s".format([variables.invalid]) | ..................................................................^ -ERROR: testdata/errors/policy.yaml:38:16: incompatible output types: bool not assignable to string +ERROR: testdata/errors/policy.yaml:45:16: incompatible output types: bool not assignable to string | output: "'false'" | ...............^`, }, diff --git a/policy/parser.go b/policy/parser.go index e2035abc..78905e3e 100644 --- a/policy/parser.go +++ b/policy/parser.go @@ -65,6 +65,7 @@ func (p *Policy) SourceInfo() *ast.SourceInfo { return p.info } +// Imports returns the list of imports associated with the policy. func (p *Policy) Imports() []*Import { return p.imports } @@ -94,8 +95,9 @@ func (p *Policy) MetadataKeys() []string { return keys } -func (p *Policy) AddImport(name ValueString) { - p.imports = append(p.imports, &Import{name: name}) +// AddImport adds an import to the policy. +func (p *Policy) AddImport(i *Import) { + p.imports = append(p.imports, i) } // SetName configures the policy name. @@ -129,18 +131,28 @@ func (p *Policy) GetExplanationOutputPolicy() *Policy { return &ep } -func NewImport() *Import { - return &Import{} +// NewImport creates a new typename import node +func NewImport(exprID int64) *Import { + return &Import{exprID: exprID} } +// Import represents an imported type name which is aliased within CEL expressions. type Import struct { - name ValueString + exprID int64 + name ValueString } +// SourceID returns the source identifier associated with the import. +func (i *Import) SourceID() int64 { + return i.exprID +} + +// Name returns the fully qualified type name. func (i *Import) Name() ValueString { return i.name } +// SetName updates the fully qualified type name for the import. func (i *Import) SetName(name ValueString) { i.name = name } @@ -627,13 +639,13 @@ func (p *parserImpl) parseImports(ctx ParserContext, policy *Policy, node *yaml. return } for _, val := range node.Content { - policy.AddImport(p.parseImport(ctx, policy, val).Name()) + policy.AddImport(p.parseImport(ctx, policy, val)) } } func (p *parserImpl) parseImport(ctx ParserContext, _ *Policy, node *yaml.Node) *Import { id := ctx.CollectMetadata(node) - imp := NewImport() + imp := NewImport(id) if p.assertYamlType(id, node, yamlMap) == nil || !p.checkMapValid(ctx, id, node) { return imp } diff --git a/policy/parser_test.go b/policy/parser_test.go index 89912683..fc0b9aa5 100644 --- a/policy/parser_test.go +++ b/policy/parser_test.go @@ -147,6 +147,65 @@ rule: explanation: "hi"`, err: `ERROR: :8:7: explanation can only be set on output match cases, not nested rules | explanation: "hi" + | ......^`, + }, + { + txt: ` +imports: + - first`, + err: `ERROR: :3:5: got yaml node type tag:yaml.org,2002:str, wanted type(s) [tag:yaml.org,2002:map] + | - first + | ....^`, + }, + { + txt: ` +imports: + first: name`, + err: `ERROR: :3:3: got yaml node type tag:yaml.org,2002:map, wanted type(s) [tag:yaml.org,2002:seq] + | first: name + | ..^`, + }, + { + txt: ` +rule: + - variables: name`, + err: `ERROR: :3:3: got yaml node type tag:yaml.org,2002:seq, wanted type(s) [tag:yaml.org,2002:map] + | - variables: name + | ..^`, + }, + { + txt: ` +rule: + variables: name`, + err: `ERROR: :3:14: got yaml node type tag:yaml.org,2002:str, wanted type(s) [tag:yaml.org,2002:seq] + | variables: name + | .............^`, + }, + { + txt: ` +rule: + variables: + - name`, + err: `ERROR: :4:7: got yaml node type tag:yaml.org,2002:str, wanted type(s) [tag:yaml.org,2002:map] + | - name + | ......^`, + }, + { + txt: ` +rule: + match: + name: value`, + err: `ERROR: :4:5: got yaml node type tag:yaml.org,2002:map, wanted type(s) [tag:yaml.org,2002:seq] + | name: value + | ....^`, + }, + { + txt: ` +rule: + match: + - name`, + err: `ERROR: :4:7: got yaml node type tag:yaml.org,2002:str, wanted type(s) [tag:yaml.org,2002:map] + | - name | ......^`, }, } diff --git a/policy/testdata/errors/policy.yaml b/policy/testdata/errors/policy.yaml index 02775801..b613362c 100644 --- a/policy/testdata/errors/policy.yaml +++ b/policy/testdata/errors/policy.yaml @@ -14,6 +14,9 @@ name: "errors" imports: + - name: " untrimmed.Import1 " + - name: > + punc.Import! - name: "bad import" rule: variables: diff --git a/policy/testdata/pb/config.yaml b/policy/testdata/pb/config.yaml index bd554694..963996f1 100644 --- a/policy/testdata/pb/config.yaml +++ b/policy/testdata/pb/config.yaml @@ -13,7 +13,7 @@ # limitations under the License. name: "pb" -container: "google.expr.proto3.test" +container: "google.expr.proto3" extensions: - name: "strings" version: 2 diff --git a/policy/testdata/pb/policy.yaml b/policy/testdata/pb/policy.yaml index f16d5fbe..cebff4c6 100644 --- a/policy/testdata/pb/policy.yaml +++ b/policy/testdata/pb/policy.yaml @@ -15,8 +15,10 @@ name: "pb" imports: + - name: google.expr.proto3.test.TestAllTypes - name: google.expr.proto3.test.TestAllTypes.NestedEnum - - name: google.expr.proto3.test.ImportedGlobalEnum + - name: | + google.expr.proto3.test.ImportedGlobalEnum rule: match: diff --git a/policy/testdata/pb/tests.yaml b/policy/testdata/pb/tests.yaml index c06c19b1..770bcad0 100644 --- a/policy/testdata/pb/tests.yaml +++ b/policy/testdata/pb/tests.yaml @@ -18,17 +18,16 @@ section: tests: - name: "good spec" input: - spec: + spec: expr: > - TestAllTypes{single_int32: 10} + test.TestAllTypes{single_int32: 10} output: "optional.none()" - name: "invalid" tests: - name: "bad spec" input: - spec: + spec: expr: > - TestAllTypes{single_int32: 11} + test.TestAllTypes{single_int32: 11} output: > "invalid spec, got single_int32=11, wanted <= 10" - \ No newline at end of file