Merge pull request #61984 from mikedanese/fix4

Automatic merge from submit-queue (batch tested with PRs 63492, 62379, 61984, 63805, 63807). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

validation: improve ProjectedVolume validation errors

* only report "may not specify more than 1 volume type" once
* fix incorrectly reported field paths
* continue to traverse into projections to report further errors.

@kubernetes/sig-storage-pr-reviews 

```release-note
NONE
```
This commit is contained in:
Kubernetes Submit Queue 2018-05-14 17:11:20 -07:00 committed by GitHub
commit 84914c6a38
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 395 additions and 214 deletions

View File

@ -984,74 +984,64 @@ func validateProjectionSources(projection *core.ProjectedVolumeSource, projectio
allErrs := field.ErrorList{} allErrs := field.ErrorList{}
allPaths := sets.String{} allPaths := sets.String{}
for _, source := range projection.Sources { for i, source := range projection.Sources {
numSources := 0 numSources := 0
if source.Secret != nil { srcPath := fldPath.Child("sources").Index(i)
if numSources > 0 { if projPath := srcPath.Child("secret"); source.Secret != nil {
allErrs = append(allErrs, field.Forbidden(fldPath.Child("secret"), "may not specify more than 1 volume type")) numSources++
} else { if len(source.Secret.Name) == 0 {
numSources++ allErrs = append(allErrs, field.Required(projPath.Child("name"), ""))
if len(source.Secret.Name) == 0 { }
allErrs = append(allErrs, field.Required(fldPath.Child("name"), "")) itemsPath := projPath.Child("items")
} for i, kp := range source.Secret.Items {
itemsPath := fldPath.Child("items") itemPath := itemsPath.Index(i)
for i, kp := range source.Secret.Items { allErrs = append(allErrs, validateKeyToPath(&kp, itemPath)...)
itemPath := itemsPath.Index(i) if len(kp.Path) > 0 {
allErrs = append(allErrs, validateKeyToPath(&kp, itemPath)...) curPath := kp.Path
if len(kp.Path) > 0 { if !allPaths.Has(curPath) {
curPath := kp.Path allPaths.Insert(curPath)
if !allPaths.Has(curPath) { } else {
allPaths.Insert(curPath) allErrs = append(allErrs, field.Invalid(fldPath, source.Secret.Name, "conflicting duplicate paths"))
} else {
allErrs = append(allErrs, field.Invalid(fldPath, source.Secret.Name, "conflicting duplicate paths"))
}
} }
} }
} }
} }
if source.ConfigMap != nil { if projPath := srcPath.Child("configMap"); source.ConfigMap != nil {
if numSources > 0 { numSources++
allErrs = append(allErrs, field.Forbidden(fldPath.Child("configMap"), "may not specify more than 1 volume type")) if len(source.ConfigMap.Name) == 0 {
} else { allErrs = append(allErrs, field.Required(projPath.Child("name"), ""))
numSources++ }
if len(source.ConfigMap.Name) == 0 { itemsPath := projPath.Child("items")
allErrs = append(allErrs, field.Required(fldPath.Child("name"), "")) for i, kp := range source.ConfigMap.Items {
} itemPath := itemsPath.Index(i)
itemsPath := fldPath.Child("items") allErrs = append(allErrs, validateKeyToPath(&kp, itemPath)...)
for i, kp := range source.ConfigMap.Items { if len(kp.Path) > 0 {
itemPath := itemsPath.Index(i) curPath := kp.Path
allErrs = append(allErrs, validateKeyToPath(&kp, itemPath)...) if !allPaths.Has(curPath) {
if len(kp.Path) > 0 { allPaths.Insert(curPath)
curPath := kp.Path } else {
if !allPaths.Has(curPath) { allErrs = append(allErrs, field.Invalid(fldPath, source.ConfigMap.Name, "conflicting duplicate paths"))
allPaths.Insert(curPath)
} else {
allErrs = append(allErrs, field.Invalid(fldPath, source.ConfigMap.Name, "conflicting duplicate paths"))
}
} }
} }
} }
} }
if source.DownwardAPI != nil { if projPath := srcPath.Child("downwardAPI"); source.DownwardAPI != nil {
if numSources > 0 { numSources++
allErrs = append(allErrs, field.Forbidden(fldPath.Child("downwardAPI"), "may not specify more than 1 volume type")) for _, file := range source.DownwardAPI.Items {
} else { allErrs = append(allErrs, validateDownwardAPIVolumeFile(&file, projPath)...)
numSources++ if len(file.Path) > 0 {
for _, file := range source.DownwardAPI.Items { curPath := file.Path
allErrs = append(allErrs, validateDownwardAPIVolumeFile(&file, fldPath.Child("downwardAPI"))...) if !allPaths.Has(curPath) {
if len(file.Path) > 0 { allPaths.Insert(curPath)
curPath := file.Path } else {
if !allPaths.Has(curPath) { allErrs = append(allErrs, field.Invalid(fldPath, curPath, "conflicting duplicate paths"))
allPaths.Insert(curPath)
} else {
allErrs = append(allErrs, field.Invalid(fldPath, curPath, "conflicting duplicate paths"))
}
} }
} }
} }
} }
if numSources > 1 {
allErrs = append(allErrs, field.Forbidden(srcPath, "may not specify more than 1 volume type"))
}
} }
return allErrs return allErrs
} }

View File

@ -1844,12 +1844,17 @@ func TestValidateCSIVolumeSource(t *testing.T) {
func TestValidateVolumes(t *testing.T) { func TestValidateVolumes(t *testing.T) {
validInitiatorName := "iqn.2015-02.example.com:init" validInitiatorName := "iqn.2015-02.example.com:init"
invalidInitiatorName := "2015-02.example.com:init" invalidInitiatorName := "2015-02.example.com:init"
type verr struct {
etype field.ErrorType
field string
detail string
}
testCases := []struct { testCases := []struct {
name string name string
vol core.Volume vol core.Volume
errtype field.ErrorType errs []verr
errfield string
errdetail string
}{ }{
// EmptyDir and basic volume names // EmptyDir and basic volume names
{ {
@ -1894,8 +1899,10 @@ func TestValidateVolumes(t *testing.T) {
Name: "", Name: "",
VolumeSource: core.VolumeSource{EmptyDir: &core.EmptyDirVolumeSource{}}, VolumeSource: core.VolumeSource{EmptyDir: &core.EmptyDirVolumeSource{}},
}, },
errtype: field.ErrorTypeRequired, errs: []verr{{
errfield: "name", etype: field.ErrorTypeRequired,
field: "name",
}},
}, },
{ {
name: "name > 63 characters", name: "name > 63 characters",
@ -1903,9 +1910,11 @@ func TestValidateVolumes(t *testing.T) {
Name: strings.Repeat("a", 64), Name: strings.Repeat("a", 64),
VolumeSource: core.VolumeSource{EmptyDir: &core.EmptyDirVolumeSource{}}, VolumeSource: core.VolumeSource{EmptyDir: &core.EmptyDirVolumeSource{}},
}, },
errtype: field.ErrorTypeInvalid, errs: []verr{{
errfield: "name", etype: field.ErrorTypeInvalid,
errdetail: "must be no more than", field: "name",
detail: "must be no more than",
}},
}, },
{ {
name: "name not a DNS label", name: "name not a DNS label",
@ -1913,9 +1922,11 @@ func TestValidateVolumes(t *testing.T) {
Name: "a.b.c", Name: "a.b.c",
VolumeSource: core.VolumeSource{EmptyDir: &core.EmptyDirVolumeSource{}}, VolumeSource: core.VolumeSource{EmptyDir: &core.EmptyDirVolumeSource{}},
}, },
errtype: field.ErrorTypeInvalid, errs: []verr{{
errfield: "name", etype: field.ErrorTypeInvalid,
errdetail: dnsLabelErrMsg, field: "name",
detail: dnsLabelErrMsg,
}},
}, },
// More than one source field specified. // More than one source field specified.
{ {
@ -1930,9 +1941,11 @@ func TestValidateVolumes(t *testing.T) {
}, },
}, },
}, },
errtype: field.ErrorTypeForbidden, errs: []verr{{
errfield: "hostPath", etype: field.ErrorTypeForbidden,
errdetail: "may not specify more than 1 volume", field: "hostPath",
detail: "may not specify more than 1 volume",
}},
}, },
// HostPath Default // HostPath Default
{ {
@ -1972,8 +1985,10 @@ func TestValidateVolumes(t *testing.T) {
}, },
}, },
}, },
errtype: field.ErrorTypeNotSupported, errs: []verr{{
errfield: "type", etype: field.ErrorTypeNotSupported,
field: "type",
}},
}, },
{ {
name: "invalid HostPath backsteps", name: "invalid HostPath backsteps",
@ -1986,9 +2001,11 @@ func TestValidateVolumes(t *testing.T) {
}, },
}, },
}, },
errtype: field.ErrorTypeInvalid, errs: []verr{{
errfield: "path", etype: field.ErrorTypeInvalid,
errdetail: "must not contain '..'", field: "path",
detail: "must not contain '..'",
}},
}, },
// GcePersistentDisk // GcePersistentDisk
{ {
@ -2069,9 +2086,11 @@ func TestValidateVolumes(t *testing.T) {
}, },
}, },
}, },
errtype: field.ErrorTypeInvalid, errs: []verr{{
errfield: "gitRepo.directory", etype: field.ErrorTypeInvalid,
errdetail: `must not contain '..'`, field: "gitRepo.directory",
detail: `must not contain '..'`,
}},
}, },
{ {
name: "GitRepo contains ..", name: "GitRepo contains ..",
@ -2084,9 +2103,11 @@ func TestValidateVolumes(t *testing.T) {
}, },
}, },
}, },
errtype: field.ErrorTypeInvalid, errs: []verr{{
errfield: "gitRepo.directory", etype: field.ErrorTypeInvalid,
errdetail: `must not contain '..'`, field: "gitRepo.directory",
detail: `must not contain '..'`,
}},
}, },
{ {
name: "GitRepo absolute target", name: "GitRepo absolute target",
@ -2099,8 +2120,10 @@ func TestValidateVolumes(t *testing.T) {
}, },
}, },
}, },
errtype: field.ErrorTypeInvalid, errs: []verr{{
errfield: "gitRepo.directory", etype: field.ErrorTypeInvalid,
field: "gitRepo.directory",
}},
}, },
// ISCSI // ISCSI
{ {
@ -2162,8 +2185,10 @@ func TestValidateVolumes(t *testing.T) {
}, },
}, },
}, },
errtype: field.ErrorTypeRequired, errs: []verr{{
errfield: "iscsi.targetPortal", etype: field.ErrorTypeRequired,
field: "iscsi.targetPortal",
}},
}, },
{ {
name: "empty iqn", name: "empty iqn",
@ -2179,8 +2204,10 @@ func TestValidateVolumes(t *testing.T) {
}, },
}, },
}, },
errtype: field.ErrorTypeRequired, errs: []verr{{
errfield: "iscsi.iqn", etype: field.ErrorTypeRequired,
field: "iscsi.iqn",
}},
}, },
{ {
name: "invalid IQN: iqn format", name: "invalid IQN: iqn format",
@ -2196,8 +2223,10 @@ func TestValidateVolumes(t *testing.T) {
}, },
}, },
}, },
errtype: field.ErrorTypeInvalid, errs: []verr{{
errfield: "iscsi.iqn", etype: field.ErrorTypeInvalid,
field: "iscsi.iqn",
}},
}, },
{ {
name: "invalid IQN: eui format", name: "invalid IQN: eui format",
@ -2213,8 +2242,10 @@ func TestValidateVolumes(t *testing.T) {
}, },
}, },
}, },
errtype: field.ErrorTypeInvalid, errs: []verr{{
errfield: "iscsi.iqn", etype: field.ErrorTypeInvalid,
field: "iscsi.iqn",
}},
}, },
{ {
name: "invalid IQN: naa format", name: "invalid IQN: naa format",
@ -2230,8 +2261,10 @@ func TestValidateVolumes(t *testing.T) {
}, },
}, },
}, },
errtype: field.ErrorTypeInvalid, errs: []verr{{
errfield: "iscsi.iqn", etype: field.ErrorTypeInvalid,
field: "iscsi.iqn",
}},
}, },
{ {
name: "valid initiatorName", name: "valid initiatorName",
@ -2264,8 +2297,10 @@ func TestValidateVolumes(t *testing.T) {
}, },
}, },
}, },
errtype: field.ErrorTypeInvalid, errs: []verr{{
errfield: "iscsi.initiatorname", etype: field.ErrorTypeInvalid,
field: "iscsi.initiatorname",
}},
}, },
{ {
name: "empty secret", name: "empty secret",
@ -2282,8 +2317,10 @@ func TestValidateVolumes(t *testing.T) {
}, },
}, },
}, },
errtype: field.ErrorTypeRequired, errs: []verr{{
errfield: "iscsi.secretRef", etype: field.ErrorTypeRequired,
field: "iscsi.secretRef",
}},
}, },
{ {
name: "empty secret", name: "empty secret",
@ -2300,8 +2337,10 @@ func TestValidateVolumes(t *testing.T) {
}, },
}, },
}, },
errtype: field.ErrorTypeRequired, errs: []verr{{
errfield: "iscsi.secretRef", etype: field.ErrorTypeRequired,
field: "iscsi.secretRef",
}},
}, },
// Secret // Secret
{ {
@ -2369,8 +2408,10 @@ func TestValidateVolumes(t *testing.T) {
}, },
}, },
}, },
errtype: field.ErrorTypeRequired, errs: []verr{{
errfield: "secret.items[0].path", etype: field.ErrorTypeRequired,
field: "secret.items[0].path",
}},
}, },
{ {
name: "secret with leading ..", name: "secret with leading ..",
@ -2383,8 +2424,10 @@ func TestValidateVolumes(t *testing.T) {
}, },
}, },
}, },
errtype: field.ErrorTypeInvalid, errs: []verr{{
errfield: "secret.items[0].path", etype: field.ErrorTypeInvalid,
field: "secret.items[0].path",
}},
}, },
{ {
name: "secret with .. inside", name: "secret with .. inside",
@ -2397,8 +2440,10 @@ func TestValidateVolumes(t *testing.T) {
}, },
}, },
}, },
errtype: field.ErrorTypeInvalid, errs: []verr{{
errfield: "secret.items[0].path", etype: field.ErrorTypeInvalid,
field: "secret.items[0].path",
}},
}, },
{ {
name: "secret with invalid positive defaultMode", name: "secret with invalid positive defaultMode",
@ -2411,8 +2456,10 @@ func TestValidateVolumes(t *testing.T) {
}, },
}, },
}, },
errtype: field.ErrorTypeInvalid, errs: []verr{{
errfield: "secret.defaultMode", etype: field.ErrorTypeInvalid,
field: "secret.defaultMode",
}},
}, },
{ {
name: "secret with invalid negative defaultMode", name: "secret with invalid negative defaultMode",
@ -2425,8 +2472,10 @@ func TestValidateVolumes(t *testing.T) {
}, },
}, },
}, },
errtype: field.ErrorTypeInvalid, errs: []verr{{
errfield: "secret.defaultMode", etype: field.ErrorTypeInvalid,
field: "secret.defaultMode",
}},
}, },
// ConfigMap // ConfigMap
{ {
@ -2500,8 +2549,10 @@ func TestValidateVolumes(t *testing.T) {
}, },
}, },
}, },
errtype: field.ErrorTypeRequired, errs: []verr{{
errfield: "configMap.items[0].path", etype: field.ErrorTypeRequired,
field: "configMap.items[0].path",
}},
}, },
{ {
name: "configmap with leading ..", name: "configmap with leading ..",
@ -2514,8 +2565,10 @@ func TestValidateVolumes(t *testing.T) {
}, },
}, },
}, },
errtype: field.ErrorTypeInvalid, errs: []verr{{
errfield: "configMap.items[0].path", etype: field.ErrorTypeInvalid,
field: "configMap.items[0].path",
}},
}, },
{ {
name: "configmap with .. inside", name: "configmap with .. inside",
@ -2528,8 +2581,10 @@ func TestValidateVolumes(t *testing.T) {
}, },
}, },
}, },
errtype: field.ErrorTypeInvalid, errs: []verr{{
errfield: "configMap.items[0].path", etype: field.ErrorTypeInvalid,
field: "configMap.items[0].path",
}},
}, },
{ {
name: "configmap with invalid positive defaultMode", name: "configmap with invalid positive defaultMode",
@ -2542,8 +2597,10 @@ func TestValidateVolumes(t *testing.T) {
}, },
}, },
}, },
errtype: field.ErrorTypeInvalid, errs: []verr{{
errfield: "configMap.defaultMode", etype: field.ErrorTypeInvalid,
field: "configMap.defaultMode",
}},
}, },
{ {
name: "configmap with invalid negative defaultMode", name: "configmap with invalid negative defaultMode",
@ -2556,8 +2613,10 @@ func TestValidateVolumes(t *testing.T) {
}, },
}, },
}, },
errtype: field.ErrorTypeInvalid, errs: []verr{{
errfield: "configMap.defaultMode", etype: field.ErrorTypeInvalid,
field: "configMap.defaultMode",
}},
}, },
// Glusterfs // Glusterfs
{ {
@ -2585,8 +2644,10 @@ func TestValidateVolumes(t *testing.T) {
}, },
}, },
}, },
errtype: field.ErrorTypeRequired, errs: []verr{{
errfield: "glusterfs.endpoints", etype: field.ErrorTypeRequired,
field: "glusterfs.endpoints",
}},
}, },
{ {
name: "empty path", name: "empty path",
@ -2600,8 +2661,10 @@ func TestValidateVolumes(t *testing.T) {
}, },
}, },
}, },
errtype: field.ErrorTypeRequired, errs: []verr{{
errfield: "glusterfs.path", etype: field.ErrorTypeRequired,
field: "glusterfs.path",
}},
}, },
// Flocker // Flocker
{ {
@ -2636,8 +2699,10 @@ func TestValidateVolumes(t *testing.T) {
}, },
}, },
}, },
errtype: field.ErrorTypeRequired, errs: []verr{{
errfield: "flocker", etype: field.ErrorTypeRequired,
field: "flocker",
}},
}, },
{ {
name: "both specified", name: "both specified",
@ -2650,8 +2715,10 @@ func TestValidateVolumes(t *testing.T) {
}, },
}, },
}, },
errtype: field.ErrorTypeInvalid, errs: []verr{{
errfield: "flocker", etype: field.ErrorTypeInvalid,
field: "flocker",
}},
}, },
{ {
name: "slash in flocker datasetName", name: "slash in flocker datasetName",
@ -2663,9 +2730,11 @@ func TestValidateVolumes(t *testing.T) {
}, },
}, },
}, },
errtype: field.ErrorTypeInvalid, errs: []verr{{
errfield: "flocker.datasetName", etype: field.ErrorTypeInvalid,
errdetail: "must not contain '/'", field: "flocker.datasetName",
detail: "must not contain '/'",
}},
}, },
// RBD // RBD
{ {
@ -2693,8 +2762,10 @@ func TestValidateVolumes(t *testing.T) {
}, },
}, },
}, },
errtype: field.ErrorTypeRequired, errs: []verr{{
errfield: "rbd.monitors", etype: field.ErrorTypeRequired,
field: "rbd.monitors",
}},
}, },
{ {
name: "empty image", name: "empty image",
@ -2708,8 +2779,10 @@ func TestValidateVolumes(t *testing.T) {
}, },
}, },
}, },
errtype: field.ErrorTypeRequired, errs: []verr{{
errfield: "rbd.image", etype: field.ErrorTypeRequired,
field: "rbd.image",
}},
}, },
// Cinder // Cinder
{ {
@ -2747,8 +2820,10 @@ func TestValidateVolumes(t *testing.T) {
}, },
}, },
}, },
errtype: field.ErrorTypeRequired, errs: []verr{{
errfield: "cephfs.monitors", etype: field.ErrorTypeRequired,
field: "cephfs.monitors",
}},
}, },
// DownwardAPI // DownwardAPI
{ {
@ -2921,8 +2996,10 @@ func TestValidateVolumes(t *testing.T) {
}, },
}, },
}, },
errtype: field.ErrorTypeInvalid, errs: []verr{{
errfield: "downwardAPI.mode", etype: field.ErrorTypeInvalid,
field: "downwardAPI.mode",
}},
}, },
{ {
name: "downapi invalid negative item mode", name: "downapi invalid negative item mode",
@ -2941,8 +3018,10 @@ func TestValidateVolumes(t *testing.T) {
}, },
}, },
}, },
errtype: field.ErrorTypeInvalid, errs: []verr{{
errfield: "downwardAPI.mode", etype: field.ErrorTypeInvalid,
field: "downwardAPI.mode",
}},
}, },
{ {
name: "downapi empty metatada path", name: "downapi empty metatada path",
@ -2960,8 +3039,10 @@ func TestValidateVolumes(t *testing.T) {
}, },
}, },
}, },
errtype: field.ErrorTypeRequired, errs: []verr{{
errfield: "downwardAPI.path", etype: field.ErrorTypeRequired,
field: "downwardAPI.path",
}},
}, },
{ {
name: "downapi absolute path", name: "downapi absolute path",
@ -2979,8 +3060,10 @@ func TestValidateVolumes(t *testing.T) {
}, },
}, },
}, },
errtype: field.ErrorTypeInvalid, errs: []verr{{
errfield: "downwardAPI.path", etype: field.ErrorTypeInvalid,
field: "downwardAPI.path",
}},
}, },
{ {
name: "downapi dot dot path", name: "downapi dot dot path",
@ -2998,9 +3081,11 @@ func TestValidateVolumes(t *testing.T) {
}, },
}, },
}, },
errtype: field.ErrorTypeInvalid, errs: []verr{{
errfield: "downwardAPI.path", etype: field.ErrorTypeInvalid,
errdetail: `must not contain '..'`, field: "downwardAPI.path",
detail: `must not contain '..'`,
}},
}, },
{ {
name: "downapi dot dot file name", name: "downapi dot dot file name",
@ -3018,9 +3103,11 @@ func TestValidateVolumes(t *testing.T) {
}, },
}, },
}, },
errtype: field.ErrorTypeInvalid, errs: []verr{{
errfield: "downwardAPI.path", etype: field.ErrorTypeInvalid,
errdetail: `must not start with '..'`, field: "downwardAPI.path",
detail: `must not start with '..'`,
}},
}, },
{ {
name: "downapi dot dot first level dirent", name: "downapi dot dot first level dirent",
@ -3038,9 +3125,11 @@ func TestValidateVolumes(t *testing.T) {
}, },
}, },
}, },
errtype: field.ErrorTypeInvalid, errs: []verr{{
errfield: "downwardAPI.path", etype: field.ErrorTypeInvalid,
errdetail: `must not start with '..'`, field: "downwardAPI.path",
detail: `must not start with '..'`,
}},
}, },
{ {
name: "downapi fieldRef and ResourceFieldRef together", name: "downapi fieldRef and ResourceFieldRef together",
@ -3062,9 +3151,11 @@ func TestValidateVolumes(t *testing.T) {
}, },
}, },
}, },
errtype: field.ErrorTypeInvalid, errs: []verr{{
errfield: "downwardAPI", etype: field.ErrorTypeInvalid,
errdetail: "fieldRef and resourceFieldRef can not be specified simultaneously", field: "downwardAPI",
detail: "fieldRef and resourceFieldRef can not be specified simultaneously",
}},
}, },
{ {
name: "downapi invalid positive defaultMode", name: "downapi invalid positive defaultMode",
@ -3076,8 +3167,10 @@ func TestValidateVolumes(t *testing.T) {
}, },
}, },
}, },
errtype: field.ErrorTypeInvalid, errs: []verr{{
errfield: "downwardAPI.defaultMode", etype: field.ErrorTypeInvalid,
field: "downwardAPI.defaultMode",
}},
}, },
{ {
name: "downapi invalid negative defaultMode", name: "downapi invalid negative defaultMode",
@ -3089,8 +3182,10 @@ func TestValidateVolumes(t *testing.T) {
}, },
}, },
}, },
errtype: field.ErrorTypeInvalid, errs: []verr{{
errfield: "downwardAPI.defaultMode", etype: field.ErrorTypeInvalid,
field: "downwardAPI.defaultMode",
}},
}, },
// FC // FC
{ {
@ -3134,9 +3229,11 @@ func TestValidateVolumes(t *testing.T) {
}, },
}, },
}, },
errtype: field.ErrorTypeRequired, errs: []verr{{
errfield: "fc.targetWWNs", etype: field.ErrorTypeRequired,
errdetail: "must specify either targetWWNs or wwids", field: "fc.targetWWNs",
detail: "must specify either targetWWNs or wwids",
}},
}, },
{ {
name: "FC invalid: both targetWWNs and wwids simultaneously", name: "FC invalid: both targetWWNs and wwids simultaneously",
@ -3152,9 +3249,11 @@ func TestValidateVolumes(t *testing.T) {
}, },
}, },
}, },
errtype: field.ErrorTypeInvalid, errs: []verr{{
errfield: "fc.targetWWNs", etype: field.ErrorTypeInvalid,
errdetail: "targetWWNs and wwids can not be specified simultaneously", field: "fc.targetWWNs",
detail: "targetWWNs and wwids can not be specified simultaneously",
}},
}, },
{ {
name: "FC valid targetWWNs and empty lun", name: "FC valid targetWWNs and empty lun",
@ -3169,9 +3268,11 @@ func TestValidateVolumes(t *testing.T) {
}, },
}, },
}, },
errtype: field.ErrorTypeRequired, errs: []verr{{
errfield: "fc.lun", etype: field.ErrorTypeRequired,
errdetail: "lun is required if targetWWNs is specified", field: "fc.lun",
detail: "lun is required if targetWWNs is specified",
}},
}, },
{ {
name: "FC valid targetWWNs and invalid lun", name: "FC valid targetWWNs and invalid lun",
@ -3186,9 +3287,11 @@ func TestValidateVolumes(t *testing.T) {
}, },
}, },
}, },
errtype: field.ErrorTypeInvalid, errs: []verr{{
errfield: "fc.lun", etype: field.ErrorTypeInvalid,
errdetail: validation.InclusiveRangeError(0, 255), field: "fc.lun",
detail: validation.InclusiveRangeError(0, 255),
}},
}, },
// FlexVolume // FlexVolume
{ {
@ -3229,8 +3332,10 @@ func TestValidateVolumes(t *testing.T) {
}, },
}, },
}, },
errtype: field.ErrorTypeRequired, errs: []verr{{
errfield: "azureFile.secretName", etype: field.ErrorTypeRequired,
field: "azureFile.secretName",
}},
}, },
{ {
name: "AzureFile empty share", name: "AzureFile empty share",
@ -3244,8 +3349,10 @@ func TestValidateVolumes(t *testing.T) {
}, },
}, },
}, },
errtype: field.ErrorTypeRequired, errs: []verr{{
errfield: "azureFile.shareName", etype: field.ErrorTypeRequired,
field: "azureFile.shareName",
}},
}, },
// Quobyte // Quobyte
{ {
@ -3273,8 +3380,10 @@ func TestValidateVolumes(t *testing.T) {
}, },
}, },
}, },
errtype: field.ErrorTypeRequired, errs: []verr{{
errfield: "quobyte.registry", etype: field.ErrorTypeRequired,
field: "quobyte.registry",
}},
}, },
{ {
name: "wrong format registry quobyte", name: "wrong format registry quobyte",
@ -3287,8 +3396,10 @@ func TestValidateVolumes(t *testing.T) {
}, },
}, },
}, },
errtype: field.ErrorTypeInvalid, errs: []verr{{
errfield: "quobyte.registry", etype: field.ErrorTypeInvalid,
field: "quobyte.registry",
}},
}, },
{ {
name: "wrong format multiple registries quobyte", name: "wrong format multiple registries quobyte",
@ -3301,8 +3412,10 @@ func TestValidateVolumes(t *testing.T) {
}, },
}, },
}, },
errtype: field.ErrorTypeInvalid, errs: []verr{{
errfield: "quobyte.registry", etype: field.ErrorTypeInvalid,
field: "quobyte.registry",
}},
}, },
{ {
name: "empty volume quobyte", name: "empty volume quobyte",
@ -3314,8 +3427,10 @@ func TestValidateVolumes(t *testing.T) {
}, },
}, },
}, },
errtype: field.ErrorTypeRequired, errs: []verr{{
errfield: "quobyte.volume", etype: field.ErrorTypeRequired,
field: "quobyte.volume",
}},
}, },
// AzureDisk // AzureDisk
{ {
@ -3341,8 +3456,10 @@ func TestValidateVolumes(t *testing.T) {
}, },
}, },
}, },
errtype: field.ErrorTypeRequired, errs: []verr{{
errfield: "azureDisk.diskName", etype: field.ErrorTypeRequired,
field: "azureDisk.diskName",
}},
}, },
{ {
name: "AzureDisk empty disk uri", name: "AzureDisk empty disk uri",
@ -3355,8 +3472,10 @@ func TestValidateVolumes(t *testing.T) {
}, },
}, },
}, },
errtype: field.ErrorTypeRequired, errs: []verr{{
errfield: "azureDisk.diskURI", etype: field.ErrorTypeRequired,
field: "azureDisk.diskURI",
}},
}, },
// ScaleIO // ScaleIO
{ {
@ -3384,8 +3503,10 @@ func TestValidateVolumes(t *testing.T) {
}, },
}, },
}, },
errtype: field.ErrorTypeRequired, errs: []verr{{
errfield: "scaleIO.volumeName", etype: field.ErrorTypeRequired,
field: "scaleIO.volumeName",
}},
}, },
{ {
name: "ScaleIO with empty gateway", name: "ScaleIO with empty gateway",
@ -3399,8 +3520,10 @@ func TestValidateVolumes(t *testing.T) {
}, },
}, },
}, },
errtype: field.ErrorTypeRequired, errs: []verr{{
errfield: "scaleIO.gateway", etype: field.ErrorTypeRequired,
field: "scaleIO.gateway",
}},
}, },
{ {
name: "ScaleIO with empty system", name: "ScaleIO with empty system",
@ -3414,32 +3537,100 @@ func TestValidateVolumes(t *testing.T) {
}, },
}, },
}, },
errtype: field.ErrorTypeRequired, errs: []verr{{
errfield: "scaleIO.system", etype: field.ErrorTypeRequired,
field: "scaleIO.system",
}},
},
// ProjectedVolumeSource
{
name: "ProjectedVolumeSource more than one projection in a source",
vol: core.Volume{
Name: "projected-volume",
VolumeSource: core.VolumeSource{
Projected: &core.ProjectedVolumeSource{
Sources: []core.VolumeProjection{
{
Secret: &core.SecretProjection{
LocalObjectReference: core.LocalObjectReference{
Name: "foo",
},
},
},
{
Secret: &core.SecretProjection{
LocalObjectReference: core.LocalObjectReference{
Name: "foo",
},
},
DownwardAPI: &core.DownwardAPIProjection{},
},
},
},
},
},
errs: []verr{{
etype: field.ErrorTypeForbidden,
field: "projected.sources[1]",
}},
},
{
name: "ProjectedVolumeSource more than one projection in a source",
vol: core.Volume{
Name: "projected-volume",
VolumeSource: core.VolumeSource{
Projected: &core.ProjectedVolumeSource{
Sources: []core.VolumeProjection{
{
Secret: &core.SecretProjection{},
},
{
Secret: &core.SecretProjection{},
DownwardAPI: &core.DownwardAPIProjection{},
},
},
},
},
},
errs: []verr{
{
etype: field.ErrorTypeRequired,
field: "projected.sources[0].secret.name",
},
{
etype: field.ErrorTypeRequired,
field: "projected.sources[1].secret.name",
},
{
etype: field.ErrorTypeForbidden,
field: "projected.sources[1]",
},
},
}, },
} }
for i, tc := range testCases { for _, tc := range testCases {
names, errs := ValidateVolumes([]core.Volume{tc.vol}, field.NewPath("field")) t.Run(tc.name, func(t *testing.T) {
if len(errs) > 0 && tc.errtype == "" { names, errs := ValidateVolumes([]core.Volume{tc.vol}, field.NewPath("field"))
t.Errorf("[%d: %q] unexpected error(s): %v", i, tc.name, errs) if len(errs) != len(tc.errs) {
} else if len(errs) > 1 { t.Fatalf("unexpected error(s): got %d, want %d: %v", len(tc.errs), len(errs), errs)
t.Errorf("[%d: %q] expected 1 error, got %d: %v", i, tc.name, len(errs), 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)
} else if !strings.Contains(errs[0].Detail, tc.errdetail) {
t.Errorf("[%d: %q] expected error detail %q, got %q", i, tc.name, tc.errdetail, errs[0].Detail)
} }
} else { if len(errs) == 0 && (len(names) > 1 || !IsMatchedVolume(tc.vol.Name, names)) {
if len(names) != 1 || !IsMatchedVolume(tc.vol.Name, names) { t.Errorf("wrong names result: %v", names)
t.Errorf("[%d: %q] wrong names result: %v", i, tc.name, names)
} }
} for i, err := range errs {
expErr := tc.errs[i]
if err.Type != expErr.etype {
t.Errorf("unexpected error type: got %v, want %v", expErr.etype, err.Type)
}
if !strings.HasSuffix(err.Field, "."+expErr.field) {
t.Errorf("unexpected error field: got %v, want %v", expErr.field, err.Field)
}
if !strings.Contains(err.Detail, expErr.detail) {
t.Errorf("unexpected error detail: got %v, want %v", expErr.detail, err.Detail)
}
}
})
} }
dupsCase := []core.Volume{ dupsCase := []core.Volume{