Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feature: namespace options and supplemental groups for cri manager #753

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions cli/common_flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,8 @@ func addCommonFlags(flagSet *pflag.FlagSet) *container {
// user
flagSet.StringVarP(&c.user, "user", "u", "", "UID")

flagSet.StringSliceVar(&c.groupAdd, "group-add", nil, "Add additional groups to join")

flagSet.StringVar(&c.utsMode, "uts", "", "UTS namespace to use")

flagSet.StringSliceVarP(&c.volume, "volume", "v", nil, "Bind mount volumes to container")
Expand Down
2 changes: 2 additions & 0 deletions cli/container.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ type container struct {
entrypoint string
workdir string
user string
groupAdd []string
hostname string
cpushare int64
cpusetcpus string
Expand Down Expand Up @@ -228,6 +229,7 @@ func (c *container) config() (*types.ContainerCreateConfig, error) {
IpcMode: c.ipcMode,
PidMode: c.pidMode,
UTSMode: c.utsMode,
GroupAdd: c.groupAdd,
Sysctls: sysctls,
SecurityOpt: c.securityOpt,
NetworkMode: networkMode,
Expand Down
3 changes: 3 additions & 0 deletions daemon/mgr/cri.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,9 @@ const (
// TODO: specify them in the parameters of pouchd.
streamServerAddress = ""
streamServerPort = "10010"

namespaceModeHost = "host"
namespaceModeNone = "none"
)

var (
Expand Down
86 changes: 82 additions & 4 deletions daemon/mgr/cri_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,12 +157,59 @@ func parseSandboxName(name string) (*runtime.PodSandboxMetadata, error) {
}, nil
}

func modifySandboxNamespaceOptions(nsOpts *runtime.NamespaceOption, hostConfig *apitypes.HostConfig) {
if nsOpts == nil {
return
}
if nsOpts.HostPid {
hostConfig.PidMode = namespaceModeHost
}
if nsOpts.HostIpc {
hostConfig.IpcMode = namespaceModeHost
}
if nsOpts.HostNetwork {
hostConfig.NetworkMode = namespaceModeHost
}
}

func applySandboxSecurityContext(lc *runtime.LinuxPodSandboxConfig, config *apitypes.ContainerConfig, hc *apitypes.HostConfig) error {
if lc == nil {
return nil
}

var sc *runtime.LinuxContainerSecurityContext
if lc.SecurityContext != nil {
sc = &runtime.LinuxContainerSecurityContext{
SupplementalGroups: lc.SecurityContext.SupplementalGroups,
RunAsUser: lc.SecurityContext.RunAsUser,
ReadonlyRootfs: lc.SecurityContext.ReadonlyRootfs,
SelinuxOptions: lc.SecurityContext.SelinuxOptions,
NamespaceOptions: lc.SecurityContext.NamespaceOptions,
}
}

modifyContainerConfig(sc, config)
err := modifyHostConfig(sc, hc)
if err != nil {
return err
}
modifySandboxNamespaceOptions(sc.GetNamespaceOptions(), hc)

return nil
}

// applySandboxLinuxOptions applies LinuxPodSandboxConfig to pouch's HostConfig and ContainerCreateConfig.
func applySandboxLinuxOptions(hc *apitypes.HostConfig, lc *runtime.LinuxPodSandboxConfig, createConfig *apitypes.ContainerCreateConfig, image string) error {
if lc == nil {
return nil
}

// Apply security context.
err := applySandboxSecurityContext(lc, &createConfig.ContainerConfig, hc)
if err != nil {
return err
}

// Set sysctls.
hc.Sysctls = lc.Sysctls
return nil
Expand Down Expand Up @@ -292,10 +339,36 @@ func parseContainerName(name string) (*runtime.ContainerMetadata, error) {
func modifyContainerNamespaceOptions(nsOpts *runtime.NamespaceOption, podSandboxID string, hostConfig *apitypes.HostConfig) {
sandboxNSMode := fmt.Sprintf("container:%v", podSandboxID)

hostConfig.PidMode = sandboxNSMode
hostConfig.NetworkMode = sandboxNSMode
hostConfig.IpcMode = sandboxNSMode
hostConfig.UTSMode = sandboxNSMode
if nsOpts == nil {
hostConfig.PidMode = sandboxNSMode
hostConfig.IpcMode = sandboxNSMode
hostConfig.NetworkMode = sandboxNSMode
return
}

for _, n := range []struct {
hostMode bool
nsMode *string
}{
{
hostMode: nsOpts.HostPid,
nsMode: &hostConfig.PidMode,
},
{
hostMode: nsOpts.HostIpc,
nsMode: &hostConfig.IpcMode,
},
{
hostMode: nsOpts.HostNetwork,
nsMode: &hostConfig.NetworkMode,
},
} {
if n.hostMode {
*n.nsMode = namespaceModeHost
} else {
*n.nsMode = sandboxNSMode
}
}
}

// getAppArmorSecurityOpts gets appArmor options from container config.
Expand Down Expand Up @@ -346,6 +419,11 @@ func modifyHostConfig(sc *runtime.LinuxContainerSecurityContext, hostConfig *api
return nil
}

// Apply supplemental groups.
for _, group := range sc.SupplementalGroups {
hostConfig.GroupAdd = append(hostConfig.GroupAdd, strconv.FormatInt(group, 10))
}

// TODO: apply other security options.

// Apply capability options.
Expand Down
30 changes: 14 additions & 16 deletions daemon/mgr/spec_process.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,31 +63,29 @@ func setupProcessTTY(ctx context.Context, c *ContainerMeta, spec *SpecWrapper) e
}

func setupProcessUser(ctx context.Context, c *ContainerMeta, spec *SpecWrapper) (err error) {
// The user option is complicated, now we only handle case "uid".
// resolve uid:gid and user:group.
if c.Config.User == "" {
return nil
}

// container rootfs is created by containerd, pouch just creates a snapshot
// id and keeps it in memory. If container is in start process, we can not
// find if user if exist in container image, so we do some simple check.
var uid, gid uint32

if _, err := os.Stat(c.BaseFS); err != nil {
logrus.Infof("snapshot %s is not exist, maybe in start process.", c.BaseFS)
uid, gid = user.GetIntegerID(c.Config.User)
} else {
uid, gid, err = user.Get(c.BaseFS, c.Config.User)
if err != nil {
return err
if c.Config.User != "" {
if _, err := os.Stat(c.BaseFS); err != nil {
logrus.Infof("snapshot %s is not exist, maybe in start process.", c.BaseFS)
uid, gid = user.GetIntegerID(c.Config.User)
} else {
uid, gid, err = user.Get(c.BaseFS, c.Config.User)
if err != nil {
return err
}
}
}

additionalGids := user.GetAdditionalGids(c.HostConfig.GroupAdd)

spec.s.Process.User = specs.User{
UID: uid,
GID: gid,
UID: uid,
GID: gid,
AdditionalGids: additionalGids,
}

return nil
}
4 changes: 2 additions & 2 deletions hack/cri-test/test-cri.sh
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,10 @@ POUCH_SOCK="/var/run/pouchcri.sock"

# CRI_FOCUS focuses the test to run.
# With the CRI manager completes its function, we may need to expand this field.
CRI_FOCUS=${CRI_FOCUS:-"PodSandbox|AppArmor|Privileged is true|basic operations on container|Runtime info|mount propagation|volume and device|RunAsUser|container port|Streaming"}
CRI_FOCUS=${CRI_FOCUS:-"PodSandbox|AppArmor|Privileged is true|basic operations on container|Runtime info|mount propagation|volume and device|RunAsUser|container port|Streaming|NamespaceOption|SupplementalGroups"}

# CRI_SKIP skips the test to skip.
CRI_SKIP=${CRI_SKIP:-"RunAsUserName|attach"}
CRI_SKIP=${CRI_SKIP:-"RunAsUserName|attach|HostNetwork"}
# REPORT_DIR is the the directory to store test logs.
REPORT_DIR=${REPORT_DIR:-"/tmp/test-cri"}

Expand Down
16 changes: 16 additions & 0 deletions pkg/user/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,22 @@ func GetIntegerID(user string) (uint32, uint32) {
return uint32(uid), uint32(gid)
}

// GetAdditionalGids parse supplementary gids from slice groups.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a unit test case for this func GetAdditionalGids?

func GetAdditionalGids(groups []string) []uint32 {
var additionalGids []uint32

// TODO: check whether group is valid and support group name format like "nobody".
for _, group := range groups {
gid, err := strconv.ParseUint(group, 10, 32)
if err != nil {
continue
}
additionalGids = append(additionalGids, uint32(gid))
}

return additionalGids
}

// parseID parses uid from /etc/passwd.
func parseID(file, str string, parserFilter filterFunc) (uint32, error) {
idInt, idErr := strconv.Atoi(str)
Expand Down
10 changes: 10 additions & 0 deletions pkg/user/user_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package user

import (
"fmt"
"reflect"
"testing"

"github.com/stretchr/testify/assert"
Expand All @@ -20,3 +21,12 @@ func TestParseString(t *testing.T) {
assert.Equal(fmt.Sprintf("%s:%s:%d:%d", u1, u2, i1, i2), l)
}
}

func TestGetAdditionalGids(t *testing.T) {
groups := []string{"1234", "5678"}
expected := []uint32{1234, 5678}

result := GetAdditionalGids(groups)
assert := assert.New(t)
assert.True(reflect.DeepEqual(expected, result), true)
}