From 8ae6e10fd0ccf28f60d505e2aec7aacdcb9470d9 Mon Sep 17 00:00:00 2001 From: Humble Chirammal Date: Mon, 28 Feb 2022 19:32:43 +0530 Subject: [PATCH 1/3] csi: correct typo and use strings.Repeat func for long driver name Signed-off-by: Humble Chirammal --- pkg/apis/core/validation/validation_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/apis/core/validation/validation_test.go b/pkg/apis/core/validation/validation_test.go index b9748b9b418..9144cadb982 100644 --- a/pkg/apis/core/validation/validation_test.go +++ b/pkg/apis/core/validation/validation_test.go @@ -2583,7 +2583,7 @@ func TestValidateCSIVolumeSource(t *testing.T) { errfield: "driver", }, { - name: "driver name: ok beginnin with number", + name: "driver name: ok beginning with number", csi: &core.CSIPersistentVolumeSource{Driver: "2io.kubernetes.storage-csi.flex", VolumeHandle: "test-123"}, }, { @@ -2608,7 +2608,7 @@ func TestValidateCSIVolumeSource(t *testing.T) { }, { name: "driver name: invalid length > 63", - csi: &core.CSIPersistentVolumeSource{Driver: "comgooglestoragecsigcepdcomgooglestoragecsigcepdcomgooglestoragecsigcepdcomgooglestoragecsigcepd", VolumeHandle: "test-123"}, + csi: &core.CSIPersistentVolumeSource{Driver: strings.Repeat("g", 65), VolumeHandle: "test-123"}, errtype: field.ErrorTypeTooLong, errfield: "driver", }, From 1f0d37c0828a6adc1897cad54461ed30baf73b50 Mon Sep 17 00:00:00 2001 From: Humble Chirammal Date: Mon, 28 Feb 2022 18:33:33 +0530 Subject: [PATCH 2/3] csi: add unit tests for {controller,node}Publish name & namespace Extra test conditions are added in CSIPersistentVolumeSource validation for controllerPublishSecretRef and nodePublishSecretRef name and namespace to check whether name field or namespace field is missing from the secretRef while validating CSI PersistentVolumeSource Signed-off-by: Humble Chirammal --- pkg/apis/core/validation/validation.go | 4 +-- pkg/apis/core/validation/validation_test.go | 34 ++++++++++++++++++++- 2 files changed, 35 insertions(+), 3 deletions(-) diff --git a/pkg/apis/core/validation/validation.go b/pkg/apis/core/validation/validation.go index dbb49be1ca8..baa2e1540dd 100644 --- a/pkg/apis/core/validation/validation.go +++ b/pkg/apis/core/validation/validation.go @@ -1588,12 +1588,12 @@ func validateCSIPersistentVolumeSource(csi *core.CSIPersistentVolumeSource, fldP if csi.NodePublishSecretRef != nil { if len(csi.NodePublishSecretRef.Name) == 0 { - allErrs = append(allErrs, field.Required(fldPath.Child("nodePublishSecretRef ", "name"), "")) + allErrs = append(allErrs, field.Required(fldPath.Child("nodePublishSecretRef", "name"), "")) } else { allErrs = append(allErrs, ValidateDNS1123Label(csi.NodePublishSecretRef.Name, fldPath.Child("name"))...) } if len(csi.NodePublishSecretRef.Namespace) == 0 { - allErrs = append(allErrs, field.Required(fldPath.Child("nodePublishSecretRef ", "namespace"), "")) + allErrs = append(allErrs, field.Required(fldPath.Child("nodePublishSecretRef", "namespace"), "")) } else { allErrs = append(allErrs, ValidateDNS1123Label(csi.NodePublishSecretRef.Namespace, fldPath.Child("namespace"))...) } diff --git a/pkg/apis/core/validation/validation_test.go b/pkg/apis/core/validation/validation_test.go index 9144cadb982..c4c1baac006 100644 --- a/pkg/apis/core/validation/validation_test.go +++ b/pkg/apis/core/validation/validation_test.go @@ -2531,7 +2531,7 @@ func TestValidateGlusterfsPersistentVolumeSource(t *testing.T) { } } -func TestValidateCSIVolumeSource(t *testing.T) { +func TestValidateCSIPersistentVolumeSource(t *testing.T) { testCases := []struct { name string csi *core.CSIPersistentVolumeSource @@ -2646,6 +2646,38 @@ func TestValidateCSIVolumeSource(t *testing.T) { name: "valid controllerExpandSecretRef", csi: &core.CSIPersistentVolumeSource{Driver: "com.google.gcepd", VolumeHandle: "foobar", ControllerExpandSecretRef: &core.SecretReference{Name: "foobar", Namespace: "default"}}, }, + { + name: "controllerPublishSecretRef: invalid name missing", + csi: &core.CSIPersistentVolumeSource{Driver: "com.google.gcepd", VolumeHandle: "foobar", ControllerPublishSecretRef: &core.SecretReference{Namespace: "default"}}, + errtype: field.ErrorTypeRequired, + errfield: "controllerPublishSecretRef.name", + }, + { + name: "controllerPublishSecretRef: invalid namespace missing", + csi: &core.CSIPersistentVolumeSource{Driver: "com.google.gcepd", VolumeHandle: "foobar", ControllerPublishSecretRef: &core.SecretReference{Name: "foobar"}}, + errtype: field.ErrorTypeRequired, + errfield: "controllerPublishSecretRef.namespace", + }, + { + name: "valid controllerPublishSecretRef", + csi: &core.CSIPersistentVolumeSource{Driver: "com.google.gcepd", VolumeHandle: "foobar", ControllerPublishSecretRef: &core.SecretReference{Name: "foobar", Namespace: "default"}}, + }, + { + name: "valid nodePublishSecretRef", + csi: &core.CSIPersistentVolumeSource{Driver: "com.google.gcepd", VolumeHandle: "foobar", NodePublishSecretRef: &core.SecretReference{Name: "foobar", Namespace: "default"}}, + }, + { + name: "nodePublishSecretRef: invalid name missing", + csi: &core.CSIPersistentVolumeSource{Driver: "com.google.gcepd", VolumeHandle: "foobar", NodePublishSecretRef: &core.SecretReference{Namespace: "foobar"}}, + errtype: field.ErrorTypeRequired, + errfield: "nodePublishSecretRef.name", + }, + { + name: "nodePublishSecretRef: invalid namespace missing", + csi: &core.CSIPersistentVolumeSource{Driver: "com.google.gcepd", VolumeHandle: "foobar", NodePublishSecretRef: &core.SecretReference{Name: "foobar"}}, + errtype: field.ErrorTypeRequired, + errfield: "nodePublishSecretRef.namespace", + }, } for i, tc := range testCases { From d727d7db1a675e32e030c4ab24785f553b1633ca Mon Sep 17 00:00:00 2001 From: Humble Chirammal Date: Wed, 2 Mar 2022 15:47:04 +0530 Subject: [PATCH 3/3] csi: add validation tests for CSIVolumeSource This commit adds the validation tests for CSIVolumeSource explictly. Also validate driver,nodePublishSecretRef..etc Signed-off-by: Humble Chirammal --- pkg/apis/core/validation/validation.go | 2 +- pkg/apis/core/validation/validation_test.go | 113 ++++++++++++++++++++ 2 files changed, 114 insertions(+), 1 deletion(-) diff --git a/pkg/apis/core/validation/validation.go b/pkg/apis/core/validation/validation.go index baa2e1540dd..38737542c40 100644 --- a/pkg/apis/core/validation/validation.go +++ b/pkg/apis/core/validation/validation.go @@ -1608,7 +1608,7 @@ func validateCSIVolumeSource(csi *core.CSIVolumeSource, fldPath *field.Path) fie if csi.NodePublishSecretRef != nil { if len(csi.NodePublishSecretRef.Name) == 0 { - allErrs = append(allErrs, field.Required(fldPath.Child("nodePublishSecretRef ", "name"), "")) + allErrs = append(allErrs, field.Required(fldPath.Child("nodePublishSecretRef", "name"), "")) } else { for _, msg := range ValidateSecretName(csi.NodePublishSecretRef.Name, false) { allErrs = append(allErrs, field.Invalid(fldPath.Child("name"), csi.NodePublishSecretRef.Name, msg)) diff --git a/pkg/apis/core/validation/validation_test.go b/pkg/apis/core/validation/validation_test.go index c4c1baac006..788888f8b44 100644 --- a/pkg/apis/core/validation/validation_test.go +++ b/pkg/apis/core/validation/validation_test.go @@ -2531,6 +2531,119 @@ func TestValidateGlusterfsPersistentVolumeSource(t *testing.T) { } } +func TestValidateCSIVolumeSource(t *testing.T) { + testCases := []struct { + name string + csi *core.CSIVolumeSource + errtype field.ErrorType + errfield string + }{ + { + name: "all required fields ok", + csi: &core.CSIVolumeSource{Driver: "test-driver"}, + }, + { + name: "missing driver name", + csi: &core.CSIVolumeSource{Driver: ""}, + errtype: field.ErrorTypeRequired, + errfield: "driver", + }, + { + name: "driver name: ok no punctuations", + csi: &core.CSIVolumeSource{Driver: "comgooglestoragecsigcepd"}, + }, + { + name: "driver name: ok dot only", + csi: &core.CSIVolumeSource{Driver: "io.kubernetes.storage.csi.flex"}, + }, + { + name: "driver name: ok dash only", + csi: &core.CSIVolumeSource{Driver: "io-kubernetes-storage-csi-flex"}, + }, + { + name: "driver name: invalid underscore", + csi: &core.CSIVolumeSource{Driver: "io_kubernetes_storage_csi_flex"}, + errtype: field.ErrorTypeInvalid, + errfield: "driver", + }, + { + name: "driver name: invalid dot underscores", + csi: &core.CSIVolumeSource{Driver: "io.kubernetes.storage_csi.flex"}, + errtype: field.ErrorTypeInvalid, + errfield: "driver", + }, + { + name: "driver name: ok beginning with number", + csi: &core.CSIVolumeSource{Driver: "2io.kubernetes.storage-csi.flex"}, + }, + { + name: "driver name: ok ending with number", + csi: &core.CSIVolumeSource{Driver: "io.kubernetes.storage-csi.flex2"}, + }, + { + name: "driver name: invalid dot dash underscores", + csi: &core.CSIVolumeSource{Driver: "io.kubernetes-storage.csi_flex"}, + errtype: field.ErrorTypeInvalid, + errfield: "driver", + }, + + { + name: "driver name: ok length 1", + csi: &core.CSIVolumeSource{Driver: "a"}, + }, + { + name: "driver name: invalid length > 63", + csi: &core.CSIVolumeSource{Driver: strings.Repeat("g", 65)}, + errtype: field.ErrorTypeTooLong, + errfield: "driver", + }, + { + name: "driver name: invalid start char", + csi: &core.CSIVolumeSource{Driver: "_comgooglestoragecsigcepd"}, + errtype: field.ErrorTypeInvalid, + errfield: "driver", + }, + { + name: "driver name: invalid end char", + csi: &core.CSIVolumeSource{Driver: "comgooglestoragecsigcepd/"}, + errtype: field.ErrorTypeInvalid, + errfield: "driver", + }, + { + name: "driver name: invalid separators", + csi: &core.CSIVolumeSource{Driver: "com/google/storage/csi~gcepd"}, + errtype: field.ErrorTypeInvalid, + errfield: "driver", + }, + { + name: "valid nodePublishSecretRef", + csi: &core.CSIVolumeSource{Driver: "com.google.gcepd", NodePublishSecretRef: &core.LocalObjectReference{Name: "foobar"}}, + }, + { + name: "nodePublishSecretRef: invalid name missing", + csi: &core.CSIVolumeSource{Driver: "com.google.gcepd", NodePublishSecretRef: &core.LocalObjectReference{Name: ""}}, + errtype: field.ErrorTypeRequired, + errfield: "nodePublishSecretRef.name", + }, + } + + for i, tc := range testCases { + errs := validateCSIVolumeSource(tc.csi, field.NewPath("field")) + + if len(errs) > 0 && tc.errtype == "" { + t.Errorf("[%d: %q] unexpected error(s): %v", i, tc.name, errs) + } else if len(errs) == 0 && tc.errtype != "" { + t.Errorf("[%d: %q] expected error type %v", i, tc.name, tc.errtype) + } else if len(errs) >= 1 { + if errs[0].Type != tc.errtype { + t.Errorf("[%d: %q] expected error type %v, got %v", i, tc.name, tc.errtype, errs[0].Type) + } else if !strings.HasSuffix(errs[0].Field, "."+tc.errfield) { + t.Errorf("[%d: %q] expected error on field %q, got %q", i, tc.name, tc.errfield, errs[0].Field) + } + } + } +} + func TestValidateCSIPersistentVolumeSource(t *testing.T) { testCases := []struct { name string