Merge pull request #7070 from junnplus/additional-gids

dmcgowan · web-flow · commit 94fe150b87d2 · 2022-06-21T12:17:27.000-07:00
add WithAppendAdditionalGroups helper
diff --git a/oci/spec_opts.go b/oci/spec_opts.go
@@ -875,6 +875,63 @@ func WithAdditionalGIDs(userstr string) SpecOpts {
 	}
 }
 
+// WithAppendAdditionalGroups append additional groups within the container.
+// The passed in groups can be either a gid or a groupname.
+func WithAppendAdditionalGroups(groups ...string) SpecOpts {
+	return func(ctx context.Context, client Client, c *containers.Container, s *Spec) (err error) {
+		// For LCOW or on Darwin additional GID's are not supported
+		if s.Windows != nil || runtime.GOOS == "darwin" {
+			return nil
+		}
+		setProcess(s)
+		setAdditionalGids := func(root string) error {
+			gpath, err := fs.RootPath(root, "/etc/group")
+			if err != nil {
+				return err
+			}
+			ugroups, err := user.ParseGroupFile(gpath)
+			if err != nil {
+				return err
+			}
+			groupMap := make(map[string]user.Group)
+			for _, group := range ugroups {
+				groupMap[group.Name] = group
+				groupMap[strconv.Itoa(group.Gid)] = group
+			}
+			var gids []uint32
+			for _, group := range groups {
+				g, ok := groupMap[group]
+				if !ok {
+					return fmt.Errorf("unable to find group %s", group)
+				}
+				gids = append(gids, uint32(g.Gid))
+			}
+			s.Process.User.AdditionalGids = append(s.Process.User.AdditionalGids, gids...)
+			return nil
+		}
+		if c.Snapshotter == "" && c.SnapshotKey == "" {
+			if !filepath.IsAbs(s.Root.Path) {
+				return errors.New("rootfs absolute path is required")
+			}
+			return setAdditionalGids(s.Root.Path)
+		}
+		if c.Snapshotter == "" {
+			return errors.New("no snapshotter set for container")
+		}
+		if c.SnapshotKey == "" {
+			return errors.New("rootfs snapshot not created for container")
+		}
+		snapshotter := client.SnapshotService(c.Snapshotter)
+		mounts, err := snapshotter.Mounts(ctx, c.SnapshotKey)
+		if err != nil {
+			return err
+		}
+
+		mounts = tryReadonlyMounts(mounts)
+		return mount.WithTempMount(ctx, mounts, setAdditionalGids)
+	}
+}
+
 // WithCapabilities sets Linux capabilities on the process
 func WithCapabilities(caps []string) SpecOpts {
 	return func(_ context.Context, _ Client, _ *containers.Container, s *Spec) error {
@@ -979,7 +1036,7 @@ func UserFromPath(root string, filter func(user.User) bool) (user.User, error) {
 // ErrNoGroupsFound can be returned from GIDFromPath
 var ErrNoGroupsFound = errors.New("no groups found")
 
-// GIDFromPath inspects the GID using /etc/passwd in the specified rootfs.
+// GIDFromPath inspects the GID using /etc/group in the specified rootfs.
 // filter can be nil.
 func GIDFromPath(root string, filter func(user.Group) bool) (gid uint32, err error) {
 	gpath, err := fs.RootPath(root, "/etc/group")
diff --git a/oci/spec_opts_linux_test.go b/oci/spec_opts_linux_test.go
@@ -22,8 +22,11 @@ import (
 	"path/filepath"
 	"testing"
 
+	"github.com/containerd/containerd/containers"
 	"github.com/containerd/containerd/pkg/testutil"
+	"github.com/containerd/continuity/fs/fstest"
 	specs "github.com/opencontainers/runtime-spec/specs-go"
+	"github.com/stretchr/testify/assert"
 	"golang.org/x/sys/unix"
 )
 
@@ -247,3 +250,76 @@ func TestGetDevices(t *testing.T) {
 		})
 	})
 }
+
+func TestWithAppendAdditionalGroups(t *testing.T) {
+	t.Parallel()
+	expectedContent := `root:x:0:root
+bin:x:1:root,bin,daemon
+daemon:x:2:root,bin,daemon
+`
+	td := t.TempDir()
+	apply := fstest.Apply(
+		fstest.CreateDir("/etc", 0777),
+		fstest.CreateFile("/etc/group", []byte(expectedContent), 0777),
+	)
+	if err := apply.Apply(td); err != nil {
+		t.Fatalf("failed to apply: %v", err)
+	}
+	c := containers.Container{ID: t.Name()}
+
+	testCases := []struct {
+		name           string
+		additionalGIDs []uint32
+		groups         []string
+		expected       []uint32
+		err            string
+	}{
+		{
+			name:   "no additional gids",
+			groups: []string{},
+		},
+		{
+			name:     "no additional gids, append root gid",
+			groups:   []string{"root"},
+			expected: []uint32{0},
+		},
+		{
+			name:     "no additional gids, append bin and daemon gids",
+			groups:   []string{"bin", "daemon"},
+			expected: []uint32{1, 2},
+		},
+		{
+			name:           "has root additional gids, append bin and daemon gids",
+			additionalGIDs: []uint32{0},
+			groups:         []string{"bin", "daemon"},
+			expected:       []uint32{0, 1, 2},
+		},
+		{
+			name:   "unknown group",
+			groups: []string{"unknown"},
+			err:    "unable to find group unknown",
+		},
+	}
+
+	for _, testCase := range testCases {
+		t.Run(testCase.name, func(t *testing.T) {
+			t.Parallel()
+			s := Spec{
+				Version: specs.Version,
+				Root: &specs.Root{
+					Path: td,
+				},
+				Process: &specs.Process{
+					User: specs.User{
+						AdditionalGids: testCase.additionalGIDs,
+					},
+				},
+			}
+			err := WithAppendAdditionalGroups(testCase.groups...)(context.Background(), nil, &c, &s)
+			if err != nil {
+				assert.EqualError(t, err, testCase.err)
+			}
+			assert.Equal(t, testCase.expected, s.Process.User.AdditionalGids)
+		})
+	}
+}
