Migrate from satori/go.uuid to gofrs/uuid #6554
Conversation
k8s-ci-robot
added
the
kind/cleanup
k8s-ci-robot
added
cncf-cla: yes
k8s-ci-robot
added
do-not-merge/work-in-progress
k8s-ci-robot
added
size/S
k8s-ci-robot
added
size/XL
k8s-ci-robot
removed
the
do-not-merge/work-in-progress
cluster-autoscaler/go.mod
Outdated
| @@ -263,3 +262,5 @@ replace k8s.io/dynamic-resource-allocation => k8s.io/dynamic-resource-allocation | |||
| replace k8s.io/kms => k8s.io/kms v0.29.0-alpha.3 | |||
|
|
|||
| replace k8s.io/endpointslice => k8s.io/endpointslice v0.29.0-alpha.3 | |||
|
|
|||
| replace github.com/satori/go.uuid => github.com/gofrs/uuid/v5 v5.0.0 | |||
There was a problem hiding this comment.
if you are removing the usage of /satori, why do we need the replace?
There was a problem hiding this comment.
satori replace is deleted, thank you
| @@ -22,7 +22,7 @@ import ( | |||
| "encoding/hex" | |||
| "encoding/json" | |||
| "fmt" | |||
| "github.com/satori/go.uuid" | |||
| "github.com/gofrs/uuid" | |||
There was a problem hiding this comment.
should we be using github.com/google/uuid instead? For consistency with kubernetes/kubernetes.
Are there any advantages to either?
There was a problem hiding this comment.
make sense as we already have github.com/google/uuid in dependency, I've replaced gofrs/uuid with github.com/google/uuid using the latest version
k8s-ci-robot
added
size/XXL
|
PR needs rebase. 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/test-infra repository. |
k8s-ci-robot
added
the
needs-rebase
cluster-autoscaler/go.mod
Outdated
| @@ -1,6 +1,6 @@ | |||
| module k8s.io/autoscaler/cluster-autoscaler | |||
|
|
|||
| go 1.21 | |||
| go 1.21.6 | |||
There was a problem hiding this comment.
Why is the required Go version bumped? It doesn't seem like github.com/google/uuid v1.6.0 requires anything higher than go1.21.
There was a problem hiding this comment.
@towca thanks, I returned the previous version 1.21
|
/assign @towca |
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: oksanabaza, towca The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
| func GetUUIDV4() (uuidHex string) { | ||
| uuidV4 := uuid.NewV4() | ||
| uuidHex = hex.EncodeToString(uuidV4.Bytes()) | ||
| uuidV4 := uuid.New() | ||
| binaryUUID, _ := uuidV4.MarshalBinary() | ||
| uuidHex = hex.EncodeToString(binaryUUID) | ||
| return | ||
| } |
There was a problem hiding this comment.
This whole function can be removed in favor of uuid.NewString
|
@oksanabaza any plans to apply the same change for previous versions, 1.27, 1.28 and 1.29? The CVEs still exist for those versions. Thank you! |
What type of PR is this?
/kind cleanup
What this PR does / why we need it:
Replace satori/go.uuid with gofrs/uuid to remove a CVE.
Which issue(s) this PR fixes:
Fixes # High CVEs
Special notes for your reviewer:
Does this PR introduce a user-facing change?
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: