Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve node restriction message #48748

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
26 changes: 13 additions & 13 deletions plugin/pkg/admission/noderestriction/admission.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ func (c *nodePlugin) Admit(a admission.Attributes) error {

if len(nodeName) == 0 {
// disallow requests we cannot match to a particular node
return admission.NewForbidden(a, fmt.Errorf("could not determine node from user %s", a.GetUserInfo().GetName()))
return admission.NewForbidden(a, fmt.Errorf("could not determine node from user %q", a.GetUserInfo().GetName()))
}

switch a.GetResource().GroupResource() {
Expand All @@ -103,7 +103,7 @@ func (c *nodePlugin) Admit(a admission.Attributes) error {
case "status":
return c.admitPodStatus(nodeName, a)
default:
return admission.NewForbidden(a, fmt.Errorf("unexpected pod subresource %s", a.GetSubresource()))
return admission.NewForbidden(a, fmt.Errorf("unexpected pod subresource %q", a.GetSubresource()))
}

case nodeResource:
Expand All @@ -125,31 +125,31 @@ func (c *nodePlugin) admitPod(nodeName string, a admission.Attributes) error {

// 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 %s can only create mirror pods", api.MirrorPodAnnotationKey, nodeName))
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 %s can only create pods with spec.nodeName set to itself", 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 %s can not create pods that reference a service account", nodeName))
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 %s can not create pods that reference secrets", nodeName))
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 %s can not create pods that reference configmaps", nodeName))
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 %s can not create pods that reference persistentvolumeclaims", nodeName))
return admission.NewForbidden(a, fmt.Errorf("node %q can not create pods that reference persistentvolumeclaims", nodeName))
}
}

Expand All @@ -167,12 +167,12 @@ func (c *nodePlugin) admitPod(nodeName string, a admission.Attributes) error {
}
// only allow a node to delete a pod bound to itself
if existingPod.Spec.NodeName != nodeName {
return admission.NewForbidden(a, fmt.Errorf("node %s can only delete pods with spec.nodeName set to itself", nodeName))
return admission.NewForbidden(a, fmt.Errorf("node %q can only delete pods with spec.nodeName set to itself", nodeName))
}
return nil

default:
return admission.NewForbidden(a, fmt.Errorf("unexpected operation %s", a.GetOperation()))
return admission.NewForbidden(a, fmt.Errorf("unexpected operation %q", a.GetOperation()))
}
}

Expand All @@ -186,12 +186,12 @@ func (c *nodePlugin) admitPodStatus(nodeName string, a admission.Attributes) err
}
// only allow a node to update status of a pod bound to itself
if pod.Spec.NodeName != nodeName {
return admission.NewForbidden(a, fmt.Errorf("node %s can only update pod status for pods with spec.nodeName set to itself", nodeName))
return admission.NewForbidden(a, fmt.Errorf("node %q can only update pod status for pods with spec.nodeName set to itself", nodeName))
}
return nil

default:
return admission.NewForbidden(a, fmt.Errorf("unexpected operation %s", a.GetOperation()))
return admission.NewForbidden(a, fmt.Errorf("unexpected operation %q", a.GetOperation()))
}
}

Expand All @@ -208,7 +208,7 @@ func (c *nodePlugin) admitNode(nodeName string, a admission.Attributes) error {
}

if requestedName != nodeName {
return admission.NewForbidden(a, fmt.Errorf("node %s cannot modify node %s", nodeName, requestedName))
return admission.NewForbidden(a, fmt.Errorf("node %q cannot modify node %q", nodeName, requestedName))
}
return nil
}
8 changes: 4 additions & 4 deletions plugin/pkg/auth/authorizer/node/node_authorizer.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ func (r *NodeAuthorizer) authorizeGet(nodeName string, startingType vertexType,
return false, "no path found to object", nil
}
if !ok {
glog.V(2).Infof("NODE DENY: %s %#v", nodeName, attrs)
glog.V(2).Infof("NODE DENY: %q %#v", nodeName, attrs)
return false, "no path found to object", nil
}
return ok, "", nil
Expand All @@ -126,12 +126,12 @@ func (r *NodeAuthorizer) hasPathFrom(nodeName string, startingType vertexType, s

nodeVertex, exists := r.graph.getVertex_rlocked(nodeVertexType, "", nodeName)
if !exists {
return false, fmt.Errorf("unknown node %s cannot get %s %s/%s", nodeName, vertexTypes[startingType], startingNamespace, startingName)
return false, fmt.Errorf("unknown node %q cannot get %s %s/%s", nodeName, vertexTypes[startingType], startingNamespace, startingName)
}

startingVertex, exists := r.graph.getVertex_rlocked(startingType, startingNamespace, startingName)
if !exists {
return false, fmt.Errorf("node %s cannot get unknown %s %s/%s", nodeName, vertexTypes[startingType], startingNamespace, startingName)
return false, fmt.Errorf("node %q cannot get unknown %s %s/%s", nodeName, vertexTypes[startingType], startingNamespace, startingName)
}

found := false
Expand All @@ -158,7 +158,7 @@ func (r *NodeAuthorizer) hasPathFrom(nodeName string, startingType vertexType, s
return found
})
if !found {
return false, fmt.Errorf("node %s cannot get %s %s/%s, no path was found", nodeName, vertexTypes[startingType], startingNamespace, startingName)
return false, fmt.Errorf("node %q cannot get %s %s/%s, no path was found", nodeName, vertexTypes[startingType], startingNamespace, startingName)
}
return true, nil
}