Browse Source

pkg/chrootarchive: replace system.MkdirAll for os.Mkdir

system.MkdirAll is a special version of os.Mkdir to handle creating directories
using Windows volume paths ("\\?\Volume{4c1b02c1-d990-11dc-99ae-806e6f6e6963}").
This may be important when MkdirAll is used, which traverses all parent paths to
create them if missing (ultimately landing on the "volume" path).

Commit 62f648b06137a7e21b800f63ac403f7bb4d4f5b4 introduced the system.MkdirAll
calls, as a change was made in applyLayer() for Windows to use Windows volume
paths as an alternative for chroot (which is not supported on Windows). Later
iteractions changed this to regular Windows long-paths (`\\?\<path>`) in
230cfc6ed21e9398b9b3df765e6c02e90031d728, and 9b648dfac6453de5944ee4bb749115d85a253a05.
Such paths are handled by the `os` package.

However, in these tests, the parent path already exists (all paths created are
a direct subdirectory within `tmpDir`). It looks like `MkdirAll` here is used
out of convenience to not have to handle `os.ErrExist` errors. As all these
tests are running in a fresh temporary directory, there should be no need to
handle those, and it's actually desirable to produce an error in that case, as
the directory already existing would be unexpected.

Because of the above, this test changes `system.MkdirAll` to `os.Mkdir`. As we
are changing these lines, this patch also changes the legacy octal notation
(`0700`) to the now preferred `0o700`.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Sebastiaan van Stijn 2 năm trước cách đây
mục cha
commit
dee3f716b3
2 tập tin đã thay đổi với 16 bổ sung17 xóa
  1. 12 13
      pkg/chrootarchive/archive_test.go
  2. 4 4
      pkg/chrootarchive/archive_unix_test.go

+ 12 - 13
pkg/chrootarchive/archive_test.go

@@ -15,7 +15,6 @@ import (
 	"github.com/docker/docker/pkg/archive"
 	"github.com/docker/docker/pkg/idtools"
 	"github.com/docker/docker/pkg/reexec"
-	"github.com/docker/docker/pkg/system"
 	"gotest.tools/v3/skip"
 )
 
@@ -45,7 +44,7 @@ func TestChrootTarUntar(t *testing.T) {
 	skip.If(t, os.Getuid() != 0, "skipping test that requires root")
 	tmpdir := t.TempDir()
 	src := filepath.Join(tmpdir, "src")
-	if err := system.MkdirAll(src, 0700); err != nil {
+	if err := os.Mkdir(src, 0o700); err != nil {
 		t.Fatal(err)
 	}
 	if err := os.WriteFile(filepath.Join(src, "toto"), []byte("hello toto"), 0644); err != nil {
@@ -59,7 +58,7 @@ func TestChrootTarUntar(t *testing.T) {
 		t.Fatal(err)
 	}
 	dest := filepath.Join(tmpdir, "dest")
-	if err := system.MkdirAll(dest, 0700); err != nil {
+	if err := os.Mkdir(dest, 0o700); err != nil {
 		t.Fatal(err)
 	}
 	if err := Untar(stream, dest, &archive.TarOptions{ExcludePatterns: []string{"lolo"}}); err != nil {
@@ -73,7 +72,7 @@ func TestChrootUntarWithHugeExcludesList(t *testing.T) {
 	skip.If(t, os.Getuid() != 0, "skipping test that requires root")
 	tmpdir := t.TempDir()
 	src := filepath.Join(tmpdir, "src")
-	if err := system.MkdirAll(src, 0700); err != nil {
+	if err := os.Mkdir(src, 0o700); err != nil {
 		t.Fatal(err)
 	}
 	if err := os.WriteFile(filepath.Join(src, "toto"), []byte("hello toto"), 0644); err != nil {
@@ -84,7 +83,7 @@ func TestChrootUntarWithHugeExcludesList(t *testing.T) {
 		t.Fatal(err)
 	}
 	dest := filepath.Join(tmpdir, "dest")
-	if err := system.MkdirAll(dest, 0700); err != nil {
+	if err := os.Mkdir(dest, 0o700); err != nil {
 		t.Fatal(err)
 	}
 	options := &archive.TarOptions{}
@@ -165,7 +164,7 @@ func TestChrootTarUntarWithSymlink(t *testing.T) {
 	skip.If(t, os.Getuid() != 0, "skipping test that requires root")
 	tmpdir := t.TempDir()
 	src := filepath.Join(tmpdir, "src")
-	if err := system.MkdirAll(src, 0700); err != nil {
+	if err := os.Mkdir(src, 0o700); err != nil {
 		t.Fatal(err)
 	}
 	if _, err := prepareSourceDirectory(10, src, false); err != nil {
@@ -185,7 +184,7 @@ func TestChrootCopyWithTar(t *testing.T) {
 	skip.If(t, os.Getuid() != 0, "skipping test that requires root")
 	tmpdir := t.TempDir()
 	src := filepath.Join(tmpdir, "src")
-	if err := system.MkdirAll(src, 0700); err != nil {
+	if err := os.Mkdir(src, 0o700); err != nil {
 		t.Fatal(err)
 	}
 	if _, err := prepareSourceDirectory(10, src, true); err != nil {
@@ -228,7 +227,7 @@ func TestChrootCopyFileWithTar(t *testing.T) {
 	skip.If(t, os.Getuid() != 0, "skipping test that requires root")
 	tmpdir := t.TempDir()
 	src := filepath.Join(tmpdir, "src")
-	if err := system.MkdirAll(src, 0700); err != nil {
+	if err := os.Mkdir(src, 0o700); err != nil {
 		t.Fatal(err)
 	}
 	if _, err := prepareSourceDirectory(10, src, true); err != nil {
@@ -269,7 +268,7 @@ func TestChrootUntarPath(t *testing.T) {
 	skip.If(t, os.Getuid() != 0, "skipping test that requires root")
 	tmpdir := t.TempDir()
 	src := filepath.Join(tmpdir, "src")
-	if err := system.MkdirAll(src, 0700); err != nil {
+	if err := os.Mkdir(src, 0o700); err != nil {
 		t.Fatal(err)
 	}
 	if _, err := prepareSourceDirectory(10, src, false); err != nil {
@@ -327,7 +326,7 @@ func TestChrootUntarEmptyArchiveFromSlowReader(t *testing.T) {
 	skip.If(t, os.Getuid() != 0, "skipping test that requires root")
 	tmpdir := t.TempDir()
 	dest := filepath.Join(tmpdir, "dest")
-	if err := system.MkdirAll(dest, 0700); err != nil {
+	if err := os.Mkdir(dest, 0o700); err != nil {
 		t.Fatal(err)
 	}
 	stream := &slowEmptyTarReader{size: 10240, chunkSize: 1024}
@@ -340,7 +339,7 @@ func TestChrootApplyEmptyArchiveFromSlowReader(t *testing.T) {
 	skip.If(t, os.Getuid() != 0, "skipping test that requires root")
 	tmpdir := t.TempDir()
 	dest := filepath.Join(tmpdir, "dest")
-	if err := system.MkdirAll(dest, 0700); err != nil {
+	if err := os.Mkdir(dest, 0o700); err != nil {
 		t.Fatal(err)
 	}
 	stream := &slowEmptyTarReader{size: 10240, chunkSize: 1024}
@@ -353,7 +352,7 @@ func TestChrootApplyDotDotFile(t *testing.T) {
 	skip.If(t, os.Getuid() != 0, "skipping test that requires root")
 	tmpdir := t.TempDir()
 	src := filepath.Join(tmpdir, "src")
-	if err := system.MkdirAll(src, 0700); err != nil {
+	if err := os.Mkdir(src, 0o700); err != nil {
 		t.Fatal(err)
 	}
 	if err := os.WriteFile(filepath.Join(src, "..gitme"), []byte(""), 0644); err != nil {
@@ -364,7 +363,7 @@ func TestChrootApplyDotDotFile(t *testing.T) {
 		t.Fatal(err)
 	}
 	dest := filepath.Join(tmpdir, "dest")
-	if err := system.MkdirAll(dest, 0700); err != nil {
+	if err := os.Mkdir(dest, 0o700); err != nil {
 		t.Fatal(err)
 	}
 	if _, err := ApplyLayer(dest, stream); err != nil {

+ 4 - 4
pkg/chrootarchive/archive_unix_test.go

@@ -29,7 +29,7 @@ func TestUntarWithMaliciousSymlinks(t *testing.T) {
 
 	root := filepath.Join(dir, "root")
 
-	err := os.MkdirAll(root, 0o755)
+	err := os.Mkdir(root, 0o755)
 	assert.NilError(t, err)
 
 	// Add a file into a directory above root
@@ -42,7 +42,7 @@ func TestUntarWithMaliciousSymlinks(t *testing.T) {
 	// Before this change, the copy would overwrite the "host" content.
 	// With this change it should not.
 	data := filepath.Join(dir, "data")
-	err = os.MkdirAll(data, 0755)
+	err = os.Mkdir(data, 0o755)
 	assert.NilError(t, err)
 	err = os.WriteFile(filepath.Join(data, "local-file"), []byte("pwn3d"), 0644)
 	assert.NilError(t, err)
@@ -92,7 +92,7 @@ func TestTarWithMaliciousSymlinks(t *testing.T) {
 
 	root := filepath.Join(dir, "root")
 
-	err = os.MkdirAll(root, 0755)
+	err = os.Mkdir(root, 0o755)
 	assert.NilError(t, err)
 
 	hostFileData := []byte("I am a host file")
@@ -107,7 +107,7 @@ func TestTarWithMaliciousSymlinks(t *testing.T) {
 	assert.NilError(t, err)
 
 	data := filepath.Join(dir, "data")
-	err = os.MkdirAll(data, 0755)
+	err = os.Mkdir(data, 0o755)
 	assert.NilError(t, err)
 
 	type testCase struct {