Merge pull request #497 from mlaventure/cgroups-path

Replace Cgroup Parent and Name fields by CgroupsPath

libcontainer/cgroups/fs/apply_raw.go

@@ -94,11 +94,10 @@ func getCgroupRoot() (string, error) {
 }
 
 type cgroupData struct {
-	root   string
-	parent string
-	name   string
-	config *configs.Cgroup
-	pid    int
+	root      string
+	innerPath string
+	config    *configs.Cgroup
+	pid       int
 }
 
 func (m *Manager) Apply(pid int) (err error) {
@@ -267,45 +266,26 @@ func getCgroupPath(c *configs.Cgroup) (string, error) {
 	return d.path("devices")
 }
 
-// pathClean makes a path safe for use with filepath.Join. This is done by not
-// only cleaning the path, but also (if the path is relative) adding a leading
-// '/' and cleaning it (then removing the leading '/'). This ensures that a
-// path resulting from prepending another path will always resolve to lexically
-// be a subdirectory of the prefixed path. This is all done lexically, so paths
-// that include symlinks won't be safe as a result of using pathClean.
-func pathClean(path string) string {
-	// Ensure that all paths are cleaned (especially problematic ones like
-	// "/../../../../../" which can cause lots of issues).
-	path = filepath.Clean(path)
-
-	// If the path isn't absolute, we need to do more processing to fix paths
-	// such as "../../../../<etc>/some/path". We also shouldn't convert absolute
-	// paths to relative ones.
-	if !filepath.IsAbs(path) {
-		path = filepath.Clean(string(os.PathSeparator) + path)
-		// This can't fail, as (by definition) all paths are relative to root.
-		path, _ = filepath.Rel(string(os.PathSeparator), path)
-	}
-
-	// Clean the path again for good measure.
-	return filepath.Clean(path)
-}
-
 func getCgroupData(c *configs.Cgroup, pid int) (*cgroupData, error) {
 	root, err := getCgroupRoot()
 	if err != nil {
 		return nil, err
 	}
 
-	// Clean the parent slice path.
-	c.Parent = pathClean(c.Parent)
+	if (c.Name != "" || c.Parent != "") && c.Path != "" {
+		return nil, fmt.Errorf("cgroup: either Path or Name and Parent should be used")
+	}
+
+	innerPath := c.Path
+	if innerPath == "" {
+		innerPath = filepath.Join(c.Parent, c.Name)
+	}
 
 	return &cgroupData{
-		root:   root,
-		parent: c.Parent,
-		name:   c.Name,
-		config: c,
-		pid:    pid,
+		root:      root,
+		innerPath: c.Path,
+		config:    c,
+		pid:       pid,
 	}, nil
 }
 
@@ -333,19 +313,18 @@ func (raw *cgroupData) path(subsystem string) (string, error) {
 		return "", err
 	}
 
-	cgPath := filepath.Join(raw.parent, raw.name)
 	// If the cgroup name/path is absolute do not look relative to the cgroup of the init process.
-	if filepath.IsAbs(cgPath) {
+	if filepath.IsAbs(raw.innerPath) {
 		// Sometimes subsystems can be mounted togethger as 'cpu,cpuacct'.
-		return filepath.Join(raw.root, filepath.Base(mnt), cgPath), nil
+		return filepath.Join(raw.root, filepath.Base(mnt), raw.innerPath), nil
 	}
 
 	parentPath, err := raw.parentPath(subsystem, mnt, root)
 	if err != nil {
 		return "", err
 	}
 
-	return filepath.Join(parentPath, cgPath), nil
+	return filepath.Join(parentPath, raw.innerPath), nil
 }
 
 func (raw *cgroupData) join(subsystem string) (string, error) {

libcontainer/cgroups/fs/cpuset.go

@@ -12,6 +12,7 @@ import (
 
 	"github.com/opencontainers/runc/libcontainer/cgroups"
 	"github.com/opencontainers/runc/libcontainer/configs"
+	libcontainerUtils "github.com/opencontainers/runc/libcontainer/utils"
 )
 
 type CpusetGroup struct {
@@ -88,7 +89,7 @@ func (s *CpusetGroup) getSubsystemSettings(parent string) (cpus []byte, mems []b
 // it's parent.
 func (s *CpusetGroup) ensureParent(current, root string) error {
 	parent := filepath.Dir(current)
-	if filepath.Clean(parent) == root {
+	if libcontainerUtils.CleanPath(parent) == root {
 		return nil
 	}
 	// Avoid infinite recursion.

libcontainer/configs/cgroup_unix.go

@@ -11,15 +11,22 @@ const (
 )
 
 type Cgroup struct {
-	Name string `json:"name"`
+	// Deprecated, use Path instead
+	Name string `json:"name,omitempty"`
 
-	// name of parent cgroup or slice
-	Parent string `json:"parent"`
+	// name of parent of cgroup or slice
+	// Deprecated, use Path instead
+	Parent string `json:"parent,omitempty"`
+
+	// Path specifies the path to cgroups that are created and/or joined by the container.
+	// The path is assumed to be relative to the host system cgroup mountpoint.
+	Path string `json:"path"`
 
 	// ScopePrefix decribes prefix for the scope name
 	ScopePrefix string `json:"scope_prefix"`
 
-	// Paths represent the cgroups paths to join
+	// Paths represent the absolute cgroups paths to join.
+	// This takes precedence over Path.
 	Paths map[string]string
 
 	// Resources contains various cgroups settings to apply

libcontainer/integration/template_test.go

@@ -46,8 +46,7 @@ func newTemplateConfig(rootfs string) *configs.Config {
 			{Type: configs.NEWNET},
 		}),
 		Cgroups: &configs.Cgroup{
-			Name:   "test",
-			Parent: "integration",
+			Path: "integration/test",
 			Resources: &configs.Resources{
 				MemorySwappiness: -1,
 				AllowAllDevices:  false,

libcontainer/rootfs_linux.go

@@ -19,6 +19,7 @@ import (
 	"github.com/opencontainers/runc/libcontainer/configs"
 	"github.com/opencontainers/runc/libcontainer/label"
 	"github.com/opencontainers/runc/libcontainer/system"
+	libcontainerUtils "github.com/opencontainers/runc/libcontainer/utils"
 )
 
 const defaultMountFlags = syscall.MS_NOEXEC | syscall.MS_NOSUID | syscall.MS_NODEV
@@ -294,7 +295,7 @@ func getCgroupMounts(m *configs.Mount) ([]*configs.Mount, error) {
 // checkMountDestination checks to ensure that the mount destination is not over the top of /proc.
 // dest is required to be an abs path and have any symlinks resolved before calling this function.
 func checkMountDestination(rootfs, dest string) error {
-	if filepath.Clean(rootfs) == filepath.Clean(dest) {
+	if libcontainerUtils.CleanPath(rootfs) == libcontainerUtils.CleanPath(dest) {
 		return fmt.Errorf("mounting into / is prohibited")
 	}
 	invalidDestinations := []string{

libcontainer/utils/utils.go

@@ -5,6 +5,7 @@ import (
 	"encoding/hex"
 	"encoding/json"
 	"io"
+	"os"
 	"path/filepath"
 	"syscall"
 )
@@ -54,3 +55,27 @@ func WriteJSON(w io.Writer, v interface{}) error {
 	_, err = w.Write(data)
 	return err
 }
+
+// CleanPath makes a path safe for use with filepath.Join. This is done by not
+// only cleaning the path, but also (if the path is relative) adding a leading
+// '/' and cleaning it (then removing the leading '/'). This ensures that a
+// path resulting from prepending another path will always resolve to lexically
+// be a subdirectory of the prefixed path. This is all done lexically, so paths
+// that include symlinks won't be safe as a result of using CleanPath.
+func CleanPath(path string) string {
+	// Ensure that all paths are cleaned (especially problematic ones like
+	// "/../../../../../" which can cause lots of issues).
+	path = filepath.Clean(path)
+
+	// If the path isn't absolute, we need to do more processing to fix paths
+	// such as "../../../../<etc>/some/path". We also shouldn't convert absolute
+	// paths to relative ones.
+	if !filepath.IsAbs(path) {
+		path = filepath.Clean(string(os.PathSeparator) + path)
+		// This can't fail, as (by definition) all paths are relative to root.
+		path, _ = filepath.Rel(string(os.PathSeparator), path)
+	}
+
+	// Clean the path again for good measure.
+	return filepath.Clean(path)
+}

spec.go

@@ -18,6 +18,7 @@ import (
 	"github.com/opencontainers/runc/libcontainer/cgroups"
 	"github.com/opencontainers/runc/libcontainer/configs"
 	"github.com/opencontainers/runc/libcontainer/seccomp"
+	libcontainerUtils "github.com/opencontainers/runc/libcontainer/utils"
 	"github.com/opencontainers/specs"
 )
 
@@ -326,13 +327,22 @@ func createLibcontainerMount(cwd string, m specs.Mount) *configs.Mount {
 }
 
 func createCgroupConfig(name string, spec *specs.LinuxSpec) (*configs.Cgroup, error) {
-	myCgroupPath, err := cgroups.GetThisCgroupDir("devices")
-	if err != nil {
-		return nil, err
+	var (
+		err          error
+		myCgroupPath string
+	)
+
+	if spec.Linux.CgroupsPath != nil {
+		myCgroupPath = libcontainerUtils.CleanPath(*spec.Linux.CgroupsPath)
+	} else {
+		myCgroupPath, err = cgroups.GetThisCgroupDir("devices")
+		if err != nil {
+			return nil, err
+		}
 	}
+
 	c := &configs.Cgroup{
-		Name:      name,
-		Parent:    myCgroupPath,
+		Path:      filepath.Join(myCgroupPath, name),
 		Resources: &configs.Resources{},
 	}
 	c.Resources.AllowedDevices = allowedDevices