From b60f08e24b5fa0bcc99dd985e6c25b05939ff665 Mon Sep 17 00:00:00 2001
From: Patrick Ohly <patrick.ohly@intel.com>
Date: Fri, 26 Jul 2019 13:52:37 +0200
Subject: [PATCH] storage: validate CSIDriver.Spec.VolumeLifecycleModes

This ensures that users get a good error message early on when trying
to do something that isn't okay:

  $ kubectl create -f csi-hostpath-driverinfo.yaml
  The CSIDriver "hostpath.csi.k8s.io" is invalid: spec.volumeLifecycleModes: Unsupported value: "foobar": supported values: "persistent", "ephemeral"
---
 pkg/apis/storage/validation/validation.go     | 19 ++++++
 .../storage/validation/validation_test.go     | 54 +++++++++++++++
 .../storage/csidriver/strategy_test.go        | 65 +++++++++++++++++++
 3 files changed, 138 insertions(+)

diff --git a/pkg/apis/storage/validation/validation.go b/pkg/apis/storage/validation/validation.go
index fd8e9984135..2f3b438af27 100644
--- a/pkg/apis/storage/validation/validation.go
+++ b/pkg/apis/storage/validation/validation.go
@@ -419,6 +419,7 @@ func validateCSIDriverSpec(
 	allErrs := field.ErrorList{}
 	allErrs = append(allErrs, validateAttachRequired(spec.AttachRequired, fldPath.Child("attachedRequired"))...)
 	allErrs = append(allErrs, validatePodInfoOnMount(spec.PodInfoOnMount, fldPath.Child("podInfoOnMount"))...)
+	allErrs = append(allErrs, validateVolumeLifecycleModes(spec.VolumeLifecycleModes, fldPath.Child("volumeLifecycleModes"))...)
 	return allErrs
 }
 
@@ -441,3 +442,21 @@ func validatePodInfoOnMount(podInfoOnMount *bool, fldPath *field.Path) field.Err
 
 	return allErrs
 }
+
+// validateVolumeLifecycleModes tests if mode has one of the allowed values.
+func validateVolumeLifecycleModes(modes []storage.VolumeLifecycleMode, fldPath *field.Path) field.ErrorList {
+	allErrs := field.ErrorList{}
+	for _, mode := range modes {
+		switch mode {
+		case storage.VolumeLifecyclePersistent, storage.VolumeLifecycleEphemeral:
+		default:
+			allErrs = append(allErrs, field.NotSupported(fldPath, mode,
+				[]string{
+					string(storage.VolumeLifecyclePersistent),
+					string(storage.VolumeLifecycleEphemeral),
+				}))
+		}
+	}
+
+	return allErrs
+}
diff --git a/pkg/apis/storage/validation/validation_test.go b/pkg/apis/storage/validation/validation_test.go
index 15f54b419e9..81f5c0eca6a 100644
--- a/pkg/apis/storage/validation/validation_test.go
+++ b/pkg/apis/storage/validation/validation_test.go
@@ -1726,6 +1726,49 @@ func TestCSIDriverValidation(t *testing.T) {
 				PodInfoOnMount: &notPodInfoOnMount,
 			},
 		},
+		{
+			ObjectMeta: metav1.ObjectMeta{Name: driverName},
+			Spec: storage.CSIDriverSpec{
+				AttachRequired: &attachNotRequired,
+				PodInfoOnMount: &notPodInfoOnMount,
+				VolumeLifecycleModes: []storage.VolumeLifecycleMode{
+					storage.VolumeLifecyclePersistent,
+				},
+			},
+		},
+		{
+			ObjectMeta: metav1.ObjectMeta{Name: driverName},
+			Spec: storage.CSIDriverSpec{
+				AttachRequired: &attachNotRequired,
+				PodInfoOnMount: &notPodInfoOnMount,
+				VolumeLifecycleModes: []storage.VolumeLifecycleMode{
+					storage.VolumeLifecycleEphemeral,
+				},
+			},
+		},
+		{
+			ObjectMeta: metav1.ObjectMeta{Name: driverName},
+			Spec: storage.CSIDriverSpec{
+				AttachRequired: &attachNotRequired,
+				PodInfoOnMount: &notPodInfoOnMount,
+				VolumeLifecycleModes: []storage.VolumeLifecycleMode{
+					storage.VolumeLifecycleEphemeral,
+					storage.VolumeLifecyclePersistent,
+				},
+			},
+		},
+		{
+			ObjectMeta: metav1.ObjectMeta{Name: driverName},
+			Spec: storage.CSIDriverSpec{
+				AttachRequired: &attachNotRequired,
+				PodInfoOnMount: &notPodInfoOnMount,
+				VolumeLifecycleModes: []storage.VolumeLifecycleMode{
+					storage.VolumeLifecycleEphemeral,
+					storage.VolumeLifecyclePersistent,
+					storage.VolumeLifecycleEphemeral,
+				},
+			},
+		},
 	}
 
 	for _, csiDriver := range successCases {
@@ -1764,6 +1807,17 @@ func TestCSIDriverValidation(t *testing.T) {
 				PodInfoOnMount: nil,
 			},
 		},
+		{
+			// invalid mode
+			ObjectMeta: metav1.ObjectMeta{Name: driverName},
+			Spec: storage.CSIDriverSpec{
+				AttachRequired: &attachNotRequired,
+				PodInfoOnMount: &notPodInfoOnMount,
+				VolumeLifecycleModes: []storage.VolumeLifecycleMode{
+					"no-such-mode",
+				},
+			},
+		},
 	}
 
 	for _, csiDriver := range errorCases {
diff --git a/pkg/registry/storage/csidriver/strategy_test.go b/pkg/registry/storage/csidriver/strategy_test.go
index 442b1005e8d..75c227b2c83 100644
--- a/pkg/registry/storage/csidriver/strategy_test.go
+++ b/pkg/registry/storage/csidriver/strategy_test.go
@@ -296,6 +296,71 @@ func TestCSIDriverValidation(t *testing.T) {
 			},
 			true,
 		},
+		{
+			"invalid volume mode",
+			&storage.CSIDriver{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "foo",
+				},
+				Spec: storage.CSIDriverSpec{
+					AttachRequired: &attachRequired,
+					PodInfoOnMount: &podInfoOnMount,
+					VolumeLifecycleModes: []storage.VolumeLifecycleMode{
+						storage.VolumeLifecycleMode("no-such-mode"),
+					},
+				},
+			},
+			true,
+		},
+		{
+			"persistent volume mode",
+			&storage.CSIDriver{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "foo",
+				},
+				Spec: storage.CSIDriverSpec{
+					AttachRequired: &attachRequired,
+					PodInfoOnMount: &podInfoOnMount,
+					VolumeLifecycleModes: []storage.VolumeLifecycleMode{
+						storage.VolumeLifecyclePersistent,
+					},
+				},
+			},
+			false,
+		},
+		{
+			"ephemeral volume mode",
+			&storage.CSIDriver{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "foo",
+				},
+				Spec: storage.CSIDriverSpec{
+					AttachRequired: &attachRequired,
+					PodInfoOnMount: &podInfoOnMount,
+					VolumeLifecycleModes: []storage.VolumeLifecycleMode{
+						storage.VolumeLifecycleEphemeral,
+					},
+				},
+			},
+			false,
+		},
+		{
+			"both volume modes",
+			&storage.CSIDriver{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "foo",
+				},
+				Spec: storage.CSIDriverSpec{
+					AttachRequired: &attachRequired,
+					PodInfoOnMount: &podInfoOnMount,
+					VolumeLifecycleModes: []storage.VolumeLifecycleMode{
+						storage.VolumeLifecyclePersistent,
+						storage.VolumeLifecycleEphemeral,
+					},
+				},
+			},
+			false,
+		},
 	}
 
 	for _, test := range tests {