Skip to content

Commit 0a4e863

Browse files
jpbetzjpbetz
committed
Fix estimated cost for Kubernetes defined CEL types
1 parent b95f9c3 commit 0a4e863

File tree

6 files changed

+98
-9
lines changed

6 files changed

+98
-9
lines changed

‎staging/src/k8s.io/apiserver/pkg/cel/cidr.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ type CIDR struct {
3333
}
3434

3535
var (
36-
CIDRType = cel.OpaqueType("net.CIDR")
36+
CIDRType = cel.ObjectType("net.CIDR")
3737
)
3838

3939
// ConvertToNative implements ref.Val.ConvertToNative.

‎staging/src/k8s.io/apiserver/pkg/cel/ip.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ type IP struct {
3333
}
3434

3535
var (
36-
IPType = cel.OpaqueType("net.IP")
36+
IPType = cel.ObjectType("net.IP")
3737
)
3838

3939
// ConvertToNative implements ref.Val.ConvertToNative.

‎staging/src/k8s.io/apiserver/pkg/cel/library/cost.go‎

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -235,6 +235,23 @@ func (l *CostEstimator) CallCost(function, overloadId string, args []ref.Val, re
235235
// url accessors
236236
cost := uint64(1)
237237
return &cost
238+
case "_==_":
239+
if len(args) == 2 {
240+
unitCost := uint64(1)
241+
lhs := args[0]
242+
switch lhs.(type) {
243+
case cel.Quantity:
244+
return &unitCost
245+
case cel.IP:
246+
return &unitCost
247+
case cel.CIDR:
248+
return &unitCost
249+
case *cel.Format: // Formats have a small max size.
250+
return &unitCost
251+
case cel.URL: // TODO: Computing the actual cost is expensive, and changing this would be a breaking change
252+
return &unitCost
253+
}
254+
}
238255
}
239256
if panicOnUnknown && !knownUnhandledFunctions[function] {
240257
panic(fmt.Errorf("CallCost: unhandled function %q or args %v", function, args))
@@ -278,7 +295,7 @@ func (l *CostEstimator) EstimateCallCost(function, overloadId string, target *ch
278295
case "url":
279296
if len(args) == 1 {
280297
sz := l.sizeEstimate(args[0])
281-
return &checker.CallEstimate{CostEstimate: sz.MultiplyByCostFactor(common.StringTraversalCostFactor)}
298+
return &checker.CallEstimate{CostEstimate: sz.MultiplyByCostFactor(common.StringTraversalCostFactor), ResultSize: &sz}
282299
}
283300
case "lowerAscii", "upperAscii", "substring", "trim":
284301
if target != nil {
@@ -475,6 +492,28 @@ func (l *CostEstimator) EstimateCallCost(function, overloadId string, target *ch
475492
case "getScheme", "getHostname", "getHost", "getPort", "getEscapedPath", "getQuery":
476493
// url accessors
477494
return &checker.CallEstimate{CostEstimate: checker.CostEstimate{Min: 1, Max: 1}}
495+
case "_==_":
496+
if len(args) == 2 {
497+
lhs := args[0]
498+
rhs := args[1]
499+
if lhs.Type().Equal(rhs.Type()) == types.True {
500+
t := lhs.Type()
501+
switch t {
502+
case cel.IPType, cel.CIDRType, cel.QuantityType: // O(1) cost equality checks
503+
return &checker.CallEstimate{CostEstimate: checker.CostEstimate{Min: 1, Max: 1}}
504+
case cel.FormatType:
505+
return &checker.CallEstimate{CostEstimate: checker.CostEstimate{Min: 1, Max: cel.MaxFormatSize}.MultiplyByCostFactor(common.StringTraversalCostFactor)}
506+
case cel.URLType:
507+
size := checker.SizeEstimate{Min: 1, Max: 1}
508+
rhSize := rhs.ComputedSize()
509+
lhSize := rhs.ComputedSize()
510+
if rhSize != nil && lhSize != nil {
511+
size = rhSize.Union(*lhSize)
512+
}
513+
return &checker.CallEstimate{CostEstimate: checker.CostEstimate{Min: 1, Max: size.Max}.MultiplyByCostFactor(common.StringTraversalCostFactor)}
514+
}
515+
}
516+
}
478517
}
479518
if panicOnUnknown && !knownUnhandledFunctions[function] {
480519
panic(fmt.Errorf("EstimateCallCost: unhandled function %q, target %v, args %v", function, target, args))

‎staging/src/k8s.io/apiserver/pkg/cel/library/cost_test.go‎

Lines changed: 44 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,16 @@ func TestURLsCost(t *testing.T) {
206206
expectEsimatedCost: checker.CostEstimate{Min: 4, Max: 4},
207207
expectRuntimeCost: 4,
208208
},
209+
{
210+
ops: []string{" == url('https:://kubernetes.io/')"},
211+
expectEsimatedCost: checker.CostEstimate{Min: 7, Max: 9},
212+
expectRuntimeCost: 7,
213+
},
214+
{
215+
ops: []string{" == url('http://x.b')"},
216+
expectEsimatedCost: checker.CostEstimate{Min: 5, Max: 5},
217+
expectRuntimeCost: 5,
218+
},
209219
}
210220

211221
for _, tc := range cases {
@@ -245,6 +255,14 @@ func TestIPCost(t *testing.T) {
245255
},
246256
expectRuntimeCost: func(c uint64) uint64 { return c + 1 },
247257
},
258+
{
259+
ops: []string{" == ip('192.168.0.1')"},
260+
// For most other operations, the cost is expected to be the base + 1.
261+
expectEsimatedCost: func(c checker.CostEstimate) checker.CostEstimate {
262+
return c.Add(ipv4BaseEstimatedCost).Add(checker.CostEstimate{Min: 1, Max: 1})
263+
},
264+
expectRuntimeCost: func(c uint64) uint64 { return c + ipv4BaseRuntimeCost + 1 },
265+
},
248266
}
249267

250268
for _, tc := range testCases {
@@ -320,6 +338,14 @@ func TestCIDRCost(t *testing.T) {
320338
},
321339
expectRuntimeCost: func(c uint64) uint64 { return c + 1 },
322340
},
341+
{
342+
ops: []string{" == cidr('2001:db8::/32')"},
343+
// For most other operations, the cost is expected to be the base + 1.
344+
expectEsimatedCost: func(c checker.CostEstimate) checker.CostEstimate {
345+
return c.Add(ipv6BaseEstimatedCost).Add(checker.CostEstimate{Min: 1, Max: 1})
346+
},
347+
expectRuntimeCost: func(c uint64) uint64 { return c + ipv6BaseRuntimeCost + 1 },
348+
},
323349
}
324350

325351
//nolint:gocritic
@@ -708,19 +734,19 @@ func TestQuantityCost(t *testing.T) {
708734
{
709735
name: "equality_reflexivity",
710736
expr: `quantity("200M") == quantity("200M")`,
711-
expectEstimatedCost: checker.CostEstimate{Min: 3, Max: 1844674407370955266},
737+
expectEstimatedCost: checker.CostEstimate{Min: 3, Max: 3},
712738
expectRuntimeCost: 3,
713739
},
714740
{
715741
name: "equality_symmetry",
716742
expr: `quantity("200M") == quantity("0.2G") && quantity("0.2G") == quantity("200M")`,
717-
expectEstimatedCost: checker.CostEstimate{Min: 3, Max: 3689348814741910532},
743+
expectEstimatedCost: checker.CostEstimate{Min: 3, Max: 6},
718744
expectRuntimeCost: 6,
719745
},
720746
{
721747
name: "equality_transitivity",
722748
expr: `quantity("2M") == quantity("0.002G") && quantity("2000k") == quantity("2M") && quantity("0.002G") == quantity("2000k")`,
723-
expectEstimatedCost: checker.CostEstimate{Min: 3, Max: 5534023222112865798},
749+
expectEstimatedCost: checker.CostEstimate{Min: 3, Max: 9},
724750
expectRuntimeCost: 9,
725751
},
726752
{
@@ -744,19 +770,19 @@ func TestQuantityCost(t *testing.T) {
744770
{
745771
name: "add_quantity",
746772
expr: `quantity("50k").add(quantity("20")) == quantity("50.02k")`,
747-
expectEstimatedCost: checker.CostEstimate{Min: 5, Max: 1844674407370955268},
773+
expectEstimatedCost: checker.CostEstimate{Min: 5, Max: 5},
748774
expectRuntimeCost: 5,
749775
},
750776
{
751777
name: "sub_quantity",
752778
expr: `quantity("50k").sub(quantity("20")) == quantity("49.98k")`,
753-
expectEstimatedCost: checker.CostEstimate{Min: 5, Max: 1844674407370955268},
779+
expectEstimatedCost: checker.CostEstimate{Min: 5, Max: 5},
754780
expectRuntimeCost: 5,
755781
},
756782
{
757783
name: "sub_int",
758784
expr: `quantity("50k").sub(20) == quantity("49980")`,
759-
expectEstimatedCost: checker.CostEstimate{Min: 4, Max: 1844674407370955267},
785+
expectEstimatedCost: checker.CostEstimate{Min: 4, Max: 4},
760786
expectRuntimeCost: 4,
761787
},
762788
{
@@ -825,6 +851,18 @@ func TestNameFormatCost(t *testing.T) {
825851
expectEstimatedCost: checker.CostEstimate{Min: 34, Max: 34},
826852
expectRuntimeCost: 10,
827853
},
854+
{
855+
name: "format.dns1123label.validate",
856+
expr: `format.named("dns1123Label").value().validate("my-name")`,
857+
expectEstimatedCost: checker.CostEstimate{Min: 34, Max: 34},
858+
expectRuntimeCost: 10,
859+
},
860+
{
861+
name: "format.dns1123label.validate",
862+
expr: `format.named("dns1123Label").value() == format.named("dns1123Label").value()`,
863+
expectEstimatedCost: checker.CostEstimate{Min: 5, Max: 11},
864+
expectRuntimeCost: 5,
865+
},
828866
}
829867

830868
for _, tc := range cases {

‎staging/src/k8s.io/apiserver/pkg/cel/library/format_test.go‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@ import (
2222

2323
"github.com/google/cel-go/common/types"
2424
"github.com/google/cel-go/common/types/ref"
25+
26+
"k8s.io/apiserver/pkg/cel"
2527
"k8s.io/apiserver/pkg/cel/library"
2628
)
2729

@@ -228,3 +230,11 @@ func TestFormat(t *testing.T) {
228230
})
229231
}
230232
}
233+
234+
func TestSizeLimit(t *testing.T) {
235+
for name := range library.ConstantFormats {
236+
if len(name) > cel.MaxFormatSize {
237+
t.Fatalf("All formats must be <= %d chars in length", cel.MaxFormatSize)
238+
}
239+
}
240+
}

‎staging/src/k8s.io/apiserver/pkg/cel/limits.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,5 +48,7 @@ const (
4848
// MinNumberSize is the length of literal 0
4949
MinNumberSize = 1
5050

51+
// MaxFormatSize is the maximum size we allow for format strings
52+
MaxFormatSize = 64
5153
MaxNameFormatRegexSize = 128
5254
)

0 commit comments

Comments
 (0)