Migrate google/btree to k8s util #336
Conversation
k8s-ci-robot
added
the
kind/cleanup
k8s-ci-robot
added
the
cncf-cla: yes
|
Welcome @pjsharath28! |
k8s-ci-robot
added
the
size/XXL
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: pjsharath28 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
btree/btree_generic.go
Outdated
| // In Go 1.18 and beyond, a BTreeG generic is created, and BTree is a specific | ||
| // instantiation of that generic for the Item interface, with a backwards- | ||
| // compatible API. Before go1.18, generics are not supported, | ||
| // and BTree is just an implementation based around the Item interface. | ||
|
|
There was a problem hiding this comment.
This isn’t relevant and should be removed.
btree/btree_generic.go
Outdated
| // There are two implementations; those suffixed with 'G' are generics, usable | ||
| // for any type, and require a passed-in "less" function to define their ordering. | ||
| // Those without this prefix are specific to the 'Item' interface, and use | ||
| // its 'Less' function for ordering. |
There was a problem hiding this comment.
k/k only uses the generic implementation, do we need to keep both in utils?
There was a problem hiding this comment.
Ideally not needed, I removed the non-generic implementation
pjsharath28
force-pushed
the
migrate-btree-to-k8s-util
branch
from
d458954 to
af10f52
Compare
|
/retest |
|
@pjsharath28: Cannot trigger testing until a trusted user reviews the PR and leaves an DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
btree/btree.go
Outdated
| // BTree has its own FreeList, but multiple BTrees can share the same | ||
| // FreeList, in particular when they're created with Clone. | ||
| // Two Btrees using the same freelist are safe for concurrent write access. | ||
| type FreeListG[T any] struct { |
There was a problem hiding this comment.
Since we’re only keeping the generic variant, the G suffix isn’t necessary; please remove it throughout.
btree/btree.go
Outdated
| type ItemIterator[T any] func(item T) bool | ||
|
|
||
| // Ordered represents the set of types for which the '<' operator work. | ||
| type Ordered interface { |
There was a problem hiding this comment.
Ah, I missed this previously, sorry — please use cmp.Ordered instead.
There was a problem hiding this comment.
Good catch, thanks
I've resolved it as well
5 participants
What type of PR is this?
/kind cleanup
What this PR does / why we need it:
The
github.com/google/btreelibrary used by k8s and etcd has been archived and is no longer maintained. This raises long-term concerns around security fixes and CVEs.We discussed multiple options to address this in the issue. Given that we only rely on a single source file from the library, we concluded that copying the code into
k8s.io/utilsis the simplest option. This avoids introducing or maintaining a separate fork while preserving identical behaviour.Which issue(s) this PR is related to:
Discussion issue: etcd-io/etcd#20991
Special notes for your reviewer:
Upstream Google attribution is preserved and the file header follows Kubernetes conventions while remaining Apache-2.0 compliant.
Does this PR introduce a user-facing change?
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: