refactor: fix add-path / GITHUB_PATH commands (#1472)

* fix: add-path / GITHUB_PATH commands

* Disable old code

* fix: missing mock

* Update tests

* fix tests

* UpdateExtraPath skip on dryrun

* patch test

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
This commit is contained in:
ChristopherHX 2022-12-09 12:16:15 +01:00 committed by GitHub
parent 7d9951200f
commit d281230cce
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
12 changed files with 95 additions and 46 deletions

View file

@ -154,6 +154,8 @@ func runActionImpl(step actionStep, actionDir string, remoteAction *remoteAction
containerArgs := []string{"node", path.Join(containerActionDir, action.Runs.Main)}
logger.Debugf("executing remote job container: %s", containerArgs)
rc.ApplyExtraPath(step.getEnv())
return rc.execJobContainer(containerArgs, *step.getEnv(), "", "")(ctx)
case model.ActionRunsUsingDocker:
location := actionLocation
@ -486,6 +488,8 @@ func runPreStep(step actionStep) common.Executor {
containerArgs := []string{"node", path.Join(containerActionDir, action.Runs.Pre)}
logger.Debugf("executing remote job container: %s", containerArgs)
rc.ApplyExtraPath(step.getEnv())
return rc.execJobContainer(containerArgs, *step.getEnv(), "", "")(ctx)
case model.ActionRunsUsingComposite:
@ -573,6 +577,8 @@ func runPostStep(step actionStep) common.Executor {
containerArgs := []string{"node", path.Join(containerActionDir, action.Runs.Post)}
logger.Debugf("executing remote job container: %s", containerArgs)
rc.ApplyExtraPath(step.getEnv())
return rc.execJobContainer(containerArgs, *step.getEnv(), "", "")(ctx)
case model.ActionRunsUsingComposite:

View file

@ -101,7 +101,13 @@ func (rc *RunContext) setOutput(ctx context.Context, kvPairs map[string]string,
}
func (rc *RunContext) addPath(ctx context.Context, arg string) {
common.Logger(ctx).Infof(" \U00002699 ::add-path:: %s", arg)
rc.ExtraPath = append(rc.ExtraPath, arg)
extraPath := []string{arg}
for _, v := range rc.ExtraPath {
if v != arg {
extraPath = append(extraPath, v)
}
}
rc.ExtraPath = extraPath
}
func parseKeyValuePairs(kvPairs string, separator string) map[string]string {

View file

@ -64,7 +64,7 @@ func TestAddpath(t *testing.T) {
a.Equal("/zoo", rc.ExtraPath[0])
handler("::add-path::/boo\n")
a.Equal("/boo", rc.ExtraPath[1])
a.Equal("/boo", rc.ExtraPath[0])
}
func TestStopCommands(t *testing.T) {
@ -102,7 +102,7 @@ func TestAddpathADO(t *testing.T) {
a.Equal("/zoo", rc.ExtraPath[0])
handler("##[add-path]/boo\n")
a.Equal("/boo", rc.ExtraPath[1])
a.Equal("/boo", rc.ExtraPath[0])
}
func TestAddmask(t *testing.T) {

View file

@ -2,6 +2,7 @@ package runner
import (
"context"
"io"
"github.com/nektos/act/pkg/common"
"github.com/nektos/act/pkg/container"
@ -63,7 +64,17 @@ func (cm *containerMock) CopyDir(destPath string, srcPath string, useGitIgnore b
args := cm.Called(destPath, srcPath, useGitIgnore)
return args.Get(0).(func(context.Context) error)
}
func (cm *containerMock) Exec(command []string, env map[string]string, user, workdir string) common.Executor {
args := cm.Called(command, env, user, workdir)
return args.Get(0).(func(context.Context) error)
}
func (cm *containerMock) GetContainerArchive(ctx context.Context, srcPath string) (io.ReadCloser, error) {
args := cm.Called(ctx, srcPath)
err, hasErr := args.Get(1).(error)
if !hasErr {
err = nil
}
return args.Get(0).(io.ReadCloser), err
}

View file

@ -1,12 +1,15 @@
package runner
import (
"archive/tar"
"bufio"
"context"
"crypto/rand"
"encoding/hex"
"encoding/json"
"errors"
"fmt"
"io"
"os"
"path/filepath"
"regexp"
@ -188,10 +191,6 @@ func (rc *RunContext) startHostEnvironment() common.Executor {
Name: "workflow/envs.txt",
Mode: 0666,
Body: "",
}, &container.FileEntry{
Name: "workflow/paths.txt",
Mode: 0666,
Body: "",
}),
)(ctx)
}
@ -277,10 +276,6 @@ func (rc *RunContext) startJobContainer() common.Executor {
Name: "workflow/envs.txt",
Mode: 0666,
Body: "",
}, &container.FileEntry{
Name: "workflow/paths.txt",
Mode: 0666,
Body: "",
}),
)(ctx)
}
@ -292,6 +287,41 @@ func (rc *RunContext) execJobContainer(cmd []string, env map[string]string, user
}
}
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()
}
(*env)[path] = rc.JobContainer.JoinPathVariable(append(rc.ExtraPath, (*env)[path])...)
}
}
func (rc *RunContext) UpdateExtraPath(ctx context.Context, githubEnvPath string) error {
if common.Dryrun(ctx) {
return nil
}
pathTar, err := rc.JobContainer.GetContainerArchive(ctx, githubEnvPath)
if err != nil {
return err
}
defer pathTar.Close()
reader := tar.NewReader(pathTar)
_, err = reader.Next()
if err != nil && err != io.EOF {
return err
}
s := bufio.NewScanner(reader)
for s.Scan() {
line := s.Text()
if len(line) > 0 {
rc.addPath(ctx, line)
}
}
return nil
}
// stopJobContainer removes the job container (if it exists) and its volume (if it exists) if !rc.Config.ReuseContainers
func (rc *RunContext) stopJobContainer() common.Executor {
return func(ctx context.Context) error {
@ -639,7 +669,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_PATH"] = rc.JobContainer.GetActPath() + "/workflow/paths.txt"
env["GITHUB_WORKFLOW"] = github.Workflow
env["GITHUB_RUN_ID"] = github.RunID
env["GITHUB_RUN_NUMBER"] = github.RunNumber

View file

@ -100,14 +100,19 @@ func runStepExecutor(step step, stage stepStage, executor common.Executor) commo
actPath := rc.JobContainer.GetActPath()
outputFileCommand := path.Join("workflow", "outputcmd.txt")
stateFileCommand := path.Join("workflow", "statecmd.txt")
pathFileCommand := path.Join("workflow", "pathcmd.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)
_ = rc.JobContainer.Copy(actPath, &container.FileEntry{
Name: outputFileCommand,
Mode: 0666,
}, &container.FileEntry{
Name: stateFileCommand,
Mode: 0666,
}, &container.FileEntry{
Name: pathFileCommand,
Mode: 0666,
})(ctx)
err = executor(ctx)
@ -151,6 +156,10 @@ func runStepExecutor(step step, stage stepStage, executor common.Executor) commo
for k, v := range output {
rc.setOutput(ctx, map[string]string{"name": k}, v)
}
err = rc.UpdateExtraPath(ctx, path.Join(actPath, pathFileCommand))
if err != nil {
return err
}
if orgerr != nil {
return orgerr
}
@ -170,10 +179,6 @@ func setupEnv(ctx context.Context, step step) error {
if err != nil {
return err
}
err = rc.JobContainer.UpdateFromPath(step.getEnv())(ctx)
if err != nil {
return err
}
// merge step env last, since it should not be overwritten
mergeIntoMap(step.getEnv(), step.getStepModel().GetEnv())
@ -209,14 +214,6 @@ func mergeEnv(ctx context.Context, step step) {
mergeIntoMap(env, rc.GetEnv())
}
path := rc.JobContainer.GetPathVariableName()
if (*env)[path] == "" {
(*env)[path] = rc.JobContainer.DefaultPathVariable()
}
if rc.ExtraPath != nil && len(rc.ExtraPath) > 0 {
(*env)[path] = rc.JobContainer.JoinPathVariable(append(rc.ExtraPath, (*env)[path])...)
}
rc.withGithubEnv(ctx, step.getGithubContext(ctx), *env)
}

View file

@ -1,7 +1,9 @@
package runner
import (
"bytes"
"context"
"io"
"path/filepath"
"strings"
"testing"
@ -75,10 +77,6 @@ func TestStepActionLocalTest(t *testing.T) {
return nil
})
cm.On("UpdateFromPath", mock.AnythingOfType("*map[string]string")).Return(func(ctx context.Context) error {
return nil
})
cm.On("Copy", "/var/run/act", mock.AnythingOfType("[]*container.FileEntry")).Return(func(ctx context.Context) error {
return nil
})
@ -91,6 +89,8 @@ func TestStepActionLocalTest(t *testing.T) {
return nil
})
cm.On("GetContainerArchive", ctx, "/var/run/act/workflow/pathcmd.txt").Return(io.NopCloser(&bytes.Buffer{}), nil)
salm.On("runAction", sal, filepath.Clean("/tmp/path/to/action"), (*remoteAction)(nil)).Return(func(ctx context.Context) error {
return nil
})
@ -280,7 +280,6 @@ func TestStepActionLocalPost(t *testing.T) {
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 })
cm.On("UpdateFromPath", &sal.env).Return(func(ctx context.Context) error { return nil })
}
if tt.mocks.exec {
suffixMatcher := func(suffix string) interface{} {
@ -301,6 +300,8 @@ func TestStepActionLocalPost(t *testing.T) {
cm.On("UpdateFromEnv", "/var/run/act/workflow/outputcmd.txt", mock.AnythingOfType("*map[string]string")).Return(func(ctx context.Context) error {
return nil
})
cm.On("GetContainerArchive", ctx, "/var/run/act/workflow/pathcmd.txt").Return(io.NopCloser(&bytes.Buffer{}), nil)
}
err := sal.post()(ctx)

View file

@ -1,8 +1,10 @@
package runner
import (
"bytes"
"context"
"errors"
"io"
"strings"
"testing"
@ -166,7 +168,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 })
cm.On("UpdateFromPath", &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)
@ -185,6 +186,8 @@ func TestStepActionRemote(t *testing.T) {
cm.On("UpdateFromEnv", "/var/run/act/workflow/outputcmd.txt", mock.AnythingOfType("*map[string]string")).Return(func(ctx context.Context) error {
return nil
})
cm.On("GetContainerArchive", ctx, "/var/run/act/workflow/pathcmd.txt").Return(io.NopCloser(&bytes.Buffer{}), nil)
}
err := sar.pre()(ctx)
@ -592,7 +595,6 @@ func TestStepActionRemotePost(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 })
cm.On("UpdateFromPath", &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 })
@ -608,6 +610,8 @@ func TestStepActionRemotePost(t *testing.T) {
cm.On("UpdateFromEnv", "/var/run/act/workflow/outputcmd.txt", mock.AnythingOfType("*map[string]string")).Return(func(ctx context.Context) error {
return nil
})
cm.On("GetContainerArchive", ctx, "/var/run/act/workflow/pathcmd.txt").Return(io.NopCloser(&bytes.Buffer{}), nil)
}
err := sar.post()(ctx)

View file

@ -1,7 +1,9 @@
package runner
import (
"bytes"
"context"
"io"
"testing"
"github.com/nektos/act/pkg/container"
@ -63,10 +65,6 @@ func TestStepDockerMain(t *testing.T) {
return nil
})
cm.On("UpdateFromPath", 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
})
@ -99,6 +97,8 @@ func TestStepDockerMain(t *testing.T) {
return nil
})
cm.On("GetContainerArchive", ctx, "/var/run/act/workflow/pathcmd.txt").Return(io.NopCloser(&bytes.Buffer{}), nil)
err := sd.main()(ctx)
assert.Nil(t, err)

View file

@ -30,6 +30,7 @@ func (sr *stepRun) main() common.Executor {
return runStepExecutor(sr, stepStageMain, common.NewPipelineExecutor(
sr.setupShellCommandExecutor(),
func(ctx context.Context) error {
sr.getRunContext().ApplyExtraPath(&sr.env)
return sr.getRunContext().JobContainer.Exec(sr.cmd, sr.env, "", sr.Step.WorkingDirectory)(ctx)
},
))

View file

@ -1,7 +1,9 @@
package runner
import (
"bytes"
"context"
"io"
"testing"
"github.com/nektos/act/pkg/container"
@ -61,10 +63,6 @@ func TestStepRun(t *testing.T) {
return nil
})
cm.On("UpdateFromPath", mock.AnythingOfType("*map[string]string")).Return(func(ctx context.Context) error {
return nil
})
cm.On("Copy", "/var/run/act", mock.AnythingOfType("[]*container.FileEntry")).Return(func(ctx context.Context) error {
return nil
})
@ -79,6 +77,8 @@ func TestStepRun(t *testing.T) {
ctx := context.Background()
cm.On("GetContainerArchive", ctx, "/var/run/act/workflow/pathcmd.txt").Return(io.NopCloser(&bytes.Buffer{}), nil)
err := sr.main()(ctx)
assert.Nil(t, err)

View file

@ -134,7 +134,6 @@ func TestSetupEnv(t *testing.T) {
Env: map[string]string{
"RC_KEY": "rcvalue",
},
ExtraPath: []string{"/path/to/extra/file"},
JobContainer: cm,
}
step := &model.Step{
@ -142,9 +141,7 @@ func TestSetupEnv(t *testing.T) {
"STEP_WITH": "with-value",
},
}
env := map[string]string{
"PATH": "",
}
env := map[string]string{}
sm.On("getRunContext").Return(rc)
sm.On("getGithubContext").Return(rc)
@ -153,7 +150,6 @@ func TestSetupEnv(t *testing.T) {
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 })
cm.On("UpdateFromPath", &env).Return(func(ctx context.Context) error { return nil })
err := setupEnv(context.Background(), sm)
assert.Nil(t, err)
@ -184,7 +180,6 @@ func TestSetupEnv(t *testing.T) {
"GITHUB_GRAPHQL_URL": "https:///api/graphql",
"GITHUB_HEAD_REF": "",
"GITHUB_JOB": "",
"GITHUB_PATH": "/var/run/act/workflow/paths.txt",
"GITHUB_RETENTION_DAYS": "0",
"GITHUB_RUN_ID": "runId",
"GITHUB_RUN_NUMBER": "1",
@ -192,7 +187,6 @@ func TestSetupEnv(t *testing.T) {
"GITHUB_TOKEN": "",
"GITHUB_WORKFLOW": "",
"INPUT_STEP_WITH": "with-value",
"PATH": "/path/to/extra/file:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin",
"RC_KEY": "rcvalue",
"RUNNER_PERFLOG": "/dev/null",
"RUNNER_TRACKING_ID": "",