Skip to content

Commit 581d3e2

Browse files
tallclairk8s-ci-robot
authored andcommitted
Restrict mirror pod owner references (kubernetes#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
1 parent 3202bc1 commit 581d3e2

File tree

2 files changed

+250
-62
lines changed

2 files changed

+250
-62
lines changed

plugin/pkg/admission/noderestriction/admission.go

Lines changed: 77 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ type Plugin struct {
7070
*admission.Handler
7171
nodeIdentifier nodeidentifier.NodeIdentifier
7272
podsGetter corev1lister.PodLister
73+
nodesGetter corev1lister.NodeLister
7374

7475
tokenRequestEnabled bool
7576
csiNodeInfoEnabled bool
@@ -92,6 +93,7 @@ func (p *Plugin) InspectFeatureGates(featureGates featuregate.FeatureGate) {
9293
// SetExternalKubeInformerFactory registers an informer factory into Plugin
9394
func (p *Plugin) SetExternalKubeInformerFactory(f informers.SharedInformerFactory) {
9495
p.podsGetter = f.Core().V1().Pods().Lister()
96+
p.nodesGetter = f.Core().V1().Nodes().Lister()
9597
}
9698

9799
// ValidateInitialization validates the Plugin was initialized properly
@@ -102,6 +104,9 @@ func (p *Plugin) ValidateInitialization() error {
102104
if p.podsGetter == nil {
103105
return fmt.Errorf("%s requires a pod getter", PluginName)
104106
}
107+
if p.nodesGetter == nil {
108+
return fmt.Errorf("%s requires a node getter", PluginName)
109+
}
105110
return nil
106111
}
107112

@@ -128,6 +133,8 @@ func (p *Plugin) Admit(ctx context.Context, a admission.Attributes, o admission.
128133
return admission.NewForbidden(a, fmt.Errorf("could not determine node from user %q", a.GetUserInfo().GetName()))
129134
}
130135

136+
// TODO: if node doesn't exist and this isn't a create node request, then reject.
137+
131138
switch a.GetResource().GroupResource() {
132139
case podResource:
133140
switch a.GetSubresource() {
@@ -177,43 +184,7 @@ func (p *Plugin) Admit(ctx context.Context, a admission.Attributes, o admission.
177184
func (p *Plugin) admitPod(nodeName string, a admission.Attributes) error {
178185
switch a.GetOperation() {
179186
case admission.Create:
180-
// require a pod object
181-
pod, ok := a.GetObject().(*api.Pod)
182-
if !ok {
183-
return admission.NewForbidden(a, fmt.Errorf("unexpected type %T", a.GetObject()))
184-
}
185-
186-
// only allow nodes to create mirror pods
187-
if _, isMirrorPod := pod.Annotations[api.MirrorPodAnnotationKey]; !isMirrorPod {
188-
return admission.NewForbidden(a, fmt.Errorf("pod does not have %q annotation, node %q can only create mirror pods", api.MirrorPodAnnotationKey, nodeName))
189-
}
190-
191-
// only allow nodes to create a pod bound to itself
192-
if pod.Spec.NodeName != nodeName {
193-
return admission.NewForbidden(a, fmt.Errorf("node %q can only create pods with spec.nodeName set to itself", nodeName))
194-
}
195-
196-
// don't allow a node to create a pod that references any other API objects
197-
if pod.Spec.ServiceAccountName != "" {
198-
return admission.NewForbidden(a, fmt.Errorf("node %q can not create pods that reference a service account", nodeName))
199-
}
200-
hasSecrets := false
201-
podutil.VisitPodSecretNames(pod, func(name string) (shouldContinue bool) { hasSecrets = true; return false })
202-
if hasSecrets {
203-
return admission.NewForbidden(a, fmt.Errorf("node %q can not create pods that reference secrets", nodeName))
204-
}
205-
hasConfigMaps := false
206-
podutil.VisitPodConfigmapNames(pod, func(name string) (shouldContinue bool) { hasConfigMaps = true; return false })
207-
if hasConfigMaps {
208-
return admission.NewForbidden(a, fmt.Errorf("node %q can not create pods that reference configmaps", nodeName))
209-
}
210-
for _, v := range pod.Spec.Volumes {
211-
if v.PersistentVolumeClaim != nil {
212-
return admission.NewForbidden(a, fmt.Errorf("node %q can not create pods that reference persistentvolumeclaims", nodeName))
213-
}
214-
}
215-
216-
return nil
187+
return p.admitPodCreate(nodeName, a)
217188

218189
case admission.Delete:
219190
// get the existing pod
@@ -235,6 +206,75 @@ func (p *Plugin) admitPod(nodeName string, a admission.Attributes) error {
235206
}
236207
}
237208

209+
func (p *Plugin) admitPodCreate(nodeName string, a admission.Attributes) error {
210+
// require a pod object
211+
pod, ok := a.GetObject().(*api.Pod)
212+
if !ok {
213+
return admission.NewForbidden(a, fmt.Errorf("unexpected type %T", a.GetObject()))
214+
}
215+
216+
// only allow nodes to create mirror pods
217+
if _, isMirrorPod := pod.Annotations[api.MirrorPodAnnotationKey]; !isMirrorPod {
218+
return admission.NewForbidden(a, fmt.Errorf("pod does not have %q annotation, node %q can only create mirror pods", api.MirrorPodAnnotationKey, nodeName))
219+
}
220+
221+
// only allow nodes to create a pod bound to itself
222+
if pod.Spec.NodeName != nodeName {
223+
return admission.NewForbidden(a, fmt.Errorf("node %q can only create pods with spec.nodeName set to itself", nodeName))
224+
}
225+
if len(pod.OwnerReferences) > 1 {
226+
return admission.NewForbidden(a, fmt.Errorf("node %q can only create pods with a single owner reference set to itself", nodeName))
227+
}
228+
if len(pod.OwnerReferences) == 1 {
229+
owner := pod.OwnerReferences[0]
230+
if owner.APIVersion != v1.SchemeGroupVersion.String() ||
231+
owner.Kind != "Node" ||
232+
owner.Name != nodeName {
233+
return admission.NewForbidden(a, fmt.Errorf("node %q can only create pods with an owner reference set to itself", nodeName))
234+
}
235+
if owner.Controller == nil || !*owner.Controller {
236+
return admission.NewForbidden(a, fmt.Errorf("node %q can only create pods with a controller owner reference set to itself", nodeName))
237+
}
238+
if owner.BlockOwnerDeletion != nil && *owner.BlockOwnerDeletion {
239+
return admission.NewForbidden(a, fmt.Errorf("node %q must not set blockOwnerDeletion on an owner reference", nodeName))
240+
}
241+
242+
// Verify the node UID.
243+
node, err := p.nodesGetter.Get(nodeName)
244+
if errors.IsNotFound(err) {
245+
return err
246+
}
247+
if err != nil {
248+
return admission.NewForbidden(a, fmt.Errorf("error looking up node %s to verify uid: %v", nodeName, err))
249+
}
250+
if owner.UID != node.UID {
251+
return admission.NewForbidden(a, fmt.Errorf("node %s UID mismatch: expected %s got %s", nodeName, owner.UID, node.UID))
252+
}
253+
}
254+
255+
// don't allow a node to create a pod that references any other API objects
256+
if pod.Spec.ServiceAccountName != "" {
257+
return admission.NewForbidden(a, fmt.Errorf("node %q can not create pods that reference a service account", nodeName))
258+
}
259+
hasSecrets := false
260+
podutil.VisitPodSecretNames(pod, func(name string) (shouldContinue bool) { hasSecrets = true; return false })
261+
if hasSecrets {
262+
return admission.NewForbidden(a, fmt.Errorf("node %q can not create pods that reference secrets", nodeName))
263+
}
264+
hasConfigMaps := false
265+
podutil.VisitPodConfigmapNames(pod, func(name string) (shouldContinue bool) { hasConfigMaps = true; return false })
266+
if hasConfigMaps {
267+
return admission.NewForbidden(a, fmt.Errorf("node %q can not create pods that reference configmaps", nodeName))
268+
}
269+
for _, v := range pod.Spec.Volumes {
270+
if v.PersistentVolumeClaim != nil {
271+
return admission.NewForbidden(a, fmt.Errorf("node %q can not create pods that reference persistentvolumeclaims", nodeName))
272+
}
273+
}
274+
275+
return nil
276+
}
277+
238278
// admitPodStatus allows to update the status of a pod if it is
239279
// assigned to the current node.
240280
func (p *Plugin) admitPodStatus(nodeName string, a admission.Attributes) error {

0 commit comments

Comments
 (0)