Skip to content

Commit

Permalink
Merge pull request openshift#219 from giuseppe/drop-ensure-container-…
Browse files Browse the repository at this point in the history 
…path

builder: create WORKDIR with USER ownership
  • Loading branch information
@rhatdan
rhatdan authored Mar 29, 2022
2 parents 80cd28a + af3197b commit 9afddc0
Show file tree
Hide file tree
Showing 3 changed files with 135 additions and 93 deletions.
16 changes: 15 additions & 1 deletion builder.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ type Run struct {
type Executor interface {
Preserve(path string) error
EnsureContainerPath(path string) error
EnsureContainerPathAs(path, user string, mode *os.FileMode) error
Copy(excludes []string, copies ...Copy) error
Run(run Run, config docker.Config) error
UnrecognizedInstruction(step *Step) error
Expand All @@ -61,6 +62,15 @@ func (logExecutor) EnsureContainerPath(path string) error {
return nil
}

func (logExecutor) EnsureContainerPathAs(path, user string, mode *os.FileMode) error {
if mode != nil {
log.Printf("ENSURE %s AS %q with MODE=%q", path, user, *mode)
} else {
log.Printf("ENSURE %s AS %q", path, user)
}
return nil
}

func (logExecutor) Copy(excludes []string, copies ...Copy) error {
for _, c := range copies {
log.Printf("COPY %v -> %s (from:%s download:%t), chown: %s, chmod %s", c.Src, c.Dest, c.From, c.Download, c.Chown, c.Chmod)
Expand Down Expand Up @@ -88,6 +98,10 @@ func (noopExecutor) EnsureContainerPath(path string) error {
return nil
}

func (noopExecutor) EnsureContainerPathAs(path, user string, mode *os.FileMode) error {
return nil
}

func (noopExecutor) Copy(excludes []string, copies ...Copy) error {
return nil
}
Expand Down Expand Up @@ -378,7 +392,7 @@ func (b *Builder) Run(step *Step, exec Executor, noRunsRemaining bool) error {
}

if len(b.RunConfig.WorkingDir) > 0 {
if err := exec.EnsureContainerPath(b.RunConfig.WorkingDir); err != nil {
if err := exec.EnsureContainerPathAs(b.RunConfig.WorkingDir, b.RunConfig.User, nil); err != nil {
return err
}
}
Expand Down
4 changes: 4 additions & 0 deletions builder_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -479,6 +479,10 @@ func (e *testExecutor) EnsureContainerPath(path string) error {
return e.Err
}

func (e *testExecutor) EnsureContainerPathAs(path, user string, mode *os.FileMode) error {
return e.Err
}

func (e *testExecutor) Copy(excludes []string, copies ...Copy) error {
e.Copies = append(e.Copies, copies...)
return e.Err
Expand Down
208 changes: 116 additions & 92 deletions dockerclient/client.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -682,10 +682,26 @@ func (e *ClientExecutor) Preserve(path string) error {
}

func (e *ClientExecutor) EnsureContainerPath(path string) error {
return e.createOrReplaceContainerPathWithOwner(path, 0, 0)
return e.createOrReplaceContainerPathWithOwner(path, 0, 0, nil)
}

func (e *ClientExecutor) createOrReplaceContainerPathWithOwner(path string, uid, gid int) error {
func (e *ClientExecutor) EnsureContainerPathAs(path, user string, mode *os.FileMode) error {
uid, gid := 0, 0

u, g, err := e.getUser(user)
if err == nil {
uid = u
gid = g
}

return e.createOrReplaceContainerPathWithOwner(path, uid, gid, mode)
}

func (e *ClientExecutor) createOrReplaceContainerPathWithOwner(path string, uid, gid int, mode *os.FileMode) error {
if mode == nil {
m := os.FileMode(0755)
mode = &m
}
createPath := func(dest string) error {
var writerErr error
if !strings.HasSuffix(dest, "/") {
Expand All @@ -704,7 +720,7 @@ func (e *ClientExecutor) createOrReplaceContainerPathWithOwner(path string, uid,
writerErr = tarball.WriteHeader(&tar.Header{
Name: dest,
Typeflag: tar.TypeDir,
Mode: 0755,
Mode: int64(*mode),
Uid: uid,
Gid: gid,
})
Expand Down Expand Up @@ -848,21 +864,22 @@ func (e *ClientExecutor) Copy(excludes []string, copies ...imagebuilder.Copy) er
return e.CopyContainer(e.Container, excludes, copies...)
}

// CopyContainer copies the provided content into a destination container.
func (e *ClientExecutor) CopyContainer(container *docker.Container, excludes []string, copies ...imagebuilder.Copy) error {
chownUid, chownGid := -1, -1
chown := func(h *tar.Header, r io.Reader) (data []byte, update bool, skip bool, err error) {
if chownUid != -1 {
h.Uid = chownUid
}
if chownGid != -1 {
h.Gid = chownGid
func (e *ClientExecutor) findMissingParents(container *docker.Container, dest string) (parents []string, err error) {
destParent := filepath.Clean(dest)
for filepath.Dir(destParent) != destParent {
exists, err := isContainerPathDirectory(e.Client, container.ID, destParent)
if err != nil {
return nil, err
}
if (h.Uid > 0x1fffff || h.Gid > 0x1fffff) && h.Format == tar.FormatUSTAR {
h.Format = tar.FormatPAX
if !exists {
parents = append(parents, destParent)
}
return nil, false, false, nil
destParent = filepath.Dir(destParent)
}
return parents, nil
}

func (e *ClientExecutor) getUser(userspec string) (int, int, error) {
readFile := func(path string) ([]byte, error) {
var buffer, contents bytes.Buffer
if err := e.Client.DownloadFromContainer(e.Container.ID, docker.DownloadFromContainerOptions{
Expand Down Expand Up @@ -922,89 +939,96 @@ func (e *ClientExecutor) CopyContainer(container *docker.Container, excludes []s
}
return *value, nil
}
findMissingParents := func(dest string) (parents []string, err error) {
destParent := filepath.Clean(dest)
for filepath.Dir(destParent) != destParent {
exists, err := isContainerPathDirectory(e.Client, container.ID, destParent)

spec := strings.SplitN(userspec, ":", 2)
if len(spec) == 2 {
parsedUid, err := strconv.ParseUint(spec[0], 10, 32)
if err != nil {
// maybe it's a user name? look up the UID
passwdFile, err := readFile("/etc/passwd")
if err != nil {
return nil, err
return -1, -1, err
}
uid, err := parse(passwdFile, 0, spec[0], 7, 2)
if err != nil {
return -1, -1, fmt.Errorf("error reading UID value from passwd file for --chown=%s: %v", spec[0], err)
}
parsedUid, err = strconv.ParseUint(uid, 10, 32)
if err != nil {
return -1, -1, fmt.Errorf("error parsing UID value %q from passwd file for --chown=%s", uid, userspec)
}
}
parsedGid, err := strconv.ParseUint(spec[1], 10, 32)
if err != nil {
// maybe it's a group name? look up the GID
groupFile, err := readFile("/etc/group")
if err != nil {
return -1, -1, err
}
if !exists {
parents = append(parents, destParent)
gid, err := parse(groupFile, 0, spec[1], 4, 2)
if err != nil {
return -1, -1, err
}
parsedGid, err = strconv.ParseUint(gid, 10, 32)
if err != nil {
return -1, -1, fmt.Errorf("error parsing GID value %q from group file for --chown=%s", gid, userspec)
}
destParent = filepath.Dir(destParent)
}
return parents, nil
return int(parsedUid), int(parsedGid), nil
}

var parsedUid, parsedGid uint64
if id, err := strconv.ParseUint(spec[0], 10, 32); err == nil {
// it's an ID. use it as both the UID and the GID
parsedUid = id
parsedGid = id
} else {
// it's a user name, we'll need to look up their UID and primary GID
passwdFile, err := readFile("/etc/passwd")
if err != nil {
return -1, -1, err
}
// read the UID and primary GID
uid, err := parse(passwdFile, 0, spec[0], 7, 2)
if err != nil {
return -1, -1, fmt.Errorf("error reading UID value from /etc/passwd for --chown=%s", userspec)
}
gid, err := parse(passwdFile, 0, spec[0], 7, 3)
if err != nil {
return -1, -1, fmt.Errorf("error reading GID value from /etc/passwd for --chown=%s", userspec)
}
if parsedUid, err = strconv.ParseUint(uid, 10, 32); err != nil {
return -1, -1, fmt.Errorf("error parsing UID value %q from /etc/passwd for --chown=%s", uid, userspec)
}
if parsedGid, err = strconv.ParseUint(gid, 10, 32); err != nil {
return -1, -1, fmt.Errorf("error parsing GID value %q from /etc/passwd for --chown=%s", gid, userspec)
}
}
return int(parsedUid), int(parsedGid), nil
}

// CopyContainer copies the provided content into a destination container.
func (e *ClientExecutor) CopyContainer(container *docker.Container, excludes []string, copies ...imagebuilder.Copy) error {
chownUid, chownGid := -1, -1
chown := func(h *tar.Header, r io.Reader) (data []byte, update bool, skip bool, err error) {
if chownUid != -1 {
h.Uid = chownUid
}
if chownGid != -1 {
h.Gid = chownGid
}
if (h.Uid > 0x1fffff || h.Gid > 0x1fffff) && h.Format == tar.FormatUSTAR {
h.Format = tar.FormatPAX
}
return nil, false, false, nil
}
for _, c := range copies {
chownUid, chownGid = -1, -1
if c.Chown != "" {
spec := strings.SplitN(c.Chown, ":", 2)
if len(spec) == 2 {
parsedUid, err := strconv.ParseUint(spec[0], 10, 32)
if err != nil {
// maybe it's a user name? look up the UID
passwdFile, err := readFile("/etc/passwd")
if err != nil {
return err
}
uid, err := parse(passwdFile, 0, spec[0], 7, 2)
if err != nil {
return fmt.Errorf("error reading UID value from passwd file for --chown=%s: %v", spec[0], err)
}
parsedUid, err = strconv.ParseUint(uid, 10, 32)
if err != nil {
return fmt.Errorf("error parsing UID value %q from passwd file for --chown=%s", uid, c.Chown)
}
}
parsedGid, err := strconv.ParseUint(spec[1], 10, 32)
if err != nil {
// maybe it's a group name? look up the GID
groupFile, err := readFile("/etc/group")
if err != nil {
return err
}
gid, err := parse(groupFile, 0, spec[1], 4, 2)
if err != nil {
return err
}
parsedGid, err = strconv.ParseUint(gid, 10, 32)
if err != nil {
return fmt.Errorf("error parsing GID value %q from group file for --chown=%s", gid, c.Chown)
}
}
chownUid = int(parsedUid)
chownGid = int(parsedGid)
} else {
var parsedUid, parsedGid uint64
if id, err := strconv.ParseUint(spec[0], 10, 32); err == nil {
// it's an ID. use it as both the UID and the GID
parsedUid = id
parsedGid = id
} else {
// it's a user name, we'll need to look up their UID and primary GID
passwdFile, err := readFile("/etc/passwd")
if err != nil {
return err
}
// read the UID and primary GID
uid, err := parse(passwdFile, 0, spec[0], 7, 2)
if err != nil {
return fmt.Errorf("error reading UID value from /etc/passwd for --chown=%s", c.Chown)
}
gid, err := parse(passwdFile, 0, spec[0], 7, 3)
if err != nil {
return fmt.Errorf("error reading GID value from /etc/passwd for --chown=%s", c.Chown)
}
if parsedUid, err = strconv.ParseUint(uid, 10, 32); err != nil {
return fmt.Errorf("error parsing UID value %q from /etc/passwd for --chown=%s", uid, c.Chown)
}
if parsedGid, err = strconv.ParseUint(gid, 10, 32); err != nil {
return fmt.Errorf("error parsing GID value %q from /etc/passwd for --chown=%s", gid, c.Chown)
}
}
chownUid = int(parsedUid)
chownGid = int(parsedGid)
var err error
chownUid, chownGid, err = e.getUser(c.Chown)
if err != nil {
return err
}
}
// TODO: reuse source
Expand Down Expand Up @@ -1039,15 +1063,15 @@ func (e *ClientExecutor) CopyContainer(container *docker.Container, excludes []s
// directories with the wrong ownership, so
// check for any that don't exist and create
// them ourselves
missingParents, err := findMissingParents(c.Dest)
missingParents, err := e.findMissingParents(container, c.Dest)
if err != nil {
return err
}
if len(missingParents) > 0 {
sort.Strings(missingParents)
klog.V(5).Infof("Uploading directories %v to %s%s", missingParents, container.ID, asOwner)
for _, missingParent := range missingParents {
if err := e.createOrReplaceContainerPathWithOwner(missingParent, chownUid, chownGid); err != nil {
if err := e.createOrReplaceContainerPathWithOwner(missingParent, chownUid, chownGid, nil); err != nil {
return err
}
}
Expand Down

0 comments on commit 9afddc0

Please sign in to comment.