Skip to content

Commit

Permalink
oci: Allow environment values to be empty
Browse files Browse the repository at this point in the history
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 kata-containers#288

Signed-off-by: Archana Shinde <archana.m.shinde@intel.com>
  • Loading branch information
amshinde committed May 8, 2018
1 parent 992c895 commit b7674de
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 15 deletions.
49 changes: 44 additions & 5 deletions cli/exec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -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) {
Expand Down
4 changes: 0 additions & 4 deletions virtcontainers/pkg/oci/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -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],
Expand Down
6 changes: 0 additions & 6 deletions virtcontainers/pkg/oci/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit b7674de

Please sign in to comment.