-
Notifications
You must be signed in to change notification settings - Fork 3
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
[RELEASE-1.9][CVE-2023-44487] Update GRPC to 1.58.3 #109
Conversation
skonto
commented
Oct 24, 2023
- We use grpc for probes /xds so we need to bump that.
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: skonto 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 |
@@ -5,18 +5,18 @@ module knative.dev/net-kourier | |||
go 1.18 |
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.
Hm we have quite some jumps of deps here. Is this the minimal amount we can do, so that the replace works?
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.
Yes I only did a go mod edit.
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.
Maybe bumping to 1.56.3 is smaller let me see.
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.
It seems not:
git diff openshift/release-1.9 --stat | wc -l
815
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.
@nak3 before we merge this, what do you think about the jump?
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.
A little bit concerned for the side effect but I think it is alright.
Alternatively we can patch this change grpc/grpc-go@c40c9ba (without test) to the lib directory, but I think this PR (bumps deps) is safer than that.
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.
@skonto 1.56.3
is also patches. Can we maybe just bump to that, to avoid having a too big jump here? Maybe that does not pull-in a years worth of commits in genproto.
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.
Upstream we have 1.58.3 https://github.com/knative-extensions/net-kourier/blob/release-1.12/go.mod#L17, so I was thinking of aligning stuff. I will do another PR to compare.
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.
Yeah, but why bumping too much in 1.9 when it is not necessary? Just thinking about risk minimization.
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.
Here it is: #112