support exported bash functions #17

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.

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)
 

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,29 +113,34 @@ 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:
     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
   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"))
 }
 

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