Merge pull request #20490 from swagiaal/auto-supplemental-group-kubelet

Automatic merge from submit-queue

Automatically Add Supplemental Groups from Volumes to Pods

This adds support for a "GID" annotation that one can add to their PVs. When this annotation is seen the kubelet automatically adds the given GID to the list of supplemental groups for the pod to which the PV is attached. This allows admins to create volumes and suggest a GID to use to access the volume. This is needed for volumes which do not support ownership management such as NFS.

@markturansky PTAL

pkg/volume/persistent_claim/persistent_claim.go

@@ -18,6 +18,7 @@ package persistent_claim
 
 import (
 	"fmt"
+	"strconv"
 
 	"github.com/golang/glog"
 	"k8s.io/kubernetes/pkg/api"
@@ -38,6 +39,7 @@ var _ volume.VolumePlugin = &persistentClaimPlugin{}
 
 const (
 	persistentClaimPluginName = "kubernetes.io/persistent-claim"
+	volumeGidAnnotationKey    = "pv.beta.kubernetes.io/gid"
 )
 
 func (plugin *persistentClaimPlugin) Init(host volume.VolumeHost) error {
@@ -80,6 +82,19 @@ func (plugin *persistentClaimPlugin) NewMounter(spec *volume.Spec, pod *api.Pod,
 		return nil, err
 	}
 
+	// If a GID annotation is provided set the GID attribute.
+	if volumeGid, ok := pv.Annotations[volumeGidAnnotationKey]; ok {
+		gid, err := strconv.ParseInt(volumeGid, 10, 64)
+		if err != nil {
+			return nil, fmt.Errorf("Invalid value for %s %v", volumeGidAnnotationKey, err)
+		}
+
+		if pod.Spec.SecurityContext == nil {
+			pod.Spec.SecurityContext = &api.PodSecurityContext{}
+		}
+		pod.Spec.SecurityContext.SupplementalGroups = append(pod.Spec.SecurityContext.SupplementalGroups, gid)
+	}
+
 	mounter, err := plugin.host.NewWrapperMounter(claim.Spec.VolumeName, *volume.NewSpecFromPersistentVolume(pv, spec.ReadOnly), pod, opts)
 	if err != nil {
 		glog.Errorf("Error creating mounter for claim: %+v\n", claim.Name)

pkg/volume/persistent_claim/persistent_claim_test.go

@@ -72,7 +72,7 @@ func TestNewMounter(t *testing.T) {
 		claim           *api.PersistentVolumeClaim
 		plugin          volume.VolumePlugin
 		podVolume       api.VolumeSource
-		testFunc        func(mounter volume.Mounter, plugin volume.VolumePlugin) error
+		testFunc        func(mounter volume.Mounter, plugin volume.VolumePlugin, pod *api.Pod) error
 		expectedFailure bool
 	}{
 		{
@@ -108,7 +108,7 @@ func TestNewMounter(t *testing.T) {
 				},
 			},
 			plugin: gce_pd.ProbeVolumePlugins()[0],
-			testFunc: func(mounter volume.Mounter, plugin volume.VolumePlugin) error {
+			testFunc: func(mounter volume.Mounter, plugin volume.VolumePlugin, pod *api.Pod) error {
 				if !strings.Contains(mounter.GetPath(), utilstrings.EscapeQualifiedNameForDisk(plugin.Name())) {
 					return fmt.Errorf("mounter path expected to contain plugin name.  Got: %s", mounter.GetPath())
 				}
@@ -146,7 +146,7 @@ func TestNewMounter(t *testing.T) {
 				},
 			},
 			plugin: host_path.ProbeVolumePlugins(volume.VolumeConfig{})[0],
-			testFunc: func(mounter volume.Mounter, plugin volume.VolumePlugin) error {
+			testFunc: func(mounter volume.Mounter, plugin volume.VolumePlugin, pod *api.Pod) error {
 				if mounter.GetPath() != "/somepath" {
 					return fmt.Errorf("Expected HostPath.Path /somepath, got: %s", mounter.GetPath())
 				}
@@ -184,7 +184,7 @@ func TestNewMounter(t *testing.T) {
 				},
 			},
 			plugin: gce_pd.ProbeVolumePlugins()[0],
-			testFunc: func(mounter volume.Mounter, plugin volume.VolumePlugin) error {
+			testFunc: func(mounter volume.Mounter, plugin volume.VolumePlugin, pod *api.Pod) error {
 				if mounter != nil {
 					return fmt.Errorf("Unexpected non-nil mounter: %+v", mounter)
 				}
@@ -227,7 +227,7 @@ func TestNewMounter(t *testing.T) {
 				},
 			},
 			plugin: gce_pd.ProbeVolumePlugins()[0],
-			testFunc: func(mounter volume.Mounter, plugin volume.VolumePlugin) error {
+			testFunc: func(mounter volume.Mounter, plugin volume.VolumePlugin, pod *api.Pod) error {
 				if mounter != nil {
 					return fmt.Errorf("Unexpected non-nil mounter: %+v", mounter)
 				}
@@ -235,6 +235,56 @@ func TestNewMounter(t *testing.T) {
 			},
 			expectedFailure: true, // mismatched pv.Spec.ClaimRef and pvc
 		},
+		{ // Test GID annotation
+			pv: &api.PersistentVolume{
+				ObjectMeta: api.ObjectMeta{
+					Name: "pv",
+					Annotations: map[string]string{
+						volumeGidAnnotationKey: "12345",
+					},
+				},
+				Spec: api.PersistentVolumeSpec{
+					PersistentVolumeSource: api.PersistentVolumeSource{
+						GCEPersistentDisk: &api.GCEPersistentDiskVolumeSource{},
+					},
+					ClaimRef: &api.ObjectReference{
+						Name: "claim",
+						UID:  types.UID("abc123"),
+					},
+				},
+			},
+			claim: &api.PersistentVolumeClaim{
+				ObjectMeta: api.ObjectMeta{
+					Name: "claim",
+					UID:  types.UID("abc123"),
+				},
+				Spec: api.PersistentVolumeClaimSpec{
+					VolumeName: "pv",
+				},
+				Status: api.PersistentVolumeClaimStatus{
+					Phase: api.ClaimBound,
+				},
+			},
+			podVolume: api.VolumeSource{
+				PersistentVolumeClaim: &api.PersistentVolumeClaimVolumeSource{
+					ReadOnly:  false,
+					ClaimName: "claim",
+				},
+			},
+			plugin: gce_pd.ProbeVolumePlugins()[0],
+			testFunc: func(mounter volume.Mounter, plugin volume.VolumePlugin, pod *api.Pod) error {
+				if pod.Spec.SecurityContext == nil {
+					return fmt.Errorf("Pod SecurityContext was not set")
+				}
+
+				if pod.Spec.SecurityContext.SupplementalGroups[0] != 12345 {
+					return fmt.Errorf("Pod's SupplementalGroups list does not contain expect group")
+				}
+
+				return nil
+			},
+			expectedFailure: false,
+		},
 	}
 
 	for _, item := range tests {
@@ -262,7 +312,7 @@ func TestNewMounter(t *testing.T) {
 			}
 		}
 
-		if err := item.testFunc(mounter, item.plugin); err != nil {
+		if err := item.testFunc(mounter, item.plugin, pod); err != nil {
 			t.Errorf("Unexpected error %+v", err)
 		}
 	}