From 3bb33f53b707de968dace2a1bcd19825e7556090 Mon Sep 17 00:00:00 2001 From: Gereon Frey Date: Wed, 23 Apr 2025 15:45:09 +0200 Subject: [PATCH 1/7] Implement kro CEL library for maps Only function right now is merge. Potentially there are more. This implementation is inspired by the CEL lists extension. --- pkg/cel/environment.go | 1 + pkg/cel/library/maps.go | 106 +++++++++++++++++++++++++++++++++++ pkg/cel/library/maps_test.go | 77 +++++++++++++++++++++++++ 3 files changed, 184 insertions(+) create mode 100644 pkg/cel/library/maps.go create mode 100644 pkg/cel/library/maps_test.go diff --git a/pkg/cel/environment.go b/pkg/cel/environment.go index db8921daf..1b78f68cc 100644 --- a/pkg/cel/environment.go +++ b/pkg/cel/environment.go @@ -57,6 +57,7 @@ func DefaultEnvironment(options ...EnvOption) (*cel.Env, error) { cel.OptionalTypes(), ext.Encoders(), library.Random(), + library.Maps(), } opts := &envOptions{} diff --git a/pkg/cel/library/maps.go b/pkg/cel/library/maps.go new file mode 100644 index 000000000..afe363b2f --- /dev/null +++ b/pkg/cel/library/maps.go @@ -0,0 +1,106 @@ +package library + +import ( + "math" + + "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/common/types/traits" +) + +// Maps returns a cel.EnvOption to configure extended functions for map manipulation. +// +// # Merge +// +// Merges two maps. Keys from the first map take precedence over keys in the second map. +// +// map(string, int).merge(map(string, int)) -> map(string, int) +// +// Examples: +// +// {}.merge({}) == {} +// {}.merge({'a': 1}) == {'a': 1} +// {'a': 1}.merge({}) == {'a': 1} +// {'a': 1}.merge({'b': 2}) == {'a': 1, 'b': 2} +// {'a': 1}.merge({'a': 2, 'b': 2}) == {'a': 1, 'b': 2} +func Maps(options ...MapsOption) cel.EnvOption { + l := &mapsLib{version: math.MaxUint32} + for _, opt := range options { + opt(l) + } + return cel.Lib(l) +} + +type mapsLib struct { + version uint32 +} + +type MapsOption func(*mapsLib) *mapsLib + +// LibraryName implements the cel.SingletonLibrary interface method. +func (l *mapsLib) LibraryName() string { + return "cel.lib.ext.kro.maps" +} + +// CompileOptions implements the cel.Library interface method. +func (l *mapsLib) CompileOptions() []cel.EnvOption { + mapType := cel.MapType(cel.TypeParamType("K"), cel.DynType) + // mapDynType := cel.MapType(cel.DynType, cel.DynType) + opts := []cel.EnvOption{ + cel.Function("merge", + cel.MemberOverload("map_merge", + []*cel.Type{mapType, mapType}, + mapType, + cel.BinaryBinding(func(arg1, arg2 ref.Val) ref.Val { + self, ok := arg1.(traits.Mapper) + if !ok { + return types.ValOrErr(arg1, "no such overload: %v.merge(%v)", arg1.Type(), arg2.Type()) + } + other, ok := arg2.(traits.Mapper) + if !ok { + return types.ValOrErr(arg1, "no such overload: %v.merge(%v)", arg1.Type(), arg2.Type()) + } + result, err := merge(self, other) + if err != nil { + return types.WrapErr(err) + } + return result + }), + ), + ), + } + return opts +} + +// ProgramOptions implements the cel.Library interface method. +func (l *mapsLib) ProgramOptions() []cel.ProgramOption { + return []cel.ProgramOption{} +} + +// merge merges two maps. Keys from the first map take precedence over keys in +// the second map. +func merge(self traits.Mapper, other traits.Mapper) (traits.Mapper, error) { + var result traits.MutableMapper + + if mapVal, ok := self.Value().(map[ref.Val]ref.Val); ok { + result = types.NewMutableMap(types.DefaultTypeAdapter, mapVal) + } else { + result = types.NewMutableMap(types.DefaultTypeAdapter, nil) + for i := self.Iterator(); i.HasNext().(types.Bool); { + k := i.Next() + v := self.Get(k) + result.Insert(k, v) + } + } + + for i := other.Iterator(); i.HasNext().(types.Bool); { + k := i.Next() + if result.Contains(k).(types.Bool) { + continue + } + v := other.Get(k) + result.Insert(k, v) + } + return result.ToImmutableMap(), nil +} diff --git a/pkg/cel/library/maps_test.go b/pkg/cel/library/maps_test.go new file mode 100644 index 000000000..a4d27503b --- /dev/null +++ b/pkg/cel/library/maps_test.go @@ -0,0 +1,77 @@ +package library + +import ( + "fmt" + "strings" + "testing" + + "github.com/google/cel-go/cel" +) + +func TestMaps(t *testing.T) { + mapsTests := []struct { + expr string + err string + }{ + {expr: `{}.merge({}) == {}`}, + {expr: `{}.merge({'a': 1}) == {'a': 1}`}, + {expr: `{'a': 1}.merge({}) == {'a': 1}`}, + {expr: `{'a': 1}.merge({'b': 2}) == {'a': 1, 'b': 2}`}, + {expr: `{'a': 1}.merge({'a': 2, 'b': 2}) == {'a': 1, 'b': 2}`}, + {expr: `{'a': 1}.merge({'b': 2.1}) == {'a': 1, 'b': 2.1}`}, + {expr: `{'a': 1}.merge({'b': 'foo'}) == {'a': 1, 'b': 'foo'}`}, + {expr: `{'a': 1}.merge({'b': {'c': 2}}) == {'a': 1, 'b': {'c': 2}}`}, + + // {expr: `{}.merge([])`, err: "ERROR: :1:9: found no matching overload for 'merge' applied to 'map(dyn, dyn).(list(dyn))'"}, + } + + env := testMapsEnv(t) + for i, tc := range mapsTests { + t.Run(fmt.Sprintf("%d", i), func(t *testing.T) { + var asts []*cel.Ast + pAst, iss := env.Parse(tc.expr) + if iss.Err() != nil { + t.Fatalf("env.Parse(%v) failed: %v", tc.expr, iss.Err()) + } + asts = append(asts, pAst) + cAst, iss := env.Check(pAst) + if iss.Err() != nil { + t.Fatalf("env.Check(%v) failed: %v", tc.expr, iss.Err()) + } + asts = append(asts, cAst) + + for _, ast := range asts { + prg, err := env.Program(ast) + if err != nil { + t.Fatalf("env.Program() failed: %v", err) + } + out, _, err := prg.Eval(cel.NoVars()) + if tc.err != "" { + if err == nil { + t.Fatalf("got value %v, wanted error %s for expr: %s", + out.Value(), tc.err, tc.expr) + } + if !strings.Contains(err.Error(), tc.err) { + t.Errorf("got error %v, wanted error %s for expr: %s", err, tc.err, tc.expr) + } + } else if err != nil { + t.Fatal(err) + } else if out.Value() != true { + t.Errorf("got %v, wanted true for expr: %s", out.Value(), tc.expr) + } + } + }) + } +} + +func testMapsEnv(t *testing.T, opts ...cel.EnvOption) *cel.Env { + t.Helper() + baseOpts := []cel.EnvOption{ + Maps(), + } + env, err := cel.NewEnv(append(baseOpts, opts...)...) + if err != nil { + t.Fatalf("cel.NewEnv(Maps()) failed: %v", err) + } + return env +} From 1aaecef6020fd6caa6a13ddd3023c1d30ad15bbf Mon Sep 17 00:00:00 2001 From: Gereon Frey Date: Mon, 18 Aug 2025 15:08:37 +0200 Subject: [PATCH 2/7] Remove redundant return value There will never be an error, so no need to always return nil. --- pkg/cel/library/maps.go | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/pkg/cel/library/maps.go b/pkg/cel/library/maps.go index afe363b2f..00894e6cc 100644 --- a/pkg/cel/library/maps.go +++ b/pkg/cel/library/maps.go @@ -61,11 +61,7 @@ func (l *mapsLib) CompileOptions() []cel.EnvOption { if !ok { return types.ValOrErr(arg1, "no such overload: %v.merge(%v)", arg1.Type(), arg2.Type()) } - result, err := merge(self, other) - if err != nil { - return types.WrapErr(err) - } - return result + return merge(self, other) }), ), ), @@ -80,7 +76,7 @@ func (l *mapsLib) ProgramOptions() []cel.ProgramOption { // merge merges two maps. Keys from the first map take precedence over keys in // the second map. -func merge(self traits.Mapper, other traits.Mapper) (traits.Mapper, error) { +func merge(self traits.Mapper, other traits.Mapper) traits.Mapper { var result traits.MutableMapper if mapVal, ok := self.Value().(map[ref.Val]ref.Val); ok { @@ -102,5 +98,5 @@ func merge(self traits.Mapper, other traits.Mapper) (traits.Mapper, error) { v := other.Get(k) result.Insert(k, v) } - return result.ToImmutableMap(), nil + return result.ToImmutableMap() } From efa762ea407ff24f7a1d550c5ae94aeb3da80e77 Mon Sep 17 00:00:00 2001 From: Gereon Frey Date: Mon, 18 Aug 2025 15:09:20 +0200 Subject: [PATCH 3/7] Change order of merge operation The first map's values will be overwritten by values from the second map. This is what merge should do. --- pkg/cel/library/maps.go | 15 ++++++++------- pkg/cel/library/maps_test.go | 2 +- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/pkg/cel/library/maps.go b/pkg/cel/library/maps.go index 00894e6cc..da0ae6244 100644 --- a/pkg/cel/library/maps.go +++ b/pkg/cel/library/maps.go @@ -13,9 +13,10 @@ import ( // // # Merge // -// Merges two maps. Keys from the first map take precedence over keys in the second map. +// Merges two maps. Keys from the second map overwrite already available keys in the first map. +// Keys must be of type string, values can be of any type. // -// map(string, int).merge(map(string, int)) -> map(string, int) +// map(string, any).merge(map(string, any)) -> map(string, any) // // Examples: // @@ -79,23 +80,23 @@ func (l *mapsLib) ProgramOptions() []cel.ProgramOption { func merge(self traits.Mapper, other traits.Mapper) traits.Mapper { var result traits.MutableMapper - if mapVal, ok := self.Value().(map[ref.Val]ref.Val); ok { + if mapVal, ok := other.Value().(map[ref.Val]ref.Val); ok { result = types.NewMutableMap(types.DefaultTypeAdapter, mapVal) } else { result = types.NewMutableMap(types.DefaultTypeAdapter, nil) - for i := self.Iterator(); i.HasNext().(types.Bool); { + for i := other.Iterator(); i.HasNext().(types.Bool); { k := i.Next() - v := self.Get(k) + v := other.Get(k) result.Insert(k, v) } } - for i := other.Iterator(); i.HasNext().(types.Bool); { + for i := self.Iterator(); i.HasNext().(types.Bool); { k := i.Next() if result.Contains(k).(types.Bool) { continue } - v := other.Get(k) + v := self.Get(k) result.Insert(k, v) } return result.ToImmutableMap() diff --git a/pkg/cel/library/maps_test.go b/pkg/cel/library/maps_test.go index a4d27503b..af1345031 100644 --- a/pkg/cel/library/maps_test.go +++ b/pkg/cel/library/maps_test.go @@ -17,7 +17,7 @@ func TestMaps(t *testing.T) { {expr: `{}.merge({'a': 1}) == {'a': 1}`}, {expr: `{'a': 1}.merge({}) == {'a': 1}`}, {expr: `{'a': 1}.merge({'b': 2}) == {'a': 1, 'b': 2}`}, - {expr: `{'a': 1}.merge({'a': 2, 'b': 2}) == {'a': 1, 'b': 2}`}, + {expr: `{'a': 1}.merge({'a': 2, 'b': 2}) == {'a': 2, 'b': 2}`}, {expr: `{'a': 1}.merge({'b': 2.1}) == {'a': 1, 'b': 2.1}`}, {expr: `{'a': 1}.merge({'b': 'foo'}) == {'a': 1, 'b': 'foo'}`}, {expr: `{'a': 1}.merge({'b': {'c': 2}}) == {'a': 1, 'b': {'c': 2}}`}, From e28f93eab6d583e4a9ea704b445ee51b6f341880 Mon Sep 17 00:00:00 2001 From: Gereon Frey Date: Mon, 18 Aug 2025 15:28:49 +0200 Subject: [PATCH 4/7] Make sure merged map values have same type This is more restrictive. The other way is supported, but the consequences are not clear. So let's keep it simple. --- pkg/cel/library/maps.go | 6 +++--- pkg/cel/library/maps_test.go | 5 ++--- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/pkg/cel/library/maps.go b/pkg/cel/library/maps.go index da0ae6244..57b8024e9 100644 --- a/pkg/cel/library/maps.go +++ b/pkg/cel/library/maps.go @@ -14,9 +14,9 @@ import ( // # Merge // // Merges two maps. Keys from the second map overwrite already available keys in the first map. -// Keys must be of type string, values can be of any type. +// Keys must be of type string, value types must be identical in the maps merged. // -// map(string, any).merge(map(string, any)) -> map(string, any) +// map(string, T).merge(map(string, T)) -> map(string, T) // // Examples: // @@ -46,7 +46,7 @@ func (l *mapsLib) LibraryName() string { // CompileOptions implements the cel.Library interface method. func (l *mapsLib) CompileOptions() []cel.EnvOption { - mapType := cel.MapType(cel.TypeParamType("K"), cel.DynType) + mapType := cel.MapType(cel.TypeParamType("K"), cel.TypeParamType("V")) // mapDynType := cel.MapType(cel.DynType, cel.DynType) opts := []cel.EnvOption{ cel.Function("merge", diff --git a/pkg/cel/library/maps_test.go b/pkg/cel/library/maps_test.go index af1345031..2b0a101e9 100644 --- a/pkg/cel/library/maps_test.go +++ b/pkg/cel/library/maps_test.go @@ -15,12 +15,11 @@ func TestMaps(t *testing.T) { }{ {expr: `{}.merge({}) == {}`}, {expr: `{}.merge({'a': 1}) == {'a': 1}`}, + {expr: `{}.merge({'a': 2.1}) == {'a': 2.1}`}, + {expr: `{}.merge({'a': 'foo'}) == {'a': 'foo'}`}, {expr: `{'a': 1}.merge({}) == {'a': 1}`}, {expr: `{'a': 1}.merge({'b': 2}) == {'a': 1, 'b': 2}`}, {expr: `{'a': 1}.merge({'a': 2, 'b': 2}) == {'a': 2, 'b': 2}`}, - {expr: `{'a': 1}.merge({'b': 2.1}) == {'a': 1, 'b': 2.1}`}, - {expr: `{'a': 1}.merge({'b': 'foo'}) == {'a': 1, 'b': 'foo'}`}, - {expr: `{'a': 1}.merge({'b': {'c': 2}}) == {'a': 1, 'b': {'c': 2}}`}, // {expr: `{}.merge([])`, err: "ERROR: :1:9: found no matching overload for 'merge' applied to 'map(dyn, dyn).(list(dyn))'"}, } From 44ece4db278f39dd4e0dbb4d2d3abaa789193e66 Mon Sep 17 00:00:00 2001 From: Gereon Frey Date: Tue, 19 Aug 2025 16:04:25 +0200 Subject: [PATCH 5/7] Add license headers --- pkg/cel/library/maps.go | 14 ++++++++++++++ pkg/cel/library/maps_test.go | 14 ++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/pkg/cel/library/maps.go b/pkg/cel/library/maps.go index 57b8024e9..000c63f2e 100644 --- a/pkg/cel/library/maps.go +++ b/pkg/cel/library/maps.go @@ -1,3 +1,17 @@ +// Copyright 2025 The Kube Resource Orchestrator Authors +// +// 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 +// +// http://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. + package library import ( diff --git a/pkg/cel/library/maps_test.go b/pkg/cel/library/maps_test.go index 2b0a101e9..234666917 100644 --- a/pkg/cel/library/maps_test.go +++ b/pkg/cel/library/maps_test.go @@ -1,3 +1,17 @@ +// Copyright 2025 The Kube Resource Orchestrator Authors +// +// 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 +// +// http://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. + package library import ( From 89467b4cac8c8582fa478f30721d7717a5848060 Mon Sep 17 00:00:00 2001 From: Gereon Frey Date: Tue, 19 Aug 2025 16:11:48 +0200 Subject: [PATCH 6/7] Simplify merge function --- pkg/cel/library/maps.go | 41 +++++++++++++++++++++++------------------ 1 file changed, 23 insertions(+), 18 deletions(-) diff --git a/pkg/cel/library/maps.go b/pkg/cel/library/maps.go index 000c63f2e..b9937bfbb 100644 --- a/pkg/cel/library/maps.go +++ b/pkg/cel/library/maps.go @@ -89,29 +89,34 @@ func (l *mapsLib) ProgramOptions() []cel.ProgramOption { return []cel.ProgramOption{} } -// merge merges two maps. Keys from the first map take precedence over keys in -// the second map. -func merge(self traits.Mapper, other traits.Mapper) traits.Mapper { - var result traits.MutableMapper - - if mapVal, ok := other.Value().(map[ref.Val]ref.Val); ok { - result = types.NewMutableMap(types.DefaultTypeAdapter, mapVal) - } else { - result = types.NewMutableMap(types.DefaultTypeAdapter, nil) - for i := other.Iterator(); i.HasNext().(types.Bool); { - k := i.Next() - v := other.Get(k) - result.Insert(k, v) - } +func mergeVals(lhs, rhs ref.Val) ref.Val { + left, lok := lhs.(traits.Mapper) + right, rok := rhs.(traits.Mapper) + if !lok || !rok { + return types.ValOrErr(lhs, "no such overload: %v.merge(%v)", lhs.Type(), rhs.Type()) } + return merge(left, right) +} +// merge returns a new map containing entries from both maps. +// Keys in 'other' overwrite keys in 'self'. +func merge(self, other traits.Mapper) traits.Mapper { + result := mapperTraitToMutableMapper(other) for i := self.Iterator(); i.HasNext().(types.Bool); { k := i.Next() - if result.Contains(k).(types.Bool) { - continue + if !result.Contains(k).(types.Bool) { + result.Insert(k, self.Get(k)) } - v := self.Get(k) - result.Insert(k, v) } return result.ToImmutableMap() } + +// mapperTraitToMutableMapper copies a traits.Mapper into a MutableMap. +func mapperTraitToMutableMapper(m traits.Mapper) traits.MutableMapper { + vals := make(map[ref.Val]ref.Val, m.Size().(types.Int)) + for it := m.Iterator(); it.HasNext().(types.Bool); { + k := it.Next() + vals[k] = m.Get(k) + } + return types.NewMutableMap(types.DefaultTypeAdapter, vals) +} From 904e210cf10b1b617e34c8e191e6ef4aa4e41ad5 Mon Sep 17 00:00:00 2001 From: Gereon Frey Date: Tue, 19 Aug 2025 16:23:53 +0200 Subject: [PATCH 7/7] Simplify merge support function call and tests --- pkg/cel/library/maps.go | 12 +-------- pkg/cel/library/maps_test.go | 52 +++++++++++++----------------------- 2 files changed, 20 insertions(+), 44 deletions(-) diff --git a/pkg/cel/library/maps.go b/pkg/cel/library/maps.go index b9937bfbb..b30d23a65 100644 --- a/pkg/cel/library/maps.go +++ b/pkg/cel/library/maps.go @@ -67,17 +67,7 @@ func (l *mapsLib) CompileOptions() []cel.EnvOption { cel.MemberOverload("map_merge", []*cel.Type{mapType, mapType}, mapType, - cel.BinaryBinding(func(arg1, arg2 ref.Val) ref.Val { - self, ok := arg1.(traits.Mapper) - if !ok { - return types.ValOrErr(arg1, "no such overload: %v.merge(%v)", arg1.Type(), arg2.Type()) - } - other, ok := arg2.(traits.Mapper) - if !ok { - return types.ValOrErr(arg1, "no such overload: %v.merge(%v)", arg1.Type(), arg2.Type()) - } - return merge(self, other) - }), + cel.BinaryBinding(mergeVals), ), ), } diff --git a/pkg/cel/library/maps_test.go b/pkg/cel/library/maps_test.go index 234666917..ffa8808ee 100644 --- a/pkg/cel/library/maps_test.go +++ b/pkg/cel/library/maps_test.go @@ -16,16 +16,17 @@ package library import ( "fmt" - "strings" "testing" "github.com/google/cel-go/cel" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestMaps(t *testing.T) { mapsTests := []struct { expr string - err string + err require.ErrorAssertionFunc }{ {expr: `{}.merge({}) == {}`}, {expr: `{}.merge({'a': 1}) == {'a': 1}`}, @@ -35,44 +36,29 @@ func TestMaps(t *testing.T) { {expr: `{'a': 1}.merge({'b': 2}) == {'a': 1, 'b': 2}`}, {expr: `{'a': 1}.merge({'a': 2, 'b': 2}) == {'a': 2, 'b': 2}`}, - // {expr: `{}.merge([])`, err: "ERROR: :1:9: found no matching overload for 'merge' applied to 'map(dyn, dyn).(list(dyn))'"}, + {expr: `{}.merge([])`, err: func(t require.TestingT, err error, i ...interface{}) { + require.ErrorContains(t, err, "no matching overload for 'merge'") + }}, } env := testMapsEnv(t) for i, tc := range mapsTests { t.Run(fmt.Sprintf("%d", i), func(t *testing.T) { - var asts []*cel.Ast - pAst, iss := env.Parse(tc.expr) - if iss.Err() != nil { - t.Fatalf("env.Parse(%v) failed: %v", tc.expr, iss.Err()) - } - asts = append(asts, pAst) - cAst, iss := env.Check(pAst) - if iss.Err() != nil { - t.Fatalf("env.Check(%v) failed: %v", tc.expr, iss.Err()) - } - asts = append(asts, cAst) + r := require.New(t) - for _, ast := range asts { - prg, err := env.Program(ast) - if err != nil { - t.Fatalf("env.Program() failed: %v", err) - } - out, _, err := prg.Eval(cel.NoVars()) - if tc.err != "" { - if err == nil { - t.Fatalf("got value %v, wanted error %s for expr: %s", - out.Value(), tc.err, tc.expr) - } - if !strings.Contains(err.Error(), tc.err) { - t.Errorf("got error %v, wanted error %s for expr: %s", err, tc.err, tc.expr) - } - } else if err != nil { - t.Fatal(err) - } else if out.Value() != true { - t.Errorf("got %v, wanted true for expr: %s", out.Value(), tc.expr) - } + ast, iss := env.Compile(tc.expr) + if tc.err != nil { + tc.err(t, iss.Err()) + return } + r.NoError(iss.Err(), "compile failed for expr: %s", tc.expr) + + prg, err := env.Program(ast) + require.NoError(t, err) + + out, _, err := prg.Eval(cel.NoVars()) + require.NoError(t, err) + assert.True(t, out.Value().(bool)) }) } }