Restrict mirror pod owner references (#84657)

* Restrict mirror pod owners.

See http://git.k8s.io/enhancements/keps/sig-auth/20190916-noderestriction-pods.md

* Address feedback, refactor test

* Verify node owner UID
This commit is contained in:
Tim Allclair (St. Clair) 2019-11-14 20:52:16 -08:00 committed by Kubernetes Prow Robot
parent 3202bc1044
commit 581d3e26c9
2 changed files with 250 additions and 62 deletions

View File

@ -70,6 +70,7 @@ type Plugin struct {
*admission.Handler
nodeIdentifier nodeidentifier.NodeIdentifier
podsGetter corev1lister.PodLister
nodesGetter corev1lister.NodeLister
tokenRequestEnabled bool
csiNodeInfoEnabled bool
@ -92,6 +93,7 @@ func (p *Plugin) InspectFeatureGates(featureGates featuregate.FeatureGate) {
// SetExternalKubeInformerFactory registers an informer factory into Plugin
func (p *Plugin) SetExternalKubeInformerFactory(f informers.SharedInformerFactory) {
p.podsGetter = f.Core().V1().Pods().Lister()
p.nodesGetter = f.Core().V1().Nodes().Lister()
}
// ValidateInitialization validates the Plugin was initialized properly
@ -102,6 +104,9 @@ func (p *Plugin) ValidateInitialization() error {
if p.podsGetter == nil {
return fmt.Errorf("%s requires a pod getter", PluginName)
}
if p.nodesGetter == nil {
return fmt.Errorf("%s requires a node getter", PluginName)
}
return nil
}
@ -128,6 +133,8 @@ func (p *Plugin) Admit(ctx context.Context, a admission.Attributes, o admission.
return admission.NewForbidden(a, fmt.Errorf("could not determine node from user %q", a.GetUserInfo().GetName()))
}
// TODO: if node doesn't exist and this isn't a create node request, then reject.
switch a.GetResource().GroupResource() {
case podResource:
switch a.GetSubresource() {
@ -177,43 +184,7 @@ func (p *Plugin) Admit(ctx context.Context, a admission.Attributes, o admission.
func (p *Plugin) admitPod(nodeName string, a admission.Attributes) error {
switch a.GetOperation() {
case admission.Create:
// require a pod object
pod, ok := a.GetObject().(*api.Pod)
if !ok {
return admission.NewForbidden(a, fmt.Errorf("unexpected type %T", a.GetObject()))
}
// only allow nodes to create mirror pods
if _, isMirrorPod := pod.Annotations[api.MirrorPodAnnotationKey]; !isMirrorPod {
return admission.NewForbidden(a, fmt.Errorf("pod does not have %q annotation, node %q can only create mirror pods", api.MirrorPodAnnotationKey, nodeName))
}
// only allow nodes to create a pod bound to itself
if pod.Spec.NodeName != nodeName {
return admission.NewForbidden(a, fmt.Errorf("node %q can only create pods with spec.nodeName set to itself", nodeName))
}
// don't allow a node to create a pod that references any other API objects
if pod.Spec.ServiceAccountName != "" {
return admission.NewForbidden(a, fmt.Errorf("node %q can not create pods that reference a service account", nodeName))
}
hasSecrets := false
podutil.VisitPodSecretNames(pod, func(name string) (shouldContinue bool) { hasSecrets = true; return false })
if hasSecrets {
return admission.NewForbidden(a, fmt.Errorf("node %q can not create pods that reference secrets", nodeName))
}
hasConfigMaps := false
podutil.VisitPodConfigmapNames(pod, func(name string) (shouldContinue bool) { hasConfigMaps = true; return false })
if hasConfigMaps {
return admission.NewForbidden(a, fmt.Errorf("node %q can not create pods that reference configmaps", nodeName))
}
for _, v := range pod.Spec.Volumes {
if v.PersistentVolumeClaim != nil {
return admission.NewForbidden(a, fmt.Errorf("node %q can not create pods that reference persistentvolumeclaims", nodeName))
}
}
return nil
return p.admitPodCreate(nodeName, a)
case admission.Delete:
// get the existing pod
@ -235,6 +206,75 @@ func (p *Plugin) admitPod(nodeName string, a admission.Attributes) error {
}
}
func (p *Plugin) admitPodCreate(nodeName string, a admission.Attributes) error {
// require a pod object
pod, ok := a.GetObject().(*api.Pod)
if !ok {
return admission.NewForbidden(a, fmt.Errorf("unexpected type %T", a.GetObject()))
}
// only allow nodes to create mirror pods
if _, isMirrorPod := pod.Annotations[api.MirrorPodAnnotationKey]; !isMirrorPod {
return admission.NewForbidden(a, fmt.Errorf("pod does not have %q annotation, node %q can only create mirror pods", api.MirrorPodAnnotationKey, nodeName))
}
// only allow nodes to create a pod bound to itself
if pod.Spec.NodeName != nodeName {
return admission.NewForbidden(a, fmt.Errorf("node %q can only create pods with spec.nodeName set to itself", nodeName))
}
if len(pod.OwnerReferences) > 1 {
return admission.NewForbidden(a, fmt.Errorf("node %q can only create pods with a single owner reference set to itself", nodeName))
}
if len(pod.OwnerReferences) == 1 {
owner := pod.OwnerReferences[0]
if owner.APIVersion != v1.SchemeGroupVersion.String() ||
owner.Kind != "Node" ||
owner.Name != nodeName {
return admission.NewForbidden(a, fmt.Errorf("node %q can only create pods with an owner reference set to itself", nodeName))
}
if owner.Controller == nil || !*owner.Controller {
return admission.NewForbidden(a, fmt.Errorf("node %q can only create pods with a controller owner reference set to itself", nodeName))
}
if owner.BlockOwnerDeletion != nil && *owner.BlockOwnerDeletion {
return admission.NewForbidden(a, fmt.Errorf("node %q must not set blockOwnerDeletion on an owner reference", nodeName))
}
// Verify the node UID.
node, err := p.nodesGetter.Get(nodeName)
if errors.IsNotFound(err) {
return err
}
if err != nil {
return admission.NewForbidden(a, fmt.Errorf("error looking up node %s to verify uid: %v", nodeName, err))
}
if owner.UID != node.UID {
return admission.NewForbidden(a, fmt.Errorf("node %s UID mismatch: expected %s got %s", nodeName, owner.UID, node.UID))
}
}
// don't allow a node to create a pod that references any other API objects
if pod.Spec.ServiceAccountName != "" {
return admission.NewForbidden(a, fmt.Errorf("node %q can not create pods that reference a service account", nodeName))
}
hasSecrets := false
podutil.VisitPodSecretNames(pod, func(name string) (shouldContinue bool) { hasSecrets = true; return false })
if hasSecrets {
return admission.NewForbidden(a, fmt.Errorf("node %q can not create pods that reference secrets", nodeName))
}
hasConfigMaps := false
podutil.VisitPodConfigmapNames(pod, func(name string) (shouldContinue bool) { hasConfigMaps = true; return false })
if hasConfigMaps {
return admission.NewForbidden(a, fmt.Errorf("node %q can not create pods that reference configmaps", nodeName))
}
for _, v := range pod.Spec.Volumes {
if v.PersistentVolumeClaim != nil {
return admission.NewForbidden(a, fmt.Errorf("node %q can not create pods that reference persistentvolumeclaims", nodeName))
}
}
return nil
}
// admitPodStatus allows to update the status of a pod if it is
// assigned to the current node.
func (p *Plugin) admitPodStatus(nodeName string, a admission.Attributes) error {

View File

@ -25,6 +25,7 @@ import (
"time"
corev1 "k8s.io/api/core/v1"
v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/types"
@ -48,7 +49,6 @@ import (
var (
trEnabledFeature = featuregate.NewFeatureGate()
trDisabledFeature = featuregate.NewFeatureGate()
csiNodeInfoEnabledFeature = featuregate.NewFeatureGate()
csiNodeInfoDisabledFeature = featuregate.NewFeatureGate()
)
@ -61,7 +61,6 @@ func init() {
features.ExpandPersistentVolumes: {Default: false},
}
utilruntime.Must(trEnabledFeature.Add(relevantFeatures))
utilruntime.Must(trDisabledFeature.Add(relevantFeatures))
utilruntime.Must(csiNodeInfoEnabledFeature.Add(relevantFeatures))
utilruntime.Must(csiNodeInfoDisabledFeature.Add(relevantFeatures))
@ -83,6 +82,18 @@ func makeTestPod(namespace, name, node string, mirror bool) (*api.Pod, *corev1.P
if mirror {
corePod.Annotations = map[string]string{api.MirrorPodAnnotationKey: "true"}
v1Pod.Annotations = map[string]string{api.MirrorPodAnnotationKey: "true"}
// Insert a valid owner reference by default.
controller := true
owner := metav1.OwnerReference{
APIVersion: "v1",
Kind: "Node",
Name: node,
UID: types.UID(node + "-uid"),
Controller: &controller,
}
corePod.OwnerReferences = []metav1.OwnerReference{owner}
v1Pod.OwnerReferences = []metav1.OwnerReference{owner}
}
return corePod, v1Pod
}
@ -218,12 +229,40 @@ func setForbiddenUpdateLabels(node *api.Node, value string) *api.Node {
return node
}
type admitTestCase struct {
name string
podsGetter corev1lister.PodLister
nodesGetter corev1lister.NodeLister
attributes admission.Attributes
features featuregate.FeatureGate
err string
}
func (a *admitTestCase) run(t *testing.T) {
t.Run(a.name, func(t *testing.T) {
c := NewPlugin(nodeidentifier.NewDefaultNodeIdentifier())
if a.features != nil {
c.InspectFeatureGates(a.features)
}
c.podsGetter = a.podsGetter
c.nodesGetter = a.nodesGetter
err := c.Admit(context.TODO(), a.attributes, nil)
if (err == nil) != (len(a.err) == 0) {
t.Errorf("nodePlugin.Admit() error = %v, expected %v", err, a.err)
return
}
if len(a.err) > 0 && !strings.Contains(err.Error(), a.err) {
t.Errorf("nodePlugin.Admit() error = %v, expected %v", err, a.err)
}
})
}
func Test_nodePlugin_Admit(t *testing.T) {
var (
mynode = &user.DefaultInfo{Name: "system:node:mynode", Groups: []string{"system:nodes"}}
bob = &user.DefaultInfo{Name: "bob"}
mynodeObjMeta = metav1.ObjectMeta{Name: "mynode"}
mynodeObjMeta = metav1.ObjectMeta{Name: "mynode", UID: "mynode-uid"}
mynodeObj = &api.Node{ObjectMeta: mynodeObjMeta}
mynodeObjConfigA = &api.Node{ObjectMeta: mynodeObjMeta, Spec: api.NodeSpec{ConfigSource: &api.NodeConfigSource{
ConfigMap: &api.ConfigMapNodeConfigSource{
@ -340,6 +379,9 @@ func Test_nodePlugin_Admit(t *testing.T) {
},
}
existingNodesIndex = cache.NewIndexer(cache.MetaNamespaceKeyFunc, nil)
existingNodes = corev1lister.NewNodeLister(existingNodesIndex)
noExistingPodsIndex = cache.NewIndexer(cache.MetaNamespaceKeyFunc, nil)
noExistingPods = corev1lister.NewPodLister(noExistingPodsIndex)
@ -364,6 +406,8 @@ func Test_nodePlugin_Admit(t *testing.T) {
existingPodsIndex.Add(v1otherpod)
existingPodsIndex.Add(v1unboundpod)
existingNodesIndex.Add(&v1.Node{ObjectMeta: mynodeObjMeta})
sapod, _ := makeTestPod("ns", "mysapod", "mynode", true)
sapod.Spec.ServiceAccountName = "foo"
@ -376,13 +420,7 @@ func Test_nodePlugin_Admit(t *testing.T) {
pvcpod, _ := makeTestPod("ns", "mypvcpod", "mynode", true)
pvcpod.Spec.Volumes = []api.Volume{{VolumeSource: api.VolumeSource{PersistentVolumeClaim: &api.PersistentVolumeClaimVolumeSource{ClaimName: "foo"}}}}
tests := []struct {
name string
podsGetter corev1lister.PodLister
attributes admission.Attributes
features featuregate.FeatureGate
err string
}{
tests := []admitTestCase{
// Mirror pods bound to us
{
name: "allow creating a mirror pod bound to self",
@ -1232,21 +1270,125 @@ func Test_nodePlugin_Admit(t *testing.T) {
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
c := NewPlugin(nodeidentifier.NewDefaultNodeIdentifier())
if tt.features != nil {
c.InspectFeatureGates(tt.features)
}
c.podsGetter = tt.podsGetter
err := c.Admit(context.TODO(), tt.attributes, nil)
if (err == nil) != (len(tt.err) == 0) {
t.Errorf("nodePlugin.Admit() error = %v, expected %v", err, tt.err)
return
}
if len(tt.err) > 0 && !strings.Contains(err.Error(), tt.err) {
t.Errorf("nodePlugin.Admit() error = %v, expected %v", err, tt.err)
}
})
tt.nodesGetter = existingNodes
tt.run(t)
}
}
func Test_nodePlugin_Admit_OwnerReference(t *testing.T) {
expectedNodeIndex := cache.NewIndexer(cache.MetaNamespaceKeyFunc, nil)
expectedNodeIndex.Add(&v1.Node{ObjectMeta: metav1.ObjectMeta{Name: "mynode", UID: "mynode-uid"}})
expectedNode := corev1lister.NewNodeLister(expectedNodeIndex)
unexpectedNodeIndex := cache.NewIndexer(cache.MetaNamespaceKeyFunc, nil)
unexpectedNodeIndex.Add(&v1.Node{ObjectMeta: metav1.ObjectMeta{Name: "mynode", UID: "mynode-unexpected-uid"}})
unexpectedNode := corev1lister.NewNodeLister(unexpectedNodeIndex)
noNodesIndex := cache.NewIndexer(cache.MetaNamespaceKeyFunc, nil)
noNodes := corev1lister.NewNodeLister(noNodesIndex)
noExistingPodsIndex := cache.NewIndexer(cache.MetaNamespaceKeyFunc, nil)
noExistingPods := corev1lister.NewPodLister(noExistingPodsIndex)
mynode := &user.DefaultInfo{Name: "system:node:mynode", Groups: []string{"system:nodes"}}
validOwner := metav1.OwnerReference{
APIVersion: "v1",
Kind: "Node",
Name: "mynode",
UID: "mynode-uid",
Controller: pointer.BoolPtr(true),
}
invalidName := validOwner
invalidName.Name = "other"
invalidKind := validOwner
invalidKind.Kind = "Pod"
invalidAPI := validOwner
invalidAPI.APIVersion = "v2"
invalidControllerNil := validOwner
invalidControllerNil.Controller = nil
invalidControllerFalse := validOwner
invalidControllerFalse.Controller = pointer.BoolPtr(false)
invalidBlockDeletion := validOwner
invalidBlockDeletion.BlockOwnerDeletion = pointer.BoolPtr(true)
tests := []struct {
name string
owners []metav1.OwnerReference
nodesGetter corev1lister.NodeLister
expectErr string
}{
{
name: "no owner",
owners: nil,
},
{
name: "valid owner",
owners: []metav1.OwnerReference{validOwner},
},
{
name: "duplicate owner",
owners: []metav1.OwnerReference{validOwner, validOwner},
expectErr: "can only create pods with a single owner reference set to itself",
},
{
name: "invalid name",
owners: []metav1.OwnerReference{invalidName},
expectErr: "can only create pods with an owner reference set to itself",
},
{
name: "invalid UID",
owners: []metav1.OwnerReference{validOwner},
nodesGetter: unexpectedNode,
expectErr: "UID mismatch",
},
{
name: "node not found",
owners: []metav1.OwnerReference{validOwner},
nodesGetter: noNodes,
expectErr: "not found",
},
{
name: "invalid API version",
owners: []metav1.OwnerReference{invalidAPI},
expectErr: "can only create pods with an owner reference set to itself",
},
{
name: "invalid kind",
owners: []metav1.OwnerReference{invalidKind},
expectErr: "can only create pods with an owner reference set to itself",
},
{
name: "nil controller",
owners: []metav1.OwnerReference{invalidControllerNil},
expectErr: "can only create pods with a controller owner reference set to itself",
},
{
name: "false controller",
owners: []metav1.OwnerReference{invalidControllerFalse},
expectErr: "can only create pods with a controller owner reference set to itself",
},
{
name: "invalid blockOwnerDeletion",
owners: []metav1.OwnerReference{invalidBlockDeletion},
expectErr: "must not set blockOwnerDeletion on an owner reference",
},
}
for _, test := range tests {
if test.nodesGetter == nil {
test.nodesGetter = expectedNode
}
pod, _ := makeTestPod("ns", "test", "mynode", true)
pod.OwnerReferences = test.owners
a := &admitTestCase{
name: test.name,
podsGetter: noExistingPods,
nodesGetter: test.nodesGetter,
attributes: createPodAttributes(pod, mynode),
err: test.expectErr,
}
a.run(t)
}
}
@ -1326,3 +1468,9 @@ func Test_getModifiedLabels(t *testing.T) {
})
}
}
func createPodAttributes(pod *api.Pod, user user.Info) admission.Attributes {
podResource := api.Resource("pods").WithVersion("v1")
podKind := api.Kind("Pod").WithVersion("v1")
return admission.NewAttributesRecord(pod, nil, podKind, pod.Namespace, pod.Name, podResource, "", admission.Create, &metav1.CreateOptions{}, false, user)
}