Skip to content

Commit

Permalink
Support for typename import aliases (#993)
Browse files Browse the repository at this point in the history 
* Support for typename import aliases
  • Loading branch information
@TristonianJones
TristonianJones authored Aug 10, 2024
1 parent 3545aac commit 851a4b8
Show file tree
Hide file tree
Showing 12 changed files with 327 additions and 83 deletions.
12 changes: 12 additions & 0 deletions common/containers/container.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ package containers
import (
"fmt"
"strings"
"unicode"

"github.com/google/cel-go/common/ast"
)
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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) {
Expand Down
122 changes: 74 additions & 48 deletions common/containers/container_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package containers

import (
"fmt"
"reflect"
"testing"

Expand Down Expand Up @@ -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)
}
})
}
}

Expand Down
1 change: 1 addition & 0 deletions policy/BUILD.bazel
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
41 changes: 34 additions & 7 deletions policy/compiler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -211,17 +212,42 @@ 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, 0, importCount)
for _, imp := range p.Imports() {
typeName := imp.Name().Value
_, 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 {
// 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
}
}
return c.compileRule(p.Rule(), c.env, iss)
}

type compiler struct {
Expand All @@ -234,7 +260,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())
Expand All @@ -254,9 +279,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,
Expand Down
14 changes: 14 additions & 0 deletions policy/compiler_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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...)
Expand Down
55 changes: 34 additions & 21 deletions policy/helper_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,33 +63,37 @@ 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()))`,
},
{
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{}),
},
Expand Down Expand Up @@ -168,25 +172,34 @@ var (
}{
{
name: "errors",
err: `ERROR: testdata/errors/policy.yaml:19:19: undeclared reference to 'spec' (in container '')
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:24: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:25:7: invalid variable declaration: overlapping identifier for name 'variables.want'
| - name: want
| ......^
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:21:66: Syntax error: extraneous input ')' expecting <EOF>
ERROR: testdata/errors/policy.yaml:28:66: Syntax error: extraneous input ')' expecting <EOF>
| 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:30: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:38: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: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'"
| ...............^`,
},
Expand Down
Loading

0 comments on commit 851a4b8

Please sign in to comment.