STOR-2997: Minimal implementation of GCP PD CSI driver operator#576
Open
mpatlasov wants to merge 2 commits into
Open
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@mpatlasov: This pull request references STOR-2997 which is a valid jira issue. 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. |
Contributor
|
@mpatlasov: GitHub didn't allow me to request PR reviews from the following users: openshift/storage. Note that only openshift members and repo collaborators can review this PR, and authors cannot review their own PRs. 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 kubernetes-sigs/prow repository. |
📝 WalkthroughWalkthroughAdds a GCP PD CSI Driver Operator CLI entrypoint with Cobra-based root and Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (2 errors)
✅ Passed checks (13 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mpatlasov 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 |
openshift-ci
Bot
added
the
approved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
pkg/driver/gcp-pd/gcp_pd.go (1)
36-39: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winFix malformed log call and redundant/non-idiomatic error message.
klog.Error(nil, "...")treatsnilas afmt.Print-style argument (klog'sErrorargs are "handled in the manner offmt.Print"), so this prints something like<nil>Flavour HyperShift is not supported!instead of a clean message. Additionally, this error is already logged again by the caller (RunOperatorlogs"error building operator config: %v"on any non-nil error from this builder), so the localklog.Errorcall is redundant. Thefmt.Errorfmessage is also capitalized and ends with!, which violates Go's error-string convention (lower-case, no trailing punctuation).🔧 Proposed fix
if flavour != generator.FlavourStandalone { - klog.Error(nil, "Flavour HyperShift is not supported!") - return nil, fmt.Errorf("Flavour HyperShift is not supported!") + return nil, fmt.Errorf("flavour %q is not supported", flavour) }🤖 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/driver/gcp-pd/gcp_pd.go` around lines 36 - 39, The gcp_pd builder’s unsupported-flavour branch is logging incorrectly and returning a non-idiomatic error. In the config builder path in `gcp_pd.go`, remove the local `klog.Error(nil, ...)` call because `RunOperator` already logs builder failures, and change the returned `fmt.Errorf` message to a lowercase, punctuation-free error string. Keep the check on `flavour != generator.FlavourStandalone` and ensure the branch only returns the clean error.cmd/gcp-pd-csi-driver-operator/main.go (1)
27-30: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueIgnored error return from
cmd.Help().
cmd.Help()returns anerrorthat's discarded here.🔧 Proposed fix
Run: func(cmd *cobra.Command, args []string) { - cmd.Help() + if err := cmd.Help(); err != nil { + klog.Errorf("failed to print help: %v", err) + } os.Exit(1) },As per path instructions, "Never ignore error returns" for
**/*.go.🤖 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 `@cmd/gcp-pd-csi-driver-operator/main.go` around lines 27 - 30, The cobra command’s Run handler is discarding the error from cmd.Help(), which violates the “never ignore error returns” rule. Update the Run function in main.go to check and handle the return value from cmd.Help() explicitly, using the cmd.Help() call itself as the unique locator, and ensure the process still exits with a non-zero status if Help fails.Source: Path instructions
🤖 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 `@cmd/gcp-pd-csi-driver-operator/main.go`:
- Around line 27-30: The cobra command’s Run handler is discarding the error
from cmd.Help(), which violates the “never ignore error returns” rule. Update
the Run function in main.go to check and handle the return value from cmd.Help()
explicitly, using the cmd.Help() call itself as the unique locator, and ensure
the process still exits with a non-zero status if Help fails.
In `@pkg/driver/gcp-pd/gcp_pd.go`:
- Around line 36-39: The gcp_pd builder’s unsupported-flavour branch is logging
incorrectly and returning a non-idiomatic error. In the config builder path in
`gcp_pd.go`, remove the local `klog.Error(nil, ...)` call because `RunOperator`
already logs builder failures, and change the returned `fmt.Errorf` message to a
lowercase, punctuation-free error string. Keep the check on `flavour !=
generator.FlavourStandalone` and ensure the branch only returns the clean error.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 51202570-b405-4a12-9cee-b72111c8a207
⛔ Files ignored due to path filters (30)
assets/overlays/gcp-pd/generated/standalone/cabundle_cm.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/controller.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/controller_pdb.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/controller_sa.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/csidriver.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/kube_rbac_proxy_binding.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/kube_rbac_proxy_role.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/lease_leader_election_role.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/lease_leader_election_rolebinding.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/main_attacher_binding.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/main_provisioner_binding.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/main_resizer_binding.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/main_snapshotter_binding.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/manifests.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/node.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/node_privileged_binding.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/node_sa.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/privileged_role.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/prometheus_role.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/prometheus_rolebinding.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/service.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/servicemonitor.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/storageclass.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/storageclass_reader_resizer_binding.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/storageclass_ssd.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/volumeattributesclass_reader_provisioner_binding.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/volumeattributesclass_reader_resizer_binding.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/volumesnapshot_reader_provisioner_binding.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/volumesnapshotclass.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/volumesnapshotclass_images.yamlis excluded by!**/generated/**
📒 Files selected for processing (2)
cmd/gcp-pd-csi-driver-operator/main.gopkg/driver/gcp-pd/gcp_pd.go
Contributor
Author
|
/retest |
Contributor
Author
|
/retest-required |
2 similar comments
Contributor
Author
|
/retest-required |
Contributor
Author
|
/retest-required |
Contributor
|
/test hypershift-e2e-aks |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
Dockerfile.gcp-pd (2)
3-3: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAvoid copying entire build context.
COPY . .copies the full repository into the builder stage instead of specific files/directories needed for the build. As per path instructions, "COPY specific files, not entire 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 `@Dockerfile.gcp-pd` at line 3, Replace the broad COPY instruction in the Docker build stage with explicit COPY directives for only the files and directories required by the build, excluding the rest of the repository context.Source: Path instructions
6-10: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMissing
HEALTHCHECKinstruction.No
HEALTHCHECKis defined for the final image. As per path instructions, "HEALTHCHECK defined."🤖 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 `@Dockerfile.gcp-pd` around lines 6 - 10, Add a HEALTHCHECK instruction to the final image definition in Dockerfile.gcp-pd, alongside the existing ENTRYPOINT and LABEL directives. Configure it to invoke an appropriate health probe for gcp-pd-csi-driver-operator and preserve the current startup command.Source: Path instructions
🤖 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.
Inline comments:
In `@Dockerfile.gcp-pd`:
- Around line 6-10: Update the final image stage in the Dockerfile around the
gcp-pd-csi-driver-operator ENTRYPOINT to add a non-root USER directive, ensuring
the operator runs as non-root while preserving the existing image metadata and
startup command.
---
Nitpick comments:
In `@Dockerfile.gcp-pd`:
- Line 3: Replace the broad COPY instruction in the Docker build stage with
explicit COPY directives for only the files and directories required by the
build, excluding the rest of the repository context.
- Around line 6-10: Add a HEALTHCHECK instruction to the final image definition
in Dockerfile.gcp-pd, alongside the existing ENTRYPOINT and LABEL directives.
Configure it to invoke an appropriate health probe for
gcp-pd-csi-driver-operator and preserve the current startup command.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 19e8acdf-4877-4e40-93b2-802e0864bd31
📒 Files selected for processing (1)
Dockerfile.gcp-pd
Comment on lines
6
to
10
| FROM registry.ci.openshift.org/ocp/5.0:base-rhel9 | ||
| COPY --from=builder /go/src/github.com/openshift/gcp-pd-csi-driver-operator/gcp-pd-csi-driver-operator /usr/bin/ | ||
| COPY --from=builder /go/src/github.com/openshift/csi-operator/bin/gcp-pd-csi-driver-operator /usr/bin/ | ||
| ENTRYPOINT ["/usr/bin/gcp-pd-csi-driver-operator"] | ||
| LABEL io.k8s.display-name="OpenShift GCP PD CSI Driver Operator" \ | ||
| io.k8s.description="The GCP PD CSI Driver Operator installs and maintains the GCP PD CSI Driver on a cluster." |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Missing non-root USER directive in final image.
The final stage runs the operator without setting a non-root USER, so the container defaults to root. As per path instructions, "USER non-root; never run as root."
🔒 Proposed fix
FROM registry.ci.openshift.org/ocp/5.0:base-rhel9
COPY --from=builder /go/src/github.com/openshift/csi-operator/bin/gcp-pd-csi-driver-operator /usr/bin/
+USER 65532:65532
ENTRYPOINT ["/usr/bin/gcp-pd-csi-driver-operator"]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| FROM registry.ci.openshift.org/ocp/5.0:base-rhel9 | |
| COPY --from=builder /go/src/github.com/openshift/gcp-pd-csi-driver-operator/gcp-pd-csi-driver-operator /usr/bin/ | |
| COPY --from=builder /go/src/github.com/openshift/csi-operator/bin/gcp-pd-csi-driver-operator /usr/bin/ | |
| ENTRYPOINT ["/usr/bin/gcp-pd-csi-driver-operator"] | |
| LABEL io.k8s.display-name="OpenShift GCP PD CSI Driver Operator" \ | |
| io.k8s.description="The GCP PD CSI Driver Operator installs and maintains the GCP PD CSI Driver on a cluster." | |
| FROM registry.ci.openshift.org/ocp/5.0:base-rhel9 | |
| COPY --from=builder /go/src/github.com/openshift/csi-operator/bin/gcp-pd-csi-driver-operator /usr/bin/ | |
| USER 65532:65532 | |
| ENTRYPOINT ["/usr/bin/gcp-pd-csi-driver-operator"] | |
| LABEL io.k8s.display-name="OpenShift GCP PD CSI Driver Operator" \ | |
| io.k8s.description="The GCP PD CSI Driver Operator installs and maintains the GCP PD CSI Driver on a cluster." |
🤖 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 `@Dockerfile.gcp-pd` around lines 6 - 10, Update the final image stage in the
Dockerfile around the gcp-pd-csi-driver-operator ENTRYPOINT to add a non-root
USER directive, ensuring the operator runs as non-root while preserving the
existing image metadata and startup command.
Source: Path instructions
mpatlasov
force-pushed
the
STOR-2997-Implement-GCP-PD-CSI-driver-operator-inside-csi-operator-framework
branch
from
fa76b6a to
d85ac06
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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.
Inline comments:
In `@cmd/gcp-pd-csi-driver-operator/main.go`:
- Line 28: Update the Cobra command handler around cmd.Help() to handle its
returned error instead of discarding it. Use the error-returning command flow,
such as RunE, and return cmd.Help() so help-output failures propagate; ensure no
Go error return remains ignored.
In `@Dockerfile.gcp-pd`:
- Line 3: Replace the broad COPY . . instruction in the Dockerfile build stage
with explicit COPY directives for only the manifests and source directories
required by this build target. Exclude secrets, credentials, build artifacts,
and unrelated repository files from the builder context.
In `@pkg/driver/gcp-pd/gcp_pd.go`:
- Line 38: Update the error string returned by the HyperShift handling in the
relevant function to remove the trailing exclamation mark, preserving the exact
message text as “Flavour HyperShift is not supported”.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: b1bd05f7-b7fc-430b-877a-41c866600a9b
⛔ Files ignored due to path filters (32)
assets/overlays/gcp-pd/generated/standalone/cabundle_cm.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/controller.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/controller_hostnetwork_binding.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/controller_pdb.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/controller_sa.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/csidriver.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/hostnetwork_role.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/kube_rbac_proxy_binding.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/kube_rbac_proxy_role.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/lease_leader_election_role.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/lease_leader_election_rolebinding.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/main_attacher_binding.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/main_provisioner_binding.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/main_resizer_binding.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/main_snapshotter_binding.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/manifests.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/node.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/node_privileged_binding.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/node_sa.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/privileged_role.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/prometheus_role.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/prometheus_rolebinding.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/service.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/servicemonitor.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/storageclass.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/storageclass_reader_resizer_binding.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/storageclass_ssd.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/volumeattributesclass_reader_provisioner_binding.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/volumeattributesclass_reader_resizer_binding.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/volumesnapshot_reader_provisioner_binding.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/volumesnapshotclass.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/volumesnapshotclass_images.yamlis excluded by!**/generated/**
📒 Files selected for processing (3)
Dockerfile.gcp-pdcmd/gcp-pd-csi-driver-operator/main.gopkg/driver/gcp-pd/gcp_pd.go
| Use: "gcp-pd-csi-driver-operator", | ||
| Short: "OpenShift GCP PD CSI Driver Operator", | ||
| Run: func(cmd *cobra.Command, args []string) { | ||
| cmd.Help() |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Handle the cmd.Help() error.
Line 28 discards Cobra’s returned error, so help-output failures are silently lost. Check and propagate/handle it, for example by using RunE and returning cmd.Help().
As per path instructions: Go code must never ignore error returns.
🧰 Tools
🪛 golangci-lint (2.12.2)
[error] 28-28: Error return value of cmd.Help is not checked
(errcheck)
🤖 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 `@cmd/gcp-pd-csi-driver-operator/main.go` at line 28, Update the Cobra command
handler around cmd.Help() to handle its returned error instead of discarding it.
Use the error-returning command flow, such as RunE, and return cmd.Help() so
help-output failures propagate; ensure no Go error return remains ignored.
Sources: Path instructions, Linters/SAST tools
| COPY legacy/gcp-pd-csi-driver-operator . | ||
| RUN make | ||
| WORKDIR /go/src/github.com/openshift/csi-operator | ||
| COPY . . |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Do not copy the entire build context.
COPY . . places every repository file into the builder stage, potentially including secrets, credentials, and unrelated artifacts. Copy only the manifests and source directories required by this build target.
As per path instructions, “COPY specific files, not entire 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 `@Dockerfile.gcp-pd` at line 3, Replace the broad COPY . . instruction in the
Dockerfile build stage with explicit COPY directives for only the manifests and
source directories required by this build target. Exclude secrets, credentials,
build artifacts, and unrelated repository files from the builder context.
Source: Path instructions
This PR implements the minimal set of go-files and yaml-assets to enable the command ``` ./bin/gcp-pd-csi-driver-smb-operator start --kubeconfig $KUBECONFIG --namespace openshift-cluster-csi-drivers ``` to install and run GCP PD CSI driver: ``` gcp-pd-csi-driver-controller-c949b748-xkljz 10/10 Running 0 3m2s gcp-pd-csi-driver-controller-c949b748-xvgl9 10/10 Running 0 3m2s gcp-pd-csi-driver-node-5kcw5 3/3 Running 0 3m2s gcp-pd-csi-driver-node-frl74 3/3 Running 0 3m2s gcp-pd-csi-driver-node-gf9k7 3/3 Running 0 3m2s gcp-pd-csi-driver-node-p95sv 3/3 Running 0 3m2s gcp-pd-csi-driver-node-q4r98 3/3 Running 0 3m2s gcp-pd-csi-driver-node-xvmgq 3/3 Running 0 3m2s ```
Use gcp-pd go files from cmd/ and pkg/ dirs rather than legacy/
mpatlasov
force-pushed
the
STOR-2997-Implement-GCP-PD-CSI-driver-operator-inside-csi-operator-framework
branch
from
d85ac06 to
3c7108f
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@pkg/driver/gcp-pd/gcp_pd.go`:
- Line 38: Update the unsupported-flavour error in the relevant driver function
to start with a lowercase letter, while retaining the flavour-specific context
by formatting the flavour value in the message.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 926fa496-de4b-4f51-98ab-16491ea1ea52
⛔ Files ignored due to path filters (32)
assets/overlays/gcp-pd/generated/standalone/cabundle_cm.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/controller.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/controller_hostnetwork_binding.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/controller_pdb.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/controller_sa.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/csidriver.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/hostnetwork_role.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/kube_rbac_proxy_binding.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/kube_rbac_proxy_role.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/lease_leader_election_role.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/lease_leader_election_rolebinding.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/main_attacher_binding.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/main_provisioner_binding.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/main_resizer_binding.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/main_snapshotter_binding.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/manifests.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/node.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/node_privileged_binding.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/node_sa.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/privileged_role.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/prometheus_role.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/prometheus_rolebinding.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/service.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/servicemonitor.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/storageclass.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/storageclass_reader_resizer_binding.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/storageclass_ssd.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/volumeattributesclass_reader_provisioner_binding.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/volumeattributesclass_reader_resizer_binding.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/volumesnapshot_reader_provisioner_binding.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/volumesnapshotclass.yamlis excluded by!**/generated/**assets/overlays/gcp-pd/generated/standalone/volumesnapshotclass_images.yamlis excluded by!**/generated/**
📒 Files selected for processing (3)
Dockerfile.gcp-pdcmd/gcp-pd-csi-driver-operator/main.gopkg/driver/gcp-pd/gcp_pd.go
🚧 Files skipped from review as they are similar to previous changes (2)
- cmd/gcp-pd-csi-driver-operator/main.go
- Dockerfile.gcp-pd
| func GetGCPPDOperatorControllerConfig(ctx context.Context, flavour generator.ClusterFlavour, c *clients.Clients) (*config.OperatorControllerConfig, error) { | ||
| if flavour != generator.FlavourStandalone { | ||
| klog.Error(nil, "Flavour HyperShift is not supported") | ||
| return nil, fmt.Errorf("Flavour HyperShift is not supported") |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Lowercase the returned error string.
ST1005 still flags this line because Go error strings must not start with an uppercase letter. Use a lowercase, flavour-specific message such as fmt.Errorf("flavour %v is not supported", flavour).
Static analysis reports ST1005: error strings should not be capitalized.
🧰 Tools
🪛 golangci-lint (2.12.2)
[error] 38-38: ST1005: error strings should not be capitalized
(staticcheck)
🤖 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/driver/gcp-pd/gcp_pd.go` at line 38, Update the unsupported-flavour error
in the relevant driver function to start with a lowercase letter, while
retaining the flavour-specific context by formatting the flavour value in the
message.
Source: Linters/SAST tools
Contributor
|
@mpatlasov: The following tests failed, say
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. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR implements the minimal set of go-files and yaml-assets to enable the command
to install and run GCP PD CSI driver:
/cc @openshift/storage