github/kubernetes
4
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix directory mismatch for volume.SetVolumeOwnership()

Browse Source 
In most cases `dir` arg of `SetUpAt()` method of `volume.Mounter` interface is the same as `mounter.GetPath()` because we usually call `SetUpAt()` from `SetUp()` like this:"
```
func (ed *emptyDir) SetUp(mounterArgs volume.MounterArgs) error {
	return ed.SetUpAt(ed.GetPath(), mounterArgs)
}
```
(this example is from `volume/emptydir/empty_dir.go`, but there are plenty other examples like that in `volume/*`)

However, there is currently one exception. This is from `volume/projected/projected.go`:
```
	if err := wrapped.SetUpAt(dir, mounterArgs); err != nil {
		return err
	}
```
(see 96306f144a/pkg/volume/projected/projected.go (L203))

In this case `dir` is not equal to `wrapped.GetPath()` and `volume.SetVolumeOwnership()` fails when called from `SetUpAt()` of wrapped volume:
```
lstat /var/lib/kubelet/pods/a2f6e58f-7edf-4c48-a97c-ef1b8fd3caf6/volumes/kubernetes.io~empty-dir/wrapped_kube-api-access-knvkv: no such file or directory
```

To fix the issue let's pass `dir` arg to `volume.SetVolumeOwnership()` explicitly, and use it instead of `mounter.GetPath()`.
This commit is contained in:
Maxim Patlasov 2023-03-30 13:27:58 -07:00
parent c3e7eca7fd
commit 0a37f09c32
18 changed files with 27 additions and 29 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
pkg/volume/configmap/configmap.go
View File

@ -252,7 +252,7 @@ func (b *configMapVolumeMounter) SetUpAt(dir string, mounterArgs volume.MounterA
setPerms := func(_ string) error {
// This may be the first time writing and new files get created outside the timestamp subdirectory:
// change the permissions on the whole volume and not only in the timestamp directory.
return volume.SetVolumeOwnership(b, mounterArgs.FsGroup, nil /*fsGroupChangePolicy*/, volumeutil.FSGroupCompleteHook(b.plugin, nil))
return volume.SetVolumeOwnership(b, dir, mounterArgs.FsGroup, nil /*fsGroupChangePolicy*/, volumeutil.FSGroupCompleteHook(b.plugin, nil))
}
err = writer.Write(payload, setPerms)
if err != nil {

2
pkg/volume/csi/csi_mounter.go
View File

@ -333,7 +333,7 @@ func (c *csiMountMgr) SetUpAt(dir string, mounterArgs volume.MounterArgs) error
// Driver doesn't support applying FSGroup. Kubelet must apply it instead.
// fullPluginName helps to distinguish different driver from csi plugin
err := volume.SetVolumeOwnership(c, mounterArgs.FsGroup, mounterArgs.FSGroupChangePolicy, util.FSGroupCompleteHook(c.plugin, c.spec))
err := volume.SetVolumeOwnership(c, dir, mounterArgs.FsGroup, mounterArgs.FSGroupChangePolicy, util.FSGroupCompleteHook(c.plugin, c.spec))
if err != nil {
// At this point mount operation is successful:
// 1. Since volume can not be used by the pod because of invalid permissions, we must return error

2
pkg/volume/downwardapi/downwardapi.go
View File

@ -223,7 +223,7 @@ func (b *downwardAPIVolumeMounter) SetUpAt(dir string, mounterArgs volume.Mounte
setPerms := func(_ string) error {
// This may be the first time writing and new files get created outside the timestamp subdirectory:
// change the permissions on the whole volume and not only in the timestamp directory.
return volume.SetVolumeOwnership(b, mounterArgs.FsGroup, nil /*fsGroupChangePolicy*/, volumeutil.FSGroupCompleteHook(b.plugin, nil))
return volume.SetVolumeOwnership(b, dir, mounterArgs.FsGroup, nil /*fsGroupChangePolicy*/, volumeutil.FSGroupCompleteHook(b.plugin, nil))
}
err = writer.Write(data, setPerms)
if err != nil {

2
pkg/volume/emptydir/empty_dir.go
View File

@ -280,7 +280,7 @@ func (ed *emptyDir) SetUpAt(dir string, mounterArgs volume.MounterArgs) error {
err = fmt.Errorf("unknown storage medium %q", ed.medium)
}
volume.SetVolumeOwnership(ed, mounterArgs.FsGroup, nil /*fsGroupChangePolicy*/, volumeutil.FSGroupCompleteHook(ed.plugin, nil))
volume.SetVolumeOwnership(ed, dir, mounterArgs.FsGroup, nil /*fsGroupChangePolicy*/, volumeutil.FSGroupCompleteHook(ed.plugin, nil))
// If setting up the quota fails, just log a message but don't actually error out.
// We'll use the old du mechanism in this case, at least until we support

2
pkg/volume/fc/disk_manager.go
View File

@ -91,7 +91,7 @@ func diskSetUp(manager diskManager, b fcDiskMounter, volPath string, mounter mou
}
if !b.readOnly {
volume.SetVolumeOwnership(&b, fsGroup, fsGroupChangePolicy, util.FSGroupCompleteHook(b.plugin, nil))
volume.SetVolumeOwnership(&b, volPath, fsGroup, fsGroupChangePolicy, util.FSGroupCompleteHook(b.plugin, nil))
}
return nil

2
pkg/volume/flexvolume/mounter.go
View File

@ -95,7 +95,7 @@ func (f *flexVolumeMounter) SetUpAt(dir string, mounterArgs volume.MounterArgs)
if !f.readOnly {
if f.plugin.capabilities.FSGroup {
// fullPluginName helps to distinguish different driver from flex volume plugin
volume.SetVolumeOwnership(f, mounterArgs.FsGroup, mounterArgs.FSGroupChangePolicy, util.FSGroupCompleteHook(f.plugin, f.spec))
volume.SetVolumeOwnership(f, dir, mounterArgs.FsGroup, mounterArgs.FSGroupChangePolicy, util.FSGroupCompleteHook(f.plugin, f.spec))
}
}

2
pkg/volume/gcepd/gce_pd.go
View File

@ -430,7 +430,7 @@ func (b *gcePersistentDiskMounter) SetUpAt(dir string, mounterArgs volume.Mounte
klog.V(4).Infof("mount of disk %s succeeded", dir)
if !b.readOnly {
if err := volume.SetVolumeOwnership(b, mounterArgs.FsGroup, mounterArgs.FSGroupChangePolicy, util.FSGroupCompleteHook(b.plugin, nil)); err != nil {
if err := volume.SetVolumeOwnership(b, dir, mounterArgs.FsGroup, mounterArgs.FSGroupChangePolicy, util.FSGroupCompleteHook(b.plugin, nil)); err != nil {
klog.Errorf("SetVolumeOwnership returns error %v", err)
}
}

2
pkg/volume/git_repo/git_repo.go
View File

@ -235,7 +235,7 @@ func (b *gitRepoVolumeMounter) SetUpAt(dir string, mounterArgs volume.MounterArg
return fmt.Errorf("failed to exec 'git reset --hard': %s: %v", output, err)
}
volume.SetVolumeOwnership(b, mounterArgs.FsGroup, nil /*fsGroupChangePolicy*/, volumeutil.FSGroupCompleteHook(b.plugin, nil))
volume.SetVolumeOwnership(b, dir, mounterArgs.FsGroup, nil /*fsGroupChangePolicy*/, volumeutil.FSGroupCompleteHook(b.plugin, nil))
volumeutil.SetReady(b.getMetaDir())
return nil

2
pkg/volume/iscsi/disk_manager.go
View File

@ -96,7 +96,7 @@ func diskSetUp(manager diskManager, b iscsiDiskMounter, volPath string, mounter
}
if !b.readOnly {
volume.SetVolumeOwnership(&b, fsGroup, fsGroupChangePolicy, util.FSGroupCompleteHook(b.plugin, nil))
volume.SetVolumeOwnership(&b, volPath, fsGroup, fsGroupChangePolicy, util.FSGroupCompleteHook(b.plugin, nil))
}
return nil

2
pkg/volume/local/local.go
View File

@ -615,7 +615,7 @@ func (m *localVolumeMounter) SetUpAt(dir string, mounterArgs volume.MounterArgs)
if !m.readOnly {
// Volume owner will be written only once on the first volume mount
if len(refs) == 0 {
return volume.SetVolumeOwnership(m, mounterArgs.FsGroup, mounterArgs.FSGroupChangePolicy, util.FSGroupCompleteHook(m.plugin, nil))
return volume.SetVolumeOwnership(m, dir, mounterArgs.FsGroup, mounterArgs.FSGroupChangePolicy, util.FSGroupCompleteHook(m.plugin, nil))
}
}
return nil

2
pkg/volume/portworx/portworx.go
View File

@ -335,7 +335,7 @@ func (b *portworxVolumeMounter) SetUpAt(dir string, mounterArgs volume.MounterAr
return err
}
if !b.readOnly {
volume.SetVolumeOwnership(b, mounterArgs.FsGroup, mounterArgs.FSGroupChangePolicy, util.FSGroupCompleteHook(b.plugin, nil))
volume.SetVolumeOwnership(b, dir, mounterArgs.FsGroup, mounterArgs.FSGroupChangePolicy, util.FSGroupCompleteHook(b.plugin, nil))
}
klog.Infof("Portworx Volume %s setup at %s", b.volumeID, dir)
return nil

2
pkg/volume/projected/projected.go
View File

@ -233,7 +233,7 @@ func (s *projectedVolumeMounter) SetUpAt(dir string, mounterArgs volume.MounterA
setPerms := func(_ string) error {
// This may be the first time writing and new files get created outside the timestamp subdirectory:
// change the permissions on the whole volume and not only in the timestamp directory.
return volume.SetVolumeOwnership(s, mounterArgs.FsGroup, nil /*fsGroupChangePolicy*/, volumeutil.FSGroupCompleteHook(s.plugin, nil))
return volume.SetVolumeOwnership(s, dir, mounterArgs.FsGroup, nil /*fsGroupChangePolicy*/, volumeutil.FSGroupCompleteHook(s.plugin, nil))
}
err = writer.Write(data, setPerms)
if err != nil {

2
pkg/volume/rbd/disk_manager.go
View File

@ -96,7 +96,7 @@ func diskSetUp(manager diskManager, b rbdMounter, volPath string, mounter mount.
klog.V(3).Infof("rbd: successfully bind mount %s to %s with options %v", globalPDPath, volPath, mountOptions)
if !b.ReadOnly {
volume.SetVolumeOwnership(&b, fsGroup, fsGroupChangePolicy, util.FSGroupCompleteHook(b.plugin, nil))
volume.SetVolumeOwnership(&b, volPath, fsGroup, fsGroupChangePolicy, util.FSGroupCompleteHook(b.plugin, nil))
}
return nil

2
pkg/volume/secret/secret.go
View File

@ -247,7 +247,7 @@ func (b *secretVolumeMounter) SetUpAt(dir string, mounterArgs volume.MounterArgs
setPerms := func(_ string) error {
// This may be the first time writing and new files get created outside the timestamp subdirectory:
// change the permissions on the whole volume and not only in the timestamp directory.
return volume.SetVolumeOwnership(b, mounterArgs.FsGroup, nil /*fsGroupChangePolicy*/, volumeutil.FSGroupCompleteHook(b.plugin, nil))
return volume.SetVolumeOwnership(b, dir, mounterArgs.FsGroup, nil /*fsGroupChangePolicy*/, volumeutil.FSGroupCompleteHook(b.plugin, nil))
}
err = writer.Write(payload, setPerms)
if err != nil {

16
pkg/volume/volume_linux.go
View File

@ -40,22 +40,22 @@ const (
// SetVolumeOwnership modifies the given volume to be owned by
// fsGroup, and sets SetGid so that newly created files are owned by
// fsGroup. If fsGroup is nil nothing is done.
func SetVolumeOwnership(mounter Mounter, fsGroup *int64, fsGroupChangePolicy *v1.PodFSGroupChangePolicy, completeFunc func(types.CompleteFuncParam)) error {
func SetVolumeOwnership(mounter Mounter, dir string, fsGroup *int64, fsGroupChangePolicy *v1.PodFSGroupChangePolicy, completeFunc func(types.CompleteFuncParam)) error {
if fsGroup == nil {
return nil
}
timer := time.AfterFunc(30*time.Second, func() {
klog.Warningf("Setting volume ownership for %s and fsGroup set. If the volume has a lot of files then setting volume ownership could be slow, see https://github.com/kubernetes/kubernetes/issues/69699", mounter.GetPath())
klog.Warningf("Setting volume ownership for %s and fsGroup set. If the volume has a lot of files then setting volume ownership could be slow, see https://github.com/kubernetes/kubernetes/issues/69699", dir)
})
defer timer.Stop()
if skipPermissionChange(mounter, fsGroup, fsGroupChangePolicy) {
klog.V(3).InfoS("Skipping permission and ownership change for volume", "path", mounter.GetPath())
if skipPermissionChange(mounter, dir, fsGroup, fsGroupChangePolicy) {
klog.V(3).InfoS("Skipping permission and ownership change for volume", "path", dir)
return nil
}
err := walkDeep(mounter.GetPath(), func(path string, info os.FileInfo, err error) error {
err := walkDeep(dir, func(path string, info os.FileInfo, err error) error {
if err != nil {
return err
}
@ -104,14 +104,12 @@ func changeFilePermission(filename string, fsGroup *int64, readonly bool, info o
return nil
}
func skipPermissionChange(mounter Mounter, fsGroup *int64, fsGroupChangePolicy *v1.PodFSGroupChangePolicy) bool {
dir := mounter.GetPath()
func skipPermissionChange(mounter Mounter, dir string, fsGroup *int64, fsGroupChangePolicy *v1.PodFSGroupChangePolicy) bool {
if fsGroupChangePolicy == nil || *fsGroupChangePolicy != v1.FSGroupChangeOnRootMismatch {
klog.V(4).InfoS("Perform recursive ownership change for directory", "path", dir)
return false
}
return !requiresPermissionChange(mounter.GetPath(), fsGroup, mounter.GetAttributes().ReadOnly)
return !requiresPermissionChange(dir, fsGroup, mounter.GetAttributes().ReadOnly)
}
func requiresPermissionChange(rootDir string, fsGroup *int64, readonly bool) bool {

8
pkg/volume/volume_linux_test.go
View File

@ -166,7 +166,7 @@ func TestSkipPermissionChange(t *testing.T) {
}
mounter := &localFakeMounter{path: tmpDir}
ok = skipPermissionChange(mounter, &expectedGid, test.fsGroupChangePolicy)
ok = skipPermissionChange(mounter, tmpDir, &expectedGid, test.fsGroupChangePolicy)
if ok != test.skipPermssion {
t.Errorf("for %s expected skipPermission to be %v got %v", test.description, test.skipPermssion, ok)
}
@ -302,8 +302,8 @@ func TestSetVolumeOwnershipMode(t *testing.T) {
t.Errorf("for %s error running setup with: %v", test.description, err)
}
mounter := &localFakeMounter{path: tmpDir}
err = SetVolumeOwnership(mounter, &expectedGid, test.fsGroupChangePolicy, nil)
mounter := &localFakeMounter{path: "FAKE_DIR_DOESNT_EXIST"} // SetVolumeOwnership() must rely on tmpDir
err = SetVolumeOwnership(mounter, tmpDir, &expectedGid, test.fsGroupChangePolicy, nil)
if err != nil {
t.Errorf("for %s error changing ownership with: %v", test.description, err)
}
@ -439,7 +439,7 @@ func TestSetVolumeOwnershipOwner(t *testing.T) {
mounter := &localFakeMounter{path: tmpDir}
always := v1.FSGroupChangeAlways
err = SetVolumeOwnership(mounter, test.fsGroup, &always, nil)
err = SetVolumeOwnership(mounter, tmpDir, test.fsGroup, &always, nil)
if err != nil {
t.Errorf("for %s error changing ownership with: %v", test.description, err)
}

2
pkg/volume/volume_unsupported.go
View File

@ -24,6 +24,6 @@ import (
"k8s.io/kubernetes/pkg/volume/util/types"
)
func SetVolumeOwnership(mounter Mounter, fsGroup *int64, fsGroupChangePolicy *v1.PodFSGroupChangePolicy, completeFunc func(types.CompleteFuncParam)) error {
func SetVolumeOwnership(mounter Mounter, dir string, fsGroup *int64, fsGroupChangePolicy *v1.PodFSGroupChangePolicy, completeFunc func(types.CompleteFuncParam)) error {
return nil
}

2
pkg/volume/vsphere_volume/vsphere_volume.go
View File

@ -277,7 +277,7 @@ func (b *vsphereVolumeMounter) SetUpAt(dir string, mounterArgs volume.MounterArg
os.Remove(dir)
return err
}
volume.SetVolumeOwnership(b, mounterArgs.FsGroup, mounterArgs.FSGroupChangePolicy, util.FSGroupCompleteHook(b.plugin, nil))
volume.SetVolumeOwnership(b, dir, mounterArgs.FsGroup, mounterArgs.FSGroupChangePolicy, util.FSGroupCompleteHook(b.plugin, nil))
klog.V(3).Infof("vSphere volume %s mounted to %s", b.volPath, dir)
return nil