Containers in a pod can only join namespaces in that pod

This solves some dependency problems in the state, and makes
sense from a design standpoint.

Containers not in a pod can still depend on the namespaces of
containers joined to a pod, which we might also want to change in
the future.

Signed-off-by: Matthew Heon <matthew.heon@gmail.com>

Closes: #184
Approved by: baude

6 files changed, 125 insertions, 6 deletions

diff --git a/libpod/boltdb_state_internal.go b/libpod/boltdb_state_internal.go
index ac7a9e166..f26a60905 100644
--- a/libpod/boltdb_state_internal.go
+++ b/libpod/boltdb_state_internal.go
@@ -23,6 +23,7 @@ const (
 	dependenciesName = "dependencies"
 	netNSName        = "netns"
 	containersName   = "containers"
+	podIDName        = "pod-id"
 )
 
 var (
@@ -37,6 +38,7 @@ var (
 	dependenciesBkt = []byte(dependenciesName)
 	netNSKey        = []byte(netNSName)
 	containersBkt   = []byte(containersName)
+	podIDKey        = []byte(podIDName)
 )
 
 // Check if the configuration of the database is compatible with the
@@ -329,6 +331,11 @@ func (s *BoltState) addContainer(ctr *Container, pod *Pod) error {
 		if err := newCtrBkt.Put(stateKey, stateJSON); err != nil {
 			return errors.Wrapf(err, "error adding container %s state to DB", ctr.ID())
 		}
+		if pod != nil {
+			if err := newCtrBkt.Put(podIDKey, []byte(pod.ID())); err != nil {
+				return errors.Wrapf(err, "error adding container %s pod to DB", ctr.ID())
+			}
+		}
 		if netNSPath != "" {
 			if err := newCtrBkt.Put(netNSKey, []byte(netNSPath)); err != nil {
 				return errors.Wrapf(err, "error adding container %s netns path to DB", ctr.ID())
@@ -346,6 +353,15 @@ func (s *BoltState) addContainer(ctr *Container, pod *Pod) error {
 			if depCtrBkt == nil {
 				return errors.Wrapf(ErrNoSuchCtr, "container %s depends on container %s, but it does not exist in the DB", ctr.ID(), dependsCtr)
 			}
+
+			// If we're part of a pod, make sure the dependency is part of the same pod
+			if pod != nil {
+				depCtrPod := depCtrBkt.Get(podIDKey)
+				if depCtrPod == nil {
+					return errors.Wrapf(ErrInvalidArg, "container %s depends on container%s which is not in pod %s", ctr.ID(), dependsCtr, pod.ID())
+				}
+			}
+
 			depCtrDependsBkt := depCtrBkt.Bucket(dependenciesBkt)
 			if depCtrDependsBkt == nil {
 				return errors.Wrapf(ErrInternal, "container %s does not have a dependencies bucket", dependsCtr)
diff --git a/libpod/in_memory_state.go b/libpod/in_memory_state.go
index c99a34e7a..a38e080b7 100644
--- a/libpod/in_memory_state.go
+++ b/libpod/in_memory_state.go
@@ -511,6 +511,9 @@ func (s *InMemoryState) AddContainerToPod(pod *Pod, ctr *Container) error {
 		if _, ok = s.containers[depCtr]; !ok {
 			return errors.Wrapf(ErrNoSuchCtr, "cannot depend on nonexistent container %s", depCtr)
 		}
+		if _, ok = podCtrs[depCtr]; !ok {
+			return errors.Wrapf(ErrInvalidArg, "cannot depend on container %s as it is not in pod %s", depCtr, pod.ID())
+		}
 	}
 
 	// Add container to state
diff --git a/libpod/options.go b/libpod/options.go
index f5df3349f..9d0d63777 100644
--- a/libpod/options.go
+++ b/libpod/options.go
@@ -342,6 +342,8 @@ func WithStdin() CtrCreateOption {
 }
 
 // WithPod adds the container to a pod
+// Containers which join a pod can only join the namespaces of other containers
+// in the same pod
 func (r *Runtime) WithPod(pod *Pod) CtrCreateOption {
 	return func(ctr *Container) error {
 		if ctr.valid {
@@ -426,6 +428,8 @@ func WithStopTimeout(timeout uint) CtrCreateOption {
 
 // WithIPCNSFrom indicates the the container should join the IPC namespace of
 // the given container
+// If the container has joined a pod, it can only join the namespaces of
+// containers in the same pod
 func WithIPCNSFrom(nsCtr *Container) CtrCreateOption {
 	return func(ctr *Container) error {
 		if ctr.valid {
@@ -440,6 +444,10 @@ func WithIPCNSFrom(nsCtr *Container) CtrCreateOption {
 			return errors.Wrapf(ErrInvalidArg, "must specify another container")
 		}
 
+		if ctr.config.Pod != "" && nsCtr.config.Pod != ctr.config.Pod {
+			return errors.Wrapf(ErrInvalidArg, "container has joined pod %s and dependency container %s is not a member of the pod", ctr.config.Pod, nsCtr.ID())
+		}
+
 		ctr.config.IPCNsCtr = nsCtr.ID()
 
 		return nil
@@ -448,6 +456,8 @@ func WithIPCNSFrom(nsCtr *Container) CtrCreateOption {
 
 // WithMountNSFrom indicates the the container should join the mount namespace
 // of the given container
+// If the container has joined a pod, it can only join the namespaces of
+// containers in the same pod
 func WithMountNSFrom(nsCtr *Container) CtrCreateOption {
 	return func(ctr *Container) error {
 		if ctr.valid {
@@ -462,6 +472,10 @@ func WithMountNSFrom(nsCtr *Container) CtrCreateOption {
 			return errors.Wrapf(ErrInvalidArg, "must specify another container")
 		}
 
+		if ctr.config.Pod != "" && nsCtr.config.Pod != ctr.config.Pod {
+			return errors.Wrapf(ErrInvalidArg, "container has joined pod %s and dependency container %s is not a member of the pod", ctr.config.Pod, nsCtr.ID())
+		}
+
 		ctr.config.MountNsCtr = nsCtr.ID()
 
 		return nil
@@ -470,6 +484,8 @@ func WithMountNSFrom(nsCtr *Container) CtrCreateOption {
 
 // WithNetNSFrom indicates the the container should join the network namespace
 // of the given container
+// If the container has joined a pod, it can only join the namespaces of
+// containers in the same pod
 func WithNetNSFrom(nsCtr *Container) CtrCreateOption {
 	return func(ctr *Container) error {
 		if ctr.valid {
@@ -488,6 +504,10 @@ func WithNetNSFrom(nsCtr *Container) CtrCreateOption {
 			return errors.Wrapf(ErrInvalidArg, "cannot join another container's net ns as we are making a new net ns")
 		}
 
+		if ctr.config.Pod != "" && nsCtr.config.Pod != ctr.config.Pod {
+			return errors.Wrapf(ErrInvalidArg, "container has joined pod %s and dependency container %s is not a member of the pod", ctr.config.Pod, nsCtr.ID())
+		}
+
 		ctr.config.NetNsCtr = nsCtr.ID()
 
 		return nil
@@ -496,6 +516,8 @@ func WithNetNSFrom(nsCtr *Container) CtrCreateOption {
 
 // WithPIDNSFrom indicates the the container should join the PID namespace of
 // the given container
+// If the container has joined a pod, it can only join the namespaces of
+// containers in the same pod
 func WithPIDNSFrom(nsCtr *Container) CtrCreateOption {
 	return func(ctr *Container) error {
 		if ctr.valid {
@@ -510,6 +532,10 @@ func WithPIDNSFrom(nsCtr *Container) CtrCreateOption {
 			return errors.Wrapf(ErrInvalidArg, "must specify another container")
 		}
 
+		if ctr.config.Pod != "" && nsCtr.config.Pod != ctr.config.Pod {
+			return errors.Wrapf(ErrInvalidArg, "container has joined pod %s and dependency container %s is not a member of the pod", ctr.config.Pod, nsCtr.ID())
+		}
+
 		ctr.config.PIDNsCtr = nsCtr.ID()
 
 		return nil
@@ -518,6 +544,8 @@ func WithPIDNSFrom(nsCtr *Container) CtrCreateOption {
 
 // WithUserNSFrom indicates the the container should join the user namespace of
 // the given container
+// If the container has joined a pod, it can only join the namespaces of
+// containers in the same pod
 func WithUserNSFrom(nsCtr *Container) CtrCreateOption {
 	return func(ctr *Container) error {
 		if ctr.valid {
@@ -532,6 +560,10 @@ func WithUserNSFrom(nsCtr *Container) CtrCreateOption {
 			return errors.Wrapf(ErrInvalidArg, "must specify another container")
 		}
 
+		if ctr.config.Pod != "" && nsCtr.config.Pod != ctr.config.Pod {
+			return errors.Wrapf(ErrInvalidArg, "container has joined pod %s and dependency container %s is not a member of the pod", ctr.config.Pod, nsCtr.ID())
+		}
+
 		ctr.config.UserNsCtr = nsCtr.ID()
 
 		return nil
@@ -540,6 +572,8 @@ func WithUserNSFrom(nsCtr *Container) CtrCreateOption {
 
 // WithUTSNSFrom indicates the the container should join the UTS namespace of
 // the given container
+// If the container has joined a pod, it can only join the namespaces of
+// containers in the same pod
 func WithUTSNSFrom(nsCtr *Container) CtrCreateOption {
 	return func(ctr *Container) error {
 		if ctr.valid {
@@ -554,6 +588,10 @@ func WithUTSNSFrom(nsCtr *Container) CtrCreateOption {
 			return errors.Wrapf(ErrInvalidArg, "must specify another container")
 		}
 
+		if ctr.config.Pod != "" && nsCtr.config.Pod != ctr.config.Pod {
+			return errors.Wrapf(ErrInvalidArg, "container has joined pod %s and dependency container %s is not a member of the pod", ctr.config.Pod, nsCtr.ID())
+		}
+
 		ctr.config.UTSNsCtr = nsCtr.ID()
 
 		return nil
@@ -562,6 +600,8 @@ func WithUTSNSFrom(nsCtr *Container) CtrCreateOption {
 
 // WithCgroupNSFrom indicates the the container should join the CGroup namespace
 // of the given container
+// If the container has joined a pod, it can only join the namespaces of
+// containers in the same pod
 func WithCgroupNSFrom(nsCtr *Container) CtrCreateOption {
 	return func(ctr *Container) error {
 		if ctr.valid {
@@ -576,6 +616,10 @@ func WithCgroupNSFrom(nsCtr *Container) CtrCreateOption {
 			return errors.Wrapf(ErrInvalidArg, "must specify another container")
 		}
 
+		if ctr.config.Pod != "" && nsCtr.config.Pod != ctr.config.Pod {
+			return errors.Wrapf(ErrInvalidArg, "container has joined pod %s and dependency container %s is not a member of the pod", ctr.config.Pod, nsCtr.ID())
+		}
+
 		ctr.config.CgroupNsCtr = nsCtr.ID()
 
 		return nil
@@ -585,6 +629,7 @@ func WithCgroupNSFrom(nsCtr *Container) CtrCreateOption {
 // WithNetNS indicates that the container should be given a new network
 // namespace with a minimal configuration
 // An optional array of port mappings can be provided
+// Conflicts with WithNetNSFrom()
 func WithNetNS(portMappings []ocicni.PortMapping) CtrCreateOption {
 	return func(ctr *Container) error {
 		if ctr.valid {
diff --git a/libpod/sql_state_internal.go b/libpod/sql_state_internal.go
index 3fb1ac64f..0bbdccc9f 100644
--- a/libpod/sql_state_internal.go
+++ b/libpod/sql_state_internal.go
@@ -752,13 +752,16 @@ func (s *SQLState) addContainer(ctr *Container, pod *Pod) (err error) {
                     ?, ?, ?, ?, ?,
                     ?, ?, ?
                 );`
-		addRegistry = "INSERT INTO registry VALUES (?, ?);"
+		addRegistry   = "INSERT INTO registry VALUES (?, ?);"
+		checkCtrInPod = "SELECT 1 FROM containers WHERE Id=? AND Pod=?;"
 	)
 
 	if !s.valid {
 		return ErrDBClosed
 	}
 
+	depCtrs := ctr.Dependencies()
+
 	mounts, err := json.Marshal(ctr.config.Mounts)
 	if err != nil {
 		return errors.Wrapf(err, "error marshaling container %s mounts to JSON", ctr.ID())
@@ -830,6 +833,20 @@ func (s *SQLState) addContainer(ctr *Container, pod *Pod) (err error) {
 			pod.valid = false
 			return errors.Wrapf(ErrNoSuchPod, "pod %s does not exist in state, cannot add container to it", pod.ID())
 		}
+
+		// We also need to check if our dependencies are in the pod
+		for _, depID := range depCtrs {
+			row := tx.QueryRow(checkCtrInPod, depID, pod.ID())
+			var check int
+			err := row.Scan(&check)
+			if err != nil {
+				if err == sql.ErrNoRows {
+					return errors.Wrapf(ErrInvalidArg, "container %s depends on container %s but it is not in pod %s", ctr.ID(), depID, pod.ID())
+				}
+			} else if check != 1 {
+				return errors.Wrapf(ErrInternal, "check digit for checkCtrInPod query incorrect")
+			}
+		}
 	}
 
 	// Add container to registry
diff --git a/libpod/state.go b/libpod/state.go
index 36b5a757b..ed52e3d46 100644
--- a/libpod/state.go
+++ b/libpod/state.go
@@ -9,18 +9,20 @@ type State interface {
 	// Refresh clears container and pod states after a reboot
 	Refresh() error
 
-	// Accepts full ID of container
+	// Return a container from the database from its full ID
 	Container(id string) (*Container, error)
-	// Accepts full or partial IDs (as long as they are unique) and names
+	// Return a container from the database by full or partial ID or full
+	// name
 	LookupContainer(idOrName string) (*Container, error)
-	// Checks if a container with the given ID is present in the state
+	// Check if a container with the given full ID exists in the database
 	HasContainer(id string) (bool, error)
 	// Adds container to state
 	// The container cannot be part of a pod
+	// The container must have globally unique name and ID - pod names and
+	// IDs also conflict with container names and IDs
 	AddContainer(ctr *Container) error
 	// Removes container from state
-	// The container will only be removed from the state, not from the pod
-	// which the container belongs to
+	// Containers that are part of pods must use RemoveContainerFromPod
 	RemoveContainer(ctr *Container) error
 	// UpdateContainer updates a container's state from the backing store
 	UpdateContainer(ctr *Container) error
diff --git a/libpod/state_test.go b/libpod/state_test.go
index 718da8392..53930f2d0 100644
--- a/libpod/state_test.go
+++ b/libpod/state_test.go
@@ -2174,6 +2174,42 @@ func TestAddContainerToPodBadDependencyFails(t *testing.T) {
 	})
 }
 
+func TestAddContainerToPodDependencyOutsidePodFails(t *testing.T) {
+	runForAllStates(t, func(t *testing.T, state State, lockPath string) {
+		testPod, err := getTestPod1(lockPath)
+		assert.NoError(t, err)
+
+		testCtr1, err := getTestCtr2(lockPath)
+		assert.NoError(t, err)
+
+		testCtr2, err := getTestCtrN("3", lockPath)
+		assert.NoError(t, err)
+		testCtr2.config.Pod = testPod.ID()
+		testCtr2.config.IPCNsCtr = testCtr1.ID()
+
+		err = state.AddPod(testPod)
+		assert.NoError(t, err)
+
+		err = state.AddContainer(testCtr1)
+		assert.NoError(t, err)
+
+		err = state.AddContainerToPod(testPod, testCtr2)
+		assert.Error(t, err)
+
+		ctrs, err := state.PodContainers(testPod)
+		assert.NoError(t, err)
+		assert.Equal(t, 0, len(ctrs))
+
+		allCtrs, err := state.AllContainers()
+		assert.NoError(t, err)
+		assert.Equal(t, 1, len(allCtrs))
+
+		deps, err := state.ContainerInUse(testCtr1)
+		assert.NoError(t, err)
+		assert.Equal(t, 0, len(deps))
+	})
+}
+
 func TestRemoveContainerFromPodBadPodFails(t *testing.T) {
 	runForAllStates(t, func(t *testing.T, state State, lockPath string) {
 		testCtr, err := getTestCtr1(lockPath)