Skip to content

isControllerCapabilitySupported rejects valid responses with zero controller capabilities #596

@Andrie-Amor

Description

@Andrie-Amor

Hi! While investigating #595, I noticed a related bug in isControllerCapabilitySupported that affects any driver that exposes zero controller capabilities.

What happened?

pkg/sanity/controller.go#L88-L107:

caps, err := c.ControllerGetCapabilities(
    context.Background(),
    &csi.ControllerGetCapabilitiesRequest{})

Expect(err).NotTo(HaveOccurred())
Expect(caps).NotTo(BeNil())
Expect(caps.GetCapabilities()).NotTo(BeNil())

for _, cap := range caps.GetCapabilities() {
    ...
}

ControllerGetCapabilitiesResponse.capabilities is a repeated protobuf field. In proto3, an empty repeated field and an unset repeated field are encoded identically on the wire. Both produce zero bytes for that field. After the response is decoded in Go, a driver that returns zero controller capabilities may be observed by the client as a nil slice, regardless of whether the server set Capabilities to nil or to []*ControllerServiceCapability{}.

This makes Expect(caps.GetCapabilities()).NotTo(BeNil()) too strict. A nil slice does not indicate a failed RPC or an invalid response. It can simply mean the driver returned no controller capabilities. The loop immediately below already handles this case correctly, since ranging over a nil slice performs zero iterations.

What did you expect to happen?

A driver that registers a Controller service but advertises zero controller capabilities should pass isControllerCapabilitySupported (it would correctly return false for any cap type) rather than fail the suite in BeforeEach.

How to reproduce it

  1. Implement a CSI driver that registers a ControllerServer whose ControllerGetCapabilities returns &csi.ControllerGetCapabilitiesResponse{} (or &csi.ControllerGetCapabilitiesResponse{Capabilities: []*csi.ControllerServiceCapability{}}, which serialize identically).
  2. Run sanity.GinkgoTest against it.
  3. Any spec that calls isControllerCapabilitySupported fails at the Expect(caps.GetCapabilities()).NotTo(BeNil()) assertion.

Anything else we need to know?

Proposed fix

Remove the line:

Expect(caps.GetCapabilities()).NotTo(BeNil())

The helper would still validate that the RPC succeeded and that the response itself is non-nil. The for-loop directly below already handles a nil slice correctly.

Environment

  • csi-test version: v5.4.0
  • CSI spec version: 1.x
  • Go version: 1.23

I would be happy to also drive the fix for this!

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions