Skip to content
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

Closed
wants to merge 1 commit into from

Conversation

yankay
Copy link
Contributor

@yankay yankay commented Mar 14, 2022

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

@serathius
Copy link
Member

Looks like pkg package doesn't build

% (cd pkg && 'go' 'build' './...')
stderr: go: downloading github.com/creack/pty v1.1.11
Error: stderr: ../../../../go/pkg/mod/google.golang.org/grpc@v1.38.0/credentials/credentials.go:31:2: missing go.sum entry for module providing package github.com/golang/protobuf/proto (imported by google.golang.org/grpc/test/grpc_testing); to add:
stderr: go get google.golang.org/grpc/test/grpc_testing@v1.38.0
Error: stderr: ../../../../go/pkg/mod/google.golang.org/grpc@v1.38.0/internal/binarylog/method_logger.go:28:2: missing go.sum entry for module providing package github.com/golang/protobuf/ptypes (imported by google.golang.org/grpc/internal/binarylog); to add:
stderr: go get google.golang.org/grpc/internal/binarylog@v1.38.0
Error: stderr: ../../../../go/pkg/mod/google.golang.org/grpc@v1.38.0/internal/transport/controlbuf.go:30:2: missing go.sum entry for module providing package golang.org/x/net/http2 (imported by google.golang.org/grpc/internal/transport); to add:
stderr: go get google.golang.org/grpc/internal/transport@v1.38.0
Error: stderr: ../../../../go/pkg/mod/google.golang.org/grpc@v1.38.0/internal/transport/controlbuf.go:31:2: missing go.sum entry for module providing package golang.org/x/net/http2/hpack (imported by google.golang.org/grpc/internal/transport); to add:
stderr: go get google.golang.org/grpc/internal/transport@v1.38.0
Error: stderr: ../../../../go/pkg/mod/google.golang.org/grpc@v1.38.0/server.go:36:2: missing go.sum entry for module providing package golang.org/x/net/trace (imported by google.golang.org/grpc); to add:
stderr: go get google.golang.org/grpc@v1.38.0
Error: stderr: ../../../../go/pkg/mod/google.golang.org/grpc@v1.38.0/internal/channelz/types_linux.go:26:2: missing go.sum entry for module providing package golang.org/x/sys/unix (imported by go.etcd.io/etcd/client/pkg/v3/fileutil); to add:
stderr: go get go.etcd.io/etcd/client/pkg/v3/fileutil@v3.5.2
Error: stderr: ../../../../go/pkg/mod/google.golang.org/grpc@v1.38.0/status/status.go:34:2: missing go.sum entry for module providing package google.golang.org/genproto/googleapis/rpc/status (imported by google.golang.org/grpc/internal/transport); to add:
stderr: go get google.golang.org/grpc/internal/transport@v1.38.0

@yankay yankay force-pushed the backport_13571_to_3_5 branch 3 times, most recently from 34e09c9 to 00b9e89 Compare March 14, 2022 13:55
@yankay
Copy link
Contributor Author

yankay commented Mar 14, 2022

Looks like pkg package doesn't build

% (cd pkg && 'go' 'build' './...')
stderr: go: downloading github.com/creack/pty v1.1.11
Error: stderr: ../../../../go/pkg/mod/google.golang.org/grpc@v1.38.0/credentials/credentials.go:31:2: missing go.sum entry for module providing package github.com/golang/protobuf/proto (imported by google.golang.org/grpc/test/grpc_testing); to add:
stderr: go get google.golang.org/grpc/test/grpc_testing@v1.38.0
Error: stderr: ../../../../go/pkg/mod/google.golang.org/grpc@v1.38.0/internal/binarylog/method_logger.go:28:2: missing go.sum entry for module providing package github.com/golang/protobuf/ptypes (imported by google.golang.org/grpc/internal/binarylog); to add:
stderr: go get google.golang.org/grpc/internal/binarylog@v1.38.0
Error: stderr: ../../../../go/pkg/mod/google.golang.org/grpc@v1.38.0/internal/transport/controlbuf.go:30:2: missing go.sum entry for module providing package golang.org/x/net/http2 (imported by google.golang.org/grpc/internal/transport); to add:
stderr: go get google.golang.org/grpc/internal/transport@v1.38.0
Error: stderr: ../../../../go/pkg/mod/google.golang.org/grpc@v1.38.0/internal/transport/controlbuf.go:31:2: missing go.sum entry for module providing package golang.org/x/net/http2/hpack (imported by google.golang.org/grpc/internal/transport); to add:
stderr: go get google.golang.org/grpc/internal/transport@v1.38.0
Error: stderr: ../../../../go/pkg/mod/google.golang.org/grpc@v1.38.0/server.go:36:2: missing go.sum entry for module providing package golang.org/x/net/trace (imported by google.golang.org/grpc); to add:
stderr: go get google.golang.org/grpc@v1.38.0
Error: stderr: ../../../../go/pkg/mod/google.golang.org/grpc@v1.38.0/internal/channelz/types_linux.go:26:2: missing go.sum entry for module providing package golang.org/x/sys/unix (imported by go.etcd.io/etcd/client/pkg/v3/fileutil); to add:
stderr: go get go.etcd.io/etcd/client/pkg/v3/fileutil@v3.5.2
Error: stderr: ../../../../go/pkg/mod/google.golang.org/grpc@v1.38.0/status/status.go:34:2: missing go.sum entry for module providing package google.golang.org/genproto/googleapis/rpc/status (imported by google.golang.org/grpc/internal/transport); to add:
stderr: go get google.golang.org/grpc/internal/transport@v1.38.0

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
Copy link
Member

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?

Copy link
Member

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.

Copy link
Contributor Author

@yankay yankay Mar 15, 2022

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.

Got it.
Thanks. I'd be glad to rollback the change.

企业微信截图_16473092132596

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 ?

Copy link
Member

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

Copy link
Contributor Author

@yankay yankay Mar 15, 2022

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?

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

Successfully merging this pull request may close these issues.

None yet

3 participants