From 862210cf752940d183ac3b21bf9b037b8739ae67 Mon Sep 17 00:00:00 2001
From: Drew Sirenko <68304519+AndrewSirenko@users.noreply.github.com>
Date: Tue, 2 Apr 2024 12:18:34 -0400
Subject: [PATCH] Add unit tests for invalid batch describe requests

---
 pkg/cloud/cloud.go      | 13 ++++++++-----
 pkg/cloud/cloud_test.go | 35 +++++++++++++++++++++++++++++++++--
 2 files changed, 41 insertions(+), 7 deletions(-)

diff --git a/pkg/cloud/cloud.go b/pkg/cloud/cloud.go
index fa2ebc3111..0314981dcd 100644
--- a/pkg/cloud/cloud.go
+++ b/pkg/cloud/cloud.go
@@ -178,6 +178,9 @@ var (
 
 	// ErrInvalidArgument is returned if parameters were rejected by cloud provider
 	ErrInvalidArgument = errors.New("invalid argument")
+
+	// ErrInvalidRequest is returned if parameters were rejected by driver
+	ErrInvalidRequest = errors.New("invalid request")
 )
 
 // Set during build time via -ldflags
@@ -432,7 +435,7 @@ func (c *cloud) batchDescribeVolumes(request *ec2.DescribeVolumesInput) (*types.
 		task = request.Filters[0].Values[0]
 
 	default:
-		return nil, fmt.Errorf("batchDescribeVolumes: invalid request, request: %v", request)
+		return nil, fmt.Errorf("%w: batchDescribeVolumes: request: %v", ErrInvalidRequest, request)
 	}
 
 	ch := make(chan batcher.BatchResult[*types.Volume])
@@ -675,7 +678,7 @@ func (c *cloud) batchDescribeVolumesModifications(request *ec2.DescribeVolumesMo
 	if len(request.VolumeIds) == 1 && request.VolumeIds[0] != "" {
 		task = request.VolumeIds[0]
 	} else {
-		return nil, fmt.Errorf("batchDescribeVolumesModifications: invalid request, request: %v", request)
+		return nil, fmt.Errorf("%w: batchDescribeVolumesModifications: invalid request, request: %v", ErrInvalidRequest, request)
 	}
 
 	ch := make(chan batcher.BatchResult[*types.VolumeModification])
@@ -691,7 +694,7 @@ func (c *cloud) batchDescribeVolumesModifications(request *ec2.DescribeVolumesMo
 	return r.Result, nil
 }
 
-// ResizeOrModifyDisk resizes an EBS volume in GiB increments, rouding up to the next possible allocatable unit, and/or modifies an EBS
+// ResizeOrModifyDisk resizes an EBS volume in GiB increments, rounding up to the next possible allocatable unit, and/or modifies an EBS
 // volume with the parameters in ModifyDiskOptions.
 // The resizing operation is performed only when newSizeBytes != 0.
 // It returns the volume size after this call or an error if the size couldn't be determined or the volume couldn't be modified.
@@ -795,7 +798,7 @@ func (c *cloud) batchDescribeInstances(request *ec2.DescribeInstancesInput) (*ty
 	if len(request.InstanceIds) == 1 && request.InstanceIds[0] != "" {
 		task = request.InstanceIds[0]
 	} else {
-		return nil, fmt.Errorf("batchDescribeInstances: invalid request, request: %v", request)
+		return nil, fmt.Errorf("%w: batchDescribeInstances: request: %v", ErrInvalidRequest, request)
 	}
 
 	ch := make(chan batcher.BatchResult[*types.Instance])
@@ -1161,7 +1164,7 @@ func (c *cloud) batchDescribeSnapshots(request *ec2.DescribeSnapshotsInput) (*ty
 		task = request.Filters[0].Values[0]
 
 	default:
-		return nil, fmt.Errorf("batchDescribeSnapshots: invalid request, request: %v", request)
+		return nil, fmt.Errorf("%w: batchDescribeSnapshots: request: %v", ErrInvalidRequest, request)
 	}
 
 	ch := make(chan batcher.BatchResult[*types.Snapshot])
diff --git a/pkg/cloud/cloud_test.go b/pkg/cloud/cloud_test.go
index a728f7570f..707521f5c3 100644
--- a/pkg/cloud/cloud_test.go
+++ b/pkg/cloud/cloud_test.go
@@ -149,7 +149,7 @@ func TestBatchDescribeVolumes(t *testing.T) {
 			expErr: fmt.Errorf("volume not found"),
 		},
 		{
-			name: "TestBatchDescribeVolumes: invalid tag",
+			name: "fail: invalid tag",
 			volumes: []types.Volume{
 				{
 					Tags: []types.Tag{
@@ -158,12 +158,19 @@ func TestBatchDescribeVolumes(t *testing.T) {
 				},
 			},
 			mockFunc: func(mockEC2 *MockEC2API, expErr error, volumes []types.Volume) {
-
 				volumeOutput := &ec2.DescribeVolumesOutput{Volumes: volumes}
 				mockEC2.EXPECT().DescribeVolumes(gomock.Any(), gomock.Any()).Return(volumeOutput, expErr).Times(0)
 			},
 			expErr: fmt.Errorf("invalid tag"),
 		},
+		{
+			name:    "fail: invalid request",
+			volumes: []types.Volume{{VolumeId: aws.String("")}},
+			mockFunc: func(mockEC2 *MockEC2API, expErr error, volumes []types.Volume) {
+				mockEC2.EXPECT().DescribeVolumes(gomock.Any(), gomock.Any()).Return(nil, nil).Times(0)
+			},
+			expErr: ErrInvalidRequest,
+		},
 	}
 
 	for _, tc := range testCases {
@@ -273,6 +280,14 @@ func TestBatchDescribeInstances(t *testing.T) {
 			},
 			expErr: fmt.Errorf("generic EC2 API error"),
 		},
+		{
+			name:        "fail: invalid request",
+			instanceIds: []string{""},
+			mockFunc: func(mockEC2 *MockEC2API, expErr error, reservations []types.Reservation) {
+				mockEC2.EXPECT().DescribeInstances(gomock.Any(), gomock.Any()).Return(nil, nil).Times(0)
+			},
+			expErr: ErrInvalidRequest,
+		},
 	}
 
 	for _, tc := range testCases {
@@ -440,6 +455,14 @@ func TestBatchDescribeSnapshots(t *testing.T) {
 			},
 			expErr: ErrNotFound,
 		},
+		{
+			name:      "fail: invalid request",
+			snapshots: []types.Snapshot{{SnapshotId: aws.String("")}},
+			mockFunc: func(mockEC2 *MockEC2API, expErr error, snapshots []types.Snapshot) {
+				mockEC2.EXPECT().DescribeSnapshots(gomock.Any(), gomock.Any()).Return(nil, nil).Times(0)
+			},
+			expErr: ErrInvalidRequest,
+		},
 	}
 
 	for _, tc := range testCases {
@@ -551,6 +574,14 @@ func TestBatchDescribeVolumesModifications(t *testing.T) {
 			},
 			expErr: fmt.Errorf("generic EC2 API error"),
 		},
+		{
+			name:      "fail: invalid request",
+			volumeIds: []string{""},
+			mockFunc: func(mockEC2 *MockEC2API, expErr error, volumeModifications []types.VolumeModification) {
+				mockEC2.EXPECT().DescribeVolumesModifications(gomock.Any(), gomock.Any()).Return(nil, expErr).Times(0)
+			},
+			expErr: ErrInvalidRequest,
+		},
 	}
 
 	for _, tc := range testCases {