Merge pull request #2005 from AkihiroSuda/fix-user-1.0

estesp · web-flow · commit dafb45d7b680 · 2018-01-16T10:24:12.000-05:00
[release/1.0] fix running a container with config.User
diff --git a/cmd/ctr/commands/run/run_unix.go b/cmd/ctr/commands/run/run_unix.go
@@ -72,6 +72,9 @@ 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...)
 }
diff --git a/oci/spec_opts_unix.go b/oci/spec_opts_unix.go
@@ -96,22 +96,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 err
-					}
-					return err
-				}
-				if err := WithUserID(uint32(v))(ctx, client, c, s); err != nil {
-					return err
+					return WithUsername(config.User)(ctx, client, c, s)
 				}
+				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])
@@ -126,11 +129,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
 	}
 }
@@ -260,6 +258,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")
 		}
@@ -322,6 +321,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")

Original file line number	Diff line number	Diff line change
`@@ -72,6 +72,9 @@ func newContainer(ctx gocontext.Context, client containerd.Client, context cli`
`72`	`72`	`if context.Bool("net-host") {`
`73`	`73`	`opts = append(opts, oci.WithHostNamespace(specs.NetworkNamespace), oci.WithHostHostsFile, oci.WithHostResolvconf)`
`74`	`74`	`}`
`75`		`- cOpts = append([]containerd.NewContainerOpts{containerd.WithNewSpec(opts...)}, cOpts...)`
	`75`	`+ // oci.WithImageConfig (WithUsername, WithUserID) depends on rootfs snapshot for resolving /etc/passwd.`
	`76`	`+ // So cOpts needs to have precedence over opts.`
	`77`	`+ // TODO: WithUsername, WithUserID should additionally support non-snapshot rootfs`
	`78`	`+ cOpts = append(cOpts, []containerd.NewContainerOpts{containerd.WithNewSpec(opts...)}...)`
`76`	`79`	`return client.NewContainer(ctx, id, cOpts...)`
`77`	`80`	`}`