-
Notifications
You must be signed in to change notification settings - Fork 9.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
backport 3.5: #13571 Update Cobra version to 1.2.1 to fix CVE-2020-26160 #13797
Conversation
Looks like
|
34e09c9
to
00b9e89
Compare
…2020-26160 Signed-off-by: Kay Yan <kay.yan@daocloud.io>
00b9e89
to
3ae6b50
Compare
HI, @serathius the CI issue has been fixed :-) Would you please review it. |
@@ -15,9 +14,14 @@ require ( | |||
) | |||
|
|||
replace ( | |||
go.etcd.io/etcd => ./FORBIDDEN_DEPENDENCY | |||
go.etcd.io/etcd/api/v3 => ./FORBIDDEN_DEPENDENCY | |||
go.etcd.io/etcd/api/v3 => ../api |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This adds api
as dependency to pkg
. Guess this was unintentionally backported?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
go.etcd.io/etcd/pkg/v3
doesn't need to depend on go.etcd.io/etcd/api/v3
at all. So please rollback the change, and please also make the same change on the main branch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
go.etcd.io/etcd/pkg/v3
doesn't need to depend ongo.etcd.io/etcd/api/v3
at all. So please rollback the change, and please also make the same change on the main branch.
Got it.
Thanks. I'd be glad to rollback the change.
The cobra dependents on etcd/api, to reslove this , the api has been moved to the go.mod requirements.
Whould you please tell us what would happen if the go.etcd.io/etcd/pkg/v3 depend on go.etcd.io/etcd/api/v3 ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Firstly, I believe you will NOT see this error anymore if you bump github.com/spf13/cobra
to a version >= 1.3.0
, which bumped spf13/viper
to 1.10.0
. spf13/viper
doesn't depend on bketelsen/crypt@v0.0.4
starting from 1.9.0
. spf13/cobra@1.3.0
depends on spf13/viper@1.10.0
, and spf13/cobra@1.4.0
even removed the dependency on spf13/viper
.
Secondly, etcd already contains lots of packages and dependency relationship in-between, we shouldn't introduce unnecessary dependencies without good reason.
So I suggest to bump spf13/cobra
to 1.3.0
or 1.4.0
, and rollback the change on go.etcd.io/etcd/api/v3 => ./FORBIDDEN_DEPENDENCY
. I would suggest to make the change on the main branch firstly, and cherry-picked to 3.5 afterwards. cc @serathius @ptabor @spzala
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you very much. @ahrtr @serathius
There is a PR #13802 to rollback and upgrade cobra.
If it's OK , we can cherry-pick it to 3.5 later.
Would you please review it?
backport 3.5: #13571 Update Cobra version to 1.2.1 to fix CVE-2020-26160
The kube apiserver and many others require the etcd 3.5.x.
It's good the back port the CVE fix to the 3.5.x branch.
Signed-off-by: Kay Yan kay.yan@daocloud.io