Skip to content

Commit

Permalink
Merge pull request #119807 from jpbetz/automated-cherry-pick-of-#1198…
Browse files Browse the repository at this point in the history 
…00-origin-release-1.28

Automated cherry pick of #119800: Fixes CEL estimated cost to propagate result sizes correctly

Kubernetes-commit: ab3cebfdb2cd1054f34f4287a757755810ede009
  • Loading branch information
@k8s-publishing-bot
k8s-publishing-bot committed Sep 6, 2023
2 parents bf038b7 + 915c09d commit 7e09bf3
Show file tree
Hide file tree
Showing 4 changed files with 187 additions and 12 deletions.
6 changes: 3 additions & 3 deletions go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ require (
github.com/evanphx/json-patch v4.12.0+incompatible
github.com/fsnotify/fsnotify v1.6.0
github.com/gogo/protobuf v1.3.2
github.com/google/cel-go v0.16.0
github.com/google/cel-go v0.16.1
github.com/google/gnostic-models v0.6.8
github.com/google/go-cmp v0.5.9
github.com/google/gofuzz v1.2.0
Expand Down Expand Up @@ -43,7 +43,7 @@ require (
gopkg.in/square/go-jose.v2 v2.6.0
k8s.io/api v0.0.0-20230904104028-546e4253e738
k8s.io/apimachinery v0.0.0-20230904102823-bc548d1d2406
k8s.io/client-go v0.0.0-20230904110526-29a840dbdf2a
k8s.io/client-go v0.0.0-20230904110526-745513ad7b37
k8s.io/component-base v0.0.0-20230904111932-ef6aa9891ad3
k8s.io/klog/v2 v2.100.1
k8s.io/kms v0.0.0-20230904112654-85054a342003
Expand Down Expand Up @@ -128,7 +128,7 @@ require (
replace (
k8s.io/api => k8s.io/api v0.0.0-20230904104028-546e4253e738
k8s.io/apimachinery => k8s.io/apimachinery v0.0.0-20230904102823-bc548d1d2406
k8s.io/client-go => k8s.io/client-go v0.0.0-20230904110526-29a840dbdf2a
k8s.io/client-go => k8s.io/client-go v0.0.0-20230904110526-745513ad7b37
k8s.io/component-base => k8s.io/component-base v0.0.0-20230904111932-ef6aa9891ad3
k8s.io/kms => k8s.io/kms v0.0.0-20230904112654-85054a342003
)
8 changes: 4 additions & 4 deletions go.sum
View file
Original file line number Diff line number Diff line change
Expand Up @@ -160,8 +160,8 @@ github.com/google/btree v0.0.0-20180813153112-4030bb1f1f0c/go.mod h1:lNA+9X1NB3Z
github.com/google/btree v1.0.0/go.mod h1:lNA+9X1NB3Zf8V7Ke586lFgjr2dZNuvo3lPJSGZ5JPQ=
github.com/google/btree v1.0.1 h1:gK4Kx5IaGY9CD5sPJ36FHiBJ6ZXl0kilRiiCj+jdYp4=
github.com/google/btree v1.0.1/go.mod h1:xXMiIv4Fb/0kKde4SpL7qlzvu5cMJDRkFDxJfI9uaxA=
github.com/google/cel-go v0.16.0 h1:DG9YQ8nFCFXAs/FDDwBxmL1tpKNrdlGUM9U3537bX/Y=
github.com/google/cel-go v0.16.0/go.mod h1:HXZKzB0LXqer5lHHgfWAnlYwJaQBDKMjxjulNQzhwhY=
github.com/google/cel-go v0.16.1 h1:3hZfSNiAU3KOiNtxuFXVp5WFy4hf/Ly3Sa4/7F8SXNo=
github.com/google/cel-go v0.16.1/go.mod h1:HXZKzB0LXqer5lHHgfWAnlYwJaQBDKMjxjulNQzhwhY=
github.com/google/gnostic-models v0.6.8 h1:yo/ABAfM5IMRsS1VnXjTBvUb61tFIHozhlYvRgGre9I=
github.com/google/gnostic-models v0.6.8/go.mod h1:5n7qKqH0f5wFt+aWF8CW6pZLLNOfYuF5OpfBSENuI8U=
github.com/google/go-cmp v0.2.0/go.mod h1:oXzfMopK8JAjlY9xF4vHSVASa0yLyX7SntLO5aqRK0M=
Expand Down Expand Up @@ -674,8 +674,8 @@ k8s.io/api v0.0.0-20230904104028-546e4253e738 h1:0PmEjeZX6g4bLgztHHEVVj0XrMQ+F8u
k8s.io/api v0.0.0-20230904104028-546e4253e738/go.mod h1:4GpdFbUbYEFNj6e44T5yz6hE7p8yAn4+dYvmqTPVuOs=
k8s.io/apimachinery v0.0.0-20230904102823-bc548d1d2406 h1:/YP5PLKVrV5WDzScq2agCBHPP0FCLVrU/4d9+j8uEFs=
k8s.io/apimachinery v0.0.0-20230904102823-bc548d1d2406/go.mod h1:RdzF87y/ngqk9H4z3EL2Rppv5jj95vGS/HaFXrLDApU=
k8s.io/client-go v0.0.0-20230904110526-29a840dbdf2a h1:Qgr4OBvwwsxyKonHVJCGGOJH4Zee2uMD1v2st+fVByY=
k8s.io/client-go v0.0.0-20230904110526-29a840dbdf2a/go.mod h1:9oac6LWqvfKcphw6P69YH6Ns8xgf4S3ciKrBfqXxq3c=
k8s.io/client-go v0.0.0-20230904110526-745513ad7b37 h1:+SF0CtBoQI6Eso+8poN4i55d4oUahClerZwALYSz6Tw=
k8s.io/client-go v0.0.0-20230904110526-745513ad7b37/go.mod h1:9oac6LWqvfKcphw6P69YH6Ns8xgf4S3ciKrBfqXxq3c=
k8s.io/component-base v0.0.0-20230904111932-ef6aa9891ad3 h1:6J1OGuJeD5K+0YHFN5lOfVIIEpg+Iw+0w4VEw+EipVE=
k8s.io/component-base v0.0.0-20230904111932-ef6aa9891ad3/go.mod h1:idL61MaRYcyCDRF1/WNvCkmTbPkAeViaCwM8CA3jRRQ=
k8s.io/klog/v2 v2.100.1 h1:7WCHKK6K8fNhTqfBhISHQ97KrnJNFZMcQvKp7gP/tmg=
Expand Down
48 changes: 43 additions & 5 deletions pkg/cel/library/cost.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,13 +126,51 @@ func (l *CostEstimator) EstimateCallCost(function, overloadId string, target *ch
sz := l.sizeEstimate(*target)
toReplaceSz := l.sizeEstimate(args[0])
replaceWithSz := l.sizeEstimate(args[1])
// smallest possible result: smallest input size composed of the largest possible substrings being replaced by smallest possible replacement
minSz := uint64(math.Ceil(float64(sz.Min)/float64(toReplaceSz.Max))) * replaceWithSz.Min
// largest possible result: largest input size composed of the smallest possible substrings being replaced by largest possible replacement
maxSz := uint64(math.Ceil(float64(sz.Max)/float64(toReplaceSz.Min))) * replaceWithSz.Max

var replaceCount, retainedSz checker.SizeEstimate
// find the longest replacement:
if toReplaceSz.Min == 0 {
// if the string being replaced is empty, replace surrounds all characters in the input string with the replacement.
if sz.Max < math.MaxUint64 {
replaceCount.Max = sz.Max + 1
} else {
replaceCount.Max = sz.Max
}
// Include the length of the longest possible original string length.
retainedSz.Max = sz.Max
} else if replaceWithSz.Max <= toReplaceSz.Min {
// If the replacement does not make the result longer, use the original string length.
replaceCount.Max = 0
retainedSz.Max = sz.Max
} else {
// Replace the smallest possible substrings with the largest possible replacement
// as many times as possible.
replaceCount.Max = uint64(math.Ceil(float64(sz.Max) / float64(toReplaceSz.Min)))
}

// find the shortest replacement:
if toReplaceSz.Max == 0 {
// if the string being replaced is empty, replace surrounds all characters in the input string with the replacement.
if sz.Min < math.MaxUint64 {
replaceCount.Min = sz.Min + 1
} else {
replaceCount.Min = sz.Min
}
// Include the length of the shortest possible original string length.
retainedSz.Min = sz.Min
} else if toReplaceSz.Max <= replaceWithSz.Min {
// If the replacement does not make the result shorter, use the original string length.
replaceCount.Min = 0
retainedSz.Min = sz.Min
} else {
// Replace the largest possible substrings being with the smallest possible replacement
// as many times as possible.
replaceCount.Min = uint64(math.Ceil(float64(sz.Min) / float64(toReplaceSz.Max)))
}
size := replaceCount.Multiply(replaceWithSz).Add(retainedSz)

// cost is the traversal plus the construction of the result
return &checker.CallEstimate{CostEstimate: sz.MultiplyByCostFactor(2 * common.StringTraversalCostFactor), ResultSize: &checker.SizeEstimate{Min: minSz, Max: maxSz}}
return &checker.CallEstimate{CostEstimate: sz.MultiplyByCostFactor(2 * common.StringTraversalCostFactor), ResultSize: &size}
}
case "split":
if target != nil {
Expand Down
137 changes: 137 additions & 0 deletions pkg/cel/library/cost_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -227,18 +227,54 @@ func TestStringLibrary(t *testing.T) {
expectEsimatedCost: checker.CostEstimate{Min: 3, Max: 3},
expectRuntimeCost: 3,
},
{
name: "lowerAsciiEquals",
expr: "'ABCDEFGHIJ abcdefghij'.lowerAscii() == 'abcdefghij ABCDEFGHIJ'.lowerAscii()",
expectEsimatedCost: checker.CostEstimate{Min: 7, Max: 9},
expectRuntimeCost: 9,
},
{
name: "upperAscii",
expr: "'ABCDEFGHIJ abcdefghij'.upperAscii()",
expectEsimatedCost: checker.CostEstimate{Min: 3, Max: 3},
expectRuntimeCost: 3,
},
{
name: "upperAsciiEquals",
expr: "'ABCDEFGHIJ abcdefghij'.upperAscii() == 'abcdefghij ABCDEFGHIJ'.upperAscii()",
expectEsimatedCost: checker.CostEstimate{Min: 7, Max: 9},
expectRuntimeCost: 9,
},
{
name: "quote",
expr: "strings.quote('ABCDEFGHIJ abcdefghij')",
expectEsimatedCost: checker.CostEstimate{Min: 3, Max: 3},
expectRuntimeCost: 3,
},
{
name: "quoteEquals",
expr: "strings.quote('ABCDEFGHIJ abcdefghij') == strings.quote('ABCDEFGHIJ abcdefghij')",
expectEsimatedCost: checker.CostEstimate{Min: 7, Max: 11},
expectRuntimeCost: 9,
},
{
name: "replace",
expr: "'abc 123 def 123'.replace('123', '456')",
expectEsimatedCost: checker.CostEstimate{Min: 3, Max: 3},
expectRuntimeCost: 3,
},
{
name: "replace between all chars",
expr: "'abc 123 def 123'.replace('', 'x')",
expectEsimatedCost: checker.CostEstimate{Min: 3, Max: 3},
expectRuntimeCost: 3,
},
{
name: "replace with empty",
expr: "'abc 123 def 123'.replace('123', '')",
expectEsimatedCost: checker.CostEstimate{Min: 3, Max: 3},
expectRuntimeCost: 3,
},
{
name: "replace with limit",
expr: "'abc 123 def 123'.replace('123', '456', 1)",
Expand Down Expand Up @@ -413,6 +449,107 @@ func testCost(t *testing.T, expr string, expectEsimatedCost checker.CostEstimate
}
}

func TestSize(t *testing.T) {
exactSize := func(size int) checker.SizeEstimate {
return checker.SizeEstimate{Min: uint64(size), Max: uint64(size)}
}
exactSizes := func(sizes ...int) []checker.SizeEstimate {
results := make([]checker.SizeEstimate, len(sizes))
for i, size := range sizes {
results[i] = exactSize(size)
}
return results
}
cases := []struct {
name string
function string
overload string
targetSize checker.SizeEstimate
argSizes []checker.SizeEstimate
expectSize checker.SizeEstimate
}{
{
name: "replace empty with char",
function: "replace",
targetSize: exactSize(3), // e.g. abc
argSizes: exactSizes(0, 1), // e.g. replace "" with "_"
expectSize: exactSize(7), // e.g. _a_b_c_
},
{
name: "maybe replace char with empty",
function: "replace",
targetSize: exactSize(3),
argSizes: exactSizes(1, 0),
expectSize: checker.SizeEstimate{Min: 0, Max: 3},
},
{
name: "maybe replace repeated",
function: "replace",
targetSize: exactSize(4),
argSizes: exactSizes(2, 4),
expectSize: checker.SizeEstimate{Min: 4, Max: 8},
},
{
name: "maybe replace empty",
function: "replace",
targetSize: exactSize(4),
argSizes: []checker.SizeEstimate{{Min: 0, Max: 1}, {Min: 0, Max: 2}},
expectSize: checker.SizeEstimate{Min: 0, Max: 14}, // len(__a__a__a__a__) == 14
},
{
name: "replace non-empty size range, maybe larger",
function: "replace",
targetSize: exactSize(4),
argSizes: []checker.SizeEstimate{{Min: 1, Max: 1}, {Min: 1, Max: 2}},
expectSize: checker.SizeEstimate{Min: 4, Max: 8},
},
{
name: "replace non-empty size range, maybe smaller",
function: "replace",
targetSize: exactSize(4),
argSizes: []checker.SizeEstimate{{Min: 1, Max: 2}, {Min: 1, Max: 1}},
expectSize: checker.SizeEstimate{Min: 2, Max: 4},
},
}
est := &CostEstimator{SizeEstimator: &testCostEstimator{}}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
var targetNode checker.AstNode = testSizeNode{size: tc.targetSize}
argNodes := make([]checker.AstNode, len(tc.argSizes))
for i, arg := range tc.argSizes {
argNodes[i] = testSizeNode{size: arg}
}
result := est.EstimateCallCost(tc.function, tc.overload, &targetNode, argNodes)
if result.ResultSize == nil {
t.Fatalf("Expected ResultSize but got none")
}
if *result.ResultSize != tc.expectSize {
t.Fatalf("Expected %+v but got %+v", tc.expectSize, *result.ResultSize)
}
})
}
}

type testSizeNode struct {
size checker.SizeEstimate
}

func (t testSizeNode) Path() []string {
return nil // not needed
}

func (t testSizeNode) Type() *expr.Type {
return nil // not needed
}

func (t testSizeNode) Expr() *expr.Expr {
return nil // not needed
}

func (t testSizeNode) ComputedSize() *checker.SizeEstimate {
return &t.size
}

type testCostEstimator struct {
}

Expand Down

0 comments on commit 7e09bf3

Please sign in to comment.