refactor the duplicate paths warning for pods logic

This commit is contained in:
Nikola 2024-07-14 13:59:16 +03:00
parent 205a026bb1
commit 1853742da6
2 changed files with 302 additions and 154 deletions

View File

@ -172,9 +172,8 @@ func warningsForPodSpecAndMeta(fieldPath *field.Path, podSpec *api.PodSpec, meta
} }
} }
overlappingPathInContainers := warningsForOverlappingVirtualPaths(podSpec.Volumes) if overlaps := warningsForOverlappingVirtualPaths(podSpec.Volumes); len(overlaps) > 0 {
if len(overlappingPathInContainers) > 0 { warnings = append(warnings, overlaps...)
warnings = append(warnings, overlappingPathInContainers...)
} }
// duplicate hostAliases (#91670, #58477) // duplicate hostAliases (#91670, #58477)
@ -379,54 +378,50 @@ func warningsForWeightedPodAffinityTerms(terms []api.WeightedPodAffinityTerm, fi
// //
// In such cases we either get `is directory` or 'file exists' error message. // In such cases we either get `is directory` or 'file exists' error message.
func warningsForOverlappingVirtualPaths(volumes []api.Volume) []string { func warningsForOverlappingVirtualPaths(volumes []api.Volume) []string {
warnings := make([]string, 0) var warnings []string
for _, v := range volumes { for _, v := range volumes {
if v.ConfigMap != nil && v.ConfigMap.Items != nil { if v.ConfigMap != nil && v.ConfigMap.Items != nil {
w := checkVolumeMappingForOverlap(extractPaths(v.ConfigMap.Items), fmt.Sprintf("volume %q (ConfigMap %q): overlapping path", v.Name, v.ConfigMap.Name)) warnings = append(warnings, checkVolumeMappingForOverlap(extractPaths(v.ConfigMap.Items), fmt.Sprintf("volume %q (ConfigMap %q): overlapping paths", v.Name, v.ConfigMap.Name))...)
if len(w) > 0 {
warnings = append(warnings, w...)
}
} }
if v.Secret != nil && v.Secret.Items != nil { if v.Secret != nil && v.Secret.Items != nil {
w := checkVolumeMappingForOverlap(extractPaths(v.Secret.Items), fmt.Sprintf("volume %q (Secret %q): overlapping path", v.Name, v.Secret.SecretName)) warnings = append(warnings, checkVolumeMappingForOverlap(extractPaths(v.Secret.Items), fmt.Sprintf("volume %q (Secret %q): overlapping paths", v.Name, v.Secret.SecretName))...)
if len(w) > 0 {
warnings = append(warnings, w...)
}
} }
if v.DownwardAPI != nil && v.DownwardAPI.Items != nil { if v.DownwardAPI != nil && v.DownwardAPI.Items != nil {
w := checkVolumeMappingForOverlap(extractPathsDownwardAPI(v.DownwardAPI.Items), fmt.Sprintf("volume %q (DownwardAPI): overlapping path", v.Name)) warnings = append(warnings, checkVolumeMappingForOverlap(extractPathsDownwardAPI(v.DownwardAPI.Items), fmt.Sprintf("volume %q (DownwardAPI): overlapping paths", v.Name))...)
if len(w) > 0 {
warnings = append(warnings, w...)
}
} }
if v.Projected != nil { if v.Projected != nil {
var sourcePaths, allPaths []string var sourcePaths []string
var errorMessage string var errorMessage string
allPaths := sets.New[string]()
for _, source := range v.Projected.Sources { 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 { switch {
case source.ConfigMap != nil && source.ConfigMap.Items != nil: case source.ConfigMap != nil && source.ConfigMap.Items != nil:
sourcePaths = extractPaths(source.ConfigMap.Items) sourcePaths = extractPaths(source.ConfigMap.Items)
errorMessage = fmt.Sprintf("volume %q (Projected ConfigMap %q): overlapping path", v.Name, source.ConfigMap.Name) errorMessage = fmt.Sprintf("volume %q (Projected ConfigMap %q): overlapping paths", v.Name, source.ConfigMap.Name)
case source.Secret != nil && source.Secret.Items != nil: case source.Secret != nil && source.Secret.Items != nil:
sourcePaths = extractPaths(source.Secret.Items) sourcePaths = extractPaths(source.Secret.Items)
errorMessage = fmt.Sprintf("volume %q (Projected Secret %q): overlapping path", v.Name, source.Secret.Name) errorMessage = fmt.Sprintf("volume %q (Projected Secret %q): overlapping paths", v.Name, source.Secret.Name)
case source.DownwardAPI != nil && source.DownwardAPI.Items != nil: case source.DownwardAPI != nil && source.DownwardAPI.Items != nil:
sourcePaths = extractPathsDownwardAPI(source.DownwardAPI.Items) sourcePaths = extractPathsDownwardAPI(source.DownwardAPI.Items)
errorMessage = fmt.Sprintf("volume %q (Projected DownwardAPI): overlapping path", v.Name) errorMessage = fmt.Sprintf("volume %q (Projected DownwardAPI): overlapping paths", v.Name)
case source.ServiceAccountToken != nil: case source.ServiceAccountToken != nil:
sourcePaths = []string{source.ServiceAccountToken.Path} sourcePaths = []string{source.ServiceAccountToken.Path}
errorMessage = fmt.Sprintf("volume %q (Projected ServiceAccountToken): overlapping path", v.Name) errorMessage = fmt.Sprintf("volume %q (Projected ServiceAccountToken): overlapping paths", v.Name)
case source.ClusterTrustBundle != nil: case source.ClusterTrustBundle != nil:
sourcePaths = []string{source.ClusterTrustBundle.Path} sourcePaths = []string{source.ClusterTrustBundle.Path}
if source.ClusterTrustBundle.Name != nil { if source.ClusterTrustBundle.Name != nil {
errorMessage = fmt.Sprintf("volume %q (Projected ClusterTrustBundle %q): overlapping path", v.Name, *source.ClusterTrustBundle.Name) errorMessage = fmt.Sprintf("volume %q (Projected ClusterTrustBundle %q): overlapping paths", v.Name, *source.ClusterTrustBundle.Name)
} else { } else {
errorMessage = fmt.Sprintf("volume %q (Projected ClusterTrustBundle %q): overlapping path", v.Name, *source.ClusterTrustBundle.SignerName) errorMessage = fmt.Sprintf("volume %q (Projected ClusterTrustBundle %q): overlapping paths", v.Name, *source.ClusterTrustBundle.SignerName)
} }
} }
@ -434,16 +429,14 @@ func warningsForOverlappingVirtualPaths(volumes []api.Volume) []string {
continue continue
} }
warningsInSource := checkVolumeMappingForOverlap(sourcePaths, errorMessage) warnings = append(warnings, checkVolumeMappingForOverlap(sourcePaths, errorMessage)...)
if len(warningsInSource) > 0 {
warnings = append(warnings, warningsInSource...)
}
allPaths = append(allPaths, sourcePaths...) // remove duplicates path and sort the new array, so we can get predetermined result
warningsInAllPaths := checkVolumeMappingForOverlap(allPaths, errorMessage) uniqueSourcePaths := sets.New[string](sourcePaths...).UnsortedList()
if len(warningsInAllPaths) > 0 { orderedSourcePaths := append(uniqueSourcePaths, allPaths.UnsortedList()...)
warnings = append(warnings, warningsInAllPaths...) sort.Strings(orderedSourcePaths)
} warnings = append(warnings, checkVolumeMappingForOverlap(orderedSourcePaths, errorMessage)...)
allPaths.Insert(uniqueSourcePaths...)
} }
} }
@ -470,14 +463,14 @@ func extractPathsDownwardAPI(mapping []api.DownwardAPIVolumeFile) []string {
} }
func checkVolumeMappingForOverlap(paths []string, warningMessage string) []string { func checkVolumeMappingForOverlap(paths []string, warningMessage string) []string {
var warnings []string
pathSeparator := string(os.PathSeparator) pathSeparator := string(os.PathSeparator)
warnings := make([]string, 0)
uniquePaths := sets.New[string]() uniquePaths := sets.New[string]()
for _, path := range paths { for _, path := range paths {
normalizedPath := strings.TrimRight(path, pathSeparator) normalizedPath := strings.TrimRight(path, pathSeparator)
if collision := checkForOverlap(uniquePaths, normalizedPath); collision != nil { if collision := checkForOverlap(uniquePaths, normalizedPath); collision != "" {
warnings = append(warnings, fmt.Sprintf("%s %q with %q", warningMessage, normalizedPath, *collision)) warnings = append(warnings, fmt.Sprintf("%s: %q with %q", warningMessage, normalizedPath, collision))
} }
uniquePaths.Insert(normalizedPath) uniquePaths.Insert(normalizedPath)
} }
@ -485,21 +478,21 @@ func checkVolumeMappingForOverlap(paths []string, warningMessage string) []strin
return warnings return warnings
} }
func checkForOverlap(paths sets.Set[string], path string) *string { func checkForOverlap(paths sets.Set[string], path string) string {
pathSeparator := string(os.PathSeparator) pathSeparator := string(os.PathSeparator)
p := paths.UnsortedList()
sort.Strings(p)
for _, item := range p { for item := range paths {
switch { switch {
case item == "" || path == "":
return ""
case item == path: case item == path:
return &item return item
case strings.HasPrefix(item+pathSeparator, path): case strings.HasPrefix(item+pathSeparator, path):
return &item return item
case strings.HasPrefix(path+pathSeparator, item): case strings.HasPrefix(path+pathSeparator, item):
return &item return item
} }
} }
return nil return ""
} }

View File

@ -18,6 +18,7 @@ package pod
import ( import (
"context" "context"
"strings"
"testing" "testing"
"k8s.io/apimachinery/pkg/api/resource" "k8s.io/apimachinery/pkg/api/resource"
@ -255,7 +256,7 @@ func TestWarnings(t *testing.T) {
}, },
}}, }},
expected: []string{ expected: []string{
"volume \"Test\" (ConfigMap \"foo\"): overlapping path \"test\" with \"test\"", `volume "Test" (ConfigMap "foo"): overlapping paths: "test" with "test"`,
}, },
}, },
{ {
@ -277,7 +278,7 @@ func TestWarnings(t *testing.T) {
}, },
}}, }},
expected: []string{ expected: []string{
"volume \"Test\" (ConfigMap \"foo\"): overlapping path \"test/app\" with \"test\"", `volume "Test" (ConfigMap "foo"): overlapping paths: "test/app" with "test"`,
}, },
}, },
{ {
@ -299,7 +300,7 @@ func TestWarnings(t *testing.T) {
}, },
}}, }},
expected: []string{ expected: []string{
"volume \"Test\" (ConfigMap \"foo\"): overlapping path \"test\" with \"test/app\"", `volume "Test" (ConfigMap "foo"): overlapping paths: "test" with "test/app"`,
}, },
}, },
{ {
@ -321,7 +322,7 @@ func TestWarnings(t *testing.T) {
}, },
}}, }},
expected: []string{ expected: []string{
"volume \"Test\" (Secret \"foo\"): overlapping path \"test\" with \"test\"", `volume "Test" (Secret "foo"): overlapping paths: "test" with "test"`,
}, },
}, },
{ {
@ -342,7 +343,7 @@ func TestWarnings(t *testing.T) {
}, },
}}, }},
expected: []string{ expected: []string{
"volume \"Test\" (DownwardAPI): overlapping path \"test\" with \"test\"", `volume "Test" (DownwardAPI): overlapping paths: "test" with "test"`,
}, },
}, },
{ {
@ -354,15 +355,19 @@ func TestWarnings(t *testing.T) {
VolumeSource: api.VolumeSource{ VolumeSource: api.VolumeSource{
Projected: &api.ProjectedVolumeSource{ Projected: &api.ProjectedVolumeSource{
Sources: []api.VolumeProjection{ Sources: []api.VolumeProjection{
{ConfigMap: &api.ConfigMapProjection{ {
LocalObjectReference: api.LocalObjectReference{Name: "Test"}, ConfigMap: &api.ConfigMapProjection{
Items: []api.KeyToPath{ LocalObjectReference: api.LocalObjectReference{Name: "Test"},
{Key: "foo", Path: "test"}, Items: []api.KeyToPath{
{Key: "foo", Path: "test"},
},
}, },
}}, },
{ServiceAccountToken: &api.ServiceAccountTokenProjection{ {
Path: "test", ServiceAccountToken: &api.ServiceAccountTokenProjection{
}}, Path: "test",
},
},
}, },
}, },
}, },
@ -370,11 +375,11 @@ func TestWarnings(t *testing.T) {
}, },
}}, }},
expected: []string{ expected: []string{
"volume \"foo\" (Projected ServiceAccountToken): overlapping path \"test\" with \"test\"", `volume "foo" (Projected ServiceAccountToken): overlapping paths: "test" with "test"`,
}, },
}, },
{ {
name: "overlapping paths in projected volume volume - service account dir and config file", name: "overlapping paths in projected volume volume: service account dir and config file",
template: &api.PodTemplateSpec{Spec: api.PodSpec{ template: &api.PodTemplateSpec{Spec: api.PodSpec{
Volumes: []api.Volume{ Volumes: []api.Volume{
{ {
@ -382,15 +387,19 @@ func TestWarnings(t *testing.T) {
VolumeSource: api.VolumeSource{ VolumeSource: api.VolumeSource{
Projected: &api.ProjectedVolumeSource{ Projected: &api.ProjectedVolumeSource{
Sources: []api.VolumeProjection{ Sources: []api.VolumeProjection{
{ConfigMap: &api.ConfigMapProjection{ {
LocalObjectReference: api.LocalObjectReference{Name: "Test"}, ConfigMap: &api.ConfigMapProjection{
Items: []api.KeyToPath{ LocalObjectReference: api.LocalObjectReference{Name: "Test"},
{Key: "foo", Path: "test"}, Items: []api.KeyToPath{
{Key: "foo", Path: "test"},
},
}, },
}}, },
{ServiceAccountToken: &api.ServiceAccountTokenProjection{ {
Path: "test/file", ServiceAccountToken: &api.ServiceAccountTokenProjection{
}}, Path: "test/file",
},
},
}, },
}, },
}, },
@ -398,7 +407,7 @@ func TestWarnings(t *testing.T) {
}, },
}}, }},
expected: []string{ expected: []string{
"volume \"foo\" (Projected ServiceAccountToken): overlapping path \"test/file\" with \"test\"", `volume "foo" (Projected ServiceAccountToken): overlapping paths: "test/file" with "test"`,
}, },
}, },
{ {
@ -410,15 +419,19 @@ func TestWarnings(t *testing.T) {
VolumeSource: api.VolumeSource{ VolumeSource: api.VolumeSource{
Projected: &api.ProjectedVolumeSource{ Projected: &api.ProjectedVolumeSource{
Sources: []api.VolumeProjection{ Sources: []api.VolumeProjection{
{ConfigMap: &api.ConfigMapProjection{ {
LocalObjectReference: api.LocalObjectReference{Name: "Test"}, ConfigMap: &api.ConfigMapProjection{
Items: []api.KeyToPath{ LocalObjectReference: api.LocalObjectReference{Name: "Test"},
{Key: "foo", Path: "test/file"}, Items: []api.KeyToPath{
{Key: "foo", Path: "test/file"},
},
}, },
}}, },
{ServiceAccountToken: &api.ServiceAccountTokenProjection{ {
Path: "test", ServiceAccountToken: &api.ServiceAccountTokenProjection{
}}, Path: "test",
},
},
}, },
}, },
}, },
@ -426,7 +439,7 @@ func TestWarnings(t *testing.T) {
}, },
}}, }},
expected: []string{ expected: []string{
"volume \"foo\" (Projected ServiceAccountToken): overlapping path \"test\" with \"test/file\"", `volume "foo" (Projected ServiceAccountToken): overlapping paths: "test/file" with "test"`,
}, },
}, },
{ {
@ -438,16 +451,19 @@ func TestWarnings(t *testing.T) {
VolumeSource: api.VolumeSource{ VolumeSource: api.VolumeSource{
Projected: &api.ProjectedVolumeSource{ Projected: &api.ProjectedVolumeSource{
Sources: []api.VolumeProjection{ Sources: []api.VolumeProjection{
{Secret: &api.SecretProjection{ {
LocalObjectReference: api.LocalObjectReference{Name: "Test"}, Secret: &api.SecretProjection{
Items: []api.KeyToPath{ LocalObjectReference: api.LocalObjectReference{Name: "Test"},
{Key: "foo", Path: "test"}, Items: []api.KeyToPath{
{Key: "foo", Path: "test"},
},
}, },
}, },
{
ServiceAccountToken: &api.ServiceAccountTokenProjection{
Path: "test",
},
}, },
{ServiceAccountToken: &api.ServiceAccountTokenProjection{
Path: "test",
}},
}, },
}, },
}, },
@ -455,7 +471,7 @@ func TestWarnings(t *testing.T) {
}, },
}}, }},
expected: []string{ expected: []string{
"volume \"foo\" (Projected ServiceAccountToken): overlapping path \"test\" with \"test\"", `volume "foo" (Projected ServiceAccountToken): overlapping paths: "test" with "test"`,
}, },
}, },
{ {
@ -467,14 +483,18 @@ func TestWarnings(t *testing.T) {
VolumeSource: api.VolumeSource{ VolumeSource: api.VolumeSource{
Projected: &api.ProjectedVolumeSource{ Projected: &api.ProjectedVolumeSource{
Sources: []api.VolumeProjection{ Sources: []api.VolumeProjection{
{DownwardAPI: &api.DownwardAPIProjection{ {
Items: []api.DownwardAPIVolumeFile{ DownwardAPI: &api.DownwardAPIProjection{
{FieldRef: &api.ObjectFieldSelector{APIVersion: "v1", FieldPath: "metadata.name"}, Path: "test"}, Items: []api.DownwardAPIVolumeFile{
{FieldRef: &api.ObjectFieldSelector{APIVersion: "v1", FieldPath: "metadata.name"}, Path: "test"},
},
}, },
}}, },
{ServiceAccountToken: &api.ServiceAccountTokenProjection{ {
Path: "test", ServiceAccountToken: &api.ServiceAccountTokenProjection{
}}, Path: "test",
},
},
}, },
}, },
}, },
@ -482,7 +502,7 @@ func TestWarnings(t *testing.T) {
}, },
}}, }},
expected: []string{ expected: []string{
"volume \"foo\" (Projected ServiceAccountToken): overlapping path \"test\" with \"test\"", `volume "foo" (Projected ServiceAccountToken): overlapping paths: "test" with "test"`,
}, },
}, },
{ {
@ -494,12 +514,16 @@ func TestWarnings(t *testing.T) {
VolumeSource: api.VolumeSource{ VolumeSource: api.VolumeSource{
Projected: &api.ProjectedVolumeSource{ Projected: &api.ProjectedVolumeSource{
Sources: []api.VolumeProjection{ Sources: []api.VolumeProjection{
{ClusterTrustBundle: &api.ClusterTrustBundleProjection{ {
Name: &testName, Path: "test", ClusterTrustBundle: &api.ClusterTrustBundleProjection{
}}, Name: &testName, Path: "test",
{ServiceAccountToken: &api.ServiceAccountTokenProjection{ },
Path: "test", },
}}, {
ServiceAccountToken: &api.ServiceAccountTokenProjection{
Path: "test",
},
},
}, },
}, },
}, },
@ -507,7 +531,7 @@ func TestWarnings(t *testing.T) {
}, },
}}, }},
expected: []string{ expected: []string{
"volume \"foo\" (Projected ServiceAccountToken): overlapping path \"test\" with \"test\"", `volume "foo" (Projected ServiceAccountToken): overlapping paths: "test" with "test"`,
}, },
}, },
{ {
@ -519,12 +543,16 @@ func TestWarnings(t *testing.T) {
VolumeSource: api.VolumeSource{ VolumeSource: api.VolumeSource{
Projected: &api.ProjectedVolumeSource{ Projected: &api.ProjectedVolumeSource{
Sources: []api.VolumeProjection{ Sources: []api.VolumeProjection{
{ClusterTrustBundle: &api.ClusterTrustBundleProjection{ {
SignerName: &testName, Path: "test", ClusterTrustBundle: &api.ClusterTrustBundleProjection{
}}, SignerName: &testName, Path: "test",
{ServiceAccountToken: &api.ServiceAccountTokenProjection{ },
Path: "test", },
}}, {
ServiceAccountToken: &api.ServiceAccountTokenProjection{
Path: "test",
},
},
}, },
}, },
}, },
@ -532,7 +560,7 @@ func TestWarnings(t *testing.T) {
}, },
}}, }},
expected: []string{ expected: []string{
"volume \"foo\" (Projected ServiceAccountToken): overlapping path \"test\" with \"test\"", `volume "foo" (Projected ServiceAccountToken): overlapping paths: "test" with "test"`,
}, },
}, },
{ {
@ -544,15 +572,17 @@ func TestWarnings(t *testing.T) {
VolumeSource: api.VolumeSource{ VolumeSource: api.VolumeSource{
Projected: &api.ProjectedVolumeSource{ Projected: &api.ProjectedVolumeSource{
Sources: []api.VolumeProjection{ Sources: []api.VolumeProjection{
{Secret: &api.SecretProjection{ {
LocalObjectReference: api.LocalObjectReference{Name: "TestSecret"}, Secret: &api.SecretProjection{
Items: []api.KeyToPath{ LocalObjectReference: api.LocalObjectReference{Name: "TestSecret"},
{ Items: []api.KeyToPath{
Key: "mykey", {
Path: "test", Key: "mykey",
Path: "test",
},
}, },
}, },
}}, },
{ {
ConfigMap: &api.ConfigMapProjection{ ConfigMap: &api.ConfigMapProjection{
LocalObjectReference: api.LocalObjectReference{Name: "TestConfigMap"}, LocalObjectReference: api.LocalObjectReference{Name: "TestConfigMap"},
@ -571,7 +601,7 @@ func TestWarnings(t *testing.T) {
}, },
}}, }},
expected: []string{ expected: []string{
"volume \"foo\" (Projected ConfigMap \"TestConfigMap\"): overlapping path \"test/test1\" with \"test\"", `volume "foo" (Projected ConfigMap "TestConfigMap"): overlapping paths: "test/test1" with "test"`,
}, },
}, },
{ {
@ -583,15 +613,17 @@ func TestWarnings(t *testing.T) {
VolumeSource: api.VolumeSource{ VolumeSource: api.VolumeSource{
Projected: &api.ProjectedVolumeSource{ Projected: &api.ProjectedVolumeSource{
Sources: []api.VolumeProjection{ Sources: []api.VolumeProjection{
{Secret: &api.SecretProjection{ {
LocalObjectReference: api.LocalObjectReference{Name: "TestSecret"}, Secret: &api.SecretProjection{
Items: []api.KeyToPath{ LocalObjectReference: api.LocalObjectReference{Name: "TestSecret"},
{ Items: []api.KeyToPath{
Key: "mykey", {
Path: "test", Key: "mykey",
Path: "test",
},
}, },
}, },
}}, },
{ {
DownwardAPI: &api.DownwardAPIProjection{ DownwardAPI: &api.DownwardAPIProjection{
Items: []api.DownwardAPIVolumeFile{ Items: []api.DownwardAPIVolumeFile{
@ -606,11 +638,11 @@ func TestWarnings(t *testing.T) {
}, },
}}, }},
expected: []string{ expected: []string{
"volume \"foo\" (Projected DownwardAPI): overlapping path \"test/test2\" with \"test\"", `volume "foo" (Projected DownwardAPI): overlapping paths: "test/test2" with "test"`,
}, },
}, },
{ {
name: "overlapping paths in projected volume - downward api and downward api", name: "overlapping paths in projected volume - downward api and cluster thrust bundle api",
template: &api.PodTemplateSpec{Spec: api.PodSpec{ template: &api.PodTemplateSpec{Spec: api.PodSpec{
Volumes: []api.Volume{ Volumes: []api.Volume{
{ {
@ -637,7 +669,7 @@ func TestWarnings(t *testing.T) {
}, },
}}, }},
expected: []string{ expected: []string{
"volume \"foo\" (Projected ClusterTrustBundle \"Test\"): overlapping path \"test\" with \"test/test2\"", `volume "foo" (Projected ClusterTrustBundle "Test"): overlapping paths: "test/test2" with "test"`,
}, },
}, },
{ {
@ -649,24 +681,31 @@ func TestWarnings(t *testing.T) {
VolumeSource: api.VolumeSource{ VolumeSource: api.VolumeSource{
Projected: &api.ProjectedVolumeSource{ Projected: &api.ProjectedVolumeSource{
Sources: []api.VolumeProjection{ Sources: []api.VolumeProjection{
{ClusterTrustBundle: &api.ClusterTrustBundleProjection{ {
SignerName: &testName, Path: "test/test", 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"},
}, },
}, },
{
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",
},
}, },
{ServiceAccountToken: &api.ServiceAccountTokenProjection{
Path: "test",
}},
}, },
}, },
}, },
@ -674,11 +713,64 @@ func TestWarnings(t *testing.T) {
}, },
}}, }},
expected: []string{ expected: []string{
"volume \"foo\" (Projected DownwardAPI): overlapping path \"test\" with \"test/test\"", `volume "foo" (Projected DownwardAPI): overlapping paths: "test/test" with "test"`,
"volume \"foo\" (Projected Secret \"Test\"): overlapping path \"test\" with \"test\"", `volume "foo" (Projected Secret "Test"): overlapping paths: "test" with "test"`,
"volume \"foo\" (Projected Secret \"Test\"): overlapping path \"test\" with \"test/test\"", `volume "foo" (Projected Secret "Test"): overlapping paths: "test/test" with "test"`,
"volume \"foo\" (Projected ServiceAccountToken): overlapping path \"test\" with \"test/test\"", `volume "foo" (Projected ServiceAccountToken): overlapping paths: "test/test" with "test"`,
"volume \"foo\" (Projected ServiceAccountToken): overlapping path \"test\" with \"test\"", `volume "foo" (Projected ServiceAccountToken): overlapping paths: "test" with "test"`,
},
},
{
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{
{
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"`,
},
},
{
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{
{},
},
},
},
},
},
}},
expected: []string{
`volume "foo" (Projected) has no sources provided`,
}, },
}, },
{ {
@ -1513,12 +1605,16 @@ func TestWarnings(t *testing.T) {
if tc.oldTemplate != nil { if tc.oldTemplate != nil {
oldTemplate = tc.oldTemplate oldTemplate = tc.oldTemplate
} }
actual := sets.New[string](GetWarningsForPodTemplate(context.TODO(), nil, tc.template, oldTemplate)...) actual := GetWarningsForPodTemplate(context.TODO(), nil, tc.template, oldTemplate)
expected := sets.New[string](tc.expected...) if len(actual) != len(tc.expected) {
for _, missing := range sets.List[string](expected.Difference(actual)) { t.Errorf("expected %d errors, got %d:\n%v", len(tc.expected), len(actual), strings.Join(actual, "\n"))
}
actualSet := sets.New(actual...)
expectedSet := sets.New(tc.expected...)
for _, missing := range sets.List(expectedSet.Difference(actualSet)) {
t.Errorf("missing: %s", missing) t.Errorf("missing: %s", missing)
} }
for _, extra := range sets.List[string](actual.Difference(expected)) { for _, extra := range sets.List(actualSet.Difference(expectedSet)) {
t.Errorf("extra: %s", extra) t.Errorf("extra: %s", extra)
} }
}) })
@ -1531,12 +1627,16 @@ func TestWarnings(t *testing.T) {
Spec: tc.template.Spec, Spec: tc.template.Spec,
} }
} }
actual := sets.New[string](GetWarningsForPod(context.TODO(), pod, &api.Pod{})...) actual := GetWarningsForPod(context.TODO(), pod, &api.Pod{})
expected := sets.New[string](tc.expected...) if len(actual) != len(tc.expected) {
for _, missing := range sets.List[string](expected.Difference(actual)) { t.Errorf("expected %d errors, got %d:\n%v", len(tc.expected), len(actual), strings.Join(actual, "\n"))
}
actualSet := sets.New(actual...)
expectedSet := sets.New(tc.expected...)
for _, missing := range sets.List(expectedSet.Difference(actualSet)) {
t.Errorf("missing: %s", missing) t.Errorf("missing: %s", missing)
} }
for _, extra := range sets.List[string](actual.Difference(expected)) { for _, extra := range sets.List(actualSet.Difference(expectedSet)) {
t.Errorf("extra: %s", extra) t.Errorf("extra: %s", extra)
} }
}) })
@ -1641,6 +1741,31 @@ func TestCheckForOverLap(t *testing.T) {
path: "path/path1", path: "path/path1",
expected: "path/path1", expected: "path/path1",
}, },
"no match": {
checkPaths: []string{"path/path1"},
path: "path2/path1",
expected: "",
},
"empty checkPaths": {
checkPaths: []string{},
path: "path2/path1",
expected: "",
},
"empty string in checkPaths": {
checkPaths: []string{""},
path: "path2/path1",
expected: "",
},
"empty path": {
checkPaths: []string{"test"},
path: "",
expected: "",
},
"empty strings in checkPaths and path": {
checkPaths: []string{""},
path: "",
expected: "",
},
"between file and dir": { "between file and dir": {
checkPaths: []string{"path/path1"}, checkPaths: []string{"path/path1"},
path: "path", path: "path",
@ -1651,13 +1776,43 @@ func TestCheckForOverLap(t *testing.T) {
path: "path/path1", path: "path/path1",
expected: "path", expected: "path",
}, },
"multiple paths without overlap": {
checkPaths: []string{"path1/path", "path2/path", "path3/path"},
path: "path4/path",
expected: "",
},
"multiple paths with overlap": {
checkPaths: []string{"path1/path", "path2/path", "path3/path"},
path: "path3/path3",
expected: "path3/path",
},
"partial overlap": {
checkPaths: []string{"path1/path", "path2/path", "path3/path"},
path: "path101/path3",
expected: "",
},
"partial overlap in path": {
checkPaths: []string{"path101/path", "path2/path", "path3/path"},
path: "path1/path3",
expected: "",
},
"trailing slash in path": {
checkPaths: []string{"path1/path3"},
path: "path1/path3/",
expected: "path1/path3",
},
"trailing slash in checkPaths": {
checkPaths: []string{"path1/path3/"},
path: "path1/path3",
expected: "path1/path3/",
},
} }
for name, tc := range testCase { for name, tc := range testCase {
t.Run(name, func(t *testing.T) { t.Run(name, func(t *testing.T) {
result := checkForOverlap(sets.New(tc.checkPaths...), tc.path) result := checkForOverlap(sets.New(tc.checkPaths...), tc.path)
if *result != tc.expected { if result != tc.expected {
t.Errorf("expected %v, got %v", tc.expected, *result) t.Errorf("expected %q, got %q", tc.expected, result)
} }
}) })
} }