noderestriction admission: lock down create of NodeResourceSlice

The proper value of NodeName must be checked here for create because
the node authorizer cannot do it.
This commit is contained in:
Patrick Ohly 2024-01-24 18:41:21 +01:00
parent 2e34e187c9
commit a92d2a4cea
2 changed files with 120 additions and 13 deletions

View File

@ -18,6 +18,7 @@ package noderestriction
import (
"context"
"errors"
"fmt"
"io"
"strings"
@ -25,7 +26,7 @@ import (
"github.com/google/go-cmp/cmp"
v1 "k8s.io/api/core/v1"
apiequality "k8s.io/apimachinery/pkg/api/equality"
"k8s.io/apimachinery/pkg/api/errors"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/api/meta"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/util/sets"
@ -40,6 +41,7 @@ import (
coordapi "k8s.io/kubernetes/pkg/apis/coordination"
api "k8s.io/kubernetes/pkg/apis/core"
"k8s.io/kubernetes/pkg/apis/policy"
"k8s.io/kubernetes/pkg/apis/resource"
storage "k8s.io/kubernetes/pkg/apis/storage"
"k8s.io/kubernetes/pkg/auth/nodeidentifier"
"k8s.io/kubernetes/pkg/features"
@ -71,7 +73,8 @@ type Plugin struct {
podsGetter corev1lister.PodLister
nodesGetter corev1lister.NodeLister
expansionRecoveryEnabled bool
expansionRecoveryEnabled bool
dynamicResourceAllocationEnabled bool
}
var (
@ -83,6 +86,7 @@ var (
// InspectFeatureGates allows setting bools without taking a dep on a global variable
func (p *Plugin) InspectFeatureGates(featureGates featuregate.FeatureGate) {
p.expansionRecoveryEnabled = featureGates.Enabled(features.RecoverVolumeExpansionFailure)
p.dynamicResourceAllocationEnabled = featureGates.Enabled(features.DynamicResourceAllocation)
}
// SetExternalKubeInformerFactory registers an informer factory into Plugin
@ -106,12 +110,13 @@ func (p *Plugin) ValidateInitialization() error {
}
var (
podResource = api.Resource("pods")
nodeResource = api.Resource("nodes")
pvcResource = api.Resource("persistentvolumeclaims")
svcacctResource = api.Resource("serviceaccounts")
leaseResource = coordapi.Resource("leases")
csiNodeResource = storage.Resource("csinodes")
podResource = api.Resource("pods")
nodeResource = api.Resource("nodes")
pvcResource = api.Resource("persistentvolumeclaims")
svcacctResource = api.Resource("serviceaccounts")
leaseResource = coordapi.Resource("leases")
csiNodeResource = storage.Resource("csinodes")
nodeResourceSliceResource = resource.Resource("noderesourceslices")
)
// Admit checks the admission policy and triggers corresponding actions
@ -163,6 +168,9 @@ func (p *Plugin) Admit(ctx context.Context, a admission.Attributes, o admission.
case csiNodeResource:
return p.admitCSINode(nodeName, a)
case nodeResourceSliceResource:
return p.admitNodeResourceSlice(nodeName, a)
default:
return nil
}
@ -178,7 +186,7 @@ func (p *Plugin) admitPod(nodeName string, a admission.Attributes) error {
case admission.Delete:
// get the existing pod
existingPod, err := p.podsGetter.Pods(a.GetNamespace()).Get(a.GetName())
if errors.IsNotFound(err) {
if apierrors.IsNotFound(err) {
return err
}
if err != nil {
@ -233,7 +241,7 @@ func (p *Plugin) admitPodCreate(nodeName string, a admission.Attributes) error {
// Verify the node UID.
node, err := p.nodesGetter.Get(nodeName)
if errors.IsNotFound(err) {
if apierrors.IsNotFound(err) {
return err
}
if err != nil {
@ -351,7 +359,7 @@ func (p *Plugin) admitPodEviction(nodeName string, a admission.Attributes) error
}
// get the existing pod
existingPod, err := p.podsGetter.Pods(a.GetNamespace()).Get(podName)
if errors.IsNotFound(err) {
if apierrors.IsNotFound(err) {
return err
}
if err != nil {
@ -412,7 +420,7 @@ func (p *Plugin) admitPVCStatus(nodeName string, a admission.Attributes) error {
// ensure no metadata changed. nodes should not be able to relabel, add finalizers/owners, etc
if !apiequality.Semantic.DeepEqual(oldPVC, newPVC) {
return admission.NewForbidden(a, fmt.Errorf("node %q is not allowed to update fields other than status.capacity and status.conditions: %v", nodeName, cmp.Diff(oldPVC, newPVC)))
return admission.NewForbidden(a, fmt.Errorf("node %q is not allowed to update fields other than status.quantity and status.conditions: %v", nodeName, cmp.Diff(oldPVC, newPVC)))
}
return nil
@ -564,7 +572,7 @@ func (p *Plugin) admitServiceAccount(nodeName string, a admission.Attributes) er
return admission.NewForbidden(a, fmt.Errorf("node requested token with a pod binding without a uid"))
}
pod, err := p.podsGetter.Pods(a.GetNamespace()).Get(ref.Name)
if errors.IsNotFound(err) {
if apierrors.IsNotFound(err) {
return err
}
if err != nil {
@ -630,3 +638,20 @@ func (p *Plugin) admitCSINode(nodeName string, a admission.Attributes) error {
return nil
}
func (p *Plugin) admitNodeResourceSlice(nodeName string, a admission.Attributes) error {
// The create request must come from a node with the same name as the NodeName field.
// Other requests gets checked by the node authorizer.
if a.GetOperation() == admission.Create {
slice, ok := a.GetObject().(*resource.NodeResourceSlice)
if !ok {
return admission.NewForbidden(a, fmt.Errorf("unexpected type %T", a.GetObject()))
}
if slice.NodeName != nodeName {
return admission.NewForbidden(a, errors.New("can only create NodeResourceSlice with the same NodeName as the requesting node"))
}
}
return nil
}

View File

@ -44,6 +44,7 @@ import (
"k8s.io/kubernetes/pkg/apis/coordination"
api "k8s.io/kubernetes/pkg/apis/core"
"k8s.io/kubernetes/pkg/apis/policy"
resourceapi "k8s.io/kubernetes/pkg/apis/resource"
storage "k8s.io/kubernetes/pkg/apis/storage"
"k8s.io/kubernetes/pkg/auth/nodeidentifier"
"k8s.io/utils/pointer"
@ -1601,3 +1602,84 @@ func createPodAttributes(pod *api.Pod, user user.Info) admission.Attributes {
podKind := api.Kind("Pod").WithVersion("v1")
return admission.NewAttributesRecord(pod, nil, podKind, pod.Namespace, pod.Name, podResource, "", admission.Create, &metav1.CreateOptions{}, false, user)
}
func TestAdmitNodeResourceSlice(t *testing.T) {
apiResource := resourceapi.SchemeGroupVersion.WithResource("noderesourceslices")
nodename := "mynode"
mynode := &user.DefaultInfo{Name: "system:node:" + nodename, Groups: []string{"system:nodes"}}
err := "can only create NodeResourceSlice with the same NodeName as the requesting node"
sliceNode := &resourceapi.NodeResourceSlice{
ObjectMeta: metav1.ObjectMeta{
Name: "something",
},
NodeName: nodename,
}
sliceOtherNode := &resourceapi.NodeResourceSlice{
ObjectMeta: metav1.ObjectMeta{
Name: "something",
},
NodeName: nodename + "-other",
}
tests := map[string]struct {
operation admission.Operation
obj runtime.Object
featureEnabled bool
expectError string
}{
"create allowed, enabled": {
operation: admission.Create,
obj: sliceNode,
featureEnabled: true,
expectError: "",
},
"create disallowed, enabled": {
operation: admission.Create,
obj: sliceOtherNode,
featureEnabled: true,
expectError: err,
},
"create allowed, disabled": {
operation: admission.Create,
obj: sliceNode,
featureEnabled: false,
expectError: "",
},
"create disallowed, disabled": {
operation: admission.Create,
obj: sliceOtherNode,
featureEnabled: false,
expectError: err,
},
"update allowed, same node": {
operation: admission.Update,
obj: sliceNode,
featureEnabled: true,
expectError: "",
},
"update allowed, other node": {
operation: admission.Update,
obj: sliceOtherNode,
featureEnabled: true,
expectError: "",
},
}
for name, test := range tests {
t.Run(name, func(t *testing.T) {
attributes := admission.NewAttributesRecord(
test.obj, nil, schema.GroupVersionKind{},
"", "foo", apiResource, "", test.operation, &metav1.CreateOptions{}, false, mynode)
defer featuregatetesting.SetFeatureGateDuringTest(t, feature.DefaultFeatureGate, features.DynamicResourceAllocation, test.featureEnabled)()
a := &admitTestCase{
name: name,
attributes: attributes,
features: feature.DefaultFeatureGate,
err: test.expectError,
}
a.run(t)
})
}
}