From dc6a99df4c5ea7facaca20129b2b6c5b53ddb3c1 Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Sat, 10 Feb 2018 15:11:32 -0500 Subject: 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 Closes: #184 Approved by: baude --- libpod/boltdb_state_internal.go | 16 +++++++++++++++ libpod/in_memory_state.go | 3 +++ libpod/options.go | 45 +++++++++++++++++++++++++++++++++++++++++ libpod/sql_state_internal.go | 19 ++++++++++++++++- libpod/state.go | 12 ++++++----- libpod/state_test.go | 36 +++++++++++++++++++++++++++++++++ 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) -- cgit v1.2.3-54-g00ecf