From 59d2d86099c954a3f82bb7c424855bd734c6d2ca Mon Sep 17 00:00:00 2001 From: Vu Dinh Date: Wed, 1 Dec 2021 12:18:56 -0500 Subject: [PATCH 1/4] feat(resolver): support generic constraint using CEL Introduce the new type of constraint that uses CEL (Common Expression Language) as an expression language to define the constraint in a generic way. This type of constraint will allow operator authors to specify a dependency on any arbitrary properties in the bundle. This constraint also supports logical operator such as AND and OR in the CEL expression. Signed-off-by: Vu Dinh --- go.mod | 2 + .../registry/resolver/cache/predicates.go | 40 +++++++ .../registry/resolver/constraints/cel.go | 113 ++++++++++++++++++ pkg/controller/registry/resolver/resolver.go | 25 ++++ vendor/modules.txt | 2 + 5 files changed, 182 insertions(+) create mode 100644 pkg/controller/registry/resolver/constraints/cel.go diff --git a/go.mod b/go.mod index cfdb77c548..d15c15b24a 100644 --- a/go.mod +++ b/go.mod @@ -14,6 +14,7 @@ require ( github.com/go-bindata/go-bindata/v3 v3.1.3 github.com/go-logr/logr v0.4.0 github.com/golang/mock v1.5.0 + github.com/google/cel-go v0.9.0 github.com/google/go-cmp v0.5.6 github.com/googleapis/gnostic v0.5.5 github.com/itchyny/gojq v0.11.0 @@ -38,6 +39,7 @@ require ( github.com/stretchr/testify v1.7.0 golang.org/x/net v0.0.0-20210825183410-e898025ed96a golang.org/x/time v0.0.0-20210723032227-1f47c861a9ac + google.golang.org/genproto v0.0.0-20210831024726-fe130286e0e2 google.golang.org/grpc v1.40.0 gopkg.in/yaml.v2 v2.4.0 helm.sh/helm/v3 v3.6.1 diff --git a/pkg/controller/registry/resolver/cache/predicates.go b/pkg/controller/registry/resolver/cache/predicates.go index 25200917e9..0fc6439d73 100644 --- a/pkg/controller/registry/resolver/cache/predicates.go +++ b/pkg/controller/registry/resolver/cache/predicates.go @@ -6,6 +6,8 @@ import ( "fmt" "github.com/blang/semver/v4" + + "github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry/resolver/constraints" opregistry "github.com/operator-framework/operator-registry/pkg/registry" ) @@ -354,3 +356,41 @@ func (c countingPredicate) String() string { func CountingPredicate(p Predicate, n *int) Predicate { return countingPredicate{p: p, n: n} } + +type evaluatorPredicate struct { + evaluator constraints.Evaluator + rule string + message string +} + +func (ep *evaluatorPredicate) Test(entry *Entry) bool { + props := make([]map[string]interface{}, len(entry.Properties)) + for i, p := range entry.Properties { + var v interface{} + if err := json.Unmarshal([]byte(p.Value), &v); err != nil { + continue + } + props[i] = map[string]interface{}{ + "type": p.Type, + "value": v, + } + } + + ok, err := ep.evaluator.Evaluate(map[string]interface{}{"properties": props}) + if err != nil { + return false + } + return ok +} + +func EvaluatorPredicate(provider constraints.EvaluatorProvider, rule, message string) (Predicate, error) { + eval, err := provider.Evaluator(rule) + if err != nil { + return nil, err + } + return &evaluatorPredicate{evaluator: eval, rule: rule, message: message}, nil +} + +func (ep *evaluatorPredicate) String() string { + return fmt.Sprintf("with constraint: %q and message: %q", ep.rule, ep.message) +} diff --git a/pkg/controller/registry/resolver/constraints/cel.go b/pkg/controller/registry/resolver/constraints/cel.go new file mode 100644 index 0000000000..566bb10982 --- /dev/null +++ b/pkg/controller/registry/resolver/constraints/cel.go @@ -0,0 +1,113 @@ +package constraints + +import ( + "fmt" + + "github.com/google/cel-go/cel" + "github.com/google/cel-go/checker/decls" + "github.com/google/cel-go/common/types" + "github.com/google/cel-go/common/types/ref" + "github.com/google/cel-go/interpreter/functions" + exprpb "google.golang.org/genproto/googleapis/api/expr/v1alpha1" + + "github.com/blang/semver/v4" +) + +type Cel struct { + Rule string +} + +type Evaluator interface { + Evaluate(env map[string]interface{}) (bool, error) +} + +type EvaluatorProvider interface { + Evaluator(rule string) (Evaluator, error) +} + +func NewCelEvaluatorProvider() *celEvaluatorProvider { + env, err := cel.NewEnv(cel.Declarations( + decls.NewVar("properties", decls.NewListType(decls.NewMapType(decls.String, decls.Any)))), + cel.Lib(semverLib{}), + ) + if err != nil { + panic(err) + } + return &celEvaluatorProvider{ + env: env, + } +} + +type celEvaluatorProvider struct { + env *cel.Env +} + +type celEvaluator struct { + p cel.Program +} + +type semverLib struct{} + +func (semverLib) CompileOptions() []cel.EnvOption { + return []cel.EnvOption{ + cel.Declarations( + decls.NewFunction("semver_compare", + decls.NewOverload("semver_compare", + []*exprpb.Type{decls.Any, decls.Any}, + decls.Int))), + } +} + +func (semverLib) ProgramOptions() []cel.ProgramOption { + return []cel.ProgramOption{ + cel.Functions( + &functions.Overload{ + Operator: "semver_compare", + Binary: semverCompare, + }, + ), + } +} + +func (e celEvaluator) Evaluate(env map[string]interface{}) (bool, error) { + result, _, err := e.p.Eval(env) + if err != nil { + return false, err + } + + // we should have already ensured that this will be types.Bool during compilation + if b, ok := result.Value().(bool); ok { + return b, nil + } + return false, fmt.Errorf("cel expression evalutated to %T, not bool", result.Value()) +} + +func (e *celEvaluatorProvider) Evaluator(rule string) (Evaluator, error) { + ast, issues := e.env.Compile(rule) + if err := issues.Err(); err != nil { + return nil, err + } + + if ast.ResultType() != decls.Bool { + return nil, fmt.Errorf("cel expressions must have type Bool") + } + + p, err := e.env.Program(ast) + if err != nil { + return nil, err + } + return celEvaluator{p: p}, nil +} + +func semverCompare(val1, val2 ref.Val) ref.Val { + v1, err := semver.ParseTolerant(fmt.Sprint(val1.Value())) + if err != nil { + return types.ValOrErr(val1, "unable to parse '%v' to semver format", val1.Value()) + } + + v2, err := semver.ParseTolerant(fmt.Sprint(val2.Value())) + if err != nil { + return types.ValOrErr(val2, "unable to parse '%v' to semver format", val2.Value()) + } + return types.Int(v1.Compare(v2)) +} diff --git a/pkg/controller/registry/resolver/resolver.go b/pkg/controller/registry/resolver/resolver.go index 6b1d8a6d80..6371938e51 100644 --- a/pkg/controller/registry/resolver/resolver.go +++ b/pkg/controller/registry/resolver/resolver.go @@ -15,6 +15,7 @@ import ( "github.com/operator-framework/api/pkg/operators/v1alpha1" v1alpha1listers "github.com/operator-framework/operator-lifecycle-manager/pkg/api/client/listers/operators/v1alpha1" "github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry/resolver/cache" + "github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry/resolver/constraints" "github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry/resolver/projection" "github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry/resolver/solver" "github.com/operator-framework/operator-registry/pkg/api" @@ -346,6 +347,30 @@ func (r *SatResolver) getBundleInstallables(preferredNamespace string, bundleSta continue } + for _, prop := range bundle.Properties { + if prop.Type != "olm.constraint" { + continue + } + + var val struct { + Message string `json:"message"` + Cel *constraints.Cel `json:"cel"` + } + + if err := json.Unmarshal([]byte(prop.Value), &val); err != nil { + errs = append(errs, err) + continue + } + + pred, err := cache.EvaluatorPredicate(r.evaluatorProvider, val.Cel.Rule, val.Message) + if err != nil { + errs = append(errs, err) + continue + } + + dependencyPredicates = append(dependencyPredicates, pred) + } + for _, d := range dependencyPredicates { sourcePredicate := cache.False() // Build a filter matching all (catalog, diff --git a/vendor/modules.txt b/vendor/modules.txt index b4edece474..ddeadf4d6b 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -290,6 +290,7 @@ github.com/golang/protobuf/ptypes/wrappers # github.com/google/btree v1.0.1 github.com/google/btree # github.com/google/cel-go v0.9.0 +## explicit github.com/google/cel-go/cel github.com/google/cel-go/checker github.com/google/cel-go/checker/decls @@ -856,6 +857,7 @@ google.golang.org/appengine/internal/remote_api google.golang.org/appengine/internal/urlfetch google.golang.org/appengine/urlfetch # google.golang.org/genproto v0.0.0-20210831024726-fe130286e0e2 +## explicit google.golang.org/genproto/googleapis/api/annotations google.golang.org/genproto/googleapis/api/expr/v1alpha1 google.golang.org/genproto/googleapis/api/httpbody From 4e82ce02180fa8e0976818afe659c1d799e9b30e Mon Sep 17 00:00:00 2001 From: Vu Dinh Date: Mon, 6 Dec 2021 14:27:12 -0500 Subject: [PATCH 2/4] Add unit test for generic constraint and add some context on CEL library Add unit test cases for generic constraint using CEL to ensure it works properly in the resolver. Add some context for the custom library for semver comparison in CEL. Signed-off-by: Vu Dinh --- .../registry/resolver/cache/predicates.go | 2 +- .../registry/resolver/constraints/cel.go | 48 ++++--- .../registry/resolver/resolver_test.go | 122 ++++++++++++++++++ .../registry/resolver/source_registry.go | 5 + 4 files changed, 159 insertions(+), 18 deletions(-) diff --git a/pkg/controller/registry/resolver/cache/predicates.go b/pkg/controller/registry/resolver/cache/predicates.go index 0fc6439d73..0b21faab31 100644 --- a/pkg/controller/registry/resolver/cache/predicates.go +++ b/pkg/controller/registry/resolver/cache/predicates.go @@ -383,7 +383,7 @@ func (ep *evaluatorPredicate) Test(entry *Entry) bool { return ok } -func EvaluatorPredicate(provider constraints.EvaluatorProvider, rule, message string) (Predicate, error) { +func EvaluatorPredicate(provider constraints.EvaluatorProvider, rule string, message string) (Predicate, error) { eval, err := provider.Evaluator(rule) if err != nil { return nil, err diff --git a/pkg/controller/registry/resolver/constraints/cel.go b/pkg/controller/registry/resolver/constraints/cel.go index 566bb10982..1333233de3 100644 --- a/pkg/controller/registry/resolver/constraints/cel.go +++ b/pkg/controller/registry/resolver/constraints/cel.go @@ -43,9 +43,23 @@ type celEvaluatorProvider struct { } type celEvaluator struct { - p cel.Program + program cel.Program } +/* +This section of code is for custom library for semver comparison in CEL +The code is inspired by https://github.com/google/cel-go/blob/master/cel/cel_test.go#L46 + +The semver_compare is wrriten based on `Compare` function in https://github.com/blang/semver +particularly in https://github.com/blang/semver/blob/master/semver.go#L125 + +Example: +`semver_compare(v1, v2)` is equivalent of `v1.Compare(v2)` in blang/semver library + +The result is `semver_compare` is an integer just like `Compare`. So, the CEL +expression `semver_compare(v1, v2) == 0` is equivalent v1.Compare(v2) == 0. In +the other words, it checks if v1 is equal to v2 in term of semver comparision. +*/ type semverLib struct{} func (semverLib) CompileOptions() []cel.EnvOption { @@ -69,8 +83,21 @@ func (semverLib) ProgramOptions() []cel.ProgramOption { } } +func semverCompare(val1, val2 ref.Val) ref.Val { + v1, err := semver.ParseTolerant(fmt.Sprint(val1.Value())) + if err != nil { + return types.ValOrErr(val1, "unable to parse '%v' to semver format", val1.Value()) + } + + v2, err := semver.ParseTolerant(fmt.Sprint(val2.Value())) + if err != nil { + return types.ValOrErr(val2, "unable to parse '%v' to semver format", val2.Value()) + } + return types.Int(v1.Compare(v2)) +} + func (e celEvaluator) Evaluate(env map[string]interface{}) (bool, error) { - result, _, err := e.p.Eval(env) + result, _, err := e.program.Eval(env) if err != nil { return false, err } @@ -92,22 +119,9 @@ func (e *celEvaluatorProvider) Evaluator(rule string) (Evaluator, error) { return nil, fmt.Errorf("cel expressions must have type Bool") } - p, err := e.env.Program(ast) + prog, err := e.env.Program(ast) if err != nil { return nil, err } - return celEvaluator{p: p}, nil -} - -func semverCompare(val1, val2 ref.Val) ref.Val { - v1, err := semver.ParseTolerant(fmt.Sprint(val1.Value())) - if err != nil { - return types.ValOrErr(val1, "unable to parse '%v' to semver format", val1.Value()) - } - - v2, err := semver.ParseTolerant(fmt.Sprint(val2.Value())) - if err != nil { - return types.ValOrErr(val2, "unable to parse '%v' to semver format", val2.Value()) - } - return types.Int(v1.Compare(v2)) + return celEvaluator{program: prog}, nil } diff --git a/pkg/controller/registry/resolver/resolver_test.go b/pkg/controller/registry/resolver/resolver_test.go index 90d538b99e..60995fbd2d 100644 --- a/pkg/controller/registry/resolver/resolver_test.go +++ b/pkg/controller/registry/resolver/resolver_test.go @@ -21,6 +21,7 @@ import ( "github.com/operator-framework/api/pkg/operators/v1alpha1" listersv1alpha1 "github.com/operator-framework/operator-lifecycle-manager/pkg/api/client/listers/operators/v1alpha1" "github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry/resolver/cache" + "github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry/resolver/constraints" "github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry/resolver/solver" "github.com/operator-framework/operator-registry/pkg/api" opregistry "github.com/operator-framework/operator-registry/pkg/registry" @@ -2380,3 +2381,124 @@ func TestNewOperatorFromCSV(t *testing.T) { }) } } + +func TestSolveOperators_GenericConstraint(t *testing.T) { + Provides1 := cache.APISet{opregistry.APIKey{"g", "v", "k", "ks"}: struct{}{}} + namespace := "olm" + catalog := cache.SourceKey{Name: "community", Namespace: namespace} + + deps1 := []*api.Dependency{ + { + Type: "olm.constraint", + Value: `{"message":"gvk-constraint", + "cel":{"rule":"properties.exists(p, p.type == 'olm.gvk' && p.value == {'group': 'g', 'version': 'v', 'kind': 'k'})"}}`, + }, + } + deps2 := []*api.Dependency{ + { + Type: "olm.constraint", + Value: `{"message":"gvk2-constraint", + "cel":{"rule":"properties.exists(p, p.type == 'olm.gvk' && p.value == {'group': 'g2', 'version': 'v', 'kind': 'k'})"}}`, + }, + } + deps3 := []*api.Dependency{ + { + Type: "olm.constraint", + Value: `{"message":"package-constraint", + "cel":{"rule":"properties.exists(p, p.type == 'olm.package' && p.value.packageName == 'packageB' && (semver_compare(p.value.version, '1.0.1') == 0))"}}`, + }, + } + + tests := []struct { + name string + isErr bool + subs []*v1alpha1.Subscription + catalog cache.Source + expected cache.OperatorSet + message string + }{ + { + // generic constraint for satisfiable gvk dependency + name: "Generic Constraint/Satisfiable GVK Dependency", + isErr: false, + subs: []*v1alpha1.Subscription{ + newSub(namespace, "packageA", "stable", catalog), + }, + catalog: &cache.Snapshot{ + Entries: []*cache.Entry{ + genOperator("opA.v1.0.0", "1.0.0", "", "packageA", "stable", catalog.Name, catalog.Namespace, nil, nil, deps1, "", false), + genOperator("opB.v1.0.0", "1.0.0", "", "packageB", "stable", catalog.Name, catalog.Namespace, nil, Provides1, nil, "stable", false), + }, + }, + expected: cache.OperatorSet{ + "opA.v1.0.0": genOperator("opA.v1.0.0", "1.0.0", "", "packageA", "stable", catalog.Name, catalog.Namespace, nil, nil, deps1, "", false), + "opB.v1.0.0": genOperator("opB.v1.0.0", "1.0.0", "", "packageB", "stable", catalog.Name, catalog.Namespace, nil, Provides1, nil, "stable", false), + }, + }, + { + // generic constraint for NotSatisfiable gvk dependency + name: "Generic Constraint/NotSatisfiable GVK Dependency", + isErr: true, + subs: []*v1alpha1.Subscription{ + newSub(namespace, "packageA", "stable", catalog), + }, + catalog: &cache.Snapshot{ + Entries: []*cache.Entry{ + genOperator("opA.v1.0.0", "1.0.0", "", "packageA", "stable", catalog.Name, catalog.Namespace, nil, nil, deps2, "", false), + genOperator("opB.v1.0.0", "1.0.0", "", "packageB", "stable", catalog.Name, catalog.Namespace, nil, Provides1, nil, "", false), + }, + }, + // unable to find satisfiable gvk dependency + // resolve into nothing + expected: cache.OperatorSet{}, + message: "gvk2-constraint", + }, + { + // generic constraint for package constraint + name: "Generic Constraint/Satisfiable Package Dependency", + isErr: false, + subs: []*v1alpha1.Subscription{ + newSub(namespace, "packageA", "stable", catalog), + }, + catalog: &cache.Snapshot{ + Entries: []*cache.Entry{ + genOperator("opA.v1.0.0", "1.0.0", "", "packageA", "stable", catalog.Name, catalog.Namespace, nil, nil, deps3, "", false), + genOperator("opB.v1.0.0", "1.0.0", "", "packageB", "stable", catalog.Name, catalog.Namespace, nil, nil, nil, "", false), + genOperator("opB.v1.0.1", "1.0.1", "opB.v1.0.0", "packageB", "stable", catalog.Name, catalog.Namespace, nil, nil, nil, "stable", false), + genOperator("opB.v1.0.2", "1.0.2", "opB.v1.0.1", "packageB", "stable", catalog.Name, catalog.Namespace, nil, nil, nil, "stable", false), + }, + }, + expected: cache.OperatorSet{ + "opA.v1.0.0": genOperator("opA.v1.0.1", "1.0.1", "", "packageA", "stable", catalog.Name, catalog.Namespace, nil, nil, deps3, "", false), + "opB.v1.0.1": genOperator("opB.v1.0.1", "1.0.1", "opB.v1.0.0", "packageB", "stable", catalog.Name, catalog.Namespace, nil, nil, nil, "stable", false), + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var err error + var operators cache.OperatorSet + satResolver := SatResolver{ + cache: cache.New(cache.StaticSourceProvider{ + catalog: tt.catalog, + }), + log: logrus.New(), + evaluatorProvider: constraints.NewCelEvaluatorProvider(), + } + + operators, err = satResolver.SolveOperators([]string{namespace}, nil, tt.subs) + if tt.isErr { + assert.Error(t, err) + assert.Contains(t, err.Error(), tt.message) + } else { + assert.NoError(t, err) + for k := range tt.expected { + require.NotNil(t, operators[k]) + assert.EqualValues(t, k, operators[k].Name) + } + } + assert.Equal(t, len(tt.expected), len(operators)) + }) + } +} diff --git a/pkg/controller/registry/resolver/source_registry.go b/pkg/controller/registry/resolver/source_registry.go index ebf18a1c1a..143e5a1956 100644 --- a/pkg/controller/registry/resolver/source_registry.go +++ b/pkg/controller/registry/resolver/source_registry.go @@ -233,6 +233,11 @@ func legacyDependenciesToProperties(dependencies []*api.Dependency) ([]*api.Prop Type: "olm.label.required", Value: dependency.Value, }) + case "olm.constraint": + result = append(result, &api.Property{ + Type: "olm.constraint", + Value: dependency.Value, + }) } } return result, nil From eb8cfb75bc9f72f7664f13f69c55cf30dd430ca9 Mon Sep 17 00:00:00 2001 From: Vu Dinh Date: Wed, 8 Dec 2021 21:41:28 -0500 Subject: [PATCH 3/4] Use constraints library from api with simplified CEL library Signed-off-by: Vu Dinh --- go.mod | 2 - .../registry/resolver/cache/predicates.go | 24 ++-- .../registry/resolver/constraints/cel.go | 127 ------------------ pkg/controller/registry/resolver/resolver.go | 9 +- .../registry/resolver/resolver_test.go | 5 +- vendor/modules.txt | 2 - 6 files changed, 16 insertions(+), 153 deletions(-) delete mode 100644 pkg/controller/registry/resolver/constraints/cel.go diff --git a/go.mod b/go.mod index d15c15b24a..cfdb77c548 100644 --- a/go.mod +++ b/go.mod @@ -14,7 +14,6 @@ require ( github.com/go-bindata/go-bindata/v3 v3.1.3 github.com/go-logr/logr v0.4.0 github.com/golang/mock v1.5.0 - github.com/google/cel-go v0.9.0 github.com/google/go-cmp v0.5.6 github.com/googleapis/gnostic v0.5.5 github.com/itchyny/gojq v0.11.0 @@ -39,7 +38,6 @@ require ( github.com/stretchr/testify v1.7.0 golang.org/x/net v0.0.0-20210825183410-e898025ed96a golang.org/x/time v0.0.0-20210723032227-1f47c861a9ac - google.golang.org/genproto v0.0.0-20210831024726-fe130286e0e2 google.golang.org/grpc v1.40.0 gopkg.in/yaml.v2 v2.4.0 helm.sh/helm/v3 v3.6.1 diff --git a/pkg/controller/registry/resolver/cache/predicates.go b/pkg/controller/registry/resolver/cache/predicates.go index 0b21faab31..2ce6231529 100644 --- a/pkg/controller/registry/resolver/cache/predicates.go +++ b/pkg/controller/registry/resolver/cache/predicates.go @@ -7,7 +7,7 @@ import ( "github.com/blang/semver/v4" - "github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry/resolver/constraints" + "github.com/operator-framework/api/pkg/constraints" opregistry "github.com/operator-framework/operator-registry/pkg/registry" ) @@ -357,13 +357,13 @@ func CountingPredicate(p Predicate, n *int) Predicate { return countingPredicate{p: p, n: n} } -type evaluatorPredicate struct { - evaluator constraints.Evaluator - rule string - message string +type celPredicate struct { + program constraints.CelProgram + rule string + message string } -func (ep *evaluatorPredicate) Test(entry *Entry) bool { +func (cp *celPredicate) Test(entry *Entry) bool { props := make([]map[string]interface{}, len(entry.Properties)) for i, p := range entry.Properties { var v interface{} @@ -376,21 +376,21 @@ func (ep *evaluatorPredicate) Test(entry *Entry) bool { } } - ok, err := ep.evaluator.Evaluate(map[string]interface{}{"properties": props}) + ok, err := cp.program.Evaluate(map[string]interface{}{"properties": props}) if err != nil { return false } return ok } -func EvaluatorPredicate(provider constraints.EvaluatorProvider, rule string, message string) (Predicate, error) { - eval, err := provider.Evaluator(rule) +func CreateCelPredicate(env *constraints.CelEnvironment, rule string, message string) (Predicate, error) { + prog, err := env.Validate(rule) if err != nil { return nil, err } - return &evaluatorPredicate{evaluator: eval, rule: rule, message: message}, nil + return &celPredicate{program: prog, rule: rule, message: message}, nil } -func (ep *evaluatorPredicate) String() string { - return fmt.Sprintf("with constraint: %q and message: %q", ep.rule, ep.message) +func (cp *celPredicate) String() string { + return fmt.Sprintf("with constraint: %q and message: %q", cp.rule, cp.message) } diff --git a/pkg/controller/registry/resolver/constraints/cel.go b/pkg/controller/registry/resolver/constraints/cel.go deleted file mode 100644 index 1333233de3..0000000000 --- a/pkg/controller/registry/resolver/constraints/cel.go +++ /dev/null @@ -1,127 +0,0 @@ -package constraints - -import ( - "fmt" - - "github.com/google/cel-go/cel" - "github.com/google/cel-go/checker/decls" - "github.com/google/cel-go/common/types" - "github.com/google/cel-go/common/types/ref" - "github.com/google/cel-go/interpreter/functions" - exprpb "google.golang.org/genproto/googleapis/api/expr/v1alpha1" - - "github.com/blang/semver/v4" -) - -type Cel struct { - Rule string -} - -type Evaluator interface { - Evaluate(env map[string]interface{}) (bool, error) -} - -type EvaluatorProvider interface { - Evaluator(rule string) (Evaluator, error) -} - -func NewCelEvaluatorProvider() *celEvaluatorProvider { - env, err := cel.NewEnv(cel.Declarations( - decls.NewVar("properties", decls.NewListType(decls.NewMapType(decls.String, decls.Any)))), - cel.Lib(semverLib{}), - ) - if err != nil { - panic(err) - } - return &celEvaluatorProvider{ - env: env, - } -} - -type celEvaluatorProvider struct { - env *cel.Env -} - -type celEvaluator struct { - program cel.Program -} - -/* -This section of code is for custom library for semver comparison in CEL -The code is inspired by https://github.com/google/cel-go/blob/master/cel/cel_test.go#L46 - -The semver_compare is wrriten based on `Compare` function in https://github.com/blang/semver -particularly in https://github.com/blang/semver/blob/master/semver.go#L125 - -Example: -`semver_compare(v1, v2)` is equivalent of `v1.Compare(v2)` in blang/semver library - -The result is `semver_compare` is an integer just like `Compare`. So, the CEL -expression `semver_compare(v1, v2) == 0` is equivalent v1.Compare(v2) == 0. In -the other words, it checks if v1 is equal to v2 in term of semver comparision. -*/ -type semverLib struct{} - -func (semverLib) CompileOptions() []cel.EnvOption { - return []cel.EnvOption{ - cel.Declarations( - decls.NewFunction("semver_compare", - decls.NewOverload("semver_compare", - []*exprpb.Type{decls.Any, decls.Any}, - decls.Int))), - } -} - -func (semverLib) ProgramOptions() []cel.ProgramOption { - return []cel.ProgramOption{ - cel.Functions( - &functions.Overload{ - Operator: "semver_compare", - Binary: semverCompare, - }, - ), - } -} - -func semverCompare(val1, val2 ref.Val) ref.Val { - v1, err := semver.ParseTolerant(fmt.Sprint(val1.Value())) - if err != nil { - return types.ValOrErr(val1, "unable to parse '%v' to semver format", val1.Value()) - } - - v2, err := semver.ParseTolerant(fmt.Sprint(val2.Value())) - if err != nil { - return types.ValOrErr(val2, "unable to parse '%v' to semver format", val2.Value()) - } - return types.Int(v1.Compare(v2)) -} - -func (e celEvaluator) Evaluate(env map[string]interface{}) (bool, error) { - result, _, err := e.program.Eval(env) - if err != nil { - return false, err - } - - // we should have already ensured that this will be types.Bool during compilation - if b, ok := result.Value().(bool); ok { - return b, nil - } - return false, fmt.Errorf("cel expression evalutated to %T, not bool", result.Value()) -} - -func (e *celEvaluatorProvider) Evaluator(rule string) (Evaluator, error) { - ast, issues := e.env.Compile(rule) - if err := issues.Err(); err != nil { - return nil, err - } - - if ast.ResultType() != decls.Bool { - return nil, fmt.Errorf("cel expressions must have type Bool") - } - - prog, err := e.env.Program(ast) - if err != nil { - return nil, err - } - return celEvaluator{program: prog}, nil -} diff --git a/pkg/controller/registry/resolver/resolver.go b/pkg/controller/registry/resolver/resolver.go index 6371938e51..8bfb7267cd 100644 --- a/pkg/controller/registry/resolver/resolver.go +++ b/pkg/controller/registry/resolver/resolver.go @@ -15,7 +15,6 @@ import ( "github.com/operator-framework/api/pkg/operators/v1alpha1" v1alpha1listers "github.com/operator-framework/operator-lifecycle-manager/pkg/api/client/listers/operators/v1alpha1" "github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry/resolver/cache" - "github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry/resolver/constraints" "github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry/resolver/projection" "github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry/resolver/solver" "github.com/operator-framework/operator-registry/pkg/api" @@ -352,17 +351,13 @@ func (r *SatResolver) getBundleInstallables(preferredNamespace string, bundleSta continue } - var val struct { - Message string `json:"message"` - Cel *constraints.Cel `json:"cel"` - } - + var val constraints.Constraint if err := json.Unmarshal([]byte(prop.Value), &val); err != nil { errs = append(errs, err) continue } - pred, err := cache.EvaluatorPredicate(r.evaluatorProvider, val.Cel.Rule, val.Message) + pred, err := cache.CreateCelPredicate(r.celEnv, val.Cel.Rule, val.Message) if err != nil { errs = append(errs, err) continue diff --git a/pkg/controller/registry/resolver/resolver_test.go b/pkg/controller/registry/resolver/resolver_test.go index 60995fbd2d..5b1295efca 100644 --- a/pkg/controller/registry/resolver/resolver_test.go +++ b/pkg/controller/registry/resolver/resolver_test.go @@ -21,7 +21,6 @@ import ( "github.com/operator-framework/api/pkg/operators/v1alpha1" listersv1alpha1 "github.com/operator-framework/operator-lifecycle-manager/pkg/api/client/listers/operators/v1alpha1" "github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry/resolver/cache" - "github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry/resolver/constraints" "github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry/resolver/solver" "github.com/operator-framework/operator-registry/pkg/api" opregistry "github.com/operator-framework/operator-registry/pkg/registry" @@ -2483,8 +2482,8 @@ func TestSolveOperators_GenericConstraint(t *testing.T) { cache: cache.New(cache.StaticSourceProvider{ catalog: tt.catalog, }), - log: logrus.New(), - evaluatorProvider: constraints.NewCelEvaluatorProvider(), + log: logrus.New(), + celEnv: constraints.NewCelEnvironment(), } operators, err = satResolver.SolveOperators([]string{namespace}, nil, tt.subs) diff --git a/vendor/modules.txt b/vendor/modules.txt index ddeadf4d6b..b4edece474 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -290,7 +290,6 @@ github.com/golang/protobuf/ptypes/wrappers # github.com/google/btree v1.0.1 github.com/google/btree # github.com/google/cel-go v0.9.0 -## explicit github.com/google/cel-go/cel github.com/google/cel-go/checker github.com/google/cel-go/checker/decls @@ -857,7 +856,6 @@ google.golang.org/appengine/internal/remote_api google.golang.org/appengine/internal/urlfetch google.golang.org/appengine/urlfetch # google.golang.org/genproto v0.0.0-20210831024726-fe130286e0e2 -## explicit google.golang.org/genproto/googleapis/api/annotations google.golang.org/genproto/googleapis/api/expr/v1alpha1 google.golang.org/genproto/googleapis/api/httpbody From 2b5313bb6591e665b62681c5d1e1ba319ee5a1e7 Mon Sep 17 00:00:00 2001 From: Vu Dinh Date: Tue, 14 Dec 2021 10:29:31 -0500 Subject: [PATCH 4/4] Rebase against master to use property converter interface Signed-off-by: Vu Dinh --- pkg/controller/registry/resolver/resolver.go | 30 +++++-------------- .../registry/resolver/resolver_test.go | 6 ++-- 2 files changed, 12 insertions(+), 24 deletions(-) diff --git a/pkg/controller/registry/resolver/resolver.go b/pkg/controller/registry/resolver/resolver.go index 8bfb7267cd..f1b5d42a0f 100644 --- a/pkg/controller/registry/resolver/resolver.go +++ b/pkg/controller/registry/resolver/resolver.go @@ -35,7 +35,9 @@ func NewDefaultSatResolver(rcp cache.SourceProvider, catsrcLister v1alpha1lister return &SatResolver{ cache: cache.New(rcp, cache.WithLogger(logger), cache.WithCatalogSourceLister(catsrcLister)), log: logger, - pc: &predicateConverter{}, + pc: &predicateConverter{ + celEnv: constraints.NewCelEnvironment(), + }, } } @@ -346,26 +348,6 @@ func (r *SatResolver) getBundleInstallables(preferredNamespace string, bundleSta continue } - for _, prop := range bundle.Properties { - if prop.Type != "olm.constraint" { - continue - } - - var val constraints.Constraint - if err := json.Unmarshal([]byte(prop.Value), &val); err != nil { - errs = append(errs, err) - continue - } - - pred, err := cache.CreateCelPredicate(r.celEnv, val.Cel.Rule, val.Message) - if err != nil { - errs = append(errs, err) - continue - } - - dependencyPredicates = append(dependencyPredicates, pred) - } - for _, d := range dependencyPredicates { sourcePredicate := cache.False() // Build a filter matching all (catalog, @@ -757,7 +739,9 @@ func sortChannel(bundles []*cache.Entry) ([]*cache.Entry, error) { } // predicateConverter configures olm.constraint value -> predicate conversion for the resolver. -type predicateConverter struct{} +type predicateConverter struct { + celEnv *constraints.CelEnvironment +} // convertDependencyProperties converts all known constraint properties to predicates. func (pc *predicateConverter) convertDependencyProperties(properties []*api.Property) ([]cache.Predicate, error) { @@ -834,6 +818,8 @@ func (pc *predicateConverter) convertConstraints(constraints ...constraints.Cons case constraint.None != nil: subs, perr := pc.convertConstraints(constraint.None.Constraints...) preds[i], err = cache.Not(subs...), perr + case constraint.Cel != nil: + preds[i], err = cache.CreateCelPredicate(pc.celEnv, constraint.Cel.Rule, constraint.Message) default: // Unknown constraint types are handled by constraints.Parse(), // but parsed constraints may be empty. diff --git a/pkg/controller/registry/resolver/resolver_test.go b/pkg/controller/registry/resolver/resolver_test.go index 5b1295efca..776f88e6df 100644 --- a/pkg/controller/registry/resolver/resolver_test.go +++ b/pkg/controller/registry/resolver/resolver_test.go @@ -2482,8 +2482,10 @@ func TestSolveOperators_GenericConstraint(t *testing.T) { cache: cache.New(cache.StaticSourceProvider{ catalog: tt.catalog, }), - log: logrus.New(), - celEnv: constraints.NewCelEnvironment(), + log: logrus.New(), + pc: &predicateConverter{ + celEnv: constraints.NewCelEnvironment(), + }, } operators, err = satResolver.SolveOperators([]string{namespace}, nil, tt.subs)