Merge pull request #114464 from Nordix/issue-114461

Set permissions for timestamp directory before publishing update to avoid permission denied
This commit is contained in:
Kubernetes Prow Robot 2022-12-24 16:11:26 -08:00 committed by GitHub
commit 685d639cb5
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 116 additions and 45 deletions

View File

@ -368,7 +368,7 @@ func (cephfsVolume *cephfs) execFuseMount(mountpoint string) error {
return err
}
err = writer.Write(payload)
err = writer.Write(payload, nil /*setPerms*/)
if err != nil {
klog.Errorf("failed to write payload to dir: %v", err)
return err

View File

@ -249,17 +249,17 @@ func (b *configMapVolumeMounter) SetUpAt(dir string, mounterArgs volume.MounterA
return err
}
err = writer.Write(payload)
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))
}
err = writer.Write(payload, setPerms)
if err != nil {
klog.Errorf("Error writing payload to dir: %v", err)
return err
}
err = volume.SetVolumeOwnership(b, mounterArgs.FsGroup, nil /*fsGroupChangePolicy*/, volumeutil.FSGroupCompleteHook(b.plugin, nil))
if err != nil {
klog.Errorf("Error applying volume ownership settings for group: %v", mounterArgs.FsGroup)
return err
}
setupSuccess = true
return nil
}

View File

@ -220,18 +220,17 @@ func (b *downwardAPIVolumeMounter) SetUpAt(dir string, mounterArgs volume.Mounte
return err
}
err = writer.Write(data)
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))
}
err = writer.Write(data, setPerms)
if err != nil {
klog.Errorf("Error writing payload to dir: %v", err)
return err
}
err = volume.SetVolumeOwnership(b, mounterArgs.FsGroup, nil /*fsGroupChangePolicy*/, volumeutil.FSGroupCompleteHook(b.plugin, nil))
if err != nil {
klog.Errorf("Error applying volume ownership settings for group: %v", mounterArgs.FsGroup)
return err
}
setupSuccess = true
return nil
}

View File

@ -230,17 +230,17 @@ func (s *projectedVolumeMounter) SetUpAt(dir string, mounterArgs volume.MounterA
return err
}
err = writer.Write(data)
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))
}
err = writer.Write(data, setPerms)
if err != nil {
klog.Errorf("Error writing payload to dir: %v", err)
return err
}
err = volume.SetVolumeOwnership(s, mounterArgs.FsGroup, nil /*fsGroupChangePolicy*/, volumeutil.FSGroupCompleteHook(s.plugin, nil))
if err != nil {
klog.Errorf("Error applying volume ownership settings for group: %v", mounterArgs.FsGroup)
return err
}
setupSuccess = true
return nil
}

View File

@ -244,17 +244,17 @@ func (b *secretVolumeMounter) SetUpAt(dir string, mounterArgs volume.MounterArgs
return err
}
err = writer.Write(payload)
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))
}
err = writer.Write(payload, setPerms)
if err != nil {
klog.Errorf("Error writing payload to dir: %v", err)
return err
}
err = volume.SetVolumeOwnership(b, mounterArgs.FsGroup, nil /*fsGroupChangePolicy*/, volumeutil.FSGroupCompleteHook(b.plugin, nil))
if err != nil {
klog.Errorf("Error applying volume ownership settings for group: %v", mounterArgs.FsGroup)
return err
}
setupSuccess = true
return nil
}

View File

@ -86,11 +86,16 @@ const (
// Write does an atomic projection of the given payload into the writer's target
// directory. Input paths must not begin with '..'.
// setPerms is an optional pointer to a function that caller can provide to set the
// permissions of the newly created files before they are published. The function is
// passed subPath which is the name of the timestamped directory that was created
// under target directory.
//
// The Write algorithm is:
//
// 1. The payload is validated; if the payload is invalid, the function returns
// 2.  The current timestamped directory is detected by reading the data directory
//
// 2. The current timestamped directory is detected by reading the data directory
// symlink
//
// 3. The old version of the volume is walked to determine whether any
@ -98,13 +103,19 @@ const (
//
// 4. The data in the current timestamped directory is compared to the projected
// data to determine if an update is required.
// 5.  A new timestamped dir is created
//
// 6. The payload is written to the new timestamped directory
// 7.  A symlink to the new timestamped directory ..data_tmp is created that will
// become the new data directory
// 8.  The new data directory symlink is renamed to the data directory; rename is atomic
// 9.  Symlinks and directory for new user-visible files are created (if needed).
// 5. A new timestamped dir is created.
//
// 6. The payload is written to the new timestamped directory.
//
// 7. Permissions are set (if setPerms is not nil) on the new timestamped directory and files.
//
// 8. A symlink to the new timestamped directory ..data_tmp is created that will
// become the new data directory.
//
// 9. The new data directory symlink is renamed to the data directory; rename is atomic.
//
// 10. Symlinks and directory for new user-visible files are created (if needed).
//
// For example, consider the files:
// <target-dir>/podName
@ -123,9 +134,10 @@ const (
// linking everything else. On Windows, if a target does not exist, the created symlink
// will not work properly if the target ends up being a directory.
//
// 10. Old paths are removed from the user-visible portion of the target directory
// 11.  The previous timestamped directory is removed, if it exists
func (w *AtomicWriter) Write(payload map[string]FileProjection) error {
// 11. Old paths are removed from the user-visible portion of the target directory.
//
// 12. The previous timestamped directory is removed, if it exists.
func (w *AtomicWriter) Write(payload map[string]FileProjection, setPerms func(subPath string) error) error {
// (1)
cleanPayload, err := validatePayload(payload)
if err != nil {
@ -185,6 +197,14 @@ func (w *AtomicWriter) Write(payload map[string]FileProjection) error {
klog.V(4).Infof("%s: performed write of new data to ts data directory: %s", w.logContext, tsDir)
// (7)
if setPerms != nil {
if err := setPerms(tsDirName); err != nil {
klog.Errorf("%s: error applying ownership settings: %v", w.logContext, err)
return err
}
}
// (8)
newDataDirPath := filepath.Join(w.targetDir, newDataDirName)
if err = os.Symlink(tsDirName, newDataDirPath); err != nil {
os.RemoveAll(tsDir)
@ -192,7 +212,7 @@ func (w *AtomicWriter) Write(payload map[string]FileProjection) error {
return err
}
// (8)
// (9)
if runtime.GOOS == "windows" {
os.Remove(dataDirPath)
err = os.Symlink(tsDirName, dataDirPath)
@ -207,19 +227,19 @@ func (w *AtomicWriter) Write(payload map[string]FileProjection) error {
return err
}
// (9)
// (10)
if err = w.createUserVisibleFiles(cleanPayload); err != nil {
klog.Errorf("%s: error creating visible symlinks in %s: %v", w.logContext, w.targetDir, err)
return err
}
// (10)
// (11)
if err = w.removeUserVisiblePaths(pathsToRemove); err != nil {
klog.Errorf("%s: error removing old visible symlinks: %v", w.logContext, err)
return err
}
// (11)
// (12)
if len(oldTsDir) > 0 {
if err = os.RemoveAll(oldTsPath); err != nil {
klog.Errorf("%s: error removing old data directory %s: %v", w.logContext, oldTsDir, err)

View File

@ -21,6 +21,7 @@ package util
import (
"encoding/base64"
"fmt"
"io/ioutil"
"os"
"path/filepath"
@ -229,7 +230,7 @@ func TestPathsToRemove(t *testing.T) {
defer os.RemoveAll(targetDir)
writer := &AtomicWriter{targetDir: targetDir, logContext: "-test-"}
err = writer.Write(tc.payload1)
err = writer.Write(tc.payload1, nil)
if err != nil {
t.Errorf("%v: unexpected error writing: %v", tc.name, err)
continue
@ -397,7 +398,7 @@ IAAAAAAAsDyZDwU=`
defer os.RemoveAll(targetDir)
writer := &AtomicWriter{targetDir: targetDir, logContext: "-test-"}
err = writer.Write(tc.payload)
err = writer.Write(tc.payload, nil)
if err != nil && tc.success {
t.Errorf("%v: unexpected error writing payload: %v", tc.name, err)
continue
@ -574,7 +575,7 @@ func TestUpdate(t *testing.T) {
writer := &AtomicWriter{targetDir: targetDir, logContext: "-test-"}
err = writer.Write(tc.first)
err = writer.Write(tc.first, nil)
if err != nil {
t.Errorf("%v: unexpected error writing: %v", tc.name, err)
continue
@ -585,7 +586,7 @@ func TestUpdate(t *testing.T) {
continue
}
err = writer.Write(tc.next)
err = writer.Write(tc.next, nil)
if err != nil {
if tc.shouldWrite {
t.Errorf("%v: unexpected error writing: %v", tc.name, err)
@ -743,7 +744,7 @@ func TestMultipleUpdates(t *testing.T) {
writer := &AtomicWriter{targetDir: targetDir, logContext: "-test-"}
for _, payload := range tc.payloads {
writer.Write(payload)
writer.Write(payload, nil)
checkVolumeContents(targetDir, tc.name, payload, t)
}
@ -984,3 +985,54 @@ func TestCreateUserVisibleFiles(t *testing.T) {
}
}
}
func TestSetPerms(t *testing.T) {
targetDir, err := utiltesting.MkTmpdir("atomic-write")
if err != nil {
t.Fatalf("unexpected error creating tmp dir: %v", err)
}
defer os.RemoveAll(targetDir)
// Test that setPerms() is called once and with valid timestamp directory.
payload1 := map[string]FileProjection{
"foo/bar.txt": {Mode: 0644, Data: []byte("foo")},
"bar/zab.txt": {Mode: 0644, Data: []byte("bar")},
}
var setPermsCalled int
writer := &AtomicWriter{targetDir: targetDir, logContext: "-test-"}
err = writer.Write(payload1, func(subPath string) error {
fileInfo, err := os.Stat(filepath.Join(targetDir, subPath))
if err != nil {
t.Fatalf("unexpected error getting file info: %v", err)
}
// Ensure that given timestamp directory really exists.
if !fileInfo.IsDir() {
t.Fatalf("subPath is not a directory: %v", subPath)
}
setPermsCalled++
return nil
})
if err != nil {
t.Fatalf("unexpected error writing: %v", err)
}
if setPermsCalled != 1 {
t.Fatalf("unexpected number of calls to setPerms: %v", setPermsCalled)
}
// Test that errors from setPerms() are propagated.
payload2 := map[string]FileProjection{
"foo/bar.txt": {Mode: 0644, Data: []byte("foo2")},
"bar/zab.txt": {Mode: 0644, Data: []byte("bar2")},
}
err = writer.Write(payload2, func(_ string) error {
return fmt.Errorf("error in setPerms")
})
if err == nil {
t.Fatalf("expected error while writing but got nil")
}
if !strings.Contains(err.Error(), "error in setPerms") {
t.Fatalf("unexpected error while writing: %v", err)
}
}