diff --git a/pkg/api/pod/util.go b/pkg/api/pod/util.go index dd79c4831b9..847b3abd15b 100644 --- a/pkg/api/pod/util.go +++ b/pkg/api/pod/util.go @@ -92,11 +92,23 @@ func VisitContainers(podSpec *api.PodSpec, mask ContainerType, visitor Container // Visitor is called with each object name, and returns true if visiting should continue type Visitor func(name string) (shouldContinue bool) +func skipEmptyNames(visitor Visitor) Visitor { + return func(name string) bool { + if len(name) == 0 { + // continue visiting + return true + } + // delegate to visitor + return visitor(name) + } +} + // VisitPodSecretNames invokes the visitor function with the name of every secret // referenced by the pod spec. If visitor returns false, visiting is short-circuited. // Transitive references (e.g. pod -> pvc -> pv -> secret) are not visited. // Returns true if visiting completed, false if visiting was short-circuited. func VisitPodSecretNames(pod *api.Pod, visitor Visitor, containerType ContainerType) bool { + visitor = skipEmptyNames(visitor) for _, reference := range pod.Spec.ImagePullSecrets { if !visitor(reference.Name) { return false @@ -185,6 +197,7 @@ func visitContainerSecretNames(container *api.Container, visitor Visitor) bool { // Transitive references (e.g. pod -> pvc -> pv -> secret) are not visited. // Returns true if visiting completed, false if visiting was short-circuited. func VisitPodConfigmapNames(pod *api.Pod, visitor Visitor, containerType ContainerType) bool { + visitor = skipEmptyNames(visitor) VisitContainers(&pod.Spec, containerType, func(c *api.Container, containerType ContainerType) bool { return visitContainerConfigmapNames(c, visitor) }) diff --git a/pkg/api/pod/util_test.go b/pkg/api/pod/util_test.go index 2e6b24b17b0..b3373e963a8 100644 --- a/pkg/api/pod/util_test.go +++ b/pkg/api/pod/util_test.go @@ -365,6 +365,21 @@ func TestPodSecrets(t *testing.T) { t.Logf("Extra secret names:\n%s", strings.Join(extraNames.List(), "\n")) t.Error("Extra secret names extracted. Verify VisitPodSecretNames() is correctly extracting secret names") } + + // emptyPod is a stub containing empty object names + emptyPod := &api.Pod{ + Spec: api.PodSpec{ + Containers: []api.Container{{ + EnvFrom: []api.EnvFromSource{{ + SecretRef: &api.SecretEnvSource{ + LocalObjectReference: api.LocalObjectReference{ + Name: ""}}}}}}, + }, + } + VisitPodSecretNames(emptyPod, func(name string) bool { + t.Fatalf("expected no empty names collected, got %q", name) + return false + }, AllContainers) } // collectResourcePaths traverses the object, computing all the struct paths that lead to fields with resourcename in the name. @@ -494,6 +509,21 @@ func TestPodConfigmaps(t *testing.T) { t.Logf("Extra names:\n%s", strings.Join(extraNames.List(), "\n")) t.Error("Extra names extracted. Verify VisitPodConfigmapNames() is correctly extracting resource names") } + + // emptyPod is a stub containing empty object names + emptyPod := &api.Pod{ + Spec: api.PodSpec{ + Containers: []api.Container{{ + EnvFrom: []api.EnvFromSource{{ + ConfigMapRef: &api.ConfigMapEnvSource{ + LocalObjectReference: api.LocalObjectReference{ + Name: ""}}}}}}, + }, + } + VisitPodConfigmapNames(emptyPod, func(name string) bool { + t.Fatalf("expected no empty names collected, got %q", name) + return false + }, AllContainers) } func TestDropFSGroupFields(t *testing.T) { diff --git a/pkg/api/v1/persistentvolume/util.go b/pkg/api/v1/persistentvolume/util.go index 003b2e70d97..376021a292b 100644 --- a/pkg/api/v1/persistentvolume/util.go +++ b/pkg/api/v1/persistentvolume/util.go @@ -30,10 +30,22 @@ func getClaimRefNamespace(pv *corev1.PersistentVolume) string { // Visitor is called with each object's namespace and name, and returns true if visiting should continue type Visitor func(namespace, name string, kubeletVisible bool) (shouldContinue bool) +func skipEmptyNames(visitor Visitor) Visitor { + return func(namespace, name string, kubeletVisible bool) bool { + if len(name) == 0 { + // continue visiting + return true + } + // delegate to visitor + return visitor(namespace, name, kubeletVisible) + } +} + // VisitPVSecretNames invokes the visitor function with the name of every secret // referenced by the PV spec. If visitor returns false, visiting is short-circuited. // Returns true if visiting completed, false if visiting was short-circuited. func VisitPVSecretNames(pv *corev1.PersistentVolume, visitor Visitor) bool { + visitor = skipEmptyNames(visitor) source := &pv.Spec.PersistentVolumeSource switch { case source.AzureFile != nil: diff --git a/pkg/api/v1/persistentvolume/util_test.go b/pkg/api/v1/persistentvolume/util_test.go index 1f891cb48d4..8ae481bb5be 100644 --- a/pkg/api/v1/persistentvolume/util_test.go +++ b/pkg/api/v1/persistentvolume/util_test.go @@ -238,6 +238,19 @@ func TestPVSecrets(t *testing.T) { t.Logf("Extra namespaced names:\n%s", strings.Join(extraNames.List(), "\n")) t.Error("Extra namespaced names extracted. Verify VisitPVSecretNames() is correctly extracting secret names") } + + emptyPV := &corev1.PersistentVolume{ + Spec: corev1.PersistentVolumeSpec{ + ClaimRef: &corev1.ObjectReference{Namespace: "claimrefns", Name: "claimrefname"}, + PersistentVolumeSource: corev1.PersistentVolumeSource{ + CephFS: &corev1.CephFSPersistentVolumeSource{ + SecretRef: &corev1.SecretReference{ + Name: "", + Namespace: "cephfs"}}}}} + VisitPVSecretNames(emptyPV, func(namespace, name string, kubeletVisible bool) bool { + t.Fatalf("expected no empty names collected, got %q", name) + return false + }) } // collectSecretPaths traverses the object, computing all the struct paths that lead to fields with "secret" in the name. diff --git a/pkg/api/v1/pod/util.go b/pkg/api/v1/pod/util.go index a944cba8f5b..f382509aeef 100644 --- a/pkg/api/v1/pod/util.go +++ b/pkg/api/v1/pod/util.go @@ -82,6 +82,17 @@ type ContainerVisitor func(container *v1.Container, containerType ContainerType) // Visitor is called with each object name, and returns true if visiting should continue type Visitor func(name string) (shouldContinue bool) +func skipEmptyNames(visitor Visitor) Visitor { + return func(name string) bool { + if len(name) == 0 { + // continue visiting + return true + } + // delegate to visitor + return visitor(name) + } +} + // VisitContainers invokes the visitor function with a pointer to every container // spec in the given pod spec with type set in mask. If visitor returns false, // visiting is short-circuited. VisitContainers returns true if visiting completes, @@ -116,6 +127,7 @@ func VisitContainers(podSpec *v1.PodSpec, mask ContainerType, visitor ContainerV // Transitive references (e.g. pod -> pvc -> pv -> secret) are not visited. // Returns true if visiting completed, false if visiting was short-circuited. func VisitPodSecretNames(pod *v1.Pod, visitor Visitor) bool { + visitor = skipEmptyNames(visitor) for _, reference := range pod.Spec.ImagePullSecrets { if !visitor(reference.Name) { return false @@ -205,6 +217,7 @@ func visitContainerSecretNames(container *v1.Container, visitor Visitor) bool { // Transitive references (e.g. pod -> pvc -> pv -> secret) are not visited. // Returns true if visiting completed, false if visiting was short-circuited. func VisitPodConfigmapNames(pod *v1.Pod, visitor Visitor) bool { + visitor = skipEmptyNames(visitor) VisitContainers(&pod.Spec, AllContainers, func(c *v1.Container, containerType ContainerType) bool { return visitContainerConfigmapNames(c, visitor) }) diff --git a/pkg/api/v1/pod/util_test.go b/pkg/api/v1/pod/util_test.go index c87ce4d92e0..961f801f738 100644 --- a/pkg/api/v1/pod/util_test.go +++ b/pkg/api/v1/pod/util_test.go @@ -531,6 +531,21 @@ func TestPodSecrets(t *testing.T) { t.Logf("Extra secret names:\n%s", strings.Join(extraNames.List(), "\n")) t.Error("Extra secret names extracted. Verify VisitPodSecretNames() is correctly extracting secret names") } + + // emptyPod is a stub containing empty object names + emptyPod := &v1.Pod{ + Spec: v1.PodSpec{ + Containers: []v1.Container{{ + EnvFrom: []v1.EnvFromSource{{ + SecretRef: &v1.SecretEnvSource{ + LocalObjectReference: v1.LocalObjectReference{ + Name: ""}}}}}}, + }, + } + VisitPodSecretNames(emptyPod, func(name string) bool { + t.Fatalf("expected no empty names collected, got %q", name) + return false + }) } // collectResourcePaths traverses the object, computing all the struct paths that lead to fields with resourcename in the name. @@ -660,6 +675,21 @@ func TestPodConfigmaps(t *testing.T) { t.Logf("Extra names:\n%s", strings.Join(extraNames.List(), "\n")) t.Error("Extra names extracted. Verify VisitPodConfigmapNames() is correctly extracting resource names") } + + // emptyPod is a stub containing empty object names + emptyPod := &v1.Pod{ + Spec: v1.PodSpec{ + Containers: []v1.Container{{ + EnvFrom: []v1.EnvFromSource{{ + ConfigMapRef: &v1.ConfigMapEnvSource{ + LocalObjectReference: v1.LocalObjectReference{ + Name: ""}}}}}}, + }, + } + VisitPodConfigmapNames(emptyPod, func(name string) bool { + t.Fatalf("expected no empty names collected, got %q", name) + return false + }) } func newPod(now metav1.Time, ready bool, beforeSec int) *v1.Pod { diff --git a/plugin/pkg/admission/serviceaccount/admission_test.go b/plugin/pkg/admission/serviceaccount/admission_test.go index 77ef50c6222..14fe7f223b3 100644 --- a/plugin/pkg/admission/serviceaccount/admission_test.go +++ b/plugin/pkg/admission/serviceaccount/admission_test.go @@ -129,7 +129,7 @@ func TestRejectsMirrorPodWithSecretVolumes(t *testing.T) { }, Spec: api.PodSpec{ Volumes: []api.Volume{ - {VolumeSource: api.VolumeSource{Secret: &api.SecretVolumeSource{}}}, + {VolumeSource: api.VolumeSource{Secret: &api.SecretVolumeSource{SecretName: "mysecret"}}}, }, }, }