Skip to content

Add support for symbolic chmod options #6778

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
nickjwhite wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add support for symbolic chmod options #6778

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 13 additions & 19 deletions add.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
v1 "github.com/opencontainers/image-spec/specs-go/v1"
"github.com/opencontainers/runtime-spec/specs-go"
"github.com/sirupsen/logrus"
"github.com/tonistiigi/dchapes-mode"
"go.podman.io/buildah/copier"
"go.podman.io/buildah/define"
"go.podman.io/buildah/internal/tmpdir"
Expand Down Expand Up @@ -125,7 +126,7 @@ type AddAndCopyOptions struct {
}

// getURL writes a tar archive containing the named content
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 {
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 {
url, err := url.Parse(src)
if err != nil {
return err
Expand Down Expand Up @@ -214,17 +215,21 @@ func getURL(src string, chown *idtools.IDPair, mountpoint, renameTarget string,
uid = chown.UID
gid = chown.GID
}
var mode int64 = 0o600
if chmod != nil {
mode = int64(*chmod)
var mod int64 = 0o600
if chmod != "" {
p, err := mode.Parse(chmod)
if err != nil {
return fmt.Errorf("parsing chmod %q: %w", chmod, err)
}
mod = int64(p.Apply(os.FileMode(mod)))
}
hdr := tar.Header{
Typeflag: tar.TypeReg,
Name: name,
Size: size,
Uid: uid,
Gid: gid,
Mode: mode,
Mode: mod,
ModTime: date,
}
err = tw.WriteHeader(&hdr)
Expand Down Expand Up @@ -408,15 +413,6 @@ func (b *Builder) Add(destination string, extract bool, options AddAndCopyOption
return fmt.Errorf("looking up UID/GID for %q: %w", options.Chown, err)
}
}
var chmodDirsFiles *os.FileMode
if options.Chmod != "" {
p, err := strconv.ParseUint(options.Chmod, 8, 32)
if err != nil {
return fmt.Errorf("parsing chmod %q: %w", options.Chmod, err)
}
perm := os.FileMode(p)
chmodDirsFiles = &perm
}

chownDirs = &idtools.IDPair{UID: int(userUID), GID: int(userGID)}
chownFiles = &idtools.IDPair{UID: int(userUID), GID: int(userGID)}
Expand Down Expand Up @@ -609,10 +605,9 @@ func (b *Builder) Add(destination string, extract bool, options AddAndCopyOption
GIDMap: srcGIDMap,
Excludes: options.Excludes,
ExpandArchives: extract,
Chmod: options.Chmod,
ChownDirs: chownDirs,
ChmodDirs: chmodDirsFiles,
ChownFiles: chownFiles,
ChmodFiles: chmodDirsFiles,
KeepDirectoryNames: options.DirCopyContents == types.OptionalBoolFalse,
StripSetuidBit: options.StripSetuidBit,
StripSetgidBit: options.StripSetgidBit,
Expand All @@ -626,7 +621,7 @@ func (b *Builder) Add(destination string, extract bool, options AddAndCopyOption
} else {
go func() {
getErr = retry.IfNecessary(context.TODO(), func() error {
return getURL(src, chownFiles, mountPoint, renameTarget, pipeWriter, chmodDirsFiles, srcDigest, options.CertPath, options.InsecureSkipTLSVerify, options.Timestamp)
return getURL(src, chownFiles, mountPoint, renameTarget, pipeWriter, options.Chmod, srcDigest, options.CertPath, options.InsecureSkipTLSVerify, options.Timestamp)
}, &retry.Options{
MaxRetry: options.MaxRetries,
Delay: options.RetryDelay,
Expand Down Expand Up @@ -777,10 +772,9 @@ func (b *Builder) Add(destination string, extract bool, options AddAndCopyOption
GIDMap: srcGIDMap,
Excludes: options.Excludes,
ExpandArchives: extract,
Chmod: options.Chmod,
ChownDirs: chownDirs,
ChmodDirs: chmodDirsFiles,
ChownFiles: chownFiles,
ChmodFiles: chmodDirsFiles,
KeepDirectoryNames: options.DirCopyContents == types.OptionalBoolFalse,
StripSetuidBit: options.StripSetuidBit,
StripSetgidBit: options.StripSetgidBit,
Expand Down
85 changes: 85 additions & 0 deletions commit_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -575,3 +575,88 @@ func TestCommitEmpty(t *testing.T) {
require.Equalf(t, layerDigest.Digest(), image.RootFS.DiffIDs[len(image.RootFS.DiffIDs)-1], "expected new diff ID to match the randomly-generated layer")
})
}

func TestCommitChmod(t *testing.T) {
ctx := context.TODO()
graphDriverName := os.Getenv("STORAGE_DRIVER")
if graphDriverName == "" {
graphDriverName = "vfs"
}
t.Logf("using storage driver %q", graphDriverName)
store, err := storage.GetStore(storageTypes.StoreOptions{
RunRoot: t.TempDir(),
GraphRoot: t.TempDir(),
GraphDriverName: graphDriverName,
})
require.NoError(t, err, "initializing storage")
t.Cleanup(func() { _, err := store.Shutdown(true); assert.NoError(t, err) })

// Build a from-scratch image with one layer.
builderOptions := BuilderOptions{
FromImage: "scratch",
NamespaceOptions: []NamespaceOption{{
Name: string(rspec.NetworkNamespace),
Host: true,
}},
SystemContext: &testSystemContext,
}
b, err := NewBuilder(ctx, store, builderOptions)
require.NoError(t, err, "creating builder")
imgName := "image0"
b.SetCreatedBy(imgName)

type perms struct {
chmod string
perm int64
startMode os.FileMode
}

filePerms := map[string]perms{
"symbolic": {chmod: "u=rwX,go=rX", perm: 0o644},
"symbolic adding mode bits": {chmod: "u=rwX,go=rX", perm: 0o755, startMode: 0o111},
"octal": {chmod: "750", perm: 0o750},
"octal overwrite start mode": {chmod: "644", perm: 0o644, startMode: 0o753},
"clear group and other mode bits": {chmod: "go=", perm: 0o700, startMode: 0o777},
}
for name, v := range filePerms {
f := makeFile(t, name, 0)
if v.startMode != 0 {
err = os.Chmod(f, v.startMode)
require.NoError(t, err, "chmod", f)
}
err = b.Add("/", false, AddAndCopyOptions{Chmod: v.chmod}, f)
require.NoError(t, err, "adding", f)
}

commitOptions := CommitOptions{
SystemContext: &testSystemContext,
}
ref, err := imageStorage.Transport.ParseStoreReference(store, imgName)
require.NoError(t, err, "parsing reference for to-be-committed image", imgName)
_, _, _, err = b.Commit(ctx, ref, commitOptions)
require.NoError(t, err, "committing", imgName)

src, err := ref.NewImageSource(ctx, &testSystemContext)
require.NoError(t, err, "opening image source")
defer src.Close()
img, err := ref.NewImage(ctx, &testSystemContext)
require.NoError(t, err, "opening image")
defer img.Close()

infos, err := img.LayerInfosForCopy(ctx)
require.NoError(t, err, "getting layer infos")

for i, blobInfo := range infos {
rc, _, err := src.GetBlob(ctx, blobInfo, nil)
require.NoError(t, err, "getting blob", i)
defer rc.Close()
tr := tar.NewReader(rc)
entry, err := tr.Next()
for entry != nil {
expected := filePerms[entry.Name]
require.Equal(t, expected.perm, entry.Mode)
entry, err = tr.Next()
}
require.ErrorIs(t, err, io.EOF)
}
}
53 changes: 40 additions & 13 deletions copier/copier.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"unicode"

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

if err := copierHandlerGetOne(parentInfo, parentSymlinkTarget, parentName, parent, req.GetOptions, tw, hardlinkChecker, idMappings); err != nil {
if err := copierHandlerGetOne(parentInfo, parentSymlinkTarget, parentName, parent, req.GetOptions, tw, hardlinkChecker, idMappings, chmod); err != nil {
if req.GetOptions.IgnoreUnreadable && errorIsPermission(err) {
continue
} else if errors.Is(err, os.ErrNotExist) {
Expand Down Expand Up @@ -1584,7 +1595,7 @@ func copierHandlerGet(bulkWriter io.Writer, req request, pm *fileutils.PatternMa
}
}
// add the item to the outgoing tar stream
if err := copierHandlerGetOne(info, symlinkTarget, rel, path, options, tw, hardlinkChecker, idMappings); err != nil {
if err := copierHandlerGetOne(info, symlinkTarget, rel, path, options, tw, hardlinkChecker, idMappings, chmod); err != nil {
if req.GetOptions.IgnoreUnreadable && errorIsPermission(err) {
return ok
} else if errors.Is(err, os.ErrNotExist) {
Expand Down Expand Up @@ -1626,7 +1637,7 @@ func copierHandlerGet(bulkWriter io.Writer, req request, pm *fileutils.PatternMa
return fmt.Errorf("copier: get: %w", err)
}

if err := copierHandlerGetOne(info, symlinkTarget, name, item, req.GetOptions, tw, hardlinkChecker, idMappings); err != nil {
if err := copierHandlerGetOne(info, symlinkTarget, name, item, req.GetOptions, tw, hardlinkChecker, idMappings, chmod); err != nil {
if req.GetOptions.IgnoreUnreadable && errorIsPermission(err) {
continue
}
Expand Down Expand Up @@ -1697,7 +1708,7 @@ func getTargetIfSymlink(path string, info os.FileInfo) (string, error) {
return "", nil
}

func copierHandlerGetOne(srcfi os.FileInfo, symlinkTarget, name, contentPath string, options GetOptions, tw *tar.Writer, hardlinkChecker *hardlinkChecker, idMappings *idtools.IDMappings) error {
func copierHandlerGetOne(srcfi os.FileInfo, symlinkTarget, name, contentPath string, options GetOptions, tw *tar.Writer, hardlinkChecker *hardlinkChecker, idMappings *idtools.IDMappings, chmod *mode.Set) error {
// build the header using the name provided
hdr, err := tar.FileInfoHeader(srcfi, symlinkTarget)
if err != nil {
Expand Down Expand Up @@ -1807,11 +1818,14 @@ func copierHandlerGetOne(srcfi os.FileInfo, symlinkTarget, name, contentPath str
}
}
// force ownership and/or permissions, if requested
if chmod != nil {
hdr.Mode = int64(chmod.Apply(srcfi.Mode()))
}
if hdr.Typeflag == tar.TypeDir {
if options.ChownDirs != nil {
hdr.Uid, hdr.Gid = options.ChownDirs.UID, options.ChownDirs.GID
}
if options.ChmodDirs != nil {
if options.ChmodDirs != nil && chmod == nil {
hdr.Mode = int64(*options.ChmodDirs)
}
if !strings.HasSuffix(hdr.Name, "/") {
Expand All @@ -1821,7 +1835,7 @@ func copierHandlerGetOne(srcfi os.FileInfo, symlinkTarget, name, contentPath str
if options.ChownFiles != nil {
hdr.Uid, hdr.Gid = options.ChownFiles.UID, options.ChownFiles.GID
}
if options.ChmodFiles != nil {
if options.ChmodFiles != nil && chmod == nil {
hdr.Mode = int64(*options.ChmodFiles)
}
}
Expand Down Expand Up @@ -1887,6 +1901,15 @@ func copierHandlerPut(bulkReader io.Reader, req request, idMappings *idtools.IDM
if req.PutOptions.ChmodDirs != nil {
defaultDirMode = *req.PutOptions.ChmodDirs
}
var chmod *mode.Set
if req.PutOptions.Chmod != "" {
p, err := mode.Parse(req.PutOptions.Chmod)
if err != nil {
return errorResponse("parsing chmod %q: %v", req.PutOptions.Chmod, err)
}
chmod = &p
defaultDirMode = chmod.Apply(defaultDirMode)
}
if req.PutOptions.DefaultDirOwner != nil {
defaultDirUID, defaultDirGID = req.PutOptions.DefaultDirOwner.UID, req.PutOptions.DefaultDirOwner.GID
}
Expand Down Expand Up @@ -2078,13 +2101,17 @@ func copierHandlerPut(bulkReader io.Reader, req request, idMappings *idtools.IDM
if req.PutOptions.StripStickyBit && hdr.Mode&cISVTX == cISVTX {
hdr.Mode &^= cISVTX
}
if hdr.Typeflag == tar.TypeDir {
if req.PutOptions.ChmodDirs != nil {
hdr.Mode = int64(*req.PutOptions.ChmodDirs)
}
if chmod != nil {
hdr.Mode = int64(chmod.Apply(os.FileMode(hdr.Mode)))
} else {
if req.PutOptions.ChmodFiles != nil {
hdr.Mode = int64(*req.PutOptions.ChmodFiles)
if hdr.Typeflag == tar.TypeDir {
if req.PutOptions.ChmodDirs != nil {
hdr.Mode = int64(*req.PutOptions.ChmodDirs)
}
} else {
if req.PutOptions.ChmodFiles != nil {
hdr.Mode = int64(*req.PutOptions.ChmodFiles)
}
}
}
// create the new item
Expand Down
Loading
Loading