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.
This commit is contained in:
Mike Danese 2018-03-31 14:13:35 -07:00
parent 500893cf99
commit a5d2ca8c55
2 changed files with 110 additions and 55 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

@ -3542,6 +3542,71 @@ func TestValidateVolumes(t *testing.T) {
field: "scaleIO.system", 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 _, tc := range testCases { for _, tc := range testCases {