From 375ddf4b3a7f7f1895201483ffcae2106401d0a5 Mon Sep 17 00:00:00 2001
From: "W. Trevor King" <wking@tremily.us>
Date: Fri, 19 Jan 2018 10:05:12 -0800
Subject: [PATCH] server/container_create: Allow for nil Process

OCI runtime callers (like CRI-O) are allowed to leave process unset
[1] for containers that they do not intend to 'start'.  When we don't
have any process.args, we *must* leave process unset (because
process.args is required [2]).  My personal preference would have been
to have both process and process.args optional [3], which would have
allowed for these settings to be decoupled, but that's not where the
spec ended up.

When we have no args and are clearing Process, we need to ensure that
we don't re-create an args-less structure later on by populating
process.user or similar.  This commit collects later process-creating
calls (e.g. setupContainerUser, which populates process.user) into the
"we have some args" branch.

This commit leaves earlier process-creating calls
(e.g. SetProcessTerminal) where they were.  Anything they do inside
Process will be clobbered later if we nil it, but that's fine.

[1]: https://github.com/opencontainers/runtime-spec/blame/v1.0.1/config.md#L145
[2]: https://github.com/opencontainers/runtime-spec/blame/v1.0.1/config.md#L157
[3]: https://github.com/opencontainers/runtime-spec/pull/701#issue-210601101

Signed-off-by: W. Trevor King <wking@tremily.us>
---
 server/container_create.go | 80 +++++++++++++++++++-------------------
 1 file changed, 41 insertions(+), 39 deletions(-)

diff --git a/server/container_create.go b/server/container_create.go
index 01820d2d9198..579e8268f2d1 100644
--- a/server/container_create.go
+++ b/server/container_create.go
@@ -395,10 +395,6 @@ func buildOCIProcessArgs(containerKubeConfig *pb.ContainerConfig, ociConfig *v1.
 		}
 	}
 
-	if len(kubeCommands) == 0 && len(kubeArgs) == 0 {
-		return nil, fmt.Errorf("no command specified")
-	}
-
 	processArgs := append(kubeCommands, kubeArgs...)
 
 	logrus.Debugf("OCI process args %v", processArgs)
@@ -1164,39 +1160,52 @@ func (s *Server) createSandboxContainer(ctx context.Context, containerID string,
 	}
 
 	processArgs := []string{}
-	if containerImageConfig != nil {
-		processArgs, err := buildOCIProcessArgs(containerConfig, &containerImageConfig.Config)
-		if err != nil {
-			return nil, err
-		}
+	if containerImageConfig == nil {
+		processArgs, err = buildOCIProcessArgs(containerConfig, nil)
+	} else {
+		processArgs, err = buildOCIProcessArgs(containerConfig, &containerImageConfig.Config)
 	}
-	specgen.SetProcessArgs(processArgs)
-
-	envs := mergeEnvs(containerImageConfig, containerConfig.GetEnvs())
-	for _, e := range envs {
-		parts := strings.SplitN(e, "=", 2)
-		specgen.AddProcessEnv(parts[0], parts[1])
+	if err != nil {
+		return nil, err
 	}
+	if len(processArgs) == 0 {
+		specgen.Spec().Process = nil
+	} else {
+		specgen.SetProcessArgs(processArgs)
 
-	// Set working directory
-	// Pick it up from image config first and override if specified in CRI
-	containerCwd := "/"
-	if containerImageConfig != nil {
-		imageCwd := containerImageConfig.Config.WorkingDir
-		if imageCwd != "" {
-			containerCwd = imageCwd
+		envs := mergeEnvs(containerImageConfig, containerConfig.GetEnvs())
+		for _, e := range envs {
+			parts := strings.SplitN(e, "=", 2)
+			specgen.AddProcessEnv(parts[0], parts[1])
 		}
-	}
-	runtimeCwd := containerConfig.WorkingDir
-	if runtimeCwd != "" {
-		containerCwd = runtimeCwd
-	}
-	specgen.SetProcessCwd(containerCwd)
-	if err := setupWorkingDirectory(mountPoint, mountLabel, containerCwd); err != nil {
-		if err1 := s.StorageRuntimeServer().StopContainer(containerID); err1 != nil {
-			return nil, fmt.Errorf("can't umount container after cwd error %v: %v", err, err1)
+
+		// Set working directory
+		// Pick it up from image config first and override if specified in CRI
+		containerCwd := "/"
+		if containerImageConfig != nil {
+			imageCwd := containerImageConfig.Config.WorkingDir
+			if imageCwd != "" {
+				containerCwd = imageCwd
+			}
+		}
+		runtimeCwd := containerConfig.WorkingDir
+		if runtimeCwd != "" {
+			containerCwd = runtimeCwd
+		}
+		specgen.SetProcessCwd(containerCwd)
+		if err := setupWorkingDirectory(mountPoint, mountLabel, containerCwd); err != nil {
+			if err1 := s.StorageRuntimeServer().StopContainer(containerID); err1 != nil {
+				return nil, fmt.Errorf("can't umount container after cwd error %v: %v", err, err1)
+			}
+			return nil, err
+		}
+
+		// Setup user and groups
+		if linux != nil {
+			if err = setupContainerUser(&specgen, mountPoint, linux.GetSecurityContext(), containerImageConfig); err != nil {
+				return nil, err
+			}
 		}
-		return nil, err
 	}
 
 	var secretMounts []rspec.Mount
@@ -1229,13 +1238,6 @@ func (s *Server) createSandboxContainer(ctx context.Context, containerID string,
 		return nil, err
 	}
 
-	// Setup user and groups
-	if linux != nil {
-		if err = setupContainerUser(&specgen, mountPoint, linux.GetSecurityContext(), containerImageConfig); err != nil {
-			return nil, err
-		}
-	}
-
 	// Set up pids limit if pids cgroup is mounted
 	_, err = cgroups.FindCgroupMountpoint("pids")
 	if err == nil {