Skip to content
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.

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

Add --chown flag support for ADD/COPY commands for Windows #35521

Merged
merged 1 commit into from Aug 17, 2018
Merged
Changes from all commits
Commits
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.
+610 −301
Diff settings

Always

Just for now

@@ -56,21 +56,21 @@ type SessionGetter interface {

// BuildManager is shared across all Builder objects
type BuildManager struct {
idMappings *idtools.IDMappings
backend builder.Backend
pathCache pathCache // TODO: make this persistent
sg SessionGetter
fsCache *fscache.FSCache
idMapping *idtools.IdentityMapping
backend builder.Backend
pathCache pathCache // TODO: make this persistent
sg SessionGetter
fsCache *fscache.FSCache
}

// NewBuildManager creates a BuildManager
func NewBuildManager(b builder.Backend, sg SessionGetter, fsCache *fscache.FSCache, idMappings *idtools.IDMappings) (*BuildManager, error) {
func NewBuildManager(b builder.Backend, sg SessionGetter, fsCache *fscache.FSCache, identityMapping *idtools.IdentityMapping) (*BuildManager, error) {
bm := &BuildManager{
backend: b,
pathCache: &syncmap.Map{},
sg: sg,
idMappings: idMappings,
fsCache: fsCache,
backend: b,
pathCache: &syncmap.Map{},
sg: sg,
idMapping: identityMapping,
fsCache: fsCache,
}
if err := fsCache.RegisterTransport(remotecontext.ClientSessionRemote, NewClientSessionTransport()); err != nil {
return nil, err
@@ -111,7 +111,7 @@ func (bm *BuildManager) Build(ctx context.Context, config backend.BuildConfig) (
ProgressWriter: config.ProgressWriter,
Backend: bm.backend,
PathCache: bm.pathCache,
IDMappings: bm.idMappings,
IDMapping: bm.idMapping,
}
b, err := newBuilder(ctx, builderOptions)
if err != nil {
@@ -159,7 +159,7 @@ type builderOptions struct {
Backend builder.Backend
ProgressWriter backend.ProgressWriter
PathCache pathCache
IDMappings *idtools.IDMappings
IDMapping *idtools.IdentityMapping
}

// Builder is a Dockerfile builder
@@ -175,7 +175,7 @@ type Builder struct {
docker builder.Backend
clientCtx context.Context

idMappings *idtools.IDMappings
idMapping *idtools.IdentityMapping
disableCommit bool
imageSources *imageSources
pathCache pathCache
@@ -199,7 +199,7 @@ func newBuilder(clientCtx context.Context, options builderOptions) (*Builder, er
Aux: options.ProgressWriter.AuxFormatter,
Output: options.ProgressWriter.Output,
docker: options.Backend,
idMappings: options.IDMappings,
idMapping: options.IDMapping,
imageSources: newImageSources(clientCtx, options),
pathCache: options.PathCache,
imageProber: newImageProber(options.Backend, config.CacheFrom, config.NoCache),
@@ -451,7 +451,7 @@ func downloadSource(output io.Writer, stdout io.Writer, srcURL string) (remote b

type copyFileOptions struct {
decompress bool
chownPair idtools.IDPair
identity idtools.Identity
archiver Archiver
}

@@ -481,7 +481,7 @@ func performCopyForInfo(dest copyInfo, source copyInfo, options copyFileOptions)
return errors.Wrapf(err, "source path not found")
}
if src.IsDir() {
return copyDirectory(archiver, srcEndpoint, destEndpoint, options.chownPair)
return copyDirectory(archiver, srcEndpoint, destEndpoint, options.identity)
}
if options.decompress && isArchivePath(source.root, srcPath) && !source.noDecompress {
return archiver.UntarPath(srcPath, destPath)
@@ -499,7 +499,7 @@ func performCopyForInfo(dest copyInfo, source copyInfo, options copyFileOptions)
destPath = dest.root.Join(destPath, source.root.Base(source.path))
destEndpoint = &copyEndpoint{driver: dest.root, path: destPath}
}
return copyFile(archiver, srcEndpoint, destEndpoint, options.chownPair)
return copyFile(archiver, srcEndpoint, destEndpoint, options.identity)
}

func isArchivePath(driver containerfs.ContainerFS, path string) bool {
@@ -517,7 +517,7 @@ func isArchivePath(driver containerfs.ContainerFS, path string) bool {
return err == nil
}

func copyDirectory(archiver Archiver, source, dest *copyEndpoint, chownPair idtools.IDPair) error {
func copyDirectory(archiver Archiver, source, dest *copyEndpoint, identity idtools.Identity) error {
destExists, err := isExistingDirectory(dest)
if err != nil {
return errors.Wrapf(err, "failed to query destination path")
@@ -527,17 +527,17 @@ func copyDirectory(archiver Archiver, source, dest *copyEndpoint, chownPair idto
return errors.Wrapf(err, "failed to copy directory")
}
// TODO: @gupta-ak. Investigate how LCOW permission mappings will work.
return fixPermissions(source.path, dest.path, chownPair, !destExists)
return fixPermissions(source.path, dest.path, identity, !destExists)
}

func copyFile(archiver Archiver, source, dest *copyEndpoint, chownPair idtools.IDPair) error {
func copyFile(archiver Archiver, source, dest *copyEndpoint, identity idtools.Identity) error {
if runtime.GOOS == "windows" && dest.driver.OS() == "linux" {
// LCOW
if err := dest.driver.MkdirAll(dest.driver.Dir(dest.path), 0755); err != nil {
return errors.Wrapf(err, "failed to create new directory")
}
} else {
if err := idtools.MkdirAllAndChownNew(filepath.Dir(dest.path), 0755, chownPair); err != nil {
if err := idtools.MkdirAllAndChownNew(filepath.Dir(dest.path), 0755, identity); err != nil {
// Normal containers
return errors.Wrapf(err, "failed to create new directory")
}
@@ -547,7 +547,7 @@ func copyFile(archiver Archiver, source, dest *copyEndpoint, chownPair idtools.I
return errors.Wrapf(err, "failed to copy file")
}
// TODO: @gupta-ak. Investigate how LCOW permission mappings will work.
return fixPermissions(source.path, dest.path, chownPair, false)
return fixPermissions(source.path, dest.path, identity, false)
}

func endsInSlash(driver containerfs.Driver, path string) bool {
@@ -10,7 +10,7 @@ import (
"github.com/docker/docker/pkg/idtools"
)

func fixPermissions(source, destination string, rootIDs idtools.IDPair, overrideSkip bool) error {
func fixPermissions(source, destination string, identity idtools.Identity, overrideSkip bool) error {
var (
skipChownRoot bool
err error
@@ -39,7 +39,7 @@ func fixPermissions(source, destination string, rootIDs idtools.IDPair, override
}

fullpath = filepath.Join(destination, cleaned)
return os.Lchown(fullpath, rootIDs.UID, rootIDs.GID)
return os.Lchown(fullpath, identity.UID, identity.GID)
})
}

@@ -1,21 +1,87 @@
package dockerfile // import "github.com/docker/docker/builder/dockerfile"

import (
"errors"
"fmt"
"os"
"path/filepath"
"strings"

"github.com/Microsoft/go-winio"
"github.com/docker/docker/pkg/idtools"
"github.com/docker/docker/pkg/reexec"
"github.com/docker/docker/pkg/system"
"github.com/pkg/errors"
"golang.org/x/sys/windows"
)

var pathBlacklist = map[string]bool{
"c:\\": true,
"c:\\windows": true,
}

func fixPermissions(source, destination string, rootIDs idtools.IDPair, overrideSkip bool) error {
// chown is not supported on Windows
return nil
func init() {
reexec.Register("windows-fix-permissions", fixPermissionsReexec)
}

func fixPermissions(source, destination string, identity idtools.Identity, _ bool) error {
if identity.SID == "" {
return nil
}

cmd := reexec.Command("windows-fix-permissions", source, destination, identity.SID)
output, err := cmd.CombinedOutput()

return errors.Wrapf(err, "failed to exec windows-fix-permissions: %s", output)
}

func fixPermissionsReexec() {
err := fixPermissionsWindows(os.Args[1], os.Args[2], os.Args[3])

This comment has been minimized.

Copy link
@thaJeztah

thaJeztah Aug 17, 2018

Member

nit: a more idiomatic way to write these is to scope the err to the if;

if err := fixPermissionsWindows(os.Args[1], os.Args[2], os.Args[3]); err != nil {
 ....
}
if err != nil {
fmt.Fprint(os.Stderr, err)
os.Exit(1)
}
}

func fixPermissionsWindows(source, destination, SID string) error {

privileges := []string{winio.SeRestorePrivilege, system.SeTakeOwnershipPrivilege}

err := winio.EnableProcessPrivileges(privileges)
if err != nil {
return err
}

defer winio.DisableProcessPrivileges(privileges)

sid, err := windows.StringToSid(SID)
if err != nil {
return err
}

// Owners on *nix have read/write/delete/read control and write DAC.
// Add an ACE that grants this to the user/group specified with the
// chown option. Currently Windows is not honoring the owner change,
// however, they are aware of this and it should be fixed at some
// point.

sddlString := system.SddlAdministratorsLocalSystem
sddlString += "(A;OICI;GRGWGXRCWDSD;;;" + SID + ")"

securityDescriptor, err := winio.SddlToSecurityDescriptor(sddlString)
if err != nil {
return err
}

var daclPresent uint32
var daclDefaulted uint32
var dacl *byte

This comment has been minimized.

Copy link
@thaJeztah

thaJeztah Aug 17, 2018

Member

style nit; no need to change; generally we group these, i.e.;

var (
    daclPresent uint32
    daclDefaulted uint32
    dacl *byte
)

err = system.GetSecurityDescriptorDacl(&securityDescriptor[0], &daclPresent, &dacl, &daclDefaulted)

This comment has been minimized.

Copy link
@thaJeztah

thaJeztah Aug 17, 2018

Member

same here (not a blocker)

if err := system.GetSecurityDescriptorDacl(&securityDescriptor[0], &daclPresent, &dacl, &daclDefaulted); err != nil {
    return err
}
if err != nil {
return err
}

return system.SetNamedSecurityInfo(windows.StringToUTF16Ptr(destination), system.SE_FILE_OBJECT, system.OWNER_SECURITY_INFORMATION|system.DACL_SECURITY_INFORMATION, sid, nil, dacl, nil)
}

func validateCopySourcePath(imageSource *imageMount, origPath, platform string) error {
@@ -37,7 +37,7 @@ type Archiver interface {
UntarPath(src, dst string) error
CopyWithTar(src, dst string) error
CopyFileWithTar(src, dst string) error
IDMappings() *idtools.IDMappings
IdentityMapping() *idtools.IdentityMapping
}

// The builder will use the following interfaces if the container fs implements
@@ -68,11 +68,11 @@ func tarFunc(i interface{}) containerfs.TarFunc {
func (b *Builder) getArchiver(src, dst containerfs.Driver) Archiver {
t, u := tarFunc(src), untarFunc(dst)
return &containerfs.Archiver{
SrcDriver: src,
DstDriver: dst,
Tar: t,
Untar: u,
IDMappingsVar: b.idMappings,
SrcDriver: src,
DstDriver: dst,
Tar: t,
Untar: u,
IDMapping: b.idMapping,
}
}

@@ -185,22 +185,26 @@ func (b *Builder) performCopy(req dispatchRequest, inst copyInstruction) error {
return err
}

chownPair := b.idMappings.RootPair()
identity := b.idMapping.RootPair()
// if a chown was requested, perform the steps to get the uid, gid
// translated (if necessary because of user namespaces), and replace
// the root pair with the chown pair for copy operations
if inst.chownStr != "" {
chownPair, err = parseChownFlag(inst.chownStr, destInfo.root.Path(), b.idMappings)
identity, err = parseChownFlag(b, state, inst.chownStr, destInfo.root.Path(), b.idMapping)
if err != nil {
return errors.Wrapf(err, "unable to convert uid/gid chown string to host mapping")
if b.options.Platform != "windows" {
return errors.Wrapf(err, "unable to convert uid/gid chown string to host mapping")
}

return errors.Wrapf(err, "unable to map container user account name to SID")
}
}

for _, info := range inst.infos {
opts := copyFileOptions{
decompress: inst.allowLocalDecompression,
archiver: b.getArchiver(info.root, destInfo.root),
chownPair: chownPair,
identity: identity,
}
if err := performCopyForInfo(destInfo, info, opts); err != nil {
return errors.Wrapf(err, "failed to copy files")
@@ -11,11 +11,11 @@ import (
"github.com/pkg/errors"
)

func parseChownFlag(chown, ctrRootPath string, idMappings *idtools.IDMappings) (idtools.IDPair, error) {
func parseChownFlag(builder *Builder, state *dispatchState, chown, ctrRootPath string, identityMapping *idtools.IdentityMapping) (idtools.Identity, error) {
var userStr, grpStr string
parts := strings.Split(chown, ":")
if len(parts) > 2 {
return idtools.IDPair{}, errors.New("invalid chown string format: " + chown)
return idtools.Identity{}, errors.New("invalid chown string format: " + chown)
}
if len(parts) == 1 {
// if no group specified, use the user spec as group as well
@@ -26,25 +26,25 @@ func parseChownFlag(chown, ctrRootPath string, idMappings *idtools.IDMappings) (

passwdPath, err := symlink.FollowSymlinkInScope(filepath.Join(ctrRootPath, "etc", "passwd"), ctrRootPath)
if err != nil {
return idtools.IDPair{}, errors.Wrapf(err, "can't resolve /etc/passwd path in container rootfs")
return idtools.Identity{}, errors.Wrapf(err, "can't resolve /etc/passwd path in container rootfs")
}
groupPath, err := symlink.FollowSymlinkInScope(filepath.Join(ctrRootPath, "etc", "group"), ctrRootPath)
if err != nil {
return idtools.IDPair{}, errors.Wrapf(err, "can't resolve /etc/group path in container rootfs")
return idtools.Identity{}, errors.Wrapf(err, "can't resolve /etc/group path in container rootfs")
}
uid, err := lookupUser(userStr, passwdPath)
if err != nil {
return idtools.IDPair{}, errors.Wrapf(err, "can't find uid for user "+userStr)
return idtools.Identity{}, errors.Wrapf(err, "can't find uid for user "+userStr)
}
gid, err := lookupGroup(grpStr, groupPath)
if err != nil {
return idtools.IDPair{}, errors.Wrapf(err, "can't find gid for group "+grpStr)
return idtools.Identity{}, errors.Wrapf(err, "can't find gid for group "+grpStr)
}

// convert as necessary because of user namespaces
chownPair, err := idMappings.ToHost(idtools.IDPair{UID: uid, GID: gid})
chownPair, err := identityMapping.ToHost(idtools.Identity{UID: uid, GID: gid})
if err != nil {
return idtools.IDPair{}, errors.Wrapf(err, "unable to convert uid/gid to host mapping")
return idtools.Identity{}, errors.Wrapf(err, "unable to convert uid/gid to host mapping")
}
return chownPair, nil
}
Oops, something went wrong.
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.