diff --git a/pkg/kubelet/kubelet.go b/pkg/kubelet/kubelet.go index 4dcff8440ee..0ad13cb6752 100644 --- a/pkg/kubelet/kubelet.go +++ b/pkg/kubelet/kubelet.go @@ -1028,10 +1028,10 @@ func (kl *Kubelet) relabelVolumes(pod *api.Pod, volumes kubecontainer.VolumeMap) rootDirSELinuxOptions.Level = pod.Spec.SecurityContext.SELinuxOptions.Level volumeContext := fmt.Sprintf("%s:%s:%s:%s", rootDirSELinuxOptions.User, rootDirSELinuxOptions.Role, rootDirSELinuxOptions.Type, rootDirSELinuxOptions.Level) - for _, volume := range volumes { - if volume.Builder.SupportsSELinux() && !volume.Builder.IsReadOnly() { + for _, vol := range volumes { + if vol.Builder.GetAttributes().Managed && vol.Builder.GetAttributes().SupportsSELinux { // Relabel the volume and its content to match the 'Level' of the pod - err := filepath.Walk(volume.Builder.GetPath(), func(path string, info os.FileInfo, err error) error { + err := filepath.Walk(vol.Builder.GetPath(), func(path string, info os.FileInfo, err error) error { if err != nil { return err } @@ -1040,7 +1040,7 @@ func (kl *Kubelet) relabelVolumes(pod *api.Pod, volumes kubecontainer.VolumeMap) if err != nil { return err } - volume.SELinuxLabeled = true + vol.SELinuxLabeled = true } } return nil @@ -1067,7 +1067,7 @@ func makeMounts(pod *api.Pod, podDir string, container *api.Container, podVolume // If the volume supports SELinux and it has not been // relabeled already and it is not a read-only volume, // relabel it and mark it as labeled - if vol.Builder.SupportsSELinux() && !vol.SELinuxLabeled && !vol.Builder.IsReadOnly() { + if vol.Builder.GetAttributes().SupportsSELinux && !vol.SELinuxLabeled && !vol.Builder.GetAttributes().Managed { vol.SELinuxLabeled = true relabelVolume = true } diff --git a/pkg/kubelet/kubelet_test.go b/pkg/kubelet/kubelet_test.go index 3700ec1a3a4..e17f972a935 100644 --- a/pkg/kubelet/kubelet_test.go +++ b/pkg/kubelet/kubelet_test.go @@ -515,8 +515,8 @@ func (f *stubVolume) GetPath() string { return f.path } -func (f *stubVolume) IsReadOnly() bool { - return false +func (f *stubVolume) GetAttributes() volume.Attributes { + return volume.Attributes{} } func (f *stubVolume) SetUp() error { @@ -527,14 +527,6 @@ func (f *stubVolume) SetUpAt(dir string) error { return nil } -func (f *stubVolume) SupportsSELinux() bool { - return false -} - -func (f *stubVolume) SupportsOwnershipManagement() bool { - return false -} - func TestMakeVolumeMounts(t *testing.T) { container := api.Container{ VolumeMounts: []api.VolumeMount{ diff --git a/pkg/kubelet/volumes.go b/pkg/kubelet/volumes.go index e788e3c8951..d91df95ec02 100644 --- a/pkg/kubelet/volumes.go +++ b/pkg/kubelet/volumes.go @@ -140,10 +140,15 @@ func (kl *Kubelet) mountExternalVolumes(pod *api.Pod) (kubecontainer.VolumeMap, if err != nil { return nil, err } - if hasFSGroup && builder.SupportsOwnershipManagement() && !builder.IsReadOnly() { + if hasFSGroup && + builder.GetAttributes().Managed && + builder.GetAttributes().SupportsOwnershipManagement { err := kl.manageVolumeOwnership(pod, internal, builder, fsGroup) if err != nil { + glog.Errorf("Error managing ownership of volume %v for pod %v/%v: %v", internal.Name(), pod.Namespace, pod.Name, err) return nil, err + } else { + glog.V(3).Infof("Managed ownership of volume %v for pod %v/%v", internal.Name(), pod.Namespace, pod.Name) } } podVolumes[volSpec.Name] = kubecontainer.VolumeInfo{Builder: builder} diff --git a/pkg/kubelet/volumes_linux.go b/pkg/kubelet/volumes_linux.go index 121d4ee10c6..3f20a0a10bb 100644 --- a/pkg/kubelet/volumes_linux.go +++ b/pkg/kubelet/volumes_linux.go @@ -28,8 +28,11 @@ import ( "k8s.io/kubernetes/pkg/volume" ) -// Bitmask to OR with current ownership of volumes that allow ownership management by the Kubelet -const managedOwnershipBitmask = os.FileMode(0660) +// Bitmasks to OR with current ownership of volumes that allow ownership management by the Kubelet +const ( + rwMask = os.FileMode(0660) + roMask = os.FileMode(0440) +) // manageVolumeOwnership modifies the given volume to be owned by fsGroup. func (kl *Kubelet) manageVolumeOwnership(pod *api.Pod, volSpec *volume.Spec, builder volume.Builder, fsGroup int64) error { @@ -53,7 +56,12 @@ func (kl *Kubelet) manageVolumeOwnership(pod *api.Pod, volSpec *volume.Spec, bui glog.Errorf("Chown failed on %v: %v", path, err) } - err = kl.chmodRunner.Chmod(path, info.Mode()|managedOwnershipBitmask|os.ModeSetgid) + mask := rwMask + if builder.GetAttributes().ReadOnly { + mask = roMask + } + + err = kl.chmodRunner.Chmod(path, info.Mode()|mask|os.ModeSetgid) if err != nil { glog.Errorf("Chmod failed on %v: %v", path, err) } diff --git a/pkg/volume/aws_ebs/aws_ebs.go b/pkg/volume/aws_ebs/aws_ebs.go index bc9bd7eb573..c0f5a611851 100644 --- a/pkg/volume/aws_ebs/aws_ebs.go +++ b/pkg/volume/aws_ebs/aws_ebs.go @@ -177,8 +177,13 @@ type awsElasticBlockStoreBuilder struct { var _ volume.Builder = &awsElasticBlockStoreBuilder{} -func (_ *awsElasticBlockStoreBuilder) SupportsOwnershipManagement() bool { - return true +func (b *awsElasticBlockStoreBuilder) GetAttributes() volume.Attributes { + return volume.Attributes{ + ReadOnly: b.readOnly, + Managed: !b.readOnly, + SupportsOwnershipManagement: true, + SupportsSELinux: true, + } } // SetUp attaches the disk and bind mounts to the volume path. @@ -246,14 +251,6 @@ func (b *awsElasticBlockStoreBuilder) SetUpAt(dir string) error { return nil } -func (b *awsElasticBlockStoreBuilder) IsReadOnly() bool { - return b.readOnly -} - -func (b *awsElasticBlockStoreBuilder) SupportsSELinux() bool { - return true -} - func makeGlobalPDPath(host volume.VolumeHost, volumeID string) string { // Clean up the URI to be more fs-friendly name := volumeID diff --git a/pkg/volume/aws_ebs/aws_ebs_test.go b/pkg/volume/aws_ebs/aws_ebs_test.go index 49180f93b43..5c19baa847f 100644 --- a/pkg/volume/aws_ebs/aws_ebs_test.go +++ b/pkg/volume/aws_ebs/aws_ebs_test.go @@ -225,7 +225,7 @@ func TestPersistentClaimReadOnlyFlag(t *testing.T) { pod := &api.Pod{ObjectMeta: api.ObjectMeta{UID: types.UID("poduid")}} builder, _ := plug.NewBuilder(spec, pod, volume.VolumeOptions{}) - if !builder.IsReadOnly() { + if !builder.GetAttributes().ReadOnly { t.Errorf("Expected true for builder.IsReadOnly") } } diff --git a/pkg/volume/cephfs/cephfs.go b/pkg/volume/cephfs/cephfs.go index 97c150600f4..5924da317a1 100644 --- a/pkg/volume/cephfs/cephfs.go +++ b/pkg/volume/cephfs/cephfs.go @@ -151,8 +151,13 @@ type cephfsBuilder struct { var _ volume.Builder = &cephfsBuilder{} -func (_ *cephfsBuilder) SupportsOwnershipManagement() bool { - return false +func (cephfsVolume *cephfsBuilder) GetAttributes() volume.Attributes { + return volume.Attributes{ + ReadOnly: cephfsVolume.readonly, + Managed: false, + SupportsOwnershipManagement: false, + SupportsSELinux: false, + } } // SetUp attaches the disk and bind mounts to the volume path. @@ -183,14 +188,6 @@ func (cephfsVolume *cephfsBuilder) SetUpAt(dir string) error { return err } -func (cephfsVolume *cephfsBuilder) IsReadOnly() bool { - return cephfsVolume.readonly -} - -func (cephfsVolume *cephfsBuilder) SupportsSELinux() bool { - return false -} - type cephfsCleaner struct { *cephfs } diff --git a/pkg/volume/cinder/cinder.go b/pkg/volume/cinder/cinder.go index 6e670b34ef0..c9f0c397c8b 100644 --- a/pkg/volume/cinder/cinder.go +++ b/pkg/volume/cinder/cinder.go @@ -153,8 +153,13 @@ func detachDiskLogError(cd *cinderVolume) { } } -func (_ *cinderVolumeBuilder) SupportsOwnershipManagement() bool { - return true +func (b *cinderVolumeBuilder) GetAttributes() volume.Attributes { + return volume.Attributes{ + ReadOnly: b.readOnly, + Managed: !b.readOnly, + SupportsOwnershipManagement: true, + SupportsSELinux: true, + } } func (b *cinderVolumeBuilder) SetUp() error { @@ -221,14 +226,6 @@ func (b *cinderVolumeBuilder) SetUpAt(dir string) error { return nil } -func (b *cinderVolumeBuilder) IsReadOnly() bool { - return b.readOnly -} - -func (b *cinderVolumeBuilder) SupportsSELinux() bool { - return true -} - func makeGlobalPDName(host volume.VolumeHost, devName string) string { return path.Join(host.GetPluginDir(cinderVolumePluginName), "mounts", devName) } diff --git a/pkg/volume/downwardapi/downwardapi.go b/pkg/volume/downwardapi/downwardapi.go index fb7e79f30d3..953070cef47 100644 --- a/pkg/volume/downwardapi/downwardapi.go +++ b/pkg/volume/downwardapi/downwardapi.go @@ -107,8 +107,14 @@ type downwardAPIVolumeBuilder struct { // downwardAPIVolumeBuilder implements volume.Builder interface var _ volume.Builder = &downwardAPIVolumeBuilder{} -func (_ *downwardAPIVolumeBuilder) SupportsOwnershipManagement() bool { - return false +// downward API volumes are always ReadOnlyManaged +func (d *downwardAPIVolume) GetAttributes() volume.Attributes { + return volume.Attributes{ + ReadOnly: true, + Managed: true, + SupportsOwnershipManagement: true, + SupportsSELinux: true, + } } // SetUp puts in place the volume plugin. @@ -152,15 +158,6 @@ func (b *downwardAPIVolumeBuilder) SetUpAt(dir string) error { return nil } -// IsReadOnly func to fulfill volume.Builder interface -func (d *downwardAPIVolume) IsReadOnly() bool { - return true -} - -func (d *downwardAPIVolume) SupportsSELinux() bool { - return true -} - // collectData collects requested downwardAPI in data map. // Map's key is the requested name of file to dump // Map's value is the (sorted) content of the field to be dumped in the file. diff --git a/pkg/volume/empty_dir/empty_dir.go b/pkg/volume/empty_dir/empty_dir.go index 5415390889e..5c0a99fda7c 100644 --- a/pkg/volume/empty_dir/empty_dir.go +++ b/pkg/volume/empty_dir/empty_dir.go @@ -135,8 +135,13 @@ type emptyDir struct { rootContext string } -func (_ *emptyDir) SupportsOwnershipManagement() bool { - return true +func (ed *emptyDir) GetAttributes() volume.Attributes { + return volume.Attributes{ + ReadOnly: false, + Managed: true, + SupportsOwnershipManagement: true, + SupportsSELinux: true, + } } // SetUp creates new directory. @@ -187,14 +192,6 @@ func (ed *emptyDir) SetUpAt(dir string) error { return err } -func (ed *emptyDir) IsReadOnly() bool { - return false -} - -func (ed *emptyDir) SupportsSELinux() bool { - return true -} - // setupTmpfs creates a tmpfs mount at the specified directory with the // specified SELinux context. func (ed *emptyDir) setupTmpfs(dir string, selinuxContext string) error { diff --git a/pkg/volume/fc/fc.go b/pkg/volume/fc/fc.go index 32cc382ddcc..f7834182608 100644 --- a/pkg/volume/fc/fc.go +++ b/pkg/volume/fc/fc.go @@ -164,10 +164,14 @@ type fcDiskBuilder struct { var _ volume.Builder = &fcDiskBuilder{} -func (_ *fcDiskBuilder) SupportsOwnershipManagement() bool { - return true +func (b *fcDiskBuilder) GetAttributes() volume.Attributes { + return volume.Attributes{ + ReadOnly: b.readOnly, + Managed: !b.readOnly, + SupportsOwnershipManagement: true, + SupportsSELinux: true, + } } - func (b *fcDiskBuilder) SetUp() error { return b.SetUpAt(b.GetPath()) } @@ -187,14 +191,6 @@ type fcDiskCleaner struct { var _ volume.Cleaner = &fcDiskCleaner{} -func (b *fcDiskBuilder) IsReadOnly() bool { - return b.readOnly -} - -func (b *fcDiskBuilder) SupportsSELinux() bool { - return true -} - // Unmounts the bind mount, and detaches the disk only if the disk // resource was the last reference to that disk on the kubelet. func (c *fcDiskCleaner) TearDown() error { diff --git a/pkg/volume/fc/fc_test.go b/pkg/volume/fc/fc_test.go index e92cc41dfb4..96531fb7456 100644 --- a/pkg/volume/fc/fc_test.go +++ b/pkg/volume/fc/fc_test.go @@ -246,7 +246,7 @@ func TestPersistentClaimReadOnlyFlag(t *testing.T) { pod := &api.Pod{ObjectMeta: api.ObjectMeta{UID: types.UID("poduid")}} builder, _ := plug.NewBuilder(spec, pod, volume.VolumeOptions{}) - if !builder.IsReadOnly() { + if !builder.GetAttributes().ReadOnly { t.Errorf("Expected true for builder.IsReadOnly") } } diff --git a/pkg/volume/flocker/plugin.go b/pkg/volume/flocker/plugin.go index 995d4b43a05..4d26555bc63 100644 --- a/pkg/volume/flocker/plugin.go +++ b/pkg/volume/flocker/plugin.go @@ -113,10 +113,14 @@ type flockerBuilder struct { readOnly bool } -func (_ *flockerBuilder) SupportsOwnershipManagement() bool { - return false +func (b flockerBuilder) GetAttributes() volume.Attributes { + return volume.Attributes{ + ReadOnly: b.readOnly, + Managed: false, + SupportsOwnershipManagement: false, + SupportsSELinux: false, + } } - func (b flockerBuilder) GetPath() string { return b.flocker.path } @@ -201,14 +205,6 @@ func (b flockerBuilder) SetUpAt(dir string) error { return nil } -func (b flockerBuilder) IsReadOnly() bool { - return b.readOnly -} - -func (b flockerBuilder) SupportsSELinux() bool { - return false -} - // updateDatasetPrimary will update the primary in Flocker and wait for it to // be ready. If it never gets to ready state it will timeout and error. func (b flockerBuilder) updateDatasetPrimary(datasetID, primaryUUID string) error { diff --git a/pkg/volume/flocker/plugin_test.go b/pkg/volume/flocker/plugin_test.go index 3f5985f448a..e172d2badc7 100644 --- a/pkg/volume/flocker/plugin_test.go +++ b/pkg/volume/flocker/plugin_test.go @@ -146,8 +146,8 @@ func TestNewCleaner(t *testing.T) { } func TestIsReadOnly(t *testing.T) { - b := flockerBuilder{readOnly: true} - assert.True(t, b.IsReadOnly()) + b := &flockerBuilder{readOnly: true} + assert.True(t, b.GetAttributes().ReadOnly) } func TestGetPath(t *testing.T) { diff --git a/pkg/volume/gce_pd/gce_pd.go b/pkg/volume/gce_pd/gce_pd.go index fce652254e1..3913ba63c10 100644 --- a/pkg/volume/gce_pd/gce_pd.go +++ b/pkg/volume/gce_pd/gce_pd.go @@ -165,8 +165,13 @@ type gcePersistentDiskBuilder struct { var _ volume.Builder = &gcePersistentDiskBuilder{} -func (_ *gcePersistentDiskBuilder) SupportsOwnershipManagement() bool { - return true +func (b *gcePersistentDiskBuilder) GetAttributes() volume.Attributes { + return volume.Attributes{ + ReadOnly: b.readOnly, + Managed: !b.readOnly, + SupportsOwnershipManagement: true, + SupportsSELinux: true, + } } // SetUp attaches the disk and bind mounts to the volume path. @@ -234,14 +239,6 @@ func (b *gcePersistentDiskBuilder) SetUpAt(dir string) error { return nil } -func (b *gcePersistentDiskBuilder) IsReadOnly() bool { - return b.readOnly -} - -func (b *gcePersistentDiskBuilder) SupportsSELinux() bool { - return true -} - func makeGlobalPDName(host volume.VolumeHost, devName string) string { return path.Join(host.GetPluginDir(gcePersistentDiskPluginName), "mounts", devName) } diff --git a/pkg/volume/gce_pd/gce_pd_test.go b/pkg/volume/gce_pd/gce_pd_test.go index 55ddc5d2c74..0559c14b82f 100644 --- a/pkg/volume/gce_pd/gce_pd_test.go +++ b/pkg/volume/gce_pd/gce_pd_test.go @@ -240,7 +240,7 @@ func TestPersistentClaimReadOnlyFlag(t *testing.T) { pod := &api.Pod{ObjectMeta: api.ObjectMeta{UID: types.UID("poduid")}} builder, _ := plug.NewBuilder(spec, pod, volume.VolumeOptions{}) - if !builder.IsReadOnly() { + if !builder.GetAttributes().ReadOnly { t.Errorf("Expected true for builder.IsReadOnly") } } diff --git a/pkg/volume/git_repo/git_repo.go b/pkg/volume/git_repo/git_repo.go index 1e1c1531d29..e5a7930a611 100644 --- a/pkg/volume/git_repo/git_repo.go +++ b/pkg/volume/git_repo/git_repo.go @@ -109,8 +109,13 @@ type gitRepoVolumeBuilder struct { var _ volume.Builder = &gitRepoVolumeBuilder{} -func (_ *gitRepoVolumeBuilder) SupportsOwnershipManagement() bool { - return true +func (b *gitRepoVolumeBuilder) GetAttributes() volume.Attributes { + return volume.Attributes{ + ReadOnly: false, + Managed: true, + SupportsOwnershipManagement: false, + SupportsSELinux: true, // xattr change should be okay, TODO: double check + } } // SetUp creates new directory and clones a git repo. @@ -118,14 +123,6 @@ func (b *gitRepoVolumeBuilder) SetUp() error { return b.SetUpAt(b.GetPath()) } -func (b *gitRepoVolumeBuilder) IsReadOnly() bool { - return false -} - -func (b *gitRepoVolumeBuilder) SupportsSELinux() bool { - return true -} - // This is the spec for the volume that this plugin wraps. var wrappedVolumeSpec = &volume.Spec{ Volume: &api.Volume{VolumeSource: api.VolumeSource{EmptyDir: &api.EmptyDirVolumeSource{}}}, diff --git a/pkg/volume/glusterfs/glusterfs.go b/pkg/volume/glusterfs/glusterfs.go index c9cc63c6fa1..6869593b654 100644 --- a/pkg/volume/glusterfs/glusterfs.go +++ b/pkg/volume/glusterfs/glusterfs.go @@ -154,8 +154,13 @@ type glusterfsBuilder struct { var _ volume.Builder = &glusterfsBuilder{} -func (_ *glusterfsBuilder) SupportsOwnershipManagement() bool { - return false +func (b *glusterfsBuilder) GetAttributes() volume.Attributes { + return volume.Attributes{ + ReadOnly: b.readOnly, + Managed: false, + SupportsOwnershipManagement: false, + SupportsSELinux: false, + } } // SetUp attaches the disk and bind mounts to the volume path. @@ -185,14 +190,6 @@ func (b *glusterfsBuilder) SetUpAt(dir string) error { return err } -func (b *glusterfsBuilder) IsReadOnly() bool { - return b.readOnly -} - -func (b *glusterfsBuilder) SupportsSELinux() bool { - return false -} - func (glusterfsVolume *glusterfs) GetPath() string { name := glusterfsPluginName return glusterfsVolume.plugin.host.GetPodVolumeDir(glusterfsVolume.pod.UID, util.EscapeQualifiedNameForDisk(name), glusterfsVolume.volName) diff --git a/pkg/volume/glusterfs/glusterfs_test.go b/pkg/volume/glusterfs/glusterfs_test.go index 4d7f55938a2..f074e669009 100644 --- a/pkg/volume/glusterfs/glusterfs_test.go +++ b/pkg/volume/glusterfs/glusterfs_test.go @@ -209,7 +209,7 @@ func TestPersistentClaimReadOnlyFlag(t *testing.T) { pod := &api.Pod{ObjectMeta: api.ObjectMeta{UID: types.UID("poduid")}} builder, _ := plug.NewBuilder(spec, pod, volume.VolumeOptions{}) - if !builder.IsReadOnly() { + if !builder.GetAttributes().ReadOnly { t.Errorf("Expected true for builder.IsReadOnly") } } diff --git a/pkg/volume/host_path/host_path.go b/pkg/volume/host_path/host_path.go index 28f151188a2..8c8d222250f 100644 --- a/pkg/volume/host_path/host_path.go +++ b/pkg/volume/host_path/host_path.go @@ -167,8 +167,13 @@ type hostPathBuilder struct { var _ volume.Builder = &hostPathBuilder{} -func (_ *hostPathBuilder) SupportsOwnershipManagement() bool { - return false +func (b *hostPathBuilder) GetAttributes() volume.Attributes { + return volume.Attributes{ + ReadOnly: b.readOnly, + Managed: false, + SupportsOwnershipManagement: false, + SupportsSELinux: false, + } } // SetUp does nothing. @@ -181,14 +186,6 @@ func (b *hostPathBuilder) SetUpAt(dir string) error { return fmt.Errorf("SetUpAt() does not make sense for host paths") } -func (b *hostPathBuilder) IsReadOnly() bool { - return b.readOnly -} - -func (b *hostPathBuilder) SupportsSELinux() bool { - return false -} - func (b *hostPathBuilder) GetPath() string { return b.path } diff --git a/pkg/volume/host_path/host_path_test.go b/pkg/volume/host_path/host_path_test.go index 68a5f8ec4b1..0ec026346c5 100644 --- a/pkg/volume/host_path/host_path_test.go +++ b/pkg/volume/host_path/host_path_test.go @@ -268,7 +268,7 @@ func TestPersistentClaimReadOnlyFlag(t *testing.T) { pod := &api.Pod{ObjectMeta: api.ObjectMeta{UID: types.UID("poduid")}} builder, _ := plug.NewBuilder(spec, pod, volume.VolumeOptions{}) - if !builder.IsReadOnly() { + if !builder.GetAttributes().ReadOnly { t.Errorf("Expected true for builder.IsReadOnly") } } diff --git a/pkg/volume/iscsi/iscsi.go b/pkg/volume/iscsi/iscsi.go index a448da31e78..a4b204309f4 100644 --- a/pkg/volume/iscsi/iscsi.go +++ b/pkg/volume/iscsi/iscsi.go @@ -158,8 +158,13 @@ type iscsiDiskBuilder struct { var _ volume.Builder = &iscsiDiskBuilder{} -func (_ *iscsiDiskBuilder) SupportsOwnershipManagement() bool { - return true +func (b *iscsiDiskBuilder) GetAttributes() volume.Attributes { + return volume.Attributes{ + ReadOnly: b.readOnly, + Managed: !b.readOnly, + SupportsOwnershipManagement: true, + SupportsSELinux: true, + } } func (b *iscsiDiskBuilder) SetUp() error { @@ -181,14 +186,6 @@ type iscsiDiskCleaner struct { var _ volume.Cleaner = &iscsiDiskCleaner{} -func (b *iscsiDiskBuilder) IsReadOnly() bool { - return b.readOnly -} - -func (b *iscsiDiskBuilder) SupportsSELinux() bool { - return true -} - // Unmounts the bind mount, and detaches the disk only if the disk // resource was the last reference to that disk on the kubelet. func (c *iscsiDiskCleaner) TearDown() error { diff --git a/pkg/volume/iscsi/iscsi_test.go b/pkg/volume/iscsi/iscsi_test.go index 2025d922c75..aec79aa3460 100644 --- a/pkg/volume/iscsi/iscsi_test.go +++ b/pkg/volume/iscsi/iscsi_test.go @@ -246,7 +246,7 @@ func TestPersistentClaimReadOnlyFlag(t *testing.T) { pod := &api.Pod{ObjectMeta: api.ObjectMeta{UID: types.UID("poduid")}} builder, _ := plug.NewBuilder(spec, pod, volume.VolumeOptions{}) - if !builder.IsReadOnly() { + if !builder.GetAttributes().ReadOnly { t.Errorf("Expected true for builder.IsReadOnly") } } diff --git a/pkg/volume/nfs/nfs.go b/pkg/volume/nfs/nfs.go index 2b55fe359c8..c4027aeb5f1 100644 --- a/pkg/volume/nfs/nfs.go +++ b/pkg/volume/nfs/nfs.go @@ -147,8 +147,13 @@ type nfsBuilder struct { var _ volume.Builder = &nfsBuilder{} -func (_ *nfsBuilder) SupportsOwnershipManagement() bool { - return false +func (b *nfsBuilder) GetAttributes() volume.Attributes { + return volume.Attributes{ + ReadOnly: b.readOnly, + Managed: false, + SupportsOwnershipManagement: false, + SupportsSELinux: false, + } } // SetUp attaches the disk and bind mounts to the volume path. @@ -200,14 +205,6 @@ func (b *nfsBuilder) SetUpAt(dir string) error { return nil } -func (b *nfsBuilder) IsReadOnly() bool { - return b.readOnly -} - -func (b *nfsBuilder) SupportsSELinux() bool { - return false -} - // //func (c *nfsCleaner) GetPath() string { // name := nfsPluginName diff --git a/pkg/volume/nfs/nfs_test.go b/pkg/volume/nfs/nfs_test.go index 594d6fb707c..97e341b0105 100644 --- a/pkg/volume/nfs/nfs_test.go +++ b/pkg/volume/nfs/nfs_test.go @@ -247,7 +247,7 @@ func TestPersistentClaimReadOnlyFlag(t *testing.T) { pod := &api.Pod{ObjectMeta: api.ObjectMeta{UID: types.UID("poduid")}} builder, _ := plug.NewBuilder(spec, pod, volume.VolumeOptions{}) - if !builder.IsReadOnly() { + if !builder.GetAttributes().ReadOnly { t.Errorf("Expected true for builder.IsReadOnly") } } diff --git a/pkg/volume/persistent_claim/persistent_claim.go b/pkg/volume/persistent_claim/persistent_claim.go index de1659d6e93..b574c1f0fb5 100644 --- a/pkg/volume/persistent_claim/persistent_claim.go +++ b/pkg/volume/persistent_claim/persistent_claim.go @@ -88,10 +88,6 @@ func (plugin *persistentClaimPlugin) NewBuilder(spec *volume.Spec, pod *api.Pod, return builder, nil } -func (plugin *persistentClaimPlugin) IsReadOnly() bool { - return plugin.readOnly -} - func (plugin *persistentClaimPlugin) NewCleaner(_ string, _ types.UID) (volume.Cleaner, error) { return nil, fmt.Errorf("This will never be called directly. The PV backing this claim has a cleaner. Kubelet uses that cleaner, not this one, when removing orphaned volumes.") } diff --git a/pkg/volume/rbd/disk_manager.go b/pkg/volume/rbd/disk_manager.go index 8d1941d60c6..229827fd903 100644 --- a/pkg/volume/rbd/disk_manager.go +++ b/pkg/volume/rbd/disk_manager.go @@ -62,7 +62,7 @@ func diskSetUp(manager diskManager, b rbdBuilder, volPath string, mounter mount. } // Perform a bind mount to the full path to allow duplicate mounts of the same disk. options := []string{"bind"} - if b.IsReadOnly() { + if (&b).GetAttributes().ReadOnly { options = append(options, "ro") } err = mounter.Mount(globalPDPath, volPath, "", options) diff --git a/pkg/volume/rbd/rbd.go b/pkg/volume/rbd/rbd.go index 6b828c3a520..9435da4b902 100644 --- a/pkg/volume/rbd/rbd.go +++ b/pkg/volume/rbd/rbd.go @@ -192,8 +192,13 @@ type rbdBuilder struct { var _ volume.Builder = &rbdBuilder{} -func (_ *rbdBuilder) SupportsOwnershipManagement() bool { - return true +func (b *rbd) GetAttributes() volume.Attributes { + return volume.Attributes{ + ReadOnly: b.ReadOnly, + Managed: !b.ReadOnly, + SupportsOwnershipManagement: true, + SupportsSELinux: true, + } } func (b *rbdBuilder) SetUp() error { @@ -215,14 +220,6 @@ type rbdCleaner struct { var _ volume.Cleaner = &rbdCleaner{} -func (b *rbd) IsReadOnly() bool { - return b.ReadOnly -} - -func (b *rbd) SupportsSELinux() bool { - return true -} - // Unmounts the bind mount, and detaches the disk only if the disk // resource was the last reference to that disk on the kubelet. func (c *rbdCleaner) TearDown() error { diff --git a/pkg/volume/rbd/rbd_test.go b/pkg/volume/rbd/rbd_test.go index a42f609e185..f9a6096ae19 100644 --- a/pkg/volume/rbd/rbd_test.go +++ b/pkg/volume/rbd/rbd_test.go @@ -203,7 +203,7 @@ func TestPersistentClaimReadOnlyFlag(t *testing.T) { pod := &api.Pod{ObjectMeta: api.ObjectMeta{UID: types.UID("poduid")}} builder, _ := plug.NewBuilder(spec, pod, volume.VolumeOptions{}) - if !builder.IsReadOnly() { + if !builder.GetAttributes().ReadOnly { t.Errorf("Expected true for builder.IsReadOnly") } } diff --git a/pkg/volume/rbd/rbd_util.go b/pkg/volume/rbd/rbd_util.go index 3b2c66cec42..bd315d065de 100644 --- a/pkg/volume/rbd/rbd_util.go +++ b/pkg/volume/rbd/rbd_util.go @@ -193,7 +193,7 @@ func (util *RBDUtil) loadRBD(builder *rbdBuilder, mnt string) error { func (util *RBDUtil) fencing(b rbdBuilder) error { // no need to fence readOnly - if b.IsReadOnly() { + if (&b).GetAttributes().ReadOnly { return nil } return util.rbdLock(b, true) diff --git a/pkg/volume/secret/secret.go b/pkg/volume/secret/secret.go index 514e9e30464..607abb7ae8d 100644 --- a/pkg/volume/secret/secret.go +++ b/pkg/volume/secret/secret.go @@ -97,10 +97,14 @@ type secretVolumeBuilder struct { var _ volume.Builder = &secretVolumeBuilder{} -func (_ *secretVolumeBuilder) SupportsOwnershipManagement() bool { - return true +func (sv *secretVolume) GetAttributes() volume.Attributes { + return volume.Attributes{ + ReadOnly: true, + Managed: true, + SupportsOwnershipManagement: true, + SupportsSELinux: true, + } } - func (b *secretVolumeBuilder) SetUp() error { return b.SetUpAt(b.GetPath()) } @@ -172,14 +176,6 @@ func (b *secretVolumeBuilder) SetUpAt(dir string) error { return nil } -func (sv *secretVolume) IsReadOnly() bool { - return false -} - -func (sv *secretVolume) SupportsSELinux() bool { - return true -} - func totalSecretBytes(secret *api.Secret) int { totalSize := 0 for _, bytes := range secret.Data { diff --git a/pkg/volume/testing.go b/pkg/volume/testing.go index 0be6b1e86ed..cb72e831e39 100644 --- a/pkg/volume/testing.go +++ b/pkg/volume/testing.go @@ -156,8 +156,13 @@ type FakeVolume struct { Plugin *FakeVolumePlugin } -func (_ *FakeVolume) SupportsOwnershipManagement() bool { - return false +func (_ *FakeVolume) GetAttributes() Attributes { + return Attributes{ + ReadOnly: false, + Managed: true, + SupportsOwnershipManagement: true, + SupportsSELinux: true, + } } func (fv *FakeVolume) SetUp() error { @@ -168,14 +173,6 @@ func (fv *FakeVolume) SetUpAt(dir string) error { return os.MkdirAll(dir, 0750) } -func (fv *FakeVolume) IsReadOnly() bool { - return false -} - -func (fv *FakeVolume) SupportsSELinux() bool { - return false -} - func (fv *FakeVolume) GetPath() string { return path.Join(fv.Plugin.Host.GetPodVolumeDir(fv.PodUID, util.EscapeQualifiedNameForDisk(fv.Plugin.PluginName), fv.VolName)) } diff --git a/pkg/volume/volume.go b/pkg/volume/volume.go index cc2f6d93cdf..4121a09bcc9 100644 --- a/pkg/volume/volume.go +++ b/pkg/volume/volume.go @@ -30,6 +30,14 @@ type Volume interface { GetPath() string } +// Attributes represents the attributes of this builder. +type Attributes struct { + ReadOnly bool + Managed bool + SupportsOwnershipManagement bool + SupportsSELinux bool +} + // Builder interface provides methods to set up/mount the volume. type Builder interface { // Uses Interface to provide the path for Docker binds. @@ -42,21 +50,8 @@ type Builder interface { // directory path, which may or may not exist yet. This may be called // more than once, so implementations must be idempotent. SetUpAt(dir string) error - // IsReadOnly is a flag that gives the builder's ReadOnly attribute. - // All persistent volumes have a private readOnly flag in their builders. - IsReadOnly() bool - // SupportsOwnershipManagement returns whether this builder wants - // ownership management for the volume. If this method returns true, - // the Kubelet will: - // - // 1. Make the volume owned by group FSGroup - // 2. Set the setgid bit is set (new files created in the volume will be owned by FSGroup) - // 3. Logical OR the permission bits with rw-rw---- - SupportsOwnershipManagement() bool - // SupportsSELinux reports whether the given builder supports - // SELinux and would like the kubelet to relabel the volume to - // match the pod to which it will be attached. - SupportsSELinux() bool + // GetAttributes returns the attributes of the builder. + GetAttributes() Attributes } // Cleaner interface provides methods to cleanup/unmount the volumes. diff --git a/test/e2e/downwardapi_volume.go b/test/e2e/downwardapi_volume.go index 7e0ff66160c..bd0ee169d78 100644 --- a/test/e2e/downwardapi_volume.go +++ b/test/e2e/downwardapi_volume.go @@ -29,62 +29,24 @@ var _ = Describe("Downward API volume", func() { f := NewFramework("downward-api") It("should provide labels and annotations files [Conformance]", func() { + podName := "downwardapi-volume-" + string(util.NewUUID()) + pod := downwardAPIVolumePod(podName) + + testContainerOutput("downward API volume plugin", f.Client, pod, 0, []string{ + fmt.Sprintf("cluster=\"rack10\"\n"), + fmt.Sprintf("builder=\"john-doe\"\n"), + fmt.Sprintf("%s\n", podName), + }, f.Namespace.Name) + }) + + It("should provide labels and annotations files as non-root with fsgroup [Conformance] [Skipped]", func() { podName := "metadata-volume-" + string(util.NewUUID()) - pod := &api.Pod{ - ObjectMeta: api.ObjectMeta{ - Name: podName, - Labels: map[string]string{"cluster": "rack10"}, - Annotations: map[string]string{"builder": "john-doe"}, - }, - Spec: api.PodSpec{ - Containers: []api.Container{ - { - Name: "client-container", - Image: "gcr.io/google_containers/busybox", - Command: []string{"sh", "-c", "cat /etc/labels /etc/annotations /etc/podname"}, - VolumeMounts: []api.VolumeMount{ - { - Name: "podinfo", - MountPath: "/etc", - ReadOnly: false, - }, - }, - }, - }, - Volumes: []api.Volume{ - { - Name: "podinfo", - VolumeSource: api.VolumeSource{ - DownwardAPI: &api.DownwardAPIVolumeSource{ - Items: []api.DownwardAPIVolumeFile{ - { - Path: "labels", - FieldRef: api.ObjectFieldSelector{ - APIVersion: "v1", - FieldPath: "metadata.labels", - }, - }, - { - Path: "annotations", - FieldRef: api.ObjectFieldSelector{ - APIVersion: "v1", - FieldPath: "metadata.annotations", - }, - }, - { - Path: "podname", - FieldRef: api.ObjectFieldSelector{ - APIVersion: "v1", - FieldPath: "metadata.name", - }, - }, - }, - }, - }, - }, - }, - RestartPolicy: api.RestartPolicyNever, - }, + uid := int64(1001) + gid := int64(1234) + pod := downwardAPIVolumePod(podName) + pod.Spec.SecurityContext = &api.PodSecurityContext{ + RunAsUser: &uid, + FSGroup: &gid, } testContainerOutput("downward API volume plugin", f.Client, pod, 0, []string{ fmt.Sprintf("cluster=\"rack10\"\n"), @@ -94,4 +56,63 @@ var _ = Describe("Downward API volume", func() { }) }) +func downwardAPIVolumePod(name string) *api.Pod { + return &api.Pod{ + ObjectMeta: api.ObjectMeta{ + Name: name, + Labels: map[string]string{"cluster": "rack10"}, + Annotations: map[string]string{"builder": "john-doe"}, + }, + Spec: api.PodSpec{ + Containers: []api.Container{ + { + Name: "client-container", + Image: "gcr.io/google_containers/busybox", + Command: []string{"sh", "-c", "cat /etc/labels /etc/annotations /etc/podname"}, + VolumeMounts: []api.VolumeMount{ + { + Name: "podinfo", + MountPath: "/etc", + ReadOnly: false, + }, + }, + }, + }, + Volumes: []api.Volume{ + { + Name: "podinfo", + VolumeSource: api.VolumeSource{ + DownwardAPI: &api.DownwardAPIVolumeSource{ + Items: []api.DownwardAPIVolumeFile{ + { + Path: "labels", + FieldRef: api.ObjectFieldSelector{ + APIVersion: "v1", + FieldPath: "metadata.labels", + }, + }, + { + Path: "annotations", + FieldRef: api.ObjectFieldSelector{ + APIVersion: "v1", + FieldPath: "metadata.annotations", + }, + }, + { + Path: "podname", + FieldRef: api.ObjectFieldSelector{ + APIVersion: "v1", + FieldPath: "metadata.name", + }, + }, + }, + }, + }, + }, + }, + RestartPolicy: api.RestartPolicyNever, + }, + } +} + // TODO: add test-webserver example as pointed out in https://github.com/kubernetes/kubernetes/pull/5093#discussion-diff-37606771