From ff0a6da1040bd788a4ebdc262e4333946252318c Mon Sep 17 00:00:00 2001 From: Ewa Czechowska Date: Thu, 13 Aug 2020 17:31:48 +0100 Subject: [PATCH] support exported bash functions #17 --- CHANGELOG.md | 48 ++++++++++++++ docker_compose_driver.go | 18 +++-- docker_compose_driver_test.go | 19 ++++-- docker_driver.go | 16 +++-- docker_driver_test.go | 69 ++++++++++++-------- environment_service.go | 69 +++++++++++++++++--- environment_service_test.go | 94 ++++++++++++++++++++------- test/support/common.py | 7 ++ test/test-files/test-bash-function.sh | 6 ++ test/test_docker.py | 63 ++++++++++++++++++ test/test_docker_compose.py | 30 ++++++++- version.go | 2 +- 12 files changed, 360 insertions(+), 81 deletions(-) create mode 100755 test/test-files/test-bash-function.sh diff --git a/CHANGELOG.md b/CHANGELOG.md index 063d775..e0cd03d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,51 @@ +### 0.9.0 (2020-Aug-13) + +* support exported bash functions https://github.com/kudulab/dojo/issues/17 + **TLDR**: + Dojo resulted in an error when any bash function + was exported, since 0.9.0 it will succeed. However, in order to preserve all the exported bash functions, you + need to run: + ``` + source /etc/dojo.d/variables/01-bash-functions.sh + ``` + **Long vesion**: + Whenever a bash function is exported in the following way: + ``` + my_bash_func() { + echo "hello" + } + export -f my_bash_func + ``` + Bash creates an environment variable, in this case: + ``` + BASH_FUNC_my_bash_func%%=() { echo "hello" + } + ``` + So far, when there was any bash function exported, Dojo resulted in an error like: + ``` + 13-08-2020 19:53:43 Dojo entrypoint info: Sourcing: /etc/dojo.d/variables/00-multiline-vars.sh + /etc/dojo.d/variables/00-multiline-vars.sh: line 1: export: `DOJO_BASH_FUNC_my_bash_func%%=()': not a valid identifier + /etc/dojo.d/variables/00-multiline-vars.sh: line 1: export: `{': not a valid identifier + /etc/dojo.d/variables/00-multiline-vars.sh: line 1: export: `"hello"': not a valid identifier + /etc/dojo.d/variables/00-multiline-vars.sh: line 1: export: `}': not a valid identifier + ``` + This was made visible when using Bats-core v1.2.1, because they exported + a bash function [in this PR](https://github.com/bats-core/bats-core/pull/312/files). + Dojo interpreted this bash environment variable as a multiline variable and + attempted to serialize it with base64. It was fine, but deserialization lead to + the error above. + Dojo 0.9.0 treats all the environment variables which names start with "BASH_FUNC_" + prefix and which values start with "()" as exported bash functions and serializes them into + /etc/dojo.d/variables/01-bash-functions.sh file. This file is sourced at + a container start. However, the bash functions are not preserved later, + because `sudo -E` (used in Dojo entrypoint) does not preserve exported + bash functions after ShellShock. See [this](https://unix.stackexchange.com/questions/549140/why-doesnt-sudo-e-preserve-the-function-environment-variables-exported-by-ex) and [this](https://unix.stackexchange.com/a/233097). + It was decided that Dojo 0.9.0 should follow the sudo's practice and not try to + add a work around leading to bash functions being preserved. But, it was made easy for the end user to preserve them with: + ``` + source /etc/dojo.d/variables/01-bash-functions.sh + ``` + ### 0.8.0 (2020-Jan-01) * Docker-composer driver: enable printing logs of non default docker containers either to console or to file. diff --git a/docker_compose_driver.go b/docker_compose_driver.go index a07b943..7c52697 100644 --- a/docker_compose_driver.go +++ b/docker_compose_driver.go @@ -73,13 +73,14 @@ func (dc DockerComposeDriver) getDCGeneratedFilePath(dcfilePath string) string { } func (dc DockerComposeDriver) generateDCFileContentsWithEnv(expContainers []string, config Config, envFile string, - envFileMultiLine string) string { + envFileMultiLine string, envFileBashFunctions string) string { contents := fmt.Sprintf( ` volumes: - %s:%s:ro - %s:%s - %s:/etc/dojo.d/variables/00-multiline-vars.sh -`, config.IdentityDirOuter, "/dojo/identity", config.WorkDirOuter, config.WorkDirInner, envFileMultiLine) + - %s:/etc/dojo.d/variables/01-bash-functions.sh +`, config.IdentityDirOuter, "/dojo/identity", config.WorkDirOuter, config.WorkDirInner, envFileMultiLine, envFileBashFunctions) if os.Getenv("DISPLAY") != "" { // DISPLAY is set, enable running in graphical mode (opinionated) contents += " - /tmp/.X11-unix:/tmp/.X11-unix\n" @@ -100,7 +101,8 @@ func (dc DockerComposeDriver) generateDCFileContentsWithEnv(expContainers []stri - %s volumes: - %s:/etc/dojo.d/variables/00-multiline-vars.sh -`, name, envFile, envFileMultiLine) + - %s:/etc/dojo.d/variables/01-bash-functions.sh +`, name, envFile, envFileMultiLine, envFileBashFunctions) } } return contents @@ -370,14 +372,15 @@ func checkIfAnyContainerFailed(nonDefaultContainerInfos []*ContainerInfo, defaul func (dc DockerComposeDriver) HandleRun(mergedConfig Config, runID string, envService EnvServiceInterface) int { warnGeneral(dc.FileService, mergedConfig, envService, dc.Logger) - envFile, envFileMultiLine := getEnvFilePaths(runID, mergedConfig.Test) - saveEnvToFile(dc.FileService, envFile, envFileMultiLine, mergedConfig.BlacklistVariables, envService.GetVariables()) + envFile, envFileMultiLine, envFileBashFunctions := getEnvFilePaths(runID, mergedConfig.Test) + saveEnvToFile(dc.FileService, envFile, envFileMultiLine, envFileBashFunctions, + mergedConfig.BlacklistVariables, envService.GetVariables()) dojoDCGeneratedFile, err := dc.handleDCFiles(mergedConfig) if err != nil { return 1 } expContainers := dc.getExpectedContainers(mergedConfig, runID) - additionalContents := dc.generateDCFileContentsWithEnv(expContainers, mergedConfig, envFile, envFileMultiLine) + additionalContents := dc.generateDCFileContentsWithEnv(expContainers, mergedConfig, envFile, envFileMultiLine, envFileBashFunctions) dc.FileService.AppendContents(dojoDCGeneratedFile, additionalContents, "debug") cmd := dc.ConstructDockerComposeCommandRun(mergedConfig, runID) @@ -435,9 +438,10 @@ func (dc DockerComposeDriver) HandleRun(mergedConfig Config, runID string, envSe func (dc DockerComposeDriver) CleanAfterRun(mergedConfig Config, runID string) int { if mergedConfig.RemoveContainers == "true" { dc.Logger.Log("debug", "Cleaning, because RemoveContainers is set to true") - envFile, envFileMultiLine := getEnvFilePaths(runID, mergedConfig.Test) + envFile, envFileMultiLine, envFilePathBashFunctions := getEnvFilePaths(runID, mergedConfig.Test) defer dc.FileService.RemoveGeneratedFile(mergedConfig.RemoveContainers, envFile) defer dc.FileService.RemoveGeneratedFile(mergedConfig.RemoveContainers, envFileMultiLine) + defer dc.FileService.RemoveGeneratedFile(mergedConfig.RemoveContainers, envFilePathBashFunctions) dojoDCGeneratedFile := dc.getDCGeneratedFilePath(mergedConfig.DockerComposeFile) defer dc.FileService.RemoveGeneratedFile(mergedConfig.RemoveContainers, dojoDCGeneratedFile) diff --git a/docker_compose_driver_test.go b/docker_compose_driver_test.go index 18a9b7b..a3b4762 100644 --- a/docker_compose_driver_test.go +++ b/docker_compose_driver_test.go @@ -97,13 +97,14 @@ func Test_generateDCFileContentsWithEnv(t *testing.T){ setTestEnv() } contents := dc.generateDCFileContentsWithEnv(expectedServices, config, - "/tmp/env-file.txt", "/tmp/env-file-multiline.txt") + "/tmp/env-file.txt", "/tmp/env-file-multiline.txt", "/tmp/env-file-bash-functions.txt") if v.displaySet { assert.Equal(t, ` volumes: - /tmp/myidentity:/dojo/identity:ro - /tmp/bla:/dojo/work - /tmp/env-file-multiline.txt:/etc/dojo.d/variables/00-multiline-vars.sh + - /tmp/env-file-bash-functions.txt:/etc/dojo.d/variables/01-bash-functions.sh - /tmp/.X11-unix:/tmp/.X11-unix env_file: - /tmp/env-file.txt @@ -112,17 +113,20 @@ func Test_generateDCFileContentsWithEnv(t *testing.T){ - /tmp/env-file.txt volumes: - /tmp/env-file-multiline.txt:/etc/dojo.d/variables/00-multiline-vars.sh + - /tmp/env-file-bash-functions.txt:/etc/dojo.d/variables/01-bash-functions.sh def: env_file: - /tmp/env-file.txt volumes: - /tmp/env-file-multiline.txt:/etc/dojo.d/variables/00-multiline-vars.sh + - /tmp/env-file-bash-functions.txt:/etc/dojo.d/variables/01-bash-functions.sh `, contents) } else { assert.Equal(t, ` volumes: - /tmp/myidentity:/dojo/identity:ro - /tmp/bla:/dojo/work - /tmp/env-file-multiline.txt:/etc/dojo.d/variables/00-multiline-vars.sh + - /tmp/env-file-bash-functions.txt:/etc/dojo.d/variables/01-bash-functions.sh env_file: - /tmp/env-file.txt abc: @@ -130,11 +134,13 @@ func Test_generateDCFileContentsWithEnv(t *testing.T){ - /tmp/env-file.txt volumes: - /tmp/env-file-multiline.txt:/etc/dojo.d/variables/00-multiline-vars.sh + - /tmp/env-file-bash-functions.txt:/etc/dojo.d/variables/01-bash-functions.sh def: env_file: - /tmp/env-file.txt volumes: - /tmp/env-file-multiline.txt:/etc/dojo.d/variables/00-multiline-vars.sh + - /tmp/env-file-bash-functions.txt:/etc/dojo.d/variables/01-bash-functions.sh `, contents) } } @@ -317,17 +323,18 @@ two three`) exitstatus := driver.HandleRun(config, runID, envService) assert.Equal(t, 0, exitstatus) - assert.Equal(t, 3, len(fs.FilesWrittenTo)) + assert.Equal(t, 4, len(fs.FilesWrittenTo)) assert.Equal(t, "ABC=123\n", fs.FilesWrittenTo["/tmp/dojo-environment-1234"]) assert.Equal(t, "export MULTI_LINE=$(echo b25lCnR3bwp0aHJlZQ== | base64 -d)\n", fs.FilesWrittenTo["/tmp/dojo-environment-multiline-1234"]) assert.Contains(t, fs.FilesWrittenTo["docker-compose.yml.dojo"], "version: '2.2'") exitstatus = driver.CleanAfterRun(config, runID) assert.Equal(t, 0, exitstatus) - assert.Equal(t, 5, len(fs.FilesRemovals)) - assert.Equal(t, "/tmp/dojo-environment-1234", fs.FilesRemovals[0]) - assert.Equal(t, "/tmp/dojo-environment-multiline-1234", fs.FilesRemovals[1]) - assert.Equal(t, "docker-compose.yml.dojo", fs.FilesRemovals[2]) + assert.Equal(t, 7, len(fs.FilesRemovals)) + assert.Equal(t, "/tmp/dojo-environment-1234", fs.FilesRemovals[1]) + assert.Equal(t, "/tmp/dojo-environment-multiline-1234", fs.FilesRemovals[2]) + assert.Equal(t, "/tmp/dojo-environment-bash-functions-1234", fs.FilesRemovals[0]) + assert.Equal(t, "docker-compose.yml.dojo", fs.FilesRemovals[3]) assert.False(t, fileExists("/tmp/dojo-environment-1234")) } diff --git a/docker_driver.go b/docker_driver.go index 8334e4b..9786072 100644 --- a/docker_driver.go +++ b/docker_driver.go @@ -29,7 +29,7 @@ func NewDockerDriver(shellService ShellServiceInterface, fs FileServiceInterface } } -func (d DockerDriver) ConstructDockerRunCmd(config Config, envFilePath string, envFileMultiLine string, containerName string) string { +func (d DockerDriver) ConstructDockerRunCmd(config Config, envFilePath string, envFileMultiLine string, envFileBashFunctions string, containerName string) string { if envFilePath == "" { panic("envFilePath was not set") } @@ -40,8 +40,8 @@ func (d DockerDriver) ConstructDockerRunCmd(config Config, envFilePath string, e if config.RemoveContainers == "true" { cmd += " --rm" } - cmd += fmt.Sprintf(" -v %s:%s -v %s:/dojo/identity:ro -v %s:/etc/dojo.d/variables/00-multiline-vars.sh", - config.WorkDirOuter, config.WorkDirInner, config.IdentityDirOuter, envFileMultiLine) + cmd += fmt.Sprintf(" -v %s:%s -v %s:/dojo/identity:ro -v %s:/etc/dojo.d/variables/00-multiline-vars.sh -v %s:/etc/dojo.d/variables/01-bash-functions.sh", + config.WorkDirOuter, config.WorkDirInner, config.IdentityDirOuter, envFileMultiLine, envFileBashFunctions) cmd += fmt.Sprintf(" --env-file=%s", envFilePath) if os.Getenv("DISPLAY") != "" { // DISPLAY is set, enable running in graphical mode (opinionated) @@ -68,10 +68,11 @@ func (d DockerDriver) ConstructDockerRunCmd(config Config, envFilePath string, e func (d DockerDriver) HandleRun(mergedConfig Config, runID string, envService EnvServiceInterface) int { warnGeneral(d.FileService, mergedConfig, envService, d.Logger) - envFile, envFileMultiLine := getEnvFilePaths(runID, mergedConfig.Test) - saveEnvToFile(d.FileService, envFile, envFileMultiLine, mergedConfig.BlacklistVariables, envService.GetVariables()) + envFile, envFileMultiLine, envFileBashFunctions := getEnvFilePaths(runID, mergedConfig.Test) + saveEnvToFile(d.FileService, envFile, envFileMultiLine, envFileBashFunctions, + mergedConfig.BlacklistVariables, envService.GetVariables()) - cmd := d.ConstructDockerRunCmd(mergedConfig, envFile, envFileMultiLine, runID) + cmd := d.ConstructDockerRunCmd(mergedConfig, envFile, envFileMultiLine, envFileBashFunctions, runID) d.Logger.Log("info", green(fmt.Sprintf("docker command will be:\n %v", cmd))) if mergedConfig.RemoveContainers != "true" { @@ -149,9 +150,10 @@ func (d DockerDriver) HandleMultipleSignal(mergedConfig Config, runID string) in func (d DockerDriver) CleanAfterRun(mergedConfig Config, runID string) int { if mergedConfig.RemoveContainers == "true" { d.Logger.Log("debug", "Cleaning, because RemoveContainers is set to true") - envFile, envFileMultiLine := getEnvFilePaths(runID, mergedConfig.Test) + envFile, envFileMultiLine, envFilePathBashFunctions := getEnvFilePaths(runID, mergedConfig.Test) d.FileService.RemoveGeneratedFile(mergedConfig.RemoveContainers, envFile) d.FileService.RemoveGeneratedFile(mergedConfig.RemoveContainers, envFileMultiLine) + d.FileService.RemoveGeneratedFile(mergedConfig.RemoveContainers, envFilePathBashFunctions) // no need to remove the container, if it was started with "docker run --rm", it was already removed diff --git a/docker_driver_test.go b/docker_driver_test.go index 2b7e3a4..98d9e42 100644 --- a/docker_driver_test.go +++ b/docker_driver_test.go @@ -18,26 +18,28 @@ func TestDockerDriver_ConstructDockerRunCmd_Interactive(t *testing.T){ userInteractiveConfig string expOutput string } + interactiveOutput := "docker run --rm -v /tmp/bla:/dojo/work -v /tmp/myidentity:/dojo/identity:ro " + + "-v /tmp/some-env-file-multiline:/etc/dojo.d/variables/00-multiline-vars.sh " + + "-v /tmp/some-env-file-bash-functions:/etc/dojo.d/variables/01-bash-functions.sh " + + "--env-file=/tmp/some-env-file -ti --name=name1 img:1.2.3" + notInteractiveOutput := "docker run --rm -v /tmp/bla:/dojo/work -v /tmp/myidentity:/dojo/identity:ro " + + "-v /tmp/some-env-file-multiline:/etc/dojo.d/variables/00-multiline-vars.sh " + + "-v /tmp/some-env-file-bash-functions:/etc/dojo.d/variables/01-bash-functions.sh " + + "--env-file=/tmp/some-env-file --name=name1 img:1.2.3" mytests := []mytestStruct{ mytestStruct{ shellInteractive: true, userInteractiveConfig: "true", - expOutput: "docker run --rm -v /tmp/bla:/dojo/work -v /tmp/myidentity:/dojo/identity:ro " + - "-v /tmp/some-env-file-multiline:/etc/dojo.d/variables/00-multiline-vars.sh --env-file=/tmp/some-env-file -ti --name=name1 img:1.2.3"}, + expOutput: interactiveOutput}, mytestStruct{ shellInteractive: true, userInteractiveConfig: "false", - expOutput: "docker run --rm -v /tmp/bla:/dojo/work -v /tmp/myidentity:/dojo/identity:ro " + - "-v /tmp/some-env-file-multiline:/etc/dojo.d/variables/00-multiline-vars.sh --env-file=/tmp/some-env-file --name=name1 img:1.2.3"}, + expOutput: notInteractiveOutput}, mytestStruct{ shellInteractive: true, userInteractiveConfig: "", - expOutput: "docker run --rm -v /tmp/bla:/dojo/work -v /tmp/myidentity:/dojo/identity:ro " + - "-v /tmp/some-env-file-multiline:/etc/dojo.d/variables/00-multiline-vars.sh --env-file=/tmp/some-env-file -ti --name=name1 img:1.2.3"}, + expOutput: interactiveOutput}, mytestStruct{ shellInteractive: false, userInteractiveConfig: "true", - expOutput: "docker run --rm -v /tmp/bla:/dojo/work -v /tmp/myidentity:/dojo/identity:ro " + - "-v /tmp/some-env-file-multiline:/etc/dojo.d/variables/00-multiline-vars.sh --env-file=/tmp/some-env-file -ti --name=name1 img:1.2.3"}, + expOutput: interactiveOutput}, mytestStruct{ shellInteractive: false, userInteractiveConfig: "false", - expOutput: "docker run --rm -v /tmp/bla:/dojo/work -v /tmp/myidentity:/dojo/identity:ro " + - "-v /tmp/some-env-file-multiline:/etc/dojo.d/variables/00-multiline-vars.sh --env-file=/tmp/some-env-file --name=name1 img:1.2.3"}, + expOutput: notInteractiveOutput}, mytestStruct{ shellInteractive: false, userInteractiveConfig: "", - expOutput: "docker run --rm -v /tmp/bla:/dojo/work -v /tmp/myidentity:/dojo/identity:ro " + - "-v /tmp/some-env-file-multiline:/etc/dojo.d/variables/00-multiline-vars.sh --env-file=/tmp/some-env-file --name=name1 img:1.2.3"}, + expOutput: notInteractiveOutput}, } setTestEnv() logger := NewLogger("debug") @@ -51,7 +53,9 @@ func TestDockerDriver_ConstructDockerRunCmd_Interactive(t *testing.T){ ss = NewMockedShellServiceNotInteractive(logger) } d := NewDockerDriver(ss, NewMockedFileService(logger), logger) - cmd := d.ConstructDockerRunCmd(config, "/tmp/some-env-file", "/tmp/some-env-file-multiline", "name1") + cmd := d.ConstructDockerRunCmd(config, "/tmp/some-env-file", + "/tmp/some-env-file-multiline", "/tmp/some-env-file-bash-functions", + "name1") assert.Equal(t, v.expOutput, cmd, fmt.Sprintf("shellInteractive: %v, userConfig: %v", v.shellInteractive, v.userInteractiveConfig)) } } @@ -61,16 +65,17 @@ func TestDockerDriver_ConstructDockerRunCmd_Command(t *testing.T){ userCommandConfig string expOutput string } + baseOutput := "docker run --rm -v /tmp/bla:/dojo/work -v /tmp/myidentity:/dojo/identity:ro " + + "-v /tmp/some-env-file-multiline:/etc/dojo.d/variables/00-multiline-vars.sh " + + "-v /tmp/some-env-file-bash-functions:/etc/dojo.d/variables/01-bash-functions.sh " + + "--env-file=/tmp/some-env-file --name=name1 img:1.2.3" mytests := []mytestStruct{ mytestStruct{ userCommandConfig: "", - expOutput: "docker run --rm -v /tmp/bla:/dojo/work -v /tmp/myidentity:/dojo/identity:ro " + - "-v /tmp/some-env-file-multiline:/etc/dojo.d/variables/00-multiline-vars.sh --env-file=/tmp/some-env-file --name=name1 img:1.2.3"}, + expOutput: baseOutput}, mytestStruct{ userCommandConfig: "bash", - expOutput: "docker run --rm -v /tmp/bla:/dojo/work -v /tmp/myidentity:/dojo/identity:ro " + - "-v /tmp/some-env-file-multiline:/etc/dojo.d/variables/00-multiline-vars.sh --env-file=/tmp/some-env-file --name=name1 img:1.2.3 bash"}, + expOutput: baseOutput + " bash"}, mytestStruct{ userCommandConfig: "bash -c \"echo hello\"", - expOutput: "docker run --rm -v /tmp/bla:/dojo/work -v /tmp/myidentity:/dojo/identity:ro " + - "-v /tmp/some-env-file-multiline:/etc/dojo.d/variables/00-multiline-vars.sh --env-file=/tmp/some-env-file --name=name1 img:1.2.3 bash -c \"echo hello\""}, + expOutput: baseOutput + " bash -c \"echo hello\""}, } setTestEnv() for _,v := range mytests { @@ -78,7 +83,9 @@ func TestDockerDriver_ConstructDockerRunCmd_Command(t *testing.T){ config.RunCommand = v.userCommandConfig logger := NewLogger("debug") d := NewDockerDriver(NewMockedShellServiceNotInteractive(logger), NewMockedFileService(logger), logger) - cmd := d.ConstructDockerRunCmd(config, "/tmp/some-env-file", "/tmp/some-env-file-multiline","name1") + cmd := d.ConstructDockerRunCmd(config, "/tmp/some-env-file", + "/tmp/some-env-file-multiline", "/tmp/some-env-file-bash-functions", + "name1") assert.Equal(t, v.expOutput, cmd, fmt.Sprintf("userCommandConfig: %v", v.userCommandConfig)) } } @@ -91,10 +98,14 @@ func TestDockerDriver_ConstructDockerRunCmd_DisplayEnvVar(t *testing.T){ mytests := []mytestStruct{ mytestStruct{ displaySet: true, expOutput: "docker run --rm -v /tmp/bla:/dojo/work -v /tmp/myidentity:/dojo/identity:ro " + - "-v /tmp/some-env-file-multiline:/etc/dojo.d/variables/00-multiline-vars.sh --env-file=/tmp/some-env-file -v /tmp/.X11-unix:/tmp/.X11-unix --name=name1 img:1.2.3"}, + "-v /tmp/some-env-file-multiline:/etc/dojo.d/variables/00-multiline-vars.sh " + + "-v /tmp/some-env-file-bash-functions:/etc/dojo.d/variables/01-bash-functions.sh " + + "--env-file=/tmp/some-env-file -v /tmp/.X11-unix:/tmp/.X11-unix --name=name1 img:1.2.3"}, mytestStruct{ displaySet: false, expOutput: "docker run --rm -v /tmp/bla:/dojo/work -v /tmp/myidentity:/dojo/identity:ro " + - "-v /tmp/some-env-file-multiline:/etc/dojo.d/variables/00-multiline-vars.sh --env-file=/tmp/some-env-file --name=name1 img:1.2.3"}, + "-v /tmp/some-env-file-multiline:/etc/dojo.d/variables/00-multiline-vars.sh " + + "-v /tmp/some-env-file-bash-functions:/etc/dojo.d/variables/01-bash-functions.sh " + + "--env-file=/tmp/some-env-file --name=name1 img:1.2.3"}, } setTestEnv() for _,v := range mytests { @@ -106,7 +117,9 @@ func TestDockerDriver_ConstructDockerRunCmd_DisplayEnvVar(t *testing.T){ } logger := NewLogger("debug") d := NewDockerDriver(NewMockedShellServiceNotInteractive(logger), NewMockedFileService(logger), logger) - cmd := d.ConstructDockerRunCmd(config, "/tmp/some-env-file", "/tmp/some-env-file-multiline", "name1") + cmd := d.ConstructDockerRunCmd(config, "/tmp/some-env-file", + "/tmp/some-env-file-multiline", "/tmp/some-env-file-bash-functions", + "name1") assert.Equal(t, v.expOutput, cmd, fmt.Sprintf("displaySet: %v", v.displaySet)) } } @@ -125,12 +138,14 @@ three`) assert.Equal(t, 0, es) assert.False(t, fileExists("/tmp/dojo-environment-testrunid")) assert.False(t, fileExists("/tmp/dojo-environment-multiline-testrunid")) - assert.Equal(t, 2, len(fs.FilesWrittenTo)) + assert.False(t, fileExists("/tmp/dojo-environment-bash-functions-testrunid")) + assert.Equal(t, 3, len(fs.FilesWrittenTo)) assert.Equal(t, "ABC=123\n", fs.FilesWrittenTo["/tmp/dojo-environment-testrunid"]) assert.Equal(t, "export MULTI_LINE=$(echo b25lCnR3bwp0aHJlZQ== | base64 -d)\n", fs.FilesWrittenTo["/tmp/dojo-environment-multiline-testrunid"]) - assert.Equal(t, 2, len(fs.FilesRemovals)) - assert.Equal(t, "/tmp/dojo-environment-testrunid", fs.FilesRemovals[0]) - assert.Equal(t, "/tmp/dojo-environment-multiline-testrunid", fs.FilesRemovals[1]) + assert.Equal(t, 3, len(fs.FilesRemovals)) + assert.Equal(t, "/tmp/dojo-environment-testrunid", fs.FilesRemovals[1]) + assert.Equal(t, "/tmp/dojo-environment-multiline-testrunid", fs.FilesRemovals[2]) + assert.Equal(t, "/tmp/dojo-environment-bash-functions-testrunid", fs.FilesRemovals[0]) } func fileExists(filePath string) bool { diff --git a/environment_service.go b/environment_service.go index ca0e946..ce5777a 100644 --- a/environment_service.go +++ b/environment_service.go @@ -57,12 +57,34 @@ func (f EnvService) IsCurrentUserRoot() bool { // any variable starting with BASH will be blacklisted (and prefixed). // Variables with DOJO_ prefix cannot be blacklisted. func saveEnvToFile(fileService FileServiceInterface, envFilePath string, envFilePathMultiLine string, + envFilePathBashFunctions string, blacklistedVars string, currentVariables []string) { if fileService == nil { panic("fileService was nil") } filteredEnvVariables := filterBlacklistedVariables(blacklistedVars, currentVariables) + // First, we have to deal with such Bash environment variables, which were created by Bash + // when exporting a function. (A function may be one- or multi- line). Example Bash function is: + // my_bash_func() { + // echo "hello" + // } + // and it can be exported with: + // export -f my_bash_func + // In result, Bash creates the following environment variable: + // BASH_FUNC_my_bash_func%%=() { echo "hello" + // } + // We cannot serialize this variable in the same way as we serialize the other multiline variables, because we + // cannot assign any value to a bash variable ending in double percentage signs: + // $ export ABC%%=anything + // bash: export: `ABC%%=anything': not a valid identifier + // $ ABC%%=anything + // bash: ABC%%=anything: command not found + // Thus, we have to put such bash functions to a file and export the bash functions again. + fileService.RemoveFile(envFilePathBashFunctions, true) + bashFunctionsVariablesStr := bashFunctionsVariablesToString(filteredEnvVariables) + fileService.WriteToFile(envFilePathBashFunctions, bashFunctionsVariablesStr, "debug") + fileService.RemoveFile(envFilePath, true) singleLineVariablesStr := singleLineVariablesToString(filteredEnvVariables) fileService.WriteToFile(envFilePath, singleLineVariablesStr, "debug") @@ -76,6 +98,8 @@ type EnvironmentVariable struct { Key string Value string MultiLine bool + // returns true if it is a bash variable created by exporting a bash function + BashFunctionVariable bool } func (e EnvironmentVariable) String() string { return fmt.Sprintf("%s=%s", e.Key, e.Value) @@ -87,6 +111,10 @@ func (e EnvironmentVariable) encryptValue() string { return str } +func checkIfBashFunc(value string, key string) bool { + return strings.HasPrefix(value, "()") && strings.HasPrefix(key, "BASH_FUNC_") +} + // allVariables is a []string, where each element is of format: VariableName=VariableValue func filterBlacklistedVariables(blacklistedVarsNames string, allVariables []string) []EnvironmentVariable { blacklistedVarsArr := strings.Split(blacklistedVarsNames, ",") @@ -96,29 +124,48 @@ func filterBlacklistedVariables(blacklistedVarsNames string, allVariables []stri key := arr[0] value := arr[1] isMultiLine := (len(strings.Split(value, "\n")) > 1) + isBashFunc := checkIfBashFunc(value, key) var envVar EnvironmentVariable if key == "DISPLAY" { // this is highly opinionated - envVar = EnvironmentVariable{"DISPLAY", "unix:0.0", isMultiLine} + envVar = EnvironmentVariable{"DISPLAY", "unix:0.0", isMultiLine, isBashFunc} } else if existsVariableWithDOJOPrefix(key, allVariables) { // ignore this key, we will deal with DOJO_${key} continue - } else if strings.HasPrefix(key, "DOJO_") { - envVar = EnvironmentVariable{key, value, isMultiLine} + } else if strings.HasPrefix(key, "DOJO_") || isBashFunc { + // do not add DOJO_ prefix if such a prefix is already added or + // when this is an exported bash function + envVar = EnvironmentVariable{key, value, isMultiLine, isBashFunc} } else if isVariableBlacklisted(key, blacklistedVarsArr) { - envVar = EnvironmentVariable{fmt.Sprintf("DOJO_%s", key), value, isMultiLine} + envVar = EnvironmentVariable{fmt.Sprintf("DOJO_%s", key), value, isMultiLine, isBashFunc} } else { - envVar = EnvironmentVariable{key, value, isMultiLine} + envVar = EnvironmentVariable{key, value, isMultiLine, isBashFunc} } envVariables = append(envVariables, envVar) } return envVariables } +func bashFunctionsVariablesToString(variables []EnvironmentVariable) string { + bashFunctionVariablesStr := "#!/bin/bash\n" + for _, e := range variables { + if e.BashFunctionVariable { + arr := strings.SplitN(e.String(),"=", 2) + key := arr[0] + value := arr[1] + + bash_function_name := strings.TrimPrefix(key, "BASH_FUNC_") + bash_function_name = strings.TrimSuffix(bash_function_name, "%%") + bashFunctionVariablesStr += fmt.Sprintf("%s%s\nexport -f %s\n", bash_function_name, value, bash_function_name) + } + } + return bashFunctionVariablesStr +} + func singleLineVariablesToString(variables []EnvironmentVariable) string { singleLineVariablesStr := "" for _, e := range variables { - if !e.MultiLine { + if !e.MultiLine && !e.BashFunctionVariable { singleLineVariablesStr += e.String() singleLineVariablesStr += "\n" } @@ -132,20 +179,22 @@ func singleLineVariablesToString(variables []EnvironmentVariable) string { func multiLineVariablesToString(variables []EnvironmentVariable) string { multiLineVariablesStr := "" for _, e := range variables { - if e.MultiLine { + if e.MultiLine && !e.BashFunctionVariable { multiLineVariablesStr += fmt.Sprintf("export %s=$(echo %s | base64 -d)\n", e.Key, e.encryptValue()) } } return multiLineVariablesStr } -func getEnvFilePaths(runID string, test string) (string,string) { +func getEnvFilePaths(runID string, test string) (string,string,string) { if test == "true" { return fmt.Sprintf("/tmp/test-dojo-environment-%s", runID), - fmt.Sprintf("/tmp/test-dojo-environment-multiline-%s", runID) + fmt.Sprintf("/tmp/test-dojo-environment-multiline-%s", runID), + fmt.Sprintf("/tmp/test-dojo-environment-bash-functions-%s", runID) } else { return fmt.Sprintf("/tmp/dojo-environment-%s", runID), - fmt.Sprintf("/tmp/dojo-environment-multiline-%s", runID) + fmt.Sprintf("/tmp/dojo-environment-multiline-%s", runID), + fmt.Sprintf("/tmp/dojo-environment-bash-functions-%s", runID) } } diff --git a/environment_service_test.go b/environment_service_test.go index e2c1488..7f9a82e 100644 --- a/environment_service_test.go +++ b/environment_service_test.go @@ -45,27 +45,27 @@ func Test_filterBlacklistedVariables(t *testing.T) { "DOJO_WORK_INNER=/my/dir", `MULTI_LINE=one two`} filteredEnvVariables := filterBlacklistedVariables(blacklisted, allVariables) - assert.Contains(t, filteredEnvVariables, EnvironmentVariable{"DOJO_USER", "555", false}) - assert.Contains(t, filteredEnvVariables, EnvironmentVariable{"DOJO_BASH_123", "123", false}) - assert.Contains(t, filteredEnvVariables, EnvironmentVariable{"MYVAR", "999", false}) - assert.Contains(t, filteredEnvVariables, EnvironmentVariable{"DOJO_VAR1", "11", false}) - assert.Contains(t, filteredEnvVariables, EnvironmentVariable{"DOJO_USER1", "2", false}) - assert.Contains(t, filteredEnvVariables, EnvironmentVariable{"DISPLAY", "unix:0.0", false}) - assert.Contains(t, filteredEnvVariables, EnvironmentVariable{"DOJO_WORK_INNER", "/my/dir", false}) + assert.Contains(t, filteredEnvVariables, EnvironmentVariable{"DOJO_USER", "555", false, false}) + assert.Contains(t, filteredEnvVariables, EnvironmentVariable{"DOJO_BASH_123", "123", false, false}) + assert.Contains(t, filteredEnvVariables, EnvironmentVariable{"MYVAR", "999", false, false}) + assert.Contains(t, filteredEnvVariables, EnvironmentVariable{"DOJO_VAR1", "11", false, false}) + assert.Contains(t, filteredEnvVariables, EnvironmentVariable{"DOJO_USER1", "2", false, false}) + assert.Contains(t, filteredEnvVariables, EnvironmentVariable{"DISPLAY", "unix:0.0", false, false}) + assert.Contains(t, filteredEnvVariables, EnvironmentVariable{"DOJO_WORK_INNER", "/my/dir", false, false}) assert.Contains(t, filteredEnvVariables, EnvironmentVariable{"MULTI_LINE", `one -two`, true}) - assert.NotContains(t, filteredEnvVariables, EnvironmentVariable{"DOJO_USER", "dojo", false}) - assert.NotContains(t, filteredEnvVariables, EnvironmentVariable{"USER1", "1", false}) - assert.NotContains(t, filteredEnvVariables, EnvironmentVariable{"DISPLAY", "aaa", false}) +two`, true, false}) + assert.NotContains(t, filteredEnvVariables, EnvironmentVariable{"DOJO_USER", "dojo", false, false}) + assert.NotContains(t, filteredEnvVariables, EnvironmentVariable{"USER1", "1", false, false}) + assert.NotContains(t, filteredEnvVariables, EnvironmentVariable{"DISPLAY", "aaa", false, false}) } func Test_singleLineVariablesToString(t *testing.T) { allVariables := []EnvironmentVariable{ - EnvironmentVariable{"DOJO_USER", "555", false}, - EnvironmentVariable{"DOJO_WORK_INNER", "/my/dir", false}, + EnvironmentVariable{"DOJO_USER", "555", false, false}, + EnvironmentVariable{"DOJO_WORK_INNER", "/my/dir", false, false}, EnvironmentVariable{"MULTI_LINE", `one -two`, true}, - EnvironmentVariable{"MYVAR", "999", false}, +two`, true, false}, + EnvironmentVariable{"MYVAR", "999", false, false}, } genStr := singleLineVariablesToString(allVariables) assert.Contains(t, genStr, "DOJO_USER=555\n") @@ -74,28 +74,34 @@ two`, true}, assert.NotContains(t, genStr, "MULTI_LINE") } +func Test_checkIfBashFunc(t *testing.T) { + assert.True(t, checkIfBashFunc("() { echo hello }", "BASH_FUNC_abc_%%")) + assert.False(t, checkIfBashFunc("text", "BASH_FUNC_abc_%%")) + assert.False(t, checkIfBashFunc("() { echo hello }", "abc_%%")) +} + func Test_EnvVar_encryptValue(t *testing.T) { - e := EnvironmentVariable{"DOJO_USER", "555", false} + e := EnvironmentVariable{"DOJO_USER", "555", false, false} str := e.encryptValue() assert.Equal(t, "NTU1", str) } func Test_EnvVarToString(t *testing.T) { - e := EnvironmentVariable{"DOJO_USER", "555", false} + e := EnvironmentVariable{"DOJO_USER", "555", false, false} str := e.String() assert.Equal(t, "DOJO_USER=555", str) } func Test_multiLineVariablesToString(t *testing.T) { allVariables := []EnvironmentVariable{ - EnvironmentVariable{"DOJO_USER", "555", false}, - EnvironmentVariable{"DOJO_WORK_INNER", "/my/dir", false}, + EnvironmentVariable{"DOJO_USER", "555", false, false}, + EnvironmentVariable{"DOJO_WORK_INNER", "/my/dir", false, false}, EnvironmentVariable{"MULTI_LINE", `one -two`, true}, - EnvironmentVariable{"MYVAR", "999", false}, +two`, true, false}, + EnvironmentVariable{"MYVAR", "999", false, false}, EnvironmentVariable{"MULTI_LINE2", `one two -three`, true}, +three`, true, false}, } genStr := multiLineVariablesToString(allVariables) assert.NotContains(t, genStr, "DOJO_USER") @@ -105,6 +111,50 @@ three`, true}, assert.Contains(t, genStr, "export MULTI_LINE2=$(echo b25lCnR3bwp0aHJlZQ== | base64 -d)\n") } +func Test_bashFunctionsVariablesToString(t *testing.T) { + allVariables := []EnvironmentVariable{ + EnvironmentVariable{"DOJO_USER", "555", false, false}, + EnvironmentVariable{"MULTI_LINE", `one + two`, true, false}, + EnvironmentVariable{"BASH_FUNC_my_bash_func%%", `() { + echo "hello" + echo "hi" +}`, true, true}, + EnvironmentVariable{"BASH_FUNC_my_bash_func2%%", `() { + echo "abc" +}`, true, true}, + } + genStr := bashFunctionsVariablesToString(allVariables) + assert.NotContains(t, genStr, "DOJO_USER") + assert.NotContains(t, genStr, "MULTI_LINE") + assert.Contains(t, genStr, `my_bash_func() { + echo "hello" + echo "hi" +} +export -f my_bash_func +my_bash_func2() { + echo "abc" +} +export -f my_bash_func2`) +} + +//func Test_specialCharactersVariablesToString(t *testing.T) { +// allVariables := []EnvironmentVariable{ +// EnvironmentVariable{"DOJO_USER", "555", false, false}, +// EnvironmentVariable{"MULTI_LINE", `one +//two`, true, false}, +// EnvironmentVariable{"MYVAR", "999", false, false}, +// EnvironmentVariable{"BASH_FUNC_bats_readlinkf%%", `() { readlink -f "$1" +//second line +//}`, true, false}, +// } +// genStr := multiLineVariablesToString(allVariables) +// assert.NotContains(t, genStr, "DOJO_USER") +// assert.NotContains(t, genStr, "MYVAR") +// assert.Contains(t, genStr, "export MULTI_LINE=$(echo b25lCnR3bw== | base64 -d)\n") +// assert.Contains(t, genStr, "export BASH_FUNC_bats_readlinkf%%=$(echo KCkgeyAgcmVhZGxpbmsgLWYgIiQxIgpzZWNvbmQgbGluZQp9 | base64 -d)\n") +//} + func Test_addVariable(t *testing.T) { envService := NewEnvService() diff --git a/test/support/common.py b/test/support/common.py index f369ac2..ed0541f 100644 --- a/test/support/common.py +++ b/test/support/common.py @@ -29,6 +29,13 @@ def run_dojo(args, env=None): return run_command(dojo_exe, args, env) +def run_dojo_and_set_bash_func(args, env=None): + dojo_exe = os.path.join(test_dir, '..', '..', 'bin', 'dojo') + fullArgs = 'source ' + test_dir + '/../test-files/test-bash-function.sh && ' + dojo_exe + ' ' + ' '.join(args) + proc = subprocess.Popen(['bash','-c',fullArgs], stdout=subprocess.PIPE, stderr=subprocess.PIPE) + return proc + + def assert_no_warnings_or_errors(text): assert not 'warn' in text assert not 'error' in text diff --git a/test/test-files/test-bash-function.sh b/test/test-files/test-bash-function.sh new file mode 100755 index 0000000..2f19e1a --- /dev/null +++ b/test/test-files/test-bash-function.sh @@ -0,0 +1,6 @@ +#!/bin/bash + +my_bash_func() { + echo "hello" +} +export -f my_bash_func diff --git a/test/test_docker.py b/test/test_docker.py index f2ef3a5..a0b939e 100644 --- a/test/test_docker.py +++ b/test/test_docker.py @@ -108,6 +108,33 @@ def test_docker_preserves_env_vars(): assert_no_warnings_or_errors(result.stdout) assert result.returncode == 0 +# Bash experiments: +# $ export MULTILINE1="first line\nsecond line" +# $ export MULTILINE2="first line +# > second line" +# $ read -d '' MULTILINE3 < first line +# > second line +# > EOF +# $ export MULTILINE3 +# +# $ echo $MULTILINE1 +# first line\nsecond line +# $ echo $MULTILINE2 +# first line second line +# $ echo $MULTILINE3 +# first line second line +# $ echo "$MULTILINE1" +# first line\nsecond line +# $ echo "$MULTILINE2" +# first line +# second line +# $ echo "$MULTILINE3" +# first line +# second line +# +# In Dojo, only the MULTILINE2 and MULTILINE3 will be put among multiline variables. +# MULTILINE1 will be treated as oneline variable. def test_docker_preserves_multiline_env_vars(): clean_up_docker_container() @@ -115,6 +142,9 @@ def test_docker_preserves_multiline_env_vars(): envs['ABC'] = """first line second line""" result = run_dojo( + # We need to source the file: /etc/dojo.d/variables/01-bash-functions.sh + # explicitly, because the alpine docker image is not a Dojo image, i.e. + # it does not have the Dojo entrypoint.sh. ['--debug=true', '--test=true', '--image=alpine:3.8', 'sh', '-c', '"source /etc/dojo.d/variables/00-multiline-vars.sh && env | grep -A 1 ABC"'], env=envs) assert 'Dojo version' in result.stderr @@ -127,6 +157,39 @@ def test_docker_preserves_multiline_env_vars(): second line""" in result.stdout +def test_docker_preserves_bash_functions_from_env_vars(): + clean_up_docker_container() + envs = dict(os.environ) + # the following does not influence the dojo process + # envs['BASH_FUNC_my_bash_func%%'] = """()) { echo "hello" +# }""" + proc = run_dojo_and_set_bash_func( + # We need to source the file: /etc/dojo.d/variables/01-bash-functions.sh + # explicitly, because the alpine docker image is not a Dojo image, i.e. + # it does not have the Dojo entrypoint.sh. Even if it had, + # we'd still have to source the file explicitly, because + # sudo does not preserve bash functions. + # Dojo entrypoint sources this file too, but then it runs sudo. + # https://unix.stackexchange.com/questions/549140/why-doesnt-sudo-e-preserve-the-function-environment-variables-exported-by-ex + # https://unix.stackexchange.com/a/233097 + ['--debug=true', '--test=true', '--image=alpine:3.8', 'sh', '-c', '"apk add -U bash && bash -c \'source /etc/dojo.d/variables/01-bash-functions.sh && my_bash_func\'"'], + env=envs) + stdout_value_bytes, stderr_value_bytes = proc.communicate() + stdout = stdout_value_bytes.decode("utf-8") + stderr = stderr_value_bytes.decode("utf-8") + assert 'Dojo version' in stderr + # print(stdout) + # print(stderr) + assert 'Written file /tmp/test-dojo-environment-bash-functions-testdojorunid, contents:' in stderr + assert 'my_bash_func() { echo "hello"' in stderr + assert '/etc/dojo.d/variables/01-bash-functions.sh' in stderr + assert_no_warnings_or_errors(stderr) + assert_no_warnings_or_errors(stdout) + # the bash function was invoked + assert 'hello' in stdout + assert 'Exit status from run command: 0' in stderr + + def test_docker_when_custom_relative_directory(): clean_up_docker_container() result = run_dojo(['-c', 'test/test-files/Dojofile', '--debug=true', '--test=true', '--image=alpine:3.8', 'whoami']) diff --git a/test/test_docker_compose.py b/test/test_docker_compose.py index 27b7b5e..329d189 100644 --- a/test/test_docker_compose.py +++ b/test/test_docker_compose.py @@ -154,6 +154,34 @@ def test_docker_compose_run_preserves_multiline_env_vars(): test_dc_network_is_removed() +# see also: test_docker_preserves_bash_functions_from_env_vars for more comments +def test_docker_compose_run_preserves_bash_functions(): + clean_up_dc_containers() + clean_up_dc_network() + clean_up_dc_dojofile() + envs = dict(os.environ) + proc = run_dojo_and_set_bash_func( + ['--driver=docker-compose', '--dcf=./test/test-files/itest-dc.yaml', '--debug=true', '--test=true', + '--image=alpine:3.8', 'sh', '-c', + '"apk add -U bash && bash -c \'source /etc/dojo.d/variables/01-bash-functions.sh && my_bash_func\'"'], + env=envs) + stdout_value_bytes, stderr_value_bytes = proc.communicate() + stdout = str(stdout_value_bytes) + stderr = str(stderr_value_bytes) + assert 'Dojo version' in stderr + assert 'Written file /tmp/test-dojo-environment-bash-functions-testdojorunid, contents:' in stderr + assert 'my_bash_func() { echo "hello"' in stderr + assert '/etc/dojo.d/variables/01-bash-functions.sh' in stderr + assert_no_warnings_or_errors(stderr) + assert_no_warnings_or_errors(stdout) + # the bash function was invoked + assert 'hello' in stdout + assert 'Exit status from run command: 0' in stderr + test_dc_dojofile_is_removed() + test_dc_containers_are_removed() + test_dc_network_is_removed() + + def test_docker_compose_pull(): result = run_dojo('--driver=docker-compose --dcf=./test/test-files/itest-dc.yaml --debug=true --action=pull --image=alpine:3.8'.split(' ')) assert 'Dojo version' in result.stderr @@ -288,4 +316,4 @@ def test_docker_compose_run_shows_nondefault_containers_logs_when_all_constainer test_dc_dojofile_is_removed() test_dc_containers_are_removed() test_dc_network_is_removed() - clean_up_dojo_logs_file(logs_file) \ No newline at end of file + clean_up_dojo_logs_file(logs_file) diff --git a/version.go b/version.go index d772b50..3c59ca4 100644 --- a/version.go +++ b/version.go @@ -1,3 +1,3 @@ package main -const DojoVersion = "0.8.0" +const DojoVersion = "0.9.0"