Merge pull request #647 from andyzhangx/refine-vol-cap-check

chore: refine VolumeCapabilities check

Author: andyzhangx

pkg/blob/controllerserver.go

@@ -51,14 +51,8 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest)
 		return nil, status.Error(codes.InvalidArgument, "CreateVolume Name must be provided")
 	}
 
-	volumeCapabilities := req.GetVolumeCapabilities()
-	if len(volumeCapabilities) == 0 {
-		return nil, status.Error(codes.InvalidArgument, "CreateVolume Volume capabilities must be provided")
-	}
-	for _, c := range volumeCapabilities {
-		if c.GetBlock() != nil {
-			return nil, status.Error(codes.InvalidArgument, "Block volume capability not supported")
-		}
+	if err := isValidVolumeCapabilities(req.GetVolumeCapabilities()); err != nil {
+		return nil, status.Error(codes.InvalidArgument, err.Error())
 	}
 
 	if acquired := d.volumeLocks.TryAcquire(volName); !acquired {
@@ -408,14 +402,8 @@ func (d *Driver) ValidateVolumeCapabilities(ctx context.Context, req *csi.Valida
 	if len(volumeID) == 0 {
 		return nil, status.Error(codes.InvalidArgument, "Volume ID missing in request")
 	}
-	volumeCapabilities := req.GetVolumeCapabilities()
-	if len(volumeCapabilities) == 0 {
-		return nil, status.Error(codes.InvalidArgument, "Volume capabilities missing in request")
-	}
-	for _, c := range volumeCapabilities {
-		if c.GetBlock() != nil {
-			return nil, status.Error(codes.InvalidArgument, "Block volume capability not supported")
-		}
+	if err := isValidVolumeCapabilities(req.GetVolumeCapabilities()); err != nil {
+		return nil, status.Error(codes.InvalidArgument, err.Error())
 	}
 
 	resourceGroupName, accountName, containerName, err := GetContainerInfo(volumeID)
@@ -535,3 +523,16 @@ func (d *Driver) ControllerExpandVolume(ctx context.Context, req *csi.Controller
 
 	return &csi.ControllerExpandVolumeResponse{CapacityBytes: req.GetCapacityRange().GetRequiredBytes()}, nil
 }
+
+// isValidVolumeCapabilities validates the given VolumeCapability array is valid
+func isValidVolumeCapabilities(volCaps []*csi.VolumeCapability) error {
+	if len(volCaps) == 0 {
+		return fmt.Errorf("volume capabilities missing in request")
+	}
+	for _, c := range volCaps {
+		if c.GetBlock() != nil {
+			return fmt.Errorf("block volume capability not supported")
+		}
+	}
+	return nil
+}

pkg/blob/controllerserver_test.go

@@ -118,7 +118,7 @@ func TestCreateVolume(t *testing.T) {
 					controllerServiceCapability,
 				}
 				_, err := d.CreateVolume(context.Background(), req)
-				expectedErr := status.Error(codes.InvalidArgument, "CreateVolume Volume capabilities must be provided")
+				expectedErr := status.Error(codes.InvalidArgument, "volume capabilities missing in request")
 				if !reflect.DeepEqual(err, expectedErr) {
 					t.Errorf("actualErr: (%v), expectedErr: (%v)", err, expectedErr)
 				}
@@ -136,7 +136,7 @@ func TestCreateVolume(t *testing.T) {
 					controllerServiceCapability,
 				}
 				_, err := d.CreateVolume(context.Background(), req)
-				expectedErr := status.Error(codes.InvalidArgument, "Block volume capability not supported")
+				expectedErr := status.Error(codes.InvalidArgument, "block volume capability not supported")
 				if !reflect.DeepEqual(err, expectedErr) {
 					t.Errorf("actualErr: (%v), expectedErr: (%v)", err, expectedErr)
 				}
@@ -459,7 +459,7 @@ func TestValidateVolumeCapabilities(t *testing.T) {
 					VolumeId: "unit-test",
 				}
 				_, err := d.ValidateVolumeCapabilities(context.Background(), req)
-				expectedErr := status.Error(codes.InvalidArgument, "Volume capabilities missing in request")
+				expectedErr := status.Error(codes.InvalidArgument, "volume capabilities missing in request")
 				if !reflect.DeepEqual(err, expectedErr) {
 					t.Errorf("actualErr: (%v), expectedErr: (%v)", err, expectedErr)
 				}
@@ -474,7 +474,7 @@ func TestValidateVolumeCapabilities(t *testing.T) {
 					VolumeCapabilities: blockVolumeCapabilities,
 				}
 				_, err := d.ValidateVolumeCapabilities(context.Background(), req)
-				expectedErr := status.Error(codes.InvalidArgument, "Block volume capability not supported")
+				expectedErr := status.Error(codes.InvalidArgument, "block volume capability not supported")
 				if !reflect.DeepEqual(err, expectedErr) {
 					t.Errorf("actualErr: (%v), expectedErr: (%v)", err, expectedErr)
 				}