JohnHammond
/
buildkit
mirror of https://github.com/JohnHammond/buildkit.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #2614 from sipsma/fix-opaque-diffapply 

diffApply: set dir opaque when overwriting whiteout
master
Erik Sipsma 2022-02-10 09:14:31 -08:00 committed by GitHub
parent a9c6a48ad0 0a2a4fe066
commit a6fc85f163
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 85 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

25
client/mergediff_test.go
View File

@ -1070,6 +1070,31 @@ func diffOpTestCases() (tests []integration.Test) {
}
}()...)
// Regression tests
tests = append(tests, func() []integration.Test {
base := func() llb.State {
return llb.Scratch().File(llb.Mkdir("/dir", 0755))
}
return []integration.Test{
verifyContents{
// Verifies that when a directory with contents is used a a base layer
// in a merge, subsequent merges that first delete the dir (resulting in
// a whiteout device w/ overlay snapshotters) and then recreate the dir
// correctly set it as opaque.
name: "TestDiffMergeOpaqueRegression",
state: llb.Merge([]llb.State{
base().File(llb.Mkfile("/dir/a", 0644, nil)),
base().File(llb.Rm("/dir")),
base().File(llb.Mkfile("/dir/b", 0644, nil)),
}),
contents: apply(
fstest.CreateDir("/dir", 0755),
fstest.CreateFile("/dir/b", nil, 0644),
),
},
}
}()...)
return tests
}

50
snapshot/diffapply_unix.go
View File

@ -30,7 +30,7 @@ import (
// that accounts for any hardlinks made from existing snapshots. ctx is expected to have a temporary lease
// associated with it.
func (sn *mergeSnapshotter) diffApply(ctx context.Context, dest Mountable, diffs ...Diff) (_ snapshots.Usage, rerr error) {
a, err := applierFor(dest, sn.tryCrossSnapshotLink)
a, err := applierFor(dest, sn.tryCrossSnapshotLink, sn.userxattr)
if err != nil {
return snapshots.Usage{}, errors.Wrapf(err, "failed to create applier")
}
@ -112,8 +112,9 @@ type change struct {
type changeApply struct {
*change
dstPath string
dstStat *syscall.Stat_t
dstPath string
dstStat *syscall.Stat_t
setOpaque bool
}
type inode struct {
@ -137,12 +138,14 @@ type applier struct {
lowerdirs []string // ordered highest -> lowest, the order we want to check them in
crossSnapshotLinks map[inode]struct{}
createWhiteoutDelete bool
userxattr bool
dirModTimes map[string]unix.Timespec // map of dstPath -> mtime that should be set on that subPath
}
func applierFor(dest Mountable, tryCrossSnapshotLink bool) (_ *applier, rerr error) {
func applierFor(dest Mountable, tryCrossSnapshotLink, userxattr bool) (_ *applier, rerr error) {
a := &applier{
dirModTimes: make(map[string]unix.Timespec),
userxattr: userxattr,
}
defer func() {
if rerr != nil {
@ -263,6 +266,12 @@ func (a *applier) applyDelete(ctx context.Context, ca *changeApply) (bool, error
return false, nil
}
if overwrite && a.createWhiteoutDelete && isWhiteoutDevice(ca.dstStat) && ca.srcStat.Mode&unix.S_IFMT == unix.S_IFDIR {
// If we are overwriting a whiteout device with a directory, we need this new dir to be opaque
// so that any files from lowerdirs under it are not visible.
ca.setOpaque = true
}
if err := os.RemoveAll(ca.dstPath); err != nil {
return false, errors.Wrap(err, "failed to remove during apply")
}
@ -376,9 +385,9 @@ func (a *applier) applyCopy(ctx context.Context, ca *changeApply) error {
}
for _, xattr := range xattrs {
if isOpaqueXattr(xattr) {
// Don't recreate opaque xattrs during merge. These should only be set when using overlay snapshotters,
// in which case we are converting from the "opaque whiteout" format to the "explicit whiteout" format during
// the merge (as taken care of by the overlay differ).
// Don't recreate opaque xattrs during merge based on the source file. The differs take care of converting
// source path from the "opaque whiteout" format to the "explicit whiteout" format. The only time we set
// opaque xattrs is handled after this loop below.
continue
}
xattrVal, err := sysx.LGetxattr(ca.srcPath, xattr)
@ -392,6 +401,14 @@ func (a *applier) applyCopy(ctx context.Context, ca *changeApply) error {
}
}
if ca.setOpaque {
// This is set in the case where we are creating a directory that is replacing a whiteout device
xattr := opaqueXattr(a.userxattr)
if err := sysx.LSetxattr(ca.dstPath, xattr, []byte{'y'}, 0); err != nil {
return errors.Wrapf(err, "failed to set opaque xattr %q of path %s", xattr, ca.dstPath)
}
}
if err := os.Lchown(ca.dstPath, int(ca.srcStat.Uid), int(ca.srcStat.Gid)); err != nil {
return errors.Wrap(err, "failed to chown during apply")
}
@ -773,8 +790,13 @@ func safeJoin(root, path string) (string, error) {
return filepath.Join(parent, base), nil
}
const (
trustedOpaqueXattr = "trusted.overlay.opaque"
userOpaqueXattr = "user.overlay.opaque"
)
func isOpaqueXattr(s string) bool {
for _, k := range []string{"trusted.overlay.opaque", "user.overlay.opaque"} {
for _, k := range []string{trustedOpaqueXattr, userOpaqueXattr} {
if s == k {
return true
}
@ -782,6 +804,13 @@ func isOpaqueXattr(s string) bool {
return false
}
func opaqueXattr(userxattr bool) string {
if userxattr {
return userOpaqueXattr
}
return trustedOpaqueXattr
}
// needsUserXAttr checks whether overlay mounts should be provided the userxattr option. We can't use
// NeedsUserXAttr from the overlayutils package directly because we don't always have direct knowledge
// of the root of the snapshotter state (such as when using a remote snapshotter). Instead, we create
@ -820,3 +849,8 @@ func needsUserXAttr(ctx context.Context, sn Snapshotter, lm leases.Manager) (boo
}
return userxattr, nil
}
func isWhiteoutDevice(st *syscall.Stat_t) bool {
// it's a whiteout if it's a char device and has a major/minor of 0/0
return st != nil && st.Mode&unix.S_IFMT == unix.S_IFCHR && st.Rdev == unix.Mkdev(0, 0)
}

24
snapshot/merge.go
View File

@ -58,9 +58,12 @@ type mergeSnapshotter struct {
// Whether we should try to implement merges by hardlinking between underlying directories
tryCrossSnapshotLink bool
// Whether the snapshotter is overlay-based, which enables some some optimizations like
// using the first merge input as the parent snapshot.
overlayBased bool
// Whether the optimization of preparing on top of base layers is supported (see Merge method).
skipBaseLayers bool
// Whether we should use the "user.*" namespace when writing overlay xattrs. If false,
// "trusted.*" is used instead.
userxattr bool
}
func NewMergeSnapshotter(ctx context.Context, sn Snapshotter, lm leases.Manager) MergeSnapshotter {
@ -68,16 +71,24 @@ func NewMergeSnapshotter(ctx context.Context, sn Snapshotter, lm leases.Manager)
_, tryCrossSnapshotLink := hardlinkMergeSnapshotters[name]
_, overlayBased := overlayBasedSnapshotters[name]
skipBaseLayers := overlayBased // default to skipping base layer for overlay-based snapshotters
var userxattr bool
if overlayBased && userns.RunningInUserNS() {
// When using an overlay-based snapshotter, if we are running rootless on a pre-5.11
// kernel, we will not have userxattr. This results in opaque xattrs not being visible
// to us and thus breaking the overlay-optimized differ.
userxattr, err := needsUserXAttr(ctx, sn, lm)
var err error
userxattr, err = needsUserXAttr(ctx, sn, lm)
if err != nil {
bklog.G(ctx).Debugf("failed to check user xattr: %v", err)
tryCrossSnapshotLink = false
skipBaseLayers = false
} else {
tryCrossSnapshotLink = userxattr
// Disable skipping base layers when in pre-5.11 rootless mode. Skipping the base layers
// necessitates the ability to set opaque xattrs sometimes, which only works in 5.11+
// kernels that support userxattr.
skipBaseLayers = userxattr
}
}
@ -85,13 +96,14 @@ func NewMergeSnapshotter(ctx context.Context, sn Snapshotter, lm leases.Manager)
Snapshotter: sn,
lm: lm,
tryCrossSnapshotLink: tryCrossSnapshotLink,
overlayBased: overlayBased,
skipBaseLayers: skipBaseLayers,
userxattr: userxattr,
}
}
func (sn *mergeSnapshotter) Merge(ctx context.Context, key string, diffs []Diff, opts ...snapshots.Opt) error {
var baseKey string
if sn.overlayBased {
if sn.skipBaseLayers {
// Overlay-based snapshotters can skip the base snapshot of the merge (if one exists) and just use it as the
// parent of the merge snapshot. Other snapshotters will start empty (with baseKey set to "").
// Find the baseKey by following the chain of diffs for as long as it follows the pattern of the current lower