From b7674de3cfc386ecbf8766fed3433941bc10135b Mon Sep 17 00:00:00 2001
From: Archana Shinde <archana.m.shinde@intel.com>
Date: Mon, 7 May 2018 11:21:16 -0700
Subject: [PATCH] oci: Allow environment values to be empty

An empty string for an environment variable simply means that the
variable is unset. Do not error out if the env value is empty.

Fixes #288

Signed-off-by: Archana Shinde <archana.m.shinde@intel.com>
---
 cli/exec_test.go                     | 49 +++++++++++++++++++++++++---
 virtcontainers/pkg/oci/utils.go      |  4 ---
 virtcontainers/pkg/oci/utils_test.go |  6 ----
 3 files changed, 44 insertions(+), 15 deletions(-)

diff --git a/cli/exec_test.go b/cli/exec_test.go
index 7aeceb2238..d45bd3091b 100644
--- a/cli/exec_test.go
+++ b/cli/exec_test.go
@@ -560,16 +560,20 @@ func TestExecuteWithValidProcessJson(t *testing.T) {
 	assert.Equal(exitErr.ExitCode(), 0, "Exit code should have been 0 for fake workload %s", workload)
 }
 
-func TestExecuteWithInvalidEnvironment(t *testing.T) {
+func TestExecuteWithEmptyEnvironmentValue(t *testing.T) {
 	assert := assert.New(t)
 
 	tmpdir, err := ioutil.TempDir("", "")
 	assert.NoError(err)
 	defer os.RemoveAll(tmpdir)
 
+	pidFilePath := filepath.Join(tmpdir, "pid")
+	consolePath := "/dev/ptmx"
+
+	flagSet := testExecParamsSetup(t, pidFilePath, consolePath, false)
+
 	processPath := filepath.Join(tmpdir, "process.json")
 
-	flagSet := flag.NewFlagSet("", 0)
 	flagSet.String("process", processPath, "")
 	flagSet.Parse([]string{testContainerID})
 	ctx := cli.NewContext(cli.NewApp(), flagSet, nil)
@@ -600,9 +604,24 @@ func TestExecuteWithInvalidEnvironment(t *testing.T) {
 	}()
 
 	processJSON := `{
+				"consoleSize": {
+					"height": 15,
+					"width": 15
+				},
+				"terminal": true,
+				"user": {
+					"uid": 0,
+					"gid": 0
+				},
+				"args": [
+					"sh"
+				],
 				"env": [
+					"PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin",
 					"TERM="
-				]
+				],
+				"cwd": "/"
+
 			}`
 
 	f, err := os.OpenFile(processPath, os.O_RDWR|os.O_CREATE, testFileMode)
@@ -614,13 +633,33 @@ func TestExecuteWithInvalidEnvironment(t *testing.T) {
 
 	defer os.Remove(processPath)
 
+	workload := []string{"cat", "/dev/null"}
+
+	testingImpl.EnterContainerFunc = func(sandboxID, containerID string, cmd vc.Cmd) (vc.VCSandbox, vc.VCContainer, *vc.Process, error) {
+		// create a fake container process
+		command := exec.Command(workload[0], workload[1:]...)
+		err := command.Start()
+		assert.NoError(err, "Unable to start process %v: %s", workload, err)
+
+		vcProcess := vc.Process{}
+		vcProcess.Pid = command.Process.Pid
+
+		return &vcmock.Sandbox{}, &vcmock.Container{}, &vcProcess, nil
+	}
+
+	defer func() {
+		testingImpl.EnterContainerFunc = nil
+		os.Remove(pidFilePath)
+	}()
+
 	fn, ok := execCLICommand.Action.(func(context *cli.Context) error)
 	assert.True(ok)
 
 	// vcAnnotations.EnvVars error due to incorrect environment
 	err = fn(ctx)
-	assert.Error(err)
-	assert.False(vcmock.IsMockError(err))
+	exitErr, ok := err.(*cli.ExitError)
+	assert.True(ok, true, "Exit code not received for fake workload process")
+	assert.Equal(exitErr.ExitCode(), 0, "Exit code should have been 0 for empty environment variable value")
 }
 
 func TestGenerateExecParams(t *testing.T) {
diff --git a/virtcontainers/pkg/oci/utils.go b/virtcontainers/pkg/oci/utils.go
index c39c4a9ff6..2bd271c546 100644
--- a/virtcontainers/pkg/oci/utils.go
+++ b/virtcontainers/pkg/oci/utils.go
@@ -640,10 +640,6 @@ func EnvVars(envs []string) ([]vc.EnvVar, error) {
 
 		envSlice[1] = strings.Trim(envSlice[1], "' ")
 
-		if envSlice[1] == "" {
-			return []vc.EnvVar{}, fmt.Errorf("Environment value cannot be empty")
-		}
-
 		envVar := vc.EnvVar{
 			Var:   envSlice[0],
 			Value: envSlice[1],
diff --git a/virtcontainers/pkg/oci/utils_test.go b/virtcontainers/pkg/oci/utils_test.go
index 80f051847c..7b5b8a3f2d 100644
--- a/virtcontainers/pkg/oci/utils_test.go
+++ b/virtcontainers/pkg/oci/utils_test.go
@@ -479,12 +479,6 @@ func TestMalformedEnvVars(t *testing.T) {
 		t.Fatalf("EnvVars() succeeded unexpectedly: [%s] variable=%s value=%s", envVars[0], r[0].Var, r[0].Value)
 	}
 
-	envVars = []string{"TERM="}
-	r, err = EnvVars(envVars)
-	if err == nil {
-		t.Fatalf("EnvVars() succeeded unexpectedly: [%s] variable=%s value=%s", envVars[0], r[0].Var, r[0].Value)
-	}
-
 	envVars = []string{"=foo"}
 	r, err = EnvVars(envVars)
 	if err == nil {