When casting resource.Quantity to int64 it may overflow

We need to ensure that when converting resource.Quantity
to int64 it does not overflow and if it does, then an error is
returned back to the use rather than attempting expansion and
provisioning operations with scaled value.

pkg/volume/azure_file/azure_file.go

@@ -183,7 +183,13 @@ func (plugin *azureFilePlugin) ExpandVolumeDevice(
 		return oldSize, err
 	}
 
-	if err := azure.ResizeFileShare(resourceGroup, accountName, shareName, int(volumehelpers.RoundUpToGiB(newSize))); err != nil {
+	requestGiB, err := volumehelpers.RoundUpToGiBInt(newSize)
+
+	if err != nil {
+		return oldSize, err
+	}
+
+	if err := azure.ResizeFileShare(resourceGroup, accountName, shareName, requestGiB); err != nil {
 		return oldSize, err
 	}
 

pkg/volume/azure_file/azure_provision.go

@@ -158,7 +158,11 @@ func (a *azureFileProvisioner) Provision(selectedNode *v1.Node, allowedTopologie
 	var sku, resourceGroup, location, account, shareName string
 
 	capacity := a.options.PVC.Spec.Resources.Requests[v1.ResourceName(v1.ResourceStorage)]
-	requestGiB := int(volumehelpers.RoundUpToGiB(capacity))
+	requestGiB, err := volumehelpers.RoundUpToGiBInt(capacity)
+	if err != nil {
+		return nil, err
+	}
+
 	secretNamespace := a.options.PVC.Namespace
 	// Apply ProvisionerParameters (case-insensitive). We leave validation of
 	// the values to the cloud provider.

pkg/volume/gcepd/gce_util.go

@@ -101,7 +101,10 @@ func (util *GCEDiskUtil) CreateVolume(c *gcePersistentDiskProvisioner, node *v1.
 	name := volumeutil.GenerateVolumeName(c.options.ClusterName, c.options.PVName, 63) // GCE PD name can have up to 63 characters
 	capacity := c.options.PVC.Spec.Resources.Requests[v1.ResourceName(v1.ResourceStorage)]
 	// GCE PDs are allocated in chunks of GiBs
-	requestGB := volumehelpers.RoundUpToGiB(capacity)
+	requestGB, err := volumehelpers.RoundUpToGiB(capacity)
+	if err != nil {
+		return "", 0, nil, "", err
+	}
 
 	// Apply Parameters.
 	// Values for parameter "replication-type" are canonicalized to lower case.
@@ -159,7 +162,7 @@ func (util *GCEDiskUtil) CreateVolume(c *gcePersistentDiskProvisioner, node *v1.
 			name,
 			diskType,
 			selectedZones,
-			int64(requestGB),
+			requestGB,
 			*c.options.CloudTags)
 		if err != nil {
 			klog.V(2).Infof("Error creating regional GCE PD volume: %v", err)
@@ -176,7 +179,7 @@ func (util *GCEDiskUtil) CreateVolume(c *gcePersistentDiskProvisioner, node *v1.
 			name,
 			diskType,
 			selectedZone,
-			int64(requestGB),
+			requestGB,
 			*c.options.CloudTags)
 		if err != nil {
 			klog.V(2).Infof("Error creating single-zone GCE PD volume: %v", err)

pkg/volume/glusterfs/glusterfs.go

@@ -1212,11 +1212,18 @@ func (plugin *glusterfsPlugin) ExpandVolumeDevice(spec *volume.Spec, newSize res
 	}
 
 	// Find out delta size
-	expansionSize := resource.NewScaledQuantity((newSize.Value() - oldSize.Value()), 0)
-	expansionSizeGiB := int(volumehelpers.RoundUpToGiB(*expansionSize))
+	expansionSize := newSize
+	expansionSize.Sub(oldSize)
+	expansionSizeGiB, err := volumehelpers.RoundUpToGiBInt(expansionSize)
+	if err != nil {
+		return oldSize, err
+	}
 
 	// Find out requested Size
-	requestGiB := volumehelpers.RoundUpToGiB(newSize)
+	requestGiB, err := volumehelpers.RoundUpToGiB(newSize)
+	if err != nil {
+		return oldSize, err
+	}
 
 	//Check the existing volume size
 	currentVolumeInfo, err := cli.VolumeInfo(volumeID)

pkg/volume/portworx/portworx_util.go

@@ -25,7 +25,7 @@ import (
 	volumeclient "github.com/libopenstorage/openstorage/api/client/volume"
 	osdspec "github.com/libopenstorage/openstorage/api/spec"
 	volumeapi "github.com/libopenstorage/openstorage/volume"
-	"k8s.io/api/core/v1"
+	v1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	volumehelpers "k8s.io/cloud-provider/volume/helpers"
@@ -60,7 +60,10 @@ func (util *portworxVolumeUtil) CreateVolume(p *portworxVolumeProvisioner) (stri
 
 	capacity := p.options.PVC.Spec.Resources.Requests[v1.ResourceName(v1.ResourceStorage)]
 	// Portworx Volumes are specified in GiB
-	requestGiB := volumehelpers.RoundUpToGiB(capacity)
+	requestGiB, err := volumehelpers.RoundUpToGiB(capacity)
+	if err != nil {
+		return "", 0, nil, err
+	}
 
 	// Perform a best-effort parsing of parameters. Portworx 1.2.9 and later parses volume parameters from
 	// spec.VolumeLabels. So even if below SpecFromOpts() fails to parse certain parameters or
@@ -211,7 +214,11 @@ func (util *portworxVolumeUtil) ResizeVolume(spec *volume.Spec, newSize resource
 	}
 
 	vol := vols[0]
-	newSizeInBytes := uint64(volumehelpers.RoundUpToGiB(newSize) * volumehelpers.GiB)
+	tBytes, ok := newSize.AsInt64()
+	if !ok {
+		return fmt.Errorf("quantity %v is too great, overflows int64", newSize)
+	}
+	newSizeInBytes := uint64(tBytes)
 	if vol.Spec.Size >= newSizeInBytes {
 		klog.Infof("Portworx volume: %s already at size: %d greater than or equal to new "+
 			"requested size: %d. Skipping resize.", spec.Name(), vol.Spec.Size, newSizeInBytes)

pkg/volume/rbd/rbd_util.go

@@ -649,7 +649,11 @@ func (util *rbdUtil) ExpandImage(rbdExpander *rbdVolumeExpander, oldSize resourc
 	var err error
 
 	// Convert to MB that rbd defaults on.
-	sz := int(volumehelpers.RoundUpToMiB(newSize))
+	sz, err := volumehelpers.RoundUpToMiBInt(newSize)
+	if err != nil {
+		return oldSize, err
+	}
+
 	newVolSz := fmt.Sprintf("%d", sz)
 	newSizeQuant := resource.MustParse(fmt.Sprintf("%dMi", sz))
 

pkg/volume/scaleio/BUILD

@@ -23,6 +23,7 @@ go_test(
         "//staging/src/k8s.io/apimachinery/pkg/types:go_default_library",
         "//staging/src/k8s.io/client-go/kubernetes/fake:go_default_library",
         "//staging/src/k8s.io/client-go/util/testing:go_default_library",
+        "//staging/src/k8s.io/cloud-provider/volume/helpers:go_default_library",
         "//vendor/github.com/thecodeteam/goscaleio/types/v1:go_default_library",
         "//vendor/k8s.io/klog:go_default_library",
         "//vendor/k8s.io/utils/exec/testing:go_default_library",

pkg/volume/scaleio/sio_volume.go

@@ -55,6 +55,10 @@ type sioVolume struct {
 	volume.MetricsNil
 }
 
+const (
+	minimumVolumeSizeGiB = 8
+)
+
 // *******************
 // volume.Volume Impl
 var _ volume.Volume = &sioVolume{}
@@ -272,21 +276,17 @@ func (v *sioVolume) Provision(selectedNode *api.Node, allowedTopologies []api.To
 
 	// setup volume attrributes
 	genName := v.generateName("k8svol", 11)
-	eightGig := int64(8 * volumehelpers.GiB)
 
 	capacity := v.options.PVC.Spec.Resources.Requests[api.ResourceName(api.ResourceStorage)]
 
-	volSizeBytes := capacity.Value()
-	volSizeGB := int64(volumehelpers.RoundUpToGiB(capacity))
-
-	if volSizeBytes == 0 {
-		return nil, fmt.Errorf("invalid volume size of 0 specified")
+	volSizeGiB, err := volumehelpers.RoundUpToGiB(capacity)
+	if err != nil {
+		return nil, err
 	}
 
-	if volSizeBytes < eightGig {
-		eightGiBCapacity := resource.NewQuantity(eightGig, resource.BinarySI)
-		volSizeGB = int64(volumehelpers.RoundUpToGiB(*eightGiBCapacity))
-		klog.V(4).Info(log("capacity less than 8Gi found, adjusted to %dGi", volSizeGB))
+	if volSizeGiB < minimumVolumeSizeGiB {
+		volSizeGiB = minimumVolumeSizeGiB
+		klog.V(4).Info(log("capacity less than 8Gi found, adjusted to %dGi", volSizeGiB))
 
 	}
 
@@ -298,7 +298,7 @@ func (v *sioVolume) Provision(selectedNode *api.Node, allowedTopologies []api.To
 
 	// create volume
 	volName := genName
-	vol, err := v.sioMgr.CreateVolume(volName, volSizeGB)
+	vol, err := v.sioMgr.CreateVolume(volName, volSizeGiB)
 	if err != nil {
 		klog.Error(log("provision failed while creating volume: %v", err))
 		return nil, err
@@ -333,7 +333,7 @@ func (v *sioVolume) Provision(selectedNode *api.Node, allowedTopologies []api.To
 			AccessModes:                   v.options.PVC.Spec.AccessModes,
 			Capacity: api.ResourceList{
 				api.ResourceName(api.ResourceStorage): resource.MustParse(
-					fmt.Sprintf("%dGi", volSizeGB),
+					fmt.Sprintf("%dGi", volSizeGiB),
 				),
 			},
 			PersistentVolumeSource: api.PersistentVolumeSource{

pkg/volume/scaleio/sio_volume_test.go

@@ -23,6 +23,7 @@ import (
 	"strings"
 	"testing"
 
+	volumehelpers "k8s.io/cloud-provider/volume/helpers"
 	"k8s.io/klog"
 
 	api "k8s.io/api/core/v1"
@@ -425,7 +426,7 @@ func TestVolumeProvisionerWithIncompleteConfig(t *testing.T) {
 	}
 }
 
-func TestVolumeProvisionerWithZeroCapacity(t *testing.T) {
+func TestVolumeProvisionerWithMinimumCapacity(t *testing.T) {
 	plugMgr, tmpDir := newPluginMgr(t, makeScaleIOSecret(testSecret, testns))
 	defer os.RemoveAll(tmpDir)
 
@@ -441,7 +442,7 @@ func TestVolumeProvisionerWithZeroCapacity(t *testing.T) {
 	options := volume.VolumeOptions{
 		ClusterName:                   "testcluster",
 		PVName:                        "pvc-sio-dynamic-vol",
-		PVC:                           volumetest.CreateTestPVC("0Mi", []api.PersistentVolumeAccessMode{api.ReadWriteOnce}),
+		PVC:                           volumetest.CreateTestPVC("100Mi", []api.PersistentVolumeAccessMode{api.ReadWriteOnce}),
 		PersistentVolumeReclaimPolicy: api.PersistentVolumeReclaimDelete,
 	}
 	options.PVC.Namespace = testns
@@ -466,11 +467,25 @@ func TestVolumeProvisionerWithZeroCapacity(t *testing.T) {
 	}
 	sioVol.sioMgr.client = sio
 
-	_, err = provisioner.Provision(nil, nil)
-	if err == nil {
-		t.Fatalf("call to Provision() should fail with invalid capacity")
+	pv, err :=
+		provisioner.Provision(nil, nil)
+	if err != nil {
+		t.Fatalf("call to Provision() failed %v", err)
 	}
 
+	pvSize := pv.Spec.Capacity.Storage()
+	if pvSize == nil {
+		t.Fatalf("unexpected pv size: nil")
+	}
+
+	gibSize, err := volumehelpers.RoundUpToGiB(*pvSize)
+	if err != nil {
+		t.Fatalf("unexpected error while converting size to GiB: %v", err)
+	}
+
+	if gibSize != minimumVolumeSizeGiB {
+		t.Fatalf("expected GiB size to be %v got %v", minimumVolumeSizeGiB, gibSize)
+	}
 }
 
 func TestVolumeProvisionerWithSecretNamespace(t *testing.T) {