diff --git a/pkg/api/pod/BUILD b/pkg/api/pod/BUILD index eed8b2e818b..01caf79b273 100644 --- a/pkg/api/pod/BUILD +++ b/pkg/api/pod/BUILD @@ -5,6 +5,7 @@ licenses(["notice"]) load( "@io_bazel_rules_go//go:def.bzl", "go_library", + "go_test", ) go_library( @@ -26,3 +27,15 @@ filegroup( srcs = [":package-srcs"], tags = ["automanaged"], ) + +go_test( + name = "go_default_test", + srcs = ["util_test.go"], + library = ":go_default_library", + tags = ["automanaged"], + deps = [ + "//pkg/api:go_default_library", + "//vendor:k8s.io/apimachinery/pkg/util/sets", + "//vendor:k8s.io/apimachinery/pkg/util/validation/field", + ], +) diff --git a/pkg/api/pod/util.go b/pkg/api/pod/util.go index 13ad73f61d5..d2005a29bac 100644 --- a/pkg/api/pod/util.go +++ b/pkg/api/pod/util.go @@ -56,34 +56,18 @@ func VisitPodSecretNames(pod *api.Pod, visitor func(string) bool) bool { for i := range pod.Spec.Volumes { source = &pod.Spec.Volumes[i].VolumeSource switch { - // case source.AWSElasticBlockStore: - // case source.AzureDisk: case source.AzureFile != nil: - if len(source.AzureFile.SecretName) > 0 && !visitor(source.Secret.SecretName) { + if len(source.AzureFile.SecretName) > 0 && !visitor(source.AzureFile.SecretName) { return false } case source.CephFS != nil: if source.CephFS.SecretRef != nil && !visitor(source.CephFS.SecretRef.Name) { return false } - // case source.Cinder: - // case source.ConfigMap: - // case source.DownwardAPI: - // case source.EmptyDir: - // case source.FC: case source.FlexVolume != nil: if source.FlexVolume.SecretRef != nil && !visitor(source.FlexVolume.SecretRef.Name) { return false } - // case source.Flocker: - // case source.GCEPersistentDisk: - // case source.GitRepo: - // case source.Glusterfs: - // case source.HostPath: - // case source.ISCSI: - // case source.NFS: - // case source.PersistentVolumeClaim: - // case source.PhotonPersistentDisk: case source.Projected != nil: for j := range source.Projected.Sources { if source.Projected.Sources[j].Secret != nil { @@ -92,7 +76,6 @@ func VisitPodSecretNames(pod *api.Pod, visitor func(string) bool) bool { } } } - // case source.Quobyte: case source.RBD != nil: if source.RBD.SecretRef != nil && !visitor(source.RBD.SecretRef.Name) { return false @@ -102,7 +85,6 @@ func VisitPodSecretNames(pod *api.Pod, visitor func(string) bool) bool { return false } } - // case source.VsphereVolume: } return true } diff --git a/pkg/api/pod/util_test.go b/pkg/api/pod/util_test.go new file mode 100644 index 00000000000..e947b03dab6 --- /dev/null +++ b/pkg/api/pod/util_test.go @@ -0,0 +1,166 @@ +/* +Copyright 2017 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package pod + +import ( + "reflect" + "testing" + + "strings" + + "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/apimachinery/pkg/util/validation/field" + "k8s.io/kubernetes/pkg/api" +) + +func TestPodSecrets(t *testing.T) { + // Stub containing all possible secret references in a pod. + // The names of the referenced secrets match struct paths detected by reflection. + pod := &api.Pod{ + Spec: api.PodSpec{ + Containers: []api.Container{{ + EnvFrom: []api.EnvFromSource{{ + SecretRef: &api.SecretEnvSource{ + LocalObjectReference: api.LocalObjectReference{ + Name: "Spec.Containers[*].EnvFrom[*].SecretRef"}}}}, + Env: []api.EnvVar{{ + ValueFrom: &api.EnvVarSource{ + SecretKeyRef: &api.SecretKeySelector{ + LocalObjectReference: api.LocalObjectReference{ + Name: "Spec.Containers[*].Env[*].ValueFrom.SecretKeyRef"}}}}}}}, + ImagePullSecrets: []api.LocalObjectReference{{ + Name: "Spec.ImagePullSecrets"}}, + InitContainers: []api.Container{{ + EnvFrom: []api.EnvFromSource{{ + SecretRef: &api.SecretEnvSource{ + LocalObjectReference: api.LocalObjectReference{ + Name: "Spec.InitContainers[*].EnvFrom[*].SecretRef"}}}}, + Env: []api.EnvVar{{ + ValueFrom: &api.EnvVarSource{ + SecretKeyRef: &api.SecretKeySelector{ + LocalObjectReference: api.LocalObjectReference{ + Name: "Spec.InitContainers[*].Env[*].ValueFrom.SecretKeyRef"}}}}}}}, + Volumes: []api.Volume{{ + VolumeSource: api.VolumeSource{ + AzureFile: &api.AzureFileVolumeSource{ + SecretName: "Spec.Volumes[*].VolumeSource.AzureFile.SecretName"}}}, { + VolumeSource: api.VolumeSource{ + CephFS: &api.CephFSVolumeSource{ + SecretRef: &api.LocalObjectReference{ + Name: "Spec.Volumes[*].VolumeSource.CephFS.SecretRef"}}}}, { + VolumeSource: api.VolumeSource{ + FlexVolume: &api.FlexVolumeSource{ + SecretRef: &api.LocalObjectReference{ + Name: "Spec.Volumes[*].VolumeSource.FlexVolume.SecretRef"}}}}, { + VolumeSource: api.VolumeSource{ + Projected: &api.ProjectedVolumeSource{ + Sources: []api.VolumeProjection{{ + Secret: &api.SecretProjection{ + LocalObjectReference: api.LocalObjectReference{ + Name: "Spec.Volumes[*].VolumeSource.Projected.Sources[*].Secret"}}}}}}}, { + VolumeSource: api.VolumeSource{ + RBD: &api.RBDVolumeSource{ + SecretRef: &api.LocalObjectReference{ + Name: "Spec.Volumes[*].VolumeSource.RBD.SecretRef"}}}}, { + VolumeSource: api.VolumeSource{ + Secret: &api.SecretVolumeSource{ + SecretName: "Spec.Volumes[*].VolumeSource.Secret.SecretName"}}}, { + VolumeSource: api.VolumeSource{ + Secret: &api.SecretVolumeSource{ + SecretName: "Spec.Volumes[*].VolumeSource.Secret"}}}}, + }, + } + extractedNames := sets.NewString() + VisitPodSecretNames(pod, func(name string) bool { + extractedNames.Insert(name) + return true + }) + + // excludedSecretPaths holds struct paths to fields with "secret" in the name that are not actually references to secret API objects + excludedSecretPaths := sets.NewString( + "Spec.Volumes[*].VolumeSource.CephFS.SecretFile", + ) + // expectedSecretPaths holds struct paths to fields with "secret" in the name that are references to secret API objects. + // every path here should be represented as an example in the Pod stub above, with the secret name set to the path. + expectedSecretPaths := sets.NewString( + "Spec.Containers[*].EnvFrom[*].SecretRef", + "Spec.Containers[*].Env[*].ValueFrom.SecretKeyRef", + "Spec.ImagePullSecrets", + "Spec.InitContainers[*].EnvFrom[*].SecretRef", + "Spec.InitContainers[*].Env[*].ValueFrom.SecretKeyRef", + "Spec.Volumes[*].VolumeSource.AzureFile.SecretName", + "Spec.Volumes[*].VolumeSource.CephFS.SecretRef", + "Spec.Volumes[*].VolumeSource.FlexVolume.SecretRef", + "Spec.Volumes[*].VolumeSource.Projected.Sources[*].Secret", + "Spec.Volumes[*].VolumeSource.RBD.SecretRef", + "Spec.Volumes[*].VolumeSource.Secret", + "Spec.Volumes[*].VolumeSource.Secret.SecretName", + ) + secretPaths := collectSecretPaths(t, nil, "", reflect.TypeOf(&api.Pod{})) + secretPaths = secretPaths.Difference(excludedSecretPaths) + if missingPaths := expectedSecretPaths.Difference(secretPaths); len(missingPaths) > 0 { + t.Logf("Missing expected secret paths:\n%s", strings.Join(missingPaths.List(), "\n")) + t.Error("Missing expected secret paths. Verify VisitPodSecretNames() is correctly finding the missing paths, then correct expectedSecretPaths") + } + if extraPaths := secretPaths.Difference(expectedSecretPaths); len(extraPaths) > 0 { + t.Logf("Extra secret paths:\n%s", strings.Join(extraPaths.List(), "\n")) + t.Error("Extra fields with 'secret' in the name found. Verify VisitPodSecretNames() is including these fields if appropriate, then correct expectedSecretPaths") + } + + if missingNames := expectedSecretPaths.Difference(extractedNames); len(missingNames) > 0 { + t.Logf("Missing expected secret names:\n%s", strings.Join(missingNames.List(), "\n")) + t.Error("Missing expected secret names. Verify the pod stub above includes these references, then verify VisitPodSecretNames() is correctly finding the missing names") + } + if extraNames := extractedNames.Difference(expectedSecretPaths); len(extraNames) > 0 { + 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") + } +} + +// collectSecretPaths traverses the object, computing all the struct paths that lead to fields with "secret" in the name. +func collectSecretPaths(t *testing.T, path *field.Path, name string, tp reflect.Type) sets.String { + secretPaths := sets.NewString() + + if tp.Kind() == reflect.Ptr { + secretPaths.Insert(collectSecretPaths(t, path, name, tp.Elem()).List()...) + return secretPaths + } + + if strings.Contains(strings.ToLower(name), "secret") { + secretPaths.Insert(path.String()) + } + + switch tp.Kind() { + case reflect.Ptr: + secretPaths.Insert(collectSecretPaths(t, path, name, tp.Elem()).List()...) + case reflect.Struct: + for i := 0; i < tp.NumField(); i++ { + field := tp.Field(i) + secretPaths.Insert(collectSecretPaths(t, path.Child(field.Name), field.Name, field.Type).List()...) + } + case reflect.Interface: + t.Errorf("cannot find secret fields in interface{} field %s", path.String()) + case reflect.Map: + secretPaths.Insert(collectSecretPaths(t, path.Key("*"), "", tp.Elem()).List()...) + case reflect.Slice: + secretPaths.Insert(collectSecretPaths(t, path.Key("*"), "", tp.Elem()).List()...) + default: + // all primitive types + } + + return secretPaths +} diff --git a/pkg/api/v1/pod/BUILD b/pkg/api/v1/pod/BUILD index 167fd3b9149..3e19802cad6 100644 --- a/pkg/api/v1/pod/BUILD +++ b/pkg/api/v1/pod/BUILD @@ -26,6 +26,8 @@ go_test( deps = [ "//pkg/api/v1:go_default_library", "//vendor:k8s.io/apimachinery/pkg/util/intstr", + "//vendor:k8s.io/apimachinery/pkg/util/sets", + "//vendor:k8s.io/apimachinery/pkg/util/validation/field", ], ) diff --git a/pkg/api/v1/pod/util.go b/pkg/api/v1/pod/util.go index 501502bc4af..172ef5185e6 100644 --- a/pkg/api/v1/pod/util.go +++ b/pkg/api/v1/pod/util.go @@ -144,34 +144,18 @@ func VisitPodSecretNames(pod *v1.Pod, visitor func(string) bool) bool { for i := range pod.Spec.Volumes { source = &pod.Spec.Volumes[i].VolumeSource switch { - // case source.AWSElasticBlockStore: - // case source.AzureDisk: case source.AzureFile != nil: - if len(source.AzureFile.SecretName) > 0 && !visitor(source.Secret.SecretName) { + if len(source.AzureFile.SecretName) > 0 && !visitor(source.AzureFile.SecretName) { return false } case source.CephFS != nil: if source.CephFS.SecretRef != nil && !visitor(source.CephFS.SecretRef.Name) { return false } - // case source.Cinder: - // case source.ConfigMap: - // case source.DownwardAPI: - // case source.EmptyDir: - // case source.FC: case source.FlexVolume != nil: if source.FlexVolume.SecretRef != nil && !visitor(source.FlexVolume.SecretRef.Name) { return false } - // case source.Flocker: - // case source.GCEPersistentDisk: - // case source.GitRepo: - // case source.Glusterfs: - // case source.HostPath: - // case source.ISCSI: - // case source.NFS: - // case source.PersistentVolumeClaim: - // case source.PhotonPersistentDisk: case source.Projected != nil: for j := range source.Projected.Sources { if source.Projected.Sources[j].Secret != nil { @@ -180,7 +164,6 @@ func VisitPodSecretNames(pod *v1.Pod, visitor func(string) bool) bool { } } } - // case source.Quobyte: case source.RBD != nil: if source.RBD.SecretRef != nil && !visitor(source.RBD.SecretRef.Name) { return false @@ -190,7 +173,6 @@ func VisitPodSecretNames(pod *v1.Pod, visitor func(string) bool) bool { return false } } - // case source.VsphereVolume: } return true } diff --git a/pkg/api/v1/pod/util_test.go b/pkg/api/v1/pod/util_test.go index f7e40c29a6c..7599f5ca42b 100644 --- a/pkg/api/v1/pod/util_test.go +++ b/pkg/api/v1/pod/util_test.go @@ -17,9 +17,13 @@ limitations under the License. package pod import ( + "reflect" + "strings" "testing" "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/apimachinery/pkg/util/validation/field" "k8s.io/kubernetes/pkg/api/v1" ) @@ -190,3 +194,141 @@ func TestFindPort(t *testing.T) { } } } + +func TestPodSecrets(t *testing.T) { + // Stub containing all possible secret references in a pod. + // The names of the referenced secrets match struct paths detected by reflection. + pod := &v1.Pod{ + Spec: v1.PodSpec{ + Containers: []v1.Container{{ + EnvFrom: []v1.EnvFromSource{{ + SecretRef: &v1.SecretEnvSource{ + LocalObjectReference: v1.LocalObjectReference{ + Name: "Spec.Containers[*].EnvFrom[*].SecretRef"}}}}, + Env: []v1.EnvVar{{ + ValueFrom: &v1.EnvVarSource{ + SecretKeyRef: &v1.SecretKeySelector{ + LocalObjectReference: v1.LocalObjectReference{ + Name: "Spec.Containers[*].Env[*].ValueFrom.SecretKeyRef"}}}}}}}, + ImagePullSecrets: []v1.LocalObjectReference{{ + Name: "Spec.ImagePullSecrets"}}, + InitContainers: []v1.Container{{ + EnvFrom: []v1.EnvFromSource{{ + SecretRef: &v1.SecretEnvSource{ + LocalObjectReference: v1.LocalObjectReference{ + Name: "Spec.InitContainers[*].EnvFrom[*].SecretRef"}}}}, + Env: []v1.EnvVar{{ + ValueFrom: &v1.EnvVarSource{ + SecretKeyRef: &v1.SecretKeySelector{ + LocalObjectReference: v1.LocalObjectReference{ + Name: "Spec.InitContainers[*].Env[*].ValueFrom.SecretKeyRef"}}}}}}}, + Volumes: []v1.Volume{{ + VolumeSource: v1.VolumeSource{ + AzureFile: &v1.AzureFileVolumeSource{ + SecretName: "Spec.Volumes[*].VolumeSource.AzureFile.SecretName"}}}, { + VolumeSource: v1.VolumeSource{ + CephFS: &v1.CephFSVolumeSource{ + SecretRef: &v1.LocalObjectReference{ + Name: "Spec.Volumes[*].VolumeSource.CephFS.SecretRef"}}}}, { + VolumeSource: v1.VolumeSource{ + FlexVolume: &v1.FlexVolumeSource{ + SecretRef: &v1.LocalObjectReference{ + Name: "Spec.Volumes[*].VolumeSource.FlexVolume.SecretRef"}}}}, { + VolumeSource: v1.VolumeSource{ + Projected: &v1.ProjectedVolumeSource{ + Sources: []v1.VolumeProjection{{ + Secret: &v1.SecretProjection{ + LocalObjectReference: v1.LocalObjectReference{ + Name: "Spec.Volumes[*].VolumeSource.Projected.Sources[*].Secret"}}}}}}}, { + VolumeSource: v1.VolumeSource{ + RBD: &v1.RBDVolumeSource{ + SecretRef: &v1.LocalObjectReference{ + Name: "Spec.Volumes[*].VolumeSource.RBD.SecretRef"}}}}, { + VolumeSource: v1.VolumeSource{ + Secret: &v1.SecretVolumeSource{ + SecretName: "Spec.Volumes[*].VolumeSource.Secret.SecretName"}}}, { + VolumeSource: v1.VolumeSource{ + Secret: &v1.SecretVolumeSource{ + SecretName: "Spec.Volumes[*].VolumeSource.Secret"}}}}, + }, + } + extractedNames := sets.NewString() + VisitPodSecretNames(pod, func(name string) bool { + extractedNames.Insert(name) + return true + }) + + // excludedSecretPaths holds struct paths to fields with "secret" in the name that are not actually references to secret API objects + excludedSecretPaths := sets.NewString( + "Spec.Volumes[*].VolumeSource.CephFS.SecretFile", + ) + // expectedSecretPaths holds struct paths to fields with "secret" in the name that are references to secret API objects. + // every path here should be represented as an example in the Pod stub above, with the secret name set to the path. + expectedSecretPaths := sets.NewString( + "Spec.Containers[*].EnvFrom[*].SecretRef", + "Spec.Containers[*].Env[*].ValueFrom.SecretKeyRef", + "Spec.ImagePullSecrets", + "Spec.InitContainers[*].EnvFrom[*].SecretRef", + "Spec.InitContainers[*].Env[*].ValueFrom.SecretKeyRef", + "Spec.Volumes[*].VolumeSource.AzureFile.SecretName", + "Spec.Volumes[*].VolumeSource.CephFS.SecretRef", + "Spec.Volumes[*].VolumeSource.FlexVolume.SecretRef", + "Spec.Volumes[*].VolumeSource.Projected.Sources[*].Secret", + "Spec.Volumes[*].VolumeSource.RBD.SecretRef", + "Spec.Volumes[*].VolumeSource.Secret", + "Spec.Volumes[*].VolumeSource.Secret.SecretName", + ) + secretPaths := collectSecretPaths(t, nil, "", reflect.TypeOf(&v1.Pod{})) + secretPaths = secretPaths.Difference(excludedSecretPaths) + if missingPaths := expectedSecretPaths.Difference(secretPaths); len(missingPaths) > 0 { + t.Logf("Missing expected secret paths:\n%s", strings.Join(missingPaths.List(), "\n")) + t.Error("Missing expected secret paths. Verify VisitPodSecretNames() is correctly finding the missing paths, then correct expectedSecretPaths") + } + if extraPaths := secretPaths.Difference(expectedSecretPaths); len(extraPaths) > 0 { + t.Logf("Extra secret paths:\n%s", strings.Join(extraPaths.List(), "\n")) + t.Error("Extra fields with 'secret' in the name found. Verify VisitPodSecretNames() is including these fields if appropriate, then correct expectedSecretPaths") + } + + if missingNames := expectedSecretPaths.Difference(extractedNames); len(missingNames) > 0 { + t.Logf("Missing expected secret names:\n%s", strings.Join(missingNames.List(), "\n")) + t.Error("Missing expected secret names. Verify the pod stub above includes these references, then verify VisitPodSecretNames() is correctly finding the missing names") + } + if extraNames := extractedNames.Difference(expectedSecretPaths); len(extraNames) > 0 { + 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") + } +} + +// collectSecretPaths traverses the object, computing all the struct paths that lead to fields with "secret" in the name. +func collectSecretPaths(t *testing.T, path *field.Path, name string, tp reflect.Type) sets.String { + secretPaths := sets.NewString() + + if tp.Kind() == reflect.Ptr { + secretPaths.Insert(collectSecretPaths(t, path, name, tp.Elem()).List()...) + return secretPaths + } + + if strings.Contains(strings.ToLower(name), "secret") { + secretPaths.Insert(path.String()) + } + + switch tp.Kind() { + case reflect.Ptr: + secretPaths.Insert(collectSecretPaths(t, path, name, tp.Elem()).List()...) + case reflect.Struct: + for i := 0; i < tp.NumField(); i++ { + field := tp.Field(i) + secretPaths.Insert(collectSecretPaths(t, path.Child(field.Name), field.Name, field.Type).List()...) + } + case reflect.Interface: + t.Errorf("cannot find secret fields in interface{} field %s", path.String()) + case reflect.Map: + secretPaths.Insert(collectSecretPaths(t, path.Key("*"), "", tp.Elem()).List()...) + case reflect.Slice: + secretPaths.Insert(collectSecretPaths(t, path.Key("*"), "", tp.Elem()).List()...) + default: + // all primitive types + } + + return secretPaths +}