bump golang.org/grpc to v1.56.3

[sxd]

Bumping golang.org/grpc in light of CVE-2023-44487.

What type of PR is this?

What this PR does / why we need it:

Fix the issues reported by CVE-2023-44487 fixed in release 1.58.3

Does this PR introduce a user-facing change?

NONE

[k8s-ci-robot]

Welcome @sxd!

It looks like this is your first PR to kubernetes/kubernetes 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes/kubernetes has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

[k8s-ci-robot]

Hi @sxd. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

[Jefftree]

/triage accepted
/ok-to-test
/assign @liggitt

[Jefftree]

Please run hack/update-vendor.sh

[sxd]

/retest-required

[liggitt]

huh... I'm happy to see this drop out, but later / current versions of cloud.google.com/go/compute still depend on cloud.google.com/go, so I won't be surprised to see it come back.

The big win will be when cloud.google.com/go/compute/metadata stops depending on cloud.google.com/go/compute, and all the other things we have depending on cloud.google.com/go/compute below start dropping out. I think that will happen early next year along with the genproto cleanup.

[sxd]

Uff sad to hear that, the problem is that with that inside the test fails and this update is required to fix the security issue
@liggitt how do you think we can proceed with this?

[liggitt]

oh, I wouldn't block on this, dropping this here and then adding it back later if we need to is fine

@dims said there's another CVE that needs an even newer grpc - #121338 was open for that ... should we take that one?

[sxd]

I started with the even newer version 1.59.0 but that was a lot of more changes required including s2a-go package which it's also unwanted, if we can go with 1.59.0 and remove that unwanted one it's ok
The version 1.56.3 it's the one that requires less changes in the code and manage to fix the security issue with few changes

[liggitt]

the s2a-go issue in particular was discussed in slack here:

John Howard
The new version of cloud.google.com/go/compute pulls in an "unwanted dependency" s2a-go. Any action we need to do here? Remove from unwanted? Complain to go/compute repo? Don't update go/compute? Running into this in #120241. Can work around it in that PR sort of, though.

Stephen Kitt
... or add it to the status for cloud.google.com/go/compute in unwanted-dependencies.json — the former is already an unwanted module, so adding a cloud dependency to an existing unwanted cloud dependency doesn’t change much in the grand scheme of things IMO

Stephen Kitt
What would be really unwanted would be adding s2a-go as a dependency for a non-unwanted module, or a direct k/k dependency.

liggitt
there were two reasons it was marked as unwanted:

• unstable (due to vestigial content in the readme from when it was under development) that has since been removed - update readme google/s2a-go#120
• a transitive dependency it had on appengine code (cloud dependencies) that has since been removed - remove appengine dialer google/s2a-go#119

the current state of that library means it doesn't pull in objectionable transitive dependencies any more
that said, we don't want more dependencies in general, so we look hard at any additions. in this case, it looks like the only referencer is cloud.google.com/go/compute, which we have a timeline of ~next March for dropping. +1 to Stephen Kitt’s comment about tracking the things that pull in s2a-go and limiting it to cloud deps we have a plan to drop, so this will drop out along with them

[sxd]

I wasn't in that channel, ok so it make sense to restrict the s2a-go dependency to the already existed dependency that as I checked it was only the go/compute thing, probably @dims can target version 1.59.0 on his PR and we can drop this one that actually fix only the gprc because I thought #121338 was only about OTEL :D
Closing this one to let the other one proceed

[liggitt]

reopening, @dims had this feedback:

I have a feeling we should do 121364, cherry-picks to stable branches and then merge the otel one and keep it just forward looking (no backports)

(since the otel code we use is not actually impacted by the CVE being fixed)

[sxd]

oh! well that will make sense since this will not have a big impact and I'm guessing it can be backported easily
How we should proceed?

[liggitt]

/lgtm
/approve

[k8s-ci-robot]

LGTM label has been added.

Git tree hash: 913ba5d646ba6725dce0a85ccadf255e40edc160

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: liggitt, sxd

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:

• OWNERS [liggitt]
• hack/OWNERS [liggitt]
• staging/src/k8s.io/apiextensions-apiserver/OWNERS [liggitt]
• staging/src/k8s.io/apiserver/OWNERS [liggitt]
• staging/src/k8s.io/cloud-provider/OWNERS [liggitt]
• staging/src/k8s.io/component-base/OWNERS [liggitt]
• staging/src/k8s.io/controller-manager/OWNERS [liggitt]
• staging/src/k8s.io/cri-api/OWNERS [liggitt]
• staging/src/k8s.io/dynamic-resource-allocation/OWNERS [liggitt]
• staging/src/k8s.io/endpointslice/OWNERS [liggitt]
• staging/src/k8s.io/kms/OWNERS [liggitt]
• staging/src/k8s.io/kube-aggregator/OWNERS [liggitt]
• staging/src/k8s.io/kube-controller-manager/OWNERS [liggitt]
• staging/src/k8s.io/kube-proxy/OWNERS [liggitt]
• staging/src/k8s.io/kube-scheduler/OWNERS [liggitt]
• staging/src/k8s.io/kubectl/OWNERS [liggitt]
• staging/src/k8s.io/kubelet/OWNERS [liggitt]
• staging/src/k8s.io/legacy-cloud-providers/OWNERS [liggitt]
• staging/src/k8s.io/pod-security-admission/OWNERS [liggitt]
• staging/src/k8s.io/sample-apiserver/OWNERS [liggitt]
• vendor/OWNERS [liggitt]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[sxd]

/retest

[liggitt]

/kind bug

[orsenthil]

This PR bumps the google.golang.org/grpc v1.56.3 forCVE-2023-44487 but the fix is advertised as fixed in release 1.58.3

Shouldn't this be updating to google.golang.org/grpc v1.58.3 ?

[dims]

@orsenthil see https://github.com/grpc/grpc-go/releases/tag/v1.56.3 has the same fix

[jerryhe1999]

Do we have any plan merging this PR to other release versions v1.26-v1.28 rather than master/v1.29 release?

[dims]

1.26/1.27/1.28 are here, 1.25 is out of support starting tomorrow, so won't file one for that:
https://github.com/kubernetes/kubernetes/pulls?q=is%3Apr+is%3Aopen+%22bump+golang.org%2Fgrpc+to+v1.56.3%22