Merge pull request #8464 from dcantah/user-nonlinux

CRI: Support Linux username for !linux platforms

pkg/cri/sbserver/container_create.go

@@ -251,7 +251,7 @@ func (c *criService) CreateContainer(ctx context.Context, r *runtime.CreateConta
 		}
 	}()
 
-	specOpts, err := c.containerSpecOpts(config, &image.ImageSpec.Config)
+	specOpts, err := c.platformSpecOpts(platform, config, &image.ImageSpec.Config)
 	if err != nil {
 		return nil, fmt.Errorf("failed to get container spec opts: %w", err)
 	}
@@ -404,6 +404,93 @@ const (
 	hostnameEnv = "HOSTNAME"
 )
 
+// generateUserString generates valid user string based on OCI Image Spec
+// v1.0.0.
+//
+// CRI defines that the following combinations are valid:
+//
+// (none) -> ""
+// username -> username
+// username, uid -> username
+// username, uid, gid -> username:gid
+// username, gid -> username:gid
+// uid -> uid
+// uid, gid -> uid:gid
+// gid -> error
+//
+// TODO(random-liu): Add group name support in CRI.
+func generateUserString(username string, uid, gid *runtime.Int64Value) (string, error) {
+	var userstr, groupstr string
+	if uid != nil {
+		userstr = strconv.FormatInt(uid.GetValue(), 10)
+	}
+	if username != "" {
+		userstr = username
+	}
+	if gid != nil {
+		groupstr = strconv.FormatInt(gid.GetValue(), 10)
+	}
+	if userstr == "" {
+		if groupstr != "" {
+			return "", fmt.Errorf("user group %q is specified without user", groupstr)
+		}
+		return "", nil
+	}
+	if groupstr != "" {
+		userstr = userstr + ":" + groupstr
+	}
+	return userstr, nil
+}
+
+// platformSpecOpts adds additional runtime spec options that may rely on
+// runtime information (rootfs mounted), or platform specific checks with
+// no defined workaround (yet) to specify for other platforms.
+func (c *criService) platformSpecOpts(
+	platform platforms.Platform,
+	config *runtime.ContainerConfig,
+	imageConfig *imagespec.ImageConfig,
+) ([]oci.SpecOpts, error) {
+	var specOpts []oci.SpecOpts
+
+	// First deal with the set of options we can use across platforms currently.
+	// Linux user strings have workarounds on other platforms to avoid needing to
+	// mount the rootfs, but on Linux hosts it must be mounted
+	//
+	// TODO(dcantah): I think the seccomp package can be made to compile on
+	// !linux and used here as well.
+	if platform.OS == "linux" {
+		// Set container username. This could only be done by containerd, because it needs
+		// access to the container rootfs. Pass user name to containerd, and let it overwrite
+		// the spec for us.
+		securityContext := config.GetLinux().GetSecurityContext()
+		userstr, err := generateUserString(
+			securityContext.GetRunAsUsername(),
+			securityContext.GetRunAsUser(),
+			securityContext.GetRunAsGroup())
+		if err != nil {
+			return nil, fmt.Errorf("failed to generate user string: %w", err)
+		}
+		if userstr == "" {
+			// Lastly, since no user override was passed via CRI try to set via OCI
+			// Image
+			userstr = imageConfig.User
+		}
+		if userstr != "" {
+			specOpts = append(specOpts, oci.WithUser(userstr))
+		}
+	}
+
+	// Now grab the truly platform specific options (seccomp, apparmor etc. for linux
+	// for example).
+	ctrSpecOpts, err := c.containerSpecOpts(config, imageConfig)
+	if err != nil {
+		return nil, err
+	}
+	specOpts = append(specOpts, ctrSpecOpts...)
+
+	return specOpts, nil
+}
+
 // buildContainerSpec build container's OCI spec depending on controller's target platform OS.
 func (c *criService) buildContainerSpec(
 	platform platforms.Platform,

pkg/cri/sbserver/container_create_linux.go

@@ -52,28 +52,12 @@ const (
 )
 
 func (c *criService) containerSpecOpts(config *runtime.ContainerConfig, imageConfig *imagespec.ImageConfig) ([]oci.SpecOpts, error) {
-	var specOpts []oci.SpecOpts
+	var (
+		specOpts []oci.SpecOpts
+		err      error
+	)
 	securityContext := config.GetLinux().GetSecurityContext()
-	// Set container username. This could only be done by containerd, because it needs
+	userstr := "0" // runtime default
-	// access to the container rootfs. Pass user name to containerd, and let it overwrite
-	// the spec for us.
-	userstr, err := generateUserString(
-		securityContext.GetRunAsUsername(),
-		securityContext.GetRunAsUser(),
-		securityContext.GetRunAsGroup())
-	if err != nil {
-		return nil, fmt.Errorf("failed to generate user string: %w", err)
-	}
-	if userstr == "" {
-		// Lastly, since no user override was passed via CRI try to set via OCI
-		// Image
-		userstr = imageConfig.User
-	}
-	if userstr != "" {
-		specOpts = append(specOpts, oci.WithUser(userstr))
-	}
-
-	userstr = "0" // runtime default
 	if securityContext.GetRunAsUsername() != "" {
 		userstr = securityContext.GetRunAsUsername()
 	} else if securityContext.GetRunAsUser() != nil {
@@ -279,44 +263,6 @@ func appArmorProfileExists(profile string) (bool, error) {
 	}
 }
 
-// generateUserString generates valid user string based on OCI Image Spec
-// v1.0.0.
-//
-// CRI defines that the following combinations are valid:
-//
-// (none) -> ""
-// username -> username
-// username, uid -> username
-// username, uid, gid -> username:gid
-// username, gid -> username:gid
-// uid -> uid
-// uid, gid -> uid:gid
-// gid -> error
-//
-// TODO(random-liu): Add group name support in CRI.
-func generateUserString(username string, uid, gid *runtime.Int64Value) (string, error) {
-	var userstr, groupstr string
-	if uid != nil {
-		userstr = strconv.FormatInt(uid.GetValue(), 10)
-	}
-	if username != "" {
-		userstr = username
-	}
-	if gid != nil {
-		groupstr = strconv.FormatInt(gid.GetValue(), 10)
-	}
-	if userstr == "" {
-		if groupstr != "" {
-			return "", fmt.Errorf("user group %q is specified without user", groupstr)
-		}
-		return "", nil
-	}
-	if groupstr != "" {
-		userstr = userstr + ":" + groupstr
-	}
-	return userstr, nil
-}
-
 // snapshotterOpts returns any Linux specific snapshotter options for the rootfs snapshot
 func snapshotterOpts(snapshotterName string, config *runtime.ContainerConfig) []snapshots.Opt {
 	return []snapshots.Opt{}

pkg/cri/sbserver/container_create_linux_test.go

@@ -31,6 +31,7 @@ import (
 	"github.com/containerd/containerd/contrib/seccomp"
 	"github.com/containerd/containerd/mount"
 	"github.com/containerd/containerd/oci"
+	"github.com/containerd/containerd/platforms"
 	imagespec "github.com/opencontainers/image-spec/specs-go/v1"
 	runtimespec "github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/opencontainers/selinux/go-selinux"
@@ -1506,7 +1507,7 @@ additional-group-for-root:x:22222:root
 			require.NoError(t, err)
 
 			spec.Root.Path = tempRootDir // simulating /etc/{passwd, group}
-			opts, err := c.containerSpecOpts(containerConfig, imageConfig)
+			opts, err := c.platformSpecOpts(platforms.DefaultSpec(), containerConfig, imageConfig)
 			require.NoError(t, err)
 			oci.ApplyOpts(ctx, nil, testContainer, spec, opts...)