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

fix protobuf vulnerability #245

Merged
merged 1 commit into from
Mar 24, 2021

Conversation

cdlliuy
Copy link
Contributor

@cdlliuy cdlliuy commented Mar 17, 2021

To fix the golang/protobuf vulnerability

Given it is not explicitly required, I tend to use replace to pin the correct version.

@cdlliuy cdlliuy force-pushed the dev_updateprotobufversion branch from 9a153a2 to 4a2999e Compare March 17, 2021 07:25
@codecov
Copy link

codecov bot commented Mar 17, 2021

Codecov Report

Merging #245 (5b8282d) into master (6910664) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #245   +/-   ##
=======================================
  Coverage   92.07%   92.07%           
=======================================
  Files           6        6           
  Lines         757      757           
=======================================
  Hits          697      697           
  Misses         39       39           
  Partials       21       21           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6910664...5b8282d. Read the comment docs.

Copy link
Member

@mhenke1 mhenke1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

go.mod Outdated
@@ -24,3 +24,5 @@ require (
k8s.io/client-go v0.17.17
sigs.k8s.io/controller-runtime v0.5.0
)

replace github.com/gogo/protobuf => github.com/gogo/protobuf v1.3.2
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's pull this in as an indirect dependency. Replaces can have issues when upgrading deps later on, so I'd prefer to avoid them.

Running this command pulled in the new version to go.mod for me:

go get github.com/gogo/protobuf@latest

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JohnStarich , fixed per comment, Also, the softlayer-go package is updated along with the go mod tidy cmd

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JohnStarich , can you review again?

@cdlliuy cdlliuy force-pushed the dev_updateprotobufversion branch from 0244c2a to b265b1c Compare March 18, 2021 01:11
Copy link
Member

@JohnStarich JohnStarich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like the dependency scan is now failing. I think if you add back the indirect deps for etcd and web socket, that should fix it.

@cdlliuy cdlliuy force-pushed the dev_updateprotobufversion branch from b265b1c to 5b8282d Compare March 24, 2021 07:35
@cdlliuy
Copy link
Contributor Author

cdlliuy commented Mar 24, 2021

@JohnStarich thanks , updated :-)

@JohnStarich JohnStarich merged commit 471da04 into IBM:master Mar 24, 2021
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.

3 participants