Add proper support for working-directory & fix command builder (#772)

* fix: align other Docker executors to print action * fix: formatting * fix: add proper workdir support * fix: replace script filepath after slice creation * fix: match substring so it works for pwsh + rename containerPath to scriptPath to reflect what value it contains
2021-08-10 19:40:20 +00:00 · 2021-08-10 19:40:20 +00:00 · bea32d5651
parent 77b3968913
commit bea32d5651
6 changed files with 76 additions and 36 deletions
--- a/pkg/container/docker_run.go
+++ b/pkg/container/docker_run.go
@ -69,7 +69,7 @@ type Container interface {
 	GetContainerArchive(ctx context.Context, srcPath string) (io.ReadCloser, error)
 	Pull(forcePull bool) common.Executor
 	Start(attach bool) common.Executor
-	Exec(command []string, env map[string]string, user string) common.Executor
+	Exec(command []string, env map[string]string, user, workdir string) common.Executor
 	UpdateFromEnv(srcPath string, env *map[string]string) common.Executor
 	UpdateFromPath(env *map[string]string) common.Executor
 	Remove() common.Executor
@ -103,7 +103,7 @@ func supportsContainerImagePlatform(cli *client.Client) bool {

 func (cr *containerReference) Create(capAdd []string, capDrop []string) common.Executor {
 	return common.
-		NewDebugExecutor("%sdocker create image=%s platform=%s entrypoint=%+q cmd=%+q", logPrefix, cr.input.Image, cr.input.Platform, cr.input.Entrypoint, cr.input.Cmd).
+		NewInfoExecutor("%sdocker create image=%s platform=%s entrypoint=%+q cmd=%+q", logPrefix, cr.input.Image, cr.input.Platform, cr.input.Entrypoint, cr.input.Cmd).
 		Then(
 			common.NewPipelineExecutor(
 				cr.connect(),
@ -112,6 +112,7 @@ func (cr *containerReference) Create(capAdd []string, capDrop []string) common.E
 			).IfNot(common.Dryrun),
 		)
 }
+
 func (cr *containerReference) Start(attach bool) common.Executor {
 	return common.
 		NewInfoExecutor("%sdocker run image=%s platform=%s entrypoint=%+q cmd=%+q", logPrefix, cr.input.Image, cr.input.Platform, cr.input.Entrypoint, cr.input.Cmd).
@ -125,14 +126,19 @@ func (cr *containerReference) Start(attach bool) common.Executor {
 			).IfNot(common.Dryrun),
 		)
 }
+
 func (cr *containerReference) Pull(forcePull bool) common.Executor {
-	return NewDockerPullExecutor(NewDockerPullExecutorInput{
-		Image:     cr.input.Image,
-		ForcePull: forcePull,
-		Platform:  cr.input.Platform,
-		Username:  cr.input.Username,
-		Password:  cr.input.Password,
-	})
+	return common.
+		NewInfoExecutor("%sdocker pull image=%s platform=%s username=%s forcePull=%t", logPrefix, cr.input.Image, cr.input.Platform, cr.input.Username, forcePull).
+		Then(
+			NewDockerPullExecutor(NewDockerPullExecutorInput{
+				Image:     cr.input.Image,
+				ForcePull: forcePull,
+				Platform:  cr.input.Platform,
+				Username:  cr.input.Username,
+				Password:  cr.input.Password,
+			}),
+		)
 }

 func (cr *containerReference) Copy(destPath string, files ...*FileEntry) common.Executor {
@ -146,7 +152,7 @@ func (cr *containerReference) Copy(destPath string, files ...*FileEntry) common.
 func (cr *containerReference) CopyDir(destPath string, srcPath string, useGitIgnore bool) common.Executor {
 	return common.NewPipelineExecutor(
 		common.NewInfoExecutor("%sdocker cp src=%s dst=%s", logPrefix, srcPath, destPath),
-		cr.Exec([]string{"mkdir", "-p", destPath}, nil, ""),
+		cr.Exec([]string{"mkdir", "-p", destPath}, nil, "", ""),
 		cr.copyDir(destPath, srcPath, useGitIgnore),
 	).IfNot(common.Dryrun)
 }
@ -164,12 +170,12 @@ func (cr *containerReference) UpdateFromPath(env *map[string]string) common.Exec
 	return cr.extractPath(env).IfNot(common.Dryrun)
 }

-func (cr *containerReference) Exec(command []string, env map[string]string, user string) common.Executor {
+func (cr *containerReference) Exec(command []string, env map[string]string, user, workdir string) common.Executor {
 	return common.NewPipelineExecutor(
-		common.NewInfoExecutor("%sdocker exec cmd=[%s] user=%s", logPrefix, strings.Join(command, " "), user),
+		common.NewInfoExecutor("%sdocker exec cmd=[%s] user=%s workdir=%s", logPrefix, strings.Join(command, " "), user, workdir),
 		cr.connect(),
 		cr.find(),
-		cr.exec(command, env, user),
+		cr.exec(command, env, user, workdir),
 	).IfNot(common.Dryrun)
 }

@ -414,7 +420,7 @@ func (cr *containerReference) extractPath(env *map[string]string) common.Executo
 	}
 }

-func (cr *containerReference) exec(cmd []string, env map[string]string, user string) common.Executor {
+func (cr *containerReference) exec(cmd []string, env map[string]string, user, workdir string) common.Executor {
 	return func(ctx context.Context) error {
 		logger := common.Logger(ctx)
 		// Fix slashes when running on Windows
@ -433,10 +439,22 @@ func (cr *containerReference) exec(cmd []string, env map[string]string, user str
 			envList = append(envList, fmt.Sprintf("%s=%s", k, v))
 		}

+		var wd string
+		if workdir != "" {
+			if strings.HasPrefix(workdir, "/") {
+				wd = workdir
+			} else {
+				wd = fmt.Sprintf("%s/%s", cr.input.WorkingDir, workdir)
+			}
+		} else {
+			wd = cr.input.WorkingDir
+		}
+		logger.Debugf("Working directory '%s'", wd)
+
 		idResp, err := cr.cli.ContainerExecCreate(ctx, cr.id, types.ExecConfig{
 			User:         user,
 			Cmd:          cmd,
-			WorkingDir:   cr.input.WorkingDir,
+			WorkingDir:   wd,
 			Env:          envList,
 			Tty:          isTerminal,
 			AttachStderr: true,
--- a/pkg/runner/run_context.go
+++ b/pkg/runner/run_context.go
@ -151,7 +151,7 @@ func (rc *RunContext) startJobContainer() common.Executor {
 			rc.JobContainer.Create(rc.Config.ContainerCapAdd, rc.Config.ContainerCapDrop),
 			rc.JobContainer.Start(false),
 			rc.JobContainer.UpdateFromEnv("/etc/environment", &rc.Env),
-			rc.JobContainer.Exec([]string{"mkdir", "-m", "0777", "-p", ActPath}, rc.Env, "root"),
+			rc.JobContainer.Exec([]string{"mkdir", "-m", "0777", "-p", ActPath}, rc.Env, "root", ""),
 			rc.JobContainer.CopyDir(copyToPath, rc.Config.Workdir+string(filepath.Separator)+".", rc.Config.UseGitIgnore).IfBool(copyWorkspace),
 			rc.JobContainer.Copy(ActPath+"/", &container.FileEntry{
 				Name: "workflow/event.json",
@ -169,9 +169,10 @@ func (rc *RunContext) startJobContainer() common.Executor {
 		)(ctx)
 	}
 }
-func (rc *RunContext) execJobContainer(cmd []string, env map[string]string) common.Executor {
+
+func (rc *RunContext) execJobContainer(cmd []string, env map[string]string, user, workdir string) common.Executor {
 	return func(ctx context.Context) error {
-		return rc.JobContainer.Exec(cmd, env, "")(ctx)
+		return rc.JobContainer.Exec(cmd, env, user, workdir)(ctx)
 	}
 }

--- a/pkg/runner/step_context.go
+++ b/pkg/runner/step_context.go
@ -37,7 +37,7 @@ type StepContext struct {

 func (sc *StepContext) execJobContainer() common.Executor {
 	return func(ctx context.Context) error {
-		return sc.RunContext.execJobContainer(sc.Cmd, sc.Env)(ctx)
+		return sc.RunContext.execJobContainer(sc.Cmd, sc.Env, "", sc.Step.WorkingDirectory)(ctx)
 	}
 }

@ -195,12 +195,6 @@ func (sc *StepContext) setupShellCommand() common.Executor {
 			step.WorkingDirectory = rc.Run.Workflow.Defaults.Run.WorkingDirectory
 		}
 		step.WorkingDirectory = rc.ExprEval.Interpolate(step.WorkingDirectory)
-		if step.WorkingDirectory != "" {
-			_, err = script.WriteString(fmt.Sprintf("cd %s\n", step.WorkingDirectory))
-			if err != nil {
-				return err
-			}
-		}

 		run := rc.ExprEval.Interpolate(step.Run)
 		step.Shell = rc.ExprEval.Interpolate(step.Shell)
@ -233,7 +227,7 @@ func (sc *StepContext) setupShellCommand() common.Executor {
 		run = runPrepend + "\n" + run + "\n" + runAppend

 		log.Debugf("Wrote command '%s' to '%s'", run, scriptName)
-		containerPath := fmt.Sprintf("%s/%s", rc.Config.ContainerWorkdir(), scriptName)
+		scriptPath := fmt.Sprintf("%s/%s", rc.Config.ContainerWorkdir(), scriptName)

 		if step.Shell == "" {
 			step.Shell = rc.Run.Job().Defaults.Run.Shell
@ -242,13 +236,22 @@ func (sc *StepContext) setupShellCommand() common.Executor {
 			step.Shell = rc.Run.Workflow.Defaults.Run.Shell
 		}
 		scCmd := step.ShellCommand()
-		scResolvedCmd := strings.Replace(scCmd, "{0}", containerPath, 1)
+
+		var finalCMD []string
 		if step.Shell == "pwsh" || step.Shell == "powershell" {
-			sc.Cmd = strings.SplitN(scResolvedCmd, " ", 3)
+			finalCMD = strings.SplitN(scCmd, " ", 3)
 		} else {
-			sc.Cmd = strings.Fields(scResolvedCmd)
+			finalCMD = strings.Fields(scCmd)
 		}

+		for k, v := range finalCMD {
+			if strings.Contains(v, `{0}`) {
+				finalCMD[k] = strings.Replace(v, `{0}`, scriptPath, 1)
+			}
+		}
+
+		sc.Cmd = finalCMD
+
 		return rc.JobContainer.Copy(rc.Config.ContainerWorkdir(), &container.FileEntry{
 			Name: scriptName,
 			Mode: 0755,
@ -307,6 +310,7 @@ func (sc *StepContext) newStepContainer(ctx context.Context, image string, cmd [
 	})
 	return stepContainer
 }
+
 func (sc *StepContext) runUsesContainer() common.Executor {
 	rc := sc.RunContext
 	step := sc.Step
@ -485,7 +489,7 @@ func (sc *StepContext) runAction(actionDir string, actionPath string, localActio
 			}
 			containerArgs := []string{"node", path.Join(containerActionDir, action.Runs.Main)}
 			log.Debugf("executing remote job container: %s", containerArgs)
-			return rc.execJobContainer(containerArgs, sc.Env)(ctx)
+			return rc.execJobContainer(containerArgs, sc.Env, "", "")(ctx)
 		case model.ActionRunsUsingDocker:
 			return sc.execAsDocker(ctx, action, actionName, containerActionDir, actionLocation, rc, step, localAction)
 		case model.ActionRunsUsingComposite:
--- a/pkg/runner/testdata/basic/push.yml
+++ b/pkg/runner/testdata/basic/push.yml
@ -8,6 +8,7 @@ jobs:
  check:
    runs-on: ubuntu-latest
    steps:
+      - run: '[[ "$(pwd)" == "${GITHUB_WORKSPACE}" ]]'
      - run: echo ${{ env.TEST }} | grep value
      - run: env
      - uses: docker://node:12-buster-slim
--- a/pkg/runner/testdata/dir
+++ b/pkg/runner/testdata/dir
@ -0,0 +1,7 @@
+---
+jobs:
+  dir-with-spaces:
+    runs-on: ubuntu-latest
+    steps:
+      - run: echo "$PWD"
+'on': push
--- a/pkg/runner/testdata/workdir/push.yml
+++ b/pkg/runner/testdata/workdir/push.yml
@ -1,15 +1,24 @@
-name: workdir
 on: push

+defaults:
+  run:
+    working-directory: /tmp
+
 jobs:
  workdir:
    runs-on: ubuntu-latest
+    defaults:
+      run:
+        working-directory: /root
    steps:
-    - run: mkdir -p "${GITHUB_WORKSPACE}/workdir"
-    - run: '[[ "$(pwd)" == "${GITHUB_WORKSPACE}/workdir" ]]'
-      working-directory: workdir
+      - run: '[[ "$(pwd)" == "/root" ]]'

-  noworkdir:
+      - run: mkdir -p "${GITHUB_WORKSPACE}/workdir"
+
+      - run: '[[ "$(pwd)" == "${GITHUB_WORKSPACE}/workdir" ]]'
+        working-directory: workdir
+
+  top-level-workdir:
    runs-on: ubuntu-latest
    steps:
-      - run: '[[ "$(pwd)" == "${GITHUB_WORKSPACE}" ]]'
+      - run: '[[ "$(pwd)" == "/tmp" ]]'