Merge pull request #913 from cloudfoundry-incubator/addgroupsnocompatible

Let the user explicitly specify `additionalGids` on `runc exec`

libcontainer/configs/config.go

@@ -148,10 +148,6 @@ type Config struct {
 	// More information about kernel oom score calculation here: https://lwn.net/Articles/317814/
 	OomScoreAdj int `json:"oom_score_adj"`
 
-	// AdditionalGroups specifies the gids that should be added to supplementary groups
-	// in addition to those that the user belongs to.
-	AdditionalGroups []string `json:"additional_groups"`
-
 	// UidMappings is an array of User ID mappings for User Namespaces
 	UidMappings []IDMap `json:"uid_mappings"`
 

libcontainer/container_linux.go

@@ -390,6 +390,7 @@ func (c *linuxContainer) newInitConfig(process *Process) *initConfig {
 		Args:             process.Args,
 		Env:              process.Env,
 		User:             process.User,
+		AdditionalGroups: process.AdditionalGroups,
 		Cwd:              process.Cwd,
 		Console:          process.consolePath,
 		Capabilities:     process.Capabilities,

libcontainer/init_linux.go

@@ -52,6 +52,7 @@ type initConfig struct {
 	AppArmorProfile  string           `json:"apparmor_profile"`
 	NoNewPrivileges  bool             `json:"no_new_privileges"`
 	User             string           `json:"user"`
+	AdditionalGroups []string         `json:"additional_groups"`
 	Config           *configs.Config  `json:"config"`
 	Console          string           `json:"console"`
 	Networks         []*network       `json:"network"`
@@ -213,8 +214,8 @@ func setupUser(config *initConfig) error {
 	}
 
 	var addGroups []int
-	if len(config.Config.AdditionalGroups) > 0 {
-		addGroups, err = user.GetAdditionalGroupsPath(config.Config.AdditionalGroups, groupPath)
+	if len(config.AdditionalGroups) > 0 {
+		addGroups, err = user.GetAdditionalGroupsPath(config.AdditionalGroups, groupPath)
 		if err != nil {
 			return err
 		}

libcontainer/integration/exec_test.go

@@ -431,7 +431,6 @@ func TestAdditionalGroups(t *testing.T) {
 	defer remove(rootfs)
 
 	config := newTemplateConfig(rootfs)
-	config.AdditionalGroups = []string{"plugdev", "audio"}
 
 	factory, err := libcontainer.New(root, libcontainer.Cgroupfs)
 	ok(t, err)
@@ -442,11 +441,12 @@ func TestAdditionalGroups(t *testing.T) {
 
 	var stdout bytes.Buffer
 	pconfig := libcontainer.Process{
-		Cwd:    "/",
-		Args:   []string{"sh", "-c", "id", "-Gn"},
-		Env:    standardEnvironment,
-		Stdin:  nil,
-		Stdout: &stdout,
+		Cwd:              "/",
+		Args:             []string{"sh", "-c", "id", "-Gn"},
+		Env:              standardEnvironment,
+		Stdin:            nil,
+		Stdout:           &stdout,
+		AdditionalGroups: []string{"plugdev", "audio"},
 	}
 	err = container.Run(&pconfig)
 	ok(t, err)

libcontainer/integration/execin_test.go

@@ -134,6 +134,64 @@ func testExecInRlimit(t *testing.T, userns bool) {
 	}
 }
 
+func TestExecInAdditionalGroups(t *testing.T) {
+	if testing.Short() {
+		return
+	}
+
+	rootfs, err := newRootfs()
+	ok(t, err)
+	defer remove(rootfs)
+
+	config := newTemplateConfig(rootfs)
+	container, err := newContainer(config)
+	ok(t, err)
+	defer container.Destroy()
+
+	// Execute a first process in the container
+	stdinR, stdinW, err := os.Pipe()
+	ok(t, err)
+	process := &libcontainer.Process{
+		Cwd:   "/",
+		Args:  []string{"cat"},
+		Env:   standardEnvironment,
+		Stdin: stdinR,
+	}
+	err = container.Run(process)
+	stdinR.Close()
+	defer stdinW.Close()
+	ok(t, err)
+
+	var stdout bytes.Buffer
+	pconfig := libcontainer.Process{
+		Cwd:              "/",
+		Args:             []string{"sh", "-c", "id", "-Gn"},
+		Env:              standardEnvironment,
+		Stdin:            nil,
+		Stdout:           &stdout,
+		AdditionalGroups: []string{"plugdev", "audio"},
+	}
+	err = container.Run(&pconfig)
+	ok(t, err)
+
+	// Wait for process
+	waitProcess(&pconfig, t)
+
+	stdinW.Close()
+	waitProcess(process, t)
+
+	outputGroups := string(stdout.Bytes())
+
+	// Check that the groups output has the groups that we specified
+	if !strings.Contains(outputGroups, "audio") {
+		t.Fatalf("Listed groups do not contain the audio group as expected: %v", outputGroups)
+	}
+
+	if !strings.Contains(outputGroups, "plugdev") {
+		t.Fatalf("Listed groups do not contain the plugdev group as expected: %v", outputGroups)
+	}
+}
+
 func TestExecInError(t *testing.T) {
 	if testing.Short() {
 		return

libcontainer/process.go

@@ -28,6 +28,10 @@ type Process struct {
 	// local to the container's user and group configuration.
 	User string
 
+	// AdditionalGroups specifies the gids that should be added to supplementary groups
+	// in addition to those that the user belongs to.
+	AdditionalGroups []string
+
 	// Cwd will change the processes current working directory inside the container's rootfs.
 	Cwd string
 

libcontainer/specconv/spec_linux.go

@@ -8,7 +8,6 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
-	"strconv"
 	"strings"
 	"syscall"
 	"time"
@@ -229,9 +228,6 @@ func CreateLibcontainerConfig(opts *CreateOpts) (*configs.Config, error) {
 	if spec.Linux.Resources != nil && spec.Linux.Resources.OOMScoreAdj != nil {
 		config.OomScoreAdj = *spec.Linux.Resources.OOMScoreAdj
 	}
-	for _, g := range spec.Process.User.AdditionalGids {
-		config.AdditionalGroups = append(config.AdditionalGroups, strconv.FormatUint(uint64(g), 10))
-	}
 	createHooks(spec, config)
 	config.MountLabel = spec.Linux.MountLabel
 	config.Version = specs.Version

utils_linux.go

@@ -7,6 +7,7 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"strconv"
 	"syscall"
 
 	"github.com/Sirupsen/logrus"
@@ -83,6 +84,9 @@ func newProcess(p specs.Process) (*libcontainer.Process, error) {
 		NoNewPrivileges: &p.NoNewPrivileges,
 		AppArmorProfile: p.ApparmorProfile,
 	}
+	for _, gid := range p.User.AdditionalGids {
+		lp.AdditionalGroups = append(lp.AdditionalGroups, strconv.FormatUint(uint64(gid), 10))
+	}
 	for _, rlimit := range p.Rlimits {
 		rl, err := createLibContainerRlimit(rlimit)
 		if err != nil {