add unit test coverage for v1beta1

pkg/validation-webhook/scheme.go

@@ -17,7 +17,7 @@ limitations under the License.
 package webhook
 
 import (
-	snapshot "github.com/kubernetes-csi/external-snapshotter/client/v4/clientset/versioned/scheme"
+	snapshot "github.com/kubernetes-csi/external-snapshotter/client/v6/clientset/versioned/scheme"
 	admissionv1 "k8s.io/api/admission/v1"
 	admissionv1beta1 "k8s.io/api/admission/v1beta1"
 	admissionregistrationv1 "k8s.io/api/admissionregistration/v1"

pkg/validation-webhook/snapshot.go

@@ -20,10 +20,10 @@ import (
 	"fmt"
 	"reflect"
 
-	volumesnapshotv1 "github.com/kubernetes-csi/external-snapshotter/client/v4/apis/volumesnapshot/v1"
+	volumesnapshotv1 "github.com/kubernetes-csi/external-snapshotter/client/v6/apis/volumesnapshot/v1"
-	volumesnapshotv1beta1 "github.com/kubernetes-csi/external-snapshotter/client/v4/apis/volumesnapshot/v1beta1"
+	volumesnapshotv1beta1 "github.com/kubernetes-csi/external-snapshotter/client/v6/apis/volumesnapshot/v1beta1"
-	storagelisters "github.com/kubernetes-csi/external-snapshotter/client/v4/listers/volumesnapshot/v1"
+	storagelisters "github.com/kubernetes-csi/external-snapshotter/client/v6/listers/volumesnapshot/v1"
-	"github.com/kubernetes-csi/external-snapshotter/v4/pkg/utils"
+	"github.com/kubernetes-csi/external-snapshotter/v6/pkg/utils"
 	v1 "k8s.io/api/admission/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/labels"
@@ -271,14 +271,15 @@ func decideSnapshotClassV1beta1(snapClass, oldSnapClass *volumesnapshotv1beta1.V
 			continue
 		}
 		if _, ok := driverToSnapshotDefautlMap[snapshotClass.Driver]; ok {
-			// Log an error or something here because this means that old behavior is present.
 			// We probably should just preserve behavior in this case.
-			return reviewResponse
+			// but not let the problem get worse. having a single one will
+			// suffice.
+			klog.V(2).Infof("snapshot driver: %v has multiple default classes", snapshotClass.Driver)
 		}
 		driverToSnapshotDefautlMap[snapshotClass.Driver] = snapshotClass
 	}
 
-	if _, ok := driverToSnapshotDefautlMap[snapClass.Driver]; !ok {
+	if _, ok := driverToSnapshotDefautlMap[snapClass.Driver]; ok {
 		reviewResponse.Allowed = false
 		reviewResponse.Result.Message = fmt.Sprintf("default snapshot class already exits for driver: %v", snapClass.Driver)
 		return reviewResponse

pkg/validation-webhook/snapshot_test.go

@@ -23,8 +23,12 @@ import (
 
 	volumesnapshotv1 "github.com/kubernetes-csi/external-snapshotter/client/v6/apis/volumesnapshot/v1"
 	volumesnapshotv1beta1 "github.com/kubernetes-csi/external-snapshotter/client/v6/apis/volumesnapshot/v1beta1"
+	storagelisters "github.com/kubernetes-csi/external-snapshotter/client/v6/listers/volumesnapshot/v1"
+	"github.com/kubernetes-csi/external-snapshotter/v6/pkg/utils"
 	v1 "k8s.io/api/admission/v1"
 	core_v1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/runtime"
 )
 
@@ -702,6 +706,241 @@ func TestAdmitVolumeSnapshotContentV1(t *testing.T) {
 	}
 }
 
+type fakeSnapshotLister struct {
+	values []*volumesnapshotv1.VolumeSnapshotClass
+}
+
+func (f *fakeSnapshotLister) List(selector labels.Selector) (ret []*volumesnapshotv1.VolumeSnapshotClass, err error) {
+	return f.values, nil
+}
+
+func (f *fakeSnapshotLister) Get(name string) (*volumesnapshotv1.VolumeSnapshotClass, error) {
+	for _, v := range f.values {
+		if v.Name == name {
+			return v, nil
+		}
+	}
+	return nil, nil
+}
+
 func TestAdmitVolumeSnapshotClassV1beta1(t *testing.T) {
+	testCases := []struct {
+		name                   string
+		volumeSnapshotClass    *volumesnapshotv1beta1.VolumeSnapshotClass
+		oldVolumeSnapshotClass *volumesnapshotv1beta1.VolumeSnapshotClass
+		shouldAdmit            bool
+		msg                    string
+		operation              v1.Operation
+		lister                 storagelisters.VolumeSnapshotClassLister
+	}{
+		{
+			name: "new default for class with no existing classes",
+			volumeSnapshotClass: &volumesnapshotv1beta1.VolumeSnapshotClass{
+				TypeMeta: metav1.TypeMeta{},
+				ObjectMeta: metav1.ObjectMeta{
+					Annotations: map[string]string{
+						utils.IsDefaultSnapshotClassAnnotation: "true",
+					},
+				},
+				Driver: "test.csi.io",
+			},
+			oldVolumeSnapshotClass: &volumesnapshotv1beta1.VolumeSnapshotClass{},
+			shouldAdmit:            true,
+			msg:                    "",
+			operation:              v1.Create,
+			lister:                 &fakeSnapshotLister{values: []*volumesnapshotv1.VolumeSnapshotClass{}},
+		},
+		{
+			name: "new default for class for  with existing default class different drivers",
+			volumeSnapshotClass: &volumesnapshotv1beta1.VolumeSnapshotClass{
+				TypeMeta: metav1.TypeMeta{},
+				ObjectMeta: metav1.ObjectMeta{
+					Annotations: map[string]string{
+						utils.IsDefaultSnapshotClassAnnotation: "true",
+					},
+				},
+				Driver: "test.csi.io",
+			},
+			oldVolumeSnapshotClass: &volumesnapshotv1beta1.VolumeSnapshotClass{},
+			shouldAdmit:            true,
+			msg:                    "",
+			operation:              v1.Create,
+			lister: &fakeSnapshotLister{values: []*volumesnapshotv1.VolumeSnapshotClass{
+				{
+					TypeMeta: metav1.TypeMeta{},
+					ObjectMeta: metav1.ObjectMeta{
+						Annotations: map[string]string{
+							utils.IsDefaultSnapshotClassAnnotation: "true",
+						},
+					},
+					Driver: "existing.test.csi.io",
+				},
+			}},
+		},
+		{
+			name: "new default for class with existing default class same driver",
+			volumeSnapshotClass: &volumesnapshotv1beta1.VolumeSnapshotClass{
+				TypeMeta: metav1.TypeMeta{},
+				ObjectMeta: metav1.ObjectMeta{
+					Annotations: map[string]string{
+						utils.IsDefaultSnapshotClassAnnotation: "true",
+					},
+				},
+				Driver: "test.csi.io",
+			},
+			oldVolumeSnapshotClass: &volumesnapshotv1beta1.VolumeSnapshotClass{},
+			shouldAdmit:            false,
+			msg:                    "default snapshot class already exits for driver: test.csi.io",
+			operation:              v1.Create,
+			lister: &fakeSnapshotLister{values: []*volumesnapshotv1.VolumeSnapshotClass{
+				{
+					TypeMeta: metav1.TypeMeta{},
+					ObjectMeta: metav1.ObjectMeta{
+						Annotations: map[string]string{
+							utils.IsDefaultSnapshotClassAnnotation: "true",
+						},
+					},
+					Driver: "test.csi.io",
+				},
+			}},
+		},
+		{
+			name: "default for class with existing default class same driver update",
+			volumeSnapshotClass: &volumesnapshotv1beta1.VolumeSnapshotClass{
+				TypeMeta: metav1.TypeMeta{},
+				ObjectMeta: metav1.ObjectMeta{
+					Annotations: map[string]string{
+						utils.IsDefaultSnapshotClassAnnotation: "true",
+					},
+				},
+				Driver: "test.csi.io",
+			},
+			oldVolumeSnapshotClass: &volumesnapshotv1beta1.VolumeSnapshotClass{
+				TypeMeta: metav1.TypeMeta{},
+				ObjectMeta: metav1.ObjectMeta{
+					Annotations: map[string]string{
+						utils.IsDefaultSnapshotClassAnnotation: "true",
+					},
+				},
+				Driver: "test.csi.io",
+			},
+			shouldAdmit: true,
+			msg:         "",
+			operation:   v1.Update,
+			lister: &fakeSnapshotLister{values: []*volumesnapshotv1.VolumeSnapshotClass{
+				{
+					TypeMeta: metav1.TypeMeta{},
+					ObjectMeta: metav1.ObjectMeta{
+						Annotations: map[string]string{
+							utils.IsDefaultSnapshotClassAnnotation: "true",
+						},
+					},
+					Driver: "test.csi.io",
+				},
+			}},
+		},
+		{
+			name: "new snapshot for class with existing default class same driver",
+			volumeSnapshotClass: &volumesnapshotv1beta1.VolumeSnapshotClass{
+				TypeMeta:   metav1.TypeMeta{},
+				ObjectMeta: metav1.ObjectMeta{},
+				Driver:     "test.csi.io",
+			},
+			oldVolumeSnapshotClass: &volumesnapshotv1beta1.VolumeSnapshotClass{},
+			shouldAdmit:            true,
+			msg:                    "",
+			operation:              v1.Create,
+			lister: &fakeSnapshotLister{values: []*volumesnapshotv1.VolumeSnapshotClass{
+				{
+					TypeMeta: metav1.TypeMeta{},
+					ObjectMeta: metav1.ObjectMeta{
+						Annotations: map[string]string{
+							utils.IsDefaultSnapshotClassAnnotation: "true",
+						},
+					},
+					Driver: "test.csi.io",
+				},
+			}},
+		},
+		{
+			name: "new snapshot for class with existing default classes",
+			volumeSnapshotClass: &volumesnapshotv1beta1.VolumeSnapshotClass{
+				TypeMeta: metav1.TypeMeta{},
+				ObjectMeta: metav1.ObjectMeta{
+					Annotations: map[string]string{
+						utils.IsDefaultSnapshotClassAnnotation: "true",
+					},
+				},
+				Driver: "test.csi.io",
+			},
+			oldVolumeSnapshotClass: &volumesnapshotv1beta1.VolumeSnapshotClass{},
+			shouldAdmit:            false,
+			msg:                    "default snapshot class already exits for driver: test.csi.io",
+			operation:              v1.Create,
+			lister: &fakeSnapshotLister{values: []*volumesnapshotv1.VolumeSnapshotClass{
+				{
+					TypeMeta: metav1.TypeMeta{},
+					ObjectMeta: metav1.ObjectMeta{
+						Annotations: map[string]string{
+							utils.IsDefaultSnapshotClassAnnotation: "true",
+						},
+					},
+					Driver: "test.csi.io",
+				},
+				{
+					TypeMeta: metav1.TypeMeta{},
+					ObjectMeta: metav1.ObjectMeta{
+						Annotations: map[string]string{
+							utils.IsDefaultSnapshotClassAnnotation: "true",
+						},
+					},
+					Driver: "test.csi.io",
+				},
+			}},
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			snapshotContent := tc.volumeSnapshotClass
+			raw, err := json.Marshal(snapshotContent)
+			if err != nil {
+				t.Fatal(err)
+			}
+			oldSnapshotClass := tc.oldVolumeSnapshotClass
+			oldRaw, err := json.Marshal(oldSnapshotClass)
+			if err != nil {
+				t.Fatal(err)
+			}
+			review := v1.AdmissionReview{
+				Request: &v1.AdmissionRequest{
+					Object: runtime.RawExtension{
+						Raw: raw,
+					},
+					OldObject: runtime.RawExtension{
+						Raw: oldRaw,
+					},
+					Resource:  SnapshotClassV1Beta1GVR,
+					Operation: tc.operation,
+				},
+			}
+
+			response := admitSnapshot(review, tc.lister)
+
+			shouldAdmit := response.Allowed
+			msg := response.Result.Message
+
+			expectedResponse := tc.shouldAdmit
+			expectedMsg := tc.msg
+
+			if shouldAdmit != expectedResponse {
+				t.Errorf("expected \"%v\" to equal \"%v\"", shouldAdmit, expectedResponse)
+			}
+			if msg != expectedMsg {
+				t.Errorf("expected \"%v\" to equal \"%v\"", msg, expectedMsg)
+			}
+		})
+
+	}
 
 }

pkg/validation-webhook/webhook.go

@@ -26,11 +26,11 @@ import (
 	"os"
 	"time"
 
-	clientset "github.com/kubernetes-csi/external-snapshotter/client/v4/clientset/versioned"
+	clientset "github.com/kubernetes-csi/external-snapshotter/client/v6/clientset/versioned"
-	storagelisters "github.com/kubernetes-csi/external-snapshotter/client/v4/listers/volumesnapshot/v1"
+	storagelisters "github.com/kubernetes-csi/external-snapshotter/client/v6/listers/volumesnapshot/v1"
 	"github.com/spf13/cobra"
 
-	informers "github.com/kubernetes-csi/external-snapshotter/client/v4/informers/externalversions"
+	informers "github.com/kubernetes-csi/external-snapshotter/client/v6/informers/externalversions"
 	v1 "k8s.io/api/admission/v1"
 	"k8s.io/api/admission/v1beta1"
 	"k8s.io/apimachinery/pkg/runtime"

pkg/validation-webhook/webhook_test.go

@@ -14,22 +14,8 @@ import (
 	"os"
 	"testing"
 	"time"
-
-	v1 "github.com/kubernetes-csi/external-snapshotter/client/v4/apis/volumesnapshot/v1"
-	"k8s.io/apimachinery/pkg/labels"
 )
 
-type fakeSnapshotLister struct {
-}
-
-func (f *fakeSnapshotLister) List(selector labels.Selector) (ret []*v1.VolumeSnapshotClass, err error) {
-	return nil, nil
-}
-
-func (f *fakeSnapshotLister) Get(name string) (*v1.VolumeSnapshotClass, error) {
-	return nil, nil
-}
-
 func TestWebhookCertReload(t *testing.T) {
 	// Initialize test space
 	tmpDir := os.TempDir() + "/webhook-cert-tests"
@@ -46,9 +32,6 @@ func TestWebhookCertReload(t *testing.T) {
 			t.Errorf("unexpected error occurred while deleting certs: %v", err)
 		}
 	}()
-
-	//Create a fake lister
-
 	generateTestCertKeyPair(t, certFile, keyFile)
 
 	// Start test server