Skip to content

Commit 656771c

Browse files
nalindnalind
authored
Merge pull request #6778 from nickjwhite/better-chmod
Add support for symbolic chmod options
2 parents 3695d9e + df9272b commit 656771c

17 files changed

Lines changed: 1004 additions & 43 deletions

File tree

‎add.go‎

Lines changed: 13 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import (
2626
v1 "github.com/opencontainers/image-spec/specs-go/v1"
2727
"github.com/opencontainers/runtime-spec/specs-go"
2828
"github.com/sirupsen/logrus"
29+
"github.com/tonistiigi/dchapes-mode"
2930
"go.podman.io/buildah/copier"
3031
"go.podman.io/buildah/define"
3132
"go.podman.io/buildah/internal/tmpdir"
@@ -128,7 +129,7 @@ type AddAndCopyOptions struct {
128129
}
129130

130131
// getURL writes a tar archive containing the named content
131-
func getURL(src string, chown *idtools.IDPair, mountpoint, renameTarget string, writer io.Writer, chmod *os.FileMode, srcDigest digest.Digest, certPath string, insecureSkipTLSVerify types.OptionalBool, timestamp *time.Time) error {
132+
func getURL(src string, chown *idtools.IDPair, mountpoint, renameTarget string, writer io.Writer, chmod string, srcDigest digest.Digest, certPath string, insecureSkipTLSVerify types.OptionalBool, timestamp *time.Time) error {
132133
url, err := url.Parse(src)
133134
if err != nil {
134135
return err
@@ -217,17 +218,21 @@ func getURL(src string, chown *idtools.IDPair, mountpoint, renameTarget string,
217218
uid = chown.UID
218219
gid = chown.GID
219220
}
220-
var mode int64 = 0o600
221-
if chmod != nil {
222-
mode = int64(*chmod)
221+
var mod int64 = 0o600
222+
if chmod != "" {
223+
p, err := mode.Parse(chmod)
224+
if err != nil {
225+
return fmt.Errorf("parsing chmod %q: %w", chmod, err)
226+
}
227+
mod = int64(p.Apply(os.FileMode(mod)))
223228
}
224229
hdr := tar.Header{
225230
Typeflag: tar.TypeReg,
226231
Name: name,
227232
Size: size,
228233
Uid: uid,
229234
Gid: gid,
230-
Mode: mode,
235+
Mode: mod,
231236
ModTime: date,
232237
}
233238
err = tw.WriteHeader(&hdr)
@@ -411,15 +416,6 @@ func (b *Builder) Add(destination string, extract bool, options AddAndCopyOption
411416
return fmt.Errorf("looking up UID/GID for %q: %w", options.Chown, err)
412417
}
413418
}
414-
var chmodDirsFiles *os.FileMode
415-
if options.Chmod != "" {
416-
p, err := strconv.ParseUint(options.Chmod, 8, 32)
417-
if err != nil {
418-
return fmt.Errorf("parsing chmod %q: %w", options.Chmod, err)
419-
}
420-
perm := os.FileMode(p)
421-
chmodDirsFiles = &perm
422-
}
423419

424420
chownDirs = &idtools.IDPair{UID: int(userUID), GID: int(userGID)}
425421
chownFiles = &idtools.IDPair{UID: int(userUID), GID: int(userGID)}
@@ -613,10 +609,9 @@ func (b *Builder) Add(destination string, extract bool, options AddAndCopyOption
613609
GIDMap: srcGIDMap,
614610
Excludes: options.Excludes,
615611
ExpandArchives: extract,
612+
Chmod: options.Chmod,
616613
ChownDirs: chownDirs,
617-
ChmodDirs: chmodDirsFiles,
618614
ChownFiles: chownFiles,
619-
ChmodFiles: chmodDirsFiles,
620615
KeepDirectoryNames: options.DirCopyContents == types.OptionalBoolFalse,
621616
NoDerefSymlinks: options.FollowSymlink == types.OptionalBoolFalse,
622617
StripSetuidBit: options.StripSetuidBit,
@@ -631,7 +626,7 @@ func (b *Builder) Add(destination string, extract bool, options AddAndCopyOption
631626
} else {
632627
go func() {
633628
getErr = retry.IfNecessary(context.TODO(), func() error {
634-
return getURL(src, chownFiles, mountPoint, renameTarget, pipeWriter, chmodDirsFiles, srcDigest, options.CertPath, options.InsecureSkipTLSVerify, options.Timestamp)
629+
return getURL(src, chownFiles, mountPoint, renameTarget, pipeWriter, options.Chmod, srcDigest, options.CertPath, options.InsecureSkipTLSVerify, options.Timestamp)
635630
}, &retry.Options{
636631
MaxRetry: options.MaxRetries,
637632
Delay: options.RetryDelay,
@@ -782,10 +777,9 @@ func (b *Builder) Add(destination string, extract bool, options AddAndCopyOption
782777
GIDMap: srcGIDMap,
783778
Excludes: options.Excludes,
784779
ExpandArchives: extract,
780+
Chmod: options.Chmod,
785781
ChownDirs: chownDirs,
786-
ChmodDirs: chmodDirsFiles,
787782
ChownFiles: chownFiles,
788-
ChmodFiles: chmodDirsFiles,
789783
KeepDirectoryNames: options.DirCopyContents == types.OptionalBoolFalse,
790784
StripSetuidBit: options.StripSetuidBit,
791785
StripSetgidBit: options.StripSetgidBit,

‎commit_test.go‎

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -575,3 +575,88 @@ func TestCommitEmpty(t *testing.T) {
575575
require.Equalf(t, layerDigest.Digest(), image.RootFS.DiffIDs[len(image.RootFS.DiffIDs)-1], "expected new diff ID to match the randomly-generated layer")
576576
})
577577
}
578+
579+
func TestCommitChmod(t *testing.T) {
580+
ctx := context.TODO()
581+
graphDriverName := os.Getenv("STORAGE_DRIVER")
582+
if graphDriverName == "" {
583+
graphDriverName = "vfs"
584+
}
585+
t.Logf("using storage driver %q", graphDriverName)
586+
store, err := storage.GetStore(storageTypes.StoreOptions{
587+
RunRoot: t.TempDir(),
588+
GraphRoot: t.TempDir(),
589+
GraphDriverName: graphDriverName,
590+
})
591+
require.NoError(t, err, "initializing storage")
592+
t.Cleanup(func() { _, err := store.Shutdown(true); assert.NoError(t, err) })
593+
594+
// Build a from-scratch image with one layer.
595+
builderOptions := BuilderOptions{
596+
FromImage: "scratch",
597+
NamespaceOptions: []NamespaceOption{{
598+
Name: string(rspec.NetworkNamespace),
599+
Host: true,
600+
}},
601+
SystemContext: &testSystemContext,
602+
}
603+
b, err := NewBuilder(ctx, store, builderOptions)
604+
require.NoError(t, err, "creating builder")
605+
imgName := "image0"
606+
b.SetCreatedBy(imgName)
607+
608+
type perms struct {
609+
chmod string
610+
perm int64
611+
startMode os.FileMode
612+
}
613+
614+
filePerms := map[string]perms{
615+
"symbolic": {chmod: "u=rwX,go=rX", perm: 0o644},
616+
"symbolic adding mode bits": {chmod: "u=rwX,go=rX", perm: 0o755, startMode: 0o111},
617+
"octal": {chmod: "750", perm: 0o750},
618+
"octal overwrite start mode": {chmod: "644", perm: 0o644, startMode: 0o753},
619+
"clear group and other mode bits": {chmod: "go=", perm: 0o700, startMode: 0o777},
620+
}
621+
for name, v := range filePerms {
622+
f := makeFile(t, name, 0)
623+
if v.startMode != 0 {
624+
err = os.Chmod(f, v.startMode)
625+
require.NoError(t, err, "chmod", f)
626+
}
627+
err = b.Add("/", false, AddAndCopyOptions{Chmod: v.chmod}, f)
628+
require.NoError(t, err, "adding", f)
629+
}
630+
631+
commitOptions := CommitOptions{
632+
SystemContext: &testSystemContext,
633+
}
634+
ref, err := imageStorage.Transport.ParseStoreReference(store, imgName)
635+
require.NoError(t, err, "parsing reference for to-be-committed image", imgName)
636+
_, _, _, err = b.Commit(ctx, ref, commitOptions)
637+
require.NoError(t, err, "committing", imgName)
638+
639+
src, err := ref.NewImageSource(ctx, &testSystemContext)
640+
require.NoError(t, err, "opening image source")
641+
defer src.Close()
642+
img, err := ref.NewImage(ctx, &testSystemContext)
643+
require.NoError(t, err, "opening image")
644+
defer img.Close()
645+
646+
infos, err := img.LayerInfosForCopy(ctx)
647+
require.NoError(t, err, "getting layer infos")
648+
649+
for i, blobInfo := range infos {
650+
rc, _, err := src.GetBlob(ctx, blobInfo, nil)
651+
require.NoError(t, err, "getting blob", i)
652+
defer rc.Close()
653+
tr := tar.NewReader(rc)
654+
entry, err := tr.Next()
655+
for entry != nil {
656+
expected := filePerms[entry.Name]
657+
require.Equal(t, expected.perm, entry.Mode)
658+
entry, err = tr.Next()
659+
}
660+
require.ErrorIs(t, err, io.EOF)
661+
}
662+
}

‎copier/copier.go‎

Lines changed: 40 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323
"unicode"
2424

2525
"github.com/sirupsen/logrus"
26+
"github.com/tonistiigi/dchapes-mode"
2627
"go.podman.io/image/v5/pkg/compression"
2728
"go.podman.io/image/v5/types"
2829
"go.podman.io/storage/pkg/archive"
@@ -386,6 +387,7 @@ type GetOptions struct {
386387
UIDMap, GIDMap []idtools.IDMap // map from hostIDs to containerIDs in the output archive
387388
Excludes []string // contents to pretend don't exist, using the OS-specific path separator
388389
ExpandArchives bool // extract the contents of named items that are archives
390+
Chmod string // set permissions in octal or symbolic notation. overrides ChmodDirs and ChmodFiles if set. no effect on archives being extracted
389391
ChownDirs *idtools.IDPair // set ownership on directories. no effect on archives being extracted
390392
ChmodDirs *os.FileMode // set permissions on directories. no effect on archives being extracted
391393
ChownFiles *idtools.IDPair // set ownership of files. no effect on archives being extracted
@@ -444,7 +446,8 @@ func Get(root string, directory string, options GetOptions, globs []string, bulk
444446
type PutOptions struct {
445447
UIDMap, GIDMap []idtools.IDMap // map from containerIDs to hostIDs when writing contents to disk
446448
DefaultDirOwner *idtools.IDPair // set ownership of implicitly-created directories, default is ChownDirs, or 0:0 if ChownDirs not set
447-
DefaultDirMode *os.FileMode // set permissions on implicitly-created directories, default is ChmodDirs, or 0755 if ChmodDirs not set
449+
DefaultDirMode *os.FileMode // set permissions on implicitly-created directories, default is Chmod or ChmodDirs, or 0755 if neither is set
450+
Chmod string // set permissions in octal or symbolic notation. overrides ChmodDirs and ChmodFiles if set
448451
ChownDirs *idtools.IDPair // set ownership of newly-created directories
449452
ChmodDirs *os.FileMode // set permissions on newly-created directories
450453
ChownFiles *idtools.IDPair // set ownership of newly-created files
@@ -1408,6 +1411,14 @@ func copierHandlerGet(bulkWriter io.Writer, req request, pm *fileutils.PatternMa
14081411
if err != nil {
14091412
return errorResponse("copier: get: error reading info about directory %q: %v", req.Directory, err)
14101413
}
1414+
var chmod *mode.Set
1415+
if req.GetOptions.Chmod != "" {
1416+
p, err := mode.Parse(req.GetOptions.Chmod)
1417+
if err != nil {
1418+
return errorResponse("copier: get: parsing chmod %q: %v", req.GetOptions.Chmod, err)
1419+
}
1420+
chmod = &p
1421+
}
14111422
cb := func() error {
14121423
tw := tar.NewWriter(bulkWriter)
14131424
defer tw.Close()
@@ -1461,7 +1472,7 @@ func copierHandlerGet(bulkWriter io.Writer, req request, pm *fileutils.PatternMa
14611472
return fmt.Errorf("copier: get: %w", err)
14621473
}
14631474

1464-
if err := copierHandlerGetOne(parentInfo, parentSymlinkTarget, parentName, parent, req.GetOptions, tw, hardlinkChecker, idMappings); err != nil {
1475+
if err := copierHandlerGetOne(parentInfo, parentSymlinkTarget, parentName, parent, req.GetOptions, tw, hardlinkChecker, idMappings, chmod); err != nil {
14651476
if req.GetOptions.IgnoreUnreadable && errorIsPermission(err) {
14661477
continue
14671478
} else if errors.Is(err, os.ErrNotExist) {
@@ -1584,7 +1595,7 @@ func copierHandlerGet(bulkWriter io.Writer, req request, pm *fileutils.PatternMa
15841595
}
15851596
}
15861597
// add the item to the outgoing tar stream
1587-
if err := copierHandlerGetOne(info, symlinkTarget, rel, path, options, tw, hardlinkChecker, idMappings); err != nil {
1598+
if err := copierHandlerGetOne(info, symlinkTarget, rel, path, options, tw, hardlinkChecker, idMappings, chmod); err != nil {
15881599
if req.GetOptions.IgnoreUnreadable && errorIsPermission(err) {
15891600
return ok
15901601
} else if errors.Is(err, os.ErrNotExist) {
@@ -1626,7 +1637,7 @@ func copierHandlerGet(bulkWriter io.Writer, req request, pm *fileutils.PatternMa
16261637
return fmt.Errorf("copier: get: %w", err)
16271638
}
16281639

1629-
if err := copierHandlerGetOne(info, symlinkTarget, name, item, req.GetOptions, tw, hardlinkChecker, idMappings); err != nil {
1640+
if err := copierHandlerGetOne(info, symlinkTarget, name, item, req.GetOptions, tw, hardlinkChecker, idMappings, chmod); err != nil {
16301641
if req.GetOptions.IgnoreUnreadable && errorIsPermission(err) {
16311642
continue
16321643
}
@@ -1697,7 +1708,7 @@ func getTargetIfSymlink(path string, info os.FileInfo) (string, error) {
16971708
return "", nil
16981709
}
16991710

1700-
func copierHandlerGetOne(srcfi os.FileInfo, symlinkTarget, name, contentPath string, options GetOptions, tw *tar.Writer, hardlinkChecker *hardlinkChecker, idMappings *idtools.IDMappings) error {
1711+
func copierHandlerGetOne(srcfi os.FileInfo, symlinkTarget, name, contentPath string, options GetOptions, tw *tar.Writer, hardlinkChecker *hardlinkChecker, idMappings *idtools.IDMappings, chmod *mode.Set) error {
17011712
// build the header using the name provided
17021713
hdr, err := tar.FileInfoHeader(srcfi, symlinkTarget)
17031714
if err != nil {
@@ -1807,11 +1818,14 @@ func copierHandlerGetOne(srcfi os.FileInfo, symlinkTarget, name, contentPath str
18071818
}
18081819
}
18091820
// force ownership and/or permissions, if requested
1821+
if chmod != nil {
1822+
hdr.Mode = int64(chmod.Apply(srcfi.Mode()))
1823+
}
18101824
if hdr.Typeflag == tar.TypeDir {
18111825
if options.ChownDirs != nil {
18121826
hdr.Uid, hdr.Gid = options.ChownDirs.UID, options.ChownDirs.GID
18131827
}
1814-
if options.ChmodDirs != nil {
1828+
if options.ChmodDirs != nil && chmod == nil {
18151829
hdr.Mode = int64(*options.ChmodDirs)
18161830
}
18171831
if !strings.HasSuffix(hdr.Name, "/") {
@@ -1821,7 +1835,7 @@ func copierHandlerGetOne(srcfi os.FileInfo, symlinkTarget, name, contentPath str
18211835
if options.ChownFiles != nil {
18221836
hdr.Uid, hdr.Gid = options.ChownFiles.UID, options.ChownFiles.GID
18231837
}
1824-
if options.ChmodFiles != nil {
1838+
if options.ChmodFiles != nil && chmod == nil {
18251839
hdr.Mode = int64(*options.ChmodFiles)
18261840
}
18271841
}
@@ -1887,6 +1901,15 @@ func copierHandlerPut(bulkReader io.Reader, req request, idMappings *idtools.IDM
18871901
if req.PutOptions.ChmodDirs != nil {
18881902
defaultDirMode = *req.PutOptions.ChmodDirs
18891903
}
1904+
var chmod *mode.Set
1905+
if req.PutOptions.Chmod != "" {
1906+
p, err := mode.Parse(req.PutOptions.Chmod)
1907+
if err != nil {
1908+
return errorResponse("parsing chmod %q: %v", req.PutOptions.Chmod, err)
1909+
}
1910+
chmod = &p
1911+
defaultDirMode = chmod.Apply(defaultDirMode)
1912+
}
18901913
if req.PutOptions.DefaultDirOwner != nil {
18911914
defaultDirUID, defaultDirGID = req.PutOptions.DefaultDirOwner.UID, req.PutOptions.DefaultDirOwner.GID
18921915
}
@@ -2078,13 +2101,17 @@ func copierHandlerPut(bulkReader io.Reader, req request, idMappings *idtools.IDM
20782101
if req.PutOptions.StripStickyBit && hdr.Mode&cISVTX == cISVTX {
20792102
hdr.Mode &^= cISVTX
20802103
}
2081-
if hdr.Typeflag == tar.TypeDir {
2082-
if req.PutOptions.ChmodDirs != nil {
2083-
hdr.Mode = int64(*req.PutOptions.ChmodDirs)
2084-
}
2104+
if chmod != nil {
2105+
hdr.Mode = int64(chmod.Apply(os.FileMode(hdr.Mode)))
20852106
} else {
2086-
if req.PutOptions.ChmodFiles != nil {
2087-
hdr.Mode = int64(*req.PutOptions.ChmodFiles)
2107+
if hdr.Typeflag == tar.TypeDir {
2108+
if req.PutOptions.ChmodDirs != nil {
2109+
hdr.Mode = int64(*req.PutOptions.ChmodDirs)
2110+
}
2111+
} else {
2112+
if req.PutOptions.ChmodFiles != nil {
2113+
hdr.Mode = int64(*req.PutOptions.ChmodFiles)
2114+
}
20882115
}
20892116
}
20902117
// create the new item

0 commit comments

Comments
 (0)