Merge pull request #41957 from liggitt/mirror-pod-secrets

Automatic merge from submit-queue (batch tested with PRs 41814, 41922, 41957, 41406, 41077)

Use consistent helper for getting secret names from pod

Kubelet secret-manager and mirror-pod admission both need to know what secrets a pod spec references. Eventually, a node authorizer will also need to know the list of secrets.

This creates a single (well, double, because api versions) helper that can be used to traverse the secret names referenced from a pod, optionally short-circuiting (for places that are just looking to see if any secrets are referenced, like admission, or are looking for a particular secret ref, like authorization)

Fixes:
* secret manager not handling secrets used by env/envFrom in initcontainers
* admission allowing mirror pods with secret references

@smarterclayton @wojtek-t

pkg/api/pod/BUILD

@@ -11,6 +11,7 @@ go_library(
     name = "go_default_library",
     srcs = ["util.go"],
     tags = ["automanaged"],
+    deps = ["//pkg/api:go_default_library"],
 )
 
 filegroup(

pkg/api/pod/util.go

@@ -16,6 +16,8 @@ limitations under the License.
 
 package pod
 
+import "k8s.io/kubernetes/pkg/api"
+
 const (
 	// TODO: to be de!eted after v1.3 is released. PodSpec has a dedicated Hostname field.
 	// The annotation value is a string specifying the hostname to be used for the pod e.g 'my-webserver-1'
@@ -29,3 +31,96 @@ const (
 	// <hostname>.my-web-service.<namespace>.svc.<cluster domain>" would be resolved by the cluster DNS Server.
 	PodSubdomainAnnotation = "pod.beta.kubernetes.io/subdomain"
 )
+
+// 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 func(string) bool) bool {
+	for _, reference := range pod.Spec.ImagePullSecrets {
+		if !visitor(reference.Name) {
+			return false
+		}
+	}
+	for i := range pod.Spec.InitContainers {
+		if !visitContainerSecretNames(&pod.Spec.InitContainers[i], visitor) {
+			return false
+		}
+	}
+	for i := range pod.Spec.Containers {
+		if !visitContainerSecretNames(&pod.Spec.Containers[i], visitor) {
+			return false
+		}
+	}
+	var source *api.VolumeSource
+	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) {
+				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 {
+					if !visitor(source.Projected.Sources[j].Secret.Name) {
+						return false
+					}
+				}
+			}
+		// case source.Quobyte:
+		case source.RBD != nil:
+			if source.RBD.SecretRef != nil && !visitor(source.RBD.SecretRef.Name) {
+				return false
+			}
+		case source.Secret != nil:
+			if !visitor(source.Secret.SecretName) {
+				return false
+			}
+		}
+		// case source.VsphereVolume:
+	}
+	return true
+}
+
+func visitContainerSecretNames(container *api.Container, visitor func(string) bool) bool {
+	for _, env := range container.EnvFrom {
+		if env.SecretRef != nil {
+			if !visitor(env.SecretRef.Name) {
+				return false
+			}
+		}
+	}
+	for _, envVar := range container.Env {
+		if envVar.ValueFrom != nil && envVar.ValueFrom.SecretKeyRef != nil {
+			if !visitor(envVar.ValueFrom.SecretKeyRef.Name) {
+				return false
+			}
+		}
+	}
+	return true
+}

pkg/api/v1/pod/util.go

@@ -118,3 +118,97 @@ func SetInitContainersStatusesAnnotations(pod *v1.Pod) error {
 	}
 	return nil
 }
+
+// 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 *v1.Pod, visitor func(string) bool) bool {
+	for _, reference := range pod.Spec.ImagePullSecrets {
+		if !visitor(reference.Name) {
+			return false
+		}
+	}
+	for i := range pod.Spec.InitContainers {
+		if !visitContainerSecretNames(&pod.Spec.InitContainers[i], visitor) {
+			return false
+		}
+	}
+	for i := range pod.Spec.Containers {
+		if !visitContainerSecretNames(&pod.Spec.Containers[i], visitor) {
+			return false
+		}
+	}
+	var source *v1.VolumeSource
+
+	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) {
+				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 {
+					if !visitor(source.Projected.Sources[j].Secret.Name) {
+						return false
+					}
+				}
+			}
+		// case source.Quobyte:
+		case source.RBD != nil:
+			if source.RBD.SecretRef != nil && !visitor(source.RBD.SecretRef.Name) {
+				return false
+			}
+		case source.Secret != nil:
+			if !visitor(source.Secret.SecretName) {
+				return false
+			}
+		}
+		// case source.VsphereVolume:
+	}
+	return true
+}
+
+func visitContainerSecretNames(container *v1.Container, visitor func(string) bool) bool {
+	for _, env := range container.EnvFrom {
+		if env.SecretRef != nil {
+			if !visitor(env.SecretRef.Name) {
+				return false
+			}
+		}
+	}
+	for _, envVar := range container.Env {
+		if envVar.ValueFrom != nil && envVar.ValueFrom.SecretKeyRef != nil {
+			if !visitor(envVar.ValueFrom.SecretKeyRef.Name) {
+				return false
+			}
+		}
+	}
+	return true
+}

pkg/kubelet/secret/BUILD

@@ -34,6 +34,7 @@ go_library(
     tags = ["automanaged"],
     deps = [
         "//pkg/api/v1:go_default_library",
+        "//pkg/api/v1/pod:go_default_library",
         "//pkg/client/clientset_generated/clientset:go_default_library",
         "//pkg/kubelet/util:go_default_library",
         "//vendor:k8s.io/apimachinery/pkg/api/errors",

pkg/kubelet/secret/secret_manager.go

@@ -24,6 +24,7 @@ import (
 
 	storageetcd "k8s.io/apiserver/pkg/storage/etcd"
 	"k8s.io/kubernetes/pkg/api/v1"
+	podutil "k8s.io/kubernetes/pkg/api/v1/pod"
 	clientset "k8s.io/kubernetes/pkg/client/clientset_generated/clientset"
 	"k8s.io/kubernetes/pkg/kubelet/util"
 
@@ -261,32 +262,10 @@ func (c *cachingSecretManager) GetSecret(namespace, name string) (*v1.Secret, er
 
 func getSecretNames(pod *v1.Pod) sets.String {
 	result := sets.NewString()
-	for _, reference := range pod.Spec.ImagePullSecrets {
+	podutil.VisitPodSecretNames(pod, func(name string) bool {
-		result.Insert(reference.Name)
+		result.Insert(name)
-	}
+		return true
-	for i := range pod.Spec.Containers {
+	})
-		for _, env := range pod.Spec.Containers[i].EnvFrom {
-			if env.SecretRef != nil {
-				result.Insert(env.SecretRef.Name)
-			}
-		}
-		for _, envVar := range pod.Spec.Containers[i].Env {
-			if envVar.ValueFrom != nil && envVar.ValueFrom.SecretKeyRef != nil {
-				result.Insert(envVar.ValueFrom.SecretKeyRef.Name)
-			}
-		}
-	}
-	for i := range pod.Spec.Volumes {
-		if source := pod.Spec.Volumes[i].Secret; source != nil {
-			result.Insert(source.SecretName)
-		} else if source := pod.Spec.Volumes[i].Projected; source != nil {
-			for j := range source.Sources {
-				if secretVolumeSource := source.Sources[j].Secret; secretVolumeSource != nil {
-					result.Insert(secretVolumeSource.Name)
-				}
-			}
-		}
-	}
 	return result
 }
 

plugin/pkg/admission/serviceaccount/BUILD

@@ -17,6 +17,7 @@ go_library(
     tags = ["automanaged"],
     deps = [
         "//pkg/api:go_default_library",
+        "//pkg/api/pod:go_default_library",
         "//pkg/client/clientset_generated/internalclientset:go_default_library",
         "//pkg/client/informers/informers_generated/internalversion:go_default_library",
         "//pkg/client/listers/core/internalversion:go_default_library",

plugin/pkg/admission/serviceaccount/admission.go

@@ -31,6 +31,7 @@ import (
 	"k8s.io/apiserver/pkg/admission"
 	"k8s.io/apiserver/pkg/storage/names"
 	"k8s.io/kubernetes/pkg/api"
+	podutil "k8s.io/kubernetes/pkg/api/pod"
 	"k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset"
 	informers "k8s.io/kubernetes/pkg/client/informers/informers_generated/internalversion"
 	corelisters "k8s.io/kubernetes/pkg/client/listers/core/internalversion"
@@ -149,10 +150,13 @@ func (s *serviceAccount) Admit(a admission.Attributes) (err error) {
 		if len(pod.Spec.ServiceAccountName) != 0 {
 			return admission.NewForbidden(a, fmt.Errorf("a mirror pod may not reference service accounts"))
 		}
-		for _, volume := range pod.Spec.Volumes {
+		hasSecrets := false
-			if volume.VolumeSource.Secret != nil {
+		podutil.VisitPodSecretNames(pod, func(name string) bool {
-				return admission.NewForbidden(a, fmt.Errorf("a mirror pod may not reference secrets"))
+			hasSecrets = true
-			}
+			return false
+		})
+		if hasSecrets {
+			return admission.NewForbidden(a, fmt.Errorf("a mirror pod may not reference secrets"))
 		}
 		return nil
 	}