From 30c325a3dc87a43746e70dc84d319bc23dde7c3d Mon Sep 17 00:00:00 2001 From: Nikola Date: Sun, 12 May 2024 16:21:39 +0300 Subject: [PATCH] add validation for configmap, secret and downward api --- pkg/api/pod/warnings.go | 134 +++++++++++++++++++------------ pkg/api/pod/warnings_test.go | 148 ++++++++++++++++++++++++++++------- 2 files changed, 202 insertions(+), 80 deletions(-) diff --git a/pkg/api/pod/warnings.go b/pkg/api/pod/warnings.go index b72e515cd4d..ea05b82ba31 100644 --- a/pkg/api/pod/warnings.go +++ b/pkg/api/pod/warnings.go @@ -170,58 +170,14 @@ func warningsForPodSpecAndMeta(fieldPath *field.Path, podSpec *api.PodSpec, meta } } - items := sets.NewString() - for _, v := range podSpec.Volumes { - if v.Projected != nil { - for _, k := range v.Projected.Sources { - if k.ServiceAccountToken != nil { - items.Insert(k.ServiceAccountToken.Path) - } - } - } - + overlappingPathInContainers := warningsForOverlappingVirtualPaths(podSpec) + if len(overlappingPathInContainers) > 0 { + warnings = append(warnings, overlappingPathInContainers...) } - if items.Len() > 0 { - for i, item := range podSpec.Volumes { - if item.Projected != nil { - for _, k := range item.Projected.Sources { - if k.ConfigMap != nil { - if len(k.ConfigMap.Items) > 0 { - for j, item := range k.ConfigMap.Items { - if items.Has(item.Path) { - warnings = append(warnings, fmt.Sprintf("%s: has duplicated path with ServiceAccountToken %q", fieldPath.Child("spec", "volumes").Index(i).Child("projected", "sources", "configMap").Index(j), item.Path)) - } - } - } - } - - if k.Secret != nil { - if len(k.Secret.Items) > 0 { - for j, item := range k.Secret.Items { - if items.Has(item.Path) { - warnings = append(warnings, fmt.Sprintf("%s: has duplicated path with ServiceAccountToken %q", fieldPath.Child("spec", "volumes").Index(i).Child("projected", "sources", "secrets").Index(j), item.Path)) - } - } - } - } - - if k.DownwardAPI != nil { - for j, item := range k.DownwardAPI.Items { - if items.Has(item.Path) { - warnings = append(warnings, fmt.Sprintf("%s: has duplicated path with ServiceAccountToken %q", fieldPath.Child("spec", "volumes").Index(i).Child("projected", "sources", "downwardAPI").Index(j), item.Path)) - } - } - } - - if k.ClusterTrustBundle != nil { - if items.Has(k.ClusterTrustBundle.Path) { - warnings = append(warnings, fmt.Sprintf("%s: has duplicated path with ServiceAccountToken %q", fieldPath.Child("spec", "volumes").Index(i).Child("projected", "sources", "ClusterTrustBundle"), k.ClusterTrustBundle.Path)) - } - } - } - } - } + overlappingPathInInitContainers := warningsForOverlappingVirtualPaths(podSpec) + if len(overlappingPathInInitContainers) > 0 { + warnings = append(warnings, overlappingPathInInitContainers...) } // duplicate hostAliases (#91670, #58477) @@ -411,3 +367,81 @@ func warningsForWeightedPodAffinityTerms(terms []api.WeightedPodAffinityTerm, fi } return warnings } + +func warningsForOverlappingVirtualPaths(podSpec *api.PodSpec) []string { + warnings := make([]string, 0) + for _, v := range podSpec.Volumes { + if v.ConfigMap != nil && v.ConfigMap.Items != nil { + a := sets.NewString() + for _, item := range v.ConfigMap.Items { + if a.Has(item.Path) { + warnings = append(warnings, fmt.Sprintf("config map: %q - conflicting duplicate paths", v.ConfigMap.Name)) + } + a.Insert(item.Path) + } + } + + if v.Secret != nil && v.Secret.Items != nil { + a := sets.NewString() + for _, item := range v.Secret.Items { + if a.Has(item.Path) { + warnings = append(warnings, fmt.Sprintf("secret: %q - conflicting duplicate paths", v.Secret.SecretName)) + } + a.Insert(item.Path) + } + } + + if v.DownwardAPI != nil && v.DownwardAPI.Items != nil { + a := sets.NewString() + for _, item := range v.DownwardAPI.Items { + if a.Has(item.Path) { + warnings = append(warnings, "downward api - conflicting duplicate paths") + } + a.Insert(item.Path) + } + } + + if v.Projected != nil { + a := sets.NewString() + for _, item := range v.Projected.Sources { + if item.ServiceAccountToken != nil { + a.Insert(item.ServiceAccountToken.Path) + } + } + + for _, item := range v.Projected.Sources { + if item.ConfigMap != nil && item.ConfigMap.Items != nil { + for _, item := range item.ConfigMap.Items { + if a.Has(item.Path) { + warnings = append(warnings, fmt.Sprintf("projected volume: %q - conflicting duplicate paths", v.Name)) + } + } + } + + if item.Secret != nil && item.Secret.Items != nil { + for _, item := range item.Secret.Items { + if a.Has(item.Path) { + warnings = append(warnings, fmt.Sprintf("projected volume: %q - conflicting duplicate paths", v.Name)) + } + } + } + + if item.DownwardAPI != nil && item.DownwardAPI.Items != nil { + for _, item := range item.DownwardAPI.Items { + if a.Has(item.Path) { + warnings = append(warnings, fmt.Sprintf("projected volume: %q - conflicting duplicate paths", v.Name)) + } + } + } + + if item.ClusterTrustBundle != nil { + if a.Has(item.ClusterTrustBundle.Path) { + warnings = append(warnings, fmt.Sprintf("projected volume: %q - conflicting duplicate paths", v.Name)) + } + } + } + } + + } + return warnings +} diff --git a/pkg/api/pod/warnings_test.go b/pkg/api/pod/warnings_test.go index bea4dd18114..9fd678c4f14 100644 --- a/pkg/api/pod/warnings_test.go +++ b/pkg/api/pod/warnings_test.go @@ -236,83 +236,171 @@ func TestWarnings(t *testing.T) { expected: []string{`spec.volumes[0].rbd: deprecated in v1.28, non-functional in v1.31+`}, }, { - name: "duplicated path in projected volumes - secret and service account token", + name: "overlapping paths in a configmap volume", template: &api.PodTemplateSpec{Spec: api.PodSpec{ Volumes: []api.Volume{ { - Name: "t", VolumeSource: api.VolumeSource{ - Projected: &api.ProjectedVolumeSource{ - Sources: []api.VolumeProjection{ - {Secret: &api.SecretProjection{Items: []api.KeyToPath{{Path: "/test-path", Key: "test"}}}}, - {ServiceAccountToken: &api.ServiceAccountTokenProjection{Path: "/test-path"}}, - }}, + ConfigMap: &api.ConfigMapVolumeSource{ + LocalObjectReference: api.LocalObjectReference{Name: "foo"}, + Items: []api.KeyToPath{ + {Key: "foo", Path: "test"}, + {Key: "bar", Path: "test"}, + }, + }, }, }, }, }}, expected: []string{ - "spec.volumes[0].projected.sources.secrets[0]: has duplicated path with ServiceAccountToken \"/test-path\"", + "config map: \"foo\" - conflicting duplicate paths", }, }, { - name: "duplicated path in projected volumes - configMap and service account token", + name: "overlapping paths in a secret volume", template: &api.PodTemplateSpec{Spec: api.PodSpec{ Volumes: []api.Volume{ { - Name: "t", VolumeSource: api.VolumeSource{ - Projected: &api.ProjectedVolumeSource{ - Sources: []api.VolumeProjection{ - {ConfigMap: &api.ConfigMapProjection{Items: []api.KeyToPath{{Path: "/test-path", Key: "test"}}}}, - {ServiceAccountToken: &api.ServiceAccountTokenProjection{Path: "/test-path"}}, - }}, + Secret: &api.SecretVolumeSource{ + SecretName: "foo", + Items: []api.KeyToPath{ + {Key: "foo", Path: "test"}, + {Key: "bar", Path: "test"}, + }, + }, }, }, }, }}, expected: []string{ - "spec.volumes[0].projected.sources.configMap[0]: has duplicated path with ServiceAccountToken \"/test-path\"", + "secret: \"foo\" - conflicting duplicate paths", }, }, { - name: "duplicated path in projected volumes - downwardAPI and service account token", + name: "overlapping paths in a downward api volume", template: &api.PodTemplateSpec{Spec: api.PodSpec{ Volumes: []api.Volume{ { - Name: "t", VolumeSource: api.VolumeSource{ - Projected: &api.ProjectedVolumeSource{ - Sources: []api.VolumeProjection{ - {DownwardAPI: &api.DownwardAPIProjection{Items: []api.DownwardAPIVolumeFile{{Path: "/test-path"}}}}, - {ServiceAccountToken: &api.ServiceAccountTokenProjection{Path: "/test-path"}}, - }}, + DownwardAPI: &api.DownwardAPIVolumeSource{ + Items: []api.DownwardAPIVolumeFile{ + {FieldRef: &api.ObjectFieldSelector{APIVersion: "v1", FieldPath: "metadata.name\n"}, Path: "test"}, + {FieldRef: &api.ObjectFieldSelector{APIVersion: "v1", FieldPath: "metadata.labels\n"}, Path: "test"}, + }, + }, }, }, }, }}, expected: []string{ - "spec.volumes[0].projected.sources.downwardAPI[0]: has duplicated path with ServiceAccountToken \"/test-path\"", + "downward api - conflicting duplicate paths", }, }, { - name: "duplicated path in projected volumes - ClusterTrustBundle and service account token", + name: "overlapping paths in projected volume volume - service account and config", template: &api.PodTemplateSpec{Spec: api.PodSpec{ Volumes: []api.Volume{ { - Name: "t", + Name: "foo", VolumeSource: api.VolumeSource{ Projected: &api.ProjectedVolumeSource{ Sources: []api.VolumeProjection{ - {ClusterTrustBundle: &api.ClusterTrustBundleProjection{Path: "/test-path"}}, - {ServiceAccountToken: &api.ServiceAccountTokenProjection{Path: "/test-path"}}, - }}, + {ConfigMap: &api.ConfigMapProjection{ + Items: []api.KeyToPath{ + {Key: "foo", Path: "test"}, + }, + }}, + {ServiceAccountToken: &api.ServiceAccountTokenProjection{ + Path: "test", + }}, + }, + }, }, }, }, }}, expected: []string{ - "spec.volumes[0].projected.sources.ClusterTrustBundle: has duplicated path with ServiceAccountToken \"/test-path\"", + "projected volume: \"foo\" - conflicting duplicate paths", + }, + }, + { + name: "overlapping paths in projected volume 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{ + Items: []api.KeyToPath{ + {Key: "foo", Path: "test"}, + }, + }}, + {ServiceAccountToken: &api.ServiceAccountTokenProjection{ + Path: "test", + }}, + }, + }, + }, + }, + }, + }}, + expected: []string{ + "projected volume: \"foo\" - conflicting duplicate paths", + }, + }, + { + name: "overlapping paths in projected volume 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\n"}, Path: "test"}, + }, + }}, + {ServiceAccountToken: &api.ServiceAccountTokenProjection{ + Path: "test", + }}, + }, + }, + }, + }, + }, + }}, + expected: []string{ + "projected volume: \"foo\" - conflicting duplicate paths", + }, + }, + { + name: "overlapping paths in projected volume 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{ + Path: "test", + }}, + {ServiceAccountToken: &api.ServiceAccountTokenProjection{ + Path: "test", + }}, + }, + }, + }, + }, + }, + }}, + expected: []string{ + "projected volume: \"foo\" - conflicting duplicate paths", }, }, {