Skip to content

Commit 298e936

Browse files
nalindnalind
committed
drivers.ChownPathByMaps(): preserve directory ModTimes
When changing the ownership throughout a directory tree, make notes of the initial values of the ModTime for directories that we process. When removing and re-creating hard links to pick up an ownership change, or changing the owner of an directory, note the name of the directory containing the hard link, or of the directory itself. After changing the ownership throughout a directory tree, walk the list of directories whose timestamps we expect to have modified, and if their ModTime values were changed, restore them. Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
1 parent 38415f0 commit 298e936

6 files changed

Lines changed: 147 additions & 10 deletions

File tree

‎storage/drivers/chown.go‎

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,13 @@ import (
66
"fmt"
77
"io/fs"
88
"os"
9+
"sync"
10+
"syscall"
911

1012
"github.com/opencontainers/selinux/pkg/pwalkdir"
1113
"go.podman.io/storage/pkg/idtools"
1214
"go.podman.io/storage/pkg/reexec"
15+
"go.podman.io/storage/pkg/system"
1316
)
1417

1518
const (
@@ -54,19 +57,43 @@ func chownByMapsMain() {
5457
}
5558

5659
chowner := newLChowner()
60+
var dirModTimes sync.Map
5761

5862
var chown fs.WalkDirFunc = func(path string, d fs.DirEntry, _ error) error {
5963
info, err := d.Info()
6064
if path == "." || err != nil {
6165
return nil
6266
}
67+
if info.IsDir() {
68+
dirModTimes.Store(string(path), int64(info.ModTime().UnixNano()))
69+
}
6370
return chowner.LChown(path, info, toHost, toContainer)
6471
}
6572
if err := pwalkdir.Walk(".", chown); err != nil {
6673
fmt.Fprintf(os.Stderr, "error during chown: %v", err)
6774
os.Exit(1)
6875
}
69-
os.Exit(0)
76+
exitStatus := 0
77+
chowner.modifiedDirectories.Range(func(key, _ any) bool {
78+
dir := key.(string)
79+
if value, ok := dirModTimes.Load(dir); ok {
80+
st, err := os.Lstat(dir)
81+
if err != nil {
82+
fmt.Fprintf(os.Stderr, "error during chown: %v", err)
83+
exitStatus = 1
84+
}
85+
tsCurrent := syscall.NsecToTimespec(st.ModTime().UnixNano())
86+
tsSaved := syscall.NsecToTimespec(value.(int64))
87+
if tsCurrent != tsSaved {
88+
if err := system.LUtimesNano(dir, []syscall.Timespec{tsSaved, tsSaved}); err != nil {
89+
fmt.Fprintf(os.Stderr, "error during chown: %v", err)
90+
exitStatus = 1
91+
}
92+
}
93+
}
94+
return true
95+
})
96+
os.Exit(exitStatus)
7097
}
7198

7299
// ChownPathByMaps walks the filesystem tree, changing the ownership

‎storage/drivers/chown_darwin.go‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,9 @@ type inode struct {
1919
}
2020

2121
type platformChowner struct {
22-
mutex sync.Mutex
23-
inodes map[inode]bool
22+
mutex sync.Mutex
23+
inodes map[inode]bool
24+
modifiedDirectories sync.Map
2425
}
2526

2627
func newLChowner() *platformChowner {
@@ -102,7 +103,9 @@ func (c *platformChowner) LChown(path string, info os.FileInfo, toHost, toContai
102103
return fmt.Errorf("%s: %w", os.Args[0], err)
103104
}
104105
}
105-
106+
if info.IsDir() {
107+
c.modifiedDirectories.Store(path, struct{}{})
108+
}
106109
}
107110
return nil
108111
}

‎storage/drivers/chown_unix.go‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"errors"
77
"fmt"
88
"os"
9+
"path/filepath"
910
"sync"
1011
"syscall"
1112

@@ -19,8 +20,9 @@ type inode struct {
1920
}
2021

2122
type platformChowner struct {
22-
mutex sync.Mutex
23-
inodes map[inode]string
23+
mutex sync.Mutex
24+
inodes map[inode]string
25+
modifiedDirectories sync.Map
2426
}
2527

2628
func newLChowner() *platformChowner {
@@ -60,11 +62,11 @@ func (c *platformChowner) LChown(path string, info os.FileInfo, toHost, toContai
6062
// of chowning it again. This is necessary when the underlying file system breaks
6163
// inodes on copy-up (as it is with overlay with index=off) to maintain the original
6264
// link and correct file ownership.
63-
6465
// The target already exists so remove it before creating the link to the new target.
6566
if err := os.Remove(path); err != nil {
6667
return err
6768
}
69+
c.modifiedDirectories.Store(filepath.Dir(path), struct{}{})
6870
return os.Link(oldTarget, path)
6971
}
7072

@@ -120,7 +122,9 @@ func (c *platformChowner) LChown(path string, info os.FileInfo, toHost, toContai
120122
return fmt.Errorf("%s: %w", os.Args[0], err)
121123
}
122124
}
123-
125+
if info.IsDir() {
126+
c.modifiedDirectories.Store(path, struct{}{})
127+
}
124128
}
125129
return nil
126130
}

‎storage/drivers/chown_windows.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,13 @@ package graphdriver
44

55
import (
66
"os"
7+
"sync"
78
"syscall"
89

910
"go.podman.io/storage/pkg/idtools"
1011
)
1112

12-
type platformChowner struct{}
13+
type platformChowner struct{ modifiedDirectories sync.Map }
1314

1415
func newLChowner() *platformChowner {
1516
return &platformChowner{}

‎storage/tests/chown.bats‎

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
#!/usr/bin/env bats
2+
3+
load helpers
4+
5+
@test "chown-preserves-modtimes" {
6+
# This test needs "tar".
7+
if test -z "$(which tar 2> /dev/null)" ; then
8+
skip "need tar"
9+
fi
10+
if test -z "$(which dd 2> /dev/null)" ; then
11+
skip "need dd"
12+
fi
13+
14+
# Create a tree for a layer with at least one hard link and some directories.
15+
pushd $TESTDIR > /dev/null
16+
mkdir layer layer/layer1 layer/layer1/directory layer/layer1/directory/subdirectory
17+
createrandom layer/layer1/directory/subdirectory/linktarget
18+
ln layer/layer1/directory/subdirectory/linktarget layer/layer1/directory/subdirectory/link
19+
ln -s linktarget layer/layer1/directory/subdirectory/symlink
20+
createrandom layer/layer1/directory/subdirectory/otherfile
21+
touch -d 1970-01-01T00:00:00Z layer/layer1 layer/layer1/directory layer/layer1/directory/subdirectory
22+
# Create another tree with just the directories.
23+
mkdir layer/layer2 layer/layer2/directory layer/layer2/directory/subdirectory
24+
touch -d 1970-01-01T00:00:00Z layer/layer2 layer/layer2/directory layer/layer2/directory/subdirectory
25+
popd > /dev/null
26+
27+
# Create a temporary layer.
28+
run storage --debug=false create-layer
29+
echo "$output"
30+
[ "$status" -eq 0 ]
31+
[ "$output" != "" ]
32+
templayer="$output"
33+
34+
# Copy the content into it.
35+
run storage --debug=false copy --chown 0:0 $TESTDIR/layer/layer1 "$templayer":/
36+
echo "$output"
37+
[ "$status" -eq 0 ]
38+
[ "$output" == "" ]
39+
40+
# Generate a diff with the contents.
41+
run storage --debug=false diff -f $TESTDIR/layer1.tar "$templayer"
42+
echo "$output"
43+
[ "$status" -eq 0 ]
44+
[ "$output" == "" ]
45+
46+
# Create another temporary layer.
47+
run storage --debug=false create-layer
48+
echo "$output"
49+
[ "$status" -eq 0 ]
50+
[ "$output" != "" ]
51+
templayer="$output"
52+
53+
# Copy the content into it.
54+
run storage --debug=false copy --chown 0:0 $TESTDIR/layer/layer2 "$templayer":/
55+
echo "$output"
56+
[ "$status" -eq 0 ]
57+
[ "$output" == "" ]
58+
59+
# Generate a diff with the contents.
60+
run storage --debug=false diff -f $TESTDIR/layer2.tar "$templayer"
61+
echo "$output"
62+
[ "$status" -eq 0 ]
63+
[ "$output" == "" ]
64+
65+
# Create a new layer using first diff to populate it.
66+
run storage --debug=false import-layer --name lower --file $TESTDIR/layer1.tar --uidmap 0:2:1024 --gidmap 0:2:1024
67+
echo "$output"
68+
[ "$status" -eq 0 ]
69+
[ "$output" != "" ]
70+
71+
# Create a new layer based on the one we just created, overwriting some
72+
# of its directories that will also contain items that are chown'd when
73+
# being pulled up after the directories are extracted onto disk.
74+
run storage --debug=false import-layer --name middle --file $TESTDIR/layer2.tar --uidmap 0:2:1024 --gidmap 0:2:1024 lower
75+
echo "$output"
76+
[ "$status" -eq 0 ]
77+
[ "$output" != "" ]
78+
79+
# And another one with a different ID map.
80+
run storage --debug=false import-layer --name upper --file $TESTDIR/layer2.tar --uidmap 0:3:1024 --gidmap 0:3:1024 middle
81+
echo "$output"
82+
[ "$status" -eq 0 ]
83+
[ "$output" != "" ]
84+
85+
# Create an image using that layer as its top layer.
86+
run storage --debug=false create-image --name image upper
87+
echo "$output"
88+
[ "$status" -eq 0 ]
89+
[ "$output" != "" ]
90+
91+
# Create a container using that image.
92+
run storage --debug=false create-container --name container --hostuidmap --hostgidmap image
93+
echo "$output"
94+
[ "$status" -eq 0 ]
95+
[ "$output" != "" ]
96+
97+
# Check for inconsistencies.
98+
run storage --debug=false check
99+
echo "$output"
100+
[ "$status" -eq 0 ]
101+
[ "$output" == "" ]
102+
}

‎storage/tests/helpers.bash‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ function setup() {
2222
rm -fr ${TESTDIR}
2323
mkdir -p ${TESTDIR}/{root,runroot}
2424
# disable idmapped mounts in the overlay driver, since that
25-
# is the expectation in the idmaps.bats tests.
25+
# is the expectation in the chown.bats and idmaps.bats tests.
2626
export _CONTAINERS_OVERLAY_DISABLE_IDMAP=yes
2727
}
2828

0 commit comments

Comments
 (0)