Skip to content

Commit

Permalink
doc: Remove REQUIRED for boolean values
Browse files Browse the repository at this point in the history
Setting REQUIRED on boolean values is misleading, as it may cause CSI
implementers to think that the field must be present in the GRPC payload
and that they should fail if it's not there, which is incorrect.

As mentioned in the GRPC Proto3 docs: "note that if a scalar message
field is set to its default, the value will not be serialized on the
wire."

This means that boolean fields will not be sent over the wire when their
value is set to false.  Making it impossible to know whether the GRPC
message had the field set in the first place.

This patch updates the comments to reflect the default value and removes
the misleading REQUIRED notation.
  • Loading branch information
Akrog committed Aug 20, 2018
1 parent aeca98c commit 2664a3e
Show file tree
Hide file tree
Showing 3 changed files with 82 additions and 88 deletions.
8 changes: 3 additions & 5 deletions csi.proto
Original file line number Diff line number Diff line change
Expand Up @@ -580,8 +580,7 @@ message ControllerPublishVolumeRequest {
// This is a REQUIRED field.
VolumeCapability volume_capability = 3;

// Whether to publish the volume in readonly mode. This field is
// REQUIRED.
// Whether to publish the volume in readonly mode. Defaults to false.
bool readonly = 4;

// Secrets required by plugin to complete controller publish volume
Expand Down Expand Up @@ -648,7 +647,7 @@ message ValidateVolumeCapabilitiesRequest {

message ValidateVolumeCapabilitiesResponse {
// True if the Plugin supports the specified capabilities for the
// given volume. This field is REQUIRED.
// given volume. Defaults to false.
bool supported = 1;

// Message to the CO if `supported` above is false. This field is
Expand Down Expand Up @@ -992,8 +991,7 @@ message NodePublishVolumeRequest {
// This is a REQUIRED field.
VolumeCapability volume_capability = 5;

// Whether to publish the volume in readonly mode. This field is
// REQUIRED.
// Whether to publish the volume in readonly mode. Defaults to false.
bool readonly = 6;

// Secrets required by plugin to complete node publish volume request.
Expand Down
Loading

0 comments on commit 2664a3e

Please sign in to comment.