diff --git a/cli/common_flags.go b/cli/common_flags.go index e5b5d2d25..26cb08d5d 100644 --- a/cli/common_flags.go +++ b/cli/common_flags.go @@ -36,6 +36,7 @@ func addCommonFlags(flagSet *pflag.FlagSet) *container { flagSet.BoolVar(&c.enableLxcfs, "enableLxcfs", false, "Enable lxcfs for the container, only effective when enable-lxcfs switched on in Pouchd") flagSet.StringVar(&c.entrypoint, "entrypoint", "", "Overwrite the default ENTRYPOINT of the image") flagSet.StringArrayVarP(&c.env, "env", "e", nil, "Set environment variables for container('--env A=' means setting env A to empty, '--env B' means removing env B from container env inherited from image)") + flagSet.StringArrayVar(&c.envfile, "env-file", nil, "Read in a file of environment variables") flagSet.StringVar(&c.hostname, "hostname", "", "Set container's hostname") flagSet.BoolVar(&c.disableNetworkFiles, "disable-network-files", false, "Disable the generation of network files(/etc/hostname, /etc/hosts and /etc/resolv.conf) for container. If true, no network files will be generated. Default false") diff --git a/cli/container.go b/cli/container.go index 635529ca1..75fe7eed6 100644 --- a/cli/container.go +++ b/cli/container.go @@ -18,6 +18,7 @@ type container struct { volumesFrom []string runtime string env []string + envfile []string entrypoint string workdir string user string diff --git a/cli/create.go b/cli/create.go index 3a0a116f8..c1c9977d2 100644 --- a/cli/create.go +++ b/cli/create.go @@ -55,6 +55,12 @@ func (cc *CreateCommand) runCreate(args []string) error { if err != nil { return fmt.Errorf("failed to create container: %v", err) } + + // collect all the environment variables for the container + config.Env, err = readKVStrings(cc.envfile, cc.env) + if err != nil { + return nil + } config.ContainerConfig.OpenStdin = cc.openstdin config.Image = args[0] diff --git a/cli/envfile.go b/cli/envfile.go new file mode 100644 index 000000000..aa6dcc7b5 --- /dev/null +++ b/cli/envfile.go @@ -0,0 +1,71 @@ +package main + +import ( + "bufio" + "fmt" + "os" + "strings" + "unicode" +) + +// reads a file of line terminated key=value pairs, and overrides any keys +// present in the file with additional pairs specified in the override parameter +func readKVStrings(files []string, override []string) ([]string, error) { + envVariables := []string{} + for _, ef := range files { + parsedVars, err := parseEnvFile(ef) + if err != nil { + return nil, err + } + envVariables = append(envVariables, parsedVars...) + } + // parse the '-e' and '--env' after, to allow override + envVariables = append(envVariables, override...) + + return envVariables, nil +} + +// parseEnvFile reads a file with environment variables enumerated by lines +func parseEnvFile(filename string) ([]string, error) { + fh, err := os.Open(filename) + if err != nil { + return []string{}, err + } + defer fh.Close() + + lines := []string{} + scanner := bufio.NewScanner(fh) + for scanner.Scan() { + // trim the line from all leading whitespace first + line := strings.TrimLeftFunc(scanner.Text(), unicode.IsSpace) + // line is not empty, and not starting with '#' + if len(line) > 0 && !strings.HasPrefix(line, "#") { + data := strings.SplitN(line, "=", 2) + + // trim the front of a variable, but nothing else + variable := data[0] + if strings.IndexFunc(variable, unicode.IsSpace) != -1 { + return []string{}, ErrBadEnvVariable{fmt.Sprintf("variable '%s' has white spaces", variable)} + } + + if len(data) > 1 { + // pass the value through, no trimming + lines = append(lines, fmt.Sprintf("%s=%s", variable, data[1])) + } else { + // if only a pass-through variable is given, clean it up. + lines = append(lines, fmt.Sprintf("%s=%s", strings.TrimSpace(line), os.Getenv(line))) + } + } + } + return lines, scanner.Err() +} + +// ErrBadEnvVariable typed error for bad environment variable +type ErrBadEnvVariable struct { + msg string +} + +// Error implements error interface. +func (e ErrBadEnvVariable) Error() string { + return fmt.Sprintf("poorly formatted environment: %s", e.msg) +} diff --git a/cli/envfile_test.go b/cli/envfile_test.go new file mode 100644 index 000000000..d24fc5826 --- /dev/null +++ b/cli/envfile_test.go @@ -0,0 +1,148 @@ +package main + +import ( + "bufio" + "fmt" + "io/ioutil" + "os" + "reflect" + "strings" + "testing" +) + +// tmpFileWithContent provide tmp file +func tmpFileWithContent(content string, t *testing.T) string { + tmpFile, err := ioutil.TempFile("", "envfile-test") + if err != nil { + t.Fatal(err) + } + defer tmpFile.Close() + + tmpFile.WriteString(content) + return tmpFile.Name() +} + +// Test ParseEnvFile for a file with a few well formatted lines +func TestParseEnvFileGoodFile(t *testing.T) { + content := `foo=bar + baz=quux +# comment + +_foobar=foobaz +with.dots=working +and_underscore=working too +地址1=杭州 + ADDRESS=杭州 +地址2=Hangzhou +` + // Adding a newline + a line with pure whitespace. + // This is being done like this instead of the block above + // because it's common for editors to trim trailing whitespace + // from lines, which becomes annoying since that's the + // exact thing we need to test. + content += "\n \t " + tmpFile := tmpFileWithContent(content, t) + defer os.Remove(tmpFile) + + lines, err := parseEnvFile(tmpFile) + if err != nil { + t.Fatal(err) + } + + expectedLines := []string{ + "foo=bar", + "baz=quux", + "_foobar=foobaz", + "with.dots=working", + "and_underscore=working too", + "地址1=杭州", + "ADDRESS=杭州", + "地址2=Hangzhou", + } + + if !reflect.DeepEqual(lines, expectedLines) { + t.Fatal("lines not equal to expected_lines") + } +} + +// Test ParseEnvFile for an empty file +func TestParseEnvFileEmptyFile(t *testing.T) { + tmpFile := tmpFileWithContent("", t) + defer os.Remove(tmpFile) + + lines, err := parseEnvFile(tmpFile) + if err != nil { + t.Fatal(err) + } + + if len(lines) != 0 { + t.Fatal("lines not empty; expected empty") + } +} + +// Test ParseEnvFile for a non existent file +func TestParseEnvFileNonExistentFile(t *testing.T) { + _, err := parseEnvFile("foo_bar_baz") + if err == nil { + t.Fatal("ParseEnvFile succeeded; expected failure") + } + if _, ok := err.(*os.PathError); !ok { + t.Fatalf("Expected a PathError, got [%v]", err) + } +} + +// Test ParseEnvFile for a badly formatted file +func TestParseEnvFileBadlyFormattedFile(t *testing.T) { + content := `foo=bar + f =quux +` + tmpFile := tmpFileWithContent(content, t) + defer os.Remove(tmpFile) + + _, err := parseEnvFile(tmpFile) + if err == nil { + t.Fatalf("Expected an ErrBadEnvVariable, got nothing") + } + if _, ok := err.(ErrBadEnvVariable); !ok { + t.Fatalf("Expected an ErrBadEnvVariable, got [%v]", err) + } + expectedMessage := "poorly formatted environment: variable 'f ' has white spaces" + if err.Error() != expectedMessage { + t.Fatalf("Expected [%v], got [%v]", expectedMessage, err.Error()) + } +} + +// Test ParseEnvFile for a file with a line exceeding bufio.MaxScanTokenSize +func TestParseEnvFileLineTooLongFile(t *testing.T) { + content := strings.Repeat("a", bufio.MaxScanTokenSize+42) + content = fmt.Sprint("foo=", content) + + tmpFile := tmpFileWithContent(content, t) + defer os.Remove(tmpFile) + + _, err := parseEnvFile(tmpFile) + if err == nil { + t.Fatal("ParseEnvFile succeeded; expected failure") + } +} + +// ParseEnvFile with a random file, pass through +func TestParseEnvFileRandomFile(t *testing.T) { + content := `first line +another invalid line` + tmpFile := tmpFileWithContent(content, t) + defer os.Remove(tmpFile) + + _, err := parseEnvFile(tmpFile) + + if err == nil { + t.Fatalf("Expected an ErrBadEnvVariable, got nothing") + } + if _, ok := err.(ErrBadEnvVariable); !ok { + t.Fatalf("Expected an ErrBadEnvvariable, got [%v]", err) + } + expectedMessage := "poorly formatted environment: variable 'first line' has white spaces" + if err.Error() != expectedMessage { + t.Fatalf("Expected [%v], got [%v]", expectedMessage, err.Error()) + } +} diff --git a/cli/run.go b/cli/run.go index 65955c57c..1e3d3a29e 100644 --- a/cli/run.go +++ b/cli/run.go @@ -66,6 +66,11 @@ func (rc *RunCommand) runRun(args []string) error { return fmt.Errorf("failed to run container: %v", err) } + //collect all the environment variables for the container + config.Env, err = readKVStrings(rc.envfile, rc.env) + if err != nil { + return nil + } config.Image = args[0] if len(args) > 1 { config.Cmd = args[1:] diff --git a/test/cli_create_test.go b/test/cli_create_test.go index eb794cd6f..d7cb9eaaa 100644 --- a/test/cli_create_test.go +++ b/test/cli_create_test.go @@ -3,6 +3,7 @@ package main import ( "encoding/json" "fmt" + "os" "reflect" "strings" @@ -363,6 +364,93 @@ func (suite *PouchCreateSuite) TestCreateWithEnv(c *check.C) { } } +// TestCreateWithEnvfile tests creating container with envfile +func (suite *PouchCreateSuite) TestCreateWithEnvfile(c *check.C) { + name := "TestCreateWithEnvfile" + + content := "TEST1=value1\n地址1=杭州\n地址2=Hangzhou\naddress=杭州" + validfile, err := util.TmpFileWithContent(content) + if err != nil { + c.Fatal(err) + } + defer os.Remove(validfile) + + content = "TEST2=" + invalidfile, err2 := util.TmpFileWithContent(content) + if err2 != nil { + c.Fatal(err2) + } + defer os.Remove(invalidfile) + + env1 := "TEST1=value1" + env2 := "TEST2=" + env3 := "TEST3=value3" + env4 := "地址1=杭州" + env5 := "地址2=Hangzhou" + env6 := "address=杭州" + res := command.PouchRun("create", + "--name", name, + "--env-file", validfile, + "--env-file", invalidfile, + "-e", env3, + busyboxImage, + "top") + defer DelContainerForceMultyTime(c, name) + + res.Assert(c, icmd.Success) + + envs, err := inspectFilter(name, ".Config.Env") + c.Assert(err, check.IsNil) + + // check if these envs are in inspect result of container. + if !strings.Contains(envs, env1) { + c.Fatalf("container env in inspect result should have %s in %s while no\n", env1, envs) + } + if !strings.Contains(envs, env2) { + c.Fatalf("container env in inspect result should have %s in %s while no\n", env2, envs) + } + if !strings.Contains(envs, env3) { + c.Fatalf("container env in inspect result should have %s in %s while no\n", env3, envs) + } + if !strings.Contains(envs, env4) { + c.Fatalf("container env in inspect result should have %s in %s while no\n", env4, envs) + } + if !strings.Contains(envs, env5) { + c.Fatalf("container env in inspect result should have %s in %s while no\n", env5, envs) + } + if !strings.Contains(envs, env6) { + c.Fatalf("container env in inspect result should have %s in %s while no\n", env6, envs) + } + + // check if these envs are in the real container envs + res = command.PouchRun("start", name) + res.Assert(c, icmd.Success) + + ret := command.PouchRun("exec", name, "env") + ret.Assert(c, icmd.Success) + envs = ret.Stdout() + + if !strings.Contains(envs, env1) { + c.Fatalf("container's runtime env should have %s in %s while no\n", env1, envs) + } + if !strings.Contains(envs, env2) { + c.Fatalf("container's runtime env should have %s in %s while no\n", env2, envs) + } + if !strings.Contains(envs, env3) { + c.Fatalf("container's runtime env should have %s in %s while no\n", env3, envs) + } + if !strings.Contains(envs, env4) { + c.Fatalf("container's runtime env should have %s in %s while no\n", env4, envs) + } + if !strings.Contains(envs, env5) { + c.Fatalf("container's runtime env should have %s in %s while no\n", env5, envs) + } + if !strings.Contains(envs, env6) { + c.Fatalf("container's runtime env should have %s in %s while no\n", env6, envs) + } +} + +// TestCreateWithWorkDir tests creating container with a workdir works. // TestCreateWithWorkDir tests creating container with a workdir works. func (suite *PouchCreateSuite) TestCreateWithWorkDir(c *check.C) { name := "TestCreateWithWorkDir" diff --git a/test/cli_run_test.go b/test/cli_run_test.go index 18a5f7032..d2511f82b 100644 --- a/test/cli_run_test.go +++ b/test/cli_run_test.go @@ -3,6 +3,7 @@ package main import ( "encoding/json" "fmt" + "os" "strings" "time" @@ -417,6 +418,67 @@ func (suite *PouchRunSuite) TestRunWithEnv(c *check.C) { c.Assert(strings.TrimSpace(res.Stdout()), check.Equals, "a,b,c-b1") } +func (suite *PouchRunSuite) TestRunWithEnvfile(c *check.C) { + // Test one + content := "TEST1=value1" + validfile, err := util.TmpFileWithContent(content) + if err != nil { + c.Fatal(err) + } + defer os.Remove(validfile) + + res := command.PouchRun("run", "--rm", + "--env-file", validfile, + "--env", "TEST2=value2", + busyboxImage, + "sh", "-c", "echo ${TEST1}-${TEST2}", + ) + res.Assert(c, icmd.Success) + c.Assert(strings.TrimSpace(res.Stdout()), check.Equals, "value1-value2") + + // Test two + content = "TEST2=" + invalidfile, err2 := util.TmpFileWithContent(content) + if err2 != nil { + c.Fatal(err2) + } + defer os.Remove(invalidfile) + + ret := command.PouchRun("run", "--rm", + "-e", "TEST1=value1", + "--env-file", invalidfile, + busyboxImage, + "sh", "-c", "echo ${TEST1}-${TEST2}", + ) + ret.Assert(c, icmd.Success) + c.Assert(strings.TrimSpace(ret.Stdout()), check.Equals, "value1-") + + // Test three + content = "TEST1=value2" + validfile2, err3 := util.TmpFileWithContent(content) + if err3 != nil { + c.Fatal(err3) + } + defer os.Remove(validfile2) + + content = "TEST2" + invalidfile2, err4 := util.TmpFileWithContent(content) + if err4 != nil { + c.Fatal(err4) + } + defer os.Remove(invalidfile2) + + ret = command.PouchRun("run", "--rm", + "--env-file", validfile, + "--env-file", validfile2, + "--env-file", invalidfile2, + busyboxImage, + "sh", "-c", "echo ${TEST1}-${TEST2}", + ) + ret.Assert(c, icmd.Success) + c.Assert(strings.TrimSpace(ret.Stdout()), check.Equals, "value2-") +} + // TestRunWithTty tests running container with -tty flag and attach stdin in a non-tty client. func (suite *PouchRunSuite) TestRunWithTty(c *check.C) { name := "TestRunWithTty" diff --git a/test/util/util.go b/test/util/util.go index 888315ed1..c32019c78 100644 --- a/test/util/util.go +++ b/test/util/util.go @@ -2,6 +2,7 @@ package util import ( "fmt" + "io/ioutil" "strings" "time" @@ -95,3 +96,15 @@ func ParseCgroupFile(text string) map[string]string { } return cgroups } + +// TmpFileWithContent provide tmp file +func TmpFileWithContent(content string) (string, error) { + tmpFile, err := ioutil.TempFile("", "envfile-test") + if err != nil { + return "", err + } + defer tmpFile.Close() + + tmpFile.WriteString(content) + return tmpFile.Name(), nil +}