Skip to content

manager/csi: add missing nil check in PublishVolume, UnpublishVolume - #3277

Open
thaJeztah wants to merge 1 commit into
moby:masterfrom
thaJeztah:csi_nil_volume
Open

manager/csi: add missing nil check in PublishVolume, UnpublishVolume#3277
thaJeztah wants to merge 1 commit into
moby:masterfrom
thaJeztah:csi_nil_volume

Conversation

@thaJeztah

Copy link
Copy Markdown
Member

Both makeControllerPublishVolumeRequest and makeControllerUnpublishVolumeRequest check if the VolumeInfo is nil, in which case they would produce a nil request;

func (p *plugin) makeControllerPublishVolumeRequest(v *api.Volume, nodeID string) *csi.ControllerPublishVolumeRequest {
    if v.VolumeInfo == nil {
        return nil
    }
    ...

func (p *plugin) makeControllerUnpublishVolumeRequest(v *api.Volume, nodeID string) *csi.ControllerUnpublishVolumeRequest {
    if v.VolumeInfo == nil {
        return nil
    }
    ...

This request which would be sent to the CSI controller-client, potentially resulting in an obscure error during marshaling;

rpc error: code = Internal desc = grpc: error while marshaling: proto: Marshal called with nil

Update PublishVolume and UnpublishVolume to return early, similar to the existing check in DeleteVolume;

func (p *plugin) DeleteVolume(ctx context.Context, v *api.Volume) error {
    if v.VolumeInfo == nil {
        return errors.New("VolumeInfo must not be nil")
    }

- What I did

- How I did it

- How to test it

- Description for the changelog

Both makeControllerPublishVolumeRequest and makeControllerUnpublishVolumeRequest
check if the `VolumeInfo` is `nil`, in which case they would produce a `nil`
request;

    func (p *plugin) makeControllerPublishVolumeRequest(v *api.Volume, nodeID string) *csi.ControllerPublishVolumeRequest {
        if v.VolumeInfo == nil {
            return nil
        }
        ...

    func (p *plugin) makeControllerUnpublishVolumeRequest(v *api.Volume, nodeID string) *csi.ControllerUnpublishVolumeRequest {
        if v.VolumeInfo == nil {
            return nil
        }
        ...

This request which would be sent to the CSI controller-client, potentially
resulting in an obscure error during marshaling;

    rpc error: code = Internal desc = grpc: error while marshaling: proto: Marshal called with nil

Update `PublishVolume` and `UnpublishVolume` to return early, similar to the
existing check in `DeleteVolume`;

    func (p *plugin) DeleteVolume(ctx context.Context, v *api.Volume) error {
        if v.VolumeInfo == nil {
            return errors.New("VolumeInfo must not be nil")
        }

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 55.55556% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 14.73%. Comparing base (6e9e7b8) to head (dbe7dae).
⚠️ Report is 60 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #3277   +/-   ##
=======================================
  Coverage   14.73%   14.73%           
=======================================
  Files         200      200           
  Lines       93077    93025   -52     
=======================================
- Hits        13712    13711    -1     
+ Misses      78019    77968   -51     
  Partials     1346     1346           
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@thaJeztah
thaJeztah requested review from corhere and vvoland August 1, 2026 15:50
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.

2 participants