We need to sort mounts so that one mount does not over mount another.

Currently we add mounts from images, volumes and internal.
We can accidently over mount an existing mount.  This patch sorts the mounts
to make sure a parent directory is always mounted before its content.

Had to change the default propagation on image volume mounts from shared
to private to stop mount points from leaking out of the container.

Also switched from using some docker/docker/pkg to container/storage/pkg
to remove some dependencies on Docker.

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>

Closes: #1243
Approved by: mheon

10 files changed, 37 insertions, 247 deletions

diff --git a/libpod/container_api.go b/libpod/container_api.go
index 73fd96960..62281218f 100644
--- a/libpod/container_api.go
+++ b/libpod/container_api.go
@@ -8,8 +8,8 @@ import (
 	"strings"
 	"time"
 
+	"github.com/containers/storage/pkg/stringid"
 	"github.com/docker/docker/daemon/caps"
-	"github.com/docker/docker/pkg/stringid"
 	"github.com/pkg/errors"
 	"github.com/projectatomic/libpod/libpod/driver"
 	"github.com/projectatomic/libpod/pkg/inspect"
diff --git a/libpod/container_internal.go b/libpod/container_internal.go
index 7b5932541..535f34200 100644
--- a/libpod/container_internal.go
+++ b/libpod/container_internal.go
@@ -16,8 +16,8 @@ import (
 	"github.com/containers/storage"
 	"github.com/containers/storage/pkg/archive"
 	"github.com/containers/storage/pkg/chrootarchive"
-	"github.com/docker/docker/pkg/mount"
-	"github.com/docker/docker/pkg/stringid"
+	"github.com/containers/storage/pkg/mount"
+	"github.com/containers/storage/pkg/stringid"
 	spec "github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/opencontainers/runtime-tools/generate"
 	"github.com/opencontainers/selinux/go-selinux/label"
@@ -829,7 +829,6 @@ func (c *Container) cleanupStorage() error {
 		logrus.Debugf("Storage is already unmounted, skipping...")
 		return nil
 	}
-
 	for _, mount := range c.config.Mounts {
 		if err := c.unmountSHM(mount); err != nil {
 			return err
@@ -1178,7 +1177,7 @@ func (c *Container) addLocalVolumes(ctx context.Context, g *generate.Generator)
 		mount := spec.Mount{
 			Destination: k,
 			Type:        "bind",
-			Options:     []string{"rbind", "rw"},
+			Options:     []string{"private", "bind", "rw"},
 		}
 		if MountExists(g.Mounts(), k) {
 			continue
diff --git a/libpod/container_internal_linux.go b/libpod/container_internal_linux.go
index e7e3b6ce9..59fb6af87 100644
--- a/libpod/container_internal_linux.go
+++ b/libpod/container_internal_linux.go
@@ -248,6 +248,12 @@ func (c *Container) generateSpec(ctx context.Context) (*spec.Spec, error) {
 		g.SetLinuxCgroupsPath(cgroupPath)
 	}
 
+	// Mounts need to be sorted so paths will not cover other paths
+	mounts := sortMounts(g.Mounts())
+	g.ClearMounts()
+	for _, m := range mounts {
+		g.AddMount(m)
+	}
 	return g.Config, nil
 }
 
diff --git a/libpod/in_memory_state.go b/libpod/in_memory_state.go
index 8bdd0881c..0327b331e 100644
--- a/libpod/in_memory_state.go
+++ b/libpod/in_memory_state.go
@@ -3,7 +3,7 @@ package libpod
 import (
 	"strings"
 
-	"github.com/docker/docker/pkg/truncindex"
+	"github.com/containers/storage/pkg/truncindex"
 	"github.com/pkg/errors"
 	"github.com/projectatomic/libpod/pkg/registrar"
 )
diff --git a/libpod/info.go b/libpod/info.go
index 5bb77f447..1108845ea 100644
--- a/libpod/info.go
+++ b/libpod/info.go
@@ -10,7 +10,7 @@ import (
 	"strings"
 	"time"
 
-	"github.com/docker/docker/pkg/system"
+	"github.com/containers/storage/pkg/system"
 	"github.com/pkg/errors"
 	"github.com/projectatomic/libpod/utils"
 )
diff --git a/libpod/pod_internal.go b/libpod/pod_internal.go
index 9102ae28a..c8d8405bb 100644
--- a/libpod/pod_internal.go
+++ b/libpod/pod_internal.go
@@ -7,7 +7,7 @@ import (
 	"time"
 
 	"github.com/containers/storage"
-	"github.com/docker/docker/pkg/stringid"
+	"github.com/containers/storage/pkg/stringid"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 )
diff --git a/libpod/util.go b/libpod/util.go
index 106dd4666..13235059f 100644
--- a/libpod/util.go
+++ b/libpod/util.go
@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"sort"
 	"strconv"
 	"strings"
 	"time"
@@ -121,3 +122,26 @@ func WaitForFile(path string, timeout time.Duration) error {
 		return errors.Wrapf(ErrInternal, "timed out waiting for file %s", path)
 	}
 }
+
+type byDestination []spec.Mount
+
+func (m byDestination) Len() int {
+	return len(m)
+}
+
+func (m byDestination) Less(i, j int) bool {
+	return m.parts(i) < m.parts(j)
+}
+
+func (m byDestination) Swap(i, j int) {
+	m[i], m[j] = m[j], m[i]
+}
+
+func (m byDestination) parts(i int) int {
+	return strings.Count(filepath.Clean(m[i].Destination), string(os.PathSeparator))
+}
+
+func sortMounts(m []spec.Mount) []spec.Mount {
+	sort.Sort(byDestination(m))
+	return m
+}
diff --git a/vendor/github.com/docker/docker/pkg/stringid/README.md b/vendor/github.com/docker/docker/pkg/stringid/README.md
deleted file mode 100644
index 37a5098fd..000000000
--- a/vendor/github.com/docker/docker/pkg/stringid/README.md
+++ /dev/null
@@ -1 +0,0 @@
-This package provides helper functions for dealing with string identifiers
diff --git a/vendor/github.com/docker/docker/pkg/stringid/stringid.go b/vendor/github.com/docker/docker/pkg/stringid/stringid.go
deleted file mode 100644
index a0c7c42a0..000000000
--- a/vendor/github.com/docker/docker/pkg/stringid/stringid.go
+++ /dev/null
@@ -1,99 +0,0 @@
-// Package stringid provides helper functions for dealing with string identifiers
-package stringid
-
-import (
-	cryptorand "crypto/rand"
-	"encoding/hex"
-	"fmt"
-	"io"
-	"math"
-	"math/big"
-	"math/rand"
-	"regexp"
-	"strconv"
-	"strings"
-	"time"
-)
-
-const shortLen = 12
-
-var (
-	validShortID = regexp.MustCompile("^[a-f0-9]{12}$")
-	validHex     = regexp.MustCompile(`^[a-f0-9]{64}$`)
-)
-
-// IsShortID determines if an arbitrary string *looks like* a short ID.
-func IsShortID(id string) bool {
-	return validShortID.MatchString(id)
-}
-
-// TruncateID returns a shorthand version of a string identifier for convenience.
-// A collision with other shorthands is very unlikely, but possible.
-// In case of a collision a lookup with TruncIndex.Get() will fail, and the caller
-// will need to use a longer prefix, or the full-length Id.
-func TruncateID(id string) string {
-	if i := strings.IndexRune(id, ':'); i >= 0 {
-		id = id[i+1:]
-	}
-	if len(id) > shortLen {
-		id = id[:shortLen]
-	}
-	return id
-}
-
-func generateID(r io.Reader) string {
-	b := make([]byte, 32)
-	for {
-		if _, err := io.ReadFull(r, b); err != nil {
-			panic(err) // This shouldn't happen
-		}
-		id := hex.EncodeToString(b)
-		// if we try to parse the truncated for as an int and we don't have
-		// an error then the value is all numeric and causes issues when
-		// used as a hostname. ref #3869
-		if _, err := strconv.ParseInt(TruncateID(id), 10, 64); err == nil {
-			continue
-		}
-		return id
-	}
-}
-
-// GenerateRandomID returns a unique id.
-func GenerateRandomID() string {
-	return generateID(cryptorand.Reader)
-}
-
-// GenerateNonCryptoID generates unique id without using cryptographically
-// secure sources of random.
-// It helps you to save entropy.
-func GenerateNonCryptoID() string {
-	return generateID(readerFunc(rand.Read))
-}
-
-// ValidateID checks whether an ID string is a valid image ID.
-func ValidateID(id string) error {
-	if ok := validHex.MatchString(id); !ok {
-		return fmt.Errorf("image ID %q is invalid", id)
-	}
-	return nil
-}
-
-func init() {
-	// safely set the seed globally so we generate random ids. Tries to use a
-	// crypto seed before falling back to time.
-	var seed int64
-	if cryptoseed, err := cryptorand.Int(cryptorand.Reader, big.NewInt(math.MaxInt64)); err != nil {
-		// This should not happen, but worst-case fallback to time-based seed.
-		seed = time.Now().UnixNano()
-	} else {
-		seed = cryptoseed.Int64()
-	}
-
-	rand.Seed(seed)
-}
-
-type readerFunc func(p []byte) (int, error)
-
-func (fn readerFunc) Read(p []byte) (int, error) {
-	return fn(p)
-}
diff --git a/vendor/github.com/docker/docker/pkg/truncindex/truncindex.go b/vendor/github.com/docker/docker/pkg/truncindex/truncindex.go
deleted file mode 100644
index 74776e65e..000000000
--- a/vendor/github.com/docker/docker/pkg/truncindex/truncindex.go
+++ /dev/null
@@ -1,139 +0,0 @@
-// Package truncindex provides a general 'index tree', used by Docker
-// in order to be able to reference containers by only a few unambiguous
-// characters of their id.
-package truncindex
-
-import (
-	"errors"
-	"fmt"
-	"strings"
-	"sync"
-
-	"github.com/tchap/go-patricia/patricia"
-)
-
-var (
-	// ErrEmptyPrefix is an error returned if the prefix was empty.
-	ErrEmptyPrefix = errors.New("Prefix can't be empty")
-
-	// ErrIllegalChar is returned when a space is in the ID
-	ErrIllegalChar = errors.New("illegal character: ' '")
-
-	// ErrNotExist is returned when ID or its prefix not found in index.
-	ErrNotExist = errors.New("ID does not exist")
-)
-
-// ErrAmbiguousPrefix is returned if the prefix was ambiguous
-// (multiple ids for the prefix).
-type ErrAmbiguousPrefix struct {
-	prefix string
-}
-
-func (e ErrAmbiguousPrefix) Error() string {
-	return fmt.Sprintf("Multiple IDs found with provided prefix: %s", e.prefix)
-}
-
-// TruncIndex allows the retrieval of string identifiers by any of their unique prefixes.
-// This is used to retrieve image and container IDs by more convenient shorthand prefixes.
-type TruncIndex struct {
-	sync.RWMutex
-	trie *patricia.Trie
-	ids  map[string]struct{}
-}
-
-// NewTruncIndex creates a new TruncIndex and initializes with a list of IDs.
-func NewTruncIndex(ids []string) (idx *TruncIndex) {
-	idx = &TruncIndex{
-		ids: make(map[string]struct{}),
-
-		// Change patricia max prefix per node length,
-		// because our len(ID) always 64
-		trie: patricia.NewTrie(patricia.MaxPrefixPerNode(64)),
-	}
-	for _, id := range ids {
-		idx.addID(id)
-	}
-	return
-}
-
-func (idx *TruncIndex) addID(id string) error {
-	if strings.Contains(id, " ") {
-		return ErrIllegalChar
-	}
-	if id == "" {
-		return ErrEmptyPrefix
-	}
-	if _, exists := idx.ids[id]; exists {
-		return fmt.Errorf("id already exists: '%s'", id)
-	}
-	idx.ids[id] = struct{}{}
-	if inserted := idx.trie.Insert(patricia.Prefix(id), struct{}{}); !inserted {
-		return fmt.Errorf("failed to insert id: %s", id)
-	}
-	return nil
-}
-
-// Add adds a new ID to the TruncIndex.
-func (idx *TruncIndex) Add(id string) error {
-	idx.Lock()
-	defer idx.Unlock()
-	return idx.addID(id)
-}
-
-// Delete removes an ID from the TruncIndex. If there are multiple IDs
-// with the given prefix, an error is thrown.
-func (idx *TruncIndex) Delete(id string) error {
-	idx.Lock()
-	defer idx.Unlock()
-	if _, exists := idx.ids[id]; !exists || id == "" {
-		return fmt.Errorf("no such id: '%s'", id)
-	}
-	delete(idx.ids, id)
-	if deleted := idx.trie.Delete(patricia.Prefix(id)); !deleted {
-		return fmt.Errorf("no such id: '%s'", id)
-	}
-	return nil
-}
-
-// Get retrieves an ID from the TruncIndex. If there are multiple IDs
-// with the given prefix, an error is thrown.
-func (idx *TruncIndex) Get(s string) (string, error) {
-	if s == "" {
-		return "", ErrEmptyPrefix
-	}
-	var (
-		id string
-	)
-	subTreeVisitFunc := func(prefix patricia.Prefix, item patricia.Item) error {
-		if id != "" {
-			// we haven't found the ID if there are two or more IDs
-			id = ""
-			return ErrAmbiguousPrefix{prefix: string(prefix)}
-		}
-		id = string(prefix)
-		return nil
-	}
-
-	idx.RLock()
-	defer idx.RUnlock()
-	if err := idx.trie.VisitSubtree(patricia.Prefix(s), subTreeVisitFunc); err != nil {
-		return "", err
-	}
-	if id != "" {
-		return id, nil
-	}
-	return "", ErrNotExist
-}
-
-// Iterate iterates over all stored IDs and passes each of them to the given
-// handler. Take care that the handler method does not call any public
-// method on truncindex as the internal locking is not reentrant/recursive
-// and will result in deadlock.
-func (idx *TruncIndex) Iterate(handler func(id string)) {
-	idx.Lock()
-	defer idx.Unlock()
-	idx.trie.Visit(func(prefix patricia.Prefix, item patricia.Item) error {
-		handler(string(prefix))
-		return nil
-	})
-}