Skip to content

Commit

Permalink
Introduce protobuf message testing to policies (#961)
Browse files Browse the repository at this point in the history
* Introduce protobuf message testing to policies
* Remove unnecessary conditional branch
  • Loading branch information
TristonianJones authored Jun 10, 2024
1 parent b66ac6c commit 34c0bfe
Show file tree
Hide file tree
Showing 13 changed files with 215 additions and 65 deletions.
4 changes: 3 additions & 1 deletion common/decls/decls.go
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,9 @@ func (f *FunctionDecl) AddOverload(overload *OverloadDecl) error {
if oID == overload.ID() {
if o.SignatureEquals(overload) && o.IsNonStrict() == overload.IsNonStrict() {
// Allow redefinition of an overload implementation so long as the signatures match.
f.overloads[oID] = overload
if overload.hasBinding() {
f.overloads[oID] = overload
}
return nil
}
return fmt.Errorf("overload redefinition in function. %s: %s has multiple definitions", f.Name(), oID)
Expand Down
21 changes: 21 additions & 0 deletions common/decls/decls_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -413,6 +413,27 @@ func TestFunctionAddDuplicateOverloads(t *testing.T) {
}
}

func TestFunctionAddDuplicateOverloadsPreservesBinding(t *testing.T) {
f, err := NewFunction("max",
Overload("max_int", []*types.Type{types.IntType}, types.IntType),
Overload("max_int", []*types.Type{types.IntType}, types.IntType,
UnaryBinding(func(v ref.Val) ref.Val {
return v
})),
Overload("max_int", []*types.Type{types.IntType}, types.IntType),
)
if err != nil {
t.Fatalf("NewFunction() with duplicate overload signature failed: %v", err)
}
if len(f.overloads) != 1 {
t.Fatal("Duplicate overloads were not merged")
}
o := f.overloads["max_int"]
if o.unaryOp == nil {
t.Error("Duplicate overloads purged the overload binding")
}
}

func TestFunctionAddCollidingOverloads(t *testing.T) {
_, err := NewFunction("max",
Overload("max_int", []*types.Type{types.IntType}, types.IntType),
Expand Down
50 changes: 32 additions & 18 deletions policy/compiler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (

"github.com/google/cel-go/cel"
"github.com/google/cel-go/common/types"
"github.com/google/cel-go/common/types/ref"
)

func TestCompile(t *testing.T) {
Expand Down Expand Up @@ -89,6 +90,11 @@ func compile(t testing.TB, name string, parseOpts []ParserOption, envOpts []cel.
if err != nil {
t.Fatalf("cel.NewEnv() failed: %v", err)
}
// Configure any custom environment options.
env, err = env.Extend(envOpts...)
if err != nil {
t.Fatalf("env.Extend() with env options %v, failed: %v", config, err)
}
// Configure declarations
configOpts, err := config.AsEnvOptions(env)
if err != nil {
Expand All @@ -98,11 +104,6 @@ func compile(t testing.TB, name string, parseOpts []ParserOption, envOpts []cel.
if err != nil {
t.Fatalf("env.Extend() with config options %v, failed: %v", config, err)
}
// Configure any implementations
env, err = env.Extend(envOpts...)
if err != nil {
t.Fatalf("env.Extend() with config options %v, failed: %v", config, err)
}
ast, iss := Compile(env, policy)
return env, ast, iss
}
Expand Down Expand Up @@ -134,22 +135,19 @@ func (r *runner) run(t *testing.T) {
for _, tst := range s.Tests {
tc := tst
t.Run(fmt.Sprintf("%s/%s/%s", r.name, section, tc.Name), func(t *testing.T) {
out, _, err := r.prg.Eval(tc.Input)
if err != nil {
t.Fatalf("prg.Eval(tc.Input) failed: %v", err)
}
wantExpr, iss := r.env.Compile(tc.Output)
if iss.Err() != nil {
t.Fatalf("env.Compile(%q) failed :%v", tc.Output, iss.Err())
}
testPrg, err := r.env.Program(wantExpr)
if err != nil {
t.Fatalf("env.Program(wantExpr) failed: %v", err)
input := map[string]any{}
for k, v := range tc.Input {
if v.Expr == "" {
input[k] = v.Value
continue
}
input[k] = r.eval(t, v.Expr)
}
testOut, _, err := testPrg.Eval(cel.NoVars())
out, _, err := r.prg.Eval(input)
if err != nil {
t.Fatalf("testPrg.Eval() failed: %v", err)
t.Fatalf("prg.Eval(input) failed: %v", err)
}
testOut := r.eval(t, tc.Output)
if optOut, ok := out.(*types.Optional); ok {
if optOut.Equal(types.OptionalNone) == types.True {
if testOut.Equal(types.OptionalNone) != types.True {
Expand Down Expand Up @@ -182,6 +180,22 @@ func (r *runner) bench(b *testing.B) {
}
}

func (r *runner) eval(t testing.TB, expr string) ref.Val {
wantExpr, iss := r.env.Compile(expr)
if iss.Err() != nil {
t.Fatalf("env.Compile(%q) failed :%v", expr, iss.Err())
}
prg, err := r.env.Program(wantExpr)
if err != nil {
t.Fatalf("env.Program(wantExpr) failed: %v", err)
}
out, _, err := prg.Eval(cel.NoVars())
if err != nil {
t.Fatalf("prg.Eval() failed: %v", err)
}
return out
}

func normalize(s string) string {
return strings.ReplaceAll(
strings.ReplaceAll(
Expand Down
3 changes: 2 additions & 1 deletion policy/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,8 @@ func (td *TypeDecl) AsCELType(baseEnv *cel.Env) (*cel.Type, error) {
return cel.TypeParamType(td.TypeName), nil
}
if msgType, found := baseEnv.CELTypeProvider().FindStructType(td.TypeName); found {
return msgType, nil
// First parameter is the type name.
return msgType.Parameters()[0], nil
}
t, found := baseEnv.CELTypeProvider().FindIdent(td.TypeName)
if !found {
Expand Down
11 changes: 8 additions & 3 deletions policy/conformance.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,12 @@ type TestSection struct {
// Note, when a test requires additional functions to be provided to execute, the test harness
// must supply these functions.
type TestCase struct {
Name string `yaml:"name"`
Input map[string]any `yaml:"input"`
Output string `yaml:"output"`
Name string `yaml:"name"`
Input map[string]TestInput `yaml:"input"`
Output string `yaml:"output"`
}

type TestInput struct {
Value any `yaml:"value"`
Expr string `yaml:"expr"`
}
10 changes: 10 additions & 0 deletions policy/helper_test.go
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/types"
"github.com/google/cel-go/common/types/ref"
"github.com/google/cel-go/test/proto3pb"

"gopkg.in/yaml.v3"
)
Expand Down Expand Up @@ -58,6 +59,15 @@ var (
optional.of((resource.origin in variables.permitted_regions)
? {"banned": false} : {"banned": true})))`,
},
{
name: "pb",
expr: `(spec.single_int32 > 10)
? optional.of("invalid spec, got single_int32=%d, wanted <= 10".format([spec.single_int32]))
: optional.none()`,
envOpts: []cel.EnvOption{
cel.Types(&proto3pb.TestAllTypes{}),
},
},
{
name: "required_labels",
expr: `
Expand Down
13 changes: 8 additions & 5 deletions policy/testdata/k8s/tests.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,14 @@ section:
tests:
- name: "restricted_container"
input:
resource.namespace: "dev.cel"
resource.namespace:
value: "dev.cel"
resource.labels:
environment: "staging"
value:
environment: "staging"
resource.containers:
- staging.dev.cel.container1
- staging.dev.cel.container2
- preprod.dev.cel.container3
value:
- staging.dev.cel.container1
- staging.dev.cel.container2
- preprod.dev.cel.container3
output: "'only staging containers are allowed in namespace dev.cel'"
9 changes: 6 additions & 3 deletions policy/testdata/nested_rule/tests.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -19,17 +19,20 @@ section:
- name: "restricted_origin"
input:
resource:
origin: "ir"
value:
origin: "ir"
output: "{'banned': true}"
- name: "by_default"
input:
resource:
origin: "de"
value:
origin: "de"
output: "{'banned': true}"
- name: "permitted"
tests:
- name: "valid_origin"
input:
resource:
origin: "uk"
value:
origin: "uk"
output: "{'banned': false}"
23 changes: 23 additions & 0 deletions policy/testdata/pb/config.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
# Copyright 2024 Google LLC
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# https://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

name: "pb"
container: "google.expr.proto3.test"
extensions:
- name: "strings"
version: 2
variables:
- name: "spec"
type:
type_name: "google.expr.proto3.test.TestAllTypes"
20 changes: 20 additions & 0 deletions policy/testdata/pb/policy.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
# Copyright 2024 Google LLC
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# https://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

name: "pb"
rule:
match:
- condition: spec.single_int32 > 10
output: |
"invalid spec, got single_int32=%d, wanted <= 10".format([spec.single_int32])
34 changes: 34 additions & 0 deletions policy/testdata/pb/tests.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
# Copyright 2024 Google LLC
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# https://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

description: "Protobuf input tests"
section:
- name: "valid"
tests:
- name: "good spec"
input:
spec:
expr: >
TestAllTypes{single_int32: 10}
output: "optional.none()"
- name: "invalid"
tests:
- name: "bad spec"
input:
spec:
expr: >
TestAllTypes{single_int32: 11}
output: >
"invalid spec, got single_int32=11, wanted <= 10"
60 changes: 34 additions & 26 deletions policy/testdata/required_labels/tests.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -19,53 +19,61 @@ section:
- name: "matching"
input:
spec:
labels:
env: prod
experiment: "group b"
value:
labels:
env: prod
experiment: "group b"
resource:
labels:
env: prod
experiment: "group b"
release: "v0.1.0"
value:
labels:
env: prod
experiment: "group b"
release: "v0.1.0"
output: "optional.none()"
- name: "missing"
tests:
- name: "env"
input:
spec:
labels:
env: prod
experiment: "group b"
value:
labels:
env: prod
experiment: "group b"
resource:
labels:
experiment: "group b"
release: "v0.1.0"
value:
labels:
experiment: "group b"
release: "v0.1.0"
output: >
"missing one or more required labels: [\"env\"]"
- name: "experiment"
input:
spec:
labels:
env: prod
experiment: "group b"
value:
labels:
env: prod
experiment: "group b"
resource:
labels:
env: staging
release: "v0.1.0"
value:
labels:
env: staging
release: "v0.1.0"
output: >
"missing one or more required labels: [\"experiment\"]"
- name: "invalid"
tests:
- name: "env"
input:
spec:
labels:
env: prod
experiment: "group b"
value:
labels:
env: prod
experiment: "group b"
resource:
labels:
env: staging
experiment: "group b"
release: "v0.1.0"
value:
labels:
env: staging
experiment: "group b"
release: "v0.1.0"
output: >
"invalid values provided on one or more labels: [\"env\"]"
Loading

0 comments on commit 34c0bfe

Please sign in to comment.