diff --git a/plugin/pkg/admission/noderestriction/BUILD b/plugin/pkg/admission/noderestriction/BUILD index f909885429e..8d2777ea41f 100644 --- a/plugin/pkg/admission/noderestriction/BUILD +++ b/plugin/pkg/admission/noderestriction/BUILD @@ -17,11 +17,14 @@ go_library( "//pkg/auth/nodeidentifier:go_default_library", "//pkg/client/clientset_generated/internalclientset:go_default_library", "//pkg/client/clientset_generated/internalclientset/typed/core/internalversion:go_default_library", + "//pkg/features:go_default_library", "//pkg/kubeapiserver/admission:go_default_library", "//vendor/k8s.io/apimachinery/pkg/api/equality:go_default_library", "//vendor/k8s.io/apimachinery/pkg/api/errors:go_default_library", "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", + "//vendor/k8s.io/apimachinery/pkg/util/diff:go_default_library", "//vendor/k8s.io/apiserver/pkg/admission:go_default_library", + "//vendor/k8s.io/apiserver/pkg/util/feature:go_default_library", ], ) diff --git a/plugin/pkg/admission/noderestriction/admission.go b/plugin/pkg/admission/noderestriction/admission.go index 71feec8c3d4..d958dad3762 100644 --- a/plugin/pkg/admission/noderestriction/admission.go +++ b/plugin/pkg/admission/noderestriction/admission.go @@ -23,13 +23,16 @@ import ( apiequality "k8s.io/apimachinery/pkg/api/equality" "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/diff" "k8s.io/apiserver/pkg/admission" + utilfeature "k8s.io/apiserver/pkg/util/feature" podutil "k8s.io/kubernetes/pkg/api/pod" api "k8s.io/kubernetes/pkg/apis/core" "k8s.io/kubernetes/pkg/apis/policy" "k8s.io/kubernetes/pkg/auth/nodeidentifier" "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset" coreinternalversion "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/typed/core/internalversion" + "k8s.io/kubernetes/pkg/features" kubeapiserveradmission "k8s.io/kubernetes/pkg/kubeapiserver/admission" ) @@ -82,6 +85,7 @@ func (p *nodePlugin) ValidateInitialization() error { var ( podResource = api.Resource("pods") nodeResource = api.Resource("nodes") + pvcResource = api.Resource("persistentvolumeclaims") ) func (c *nodePlugin) Admit(a admission.Attributes) error { @@ -113,6 +117,14 @@ func (c *nodePlugin) Admit(a admission.Attributes) error { case nodeResource: return c.admitNode(nodeName, a) + case pvcResource: + switch a.GetSubresource() { + case "status": + return c.admitPVCStatus(nodeName, a) + default: + return admission.NewForbidden(a, fmt.Errorf("may only update PVC status")) + } + default: return nil } @@ -189,7 +201,7 @@ func (c *nodePlugin) admitPodStatus(nodeName string, a admission.Attributes) err // require an existing pod pod, ok := a.GetOldObject().(*api.Pod) if !ok { - return admission.NewForbidden(a, fmt.Errorf("unexpected type %T", a.GetObject())) + return admission.NewForbidden(a, fmt.Errorf("unexpected type %T", a.GetOldObject())) } // only allow a node to update status of a pod bound to itself if pod.Spec.NodeName != nodeName { @@ -241,6 +253,50 @@ func (c *nodePlugin) admitPodEviction(nodeName string, a admission.Attributes) e } } +func (c *nodePlugin) admitPVCStatus(nodeName string, a admission.Attributes) error { + switch a.GetOperation() { + case admission.Update: + if !utilfeature.DefaultFeatureGate.Enabled(features.ExpandPersistentVolumes) { + return admission.NewForbidden(a, fmt.Errorf("node %q may not update persistentvolumeclaim metadata", nodeName)) + } + + oldPVC, ok := a.GetOldObject().(*api.PersistentVolumeClaim) + if !ok { + return admission.NewForbidden(a, fmt.Errorf("unexpected type %T", a.GetOldObject())) + } + + newPVC, ok := a.GetObject().(*api.PersistentVolumeClaim) + if !ok { + return admission.NewForbidden(a, fmt.Errorf("unexpected type %T", a.GetObject())) + } + + // make copies for comparison + oldPVC = oldPVC.DeepCopy() + newPVC = newPVC.DeepCopy() + + // zero out resourceVersion to avoid comparing differences, + // since the new object could leave it empty to indicate an unconditional update + oldPVC.ObjectMeta.ResourceVersion = "" + newPVC.ObjectMeta.ResourceVersion = "" + + oldPVC.Status.Capacity = nil + newPVC.Status.Capacity = nil + + oldPVC.Status.Conditions = nil + newPVC.Status.Conditions = nil + + // 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 may not update fields other than status.capacity and status.conditions: %v", nodeName, diff.ObjectReflectDiff(oldPVC, newPVC))) + } + + return nil + + default: + return admission.NewForbidden(a, fmt.Errorf("unexpected operation %q", a.GetOperation())) + } +} + func (c *nodePlugin) admitNode(nodeName string, a admission.Attributes) error { requestedName := a.GetName() if a.GetOperation() == admission.Create { diff --git a/plugin/pkg/auth/authorizer/node/BUILD b/plugin/pkg/auth/authorizer/node/BUILD index 1dbbdb2458b..46bb915ba8a 100644 --- a/plugin/pkg/auth/authorizer/node/BUILD +++ b/plugin/pkg/auth/authorizer/node/BUILD @@ -36,6 +36,7 @@ go_library( "//pkg/apis/rbac:go_default_library", "//pkg/auth/nodeidentifier:go_default_library", "//pkg/client/informers/informers_generated/internalversion/core/internalversion:go_default_library", + "//pkg/features:go_default_library", "//plugin/pkg/auth/authorizer/rbac:go_default_library", "//third_party/forked/gonum/graph:go_default_library", "//third_party/forked/gonum/graph/simple:go_default_library", @@ -43,6 +44,7 @@ go_library( "//vendor/github.com/golang/glog:go_default_library", "//vendor/k8s.io/apimachinery/pkg/runtime/schema:go_default_library", "//vendor/k8s.io/apiserver/pkg/authorization/authorizer:go_default_library", + "//vendor/k8s.io/apiserver/pkg/util/feature:go_default_library", "//vendor/k8s.io/client-go/tools/cache:go_default_library", ], ) diff --git a/plugin/pkg/auth/authorizer/node/node_authorizer.go b/plugin/pkg/auth/authorizer/node/node_authorizer.go index 62ab4a5b086..bf728ccc5e6 100644 --- a/plugin/pkg/auth/authorizer/node/node_authorizer.go +++ b/plugin/pkg/auth/authorizer/node/node_authorizer.go @@ -23,9 +23,11 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apiserver/pkg/authorization/authorizer" + utilfeature "k8s.io/apiserver/pkg/util/feature" api "k8s.io/kubernetes/pkg/apis/core" rbacapi "k8s.io/kubernetes/pkg/apis/rbac" "k8s.io/kubernetes/pkg/auth/nodeidentifier" + "k8s.io/kubernetes/pkg/features" "k8s.io/kubernetes/plugin/pkg/auth/authorizer/rbac" "k8s.io/kubernetes/third_party/forked/gonum/graph" "k8s.io/kubernetes/third_party/forked/gonum/graph/traverse" @@ -85,6 +87,11 @@ func (r *NodeAuthorizer) Authorize(attrs authorizer.Attributes) (authorizer.Deci case configMapResource: return r.authorizeGet(nodeName, configMapVertexType, attrs) case pvcResource: + if utilfeature.DefaultFeatureGate.Enabled(features.ExpandPersistentVolumes) { + if attrs.GetSubresource() == "status" { + return r.authorizeStatusUpdate(nodeName, pvcVertexType, attrs) + } + } return r.authorizeGet(nodeName, pvcVertexType, attrs) case pvResource: return r.authorizeGet(nodeName, pvVertexType, attrs) @@ -98,17 +105,42 @@ func (r *NodeAuthorizer) Authorize(attrs authorizer.Attributes) (authorizer.Deci return authorizer.DecisionNoOpinion, "", nil } +// authorizeStatusUpdate authorizes get/update/patch requests to status subresources of the specified type if they are related to the specified node +func (r *NodeAuthorizer) authorizeStatusUpdate(nodeName string, startingType vertexType, attrs authorizer.Attributes) (authorizer.Decision, string, error) { + switch attrs.GetVerb() { + case "update", "patch": + // ok + default: + glog.V(2).Infof("NODE DENY: %s %#v", nodeName, attrs) + return authorizer.DecisionNoOpinion, "can only get/update/patch this type", nil + } + + if attrs.GetSubresource() != "status" { + glog.V(2).Infof("NODE DENY: %s %#v", nodeName, attrs) + return authorizer.DecisionNoOpinion, "can only update status subresource", nil + } + + return r.authorize(nodeName, startingType, attrs) +} + // authorizeGet authorizes "get" requests to objects of the specified type if they are related to the specified node func (r *NodeAuthorizer) authorizeGet(nodeName string, startingType vertexType, attrs authorizer.Attributes) (authorizer.Decision, string, error) { - if attrs.GetVerb() != "get" || len(attrs.GetName()) == 0 { + if attrs.GetVerb() != "get" { glog.V(2).Infof("NODE DENY: %s %#v", nodeName, attrs) return authorizer.DecisionNoOpinion, "can only get individual resources of this type", nil } - if len(attrs.GetSubresource()) > 0 { glog.V(2).Infof("NODE DENY: %s %#v", nodeName, attrs) return authorizer.DecisionNoOpinion, "cannot get subresource", nil } + return r.authorize(nodeName, startingType, attrs) +} + +func (r *NodeAuthorizer) authorize(nodeName string, startingType vertexType, attrs authorizer.Attributes) (authorizer.Decision, string, error) { + if len(attrs.GetName()) == 0 { + glog.V(2).Infof("NODE DENY: %s %#v", nodeName, attrs) + return authorizer.DecisionNoOpinion, "No Object name found", nil + } ok, err := r.hasPathFrom(nodeName, startingType, attrs.GetNamespace(), attrs.GetName()) if err != nil { diff --git a/plugin/pkg/auth/authorizer/rbac/bootstrappolicy/policy.go b/plugin/pkg/auth/authorizer/rbac/bootstrappolicy/policy.go index 29bd87f2a76..43a898ae2fa 100644 --- a/plugin/pkg/auth/authorizer/rbac/bootstrappolicy/policy.go +++ b/plugin/pkg/auth/authorizer/rbac/bootstrappolicy/policy.go @@ -91,7 +91,7 @@ func addClusterRoleBindingLabel(rolebindings []rbac.ClusterRoleBinding) { } func NodeRules() []rbac.PolicyRule { - return []rbac.PolicyRule{ + nodePolicyRules := []rbac.PolicyRule{ // Needed to check API access. These creates are non-mutating rbac.NewRule("create").Groups(authenticationGroup).Resources("tokenreviews").RuleOrDie(), rbac.NewRule("create").Groups(authorizationGroup).Resources("subjectaccessreviews", "localsubjectaccessreviews").RuleOrDie(), @@ -123,11 +123,12 @@ func NodeRules() []rbac.PolicyRule { // Needed for imagepullsecrets, rbd/ceph and secret volumes, and secrets in envs // Needed for configmap volume and envs - // Use the NodeRestriction admission plugin to limit a node to get secrets/configmaps referenced by pods bound to itself. + // Use the Node authorization mode to limit a node to get secrets/configmaps referenced by pods bound to itself. rbac.NewRule("get").Groups(legacyGroup).Resources("secrets", "configmaps").RuleOrDie(), // Needed for persistent volumes - // Use the NodeRestriction admission plugin to limit a node to get pv/pvc objects referenced by pods bound to itself. + // Use the Node authorization mode to limit a node to get pv/pvc objects referenced by pods bound to itself. rbac.NewRule("get").Groups(legacyGroup).Resources("persistentvolumeclaims", "persistentvolumes").RuleOrDie(), + // TODO: add to the Node authorizer and restrict to endpoints referenced by pods or PVs bound to the node // Needed for glusterfs volumes rbac.NewRule("get").Groups(legacyGroup).Resources("endpoints").RuleOrDie(), @@ -135,6 +136,14 @@ func NodeRules() []rbac.PolicyRule { // for it to be signed. This allows the kubelet to rotate it's own certificate. rbac.NewRule("create", "get", "list", "watch").Groups(certificatesGroup).Resources("certificatesigningrequests").RuleOrDie(), } + + if utilfeature.DefaultFeatureGate.Enabled(features.ExpandPersistentVolumes) { + // Use the Node authorization mode to limit a node to update status of pvc objects referenced by pods bound to itself. + // Use the NodeRestriction admission plugin to limit a node to just update the status stanza. + pvcStatusPolicyRule := rbac.NewRule("get", "update", "patch").Groups(legacyGroup).Resources("persistentvolumeclaims/status").RuleOrDie() + nodePolicyRules = append(nodePolicyRules, pvcStatusPolicyRule) + } + return nodePolicyRules } // ClusterRoles returns the cluster roles to bootstrap an API server with diff --git a/test/integration/auth/BUILD b/test/integration/auth/BUILD index c91ff5fd669..ccd4ec402c4 100644 --- a/test/integration/auth/BUILD +++ b/test/integration/auth/BUILD @@ -32,6 +32,7 @@ go_test( "//pkg/bootstrap/api:go_default_library", "//pkg/client/clientset_generated/internalclientset:go_default_library", "//pkg/client/informers/informers_generated/internalversion:go_default_library", + "//pkg/features:go_default_library", "//pkg/kubeapiserver/authorizer:go_default_library", "//pkg/master:go_default_library", "//pkg/registry/rbac/clusterrole:go_default_library", @@ -56,6 +57,7 @@ go_test( "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//vendor/k8s.io/apimachinery/pkg/labels:go_default_library", "//vendor/k8s.io/apimachinery/pkg/runtime/schema:go_default_library", + "//vendor/k8s.io/apimachinery/pkg/types:go_default_library", "//vendor/k8s.io/apimachinery/pkg/watch:go_default_library", "//vendor/k8s.io/apiserver/pkg/authentication/authenticator:go_default_library", "//vendor/k8s.io/apiserver/pkg/authentication/group:go_default_library", @@ -66,6 +68,8 @@ go_test( "//vendor/k8s.io/apiserver/pkg/authorization/authorizer:go_default_library", "//vendor/k8s.io/apiserver/pkg/authorization/authorizerfactory:go_default_library", "//vendor/k8s.io/apiserver/pkg/registry/generic:go_default_library", + "//vendor/k8s.io/apiserver/pkg/util/feature:go_default_library", + "//vendor/k8s.io/apiserver/pkg/util/feature/testing:go_default_library", "//vendor/k8s.io/apiserver/plugin/pkg/authenticator/token/tokentest:go_default_library", "//vendor/k8s.io/apiserver/plugin/pkg/authenticator/token/webhook:go_default_library", "//vendor/k8s.io/client-go/rest:go_default_library", diff --git a/test/integration/auth/node_test.go b/test/integration/auth/node_test.go index 4fab8257654..32a699b12c4 100644 --- a/test/integration/auth/node_test.go +++ b/test/integration/auth/node_test.go @@ -17,6 +17,7 @@ limitations under the License. package auth import ( + "fmt" "net/http" "net/http/httptest" "path/filepath" @@ -27,9 +28,12 @@ import ( "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" "k8s.io/apiserver/pkg/authentication/request/bearertoken" "k8s.io/apiserver/pkg/authentication/token/tokenfile" "k8s.io/apiserver/pkg/authentication/user" + utilfeature "k8s.io/apiserver/pkg/util/feature" + utilfeaturetesting "k8s.io/apiserver/pkg/util/feature/testing" restclient "k8s.io/client-go/rest" "k8s.io/kubernetes/pkg/api/legacyscheme" api "k8s.io/kubernetes/pkg/apis/core" @@ -37,6 +41,7 @@ import ( "k8s.io/kubernetes/pkg/auth/nodeidentifier" clientset "k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset" informers "k8s.io/kubernetes/pkg/client/informers/informers_generated/internalversion" + "k8s.io/kubernetes/pkg/features" "k8s.io/kubernetes/pkg/kubeapiserver/authorizer" "k8s.io/kubernetes/plugin/pkg/admission/noderestriction" "k8s.io/kubernetes/test/integration/framework" @@ -131,6 +136,7 @@ func TestNodeAuthorizer(t *testing.T) { }); err != nil { t.Fatal(err) } + if _, err := superuserClient.Core().PersistentVolumes().Create(&api.PersistentVolume{ ObjectMeta: metav1.ObjectMeta{Name: "mypv"}, Spec: api.PersistentVolumeSpec{ @@ -249,6 +255,21 @@ func TestNodeAuthorizer(t *testing.T) { }) } + capacity := 50 + updatePVCCapacity := func(client clientset.Interface) error { + capacity++ + statusString := fmt.Sprintf("{\"status\": {\"capacity\": {\"storage\": \"%dG\"}}}", capacity) + patchBytes := []byte(statusString) + _, err := client.Core().PersistentVolumeClaims("ns").Patch("mypvc", types.StrategicMergePatchType, patchBytes, "status") + return err + } + + updatePVCPhase := func(client clientset.Interface) error { + patchBytes := []byte(`{"status":{"phase": "Bound"}}`) + _, err := client.Core().PersistentVolumeClaims("ns").Patch("mypvc", types.StrategicMergePatchType, patchBytes, "status") + return err + } + nodeanonClient := clientsetForToken(tokenNodeUnknown, clientConfig) node1Client := clientsetForToken(tokenNode1, clientConfig) node2Client := clientsetForToken(tokenNode2, clientConfig) @@ -287,6 +308,7 @@ func TestNodeAuthorizer(t *testing.T) { expectForbidden(t, getConfigMap(node2Client)) expectForbidden(t, getPVC(node2Client)) expectForbidden(t, getPV(node2Client)) + expectForbidden(t, createNode2NormalPod(nodeanonClient)) // mirror pod and self node lifecycle is allowed expectAllowed(t, createNode2MirrorPod(node2Client)) @@ -333,6 +355,7 @@ func TestNodeAuthorizer(t *testing.T) { expectAllowed(t, getConfigMap(node2Client)) expectAllowed(t, getPVC(node2Client)) expectAllowed(t, getPV(node2Client)) + expectForbidden(t, createNode2NormalPod(node2Client)) expectAllowed(t, updateNode2NormalPodStatus(node2Client)) expectAllowed(t, deleteNode2NormalPod(node2Client)) @@ -343,6 +366,24 @@ func TestNodeAuthorizer(t *testing.T) { expectAllowed(t, createNode2MirrorPod(superuserClient)) expectAllowed(t, createNode2NormalPodEviction(node2Client)) expectAllowed(t, createNode2MirrorPodEviction(node2Client)) + + // re-create a pod as an admin to add object references + expectAllowed(t, createNode2NormalPod(superuserClient)) + // With ExpandPersistentVolumes feature disabled + defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ExpandPersistentVolumes, false)() + // node->pvc relationship not established + expectForbidden(t, updatePVCCapacity(node1Client)) + // node->pvc relationship established but feature is disabled + expectForbidden(t, updatePVCCapacity(node2Client)) + + //Enabled ExpandPersistentVolumes feature + defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ExpandPersistentVolumes, true)() + // Node->pvc relationship not established + expectForbidden(t, updatePVCCapacity(node1Client)) + // node->pvc relationship established and feature is enabled + expectAllowed(t, updatePVCCapacity(node2Client)) + // node->pvc relationship established but updating phase + expectForbidden(t, updatePVCPhase(node2Client)) } func expectForbidden(t *testing.T, err error) {