OCPBUGS-79905: Bump google.golang.org/grpc#398
Conversation
WalkthroughThree OpenStack service drivers (Cinder CSI, Manila CSI, and KMS) are updated with identical gRPC unary interceptors that validate request ChangesgRPC path validation across services
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (13 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)Error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions Comment |
|
@sbiradar10: This pull request references Jira Issue OCPBUGS-79905, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. DetailsIn response to this: 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 openshift-eng/jira-lifecycle-plugin repository. |
|
@sbiradar10: An error was encountered searching for bug OCPBUGS-79905 on the Jira server at https://redhat.atlassian.net. No known errors were detected, please see the full error message for details. Full error message.
No response returned: Get "https://redhat.atlassian.net/rest/api/2/issue/OCPBUGS-79905": GET https://redhat.atlassian.net/rest/api/2/issue/OCPBUGS-79905 giving up after 5 attempt(s)
Please contact an administrator to resolve this issue, then request a bug refresh with DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
|
/test e2e-openstack |
1 similar comment
|
/test e2e-openstack |
|
/lgtm |
|
@stephenfin: This PR has been marked as verified by DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: sbiradar10, stephenfin The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/hold |
…y with lower versions where fork can't be used
4a89594 to
76a4166
Compare
|
New changes are detected. LGTM label has been removed. |
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@sbiradar10: This pull request references Jira Issue OCPBUGS-79905, which is invalid:
Comment DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
There was a problem hiding this comment.
🧹 Nitpick comments (3)
pkg/csi/cinder/server.go (1)
28-31: 💤 Low valueImport grouping:
contextshould be with standard library imports.The
contextpackage is part of the Go standard library and should be grouped withnet,os, andsyncat the top of the import block for idiomatic Go style.Suggested fix
import ( + "context" "net" "os" "sync" "google.golang.org/grpc" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" "k8s.io/klog/v2" "github.com/container-storage-interface/spec/lib/go/csi" - - "google.golang.org/grpc/codes" - "google.golang.org/grpc/status" - "context" )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/csi/cinder/server.go` around lines 28 - 31, Move the "context" import into the standard-library import group at the top of the import block so it sits with other stdlib packages (e.g., net/os/sync) instead of the gRPC imports; adjust the import grouping in pkg/csi/cinder/server.go so "context" is listed in the first import group and the grpc packages ("google.golang.org/grpc/codes", "google.golang.org/grpc/status") remain in the third-party group.pkg/kms/server/server.go (1)
16-18: 💤 Low valueMinor: Remove extra blank line before new imports.
Same grouping suggestion as the other files - place these imports directly with the
google.golang.org/grpcimport at line 10.Suggested fix
"google.golang.org/grpc" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" gcfg "gopkg.in/gcfg.v1" - - "google.golang.org/grpc/codes" - "google.golang.org/grpc/status"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/kms/server/server.go` around lines 16 - 18, Remove the extra blank line in the import block so the grpc subpackages "google.golang.org/grpc/codes" and "google.golang.org/grpc/status" are grouped with the existing "google.golang.org/grpc" import; update the import block containing those symbols to be contiguous (no blank line) to match the import grouping used elsewhere.pkg/csi/manila/driver.go (1)
36-38: 💤 Low valueMinor: Remove extra blank line before new imports.
The blank line at line 36 before the new imports is unnecessary. The
grpc/codesandgrpc/statusimports should be grouped directly with the othergoogle.golang.org/grpcimport.Suggested fix
"google.golang.org/grpc" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" "k8s.io/cloud-provider-openstack/pkg/csi/manila/csiclient" - - "google.golang.org/grpc/codes" - "google.golang.org/grpc/status"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/csi/manila/driver.go` around lines 36 - 38, Remove the stray blank line in the import block so the new imports "google.golang.org/grpc/codes" and "google.golang.org/grpc/status" are grouped with the existing "google.golang.org/grpc" import; edit the import block in pkg/csi/manila/driver.go (the file-level import section) to place those two imports directly alongside the other google.golang.org/grpc import entries without extra spacing.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@pkg/csi/cinder/server.go`:
- Around line 28-31: Move the "context" import into the standard-library import
group at the top of the import block so it sits with other stdlib packages
(e.g., net/os/sync) instead of the gRPC imports; adjust the import grouping in
pkg/csi/cinder/server.go so "context" is listed in the first import group and
the grpc packages ("google.golang.org/grpc/codes",
"google.golang.org/grpc/status") remain in the third-party group.
In `@pkg/csi/manila/driver.go`:
- Around line 36-38: Remove the stray blank line in the import block so the new
imports "google.golang.org/grpc/codes" and "google.golang.org/grpc/status" are
grouped with the existing "google.golang.org/grpc" import; edit the import block
in pkg/csi/manila/driver.go (the file-level import section) to place those two
imports directly alongside the other google.golang.org/grpc import entries
without extra spacing.
In `@pkg/kms/server/server.go`:
- Around line 16-18: Remove the extra blank line in the import block so the grpc
subpackages "google.golang.org/grpc/codes" and "google.golang.org/grpc/status"
are grouped with the existing "google.golang.org/grpc" import; update the import
block containing those symbols to be contiguous (no blank line) to match the
import grouping used elsewhere.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 3e81c164-ac41-4261-b92a-531cb4293a37
📒 Files selected for processing (3)
pkg/csi/cinder/server.gopkg/csi/manila/driver.gopkg/kms/server/server.go
|
/test e2e-openstack-csi-manila |
|
/test e2e-openstack-csi-cinder |
|
@sbiradar10: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions 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-sigs/prow repository. I understand the commands that are listed here. |
|
/verified by CI |
|
@rh-akhatavk: This PR has been marked as verified by DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
|
/unhold |
|
/jira refresh |
|
@kunalmemane: This pull request references Jira Issue OCPBUGS-79905, which is valid. The bug has been moved to the POST state. 7 validation(s) were run on this bug
No GitHub users were found matching the public email listed for the QA contact in Jira (ocp-sustaining-admins@redhat.com), skipping review request. DetailsIn response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
|
/hold |
https://redhat.atlassian.net/browse/OCPBUGS-79905
Bump google.golang.org/grpc to v1.67.3-sec.1 using below command:
go mod edit -replace google.golang.org/grpc=github.com/openshift-sustaining/grpc-go@v1.64.1-sec.1
Summary by CodeRabbit