Merge pull request #119810 from jpbetz/fix-cost1_26

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

go.mod

@@ -172,7 +172,7 @@ require (
 	github.com/gofrs/uuid v4.0.0+incompatible // indirect
 	github.com/golang-jwt/jwt/v4 v4.2.0 // indirect
 	github.com/google/btree v1.0.1 // indirect
-	github.com/google/cel-go v0.12.6 // indirect
+	github.com/google/cel-go v0.12.7 // indirect
 	github.com/google/pprof v0.0.0-20210720184732-4bb14d4b1be1 // indirect
 	github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 // indirect
 	github.com/googleapis/gax-go/v2 v2.1.1 // indirect

go.sum

@@ -362,8 +362,8 @@ 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/cadvisor v0.46.1 h1:zCOqaAOS4LvAEldLFc0KrbsDX/AFIDRI1X2WtJSwgmg=
 github.com/google/cadvisor v0.46.1/go.mod h1:YnCDnR8amaS0HoMEjheOI0TMPzFKCBLc30mciLEjwGI=
-github.com/google/cel-go v0.12.6 h1:kjeKudqV0OygrAqA9fX6J55S8gj+Jre2tckIm5RoG4M=
-github.com/google/cel-go v0.12.6/go.mod h1:Jk7ljRzLBhkmiAwBoUxB1sZSCVBAzkqPF25olK/iRDw=
+github.com/google/cel-go v0.12.7 h1:jM6p55R0MKBg79hZjn1zs2OlrywZ1Vk00rxVvad1/O0=
+github.com/google/cel-go v0.12.7/go.mod h1:Jk7ljRzLBhkmiAwBoUxB1sZSCVBAzkqPF25olK/iRDw=
 github.com/google/gnostic v0.5.7-v3refs h1:FhTMOKj2VhjpouxvWJAV1TL304uMlb9zcDqkl6cEI54=
 github.com/google/gnostic v0.5.7-v3refs/go.mod h1:73MKFl6jIHelAJNaBGFzt3SPtZULs9dYrGFt8OiIsHQ=
 github.com/google/go-cmp v0.2.0/go.mod h1:oXzfMopK8JAjlY9xF4vHSVASa0yLyX7SntLO5aqRK0M=

staging/src/k8s.io/apiextensions-apiserver/go.mod

@@ -7,7 +7,7 @@ go 1.19
 require (
 	github.com/emicklei/go-restful/v3 v3.9.0
 	github.com/gogo/protobuf v1.3.2
-	github.com/google/cel-go v0.12.6
+	github.com/google/cel-go v0.12.7
 	github.com/google/gnostic v0.5.7-v3refs
 	github.com/google/go-cmp v0.5.9
 	github.com/google/gofuzz v1.1.0

staging/src/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/validation/validation_test.go

@@ -8558,6 +8558,32 @@ func TestValidateCustomResourceDefinitionValidation(t *testing.T) {
 				invalid("spec.validation.openAPIV3Schema.properties[f@2].x-kubernetes-validations[0].rule"),
 			},
 		},
+		{
+			name: "x-kubernetes-validations rule with lowerAscii check should be within estimated cost limit",
+			opts: validationOptions{requireStructuralSchema: true},
+			input: apiextensions.CustomResourceValidation{
+				OpenAPIV3Schema: &apiextensions.JSONSchemaProps{
+					Type: "object",
+					Properties: map[string]apiextensions.JSONSchemaProps{
+						"f": {
+							Type:     "array",
+							MaxItems: pointer.Int64(5),
+							Items: &apiextensions.JSONSchemaPropsOrArray{
+								Schema: &apiextensions.JSONSchemaProps{
+									Type:      "string",
+									MaxLength: pointer.Int64(5),
+								},
+							},
+							XValidations: apiextensions.ValidationRules{
+								{
+									Rule: "self.all(x, self.exists_one(y, x.lowerAscii() == y.lowerAscii()))",
+								},
+							},
+						},
+					},
+				},
+			},
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {

staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/celcoststability_test.go

@@ -114,6 +114,7 @@ func TestCelCostStability(t *testing.T) {
 				"self.val1.substring(4, 10).trim() == 'takes'":     6,
 				"self.val1.upperAscii() == 'ROOK TAKES 👑'":         6,
 				"self.val1.lowerAscii() == 'rook takes 👑'":         6,
+				"self.val1.lowerAscii() == self.val1.lowerAscii()": 10,
 			},
 		},
 		{name: "escaped strings",

staging/src/k8s.io/apiextensions-apiserver/pkg/apiserver/schema/cel/compilation_test.go

@@ -1557,17 +1557,19 @@ func TestCostEstimation(t *testing.T) {
 			name: "extended library replace",
 			schemaGenerator: func(max *int64) *schema.Structural {
 				strType := withMaxLength(primitiveType("string", ""), max)
+				beforeLen := int64(2)
+				afterLen := int64(4)
 				objType := objectType(map[string]schema.Structural{
 					"str":    strType,
-					"before": strType,
-					"after":  strType,
+					"before": withMaxLength(primitiveType("string", ""), &beforeLen),
+					"after":  withMaxLength(primitiveType("string", ""), &afterLen),
 				})
 				objType = withRule(objType, "self.str.replace(self.before, self.after) == 'does not matter'")
 				return &objType
 			},
-			expectedCalcCost: 629154,
-			setMaxElements:   10,
-			expectedSetCost:  16,
+			expectedCalcCost: 629154, // cost is based on the result size of the replace() call
+			setMaxElements:   4,
+			expectedSetCost:  12,
 		},
 		{
 			name: "extended library split",

staging/src/k8s.io/apiserver/go.mod

@@ -12,7 +12,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.12.6
+	github.com/google/cel-go v0.12.7
 	github.com/google/gnostic v0.5.7-v3refs
 	github.com/google/go-cmp v0.5.9
 	github.com/google/gofuzz v1.1.0

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

@@ -111,13 +111,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 {

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

@@ -224,18 +224,42 @@ 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:               "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)",
@@ -342,6 +366,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 {
 }
 

staging/src/k8s.io/cloud-provider/go.mod

@@ -43,7 +43,7 @@ require (
 	github.com/gogo/protobuf v1.3.2 // indirect
 	github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect
 	github.com/golang/protobuf v1.5.2 // indirect
-	github.com/google/cel-go v0.12.6 // indirect
+	github.com/google/cel-go v0.12.7 // indirect
 	github.com/google/gnostic v0.5.7-v3refs // indirect
 	github.com/google/gofuzz v1.1.0 // indirect
 	github.com/google/uuid v1.3.0 // indirect

staging/src/k8s.io/controller-manager/go.mod

@@ -39,7 +39,7 @@ require (
 	github.com/gogo/protobuf v1.3.2 // indirect
 	github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect
 	github.com/golang/protobuf v1.5.2 // indirect
-	github.com/google/cel-go v0.12.6 // indirect
+	github.com/google/cel-go v0.12.7 // indirect
 	github.com/google/gnostic v0.5.7-v3refs // indirect
 	github.com/google/go-cmp v0.5.9 // indirect
 	github.com/google/gofuzz v1.1.0 // indirect

staging/src/k8s.io/kube-aggregator/go.mod

@@ -45,7 +45,7 @@ require (
 	github.com/go-openapi/swag v0.19.14 // indirect
 	github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect
 	github.com/golang/protobuf v1.5.2 // indirect
-	github.com/google/cel-go v0.12.6 // indirect
+	github.com/google/cel-go v0.12.7 // indirect
 	github.com/google/gnostic v0.5.7-v3refs // indirect
 	github.com/google/uuid v1.3.0 // indirect
 	github.com/grpc-ecosystem/go-grpc-prometheus v1.2.0 // indirect