Skip to content

Cost library for ext/network#1345

Open
TristonianJones wants to merge 1 commit into
cel-expr:masterfrom
TristonianJones:network-cost
Open

Cost library for ext/network#1345
TristonianJones wants to merge 1 commit into
cel-expr:masterfrom
TristonianJones:network-cost

Conversation

@TristonianJones

@TristonianJones TristonianJones commented Jun 16, 2026

Copy link
Copy Markdown
Collaborator

Adds cost computations and cost tests from K8s:

https://github.com/kubernetes/kubernetes/blob/808a553dc88f0343a895e4bbf9917107500b6488/staging/src/k8s.io/apiserver/pkg/cel/library/cost_test.go#L314

There is one small nit in that the equality costing in CEL is slightly
richer than the one used in K8s and the cost estimates have a range
that's slightly different from the one computed by K8s which doesn't
look at the arguments to the function, but rather returns a unit cost
of one.

@TristonianJones

Copy link
Copy Markdown
Collaborator Author

FYI @jpbetz

@jpbetz

jpbetz commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

@lalitc375 would you check for compatibility with our downstream costs?

@impartialstate

Copy link
Copy Markdown

@lalitc375 the tests are ported from K8s. The only difference I noticed is that the cost for equality is overridden in K8s and will produce a slightly smaller cost

@lalitc375

Copy link
Copy Markdown

K8s overrides == for IP/CIDR to a flat {1,1} estimate and runtime 1, regardless of v4 or v6. This PR doesn't override ==, so it falls through to cel-go's default Equals path: estimate {1, ceil(min(lhsMax, rhsMax) * 0.1)} and runtime ceil(min(lhsSize, rhsSize) * 0.1), using the new Size() method that returns 4 for v4 and 16 for v6.

Net effect: estimate Max goes from 1 to 2 for all IP/CIDR equality; runtime stays 1 for v4 but becomes 2 for v6.

@TristonianJones

Copy link
Copy Markdown
Collaborator Author

If it's at all possible, I would prefer not to change the way equality estimates cost. If it's a breaking change, we can override it within the networking lib and mark it as something we'll remove in future libraries ... I'm not sure how best you want to approach this, to be honest.

@jpbetz

jpbetz commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

I think we should just downstream the breaking change into k8s. TBH, I don't think anyone will notice given that it's only for equality for these relatively new types.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants