Merge pull request #1994 from AkihiroSuda/fix-user

fix running a container with config.User

cmd/ctr/commands/run/run_unix.go

@@ -75,7 +75,10 @@ func newContainer(ctx gocontext.Context, client *containerd.Client, context *cli
 	if context.Bool("net-host") {
 		opts = append(opts, oci.WithHostNamespace(specs.NetworkNamespace), oci.WithHostHostsFile, oci.WithHostResolvconf)
 	}
-	cOpts = append([]containerd.NewContainerOpts{containerd.WithNewSpec(opts...)}, cOpts...)
+	// oci.WithImageConfig (WithUsername, WithUserID) depends on rootfs snapshot for resolving /etc/passwd.
+	// So cOpts needs to have precedence over opts.
+	// TODO: WithUsername, WithUserID should additionally support non-snapshot rootfs
+	cOpts = append(cOpts, []containerd.NewContainerOpts{containerd.WithNewSpec(opts...)}...)
 	return client.NewContainer(ctx, id, cOpts...)
 }
 

oci/spec_opts_unix.go

@@ -95,22 +95,25 @@ func WithImageConfig(image Image) SpecOpts {
 		s.Process.Env = append(s.Process.Env, config.Env...)
 		cmd := config.Cmd
 		s.Process.Args = append(config.Entrypoint, cmd...)
+		cwd := config.WorkingDir
+		if cwd == "" {
+			cwd = "/"
+		}
+		s.Process.Cwd = cwd
 		if config.User != "" {
+			// According to OCI Image Spec v1.0.0, the following are valid for Linux:
+			//   user, uid, user:group, uid:gid, uid:group, user:gid
 			parts := strings.Split(config.User, ":")
 			switch len(parts) {
 			case 1:
 				v, err := strconv.Atoi(parts[0])
 				if err != nil {
 					// if we cannot parse as a uint they try to see if it is a username
-					if err := WithUsername(config.User)(ctx, client, c, s); err != nil {
+					return WithUsername(config.User)(ctx, client, c, s)
-						return err
-					}
-					return err
-				}
-				if err := WithUserID(uint32(v))(ctx, client, c, s); err != nil {
-					return err
 				}
+				return WithUserID(uint32(v))(ctx, client, c, s)
 			case 2:
+				// TODO: support username and groupname
 				v, err := strconv.Atoi(parts[0])
 				if err != nil {
 					return errors.Wrapf(err, "parse uid %s", parts[0])
@@ -125,11 +128,6 @@ func WithImageConfig(image Image) SpecOpts {
 				return fmt.Errorf("invalid USER value %s", config.User)
 			}
 		}
-		cwd := config.WorkingDir
-		if cwd == "" {
-			cwd = "/"
-		}
-		s.Process.Cwd = cwd
 		return nil
 	}
 }
@@ -259,6 +257,7 @@ func WithUIDGID(uid, gid uint32) SpecOpts {
 // uid, and not returns error.
 func WithUserID(uid uint32) SpecOpts {
 	return func(ctx context.Context, client Client, c *containers.Container, s *specs.Spec) (err error) {
+		// TODO: support non-snapshot rootfs
 		if c.Snapshotter == "" {
 			return errors.Errorf("no snapshotter set for container")
 		}
@@ -307,6 +306,7 @@ func WithUserID(uid uint32) SpecOpts {
 // does not exist, or the username is not found in /etc/passwd,
 // it returns error.
 func WithUsername(username string) SpecOpts {
+	// TODO: support non-snapshot rootfs
 	return func(ctx context.Context, client Client, c *containers.Container, s *specs.Spec) (err error) {
 		if c.Snapshotter == "" {
 			return errors.Errorf("no snapshotter set for container")