From bea32d5651c94049173799c496bdc87a4b304f02 Mon Sep 17 00:00:00 2001 From: Ryan Date: Tue, 10 Aug 2021 19:40:20 +0000 Subject: [PATCH] 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 --- pkg/container/docker_run.go | 48 ++++++++++++++------ pkg/runner/run_context.go | 7 +-- pkg/runner/step_context.go | 28 +++++++----- pkg/runner/testdata/basic/push.yml | 1 + pkg/runner/testdata/dir with spaces/push.yml | 7 +++ pkg/runner/testdata/workdir/push.yml | 21 ++++++--- 6 files changed, 76 insertions(+), 36 deletions(-) create mode 100644 pkg/runner/testdata/dir with spaces/push.yml diff --git a/pkg/container/docker_run.go b/pkg/container/docker_run.go index 6e4d3d8..190ee05 100644 --- 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, diff --git a/pkg/runner/run_context.go b/pkg/runner/run_context.go index 985eed9..c736ed1 100755 --- 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) } } diff --git a/pkg/runner/step_context.go b/pkg/runner/step_context.go index 5d64241..57be09a 100644 --- 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: diff --git a/pkg/runner/testdata/basic/push.yml b/pkg/runner/testdata/basic/push.yml index 8b67f21..a1450b7 100644 --- 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 diff --git a/pkg/runner/testdata/dir with spaces/push.yml b/pkg/runner/testdata/dir with spaces/push.yml new file mode 100644 index 0000000..46d2602 --- /dev/null +++ b/pkg/runner/testdata/dir with spaces/push.yml @@ -0,0 +1,7 @@ +--- +jobs: + dir-with-spaces: + runs-on: ubuntu-latest + steps: + - run: echo "$PWD" +'on': push diff --git a/pkg/runner/testdata/workdir/push.yml b/pkg/runner/testdata/workdir/push.yml index 1d7ace4..c287d55 100644 --- 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" ]]'