diff --git a/pkg/api/pod/warnings.go b/pkg/api/pod/warnings.go index 45ac5ec042a..a4c3dc711a9 100644 --- a/pkg/api/pod/warnings.go +++ b/pkg/api/pod/warnings.go @@ -20,7 +20,6 @@ import ( "context" "fmt" "os" - "sort" "strings" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -380,119 +379,149 @@ func warningsForWeightedPodAffinityTerms(terms []api.WeightedPodAffinityTerm, fi func warningsForOverlappingVirtualPaths(volumes []api.Volume) []string { var warnings []string + mkWarn := func(volName, volDesc, body string) string { + return fmt.Sprintf("volume %q (%s): overlapping paths: %s", volName, volDesc, body) + } + for _, v := range volumes { if v.ConfigMap != nil && v.ConfigMap.Items != nil { - warnings = append(warnings, checkVolumeMappingForOverlap(extractPaths(v.ConfigMap.Items), fmt.Sprintf("volume %q (ConfigMap %q): overlapping paths", v.Name, v.ConfigMap.Name))...) + overlaps := checkVolumeMappingForOverlap(extractPaths(v.ConfigMap.Items, "")) + for _, ol := range overlaps { + warnings = append(warnings, mkWarn(v.Name, fmt.Sprintf("ConfigMap %q", v.ConfigMap.Name), ol)) + } } if v.Secret != nil && v.Secret.Items != nil { - warnings = append(warnings, checkVolumeMappingForOverlap(extractPaths(v.Secret.Items), fmt.Sprintf("volume %q (Secret %q): overlapping paths", v.Name, v.Secret.SecretName))...) + overlaps := checkVolumeMappingForOverlap(extractPaths(v.Secret.Items, "")) + for _, ol := range overlaps { + warnings = append(warnings, mkWarn(v.Name, fmt.Sprintf("Secret %q", v.Secret.SecretName), ol)) + } } if v.DownwardAPI != nil && v.DownwardAPI.Items != nil { - warnings = append(warnings, checkVolumeMappingForOverlap(extractPathsDownwardAPI(v.DownwardAPI.Items), fmt.Sprintf("volume %q (DownwardAPI): overlapping paths", v.Name))...) + overlaps := checkVolumeMappingForOverlap(extractPathsDownwardAPI(v.DownwardAPI.Items, "")) + for _, ol := range overlaps { + warnings = append(warnings, mkWarn(v.Name, "DownwardAPI", ol)) + } } if v.Projected != nil { - var sourcePaths []string - var errorMessage string - allPaths := sets.New[string]() + var sourcePaths []pathAndSource + var allPaths []pathAndSource for _, source := range v.Projected.Sources { if source == (api.VolumeProjection{}) { warnings = append(warnings, fmt.Sprintf("volume %q (Projected) has no sources provided", v.Name)) continue } + switch { case source.ConfigMap != nil && source.ConfigMap.Items != nil: - sourcePaths = extractPaths(source.ConfigMap.Items) - errorMessage = fmt.Sprintf("volume %q (Projected ConfigMap %q): overlapping paths", v.Name, source.ConfigMap.Name) + sourcePaths = extractPaths(source.ConfigMap.Items, fmt.Sprintf("ConfigMap %q", source.ConfigMap.Name)) case source.Secret != nil && source.Secret.Items != nil: - sourcePaths = extractPaths(source.Secret.Items) - errorMessage = fmt.Sprintf("volume %q (Projected Secret %q): overlapping paths", v.Name, source.Secret.Name) + sourcePaths = extractPaths(source.Secret.Items, fmt.Sprintf("Secret %q", source.Secret.Name)) case source.DownwardAPI != nil && source.DownwardAPI.Items != nil: - sourcePaths = extractPathsDownwardAPI(source.DownwardAPI.Items) - errorMessage = fmt.Sprintf("volume %q (Projected DownwardAPI): overlapping paths", v.Name) + sourcePaths = extractPathsDownwardAPI(source.DownwardAPI.Items, "DownwardAPI") case source.ServiceAccountToken != nil: - sourcePaths = []string{source.ServiceAccountToken.Path} - errorMessage = fmt.Sprintf("volume %q (Projected ServiceAccountToken): overlapping paths", v.Name) + sourcePaths = []pathAndSource{{source.ServiceAccountToken.Path, "ServiceAccountToken"}} case source.ClusterTrustBundle != nil: - sourcePaths = []string{source.ClusterTrustBundle.Path} + name := "" if source.ClusterTrustBundle.Name != nil { - errorMessage = fmt.Sprintf("volume %q (Projected ClusterTrustBundle %q): overlapping paths", v.Name, *source.ClusterTrustBundle.Name) + name = *source.ClusterTrustBundle.Name } else { - errorMessage = fmt.Sprintf("volume %q (Projected ClusterTrustBundle %q): overlapping paths", v.Name, *source.ClusterTrustBundle.SignerName) + name = *source.ClusterTrustBundle.SignerName } + sourcePaths = []pathAndSource{{source.ClusterTrustBundle.Path, fmt.Sprintf("ClusterTrustBundle %q", name)}} } if len(sourcePaths) == 0 { continue } - warnings = append(warnings, checkVolumeMappingForOverlap(sourcePaths, errorMessage)...) - - // remove duplicates path and sort the new array, so we can get predetermined result - uniqueSourcePaths := sets.New[string](sourcePaths...).UnsortedList() - orderedSourcePaths := append(uniqueSourcePaths, allPaths.UnsortedList()...) - sort.Strings(orderedSourcePaths) - warnings = append(warnings, checkVolumeMappingForOverlap(orderedSourcePaths, errorMessage)...) - allPaths.Insert(uniqueSourcePaths...) + for _, ps := range sourcePaths { + ps.path = strings.TrimRight(ps.path, string(os.PathSeparator)) + if collisions := checkForOverlap(allPaths, ps); len(collisions) > 0 { + for _, c := range collisions { + warnings = append(warnings, mkWarn(v.Name, "Projected", fmt.Sprintf("%s with %s", ps.String(), c.String()))) + } + } + allPaths = append(allPaths, ps) + } } } - } return warnings } -func extractPaths(mapping []api.KeyToPath) []string { - result := make([]string, 0, len(mapping)) +// this lets us track a path and where it came from, for better errors +type pathAndSource struct { + path string + source string +} + +func (ps pathAndSource) String() string { + if ps.source != "" { + return fmt.Sprintf("%q (%s)", ps.path, ps.source) + } + return fmt.Sprintf("%q", ps.path) +} + +func extractPaths(mapping []api.KeyToPath, source string) []pathAndSource { + result := make([]pathAndSource, 0, len(mapping)) for _, v := range mapping { - result = append(result, v.Path) + result = append(result, pathAndSource{v.Path, source}) } return result } -func extractPathsDownwardAPI(mapping []api.DownwardAPIVolumeFile) []string { - result := make([]string, 0, len(mapping)) +func extractPathsDownwardAPI(mapping []api.DownwardAPIVolumeFile, source string) []pathAndSource { + result := make([]pathAndSource, 0, len(mapping)) for _, v := range mapping { - result = append(result, v.Path) + result = append(result, pathAndSource{v.Path, source}) } return result } -func checkVolumeMappingForOverlap(paths []string, warningMessage string) []string { +func checkVolumeMappingForOverlap(paths []pathAndSource) []string { + pathSeparator := string(os.PathSeparator) var warnings []string - pathSeparator := string(os.PathSeparator) - uniquePaths := sets.New[string]() + var allPaths []pathAndSource - for _, path := range paths { - normalizedPath := strings.TrimRight(path, pathSeparator) - if collision := checkForOverlap(uniquePaths, normalizedPath); collision != "" { - warnings = append(warnings, fmt.Sprintf("%s: %q with %q", warningMessage, normalizedPath, collision)) + for _, ps := range paths { + ps.path = strings.TrimRight(ps.path, pathSeparator) + if collisions := checkForOverlap(allPaths, ps); len(collisions) > 0 { + for _, c := range collisions { + warnings = append(warnings, fmt.Sprintf("%s with %s", ps.String(), c.String())) + } } - uniquePaths.Insert(normalizedPath) + allPaths = append(allPaths, ps) } return warnings } -func checkForOverlap(paths sets.Set[string], path string) string { +func checkForOverlap(haystack []pathAndSource, needle pathAndSource) []pathAndSource { pathSeparator := string(os.PathSeparator) - for item := range paths { + if needle.path == "" { + return nil + } + + var result []pathAndSource + for _, item := range haystack { switch { - case item == "" || path == "": - return "" - case item == path: - return item - case strings.HasPrefix(item+pathSeparator, path): - return item - case strings.HasPrefix(path+pathSeparator, item): - return item + case item.path == "": + continue + case item == needle: + result = append(result, item) + case strings.HasPrefix(item.path+pathSeparator, needle.path+pathSeparator): + result = append(result, item) + case strings.HasPrefix(needle.path+pathSeparator, item.path+pathSeparator): + result = append(result, item) } } - return "" + return result } diff --git a/pkg/api/pod/warnings_test.go b/pkg/api/pod/warnings_test.go index 07aabb297f3..f3f0687131d 100644 --- a/pkg/api/pod/warnings_test.go +++ b/pkg/api/pod/warnings_test.go @@ -18,6 +18,7 @@ package pod import ( "context" + "reflect" "strings" "testing" @@ -240,20 +241,18 @@ func TestWarnings(t *testing.T) { { name: "overlapping paths in a configmap volume", template: &api.PodTemplateSpec{Spec: api.PodSpec{ - Volumes: []api.Volume{ - { - Name: "Test", - VolumeSource: api.VolumeSource{ - ConfigMap: &api.ConfigMapVolumeSource{ - LocalObjectReference: api.LocalObjectReference{Name: "foo"}, - Items: []api.KeyToPath{ - {Key: "foo", Path: "test"}, - {Key: "bar", Path: "test"}, - }, + Volumes: []api.Volume{{ + Name: "Test", + VolumeSource: api.VolumeSource{ + ConfigMap: &api.ConfigMapVolumeSource{ + LocalObjectReference: api.LocalObjectReference{Name: "foo"}, + Items: []api.KeyToPath{ + {Key: "foo", Path: "test"}, + {Key: "bar", Path: "test"}, }, }, }, - }, + }}, }}, expected: []string{ `volume "Test" (ConfigMap "foo"): overlapping paths: "test" with "test"`, @@ -262,20 +261,18 @@ func TestWarnings(t *testing.T) { { name: "overlapping paths in a configmap volume - try to mount dir path into a file", template: &api.PodTemplateSpec{Spec: api.PodSpec{ - Volumes: []api.Volume{ - { - Name: "Test", - VolumeSource: api.VolumeSource{ - ConfigMap: &api.ConfigMapVolumeSource{ - LocalObjectReference: api.LocalObjectReference{Name: "foo"}, - Items: []api.KeyToPath{ - {Key: "foo", Path: "test"}, - {Key: "bar", Path: "test/app"}, - }, + Volumes: []api.Volume{{ + Name: "Test", + VolumeSource: api.VolumeSource{ + ConfigMap: &api.ConfigMapVolumeSource{ + LocalObjectReference: api.LocalObjectReference{Name: "foo"}, + Items: []api.KeyToPath{ + {Key: "foo", Path: "test"}, + {Key: "bar", Path: "test/app"}, }, }, }, - }, + }}, }}, expected: []string{ `volume "Test" (ConfigMap "foo"): overlapping paths: "test/app" with "test"`, @@ -284,20 +281,18 @@ func TestWarnings(t *testing.T) { { name: "overlapping paths in a configmap volume - try to mount file into a dir path", template: &api.PodTemplateSpec{Spec: api.PodSpec{ - Volumes: []api.Volume{ - { - Name: "Test", - VolumeSource: api.VolumeSource{ - ConfigMap: &api.ConfigMapVolumeSource{ - LocalObjectReference: api.LocalObjectReference{Name: "foo"}, - Items: []api.KeyToPath{ - {Key: "bar", Path: "test/app"}, - {Key: "foo", Path: "test"}, - }, + Volumes: []api.Volume{{ + Name: "Test", + VolumeSource: api.VolumeSource{ + ConfigMap: &api.ConfigMapVolumeSource{ + LocalObjectReference: api.LocalObjectReference{Name: "foo"}, + Items: []api.KeyToPath{ + {Key: "bar", Path: "test/app"}, + {Key: "foo", Path: "test"}, }, }, }, - }, + }}, }}, expected: []string{ `volume "Test" (ConfigMap "foo"): overlapping paths: "test" with "test/app"`, @@ -306,20 +301,18 @@ func TestWarnings(t *testing.T) { { name: "overlapping paths in a secret volume", template: &api.PodTemplateSpec{Spec: api.PodSpec{ - Volumes: []api.Volume{ - { - Name: "Test", - VolumeSource: api.VolumeSource{ - Secret: &api.SecretVolumeSource{ - SecretName: "foo", - Items: []api.KeyToPath{ - {Key: "foo", Path: "test"}, - {Key: "bar", Path: "test"}, - }, + Volumes: []api.Volume{{ + Name: "Test", + VolumeSource: api.VolumeSource{ + Secret: &api.SecretVolumeSource{ + SecretName: "foo", + Items: []api.KeyToPath{ + {Key: "foo", Path: "test"}, + {Key: "bar", Path: "test"}, }, }, }, - }, + }}, }}, expected: []string{ `volume "Test" (Secret "foo"): overlapping paths: "test" with "test"`, @@ -328,19 +321,17 @@ func TestWarnings(t *testing.T) { { name: "overlapping paths in a downward api volume", template: &api.PodTemplateSpec{Spec: api.PodSpec{ - Volumes: []api.Volume{ - { - Name: "Test", - VolumeSource: api.VolumeSource{ - DownwardAPI: &api.DownwardAPIVolumeSource{ - Items: []api.DownwardAPIVolumeFile{ - {FieldRef: &api.ObjectFieldSelector{APIVersion: "v1", FieldPath: "metadata.name"}, Path: "test"}, - {FieldRef: &api.ObjectFieldSelector{APIVersion: "v1", FieldPath: "metadata.labels"}, Path: "test"}, - }, + Volumes: []api.Volume{{ + Name: "Test", + VolumeSource: api.VolumeSource{ + DownwardAPI: &api.DownwardAPIVolumeSource{ + Items: []api.DownwardAPIVolumeFile{ + {FieldRef: &api.ObjectFieldSelector{APIVersion: "v1", FieldPath: "metadata.name"}, Path: "test"}, + {FieldRef: &api.ObjectFieldSelector{APIVersion: "v1", FieldPath: "metadata.labels"}, Path: "test"}, }, }, }, - }, + }}, }}, expected: []string{ `volume "Test" (DownwardAPI): overlapping paths: "test" with "test"`, @@ -349,425 +340,357 @@ func TestWarnings(t *testing.T) { { name: "overlapping paths in projected volume - service account and config", template: &api.PodTemplateSpec{Spec: api.PodSpec{ - Volumes: []api.Volume{ - { - Name: "foo", - VolumeSource: api.VolumeSource{ - Projected: &api.ProjectedVolumeSource{ - Sources: []api.VolumeProjection{ - { - ConfigMap: &api.ConfigMapProjection{ - LocalObjectReference: api.LocalObjectReference{Name: "Test"}, - Items: []api.KeyToPath{ - {Key: "foo", Path: "test"}, - }, - }, - }, - { - ServiceAccountToken: &api.ServiceAccountTokenProjection{ - Path: "test", - }, + Volumes: []api.Volume{{ + Name: "foo", + VolumeSource: api.VolumeSource{ + Projected: &api.ProjectedVolumeSource{ + Sources: []api.VolumeProjection{{ + ConfigMap: &api.ConfigMapProjection{ + LocalObjectReference: api.LocalObjectReference{Name: "Test"}, + Items: []api.KeyToPath{ + {Key: "foo", Path: "test"}, }, }, - }, + }, { + ServiceAccountToken: &api.ServiceAccountTokenProjection{ + Path: "test", + }, + }}, }, }, - }, + }}, }}, expected: []string{ - `volume "foo" (Projected ServiceAccountToken): overlapping paths: "test" with "test"`, + `volume "foo" (Projected): overlapping paths: "test" (ServiceAccountToken) with "test" (ConfigMap "Test")`, }, }, { name: "overlapping paths in projected volume volume: service account dir and config file", template: &api.PodTemplateSpec{Spec: api.PodSpec{ - Volumes: []api.Volume{ - { - Name: "foo", - VolumeSource: api.VolumeSource{ - Projected: &api.ProjectedVolumeSource{ - Sources: []api.VolumeProjection{ - { - ConfigMap: &api.ConfigMapProjection{ - LocalObjectReference: api.LocalObjectReference{Name: "Test"}, - Items: []api.KeyToPath{ - {Key: "foo", Path: "test"}, - }, - }, - }, - { - ServiceAccountToken: &api.ServiceAccountTokenProjection{ - Path: "test/file", - }, + Volumes: []api.Volume{{ + Name: "foo", + VolumeSource: api.VolumeSource{ + Projected: &api.ProjectedVolumeSource{ + Sources: []api.VolumeProjection{{ + ConfigMap: &api.ConfigMapProjection{ + LocalObjectReference: api.LocalObjectReference{Name: "Test"}, + Items: []api.KeyToPath{ + {Key: "foo", Path: "test"}, }, }, - }, + }, { + ServiceAccountToken: &api.ServiceAccountTokenProjection{ + Path: "test/file", + }, + }}, }, }, - }, + }}, }}, expected: []string{ - `volume "foo" (Projected ServiceAccountToken): overlapping paths: "test/file" with "test"`, + `volume "foo" (Projected): overlapping paths: "test/file" (ServiceAccountToken) with "test" (ConfigMap "Test")`, }, }, { name: "overlapping paths in projected volume - service account file and config dir", template: &api.PodTemplateSpec{Spec: api.PodSpec{ - Volumes: []api.Volume{ - { - Name: "foo", - VolumeSource: api.VolumeSource{ - Projected: &api.ProjectedVolumeSource{ - Sources: []api.VolumeProjection{ - { - ConfigMap: &api.ConfigMapProjection{ - LocalObjectReference: api.LocalObjectReference{Name: "Test"}, - Items: []api.KeyToPath{ - {Key: "foo", Path: "test/file"}, - }, - }, - }, - { - ServiceAccountToken: &api.ServiceAccountTokenProjection{ - Path: "test", - }, + Volumes: []api.Volume{{ + Name: "foo", + VolumeSource: api.VolumeSource{ + Projected: &api.ProjectedVolumeSource{ + Sources: []api.VolumeProjection{{ + ConfigMap: &api.ConfigMapProjection{ + LocalObjectReference: api.LocalObjectReference{Name: "Test"}, + Items: []api.KeyToPath{ + {Key: "foo", Path: "test/file"}, }, }, - }, + }, { + ServiceAccountToken: &api.ServiceAccountTokenProjection{ + Path: "test", + }, + }}, }, }, - }, + }}, }}, expected: []string{ - `volume "foo" (Projected ServiceAccountToken): overlapping paths: "test/file" with "test"`, + `volume "foo" (Projected): overlapping paths: "test" (ServiceAccountToken) with "test/file" (ConfigMap "Test")`, }, }, { name: "overlapping paths in projected volume - service account and secret", template: &api.PodTemplateSpec{Spec: api.PodSpec{ - Volumes: []api.Volume{ - { - Name: "foo", - VolumeSource: api.VolumeSource{ - Projected: &api.ProjectedVolumeSource{ - Sources: []api.VolumeProjection{ - { - Secret: &api.SecretProjection{ - LocalObjectReference: api.LocalObjectReference{Name: "Test"}, - Items: []api.KeyToPath{ - {Key: "foo", Path: "test"}, - }, - }, - }, - { - ServiceAccountToken: &api.ServiceAccountTokenProjection{ - Path: "test", - }, + Volumes: []api.Volume{{ + Name: "foo", + VolumeSource: api.VolumeSource{ + Projected: &api.ProjectedVolumeSource{ + Sources: []api.VolumeProjection{{ + Secret: &api.SecretProjection{ + LocalObjectReference: api.LocalObjectReference{Name: "Test"}, + Items: []api.KeyToPath{ + {Key: "foo", Path: "test"}, }, }, - }, + }, { + ServiceAccountToken: &api.ServiceAccountTokenProjection{ + Path: "test", + }, + }}, }, }, - }, + }}, }}, expected: []string{ - `volume "foo" (Projected ServiceAccountToken): overlapping paths: "test" with "test"`, + `volume "foo" (Projected): overlapping paths: "test" (ServiceAccountToken) with "test" (Secret "Test")`, }, }, { name: "overlapping paths in projected volume - service account and downward api", template: &api.PodTemplateSpec{Spec: api.PodSpec{ - Volumes: []api.Volume{ - { - Name: "foo", - VolumeSource: api.VolumeSource{ - Projected: &api.ProjectedVolumeSource{ - Sources: []api.VolumeProjection{ - { - DownwardAPI: &api.DownwardAPIProjection{ - Items: []api.DownwardAPIVolumeFile{ - {FieldRef: &api.ObjectFieldSelector{APIVersion: "v1", FieldPath: "metadata.name"}, Path: "test"}, - }, - }, - }, - { - ServiceAccountToken: &api.ServiceAccountTokenProjection{ - Path: "test", - }, - }, + Volumes: []api.Volume{{ + Name: "foo", + VolumeSource: api.VolumeSource{ + Projected: &api.ProjectedVolumeSource{ + Sources: []api.VolumeProjection{{ + DownwardAPI: &api.DownwardAPIProjection{ + Items: []api.DownwardAPIVolumeFile{{ + FieldRef: &api.ObjectFieldSelector{APIVersion: "v1", FieldPath: "metadata.name"}, + Path: "test", + }}, }, - }, + }, { + ServiceAccountToken: &api.ServiceAccountTokenProjection{ + Path: "test", + }, + }}, }, }, - }, + }}, }}, expected: []string{ - `volume "foo" (Projected ServiceAccountToken): overlapping paths: "test" with "test"`, + `volume "foo" (Projected): overlapping paths: "test" (ServiceAccountToken) with "test" (DownwardAPI)`, }, }, { name: "overlapping paths in projected volume - service account and cluster trust bundle", template: &api.PodTemplateSpec{Spec: api.PodSpec{ - Volumes: []api.Volume{ - { - Name: "foo", - VolumeSource: api.VolumeSource{ - Projected: &api.ProjectedVolumeSource{ - Sources: []api.VolumeProjection{ - { - ClusterTrustBundle: &api.ClusterTrustBundleProjection{ - Name: &testName, Path: "test", - }, - }, - { - ServiceAccountToken: &api.ServiceAccountTokenProjection{ - Path: "test", - }, - }, + Volumes: []api.Volume{{ + Name: "foo", + VolumeSource: api.VolumeSource{ + Projected: &api.ProjectedVolumeSource{ + Sources: []api.VolumeProjection{{ + ClusterTrustBundle: &api.ClusterTrustBundleProjection{ + Name: &testName, Path: "test", }, - }, + }, { + ServiceAccountToken: &api.ServiceAccountTokenProjection{ + Path: "test", + }, + }}, }, }, - }, + }}, }}, expected: []string{ - `volume "foo" (Projected ServiceAccountToken): overlapping paths: "test" with "test"`, + `volume "foo" (Projected): overlapping paths: "test" (ServiceAccountToken) with "test" (ClusterTrustBundle "Test")`, }, }, { name: "overlapping paths in projected volume - service account and cluster trust bundle with signer name", template: &api.PodTemplateSpec{Spec: api.PodSpec{ - Volumes: []api.Volume{ - { - Name: "foo", - VolumeSource: api.VolumeSource{ - Projected: &api.ProjectedVolumeSource{ - Sources: []api.VolumeProjection{ - { - ClusterTrustBundle: &api.ClusterTrustBundleProjection{ - SignerName: &testName, Path: "test", - }, - }, - { - ServiceAccountToken: &api.ServiceAccountTokenProjection{ - Path: "test", - }, - }, + Volumes: []api.Volume{{ + Name: "foo", + VolumeSource: api.VolumeSource{ + Projected: &api.ProjectedVolumeSource{ + Sources: []api.VolumeProjection{{ + ClusterTrustBundle: &api.ClusterTrustBundleProjection{ + SignerName: &testName, Path: "test", }, - }, + }, { + ServiceAccountToken: &api.ServiceAccountTokenProjection{ + Path: "test", + }, + }}, }, }, - }, + }}, }}, expected: []string{ - `volume "foo" (Projected ServiceAccountToken): overlapping paths: "test" with "test"`, + `volume "foo" (Projected): overlapping paths: "test" (ServiceAccountToken) with "test" (ClusterTrustBundle "Test")`, }, }, { name: "overlapping paths in projected volume - secret and config map", template: &api.PodTemplateSpec{Spec: api.PodSpec{ - Volumes: []api.Volume{ - { - Name: "foo", - VolumeSource: api.VolumeSource{ - Projected: &api.ProjectedVolumeSource{ - Sources: []api.VolumeProjection{ - { - Secret: &api.SecretProjection{ - LocalObjectReference: api.LocalObjectReference{Name: "TestSecret"}, - Items: []api.KeyToPath{ - { - Key: "mykey", - Path: "test", - }, - }, - }, - }, - { - ConfigMap: &api.ConfigMapProjection{ - LocalObjectReference: api.LocalObjectReference{Name: "TestConfigMap"}, - Items: []api.KeyToPath{ - { - Key: "mykey", - Path: "test/test1", - }, - }, - }, + Volumes: []api.Volume{{ + Name: "foo", + VolumeSource: api.VolumeSource{ + Projected: &api.ProjectedVolumeSource{ + Sources: []api.VolumeProjection{{ + Secret: &api.SecretProjection{ + LocalObjectReference: api.LocalObjectReference{Name: "TestSecret"}, + Items: []api.KeyToPath{ + {Key: "mykey", Path: "test"}, }, }, - }, + }, { + ConfigMap: &api.ConfigMapProjection{ + LocalObjectReference: api.LocalObjectReference{Name: "TestConfigMap"}, + Items: []api.KeyToPath{ + {Key: "mykey", Path: "test/test1"}, + }, + }, + }}, }, }, - }, + }}, }}, expected: []string{ - `volume "foo" (Projected ConfigMap "TestConfigMap"): overlapping paths: "test/test1" with "test"`, + `volume "foo" (Projected): overlapping paths: "test/test1" (ConfigMap "TestConfigMap") with "test" (Secret "TestSecret")`, }, }, { name: "overlapping paths in projected volume - config map and downward api", template: &api.PodTemplateSpec{Spec: api.PodSpec{ - Volumes: []api.Volume{ - { - Name: "foo", - VolumeSource: api.VolumeSource{ - Projected: &api.ProjectedVolumeSource{ - Sources: []api.VolumeProjection{ - { - Secret: &api.SecretProjection{ - LocalObjectReference: api.LocalObjectReference{Name: "TestSecret"}, - Items: []api.KeyToPath{ - { - Key: "mykey", - Path: "test", - }, - }, - }, - }, - { - DownwardAPI: &api.DownwardAPIProjection{ - Items: []api.DownwardAPIVolumeFile{ - {FieldRef: &api.ObjectFieldSelector{APIVersion: "v1", FieldPath: "metadata.name"}, Path: "test/test2"}, - }, - }, + Volumes: []api.Volume{{ + Name: "foo", + VolumeSource: api.VolumeSource{ + Projected: &api.ProjectedVolumeSource{ + Sources: []api.VolumeProjection{{ + Secret: &api.SecretProjection{ + LocalObjectReference: api.LocalObjectReference{Name: "TestSecret"}, + Items: []api.KeyToPath{ + {Key: "mykey", Path: "test"}, }, }, - }, + }, { + DownwardAPI: &api.DownwardAPIProjection{ + Items: []api.DownwardAPIVolumeFile{{ + FieldRef: &api.ObjectFieldSelector{APIVersion: "v1", FieldPath: "metadata.name"}, + Path: "test/test2", + }}, + }, + }}, }, }, - }, + }}, }}, expected: []string{ - `volume "foo" (Projected DownwardAPI): overlapping paths: "test/test2" with "test"`, + `volume "foo" (Projected): overlapping paths: "test/test2" (DownwardAPI) with "test" (Secret "TestSecret")`, }, }, { name: "overlapping paths in projected volume - downward api and cluster thrust bundle api", template: &api.PodTemplateSpec{Spec: api.PodSpec{ - Volumes: []api.Volume{ - { - Name: "foo", - VolumeSource: api.VolumeSource{ - Projected: &api.ProjectedVolumeSource{ - Sources: []api.VolumeProjection{ - { - DownwardAPI: &api.DownwardAPIProjection{ - Items: []api.DownwardAPIVolumeFile{ - {FieldRef: &api.ObjectFieldSelector{APIVersion: "v1", FieldPath: "metadata.name"}, Path: "test/test2"}, - }, - }, - }, - { - ClusterTrustBundle: &api.ClusterTrustBundleProjection{ - Name: &testName, Path: "test", - }, - }, + Volumes: []api.Volume{{ + Name: "foo", + VolumeSource: api.VolumeSource{ + Projected: &api.ProjectedVolumeSource{ + Sources: []api.VolumeProjection{{ + DownwardAPI: &api.DownwardAPIProjection{ + Items: []api.DownwardAPIVolumeFile{{ + FieldRef: &api.ObjectFieldSelector{APIVersion: "v1", FieldPath: "metadata.name"}, + Path: "test/test2", + }}, }, - }, + }, { + ClusterTrustBundle: &api.ClusterTrustBundleProjection{ + Name: &testName, Path: "test", + }, + }}, }, }, - }, + }}, }}, expected: []string{ - `volume "foo" (Projected ClusterTrustBundle "Test"): overlapping paths: "test/test2" with "test"`, + `volume "foo" (Projected): overlapping paths: "test" (ClusterTrustBundle "Test") with "test/test2" (DownwardAPI)`, }, }, { name: "overlapping paths in projected volume - multiple sources", template: &api.PodTemplateSpec{Spec: api.PodSpec{ - Volumes: []api.Volume{ - { - Name: "foo", - VolumeSource: api.VolumeSource{ - Projected: &api.ProjectedVolumeSource{ - Sources: []api.VolumeProjection{ - { - ClusterTrustBundle: &api.ClusterTrustBundleProjection{ - SignerName: &testName, Path: "test/test", - }, - }, - { - DownwardAPI: &api.DownwardAPIProjection{ - Items: []api.DownwardAPIVolumeFile{ - {FieldRef: &api.ObjectFieldSelector{APIVersion: "v1", FieldPath: "metadata.name"}, Path: "test"}, - }, - }, - }, - { - Secret: &api.SecretProjection{ - LocalObjectReference: api.LocalObjectReference{Name: "Test"}, - Items: []api.KeyToPath{ - {Key: "foo", Path: "test"}, - }, - }, - }, - { - ServiceAccountToken: &api.ServiceAccountTokenProjection{ - Path: "test", - }, + Volumes: []api.Volume{{ + Name: "foo", + VolumeSource: api.VolumeSource{ + Projected: &api.ProjectedVolumeSource{ + Sources: []api.VolumeProjection{{ + ClusterTrustBundle: &api.ClusterTrustBundleProjection{ + SignerName: &testName, Path: "test/test"}, + }, { + DownwardAPI: &api.DownwardAPIProjection{ + Items: []api.DownwardAPIVolumeFile{{ + FieldRef: &api.ObjectFieldSelector{APIVersion: "v1", FieldPath: "metadata.name"}, + Path: "test", + }}, + }, + }, { + Secret: &api.SecretProjection{ + LocalObjectReference: api.LocalObjectReference{Name: "Test"}, + Items: []api.KeyToPath{ + {Key: "foo", Path: "test"}, }, }, - }, + }, { + ServiceAccountToken: &api.ServiceAccountTokenProjection{ + Path: "test", + }, + }}, }, }, - }, + }}, }}, expected: []string{ - `volume "foo" (Projected DownwardAPI): overlapping paths: "test/test" with "test"`, - `volume "foo" (Projected Secret "Test"): overlapping paths: "test" with "test"`, - `volume "foo" (Projected Secret "Test"): overlapping paths: "test/test" with "test"`, - `volume "foo" (Projected ServiceAccountToken): overlapping paths: "test/test" with "test"`, - `volume "foo" (Projected ServiceAccountToken): overlapping paths: "test" with "test"`, + `volume "foo" (Projected): overlapping paths: "test" (DownwardAPI) with "test/test" (ClusterTrustBundle "Test")`, + `volume "foo" (Projected): overlapping paths: "test" (Secret "Test") with "test/test" (ClusterTrustBundle "Test")`, + `volume "foo" (Projected): overlapping paths: "test" (Secret "Test") with "test" (DownwardAPI)`, + `volume "foo" (Projected): overlapping paths: "test" (ServiceAccountToken) with "test/test" (ClusterTrustBundle "Test")`, + `volume "foo" (Projected): overlapping paths: "test" (ServiceAccountToken) with "test" (DownwardAPI)`, + `volume "foo" (Projected): overlapping paths: "test" (ServiceAccountToken) with "test" (Secret "Test")`, }, }, { - name: "overlapping paths in projected volume - multiple sources", + name: "overlapping paths in projected volume - ServiceAccount vs. DownwardAPI", template: &api.PodTemplateSpec{Spec: api.PodSpec{ - Volumes: []api.Volume{ - { - Name: "foo", - VolumeSource: api.VolumeSource{ - Projected: &api.ProjectedVolumeSource{ - Sources: []api.VolumeProjection{ - { - ServiceAccountToken: &api.ServiceAccountTokenProjection{ - Path: "test/test2", - }, - }, - { - DownwardAPI: &api.DownwardAPIProjection{ - Items: []api.DownwardAPIVolumeFile{ - {FieldRef: &api.ObjectFieldSelector{APIVersion: "v1", FieldPath: "metadata.name"}, Path: "test"}, - {FieldRef: &api.ObjectFieldSelector{APIVersion: "v1", FieldPath: "metadata.name"}, Path: "test"}, - }, - }, + Volumes: []api.Volume{{ + Name: "foo", + VolumeSource: api.VolumeSource{ + Projected: &api.ProjectedVolumeSource{ + Sources: []api.VolumeProjection{{ + ServiceAccountToken: &api.ServiceAccountTokenProjection{ + Path: "test/test2", + }, + }, { + DownwardAPI: &api.DownwardAPIProjection{ + Items: []api.DownwardAPIVolumeFile{ + {FieldRef: &api.ObjectFieldSelector{APIVersion: "v1", FieldPath: "metadata.name"}, Path: "test"}, + {FieldRef: &api.ObjectFieldSelector{APIVersion: "v1", FieldPath: "metadata.name"}, Path: "test"}, }, }, - }, + }}, }, }, - }, + }}, }}, expected: []string{ - `volume "foo" (Projected DownwardAPI): overlapping paths: "test" with "test"`, - `volume "foo" (Projected DownwardAPI): overlapping paths: "test/test2" with "test"`, + `volume "foo" (Projected): overlapping paths: "test" (DownwardAPI) with "test/test2" (ServiceAccountToken)`, + `volume "foo" (Projected): overlapping paths: "test" (DownwardAPI) with "test/test2" (ServiceAccountToken)`, + `volume "foo" (Projected): overlapping paths: "test" (DownwardAPI) with "test" (DownwardAPI)`, }, }, { name: "empty sources in projected volume", template: &api.PodTemplateSpec{Spec: api.PodSpec{ - Volumes: []api.Volume{ - { - Name: "foo", - VolumeSource: api.VolumeSource{ - Projected: &api.ProjectedVolumeSource{ - Sources: []api.VolumeProjection{ - {}, - }, + Volumes: []api.Volume{{ + Name: "foo", + VolumeSource: api.VolumeSource{ + Projected: &api.ProjectedVolumeSource{ + Sources: []api.VolumeProjection{ + {}, // one item, no fields set }, }, }, - }, + }}, }}, expected: []string{ `volume "foo" (Projected) has no sources provided`, @@ -1615,7 +1538,7 @@ func TestWarnings(t *testing.T) { t.Errorf("missing: %s", missing) } for _, extra := range sets.List(actualSet.Difference(expectedSet)) { - t.Errorf("extra: %s", extra) + t.Errorf("extra: %s", extra) } }) @@ -1637,7 +1560,7 @@ func TestWarnings(t *testing.T) { t.Errorf("missing: %s", missing) } for _, extra := range sets.List(actualSet.Difference(expectedSet)) { - t.Errorf("extra: %s", extra) + t.Errorf("extra: %s", extra) } }) } @@ -1732,86 +1655,107 @@ func TestTemplateOnlyWarnings(t *testing.T) { func TestCheckForOverLap(t *testing.T) { testCase := map[string]struct { - checkPaths []string - path string - expected string + checkPaths []pathAndSource + path pathAndSource + found bool + expected []pathAndSource }{ "exact match": { - checkPaths: []string{"path/path1"}, - path: "path/path1", - expected: "path/path1", + checkPaths: []pathAndSource{{"path/path1", "src1"}}, + path: pathAndSource{"path/path1", "src2"}, + found: true, + expected: []pathAndSource{{"path/path1", "src1"}}, }, "no match": { - checkPaths: []string{"path/path1"}, - path: "path2/path1", - expected: "", + checkPaths: []pathAndSource{{"path/path1", "src1"}}, + path: pathAndSource{"path2/path1", "src2"}, + found: false, }, "empty checkPaths": { - checkPaths: []string{}, - path: "path2/path1", - expected: "", + checkPaths: []pathAndSource{}, + path: pathAndSource{"path2/path1", "src2"}, + found: false, }, "empty string in checkPaths": { - checkPaths: []string{""}, - path: "path2/path1", - expected: "", + checkPaths: []pathAndSource{{"", "src1"}}, + path: pathAndSource{"path2/path1", "src2"}, + found: false, }, "empty path": { - checkPaths: []string{"test"}, - path: "", - expected: "", + checkPaths: []pathAndSource{{"test", "src1"}}, + path: pathAndSource{"", ""}, + found: false, }, "empty strings in checkPaths and path": { - checkPaths: []string{""}, - path: "", - expected: "", + checkPaths: []pathAndSource{{"", "src1"}}, + path: pathAndSource{"", ""}, + expected: []pathAndSource{{"", ""}}, + found: false, }, "between file and dir": { - checkPaths: []string{"path/path1"}, - path: "path", - expected: "path/path1", + checkPaths: []pathAndSource{{"path/path1", "src1"}}, + path: pathAndSource{"path", "src2"}, + found: true, + expected: []pathAndSource{{"path/path1", "src1"}}, }, "between dir and file": { - checkPaths: []string{"path"}, - path: "path/path1", - expected: "path", + checkPaths: []pathAndSource{{"path", "src1"}}, + path: pathAndSource{"path/path1", "src2"}, + found: true, + expected: []pathAndSource{{"path", "src1"}}, }, "multiple paths without overlap": { - checkPaths: []string{"path1/path", "path2/path", "path3/path"}, - path: "path4/path", - expected: "", + checkPaths: []pathAndSource{{"path1/path", "src1"}, {"path2/path", "src2"}, {"path3/path", "src3"}}, + path: pathAndSource{"path4/path", "src4"}, + found: false, }, - "multiple paths with overlap": { - checkPaths: []string{"path1/path", "path2/path", "path3/path"}, - path: "path3/path3", - expected: "path3/path", + "multiple paths with 1 overlap": { + checkPaths: []pathAndSource{{"path1/path", "src1"}, {"path2/path", "src2"}, {"path3/path", "src3"}}, + path: pathAndSource{"path3/path", "src4"}, + found: true, + expected: []pathAndSource{{"path3/path", "src3"}}, + }, + "multiple paths with multiple overlap": { + checkPaths: []pathAndSource{{"path/path1", "src1"}, {"path/path2", "src2"}, {"path/path3", "src3"}}, + path: pathAndSource{"path", "src4"}, + found: true, + expected: []pathAndSource{{"path/path1", "src1"}, {"path/path2", "src2"}, {"path/path3", "src3"}}, }, "partial overlap": { - checkPaths: []string{"path1/path", "path2/path", "path3/path"}, - path: "path101/path3", - expected: "", + checkPaths: []pathAndSource{{"path1/path", "src1"}, {"path2/path", "src2"}, {"path3/path", "src3"}}, + path: pathAndSource{"path101/path3", "src4"}, + found: false, }, "partial overlap in path": { - checkPaths: []string{"path101/path", "path2/path", "path3/path"}, - path: "path1/path3", - expected: "", + checkPaths: []pathAndSource{{"dir/path1", "src1"}, {"dir/path2", "src2"}, {"dir/path3", "src3"}}, + path: pathAndSource{"dir/path345", "src4"}, + found: false, }, "trailing slash in path": { - checkPaths: []string{"path1/path3"}, - path: "path1/path3/", - expected: "path1/path3", + checkPaths: []pathAndSource{{"path1/path3", "src1"}}, + path: pathAndSource{"path1/path3/", "src2"}, + found: true, + expected: []pathAndSource{{"path1/path3", "src1"}}, }, "trailing slash in checkPaths": { - checkPaths: []string{"path1/path3/"}, - path: "path1/path3", - expected: "path1/path3/", + checkPaths: []pathAndSource{{"path1/path3/", "src1"}}, + path: pathAndSource{"path1/path3", "src2"}, + found: true, + expected: []pathAndSource{{"path1/path3/", "src1"}}, }, } for name, tc := range testCase { t.Run(name, func(t *testing.T) { - result := checkForOverlap(sets.New(tc.checkPaths...), tc.path) - if result != tc.expected { + result := checkForOverlap(tc.checkPaths, tc.path) + found := len(result) > 0 + if found && !tc.found { + t.Errorf("unexpected match for %q: %q", tc.path, result) + } + if !found && tc.found { + t.Errorf("expected match for %q: %q", tc.path, tc.expected) + } + if tc.found && !reflect.DeepEqual(result, tc.expected) { t.Errorf("expected %q, got %q", tc.expected, result) } })