From 92eec3a526fd39aaf2048a7f6530a35d1de671d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Torbj=C3=B8rn=20Vatn?= Date: Thu, 6 May 2021 15:30:12 +0200 Subject: [PATCH] $GITHUB_PATH support (#566) * Regression in the .golangci.yml file * This looks like an even better fix to #451 The previous solution only prevented the `starting container process caused "exec: \"bash\"` error when someone added an "extra" path in the workflow using `::add-path` * Add support for >> $GITHUB_PATH * The newRunCommand has too high cyclomatic complexity * Add "linux/arm64" to new test * The cyclop linter was complaining so I extracted some funcs * Close some readers * Fix typo * fix: add missing composite function * Fix regress from merging * Keep the error messages as is * consolidate with master * Close the tar reader on defer * New way to get ContainerWorkdir * Remove arch from runner test * Separate the UpdateFromEnv and UpdateFromPath Co-authored-by: hackercat --- .golangci.yml | 2 +- cmd/root.go | 1 + pkg/container/docker_run.go | 33 +++ pkg/model/planner.go | 1 + pkg/runner/run_context.go | 5 + pkg/runner/runner_test.go | 1 + pkg/runner/step_context.go | 285 +++++++++++---------- pkg/runner/testdata/env-and-path/push.yaml | 41 +++ 8 files changed, 234 insertions(+), 135 deletions(-) create mode 100644 pkg/runner/testdata/env-and-path/push.yaml diff --git a/.golangci.yml b/.golangci.yml index 38fd451..746e63f 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -4,7 +4,7 @@ run: linters-settings: gocyclo: # minimal code complexity to report, 30 by default (but we recommend 10-20) - mi-complexity: 15 + min-complexity: 15 gocritic: disabled-checks: - ifElseChain diff --git a/cmd/root.go b/cmd/root.go index 13867b9..c3ab1f3 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -145,6 +145,7 @@ func readEnvs(path string, envs map[string]string) bool { return false } +//nolint:gocyclo func newRunCommand(ctx context.Context, input *Input) func(*cobra.Command, []string) error { return func(cmd *cobra.Command, args []string) error { log.Debugf("Loading environment from %s", input.Envfile()) diff --git a/pkg/container/docker_run.go b/pkg/container/docker_run.go index 3dab6af..3dddd02 100644 --- a/pkg/container/docker_run.go +++ b/pkg/container/docker_run.go @@ -70,6 +70,7 @@ type Container interface { Start(attach bool) common.Executor Exec(command []string, env map[string]string) common.Executor UpdateFromEnv(srcPath string, env *map[string]string) common.Executor + UpdateFromPath(env *map[string]string) common.Executor Remove() common.Executor } @@ -155,6 +156,10 @@ func (cr *containerReference) UpdateFromEnv(srcPath string, env *map[string]stri return cr.extractEnv(srcPath, env).IfNot(common.Dryrun) } +func (cr *containerReference) UpdateFromPath(env *map[string]string) common.Executor { + return cr.extractPath(env).IfNot(common.Dryrun) +} + func (cr *containerReference) Exec(command []string, env map[string]string) common.Executor { return common.NewPipelineExecutor( cr.connect(), @@ -342,6 +347,7 @@ func (cr *containerReference) extractEnv(srcPath string, env *map[string]string) if err != nil { return nil } + defer envTar.Close() reader := tar.NewReader(envTar) _, err = reader.Next() if err != nil && err != io.EOF { @@ -376,6 +382,31 @@ func (cr *containerReference) extractEnv(srcPath string, env *map[string]string) } } +func (cr *containerReference) extractPath(env *map[string]string) common.Executor { + localEnv := *env + return func(ctx context.Context) error { + pathTar, _, err := cr.cli.CopyFromContainer(ctx, cr.id, localEnv["GITHUB_PATH"]) + if err != nil { + return errors.WithStack(err) + } + defer pathTar.Close() + + reader := tar.NewReader(pathTar) + _, err = reader.Next() + if err != nil && err != io.EOF { + return errors.WithStack(err) + } + s := bufio.NewScanner(reader) + for s.Scan() { + line := s.Text() + localEnv["PATH"] = fmt.Sprintf("%s:%s", localEnv["PATH"], line) + } + + env = &localEnv + return nil + } +} + func (cr *containerReference) exec(cmd []string, env map[string]string) common.Executor { return func(ctx context.Context) error { logger := common.Logger(ctx) @@ -413,6 +444,8 @@ func (cr *containerReference) exec(cmd []string, env map[string]string) common.E if err != nil { return errors.WithStack(err) } + defer resp.Close() + var outWriter io.Writer outWriter = cr.input.Stdout if outWriter == nil { diff --git a/pkg/model/planner.go b/pkg/model/planner.go index 3c2a660..02ba41d 100644 --- a/pkg/model/planner.go +++ b/pkg/model/planner.go @@ -90,6 +90,7 @@ type WorkflowFiles struct { } // NewWorkflowPlanner will load a specific workflow, all workflows from a directory or all workflows from a directory and its subdirectories +// nolint: gocyclo func NewWorkflowPlanner(path string, noWorkflowRecurse bool) (WorkflowPlanner, error) { path, err := filepath.Abs(path) if err != nil { diff --git a/pkg/runner/run_context.go b/pkg/runner/run_context.go index 612f473..5b50a15 100755 --- a/pkg/runner/run_context.go +++ b/pkg/runner/run_context.go @@ -154,6 +154,10 @@ func (rc *RunContext) startJobContainer() common.Executor { Name: "workflow/envs.txt", Mode: 0644, Body: "", + }, &container.FileEntry{ + Name: "workflow/paths.txt", + Mode: 0644, + Body: "", }), )(ctx) } @@ -628,6 +632,7 @@ func (rc *RunContext) withGithubEnv(env map[string]string) map[string]string { github := rc.getGithubContext() env["CI"] = "true" env["GITHUB_ENV"] = "/tmp/workflow/envs.txt" + env["GITHUB_PATH"] = "/tmp/workflow/paths.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 89e1061..53fc500 100644 --- a/pkg/runner/runner_test.go +++ b/pkg/runner/runner_test.go @@ -107,6 +107,7 @@ func TestRunEvent(t *testing.T) { {"testdata", "uses-composite", "push", "", platforms, ""}, {"testdata", "issue-597", "push", "", platforms, ""}, {"testdata", "issue-598", "push", "", platforms, ""}, + {"testdata", "env-and-path", "push", "", platforms, ""}, // {"testdata", "issue-228", "push", "", platforms, ""}, // TODO [igni]: Remove this once everything passes // single test for different architecture: linux/arm64 diff --git a/pkg/runner/step_context.go b/pkg/runner/step_context.go index d8a4975..9e9983f 100755 --- a/pkg/runner/step_context.go +++ b/pkg/runner/step_context.go @@ -112,6 +112,7 @@ func (sc *StepContext) mergeEnv() map[string]string { env = mergeMaps(rc.GetEnv(), step.GetEnv()) } + env["PATH"] = `/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin` if (rc.ExtraPath != nil) && (len(rc.ExtraPath) > 0) { env["PATH"] = strings.Join(rc.ExtraPath, `:`) } @@ -134,6 +135,10 @@ func (sc *StepContext) setupEnv(ctx context.Context) (ExpressionEvaluator, error if err != nil { return nil, err } + err = rc.JobContainer.UpdateFromPath(&sc.Env)(ctx) + if err != nil { + return nil, err + } } evaluator := sc.NewExpressionEvaluator() sc.interpolateEnv(evaluator) @@ -390,14 +395,7 @@ func (sc *StepContext) runAction(actionDir string, actionPath string) common.Exe return func(ctx context.Context) error { action := sc.Action log.Debugf("About to run action %v", action) - for inputID, input := range action.Inputs { - envKey := regexp.MustCompile("[^A-Z0-9-]").ReplaceAllString(strings.ToUpper(inputID), "_") - envKey = fmt.Sprintf("INPUT_%s", envKey) - if _, ok := sc.Env[envKey]; !ok { - sc.Env[envKey] = rc.ExprEval.Interpolate(input.Default) - } - } - + sc.populateEnvsFromInput(action, rc) actionLocation := "" if actionPath != "" { actionLocation = path.Join(actionDir, actionPath) @@ -434,133 +432,9 @@ func (sc *StepContext) runAction(actionDir string, actionPath string) common.Exe log.Debugf("executing remote job container: %s", containerArgs) return rc.execJobContainer(containerArgs, sc.Env)(ctx) case model.ActionRunsUsingDocker: - var prepImage common.Executor - var image string - if strings.HasPrefix(action.Runs.Image, "docker://") { - image = strings.TrimPrefix(action.Runs.Image, "docker://") - } else { - image = fmt.Sprintf("%s:%s", regexp.MustCompile("[^a-zA-Z0-9]").ReplaceAllString(actionName, "-"), "latest") - image = fmt.Sprintf("act-%s", strings.TrimLeft(image, "-")) - image = strings.ToLower(image) - contextDir := filepath.Join(actionDir, actionPath, action.Runs.Main) - - anyArchExists, err := container.ImageExistsLocally(ctx, image, "any") - if err != nil { - return err - } - - correctArchExists, err := container.ImageExistsLocally(ctx, image, rc.Config.ContainerArchitecture) - if err != nil { - return err - } - - if anyArchExists && !correctArchExists { - wasRemoved, err := container.RemoveImage(ctx, image, true, true) - if err != nil { - return err - } - if !wasRemoved { - return fmt.Errorf("failed to remove image '%s'", image) - } - } - - if !correctArchExists { - log.Debugf("image '%s' for architecture '%s' will be built from context '%s", image, rc.Config.ContainerArchitecture, contextDir) - prepImage = container.NewDockerBuildExecutor(container.NewDockerBuildExecutorInput{ - ContextDir: contextDir, - ImageTag: image, - Platform: rc.Config.ContainerArchitecture, - }) - } else { - log.Debugf("image '%s' for architecture '%s' already exists", image, rc.Config.ContainerArchitecture) - } - } - - cmd, err := shellquote.Split(step.With["args"]) - if err != nil { - return err - } - if len(cmd) == 0 { - cmd = action.Runs.Args - } - entrypoint := strings.Fields(step.With["entrypoint"]) - if len(entrypoint) == 0 { - entrypoint = action.Runs.Entrypoint - } - stepContainer := sc.newStepContainer(ctx, image, cmd, entrypoint) - return common.NewPipelineExecutor( - prepImage, - stepContainer.Pull(rc.Config.ForcePull), - stepContainer.Remove().IfBool(!rc.Config.ReuseContainers), - stepContainer.Create(), - stepContainer.Start(true), - ).Finally( - stepContainer.Remove().IfBool(!rc.Config.ReuseContainers), - )(ctx) + return sc.execAsDocker(ctx, action, actionName, actionDir, actionPath, rc, step) case model.ActionRunsUsingComposite: - err := maybeCopyToActionDir() - if err != nil { - return err - } - for outputName, output := range action.Outputs { - re := regexp.MustCompile(`\${{ steps\.([a-zA-Z_][a-zA-Z0-9_-]+)\.outputs\.([a-zA-Z_][a-zA-Z0-9_-]+) }}`) - matches := re.FindStringSubmatch(output.Value) - if len(matches) > 2 { - if sc.RunContext.OutputMappings == nil { - sc.RunContext.OutputMappings = make(map[MappableOutput]MappableOutput) - } - - k := MappableOutput{StepID: matches[1], OutputName: matches[2]} - v := MappableOutput{StepID: step.ID, OutputName: outputName} - sc.RunContext.OutputMappings[k] = v - } - } - - var executors []common.Executor - stepID := 0 - for _, compositeStep := range action.Runs.Steps { - stepClone := compositeStep - // Take a copy of the run context structure (rc is a pointer) - // Then take the address of the new structure - rcCloneStr := *rc - rcClone := &rcCloneStr - if stepClone.ID == "" { - stepClone.ID = fmt.Sprintf("composite-%d", stepID) - stepID++ - } - rcClone.CurrentStep = stepClone.ID - - if err := compositeStep.Validate(); err != nil { - return err - } - - // Setup the outputs for the composite steps - if _, ok := rcClone.StepResults[stepClone.ID]; !ok { - rcClone.StepResults[stepClone.ID] = &stepResult{ - Success: true, - Outputs: make(map[string]string), - } - } - - stepClone.Run = strings.ReplaceAll(stepClone.Run, "${{ github.action_path }}", filepath.Join(containerActionDir, actionName)) - - stepContext := StepContext{ - RunContext: rcClone, - Step: &stepClone, - Env: mergeMaps(sc.Env, stepClone.Env), - } - - // Interpolate the outer inputs into the composite step with items - exprEval := sc.NewExpressionEvaluator() - for k, v := range stepContext.Step.With { - if strings.Contains(v, "inputs") { - stepContext.Step.With[k] = exprEval.Interpolate(v) - } - } - - executors = append(executors, stepContext.Executor()) - } - return common.NewPipelineExecutor(executors...)(ctx) + return sc.execAsComposite(ctx, step, actionDir, rc, containerActionDir, actionName, actionPath, action, maybeCopyToActionDir) default: return fmt.Errorf(fmt.Sprintf("The runs.using key must be one of: %v, got %s", []string{ model.ActionRunsUsingDocker, @@ -571,6 +445,149 @@ func (sc *StepContext) runAction(actionDir string, actionPath string) common.Exe } } +func (sc *StepContext) execAsDocker(ctx context.Context, action *model.Action, actionName string, actionDir string, actionPath string, rc *RunContext, step *model.Step) error { + var prepImage common.Executor + var image string + if strings.HasPrefix(action.Runs.Image, "docker://") { + image = strings.TrimPrefix(action.Runs.Image, "docker://") + } else { + image = fmt.Sprintf("%s:%s", regexp.MustCompile("[^a-zA-Z0-9]").ReplaceAllString(actionName, "-"), "latest") + image = fmt.Sprintf("act-%s", strings.TrimLeft(image, "-")) + image = strings.ToLower(image) + contextDir := filepath.Join(actionDir, actionPath, action.Runs.Main) + + anyArchExists, err := container.ImageExistsLocally(ctx, image, "any") + if err != nil { + return err + } + + correctArchExists, err := container.ImageExistsLocally(ctx, image, rc.Config.ContainerArchitecture) + if err != nil { + return err + } + + if anyArchExists && !correctArchExists { + wasRemoved, err := container.RemoveImage(ctx, image, true, true) + if err != nil { + return err + } + if !wasRemoved { + return fmt.Errorf("failed to remove image '%s'", image) + } + } + + if !correctArchExists { + log.Debugf("image '%s' for architecture '%s' will be built from context '%s", image, rc.Config.ContainerArchitecture, contextDir) + prepImage = container.NewDockerBuildExecutor(container.NewDockerBuildExecutorInput{ + ContextDir: contextDir, + ImageTag: image, + Platform: rc.Config.ContainerArchitecture, + }) + } else { + log.Debugf("image '%s' for architecture '%s' already exists", image, rc.Config.ContainerArchitecture) + } + } + + cmd, err := shellquote.Split(step.With["args"]) + if err != nil { + return err + } + if len(cmd) == 0 { + cmd = action.Runs.Args + } + entrypoint := strings.Fields(step.With["entrypoint"]) + if len(entrypoint) == 0 { + entrypoint = action.Runs.Entrypoint + } + stepContainer := sc.newStepContainer(ctx, image, cmd, entrypoint) + return common.NewPipelineExecutor( + prepImage, + stepContainer.Pull(rc.Config.ForcePull), + stepContainer.Remove().IfBool(!rc.Config.ReuseContainers), + stepContainer.Create(), + stepContainer.Start(true), + ).Finally( + stepContainer.Remove().IfBool(!rc.Config.ReuseContainers), + )(ctx) +} + +func (sc *StepContext) execAsComposite(ctx context.Context, step *model.Step, _ string, rc *RunContext, containerActionDir string, actionName string, _ string, action *model.Action, maybeCopyToActionDir func() error) error { + err := maybeCopyToActionDir() + + if err != nil { + return err + } + for outputName, output := range action.Outputs { + re := regexp.MustCompile(`\${{ steps\.([a-zA-Z_][a-zA-Z0-9_-]+)\.outputs\.([a-zA-Z_][a-zA-Z0-9_-]+) }}`) + matches := re.FindStringSubmatch(output.Value) + if len(matches) > 2 { + if sc.RunContext.OutputMappings == nil { + sc.RunContext.OutputMappings = make(map[MappableOutput]MappableOutput) + } + + k := MappableOutput{StepID: matches[1], OutputName: matches[2]} + v := MappableOutput{StepID: step.ID, OutputName: outputName} + sc.RunContext.OutputMappings[k] = v + } + } + + executors := make([]common.Executor, 0, len(action.Runs.Steps)) + stepID := 0 + for _, compositeStep := range action.Runs.Steps { + stepClone := compositeStep + // Take a copy of the run context structure (rc is a pointer) + // Then take the address of the new structure + rcCloneStr := *rc + rcClone := &rcCloneStr + if stepClone.ID == "" { + stepClone.ID = fmt.Sprintf("composite-%d", stepID) + stepID++ + } + rcClone.CurrentStep = stepClone.ID + + if err := compositeStep.Validate(); err != nil { + return err + } + + // Setup the outputs for the composite steps + if _, ok := rcClone.StepResults[stepClone.ID]; !ok { + rcClone.StepResults[stepClone.ID] = &stepResult{ + Success: true, + Outputs: make(map[string]string), + } + } + + stepClone.Run = strings.ReplaceAll(stepClone.Run, "${{ github.action_path }}", filepath.Join(containerActionDir, actionName)) + + stepContext := StepContext{ + RunContext: rcClone, + Step: &stepClone, + Env: mergeMaps(sc.Env, stepClone.Env), + } + + // Interpolate the outer inputs into the composite step with items + exprEval := sc.NewExpressionEvaluator() + for k, v := range stepContext.Step.With { + if strings.Contains(v, "inputs") { + stepContext.Step.With[k] = exprEval.Interpolate(v) + } + } + + executors = append(executors, stepContext.Executor()) + } + return common.NewPipelineExecutor(executors...)(ctx) +} + +func (sc *StepContext) populateEnvsFromInput(action *model.Action, rc *RunContext) { + for inputID, input := range action.Inputs { + envKey := regexp.MustCompile("[^A-Z0-9-]").ReplaceAllString(strings.ToUpper(inputID), "_") + envKey = fmt.Sprintf("INPUT_%s", envKey) + if _, ok := sc.Env[envKey]; !ok { + sc.Env[envKey] = rc.ExprEval.Interpolate(input.Default) + } + } +} + type remoteAction struct { URL string Org string diff --git a/pkg/runner/testdata/env-and-path/push.yaml b/pkg/runner/testdata/env-and-path/push.yaml new file mode 100644 index 0000000..7972924 --- /dev/null +++ b/pkg/runner/testdata/env-and-path/push.yaml @@ -0,0 +1,41 @@ +name: env-and-path +on: push + +jobs: + build: + runs-on: ubuntu-latest + steps: + - name: "Append to $GITHUB_PATH" + run: | + echo "$HOME/someFolder" >> $GITHUB_PATH + - name: "Append some more to $GITHUB_PATH" + run: | + echo "$HOME/someOtherFolder" >> $GITHUB_PATH + - name: "Check PATH" + run: | + if [[ ! "${PATH}" =~ .*"$HOME/"someFolder.*"$HOME/"someOtherFolder ]]; then + echo "${PATH} doesn't match .*someFolder.*someOtherFolder" + exit 1 + fi + - name: "Write single line env to $GITHUB_ENV" + run: | + echo "KEY=value" >> $GITHUB_ENV + - name: "Check single line env" + run: | + if [[ "${KEY}" != "value" ]]; then + echo "${KEY} dosen't == 'value'" + exit 1 + fi + - name: "Write multiline env to $GITHUB_ENV" + run: | + echo 'KEY2<> $GITHUB_ENV + echo value2 >> $GITHUB_ENV + echo 'EOF' >> $GITHUB_ENV + - name: "Check multiline line env" + run: | + if [[ "${KEY2}" != "value2" ]]; then + echo "${KEY2} dosen't == 'value'" + exit 1 + fi + +