From 65b339a86258934728e55978f98ca63922598bd3 Mon Sep 17 00:00:00 2001 From: ChristopherHX Date: Sun, 11 Dec 2022 21:59:30 +0100 Subject: [PATCH] fix: GITHUB_ENV / PATH handling --- pkg/runner/action_composite.go | 1 + pkg/runner/run_context.go | 18 +++++++-- pkg/runner/runner_test.go | 4 +- pkg/runner/step.go | 39 +++++++++++-------- pkg/runner/step_action_local_test.go | 16 +++----- pkg/runner/step_action_remote_test.go | 16 ++++---- pkg/runner/step_docker_test.go | 12 ++---- pkg/runner/step_run_test.go | 6 +-- pkg/runner/step_test.go | 4 -- .../GITHUB_ENV-use-in-env-ctx/push.yml | 27 +++++++++++++ .../docker-action-custom-path/push.yml | 12 ++++++ .../remote-action-js-node-user/push.yml | 15 +++++++ 12 files changed, 113 insertions(+), 57 deletions(-) create mode 100644 pkg/runner/testdata/GITHUB_ENV-use-in-env-ctx/push.yml create mode 100644 pkg/runner/testdata/docker-action-custom-path/push.yml create mode 100644 pkg/runner/testdata/remote-action-js-node-user/push.yml diff --git a/pkg/runner/action_composite.go b/pkg/runner/action_composite.go index c1e94fcd2cd..348568c641e 100644 --- a/pkg/runner/action_composite.go +++ b/pkg/runner/action_composite.go @@ -99,6 +99,7 @@ func execAsComposite(step actionStep) common.Executor { rc.Masks = append(rc.Masks, compositeRC.Masks...) rc.ExtraPath = compositeRC.ExtraPath + rc.Env = compositeRC.Env return err } diff --git a/pkg/runner/run_context.go b/pkg/runner/run_context.go index c653cc7e89a..91acb22e216 100644 --- a/pkg/runner/run_context.go +++ b/pkg/runner/run_context.go @@ -15,6 +15,7 @@ import ( "regexp" "runtime" "strings" + "time" "github.com/mitchellh/go-homedir" "github.com/opencontainers/selinux/go-selinux" @@ -266,8 +267,6 @@ func (rc *RunContext) startJobContainer() common.Executor { rc.stopJobContainer(), rc.JobContainer.Create(rc.Config.ContainerCapAdd, rc.Config.ContainerCapDrop), rc.JobContainer.Start(false), - rc.JobContainer.UpdateFromImageEnv(&rc.Env), - rc.JobContainer.UpdateFromEnv("/etc/environment", &rc.Env), rc.JobContainer.Copy(rc.JobContainer.GetActPath()+"/", &container.FileEntry{ Name: "workflow/event.json", Mode: 0644, @@ -291,7 +290,19 @@ func (rc *RunContext) ApplyExtraPath(env *map[string]string) { if rc.ExtraPath != nil && len(rc.ExtraPath) > 0 { path := rc.JobContainer.GetPathVariableName() if (*env)[path] == "" { - (*env)[path] = rc.JobContainer.DefaultPathVariable() + cenv := map[string]string{} + var cpath string + ctx, cancel := context.WithTimeout(context.Background(), time.Minute) + defer cancel() + if err := rc.JobContainer.UpdateFromImageEnv(&cenv)(ctx); err == nil { + if p, ok := cenv[path]; ok { + cpath = p + } + } + if len(cpath) == 0 { + cpath = rc.JobContainer.DefaultPathVariable() + } + (*env)[path] = cpath } (*env)[path] = rc.JobContainer.JoinPathVariable(append(rc.ExtraPath, (*env)[path])...) } @@ -668,7 +679,6 @@ func nestedMapLookup(m map[string]interface{}, ks ...string) (rval interface{}) func (rc *RunContext) withGithubEnv(ctx context.Context, github *model.GithubContext, env map[string]string) map[string]string { env["CI"] = "true" - env["GITHUB_ENV"] = rc.JobContainer.GetActPath() + "/workflow/envs.txt" env["GITHUB_WORKFLOW"] = github.Workflow env["GITHUB_RUN_ID"] = github.RunID env["GITHUB_RUN_NUMBER"] = github.RunNumber diff --git a/pkg/runner/runner_test.go b/pkg/runner/runner_test.go index faa13ced30a..991507c5f19 100644 --- a/pkg/runner/runner_test.go +++ b/pkg/runner/runner_test.go @@ -162,7 +162,7 @@ func TestRunEvent(t *testing.T) { {workdir, "container-hostname", "push", "", platforms}, {workdir, "remote-action-docker", "push", "", platforms}, {workdir, "remote-action-js", "push", "", platforms}, - {workdir, "remote-action-js", "push", "", map[string]string{"ubuntu-latest": "catthehacker/ubuntu:runner-latest"}}, // Test if this works with non root container + {workdir, "remote-action-js-node-user", "push", "", platforms}, // Test if this works with non root container {workdir, "matrix", "push", "", platforms}, {workdir, "matrix-include-exclude", "push", "", platforms}, {workdir, "commands", "push", "", platforms}, @@ -186,6 +186,8 @@ func TestRunEvent(t *testing.T) { {workdir, "actions-environment-and-context-tests", "push", "", platforms}, {workdir, "uses-action-with-pre-and-post-step", "push", "", platforms}, {workdir, "evalenv", "push", "", platforms}, + {workdir, "docker-action-custom-path", "push", "", platforms}, + {workdir, "GITHUB_ENV-use-in-env-ctx", "push", "", platforms}, {workdir, "ensure-post-steps", "push", "Job 'second-post-step-should-fail' failed", platforms}, {workdir, "workflow_dispatch", "workflow_dispatch", "", platforms}, {workdir, "workflow_dispatch_no_inputs_mapping", "workflow_dispatch", "", platforms}, diff --git a/pkg/runner/step.go b/pkg/runner/step.go index f8a192fdc30..9f40240a018 100644 --- a/pkg/runner/step.go +++ b/pkg/runner/step.go @@ -56,6 +56,18 @@ func (s stepStage) getStepName(stepModel *model.Step) string { return "unknown" } +func processRunnerEnvFileCommand(ctx context.Context, fileName string, rc *RunContext, setter func(context.Context, map[string]string, string)) error { + env := map[string]string{} + err := rc.JobContainer.UpdateFromEnv(path.Join(rc.JobContainer.GetActPath(), fileName), &env)(ctx) + if err != nil { + return err + } + for k, v := range env { + setter(ctx, map[string]string{"name": k}, v) + } + return nil +} + func runStepExecutor(step step, stage stepStage, executor common.Executor) common.Executor { return func(ctx context.Context) error { logger := common.Logger(ctx) @@ -101,9 +113,11 @@ func runStepExecutor(step step, stage stepStage, executor common.Executor) commo outputFileCommand := path.Join("workflow", "outputcmd.txt") stateFileCommand := path.Join("workflow", "statecmd.txt") pathFileCommand := path.Join("workflow", "pathcmd.txt") + envFileCommand := path.Join("workflow", "envs.txt") (*step.getEnv())["GITHUB_OUTPUT"] = path.Join(actPath, outputFileCommand) (*step.getEnv())["GITHUB_STATE"] = path.Join(actPath, stateFileCommand) (*step.getEnv())["GITHUB_PATH"] = path.Join(actPath, pathFileCommand) + (*step.getEnv())["GITHUB_ENV"] = path.Join(actPath, envFileCommand) _ = rc.JobContainer.Copy(actPath, &container.FileEntry{ Name: outputFileCommand, Mode: 0666, @@ -113,6 +127,9 @@ func runStepExecutor(step step, stage stepStage, executor common.Executor) commo }, &container.FileEntry{ Name: pathFileCommand, Mode: 0666, + }, &container.FileEntry{ + Name: envFileCommand, + Mode: 0666, })(ctx) err = executor(ctx) @@ -140,21 +157,17 @@ func runStepExecutor(step step, stage stepStage, executor common.Executor) commo } // Process Runner File Commands orgerr := err - state := map[string]string{} - err = rc.JobContainer.UpdateFromEnv(path.Join(actPath, stateFileCommand), &state)(ctx) + err = processRunnerEnvFileCommand(ctx, envFileCommand, rc, rc.setEnv) if err != nil { return err } - for k, v := range state { - rc.saveState(ctx, map[string]string{"name": k}, v) - } - output := map[string]string{} - err = rc.JobContainer.UpdateFromEnv(path.Join(actPath, outputFileCommand), &output)(ctx) + err = processRunnerEnvFileCommand(ctx, stateFileCommand, rc, rc.saveState) if err != nil { return err } - for k, v := range output { - rc.setOutput(ctx, map[string]string{"name": k}, v) + err = processRunnerEnvFileCommand(ctx, outputFileCommand, rc, rc.setOutput) + if err != nil { + return err } err = rc.UpdateExtraPath(ctx, path.Join(actPath, pathFileCommand)) if err != nil { @@ -171,14 +184,6 @@ func setupEnv(ctx context.Context, step step) error { rc := step.getRunContext() mergeEnv(ctx, step) - err := rc.JobContainer.UpdateFromImageEnv(step.getEnv())(ctx) - if err != nil { - return err - } - err = rc.JobContainer.UpdateFromEnv((*step.getEnv())["GITHUB_ENV"], step.getEnv())(ctx) - if err != nil { - return err - } // merge step env last, since it should not be overwritten mergeIntoMap(step.getEnv(), step.getStepModel().GetEnv()) diff --git a/pkg/runner/step_action_local_test.go b/pkg/runner/step_action_local_test.go index 9ece06a4172..ecac7818bf5 100644 --- a/pkg/runner/step_action_local_test.go +++ b/pkg/runner/step_action_local_test.go @@ -69,7 +69,7 @@ func TestStepActionLocalTest(t *testing.T) { salm.On("readAction", sal.Step, filepath.Clean("/tmp/path/to/action"), "", mock.Anything, mock.Anything). Return(&model.Action{}, nil) - cm.On("UpdateFromImageEnv", mock.AnythingOfType("*map[string]string")).Return(func(ctx context.Context) error { + cm.On("Copy", "/var/run/act", mock.AnythingOfType("[]*container.FileEntry")).Return(func(ctx context.Context) error { return nil }) @@ -77,10 +77,6 @@ func TestStepActionLocalTest(t *testing.T) { return nil }) - cm.On("Copy", "/var/run/act", mock.AnythingOfType("[]*container.FileEntry")).Return(func(ctx context.Context) error { - return nil - }) - cm.On("UpdateFromEnv", "/var/run/act/workflow/statecmd.txt", mock.AnythingOfType("*map[string]string")).Return(func(ctx context.Context) error { return nil }) @@ -213,7 +209,7 @@ func TestStepActionLocalPost(t *testing.T) { env bool exec bool }{ - env: true, + env: false, exec: false, }, }, @@ -277,10 +273,6 @@ func TestStepActionLocalPost(t *testing.T) { } sal.RunContext.ExprEval = sal.RunContext.NewExpressionEvaluator(ctx) - if tt.mocks.env { - cm.On("UpdateFromImageEnv", &sal.env).Return(func(ctx context.Context) error { return nil }) - cm.On("UpdateFromEnv", "/var/run/act/workflow/envs.txt", &sal.env).Return(func(ctx context.Context) error { return nil }) - } if tt.mocks.exec { suffixMatcher := func(suffix string) interface{} { return mock.MatchedBy(func(array []string) bool { @@ -293,6 +285,10 @@ func TestStepActionLocalPost(t *testing.T) { return nil }) + cm.On("UpdateFromEnv", "/var/run/act/workflow/envs.txt", mock.AnythingOfType("*map[string]string")).Return(func(ctx context.Context) error { + return nil + }) + cm.On("UpdateFromEnv", "/var/run/act/workflow/statecmd.txt", mock.AnythingOfType("*map[string]string")).Return(func(ctx context.Context) error { return nil }) diff --git a/pkg/runner/step_action_remote_test.go b/pkg/runner/step_action_remote_test.go index 829e3864485..a8d29fc3720 100644 --- a/pkg/runner/step_action_remote_test.go +++ b/pkg/runner/step_action_remote_test.go @@ -165,10 +165,6 @@ func TestStepActionRemote(t *testing.T) { }) } - if tt.mocks.env { - cm.On("UpdateFromImageEnv", &sar.env).Return(func(ctx context.Context) error { return nil }) - cm.On("UpdateFromEnv", "/var/run/act/workflow/envs.txt", &sar.env).Return(func(ctx context.Context) error { return nil }) - } if tt.mocks.read { sarm.On("readAction", sar.Step, suffixMatcher("act/remote-action@v1"), "", mock.Anything, mock.Anything).Return(&model.Action{}, nil) } @@ -179,6 +175,10 @@ func TestStepActionRemote(t *testing.T) { return nil }) + cm.On("UpdateFromEnv", "/var/run/act/workflow/envs.txt", mock.AnythingOfType("*map[string]string")).Return(func(ctx context.Context) error { + return nil + }) + cm.On("UpdateFromEnv", "/var/run/act/workflow/statecmd.txt", mock.AnythingOfType("*map[string]string")).Return(func(ctx context.Context) error { return nil }) @@ -592,10 +592,6 @@ func TestStepActionRemotePost(t *testing.T) { } sar.RunContext.ExprEval = sar.RunContext.NewExpressionEvaluator(ctx) - if tt.mocks.env { - cm.On("UpdateFromImageEnv", &sar.env).Return(func(ctx context.Context) error { return nil }) - cm.On("UpdateFromEnv", "/var/run/act/workflow/envs.txt", &sar.env).Return(func(ctx context.Context) error { return nil }) - } if tt.mocks.exec { cm.On("Exec", []string{"node", "/var/run/act/actions/remote-action@v1/post.js"}, sar.env, "", "").Return(func(ctx context.Context) error { return tt.err }) @@ -603,6 +599,10 @@ func TestStepActionRemotePost(t *testing.T) { return nil }) + cm.On("UpdateFromEnv", "/var/run/act/workflow/envs.txt", mock.AnythingOfType("*map[string]string")).Return(func(ctx context.Context) error { + return nil + }) + cm.On("UpdateFromEnv", "/var/run/act/workflow/statecmd.txt", mock.AnythingOfType("*map[string]string")).Return(func(ctx context.Context) error { return nil }) diff --git a/pkg/runner/step_docker_test.go b/pkg/runner/step_docker_test.go index c26ffd685a9..3d90ac34f9e 100644 --- a/pkg/runner/step_docker_test.go +++ b/pkg/runner/step_docker_test.go @@ -57,14 +57,6 @@ func TestStepDockerMain(t *testing.T) { } sd.RunContext.ExprEval = sd.RunContext.NewExpressionEvaluator(ctx) - cm.On("UpdateFromImageEnv", mock.AnythingOfType("*map[string]string")).Return(func(ctx context.Context) error { - return nil - }) - - cm.On("UpdateFromEnv", "/var/run/act/workflow/envs.txt", mock.AnythingOfType("*map[string]string")).Return(func(ctx context.Context) error { - return nil - }) - cm.On("Pull", false).Return(func(ctx context.Context) error { return nil }) @@ -89,6 +81,10 @@ func TestStepDockerMain(t *testing.T) { return nil }) + cm.On("UpdateFromEnv", "/var/run/act/workflow/envs.txt", mock.AnythingOfType("*map[string]string")).Return(func(ctx context.Context) error { + return nil + }) + cm.On("UpdateFromEnv", "/var/run/act/workflow/statecmd.txt", mock.AnythingOfType("*map[string]string")).Return(func(ctx context.Context) error { return nil }) diff --git a/pkg/runner/step_run_test.go b/pkg/runner/step_run_test.go index 324ed5fcc8f..4ca2eb97456 100644 --- a/pkg/runner/step_run_test.go +++ b/pkg/runner/step_run_test.go @@ -55,7 +55,7 @@ func TestStepRun(t *testing.T) { return nil }) - cm.On("UpdateFromImageEnv", mock.AnythingOfType("*map[string]string")).Return(func(ctx context.Context) error { + cm.On("Copy", "/var/run/act", mock.AnythingOfType("[]*container.FileEntry")).Return(func(ctx context.Context) error { return nil }) @@ -63,10 +63,6 @@ func TestStepRun(t *testing.T) { return nil }) - cm.On("Copy", "/var/run/act", mock.AnythingOfType("[]*container.FileEntry")).Return(func(ctx context.Context) error { - return nil - }) - cm.On("UpdateFromEnv", "/var/run/act/workflow/statecmd.txt", mock.AnythingOfType("*map[string]string")).Return(func(ctx context.Context) error { return nil }) diff --git a/pkg/runner/step_test.go b/pkg/runner/step_test.go index 8930cca3e7f..86e5acc4a26 100644 --- a/pkg/runner/step_test.go +++ b/pkg/runner/step_test.go @@ -148,9 +148,6 @@ func TestSetupEnv(t *testing.T) { sm.On("getStepModel").Return(step) sm.On("getEnv").Return(&env) - cm.On("UpdateFromImageEnv", &env).Return(func(ctx context.Context) error { return nil }) - cm.On("UpdateFromEnv", "/var/run/act/workflow/envs.txt", &env).Return(func(ctx context.Context) error { return nil }) - err := setupEnv(context.Background(), sm) assert.Nil(t, err) @@ -174,7 +171,6 @@ func TestSetupEnv(t *testing.T) { "GITHUB_ACTION_REPOSITORY": "", "GITHUB_API_URL": "https:///api/v3", "GITHUB_BASE_REF": "", - "GITHUB_ENV": "/var/run/act/workflow/envs.txt", "GITHUB_EVENT_NAME": "", "GITHUB_EVENT_PATH": "/var/run/act/workflow/event.json", "GITHUB_GRAPHQL_URL": "https:///api/graphql", diff --git a/pkg/runner/testdata/GITHUB_ENV-use-in-env-ctx/push.yml b/pkg/runner/testdata/GITHUB_ENV-use-in-env-ctx/push.yml new file mode 100644 index 00000000000..c7b75a02003 --- /dev/null +++ b/pkg/runner/testdata/GITHUB_ENV-use-in-env-ctx/push.yml @@ -0,0 +1,27 @@ +on: push +jobs: + _: + runs-on: ubuntu-latest + env: + MYGLOBALENV3: myglobalval3 + steps: + - run: | + echo MYGLOBALENV1=myglobalval1 > $GITHUB_ENV + echo "::set-env name=MYGLOBALENV2::myglobalval2" + - uses: nektos/act-test-actions/script@main + with: + main: | + env + [[ "$MYGLOBALENV1" = "${{ env.MYGLOBALENV1 }}" ]] + [[ "$MYGLOBALENV1" = "${{ env.MYGLOBALENV1ALIAS }}" ]] + [[ "$MYGLOBALENV1" = "$MYGLOBALENV1ALIAS" ]] + [[ "$MYGLOBALENV2" = "${{ env.MYGLOBALENV2 }}" ]] + [[ "$MYGLOBALENV2" = "${{ env.MYGLOBALENV2ALIAS }}" ]] + [[ "$MYGLOBALENV2" = "$MYGLOBALENV2ALIAS" ]] + [[ "$MYGLOBALENV3" = "${{ env.MYGLOBALENV3 }}" ]] + [[ "$MYGLOBALENV3" = "${{ env.MYGLOBALENV3ALIAS }}" ]] + [[ "$MYGLOBALENV3" = "$MYGLOBALENV3ALIAS" ]] + env: + MYGLOBALENV1ALIAS: ${{ env.MYGLOBALENV1 }} + MYGLOBALENV2ALIAS: ${{ env.MYGLOBALENV2 }} + MYGLOBALENV3ALIAS: ${{ env.MYGLOBALENV3 }} \ No newline at end of file diff --git a/pkg/runner/testdata/docker-action-custom-path/push.yml b/pkg/runner/testdata/docker-action-custom-path/push.yml new file mode 100644 index 00000000000..37bbf417344 --- /dev/null +++ b/pkg/runner/testdata/docker-action-custom-path/push.yml @@ -0,0 +1,12 @@ +on: push +jobs: + _: + runs-on: ubuntu-latest + steps: + - run: | + FROM ubuntu:latest + ENV PATH="/opt/texlive/texdir/bin/x86_64-linuxmusl:${PATH}" + ENV ORG_PATH="${PATH}" + ENTRYPOINT [ "bash", "-c", "echo \"PATH=$PATH\" && echo \"ORG_PATH=$ORG_PATH\" && [[ \"$PATH\" = \"$ORG_PATH\" ]]" ] + shell: mv {0} Dockerfile + - uses: ./ \ No newline at end of file diff --git a/pkg/runner/testdata/remote-action-js-node-user/push.yml b/pkg/runner/testdata/remote-action-js-node-user/push.yml new file mode 100644 index 00000000000..cede7b0a204 --- /dev/null +++ b/pkg/runner/testdata/remote-action-js-node-user/push.yml @@ -0,0 +1,15 @@ +name: remote-action-js +on: push + +jobs: + test: + runs-on: ubuntu-latest + container: + image: node:16-buster-slim + options: --user node + steps: + - uses: actions/hello-world-javascript-action@v1 + with: + who-to-greet: 'Mona the Octocat' + + - uses: cloudposse/actions/github/slash-command-dispatch@0.14.0