From 54711cc7c6c2f03a2b1b8d0be0dec8455c7226ed Mon Sep 17 00:00:00 2001 From: Robert-Furth Date: Tue, 20 Jul 2021 23:03:31 +0000 Subject: [PATCH 1/9] Stop job containers once they finish running a job --- pkg/runner/run_context.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg/runner/run_context.go b/pkg/runner/run_context.go index c1431d68e18..9fab7b66625 100755 --- a/pkg/runner/run_context.go +++ b/pkg/runner/run_context.go @@ -223,7 +223,9 @@ func (rc *RunContext) Executor() common.Executor { } steps = append(steps, rc.stopJobContainer()) - return common.NewPipelineExecutor(steps...).If(rc.isEnabled) + return common.NewPipelineExecutor(steps...). + Finally(rc.stopJobContainer()). + If(rc.isEnabled) } func (rc *RunContext) newStepExecutor(step *model.Step) common.Executor { From 20215fcb6976bd89a129dfc730ca6b348523b5ca Mon Sep 17 00:00:00 2001 From: Robert-Furth Date: Tue, 20 Jul 2021 23:11:11 +0000 Subject: [PATCH 2/9] Give each workflow run a UUID Each workflow run has a UUID generated for it when it is started. This UUID is added to the container name of each job in a run. This means that multiple instances of act can run on the same workflow without their containers' names conflicting. --- cmd/root.go | 5 +++++ go.mod | 1 + go.sum | 2 ++ pkg/runner/run_context.go | 21 ++++----------------- pkg/runner/runner.go | 1 + 5 files changed, 13 insertions(+), 17 deletions(-) diff --git a/cmd/root.go b/cmd/root.go index 214dbadc9e1..36797ee449e 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -11,6 +11,7 @@ import ( "github.com/AlecAivazis/survey/v2" "github.com/andreaskoch/go-fswatch" + "github.com/google/uuid" "github.com/joho/godotenv" "github.com/mitchellh/go-homedir" gitignore "github.com/sabhiram/go-gitignore" @@ -250,6 +251,9 @@ func newRunCommand(ctx context.Context, input *Input) func(*cobra.Command, []str } } + // Generate a new UUID + runid := strings.ReplaceAll(uuid.New().String(), "-", "") + // run the plan config := &runner.Config{ Actor: input.actor, @@ -274,6 +278,7 @@ func newRunCommand(ctx context.Context, input *Input) func(*cobra.Command, []str ContainerCapAdd: input.containerCapAdd, ContainerCapDrop: input.containerCapDrop, AutoRemove: input.autoRemove, + RunID: runid, } r, err := runner.New(config) if err != nil { diff --git a/go.mod b/go.mod index 6f6662ae279..7fe200a2845 100644 --- a/go.mod +++ b/go.mod @@ -14,6 +14,7 @@ require ( github.com/go-git/go-git/v5 v5.2.0 github.com/go-ini/ini v1.62.0 github.com/golang/protobuf v1.4.3 // indirect + github.com/google/uuid v1.3.0 // indirect github.com/gopherjs/gopherjs v0.0.0-20181103185306-d547d1d9531e // indirect github.com/imdario/mergo v0.3.11 // indirect github.com/joho/godotenv v1.3.0 diff --git a/go.sum b/go.sum index d8ffbdd7b0d..7715e08ab59 100644 --- a/go.sum +++ b/go.sum @@ -466,6 +466,8 @@ github.com/google/subcommands v1.0.1/go.mod h1:ZjhPrFU+Olkh9WazFPsl27BQ4UPiG37m3 github.com/google/uuid v1.0.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= github.com/google/uuid v1.1.1/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= github.com/google/uuid v1.1.2/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= +github.com/google/uuid v1.3.0 h1:t6JiXgmwXMjEs8VusXIJk2BXHsn+wx8BZdTaoZ5fu7I= +github.com/google/uuid v1.3.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= github.com/google/wire v0.3.0/go.mod h1:i1DMg/Lu8Sz5yYl25iOdmc5CT5qusaa+zmRWs16741s= github.com/google/wire v0.4.0/go.mod h1:ngWDr9Qvq3yZA10YrxfyGELY/AFWGVpy9c1LTRi1EoU= github.com/googleapis/gax-go v2.0.0+incompatible/go.mod h1:SFVmujtThgffbyetf+mdk2eWhX2bMyUtNHzFKcPA9HY= diff --git a/pkg/runner/run_context.go b/pkg/runner/run_context.go index 9fab7b66625..f42b43efbe7 100755 --- a/pkg/runner/run_context.go +++ b/pkg/runner/run_context.go @@ -62,7 +62,8 @@ func (rc *RunContext) GetEnv() map[string]string { } func (rc *RunContext) jobContainerName() string { - return createContainerName("act", rc.String()) + name := createContainerName("act", rc.Config.RunID, rc.String()) + return name } // Returns the binds and mounts for the container, resolving paths as appopriate @@ -404,22 +405,8 @@ func mergeMaps(maps ...map[string]string) map[string]string { func createContainerName(parts ...string) string { name := make([]string, 0) pattern := regexp.MustCompile("[^a-zA-Z0-9]") - partLen := (30 / len(parts)) - 1 - for i, part := range parts { - if i == len(parts)-1 { - name = append(name, pattern.ReplaceAllString(part, "-")) - } else { - // If any part has a '-' on the end it is likely part of a matrix job. - // Let's preserve the number to prevent clashes in container names. - re := regexp.MustCompile("-[0-9]+$") - num := re.FindStringSubmatch(part) - if len(num) > 0 { - name = append(name, trimToLen(pattern.ReplaceAllString(part, "-"), partLen-len(num[0]))) - name = append(name, num[0]) - } else { - name = append(name, trimToLen(pattern.ReplaceAllString(part, "-"), partLen)) - } - } + for _, part := range parts { + name = append(name, pattern.ReplaceAllString(part, "-")) } return strings.ReplaceAll(strings.Trim(strings.Join(name, "-"), "-"), "--", "-") } diff --git a/pkg/runner/runner.go b/pkg/runner/runner.go index f34f9e80a33..0064ea88d19 100644 --- a/pkg/runner/runner.go +++ b/pkg/runner/runner.go @@ -43,6 +43,7 @@ type Config struct { ContainerCapAdd []string // list of kernel capabilities to add to the containers ContainerCapDrop []string // list of kernel capabilities to remove from the containers AutoRemove bool // controls if the container is automatically removed upon workflow completion + RunID string // A unique ID for the current workflow run } // Resolves the equivalent host path inside the container From abbe7b5e4c6f6acb61368f89bd18bbc6bcb31162 Mon Sep 17 00:00:00 2001 From: Robert-Furth Date: Tue, 20 Jul 2021 23:17:59 +0000 Subject: [PATCH 3/9] Do not update remote actions once they are cached --- pkg/common/git.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/pkg/common/git.go b/pkg/common/git.go index c87906aeaaa..6b4793f47aa 100644 --- a/pkg/common/git.go +++ b/pkg/common/git.go @@ -285,6 +285,11 @@ func NewGitCloneExecutor(input NewGitCloneExecutorInput) Executor { defer cloneLock.Unlock() refName := plumbing.ReferenceName(fmt.Sprintf("refs/heads/%s", input.Ref)) + + if _, err := git.PlainOpen(input.Dir); err == nil { + return nil + } + r, err := CloneIfRequired(ctx, refName, input, logger) if err != nil { return err From 0672119ebc5ce5588a420dc658a328fb4f131fa4 Mon Sep 17 00:00:00 2001 From: Robert-Furth <44911447+Robert-Furth@users.noreply.github.com> Date: Mon, 23 Aug 2021 12:16:36 -0700 Subject: [PATCH 4/9] Artifact support (#1) * Add "/artifacts" as a mount * Only remove job container if the job succeeds or if `--rm` given Previously, the container would be removed no matter what. Now, users can leave off the `--rm` flag to leave containers running if they fail, which can aid in debugging. --- pkg/runner/run_context.go | 3 ++- pkg/runner/runner.go | 15 ++++++++++++++- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/pkg/runner/run_context.go b/pkg/runner/run_context.go index f42b43efbe7..9f2c3493630 100755 --- a/pkg/runner/run_context.go +++ b/pkg/runner/run_context.go @@ -80,6 +80,7 @@ func (rc *RunContext) GetBindsAndMounts() ([]string, map[string]string) { mounts := map[string]string{ "act-toolcache": "/toolcache", + "act-artifacts-" + rc.Config.RunID: "/artifacts", } if rc.Config.BindWorkdir { @@ -225,7 +226,7 @@ func (rc *RunContext) Executor() common.Executor { steps = append(steps, rc.stopJobContainer()) return common.NewPipelineExecutor(steps...). - Finally(rc.stopJobContainer()). + Finally(rc.stopJobContainer().IfBool(rc.Config.AutoRemove)). If(rc.isEnabled) } diff --git a/pkg/runner/runner.go b/pkg/runner/runner.go index 0064ea88d19..75ffa7293bd 100644 --- a/pkg/runner/runner.go +++ b/pkg/runner/runner.go @@ -10,6 +10,7 @@ import ( "strings" "github.com/nektos/act/pkg/common" + "github.com/nektos/act/pkg/container" "github.com/nektos/act/pkg/model" log "github.com/sirupsen/logrus" ) @@ -150,7 +151,19 @@ func (runner *runnerImpl) NewPlanExecutor(plan *model.Plan) common.Executor { pipeline = append(pipeline, common.NewParallelExecutor(stageExecutor...)) } - return common.NewPipelineExecutor(pipeline...) + return common.NewPipelineExecutor(pipeline...). + Finally(func(ctx context.Context) error { + if !runner.config.AutoRemove { + return nil + } + artifactVolume := "act-artifacts-" + runner.config.RunID + log.Infof("Cleaning up artifacts volume \"%s\"", artifactVolume) + err := container.NewDockerVolumeRemoveExecutor(artifactVolume, false)(ctx) + if err != nil { + log.Errorf("Error cleaning up volume \"%s\": %v", artifactVolume, err) + } + return err + }) } func (runner *runnerImpl) newRunContext(run *model.Run, matrix map[string]interface{}) *RunContext { From 59c7adf07de4482280b6104280652edac1ade47c Mon Sep 17 00:00:00 2001 From: Robert-Furth <44911447+Robert-Furth@users.noreply.github.com> Date: Fri, 27 Aug 2021 13:19:32 -0700 Subject: [PATCH 5/9] Put scripts in /tmp instead of the working directory (#2) --- pkg/runner/step_context.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/runner/step_context.go b/pkg/runner/step_context.go index baa88a56e6a..a423c8eacff 100644 --- a/pkg/runner/step_context.go +++ b/pkg/runner/step_context.go @@ -227,7 +227,7 @@ func (sc *StepContext) setupShellCommand() common.Executor { run = runPrepend + "\n" + run + "\n" + runAppend log.Debugf("Wrote command '%s' to '%s'", run, scriptName) - scriptPath := fmt.Sprintf("%s/%s", rc.Config.ContainerWorkdir(), scriptName) + scriptPath := fmt.Sprintf("/tmp/%s", scriptName) if step.Shell == "" { step.Shell = rc.Run.Job().Defaults.Run.Shell @@ -252,7 +252,7 @@ func (sc *StepContext) setupShellCommand() common.Executor { sc.Cmd = finalCMD - return rc.JobContainer.Copy(rc.Config.ContainerWorkdir(), &container.FileEntry{ + return rc.JobContainer.Copy("/tmp/", &container.FileEntry{ Name: scriptName, Mode: 0755, Body: script.String(), From e83b46004cb581e0f11f7212fc8dc67d8288ee8a Mon Sep 17 00:00:00 2001 From: Robert-Furth Date: Mon, 6 Sep 2021 18:50:04 +0000 Subject: [PATCH 6/9] Make matrix 'include' section work as intented The intended behavior is for 'include' directives to modify existing entries if their common keys match, and to only add new entries if they cannot modify any existing entry. Previously, 'include' directives had no effect if they had no keys in common with an existing entry, and always added new entries if they did. --- pkg/model/workflow.go | 41 +++++++++++++++++++++++++++-------------- 1 file changed, 27 insertions(+), 14 deletions(-) diff --git a/pkg/model/workflow.go b/pkg/model/workflow.go index 7e21c5f35fb..d91c09105d4 100644 --- a/pkg/model/workflow.go +++ b/pkg/model/workflow.go @@ -226,21 +226,11 @@ func (j *Job) GetMatrixes() []map[string]interface{} { case []interface{}: for _, i := range t { i := i.(map[string]interface{}) - for k := range i { - if _, ok := m[k]; ok { - includes = append(includes, i) - break - } - } + includes = append(includes, i) } case interface{}: v := v.(map[string]interface{}) - for k := range v { - if _, ok := m[k]; ok { - includes = append(includes, v) - break - } - } + includes = append(includes, v) } } delete(m, "include") @@ -271,8 +261,20 @@ func (j *Job) GetMatrixes() []map[string]interface{} { matrixes = append(matrixes, matrix) } for _, include := range includes { - log.Debugf("Adding include '%v'", include) - matrixes = append(matrixes, include) + notAdded := true + for _, matrix := range matrixProduct { + if hasKeysInCommon(matrix, include) && commonKeysMatch(matrix, include) { + log.Debugf("Applying include '%v' to matrix entry '%v'", include, matrix) + for k, v := range include { + matrix[k] = v + } + notAdded = false + } + } + if notAdded { + log.Debugf("Appending include '%v' to matrix", include) + matrixes = append(matrixes, include) + } } } else { matrixes = append(matrixes, make(map[string]interface{})) @@ -292,6 +294,17 @@ func commonKeysMatch(a map[string]interface{}, b map[string]interface{}) bool { return true } +func hasKeysInCommon(a map[string]interface{}, b map[string]interface{}) bool { + for aKey := range a { + for bKey := range b { + if aKey == bKey { + return true + } + } + } + return false +} + // ContainerSpec is the specification of the container to use for the job type ContainerSpec struct { Image string `yaml:"image"` From 9549f7ca34c8695a796404887cacefb1ab8a43f9 Mon Sep 17 00:00:00 2001 From: Robert-Furth Date: Wed, 8 Sep 2021 22:25:49 +0000 Subject: [PATCH 7/9] Evaluate checkout ref when determining if a checkout is local When a workflow tries to run actions/checkout, the runner needs to know if the workflow wants to check out the current repository or a remote repository. It does this by comparing the provided repository name and ref with those of the current branch. This commit expands expressions in those fields before the comparison, so workflows can call actions/checkout with parameters like "ref: {{ SOME_VARIABLE }}". --- pkg/runner/run_context.go | 11 +++++++---- pkg/runner/step_context.go | 2 +- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/pkg/runner/run_context.go b/pkg/runner/run_context.go index aa42416446e..b37c9c73682 100755 --- a/pkg/runner/run_context.go +++ b/pkg/runner/run_context.go @@ -575,7 +575,7 @@ func (rc *RunContext) getGithubContext() *githubContext { return ghc } -func (ghc *githubContext) isLocalCheckout(step *model.Step) bool { +func (ghc *githubContext) isLocalCheckout(step *model.Step, ee ExpressionEvaluator) bool { if step.Type() == model.StepTypeInvalid { // This will be errored out by the executor later, we need this here to avoid a null panic though return false @@ -595,8 +595,11 @@ func (ghc *githubContext) isLocalCheckout(step *model.Step) bool { if repository, ok := step.With["repository"]; ok && repository != ghc.Repository { return false } - if repository, ok := step.With["ref"]; ok && repository != ghc.Ref { - return false + if ref, ok := step.With["ref"]; ok { + interp, ok2 := ee.InterpolateWithStringCheck(ref) + if ok2 && interp != ghc.Ref { + return false + } } return true } @@ -711,7 +714,7 @@ func (rc *RunContext) withGithubEnv(env map[string]string) map[string]string { func (rc *RunContext) localCheckoutPath() (string, bool) { ghContext := rc.getGithubContext() for _, step := range rc.Run.Job().Steps { - if ghContext.isLocalCheckout(step) { + if ghContext.isLocalCheckout(step, rc.ExprEval) { return step.With["path"], true } } diff --git a/pkg/runner/step_context.go b/pkg/runner/step_context.go index a423c8eacff..eb2447a9410 100644 --- a/pkg/runner/step_context.go +++ b/pkg/runner/step_context.go @@ -92,7 +92,7 @@ func (sc *StepContext) Executor() common.Executor { remoteAction.URL = rc.Config.GitHubInstance github := rc.getGithubContext() - if remoteAction.IsCheckout() && github.isLocalCheckout(step) { + if remoteAction.IsCheckout() && github.isLocalCheckout(step, rc.ExprEval) { return func(ctx context.Context) error { common.Logger(ctx).Debugf("Skipping local actions/checkout because workdir was already copied") return nil From 3ef342080505ca5aa9087c3399b96c2e7a1089ff Mon Sep 17 00:00:00 2001 From: Robert-Furth Date: Thu, 9 Sep 2021 21:43:52 +0000 Subject: [PATCH 8/9] Security fix: do not bind /var/run/docker.sock --- pkg/runner/run_context.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/pkg/runner/run_context.go b/pkg/runner/run_context.go index b37c9c73682..4b15ed9248c 100755 --- a/pkg/runner/run_context.go +++ b/pkg/runner/run_context.go @@ -75,7 +75,10 @@ func (rc *RunContext) GetBindsAndMounts() ([]string, map[string]string) { } binds := []string{ - fmt.Sprintf("%s:%s", rc.Config.ContainerDaemonSocket, "/var/run/docker.sock"), + // ***DO NOT BIND '/var/run/docker.sock'!*** If you do, then you're giving the runner image, running code + // scraped from the internet, access to docker ON THE HOST MACHINE. + // (I've already come across a workflow that runs 'docker image prune -af'. That was a nasty shock. --Robert) + // fmt.Sprintf("%s:%s", rc.Config.ContainerDaemonSocket, "/var/run/docker.sock"), } mounts := map[string]string{ From 4592601e7e21296ff513f2a70335f3771de9c402 Mon Sep 17 00:00:00 2001 From: Robert-Furth Date: Tue, 14 Sep 2021 18:19:53 +0000 Subject: [PATCH 9/9] Use worker pool approach for parallel jobs --- pkg/common/executor.go | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/pkg/common/executor.go b/pkg/common/executor.go index cd92a6c54d1..5e9fe501572 100644 --- a/pkg/common/executor.go +++ b/pkg/common/executor.go @@ -90,20 +90,30 @@ func NewErrorExecutor(err error) Executor { } } +func parallelExecutorWorker(executorChan <-chan Executor, errChan chan error, ctx *context.Context) { + for executor := range executorChan { + err := executor.ChannelError(errChan)(*ctx) + if err != nil { + log.Fatal(err) + } + } +} + // NewParallelExecutor creates a new executor from a parallel of other executors func NewParallelExecutor(executors ...Executor) Executor { return func(ctx context.Context) error { + maxJobs := 4 errChan := make(chan error) + executorChan := make(chan Executor, len(executors)) + for i := 0; i < maxJobs; i++ { + go parallelExecutorWorker(executorChan, errChan, &ctx) + } for _, executor := range executors { e := executor - go func() { - err := e.ChannelError(errChan)(ctx) - if err != nil { - log.Fatal(err) - } - }() + executorChan <- e } + close(executorChan) // Executor waits all executors to cleanup these resources. var firstErr error