Merge pull request #1519 from k8s-infra-cherrypick-robot/cherry-pick-1514-to-release-1.8

[release-1.8] Add error logging when error is encountered fetching a VolumeSnapshotContent source image

Author: mattcary

pkg/gce-cloud-provider/compute/fake-gce.go

@@ -17,6 +17,8 @@ package gcecloudprovider
 import (
 	"context"
 	"fmt"
+	"net/http"
+	"regexp"
 	"strings"
 
 	"github.com/GoogleCloudPlatform/k8s-cloud-provider/pkg/cloud/meta"
@@ -39,6 +41,15 @@ const (
 	imageURITemplateGlobal    = "projects/%s/global/images/%s"    //{gce.projectID}/global/images/{image.Name}"
 )
 
+var (
+	// Snaphsot and Image Regex must comply with RFC1035
+	rfc1035Regex = regexp.MustCompile("^[a-z]([-a-z0-9]*[a-z0-9])?$")
+)
+
+func isRFC1035(value string) bool {
+	return rfc1035Regex.MatchString(strings.ToLower(value))
+}
+
 type FakeCloudProvider struct {
 	project string
 	zone    string
@@ -325,6 +336,9 @@ func (cloud *FakeCloudProvider) GetInstanceOrError(ctx context.Context, instance
 
 // Snapshot Methods
 func (cloud *FakeCloudProvider) GetSnapshot(ctx context.Context, project, snapshotName string) (*computev1.Snapshot, error) {
+	if !isRFC1035(snapshotName) {
+		return nil, fmt.Errorf("invalid snapshot name %v: %w", snapshotName, invalidError())
+	}
 	snapshot, ok := cloud.snapshots[snapshotName]
 	if !ok {
 		return nil, notFoundError()
@@ -403,6 +417,9 @@ func (cloud *FakeCloudProvider) ListImages(ctx context.Context, filter string) (
 }
 
 func (cloud *FakeCloudProvider) GetImage(ctx context.Context, project, imageName string) (*computev1.Image, error) {
+	if !isRFC1035(imageName) {
+		return nil, fmt.Errorf("invalid image name %v: %w", imageName, invalidError())
+	}
 	image, ok := cloud.images[imageName]
 	if !ok {
 		return nil, notFoundError()
@@ -559,6 +576,7 @@ func notFoundError() *googleapi.Error {
 
 func invalidError() *googleapi.Error {
 	return &googleapi.Error{
+		Code: http.StatusBadRequest,
 		Errors: []googleapi.ErrorItem{
 			{
 				Reason: "invalid",

pkg/gce-pd-csi-driver/controller.go

@@ -1337,6 +1337,7 @@ func (gceCS *GCEControllerServer) getSnapshotByID(ctx context.Context, snapshotI
 				// return empty list if no snapshot is found
 				return &csi.ListSnapshotsResponse{}, nil
 			}
+			return nil, common.LoggedError("Failed to get image snapshot: ", err)
 		}
 		e, err := generateImageEntry(image)
 		if err != nil {

pkg/gce-pd-csi-driver/controller_test.go

@@ -286,13 +286,45 @@ func TestListSnapshotsArguments(t *testing.T) {
 			numImages:     2,
 			expectedCount: 1,
 		},
+		{
+			name: "valid image",
+			req: &csi.ListSnapshotsRequest{
+				SnapshotId: testImageID + "0",
+			},
+			numSnapshots:  3,
+			numImages:     2,
+			expectedCount: 1,
+		},
 		{
 			name: "invalid id",
 			req: &csi.ListSnapshotsRequest{
 				SnapshotId: testSnapshotID + "/foo",
 			},
 			expectedCount: 0,
 		},
+		{
+			name: "invalid image id",
+			req: &csi.ListSnapshotsRequest{
+				SnapshotId: testImageID + "/foo",
+			},
+			expectedCount: 0,
+		},
+		{
+			name: "invalid snapshot name",
+			req: &csi.ListSnapshotsRequest{
+				SnapshotId: testSnapshotID + "-invalid-snapshot-",
+			},
+			expectedCount: 0,
+			expErrCode:    codes.InvalidArgument,
+		},
+		{
+			name: "invalid image name",
+			req: &csi.ListSnapshotsRequest{
+				SnapshotId: testImageID + "-invalid-image-",
+			},
+			expectedCount: 0,
+			expErrCode:    codes.InvalidArgument,
+		},
 		{
 			name: "no id",
 			req: &csi.ListSnapshotsRequest{