From 5069c99ec393791ad97284e8740a119222be6176 Mon Sep 17 00:00:00 2001 From: xing-yang Date: Thu, 12 Nov 2020 03:35:12 +0000 Subject: [PATCH] Update controller based on snapshot v1 apis --- cmd/csi-snapshotter/main.go | 4 +- cmd/snapshot-controller/main.go | 6 +- .../admission-configuration-template | 2 +- pkg/common-controller/framework_test.go | 16 +- pkg/common-controller/snapshot_controller.go | 34 +- .../snapshot_controller_base.go | 6 +- .../snapshot_controller_test.go | 2 +- pkg/common-controller/snapshot_create_test.go | 2 +- pkg/common-controller/snapshot_delete_test.go | 6 +- pkg/common-controller/snapshot_update_test.go | 60 +--- pkg/sidecar-controller/content_create_test.go | 4 +- pkg/sidecar-controller/csi_handler.go | 2 +- pkg/sidecar-controller/framework_test.go | 10 +- pkg/sidecar-controller/snapshot_controller.go | 18 +- .../snapshot_controller_base.go | 6 +- .../snapshot_controller_test.go | 2 +- .../snapshot_delete_test.go | 4 +- pkg/utils/util.go | 43 ++- pkg/utils/util_test.go | 2 +- pkg/validation-webhook/snapshot.go | 145 +++++++- pkg/validation-webhook/snapshot_test.go | 333 +++++++++++++++++- 21 files changed, 569 insertions(+), 138 deletions(-) diff --git a/cmd/csi-snapshotter/main.go b/cmd/csi-snapshotter/main.go index 8d95de56..80a46413 100644 --- a/cmd/csi-snapshotter/main.go +++ b/cmd/csi-snapshotter/main.go @@ -190,8 +190,8 @@ func main() { snapClient, kubeClient, driverName, - factory.Snapshot().V1beta1().VolumeSnapshotContents(), - factory.Snapshot().V1beta1().VolumeSnapshotClasses(), + factory.Snapshot().V1().VolumeSnapshotContents(), + factory.Snapshot().V1().VolumeSnapshotClasses(), snapShotter, *csiTimeout, *resyncPeriod, diff --git a/cmd/snapshot-controller/main.go b/cmd/snapshot-controller/main.go index 8308edbe..8a6c854a 100644 --- a/cmd/snapshot-controller/main.go +++ b/cmd/snapshot-controller/main.go @@ -122,9 +122,9 @@ func main() { ctrl := controller.NewCSISnapshotCommonController( snapClient, kubeClient, - factory.Snapshot().V1beta1().VolumeSnapshots(), - factory.Snapshot().V1beta1().VolumeSnapshotContents(), - factory.Snapshot().V1beta1().VolumeSnapshotClasses(), + factory.Snapshot().V1().VolumeSnapshots(), + factory.Snapshot().V1().VolumeSnapshotContents(), + factory.Snapshot().V1().VolumeSnapshotClasses(), coreFactory.Core().V1().PersistentVolumeClaims(), metricsManager, *resyncPeriod, diff --git a/deploy/kubernetes/webhook-example/admission-configuration-template b/deploy/kubernetes/webhook-example/admission-configuration-template index bbcf9b64..6f43cc71 100644 --- a/deploy/kubernetes/webhook-example/admission-configuration-template +++ b/deploy/kubernetes/webhook-example/admission-configuration-template @@ -6,7 +6,7 @@ webhooks: - name: "validation-webhook.snapshot.storage.k8s.io" rules: - apiGroups: ["snapshot.storage.k8s.io"] - apiVersions: ["v1beta1"] + apiVersions: ["v1", "v1beta1"] operations: ["CREATE", "UPDATE"] resources: ["volumesnapshots", "volumesnapshotcontents"] scope: "*" diff --git a/pkg/common-controller/framework_test.go b/pkg/common-controller/framework_test.go index 1660e1df..3fec5c10 100644 --- a/pkg/common-controller/framework_test.go +++ b/pkg/common-controller/framework_test.go @@ -28,12 +28,12 @@ import ( "testing" "time" - crdv1 "github.com/kubernetes-csi/external-snapshotter/client/v3/apis/volumesnapshot/v1beta1" + crdv1 "github.com/kubernetes-csi/external-snapshotter/client/v3/apis/volumesnapshot/v1" clientset "github.com/kubernetes-csi/external-snapshotter/client/v3/clientset/versioned" "github.com/kubernetes-csi/external-snapshotter/client/v3/clientset/versioned/fake" snapshotscheme "github.com/kubernetes-csi/external-snapshotter/client/v3/clientset/versioned/scheme" informers "github.com/kubernetes-csi/external-snapshotter/client/v3/informers/externalversions" - storagelisters "github.com/kubernetes-csi/external-snapshotter/client/v3/listers/volumesnapshot/v1beta1" + storagelisters "github.com/kubernetes-csi/external-snapshotter/client/v3/listers/volumesnapshot/v1" "github.com/kubernetes-csi/external-snapshotter/v3/pkg/metrics" "github.com/kubernetes-csi/external-snapshotter/v3/pkg/utils" v1 "k8s.io/api/core/v1" @@ -743,9 +743,9 @@ func newTestController(kubeClient kubernetes.Interface, clientset clientset.Inte ctrl := NewCSISnapshotCommonController( clientset, kubeClient, - informerFactory.Snapshot().V1beta1().VolumeSnapshots(), - informerFactory.Snapshot().V1beta1().VolumeSnapshotContents(), - informerFactory.Snapshot().V1beta1().VolumeSnapshotClasses(), + informerFactory.Snapshot().V1().VolumeSnapshots(), + informerFactory.Snapshot().V1().VolumeSnapshotContents(), + informerFactory.Snapshot().V1().VolumeSnapshotClasses(), coreFactory.Core().V1().PersistentVolumeClaims(), metricsManager, 60*time.Second, @@ -803,7 +803,7 @@ func newContent(contentName, boundToSnapshotUID, boundToSnapshotName, snapshotHa if boundToSnapshotName != "" { content.Spec.VolumeSnapshotRef = v1.ObjectReference{ Kind: "VolumeSnapshot", - APIVersion: "snapshot.storage.k8s.io/v1beta1", + APIVersion: "snapshot.storage.k8s.io/v1", UID: types.UID(boundToSnapshotUID), Namespace: testNamespace, Name: boundToSnapshotName, @@ -907,7 +907,7 @@ func newSnapshot( Namespace: testNamespace, UID: types.UID(snapshotUID), ResourceVersion: "1", - SelfLink: "/apis/snapshot.storage.k8s.io/v1beta1/namespaces/" + testNamespace + "/volumesnapshots/" + snapshotName, + SelfLink: "/apis/snapshot.storage.k8s.io/v1/namespaces/" + testNamespace + "/volumesnapshots/" + snapshotName, DeletionTimestamp: deletionTimestamp, }, Spec: crdv1.VolumeSnapshotSpec{ @@ -970,7 +970,7 @@ func newSnapshotClass(snapshotClassName, snapshotClassUID, driverName string, is Namespace: testNamespace, UID: types.UID(snapshotClassUID), ResourceVersion: "1", - SelfLink: "/apis/snapshot.storage.k8s.io/v1beta1/namespaces/" + testNamespace + "/volumesnapshotclasses/" + snapshotClassName, + SelfLink: "/apis/snapshot.storage.k8s.io/v1/namespaces/" + testNamespace + "/volumesnapshotclasses/" + snapshotClassName, }, Driver: driverName, } diff --git a/pkg/common-controller/snapshot_controller.go b/pkg/common-controller/snapshot_controller.go index 63a2458d..53ebc66e 100644 --- a/pkg/common-controller/snapshot_controller.go +++ b/pkg/common-controller/snapshot_controller.go @@ -31,7 +31,7 @@ import ( ref "k8s.io/client-go/tools/reference" klog "k8s.io/klog/v2" - crdv1 "github.com/kubernetes-csi/external-snapshotter/client/v3/apis/volumesnapshot/v1beta1" + crdv1 "github.com/kubernetes-csi/external-snapshotter/client/v3/apis/volumesnapshot/v1" "github.com/kubernetes-csi/external-snapshotter/v3/pkg/metrics" "github.com/kubernetes-csi/external-snapshotter/v3/pkg/utils" ) @@ -321,7 +321,7 @@ func (ctrl *csiSnapshotCommonController) checkandRemoveSnapshotFinalizersAndChec // content won't be deleted immediately due to the VolumeSnapshotContentFinalizer if content != nil && deleteContent { klog.V(5).Infof("checkandRemoveSnapshotFinalizersAndCheckandDeleteContent: set DeletionTimeStamp on content [%s].", content.Name) - err := ctrl.clientset.SnapshotV1beta1().VolumeSnapshotContents().Delete(context.TODO(), content.Name, metav1.DeleteOptions{}) + err := ctrl.clientset.SnapshotV1().VolumeSnapshotContents().Delete(context.TODO(), content.Name, metav1.DeleteOptions{}) if err != nil { ctrl.eventRecorder.Event(snapshot, v1.EventTypeWarning, "SnapshotContentObjectDeleteError", "Failed to delete snapshot content API object") return fmt.Errorf("failed to delete VolumeSnapshotContent %s from API server: %q", content.Name, err) @@ -682,7 +682,7 @@ func (ctrl *csiSnapshotCommonController) createSnapshotContent(snapshot *crdv1.V klog.V(5).Infof("volume snapshot content %#v", snapshotContent) // Try to create the VolumeSnapshotContent object klog.V(5).Infof("createSnapshotContent [%s]: trying to save volume snapshot content %s", utils.SnapshotKey(snapshot), snapshotContent.Name) - if updateContent, err = ctrl.clientset.SnapshotV1beta1().VolumeSnapshotContents().Create(context.TODO(), snapshotContent, metav1.CreateOptions{}); err == nil || apierrs.IsAlreadyExists(err) { + if updateContent, err = ctrl.clientset.SnapshotV1().VolumeSnapshotContents().Create(context.TODO(), snapshotContent, metav1.CreateOptions{}); err == nil || apierrs.IsAlreadyExists(err) { // Save succeeded. if err != nil { klog.V(3).Infof("volume snapshot content %q for snapshot %q already exists, reusing", snapshotContent.Name, utils.SnapshotKey(snapshot)) @@ -780,7 +780,7 @@ func (ctrl *csiSnapshotCommonController) updateSnapshotErrorStatusWithEvent(snap snapshotClone.Status.Error = statusError ready := false snapshotClone.Status.ReadyToUse = &ready - newSnapshot, err := ctrl.clientset.SnapshotV1beta1().VolumeSnapshots(snapshotClone.Namespace).UpdateStatus(context.TODO(), snapshotClone, metav1.UpdateOptions{}) + newSnapshot, err := ctrl.clientset.SnapshotV1().VolumeSnapshots(snapshotClone.Namespace).UpdateStatus(context.TODO(), snapshotClone, metav1.UpdateOptions{}) // Emit the event even if the status update fails so that user can see the error ctrl.eventRecorder.Event(newSnapshot, eventtype, reason, message) @@ -804,7 +804,7 @@ func (ctrl *csiSnapshotCommonController) addContentFinalizer(content *crdv1.Volu contentClone := content.DeepCopy() contentClone.ObjectMeta.Finalizers = append(contentClone.ObjectMeta.Finalizers, utils.VolumeSnapshotContentFinalizer) - _, err := ctrl.clientset.SnapshotV1beta1().VolumeSnapshotContents().Update(context.TODO(), contentClone, metav1.UpdateOptions{}) + _, err := ctrl.clientset.SnapshotV1().VolumeSnapshotContents().Update(context.TODO(), contentClone, metav1.UpdateOptions{}) if err != nil { return newControllerUpdateError(content.Name, err.Error()) } @@ -981,7 +981,7 @@ func (ctrl *csiSnapshotCommonController) checkandBindSnapshotContent(snapshot *c className := *(snapshot.Spec.VolumeSnapshotClassName) contentClone.Spec.VolumeSnapshotClassName = &className } - newContent, err := ctrl.clientset.SnapshotV1beta1().VolumeSnapshotContents().Update(context.TODO(), contentClone, metav1.UpdateOptions{}) + newContent, err := ctrl.clientset.SnapshotV1().VolumeSnapshotContents().Update(context.TODO(), contentClone, metav1.UpdateOptions{}) if err != nil { klog.V(4).Infof("updating VolumeSnapshotContent[%s] error status failed %v", contentClone.Name, err) return nil, err @@ -998,7 +998,7 @@ func (ctrl *csiSnapshotCommonController) checkandBindSnapshotContent(snapshot *c // This routine sets snapshot.Spec.Source.VolumeSnapshotContentName func (ctrl *csiSnapshotCommonController) bindandUpdateVolumeSnapshot(snapshotContent *crdv1.VolumeSnapshotContent, snapshot *crdv1.VolumeSnapshot) (*crdv1.VolumeSnapshot, error) { klog.V(5).Infof("bindandUpdateVolumeSnapshot for snapshot [%s]: snapshotContent [%s]", snapshot.Name, snapshotContent.Name) - snapshotObj, err := ctrl.clientset.SnapshotV1beta1().VolumeSnapshots(snapshot.Namespace).Get(context.TODO(), snapshot.Name, metav1.GetOptions{}) + snapshotObj, err := ctrl.clientset.SnapshotV1().VolumeSnapshots(snapshot.Namespace).Get(context.TODO(), snapshot.Name, metav1.GetOptions{}) if err != nil { return nil, fmt.Errorf("error get snapshot %s from api server: %v", utils.SnapshotKey(snapshot), err) } @@ -1086,7 +1086,7 @@ func (ctrl *csiSnapshotCommonController) updateSnapshotStatus(snapshot *crdv1.Vo klog.V(5).Infof("updateSnapshotStatus: updating VolumeSnapshot [%+v] based on VolumeSnapshotContentStatus [%+v]", snapshot, content.Status) - snapshotObj, err := ctrl.clientset.SnapshotV1beta1().VolumeSnapshots(snapshot.Namespace).Get(context.TODO(), snapshot.Name, metav1.GetOptions{}) + snapshotObj, err := ctrl.clientset.SnapshotV1().VolumeSnapshots(snapshot.Namespace).Get(context.TODO(), snapshot.Name, metav1.GetOptions{}) if err != nil { return nil, fmt.Errorf("error get snapshot %s from api server: %v", utils.SnapshotKey(snapshot), err) } @@ -1157,7 +1157,7 @@ func (ctrl *csiSnapshotCommonController) updateSnapshotStatus(snapshot *crdv1.Vo ctrl.metricsManager.RecordMetrics(createAndReadyOperation, metrics.NewSnapshotOperationStatus(metrics.SnapshotStatusTypeSuccess), driverName) } - newSnapshotObj, err := ctrl.clientset.SnapshotV1beta1().VolumeSnapshots(snapshotClone.Namespace).UpdateStatus(context.TODO(), snapshotClone, metav1.UpdateOptions{}) + newSnapshotObj, err := ctrl.clientset.SnapshotV1().VolumeSnapshots(snapshotClone.Namespace).UpdateStatus(context.TODO(), snapshotClone, metav1.UpdateOptions{}) if err != nil { return nil, newControllerUpdateError(utils.SnapshotKey(snapshot), err.Error()) } @@ -1322,7 +1322,7 @@ func (ctrl *csiSnapshotCommonController) SetDefaultSnapshotClass(snapshot *crdv1 klog.V(5).Infof("setDefaultSnapshotClass [%s]: default VolumeSnapshotClassName [%s]", snapshot.Name, defaultClasses[0].Name) snapshotClone := snapshot.DeepCopy() snapshotClone.Spec.VolumeSnapshotClassName = &(defaultClasses[0].Name) - newSnapshot, err := ctrl.clientset.SnapshotV1beta1().VolumeSnapshots(snapshotClone.Namespace).Update(context.TODO(), snapshotClone, metav1.UpdateOptions{}) + newSnapshot, err := ctrl.clientset.SnapshotV1().VolumeSnapshots(snapshotClone.Namespace).Update(context.TODO(), snapshotClone, metav1.UpdateOptions{}) if err != nil { klog.V(4).Infof("updating VolumeSnapshot[%s] default class failed %v", utils.SnapshotKey(snapshot), err) } @@ -1387,7 +1387,7 @@ func (ctrl *csiSnapshotCommonController) addSnapshotFinalizer(snapshot *crdv1.Vo if addBoundFinalizer { snapshotClone.ObjectMeta.Finalizers = append(snapshotClone.ObjectMeta.Finalizers, utils.VolumeSnapshotBoundFinalizer) } - _, err := ctrl.clientset.SnapshotV1beta1().VolumeSnapshots(snapshotClone.Namespace).Update(context.TODO(), snapshotClone, metav1.UpdateOptions{}) + _, err := ctrl.clientset.SnapshotV1().VolumeSnapshots(snapshotClone.Namespace).Update(context.TODO(), snapshotClone, metav1.UpdateOptions{}) if err != nil { return newControllerUpdateError(utils.SnapshotKey(snapshot), err.Error()) } @@ -1431,7 +1431,7 @@ func (ctrl *csiSnapshotCommonController) removeSnapshotFinalizer(snapshot *crdv1 if removeBoundFinalizer { snapshotClone.ObjectMeta.Finalizers = utils.RemoveString(snapshotClone.ObjectMeta.Finalizers, utils.VolumeSnapshotBoundFinalizer) } - _, err := ctrl.clientset.SnapshotV1beta1().VolumeSnapshots(snapshotClone.Namespace).Update(context.TODO(), snapshotClone, metav1.UpdateOptions{}) + _, err := ctrl.clientset.SnapshotV1().VolumeSnapshots(snapshotClone.Namespace).Update(context.TODO(), snapshotClone, metav1.UpdateOptions{}) if err != nil { return newControllerUpdateError(snapshot.Name, err.Error()) } @@ -1479,7 +1479,7 @@ func (ctrl *csiSnapshotCommonController) setAnnVolumeSnapshotBeingDeleted(conten klog.V(5).Infof("setAnnVolumeSnapshotBeingDeleted: set annotation [%s] on content [%s].", utils.AnnVolumeSnapshotBeingDeleted, content.Name) metav1.SetMetaDataAnnotation(&content.ObjectMeta, utils.AnnVolumeSnapshotBeingDeleted, "yes") - updateContent, err := ctrl.clientset.SnapshotV1beta1().VolumeSnapshotContents().Update(context.TODO(), content, metav1.UpdateOptions{}) + updateContent, err := ctrl.clientset.SnapshotV1().VolumeSnapshotContents().Update(context.TODO(), content, metav1.UpdateOptions{}) if err != nil { return newControllerUpdateError(content.Name, err.Error()) } @@ -1499,7 +1499,7 @@ func (ctrl *csiSnapshotCommonController) setAnnVolumeSnapshotBeingDeleted(conten // checkAndSetInvalidContentLabel adds a label to unlabeled invalid content objects and removes the label from valid ones. func (ctrl *csiSnapshotCommonController) checkAndSetInvalidContentLabel(content *crdv1.VolumeSnapshotContent) (*crdv1.VolumeSnapshotContent, error) { hasLabel := utils.MapContainsKey(content.ObjectMeta.Labels, utils.VolumeSnapshotContentInvalidLabel) - err := utils.ValidateSnapshotContent(content) + err := utils.ValidateV1SnapshotContent(content) if err != nil { klog.Errorf("syncContent[%s]: Invalid content detected, %s", content.Name, err.Error()) } @@ -1519,7 +1519,7 @@ func (ctrl *csiSnapshotCommonController) checkAndSetInvalidContentLabel(content } contentClone.ObjectMeta.Labels[utils.VolumeSnapshotContentInvalidLabel] = "" } - updatedContent, err := ctrl.clientset.SnapshotV1beta1().VolumeSnapshotContents().Update(context.TODO(), contentClone, metav1.UpdateOptions{}) + updatedContent, err := ctrl.clientset.SnapshotV1().VolumeSnapshotContents().Update(context.TODO(), contentClone, metav1.UpdateOptions{}) if err != nil { return content, newControllerUpdateError(content.Name, err.Error()) } @@ -1540,7 +1540,7 @@ func (ctrl *csiSnapshotCommonController) checkAndSetInvalidContentLabel(content // checkAndSetInvalidSnapshotLabel adds a label to unlabeled invalid snapshot objects and removes the label from valid ones. func (ctrl *csiSnapshotCommonController) checkAndSetInvalidSnapshotLabel(snapshot *crdv1.VolumeSnapshot) (*crdv1.VolumeSnapshot, error) { hasLabel := utils.MapContainsKey(snapshot.ObjectMeta.Labels, utils.VolumeSnapshotInvalidLabel) - err := utils.ValidateSnapshot(snapshot) + err := utils.ValidateV1Snapshot(snapshot) if err != nil { klog.Errorf("syncSnapshot[%s]: Invalid snapshot detected, %s", utils.SnapshotKey(snapshot), err.Error()) } @@ -1561,7 +1561,7 @@ func (ctrl *csiSnapshotCommonController) checkAndSetInvalidSnapshotLabel(snapsho snapshotClone.ObjectMeta.Labels[utils.VolumeSnapshotInvalidLabel] = "" } - updatedSnapshot, err := ctrl.clientset.SnapshotV1beta1().VolumeSnapshots(snapshot.Namespace).Update(context.TODO(), snapshotClone, metav1.UpdateOptions{}) + updatedSnapshot, err := ctrl.clientset.SnapshotV1().VolumeSnapshots(snapshot.Namespace).Update(context.TODO(), snapshotClone, metav1.UpdateOptions{}) if err != nil { return snapshot, newControllerUpdateError(utils.SnapshotKey(snapshot), err.Error()) } diff --git a/pkg/common-controller/snapshot_controller_base.go b/pkg/common-controller/snapshot_controller_base.go index dbd8f9d0..afafbecc 100644 --- a/pkg/common-controller/snapshot_controller_base.go +++ b/pkg/common-controller/snapshot_controller_base.go @@ -20,10 +20,10 @@ import ( "fmt" "time" - crdv1 "github.com/kubernetes-csi/external-snapshotter/client/v3/apis/volumesnapshot/v1beta1" + crdv1 "github.com/kubernetes-csi/external-snapshotter/client/v3/apis/volumesnapshot/v1" clientset "github.com/kubernetes-csi/external-snapshotter/client/v3/clientset/versioned" - storageinformers "github.com/kubernetes-csi/external-snapshotter/client/v3/informers/externalversions/volumesnapshot/v1beta1" - storagelisters "github.com/kubernetes-csi/external-snapshotter/client/v3/listers/volumesnapshot/v1beta1" + storageinformers "github.com/kubernetes-csi/external-snapshotter/client/v3/informers/externalversions/volumesnapshot/v1" + storagelisters "github.com/kubernetes-csi/external-snapshotter/client/v3/listers/volumesnapshot/v1" "github.com/kubernetes-csi/external-snapshotter/v3/pkg/metrics" "github.com/kubernetes-csi/external-snapshotter/v3/pkg/utils" diff --git a/pkg/common-controller/snapshot_controller_test.go b/pkg/common-controller/snapshot_controller_test.go index 5bd430f5..61059206 100644 --- a/pkg/common-controller/snapshot_controller_test.go +++ b/pkg/common-controller/snapshot_controller_test.go @@ -19,7 +19,7 @@ package common_controller import ( "testing" - crdv1 "github.com/kubernetes-csi/external-snapshotter/client/v3/apis/volumesnapshot/v1beta1" + crdv1 "github.com/kubernetes-csi/external-snapshotter/client/v3/apis/volumesnapshot/v1" "github.com/kubernetes-csi/external-snapshotter/v3/pkg/utils" "k8s.io/client-go/tools/cache" ) diff --git a/pkg/common-controller/snapshot_create_test.go b/pkg/common-controller/snapshot_create_test.go index 0eb27e8f..85c0eba4 100644 --- a/pkg/common-controller/snapshot_create_test.go +++ b/pkg/common-controller/snapshot_create_test.go @@ -21,7 +21,7 @@ import ( "testing" "time" - crdv1 "github.com/kubernetes-csi/external-snapshotter/client/v3/apis/volumesnapshot/v1beta1" + crdv1 "github.com/kubernetes-csi/external-snapshotter/client/v3/apis/volumesnapshot/v1" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) diff --git a/pkg/common-controller/snapshot_delete_test.go b/pkg/common-controller/snapshot_delete_test.go index ba9f9c1b..777e5e78 100644 --- a/pkg/common-controller/snapshot_delete_test.go +++ b/pkg/common-controller/snapshot_delete_test.go @@ -20,7 +20,7 @@ import ( "errors" "testing" - crdv1 "github.com/kubernetes-csi/external-snapshotter/client/v3/apis/volumesnapshot/v1beta1" + crdv1 "github.com/kubernetes-csi/external-snapshotter/client/v3/apis/volumesnapshot/v1" "github.com/kubernetes-csi/external-snapshotter/v3/pkg/utils" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -195,7 +195,7 @@ func TestDeleteSync(t *testing.T) { expectedEvents: []string{"Warning SnapshotContentObjectDeleteError"}, initialSecrets: []*v1.Secret{secret()}, errors: []reactorError{ - // Inject error to the first client.VolumesnapshotV1beta1().VolumeSnapshotContents().Delete call. + // Inject error to the first client.VolumesnapshotV1().VolumeSnapshotContents().Delete call. // All other calls will succeed. {"delete", "volumesnapshotcontents", errors.New("mock delete error")}, }, @@ -278,7 +278,7 @@ func TestDeleteSync(t *testing.T) { expectedEvents: []string{"Warning SnapshotContentObjectDeleteError"}, initialSecrets: []*v1.Secret{secret()}, errors: []reactorError{ - // Inject error to the first client.VolumesnapshotV1beta1().VolumeSnapshotContents().Delete call. + // Inject error to the first client.VolumesnapshotV1().VolumeSnapshotContents().Delete call. // All other calls will succeed. {"delete", "volumesnapshotcontents", errors.New("mock delete error")}, }, diff --git a/pkg/common-controller/snapshot_update_test.go b/pkg/common-controller/snapshot_update_test.go index 4d36dae8..e5eed7b2 100644 --- a/pkg/common-controller/snapshot_update_test.go +++ b/pkg/common-controller/snapshot_update_test.go @@ -21,7 +21,7 @@ import ( "testing" "time" - crdv1 "github.com/kubernetes-csi/external-snapshotter/client/v3/apis/volumesnapshot/v1beta1" + crdv1 "github.com/kubernetes-csi/external-snapshotter/client/v3/apis/volumesnapshot/v1" "github.com/kubernetes-csi/external-snapshotter/v3/pkg/utils" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -117,7 +117,7 @@ func TestSync(t *testing.T) { initialVolumes: newVolumeArray("volume2-8", "pv-uid2-8", "pv-handle2-8", "1Gi", "pvc-uid2-8", "claim2-8", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty), initialSecrets: []*v1.Secret{secret()}, errors: []reactorError{ - // Inject error to the first client.VolumesnapshotV1beta1().VolumeSnapshots().Update call. + // Inject error to the first client.VolumesnapshotV1().VolumeSnapshots().Update call. // All other calls will succeed. {"update", "volumesnapshots", errors.New("mock update error")}, }, @@ -161,7 +161,7 @@ func TestSync(t *testing.T) { initialSnapshots: newSnapshotArray("snap2-12", "snapuid2-12", "", "content2-12", validSecretClass, "content2-12", &False, nil, nil, nil, false, true, nil), expectedSnapshots: newSnapshotArray("snap2-12", "snapuid2-12", "", "content2-12", validSecretClass, "content2-12", &False, nil, nil, newVolumeError("Snapshot failed to bind VolumeSnapshotContent, mock update error"), false, true, nil), errors: []reactorError{ - // Inject error to the forth client.VolumesnapshotV1beta1().VolumeSnapshots().Update call. + // Inject error to the forth client.VolumesnapshotV1().VolumeSnapshots().Update call. {"update", "volumesnapshotcontents", errors.New("mock update error")}, }, test: testSyncSnapshot, @@ -311,7 +311,7 @@ func TestSync(t *testing.T) { initialVolumes: newVolumeArray("volume5-2", "pv-uid5-2", "pv-handle5-2", "1Gi", "pvc-uid5-2", "claim5-2", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty), initialSecrets: []*v1.Secret{secret()}, errors: []reactorError{ - // Inject error to the forth client.VolumesnapshotV1beta1().VolumeSnapshots().Update call. + // Inject error to the forth client.VolumesnapshotV1().VolumeSnapshots().Update call. {"update", "volumesnapshotcontents", errors.New("mock update error")}, }, expectSuccess: false, @@ -340,7 +340,7 @@ func TestSync(t *testing.T) { initialVolumes: newVolumeArray("volume5-4", "pv-uid5-4", "pv-handle5-4", "1Gi", "pvc-uid5-4", "claim5-4", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty), initialSecrets: []*v1.Secret{secret()}, errors: []reactorError{ - // Inject error to the forth client.VolumesnapshotV1beta1().VolumeSnapshots().Update call. + // Inject error to the forth client.VolumesnapshotV1().VolumeSnapshots().Update call. {"update", "volumesnapshotcontents", errors.New("mock update error")}, }, expectSuccess: false, @@ -377,7 +377,7 @@ func TestSync(t *testing.T) { expectedContents: withContentAnnotations(newContentArray("content5-7", "snapuid5-7", "snap5-7", "sid5-7", validSecretClass, "sid5-7", "", deletionPolicy, nil, nil, true), map[string]string{utils.AnnVolumeSnapshotBeingDeleted: "yes"}), initialSecrets: []*v1.Secret{secret()}, errors: []reactorError{ - // Inject error to the forth client.VolumesnapshotV1beta1().VolumeSnapshots().Update call. + // Inject error to the forth client.VolumesnapshotV1().VolumeSnapshots().Update call. {"update", "volumesnapshotcontents", errors.New("mock update error")}, }, expectSuccess: false, @@ -406,54 +406,6 @@ func TestSync(t *testing.T) { expectSuccess: false, test: testSyncSnapshot, }, - // TODO(xiangqian@): remove test cases 7-2/7-3 when webhooks are in place - // https://github.com/kubernetes-csi/external-snapshotter/issues/187 - { - name: "7-2 - validation fail if neither VolumeSnapshotContentName nor PersistentVolumeClaimName has been specified in Snapshot.Spec.Source", - initialContents: nocontents, - expectedContents: nocontents, - initialSnapshots: newSnapshotArray("snap7-2", "snapuid7-2", "", "", validSecretClass, "", &False, nil, nil, nil, false, true, nil), - expectedSnapshots: withSnapshotInvalidLabel(newSnapshotArray("snap7-2", "snapuid7-2", "", "", validSecretClass, "", &False, nil, nil, newVolumeError("Exactly one of PersistentVolumeClaimName and VolumeSnapshotContentName should be specified"), false, true, nil)), - expectedEvents: []string{"Warning SnapshotValidationError"}, - errors: noerrors, - expectSuccess: false, - test: testSyncSnapshot, - }, - { - name: "7-3 - validation fail if both VolumeSnapshotContentName and PersistentVolumeClaimName have been specified in Snapshot.Spec.Source", - initialContents: nocontents, - expectedContents: nocontents, - initialSnapshots: newSnapshotArray("snap7-3", "snapuid7-3", "claim7-3", "snaphandle7-3", validSecretClass, "", &False, nil, nil, nil, false, true, nil), - expectedSnapshots: withSnapshotInvalidLabel(newSnapshotArray("snap7-3", "snapuid7-3", "claim7-3", "snaphandle7-3", validSecretClass, "", &False, nil, nil, newVolumeError("Exactly one of PersistentVolumeClaimName and VolumeSnapshotContentName should be specified"), false, true, nil)), - initialClaims: newClaimArray("claim7-3", "pvc-uid7-3", "1Gi", "volume7-3", v1.ClaimBound, &classEmpty), - initialVolumes: newVolumeArray("volume7-3", "pv-uid7-3", "pv-handle7-3", "1Gi", "pvc-uid7-3", "claim7-3", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty), - expectedEvents: []string{"Warning SnapshotValidationError"}, - errors: noerrors, - expectSuccess: false, - test: testSyncSnapshot, - }, - { - name: "7-4 - validation fail if both SnapshotHandle and VolumeHandle have been specified in Content.Spec.Source", - initialSnapshots: nosnapshots, - expectedSnapshots: nosnapshots, - initialContents: newContentArray("content7-4", "snapuid7-4", "snap7-4", "sid7-4", validSecretClass, "sid7-4", "pv-handle7-4", deletionPolicy, nil, nil, true), - expectedContents: withSnapshotContentInvalidLabel(newContentArray("content7-4", "snapuid7-4", "snap7-4", "sid7-4", validSecretClass, "sid7-4", "pv-handle7-4", deletionPolicy, nil, nil, true)), - expectedEvents: []string{"Warning ContentValidationError"}, - errors: noerrors, - expectSuccess: false, - test: testSyncContentError, - }, - { - name: "7-5 - validation fail if neither SnapshotHandle or VolumeHandle has been specified in Content.Spec.Source", - initialSnapshots: nosnapshots, - expectedSnapshots: nosnapshots, - initialContents: newContentArray("content7-4", "snapuid7-4", "snap7-4", "sid7-4", validSecretClass, "", "", deletionPolicy, nil, nil, true), - expectedContents: withSnapshotContentInvalidLabel(newContentArray("content7-4", "snapuid7-4", "snap7-4", "sid7-4", validSecretClass, "", "", deletionPolicy, nil, nil, true)), - expectedEvents: []string{"Warning ContentValidationError"}, - errors: noerrors, - expectSuccess: false, - test: testSyncContentError, - }, { // Update Error in snapshot status based on content status name: "6-1 - update snapshot error status", diff --git a/pkg/sidecar-controller/content_create_test.go b/pkg/sidecar-controller/content_create_test.go index 9c0b066b..817e4869 100644 --- a/pkg/sidecar-controller/content_create_test.go +++ b/pkg/sidecar-controller/content_create_test.go @@ -21,7 +21,7 @@ import ( "testing" "time" - crdv1 "github.com/kubernetes-csi/external-snapshotter/client/v3/apis/volumesnapshot/v1beta1" + crdv1 "github.com/kubernetes-csi/external-snapshotter/client/v3/apis/volumesnapshot/v1" "github.com/kubernetes-csi/external-snapshotter/v3/pkg/utils" v1 "k8s.io/api/core/v1" ) @@ -165,7 +165,7 @@ func TestSyncContent(t *testing.T) { }), initialSecrets: []*v1.Secret{}, // no initial secret created expectedEvents: []string{"Warning SnapshotCreationFailed"}, errors: []reactorError{ - // Inject error to the first client.VolumesnapshotV1beta1().VolumeSnapshots().Update call. + // Inject error to the first client.VolumesnapshotV1().VolumeSnapshots().Update call. // All other calls will succeed. {"get", "secrets", errors.New("mock secrets error")}, }, diff --git a/pkg/sidecar-controller/csi_handler.go b/pkg/sidecar-controller/csi_handler.go index 96628200..d69b1b6d 100644 --- a/pkg/sidecar-controller/csi_handler.go +++ b/pkg/sidecar-controller/csi_handler.go @@ -22,7 +22,7 @@ import ( "strings" "time" - crdv1 "github.com/kubernetes-csi/external-snapshotter/client/v3/apis/volumesnapshot/v1beta1" + crdv1 "github.com/kubernetes-csi/external-snapshotter/client/v3/apis/volumesnapshot/v1" "github.com/kubernetes-csi/external-snapshotter/v3/pkg/snapshotter" ) diff --git a/pkg/sidecar-controller/framework_test.go b/pkg/sidecar-controller/framework_test.go index 652c15a2..9cff279f 100644 --- a/pkg/sidecar-controller/framework_test.go +++ b/pkg/sidecar-controller/framework_test.go @@ -25,12 +25,12 @@ import ( "testing" "time" - crdv1 "github.com/kubernetes-csi/external-snapshotter/client/v3/apis/volumesnapshot/v1beta1" + crdv1 "github.com/kubernetes-csi/external-snapshotter/client/v3/apis/volumesnapshot/v1" clientset "github.com/kubernetes-csi/external-snapshotter/client/v3/clientset/versioned" "github.com/kubernetes-csi/external-snapshotter/client/v3/clientset/versioned/fake" snapshotscheme "github.com/kubernetes-csi/external-snapshotter/client/v3/clientset/versioned/scheme" informers "github.com/kubernetes-csi/external-snapshotter/client/v3/informers/externalversions" - storagelisters "github.com/kubernetes-csi/external-snapshotter/client/v3/listers/volumesnapshot/v1beta1" + storagelisters "github.com/kubernetes-csi/external-snapshotter/client/v3/listers/volumesnapshot/v1" "github.com/kubernetes-csi/external-snapshotter/v3/pkg/utils" v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" @@ -514,8 +514,8 @@ func newTestController(kubeClient kubernetes.Interface, clientset clientset.Inte clientset, kubeClient, mockDriverName, - informerFactory.Snapshot().V1beta1().VolumeSnapshotContents(), - informerFactory.Snapshot().V1beta1().VolumeSnapshotClasses(), + informerFactory.Snapshot().V1().VolumeSnapshotContents(), + informerFactory.Snapshot().V1().VolumeSnapshotClasses(), fakeSnapshot, 5*time.Millisecond, 60*time.Second, @@ -578,7 +578,7 @@ func newContent(contentName, boundToSnapshotUID, boundToSnapshotName, snapshotHa if boundToSnapshotName != "" { content.Spec.VolumeSnapshotRef = v1.ObjectReference{ Kind: "VolumeSnapshot", - APIVersion: "snapshot.storage.k8s.io/v1beta1", + APIVersion: "snapshot.storage.k8s.io/v1", UID: types.UID(boundToSnapshotUID), Namespace: testNamespace, Name: boundToSnapshotName, diff --git a/pkg/sidecar-controller/snapshot_controller.go b/pkg/sidecar-controller/snapshot_controller.go index 7b6ea128..7218ef48 100644 --- a/pkg/sidecar-controller/snapshot_controller.go +++ b/pkg/sidecar-controller/snapshot_controller.go @@ -22,7 +22,7 @@ import ( "strings" "time" - crdv1 "github.com/kubernetes-csi/external-snapshotter/client/v3/apis/volumesnapshot/v1beta1" + crdv1 "github.com/kubernetes-csi/external-snapshotter/client/v3/apis/volumesnapshot/v1" "github.com/kubernetes-csi/external-snapshotter/v3/pkg/utils" codes "google.golang.org/grpc/codes" "google.golang.org/grpc/status" @@ -156,7 +156,7 @@ func (ctrl *csiSnapshotSideCarController) updateContentErrorStatusWithEvent(cont } ready := false contentClone.Status.ReadyToUse = &ready - newContent, err := ctrl.clientset.SnapshotV1beta1().VolumeSnapshotContents().UpdateStatus(context.TODO(), contentClone, metav1.UpdateOptions{}) + newContent, err := ctrl.clientset.SnapshotV1().VolumeSnapshotContents().UpdateStatus(context.TODO(), contentClone, metav1.UpdateOptions{}) // Emit the event even if the status update fails so that user can see the error ctrl.eventRecorder.Event(newContent, eventtype, reason, message) @@ -367,7 +367,7 @@ func (ctrl *csiSnapshotSideCarController) clearVolumeContentStatus( contentName string) (*crdv1.VolumeSnapshotContent, error) { klog.V(5).Infof("cleanVolumeSnapshotStatus content [%s]", contentName) // get the latest version from API server - content, err := ctrl.clientset.SnapshotV1beta1().VolumeSnapshotContents().Get(context.TODO(), contentName, metav1.GetOptions{}) + content, err := ctrl.clientset.SnapshotV1().VolumeSnapshotContents().Get(context.TODO(), contentName, metav1.GetOptions{}) if err != nil { return nil, fmt.Errorf("error get snapshot content %s from api server: %v", contentName, err) } @@ -377,7 +377,7 @@ func (ctrl *csiSnapshotSideCarController) clearVolumeContentStatus( content.Status.CreationTime = nil content.Status.RestoreSize = nil } - newContent, err := ctrl.clientset.SnapshotV1beta1().VolumeSnapshotContents().UpdateStatus(context.TODO(), content, metav1.UpdateOptions{}) + newContent, err := ctrl.clientset.SnapshotV1().VolumeSnapshotContents().UpdateStatus(context.TODO(), content, metav1.UpdateOptions{}) if err != nil { return nil, newControllerUpdateError(contentName, err.Error()) } @@ -392,7 +392,7 @@ func (ctrl *csiSnapshotSideCarController) updateSnapshotContentStatus( size int64) (*crdv1.VolumeSnapshotContent, error) { klog.V(5).Infof("updateSnapshotContentStatus: updating VolumeSnapshotContent [%s], snapshotHandle %s, readyToUse %v, createdAt %v, size %d", content.Name, snapshotHandle, readyToUse, createdAt, size) - contentObj, err := ctrl.clientset.SnapshotV1beta1().VolumeSnapshotContents().Get(context.TODO(), content.Name, metav1.GetOptions{}) + contentObj, err := ctrl.clientset.SnapshotV1().VolumeSnapshotContents().Get(context.TODO(), content.Name, metav1.GetOptions{}) if err != nil { return nil, fmt.Errorf("error get snapshot content %s from api server: %v", content.Name, err) } @@ -433,7 +433,7 @@ func (ctrl *csiSnapshotSideCarController) updateSnapshotContentStatus( if updated { contentClone := contentObj.DeepCopy() contentClone.Status = newStatus - newContent, err := ctrl.clientset.SnapshotV1beta1().VolumeSnapshotContents().UpdateStatus(context.TODO(), contentClone, metav1.UpdateOptions{}) + newContent, err := ctrl.clientset.SnapshotV1().VolumeSnapshotContents().UpdateStatus(context.TODO(), contentClone, metav1.UpdateOptions{}) if err != nil { return nil, newControllerUpdateError(content.Name, err.Error()) } @@ -521,7 +521,7 @@ func (ctrl csiSnapshotSideCarController) removeContentFinalizer(content *crdv1.V contentClone := content.DeepCopy() contentClone.ObjectMeta.Finalizers = utils.RemoveString(contentClone.ObjectMeta.Finalizers, utils.VolumeSnapshotContentFinalizer) - _, err := ctrl.clientset.SnapshotV1beta1().VolumeSnapshotContents().Update(context.TODO(), contentClone, metav1.UpdateOptions{}) + _, err := ctrl.clientset.SnapshotV1().VolumeSnapshotContents().Update(context.TODO(), contentClone, metav1.UpdateOptions{}) if err != nil { return newControllerUpdateError(content.Name, err.Error()) } @@ -578,7 +578,7 @@ func (ctrl *csiSnapshotSideCarController) setAnnVolumeSnapshotBeingCreated(conte contentClone := content.DeepCopy() metav1.SetMetaDataAnnotation(&contentClone.ObjectMeta, utils.AnnVolumeSnapshotBeingCreated, "yes") - updatedContent, err := ctrl.clientset.SnapshotV1beta1().VolumeSnapshotContents().Update(context.TODO(), contentClone, metav1.UpdateOptions{}) + updatedContent, err := ctrl.clientset.SnapshotV1().VolumeSnapshotContents().Update(context.TODO(), contentClone, metav1.UpdateOptions{}) if err != nil { return newControllerUpdateError(content.Name, err.Error()) } @@ -604,7 +604,7 @@ func (ctrl csiSnapshotSideCarController) removeAnnVolumeSnapshotBeingCreated(con contentClone := content.DeepCopy() delete(contentClone.ObjectMeta.Annotations, utils.AnnVolumeSnapshotBeingCreated) - updatedContent, err := ctrl.clientset.SnapshotV1beta1().VolumeSnapshotContents().Update(context.TODO(), contentClone, metav1.UpdateOptions{}) + updatedContent, err := ctrl.clientset.SnapshotV1().VolumeSnapshotContents().Update(context.TODO(), contentClone, metav1.UpdateOptions{}) if err != nil { return newControllerUpdateError(content.Name, err.Error()) } diff --git a/pkg/sidecar-controller/snapshot_controller_base.go b/pkg/sidecar-controller/snapshot_controller_base.go index 92a80057..938ee46b 100644 --- a/pkg/sidecar-controller/snapshot_controller_base.go +++ b/pkg/sidecar-controller/snapshot_controller_base.go @@ -20,10 +20,10 @@ import ( "fmt" "time" - crdv1 "github.com/kubernetes-csi/external-snapshotter/client/v3/apis/volumesnapshot/v1beta1" + crdv1 "github.com/kubernetes-csi/external-snapshotter/client/v3/apis/volumesnapshot/v1" clientset "github.com/kubernetes-csi/external-snapshotter/client/v3/clientset/versioned" - storageinformers "github.com/kubernetes-csi/external-snapshotter/client/v3/informers/externalversions/volumesnapshot/v1beta1" - storagelisters "github.com/kubernetes-csi/external-snapshotter/client/v3/listers/volumesnapshot/v1beta1" + storageinformers "github.com/kubernetes-csi/external-snapshotter/client/v3/informers/externalversions/volumesnapshot/v1" + storagelisters "github.com/kubernetes-csi/external-snapshotter/client/v3/listers/volumesnapshot/v1" "github.com/kubernetes-csi/external-snapshotter/v3/pkg/snapshotter" "k8s.io/api/core/v1" diff --git a/pkg/sidecar-controller/snapshot_controller_test.go b/pkg/sidecar-controller/snapshot_controller_test.go index e0e454b4..ac611329 100644 --- a/pkg/sidecar-controller/snapshot_controller_test.go +++ b/pkg/sidecar-controller/snapshot_controller_test.go @@ -16,7 +16,7 @@ package sidecar_controller import ( "testing" - crdv1 "github.com/kubernetes-csi/external-snapshotter/client/v3/apis/volumesnapshot/v1beta1" + crdv1 "github.com/kubernetes-csi/external-snapshotter/client/v3/apis/volumesnapshot/v1" "github.com/kubernetes-csi/external-snapshotter/v3/pkg/utils" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/tools/cache" diff --git a/pkg/sidecar-controller/snapshot_delete_test.go b/pkg/sidecar-controller/snapshot_delete_test.go index 7f18abc0..a5c59769 100644 --- a/pkg/sidecar-controller/snapshot_delete_test.go +++ b/pkg/sidecar-controller/snapshot_delete_test.go @@ -23,7 +23,7 @@ import ( "errors" - crdv1 "github.com/kubernetes-csi/external-snapshotter/client/v3/apis/volumesnapshot/v1beta1" + crdv1 "github.com/kubernetes-csi/external-snapshotter/client/v3/apis/volumesnapshot/v1" "github.com/kubernetes-csi/external-snapshotter/v3/pkg/utils" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -227,7 +227,7 @@ func TestDeleteSync(t *testing.T) { expectedEvents: noevents, expectedDeleteCalls: []deleteCall{{"sid1-1", nil, fmt.Errorf("mock csi driver delete error")}}, errors: []reactorError{ - // Inject error to the first client.VolumesnapshotV1beta1().VolumeSnapshotContents().Delete call. + // Inject error to the first client.VolumesnapshotV1().VolumeSnapshotContents().Delete call. // All other calls will succeed. {"get", "secrets", errors.New("mock get invalid secret error")}, }, diff --git a/pkg/utils/util.go b/pkg/utils/util.go index b563ca01..bc31e7d4 100644 --- a/pkg/utils/util.go +++ b/pkg/utils/util.go @@ -24,7 +24,8 @@ import ( "strings" "time" - crdv1 "github.com/kubernetes-csi/external-snapshotter/client/v3/apis/volumesnapshot/v1beta1" + crdv1 "github.com/kubernetes-csi/external-snapshotter/client/v3/apis/volumesnapshot/v1" + crdv1beta1 "github.com/kubernetes-csi/external-snapshotter/client/v3/apis/volumesnapshot/v1beta1" v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -121,10 +122,25 @@ var SnapshotterListSecretParams = secretParamsMap{ secretNamespaceKey: PrefixedSnapshotterListSecretNamespaceKey, } -// ValidateSnapshot performs additional strict validation. +// ValidateV1Snapshot performs additional strict validation. // Do NOT rely on this function to fully validate snapshot objects. // This function will only check the additional rules provided by the webhook. -func ValidateSnapshot(snapshot *crdv1.VolumeSnapshot) error { +func ValidateV1Snapshot(snapshot *crdv1.VolumeSnapshot) error { + if snapshot == nil { + return fmt.Errorf("VolumeSnapshot is nil") + } + + vscname := snapshot.Spec.VolumeSnapshotClassName + if vscname != nil && *vscname == "" { + return fmt.Errorf("Spec.VolumeSnapshotClassName must not be the empty string") + } + return nil +} + +// ValidateV1Beta1Snapshot performs additional strict validation. +// Do NOT rely on this function to fully validate snapshot objects. +// This function will only check the additional rules provided by the webhook. +func ValidateV1Beta1Snapshot(snapshot *crdv1beta1.VolumeSnapshot) error { if snapshot == nil { return fmt.Errorf("VolumeSnapshot is nil") } @@ -144,10 +160,27 @@ func ValidateSnapshot(snapshot *crdv1.VolumeSnapshot) error { return nil } -// ValidateSnapshotContent performs additional strict validation. +// ValidateV1SnapshotContent performs additional strict validation. // Do NOT rely on this function to fully validate snapshot content objects. // This function will only check the additional rules provided by the webhook. -func ValidateSnapshotContent(snapcontent *crdv1.VolumeSnapshotContent) error { +func ValidateV1SnapshotContent(snapcontent *crdv1.VolumeSnapshotContent) error { + if snapcontent == nil { + return fmt.Errorf("VolumeSnapshotContent is nil") + } + + vsref := snapcontent.Spec.VolumeSnapshotRef + + if vsref.Name == "" || vsref.Namespace == "" { + return fmt.Errorf("both Spec.VolumeSnapshotRef.Name = %s and Spec.VolumeSnapshotRef.Namespace = %s must be set", vsref.Name, vsref.Namespace) + } + + return nil +} + +// ValidateV1Beta1SnapshotContent performs additional strict validation. +// Do NOT rely on this function to fully validate snapshot content objects. +// This function will only check the additional rules provided by the webhook. +func ValidateV1Beta1SnapshotContent(snapcontent *crdv1beta1.VolumeSnapshotContent) error { if snapcontent == nil { return fmt.Errorf("VolumeSnapshotContent is nil") } diff --git a/pkg/utils/util_test.go b/pkg/utils/util_test.go index a25d456a..18dc2fd2 100644 --- a/pkg/utils/util_test.go +++ b/pkg/utils/util_test.go @@ -20,7 +20,7 @@ import ( "reflect" "testing" - crdv1 "github.com/kubernetes-csi/external-snapshotter/client/v3/apis/volumesnapshot/v1beta1" + crdv1 "github.com/kubernetes-csi/external-snapshotter/client/v3/apis/volumesnapshot/v1" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) diff --git a/pkg/validation-webhook/snapshot.go b/pkg/validation-webhook/snapshot.go index 4fce49ae..84bcf938 100644 --- a/pkg/validation-webhook/snapshot.go +++ b/pkg/validation-webhook/snapshot.go @@ -20,6 +20,7 @@ import ( "fmt" "reflect" + volumesnapshotv1 "github.com/kubernetes-csi/external-snapshotter/client/v3/apis/volumesnapshot/v1" volumesnapshotv1beta1 "github.com/kubernetes-csi/external-snapshotter/client/v3/apis/volumesnapshot/v1beta1" "github.com/kubernetes-csi/external-snapshotter/v3/pkg/utils" v1 "k8s.io/api/admission/v1" @@ -28,10 +29,14 @@ import ( ) var ( - // SnapshotV1Beta1GVR is GroupVersionResource for volumesnapshots + // SnapshotV1Beta1GVR is GroupVersionResource for v1beta1 VolumeSnapshots SnapshotV1Beta1GVR = metav1.GroupVersionResource{Group: volumesnapshotv1beta1.GroupName, Version: "v1beta1", Resource: "volumesnapshots"} - // SnapshotContentV1Beta1GVR is GroupVersionResource for volumesnapshotcontents + // SnapshotV1GVR is GroupVersionResource for v1 VolumeSnapshots + SnapshotV1GVR = metav1.GroupVersionResource{Group: volumesnapshotv1.GroupName, Version: "v1", Resource: "volumesnapshots"} + // SnapshotContentV1Beta1GVR is GroupVersionResource for v1beta1 VolumeSnapshotContents SnapshotContentV1Beta1GVR = metav1.GroupVersionResource{Group: volumesnapshotv1beta1.GroupName, Version: "v1beta1", Resource: "volumesnapshotcontents"} + // SnapshotContentV1GVR is GroupVersionResource for v1 VolumeSnapshotContents + SnapshotContentV1GVR = metav1.GroupVersionResource{Group: volumesnapshotv1.GroupName, Version: "v1", Resource: "volumesnapshotcontents"} ) // Add a label {"added-label": "yes"} to the object @@ -65,7 +70,19 @@ func admitSnapshot(ar v1.AdmissionReview) *v1.AdmissionResponse { klog.Error(err) return toV1AdmissionResponse(err) } - return decideSnapshot(snapshot, oldSnapshot, isUpdate) + return decideSnapshotV1beta1(snapshot, oldSnapshot, isUpdate) + case SnapshotV1GVR: + snapshot := &volumesnapshotv1.VolumeSnapshot{} + if _, _, err := deserializer.Decode(raw, nil, snapshot); err != nil { + klog.Error(err) + return toV1AdmissionResponse(err) + } + oldSnapshot := &volumesnapshotv1.VolumeSnapshot{} + if _, _, err := deserializer.Decode(oldRaw, nil, oldSnapshot); err != nil { + klog.Error(err) + return toV1AdmissionResponse(err) + } + return decideSnapshotV1(snapshot, oldSnapshot, isUpdate) case SnapshotContentV1Beta1GVR: snapcontent := &volumesnapshotv1beta1.VolumeSnapshotContent{} if _, _, err := deserializer.Decode(raw, nil, snapcontent); err != nil { @@ -77,7 +94,19 @@ func admitSnapshot(ar v1.AdmissionReview) *v1.AdmissionResponse { klog.Error(err) return toV1AdmissionResponse(err) } - return decideSnapshotContent(snapcontent, oldSnapcontent, isUpdate) + return decideSnapshotContentV1beta1(snapcontent, oldSnapcontent, isUpdate) + case SnapshotContentV1GVR: + snapcontent := &volumesnapshotv1.VolumeSnapshotContent{} + if _, _, err := deserializer.Decode(raw, nil, snapcontent); err != nil { + klog.Error(err) + return toV1AdmissionResponse(err) + } + oldSnapcontent := &volumesnapshotv1.VolumeSnapshotContent{} + if _, _, err := deserializer.Decode(oldRaw, nil, oldSnapcontent); err != nil { + klog.Error(err) + return toV1AdmissionResponse(err) + } + return decideSnapshotContentV1(snapcontent, oldSnapcontent, isUpdate) default: err := fmt.Errorf("expect resource to be %s or %s", SnapshotV1Beta1GVR, SnapshotContentV1Beta1GVR) klog.Error(err) @@ -85,7 +114,7 @@ func admitSnapshot(ar v1.AdmissionReview) *v1.AdmissionResponse { } } -func decideSnapshot(snapshot, oldSnapshot *volumesnapshotv1beta1.VolumeSnapshot, isUpdate bool) *v1.AdmissionResponse { +func decideSnapshotV1beta1(snapshot, oldSnapshot *volumesnapshotv1beta1.VolumeSnapshot, isUpdate bool) *v1.AdmissionResponse { reviewResponse := &v1.AdmissionResponse{ Allowed: true, Result: &metav1.Status{}, @@ -97,12 +126,12 @@ func decideSnapshot(snapshot, oldSnapshot *volumesnapshotv1beta1.VolumeSnapshot, // Which allows the remover of finalizers and therefore deletion of this object // Don't rely on the pointers to be nil, because the deserialization method will convert it to // The empty struct value. Instead check the operation type. - if err := utils.ValidateSnapshot(oldSnapshot); err != nil { + if err := utils.ValidateV1Beta1Snapshot(oldSnapshot); err != nil { return reviewResponse } // if it is an UPDATE and oldSnapshot is valid, check immutable fields - if err := checkSnapshotImmutableFields(snapshot, oldSnapshot); err != nil { + if err := checkSnapshotImmutableFieldsV1beta1(snapshot, oldSnapshot); err != nil { reviewResponse.Allowed = false reviewResponse.Result.Message = err.Error() return reviewResponse @@ -110,14 +139,37 @@ func decideSnapshot(snapshot, oldSnapshot *volumesnapshotv1beta1.VolumeSnapshot, } // Enforce strict validation for CREATE requests. Immutable checks don't apply for CREATE requests. // Enforce strict validation for UPDATE requests where old is valid and passes immutability check. - if err := utils.ValidateSnapshot(snapshot); err != nil { + if err := utils.ValidateV1Beta1Snapshot(snapshot); err != nil { reviewResponse.Allowed = false reviewResponse.Result.Message = err.Error() } return reviewResponse } -func decideSnapshotContent(snapcontent, oldSnapcontent *volumesnapshotv1beta1.VolumeSnapshotContent, isUpdate bool) *v1.AdmissionResponse { +func decideSnapshotV1(snapshot, oldSnapshot *volumesnapshotv1.VolumeSnapshot, isUpdate bool) *v1.AdmissionResponse { + reviewResponse := &v1.AdmissionResponse{ + Allowed: true, + Result: &metav1.Status{}, + } + + if isUpdate { + // if it is an UPDATE and oldSnapshot is valid, check immutable fields + if err := checkSnapshotImmutableFieldsV1(snapshot, oldSnapshot); err != nil { + reviewResponse.Allowed = false + reviewResponse.Result.Message = err.Error() + return reviewResponse + } + } + // Enforce strict validation for CREATE requests. Immutable checks don't apply for CREATE requests. + // Enforce strict validation for UPDATE requests where old is valid and passes immutability check. + if err := utils.ValidateV1Snapshot(snapshot); err != nil { + reviewResponse.Allowed = false + reviewResponse.Result.Message = err.Error() + } + return reviewResponse +} + +func decideSnapshotContentV1beta1(snapcontent, oldSnapcontent *volumesnapshotv1beta1.VolumeSnapshotContent, isUpdate bool) *v1.AdmissionResponse { reviewResponse := &v1.AdmissionResponse{ Allowed: true, Result: &metav1.Status{}, @@ -129,12 +181,12 @@ func decideSnapshotContent(snapcontent, oldSnapcontent *volumesnapshotv1beta1.Vo // Which allows the remover of finalizers and therefore deletion of this object // Don't rely on the pointers to be nil, because the deserialization method will convert it to // The empty struct value. Instead check the operation type. - if err := utils.ValidateSnapshotContent(oldSnapcontent); err != nil { + if err := utils.ValidateV1Beta1SnapshotContent(oldSnapcontent); err != nil { return reviewResponse } // if it is an UPDATE and oldSnapcontent is valid, check immutable fields - if err := checkSnapshotContentImmutableFields(snapcontent, oldSnapcontent); err != nil { + if err := checkSnapshotContentImmutableFieldsV1beta1(snapcontent, oldSnapcontent); err != nil { reviewResponse.Allowed = false reviewResponse.Result.Message = err.Error() return reviewResponse @@ -142,7 +194,30 @@ func decideSnapshotContent(snapcontent, oldSnapcontent *volumesnapshotv1beta1.Vo } // Enforce strict validation for all CREATE requests. Immutable checks don't apply for CREATE requests. // Enforce strict validation for UPDATE requests where old is valid and passes immutability check. - if err := utils.ValidateSnapshotContent(snapcontent); err != nil { + if err := utils.ValidateV1Beta1SnapshotContent(snapcontent); err != nil { + reviewResponse.Allowed = false + reviewResponse.Result.Message = err.Error() + } + return reviewResponse +} + +func decideSnapshotContentV1(snapcontent, oldSnapcontent *volumesnapshotv1.VolumeSnapshotContent, isUpdate bool) *v1.AdmissionResponse { + reviewResponse := &v1.AdmissionResponse{ + Allowed: true, + Result: &metav1.Status{}, + } + + if isUpdate { + // if it is an UPDATE and oldSnapcontent is valid, check immutable fields + if err := checkSnapshotContentImmutableFieldsV1(snapcontent, oldSnapcontent); err != nil { + reviewResponse.Allowed = false + reviewResponse.Result.Message = err.Error() + return reviewResponse + } + } + // Enforce strict validation for all CREATE requests. Immutable checks don't apply for CREATE requests. + // Enforce strict validation for UPDATE requests where old is valid and passes immutability check. + if err := utils.ValidateV1SnapshotContent(snapcontent); err != nil { reviewResponse.Allowed = false reviewResponse.Result.Message = err.Error() } @@ -155,7 +230,8 @@ func strPtrDereference(s *string) string { } return *s } -func checkSnapshotImmutableFields(snapshot, oldSnapshot *volumesnapshotv1beta1.VolumeSnapshot) error { + +func checkSnapshotImmutableFieldsV1beta1(snapshot, oldSnapshot *volumesnapshotv1beta1.VolumeSnapshot) error { if snapshot == nil { return fmt.Errorf("VolumeSnapshot is nil") } @@ -176,7 +252,48 @@ func checkSnapshotImmutableFields(snapshot, oldSnapshot *volumesnapshotv1beta1.V return nil } -func checkSnapshotContentImmutableFields(snapcontent, oldSnapcontent *volumesnapshotv1beta1.VolumeSnapshotContent) error { +func checkSnapshotImmutableFieldsV1(snapshot, oldSnapshot *volumesnapshotv1.VolumeSnapshot) error { + if snapshot == nil { + return fmt.Errorf("VolumeSnapshot is nil") + } + if oldSnapshot == nil { + return fmt.Errorf("old VolumeSnapshot is nil") + } + + source := snapshot.Spec.Source + oldSource := oldSnapshot.Spec.Source + + if !reflect.DeepEqual(source.PersistentVolumeClaimName, oldSource.PersistentVolumeClaimName) { + return fmt.Errorf("Spec.Source.PersistentVolumeClaimName is immutable but was changed from %s to %s", strPtrDereference(oldSource.PersistentVolumeClaimName), strPtrDereference(source.PersistentVolumeClaimName)) + } + if !reflect.DeepEqual(source.VolumeSnapshotContentName, oldSource.VolumeSnapshotContentName) { + return fmt.Errorf("Spec.Source.VolumeSnapshotContentName is immutable but was changed from %s to %s", strPtrDereference(oldSource.VolumeSnapshotContentName), strPtrDereference(source.VolumeSnapshotContentName)) + } + + return nil +} + +func checkSnapshotContentImmutableFieldsV1beta1(snapcontent, oldSnapcontent *volumesnapshotv1beta1.VolumeSnapshotContent) error { + if snapcontent == nil { + return fmt.Errorf("VolumeSnapshotContent is nil") + } + if oldSnapcontent == nil { + return fmt.Errorf("old VolumeSnapshotContent is nil") + } + + source := snapcontent.Spec.Source + oldSource := oldSnapcontent.Spec.Source + + if !reflect.DeepEqual(source.VolumeHandle, oldSource.VolumeHandle) { + return fmt.Errorf("Spec.Source.VolumeHandle is immutable but was changed from %s to %s", strPtrDereference(oldSource.VolumeHandle), strPtrDereference(source.VolumeHandle)) + } + if !reflect.DeepEqual(source.SnapshotHandle, oldSource.SnapshotHandle) { + return fmt.Errorf("Spec.Source.SnapshotHandle is immutable but was changed from %s to %s", strPtrDereference(oldSource.SnapshotHandle), strPtrDereference(source.SnapshotHandle)) + } + return nil +} + +func checkSnapshotContentImmutableFieldsV1(snapcontent, oldSnapcontent *volumesnapshotv1.VolumeSnapshotContent) error { if snapcontent == nil { return fmt.Errorf("VolumeSnapshotContent is nil") } diff --git a/pkg/validation-webhook/snapshot_test.go b/pkg/validation-webhook/snapshot_test.go index ca4adaea..6b713020 100644 --- a/pkg/validation-webhook/snapshot_test.go +++ b/pkg/validation-webhook/snapshot_test.go @@ -21,13 +21,14 @@ import ( "fmt" "testing" + volumesnapshotv1 "github.com/kubernetes-csi/external-snapshotter/client/v3/apis/volumesnapshot/v1" volumesnapshotv1beta1 "github.com/kubernetes-csi/external-snapshotter/client/v3/apis/volumesnapshot/v1beta1" v1 "k8s.io/api/admission/v1" core_v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime" ) -func TestAdmitVolumeSnapshot(t *testing.T) { +func TestAdmitVolumeSnapshotV1beta1(t *testing.T) { pvcname := "pvcname1" mutatedField := "changed-immutable-field" contentname := "snapcontent1" @@ -223,7 +224,191 @@ func TestAdmitVolumeSnapshot(t *testing.T) { }) } } -func TestAdmitVolumeSnapshotContent(t *testing.T) { + +func TestAdmitVolumeSnapshotV1(t *testing.T) { + pvcname := "pvcname1" + mutatedField := "changed-immutable-field" + contentname := "snapcontent1" + volumeSnapshotClassName := "volume-snapshot-class-1" + emptyVolumeSnapshotClassName := "" + + testCases := []struct { + name string + volumeSnapshot *volumesnapshotv1.VolumeSnapshot + oldVolumeSnapshot *volumesnapshotv1.VolumeSnapshot + shouldAdmit bool + msg string + operation v1.Operation + }{ + { + name: "Delete: new and old are nil. Should admit", + volumeSnapshot: nil, + oldVolumeSnapshot: nil, + shouldAdmit: true, + operation: v1.Delete, + }, + { + name: "Create: old is nil and new is valid", + volumeSnapshot: &volumesnapshotv1.VolumeSnapshot{ + Spec: volumesnapshotv1.VolumeSnapshotSpec{ + Source: volumesnapshotv1.VolumeSnapshotSource{ + VolumeSnapshotContentName: &contentname, + }, + }, + }, + oldVolumeSnapshot: nil, + shouldAdmit: true, + operation: v1.Create, + }, + { + name: "Update: old is valid and new is invalid", + volumeSnapshot: &volumesnapshotv1.VolumeSnapshot{ + Spec: volumesnapshotv1.VolumeSnapshotSpec{ + Source: volumesnapshotv1.VolumeSnapshotSource{ + VolumeSnapshotContentName: &contentname, + }, + VolumeSnapshotClassName: &emptyVolumeSnapshotClassName, + }, + }, + oldVolumeSnapshot: &volumesnapshotv1.VolumeSnapshot{ + Spec: volumesnapshotv1.VolumeSnapshotSpec{ + Source: volumesnapshotv1.VolumeSnapshotSource{ + VolumeSnapshotContentName: &contentname, + }, + }, + }, + shouldAdmit: false, + operation: v1.Update, + msg: "Spec.VolumeSnapshotClassName must not be the empty string", + }, + { + name: "Update: old is valid and new is valid", + volumeSnapshot: &volumesnapshotv1.VolumeSnapshot{ + Spec: volumesnapshotv1.VolumeSnapshotSpec{ + Source: volumesnapshotv1.VolumeSnapshotSource{ + VolumeSnapshotContentName: &contentname, + }, + VolumeSnapshotClassName: &volumeSnapshotClassName, + }, + }, + oldVolumeSnapshot: &volumesnapshotv1.VolumeSnapshot{ + Spec: volumesnapshotv1.VolumeSnapshotSpec{ + Source: volumesnapshotv1.VolumeSnapshotSource{ + VolumeSnapshotContentName: &contentname, + }, + }, + }, + shouldAdmit: true, + operation: v1.Update, + }, + { + name: "Update: old is valid and new is valid but changes immutable field spec.source", + volumeSnapshot: &volumesnapshotv1.VolumeSnapshot{ + Spec: volumesnapshotv1.VolumeSnapshotSpec{ + Source: volumesnapshotv1.VolumeSnapshotSource{ + VolumeSnapshotContentName: &mutatedField, + }, + VolumeSnapshotClassName: &volumeSnapshotClassName, + }, + }, + oldVolumeSnapshot: &volumesnapshotv1.VolumeSnapshot{ + Spec: volumesnapshotv1.VolumeSnapshotSpec{ + Source: volumesnapshotv1.VolumeSnapshotSource{ + VolumeSnapshotContentName: &contentname, + }, + }, + }, + shouldAdmit: false, + operation: v1.Update, + msg: fmt.Sprintf("Spec.Source.VolumeSnapshotContentName is immutable but was changed from %s to %s", contentname, mutatedField), + }, + { + name: "Update: old is invalid and new is valid", + volumeSnapshot: &volumesnapshotv1.VolumeSnapshot{ + Spec: volumesnapshotv1.VolumeSnapshotSpec{ + Source: volumesnapshotv1.VolumeSnapshotSource{ + VolumeSnapshotContentName: &contentname, + }, + }, + }, + oldVolumeSnapshot: &volumesnapshotv1.VolumeSnapshot{ + Spec: volumesnapshotv1.VolumeSnapshotSpec{ + Source: volumesnapshotv1.VolumeSnapshotSource{ + PersistentVolumeClaimName: &pvcname, + VolumeSnapshotContentName: &contentname, + }, + }, + }, + shouldAdmit: false, + operation: v1.Update, + msg: fmt.Sprintf("Spec.Source.PersistentVolumeClaimName is immutable but was changed from %s to ", pvcname), + }, + { + // will be handled by schema validation + name: "Update: old is invalid and new is invalid", + volumeSnapshot: &volumesnapshotv1.VolumeSnapshot{ + Spec: volumesnapshotv1.VolumeSnapshotSpec{ + Source: volumesnapshotv1.VolumeSnapshotSource{ + VolumeSnapshotContentName: &contentname, + PersistentVolumeClaimName: &pvcname, + }, + }, + }, + oldVolumeSnapshot: &volumesnapshotv1.VolumeSnapshot{ + Spec: volumesnapshotv1.VolumeSnapshotSpec{ + Source: volumesnapshotv1.VolumeSnapshotSource{ + PersistentVolumeClaimName: &pvcname, + VolumeSnapshotContentName: &contentname, + }, + }, + }, + shouldAdmit: true, + operation: v1.Update, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + snapshot := tc.volumeSnapshot + raw, err := json.Marshal(snapshot) + if err != nil { + t.Fatal(err) + } + oldSnapshot := tc.oldVolumeSnapshot + oldRaw, err := json.Marshal(oldSnapshot) + if err != nil { + t.Fatal(err) + } + review := v1.AdmissionReview{ + Request: &v1.AdmissionRequest{ + Object: runtime.RawExtension{ + Raw: raw, + }, + OldObject: runtime.RawExtension{ + Raw: oldRaw, + }, + Resource: SnapshotV1GVR, + Operation: tc.operation, + }, + } + response := admitSnapshot(review) + 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) + } + }) + } +} + +func TestAdmitVolumeSnapshotContentV1beta1(t *testing.T) { volumeHandle := "volumeHandle1" modifiedField := "modified-field" snapshotHandle := "snapshotHandle1" @@ -372,3 +557,147 @@ func TestAdmitVolumeSnapshotContent(t *testing.T) { }) } } + +func TestAdmitVolumeSnapshotContentV1(t *testing.T) { + volumeHandle := "volumeHandle1" + modifiedField := "modified-field" + snapshotHandle := "snapshotHandle1" + volumeSnapshotClassName := "volume-snapshot-class-1" + validContent := &volumesnapshotv1.VolumeSnapshotContent{ + Spec: volumesnapshotv1.VolumeSnapshotContentSpec{ + Source: volumesnapshotv1.VolumeSnapshotContentSource{ + SnapshotHandle: &snapshotHandle, + }, + VolumeSnapshotRef: core_v1.ObjectReference{ + Name: "snapshot-ref", + Namespace: "default-ns", + }, + VolumeSnapshotClassName: &volumeSnapshotClassName, + }, + } + invalidContent := &volumesnapshotv1.VolumeSnapshotContent{ + Spec: volumesnapshotv1.VolumeSnapshotContentSpec{ + Source: volumesnapshotv1.VolumeSnapshotContentSource{ + SnapshotHandle: &snapshotHandle, + VolumeHandle: &volumeHandle, + }, + VolumeSnapshotRef: core_v1.ObjectReference{ + Name: "", + Namespace: "default-ns", + }, + }, + } + + testCases := []struct { + name string + volumeSnapshotContent *volumesnapshotv1.VolumeSnapshotContent + oldVolumeSnapshotContent *volumesnapshotv1.VolumeSnapshotContent + shouldAdmit bool + msg string + operation v1.Operation + }{ + { + name: "Delete: both new and old are nil", + volumeSnapshotContent: nil, + oldVolumeSnapshotContent: nil, + shouldAdmit: true, + operation: v1.Delete, + }, + { + name: "Create: old is nil and new is valid", + volumeSnapshotContent: validContent, + oldVolumeSnapshotContent: nil, + shouldAdmit: true, + operation: v1.Create, + }, + { + name: "Update: old is valid and new is invalid", + volumeSnapshotContent: invalidContent, + oldVolumeSnapshotContent: validContent, + shouldAdmit: false, + operation: v1.Update, + msg: fmt.Sprintf("Spec.Source.VolumeHandle is immutable but was changed from %s to %s", strPtrDereference(nil), volumeHandle), + }, + { + name: "Update: old is valid and new is valid", + volumeSnapshotContent: validContent, + oldVolumeSnapshotContent: validContent, + shouldAdmit: true, + operation: v1.Update, + }, + { + name: "Update: old is valid and new is valid but modifies immutable field", + volumeSnapshotContent: &volumesnapshotv1.VolumeSnapshotContent{ + Spec: volumesnapshotv1.VolumeSnapshotContentSpec{ + Source: volumesnapshotv1.VolumeSnapshotContentSource{ + SnapshotHandle: &modifiedField, + }, + VolumeSnapshotRef: core_v1.ObjectReference{ + Name: "snapshot-ref", + Namespace: "default-ns", + }, + }, + }, + oldVolumeSnapshotContent: validContent, + shouldAdmit: false, + operation: v1.Update, + msg: fmt.Sprintf("Spec.Source.SnapshotHandle is immutable but was changed from %s to %s", snapshotHandle, modifiedField), + }, + { + name: "Update: old is invalid and new is valid", + volumeSnapshotContent: validContent, + oldVolumeSnapshotContent: invalidContent, + shouldAdmit: false, + operation: v1.Update, + msg: fmt.Sprintf("Spec.Source.VolumeHandle is immutable but was changed from %s to ", volumeHandle), + }, + { + name: "Update: old is invalid and new is invalid", + volumeSnapshotContent: invalidContent, + oldVolumeSnapshotContent: invalidContent, + shouldAdmit: false, + operation: v1.Update, + msg: fmt.Sprintf("both Spec.VolumeSnapshotRef.Name = and Spec.VolumeSnapshotRef.Namespace = default-ns must be set"), + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + snapshotContent := tc.volumeSnapshotContent + raw, err := json.Marshal(snapshotContent) + if err != nil { + t.Fatal(err) + } + oldSnapshotContent := tc.oldVolumeSnapshotContent + oldRaw, err := json.Marshal(oldSnapshotContent) + if err != nil { + t.Fatal(err) + } + review := v1.AdmissionReview{ + Request: &v1.AdmissionRequest{ + Object: runtime.RawExtension{ + Raw: raw, + }, + OldObject: runtime.RawExtension{ + Raw: oldRaw, + }, + Resource: SnapshotContentV1GVR, + Operation: tc.operation, + }, + } + response := admitSnapshot(review) + 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) + } + }) + } +}