Simplify the CEL library code structure

Remind the original cel package to constraints pkg to reflect that
there will be more type of constraints.

Remove the evaluator/provider interfaces that are not needed yet.
The generic interface can be designed later when a new type of
constraint is introduced.

Signed-off-by: Vu Dinh <[email protected]>

Author: dinhxuanvu

pkg/cel/cel.go renamed to pkg/constraints/cel.go

@@ -1,4 +1,4 @@
-package cel
+package constraints
 
 import (
 	"fmt"
@@ -16,6 +16,7 @@ import (
 // PropertiesKey is the key for bundle properties map (input data for CEL evaluation)
 const PropertiesKey = "properties"
 
+// Constraint is a struct representing the new generic constraint type
 type Constraint struct {
 	// Constraint message that surfaces in resolution
 	// This field is optional
@@ -26,37 +27,38 @@ type Constraint struct {
 	Cel *Cel `json:"cel" yaml:"cel"`
 }
 
+// Cel is a struct representing CEL expression information
 type Cel struct {
 	// The CEL expression
 	Rule string `json:"rule" yaml:"rule"`
 }
 
-type Evaluator interface {
-	Evaluate(env map[string]interface{}) (bool, error)
-}
-
-type EvaluatorProvider interface {
-	Evaluator(rule string) (Evaluator, error)
-}
-
-func NewCelEvaluatorProvider() *celEvaluatorProvider {
+// NewCelEnvironment returns a CEL environment which can be used to
+// evaluate CEL expression and an error if occurs
+func NewCelEnvironment() *CelEnvironment {
 	env, err := cel.NewEnv(cel.Declarations(
 		decls.NewVar(PropertiesKey, decls.NewListType(decls.NewMapType(decls.String, decls.Any)))),
 		cel.Lib(semverLib{}),
 	)
+	// If an error occurs here, it means the CEL enviroment is unable to load
+	// configuration for custom libraries propertly. Hence, the CEL enviroment is
+	// unusable. Panic here will cause the program to fail immediately to prevent
+	// cascading failures later on when this CEL env is in use.
 	if err != nil {
 		panic(err)
 	}
-	return &celEvaluatorProvider{
+	return &CelEnvironment{
 		env: env,
 	}
 }
 
-type celEvaluatorProvider struct {
+// CelEnvironment is a struct that encapsulates CEL custom program enviroment
+type CelEnvironment struct {
 	env *cel.Env
 }
 
-type celEvaluator struct {
+// CelProgram is a struct that encapsulates compiled CEL program
+type CelProgram struct {
 	program cel.Program
 }
 
@@ -110,8 +112,9 @@ func semverCompare(val1, val2 ref.Val) ref.Val {
 	return types.Int(v1.Compare(v2))
 }
 
-func (e celEvaluator) Evaluate(env map[string]interface{}) (bool, error) {
-	result, _, err := e.program.Eval(env)
+// Evaluate to evaluate the compiled CEL program against input data (map)
+func (e CelProgram) Evaluate(data map[string]interface{}) (bool, error) {
+	result, _, err := e.program.Eval(data)
 	if err != nil {
 		return false, err
 	}
@@ -123,19 +126,21 @@ func (e celEvaluator) Evaluate(env map[string]interface{}) (bool, error) {
 	return false, fmt.Errorf("cel expression evalutated to %T, not bool", result.Value())
 }
 
-func (e *celEvaluatorProvider) Evaluator(rule string) (Evaluator, error) {
+// Validate to validate the CEL expression string by compiling it into CEL program
+func (e *CelEnvironment) Validate(rule string) (CelProgram, error) {
+	var celProg CelProgram
 	ast, issues := e.env.Compile(rule)
 	if err := issues.Err(); err != nil {
-		return nil, err
+		return celProg, err
 	}
 
 	if ast.ResultType() != decls.Bool {
-		return nil, fmt.Errorf("cel expressions must have type Bool")
+		return celProg, fmt.Errorf("cel expressions must have type Bool")
 	}
 
 	prog, err := e.env.Program(ast)
 	if err != nil {
-		return nil, err
+		return celProg, err
 	}
-	return celEvaluator{program: prog}, nil
+	return CelProgram{program: prog}, nil
 }

pkg/cel/cel_test.go renamed to pkg/constraints/cel_test.go

@@ -1,4 +1,4 @@
-package cel
+package constraints
 
 import (
 	"testing"
@@ -28,22 +28,37 @@ func TestCelLibrary(t *testing.T) {
 			isErr: false,
 		},
 		{
-			name:  "ValidCelExpression/False",
+			name:  "ValidCelExpression/NotEqual/False",
 			rule:  "properties.exists(p, p.type == 'olm.test' && (semver_compare(p.value, '1.0.1') == 0))",
 			out:   false,
 			isErr: false,
 		},
 		{
-			name:  "InvalidCelExpression",
+			name:  "ValidCelExpression/Less/False",
+			rule:  "properties.exists(p, p.type == 'olm.test' && (semver_compare(p.value, '1.0.0') < 0))",
+			isErr: false,
+		},
+		{
+			name:  "ValidCelExpression/Larger/False",
+			rule:  "properties.exists(p, p.type == 'olm.test' && (semver_compare(p.value, '1.0.0') > 0))",
+			isErr: false,
+		},
+		{
+			name:  "InvalidCelExpression/NotExistedFunc",
 			rule:  "properties.exists(p, p.type == 'olm.test' && (doesnt_exist(p.value, '1.0.0') == 0))",
 			isErr: true,
 		},
+		{
+			name:  "InvalidCelExpression/NonBoolReturn",
+			rule:  "1",
+			isErr: true,
+		},
 	}
 
-	validator := NewCelEvaluatorProvider()
+	validator := NewCelEnvironment()
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			prog, err := validator.Evaluator(tt.rule)
+			prog, err := validator.Validate(tt.rule)
 			if tt.isErr {
 				assert.Error(t, err)
 			} else {