Allow node to update PVC's status

Implement node policy feature gates
Add tests for node policy update

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",
     ],
 )
 

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 {

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",
     ],
 )

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 {

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

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",

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) {