diff --git a/act/container/container_types.go b/act/container/container_types.go index 84aeb0c6..c0294ffb 100644 --- a/act/container/container_types.go +++ b/act/container/container_types.go @@ -152,6 +152,8 @@ type NewDockerBuildExecutorInput struct { ContextDir string Dockerfile string BuildContext io.Reader + Dockerignore []byte // applied to BuildContext as the docker CLI applies it + Stdout io.Writer ImageTag string Platform string BuildArgs map[string]*string diff --git a/act/container/docker_build.go b/act/container/docker_build.go index 1efe797a..f97836b9 100644 --- a/act/container/docker_build.go +++ b/act/container/docker_build.go @@ -8,10 +8,13 @@ package container import ( "archive/tar" + "bytes" "context" + "fmt" "io" "io/fs" "os" + "path" "path/filepath" "runtime" "strings" @@ -63,7 +66,7 @@ func NewDockerBuildExecutor(input NewDockerBuildExecutorInput) common.Executor { } var buildContext io.ReadCloser if input.BuildContext != nil { - buildContext = io.NopCloser(input.BuildContext) + buildContext, err = filterBuildContext(input.BuildContext, input.Dockerignore, input.Dockerfile) } else { buildContext, err = createBuildContext(ctx, input.ContextDir, input.Dockerfile) } @@ -78,10 +81,97 @@ func NewDockerBuildExecutor(input NewDockerBuildExecutorInput) common.Executor { if err != nil { return err } - return logDockerResponse(logger, resp.Body, false) + err = logDockerResponse(logger, resp.Body, func(format string, args ...any) { fmt.Fprintf(input.Stdout, format, args...) }) + common.FlushWriter(input.Stdout) + return err } } +func filterBuildContext(buildContext io.Reader, dockerignore []byte, relDockerfile string) (io.ReadCloser, error) { + excludes, err := ignorefile.ReadAll(bytes.NewReader(dockerignore)) + if err != nil || len(excludes) == 0 { + return io.NopCloser(buildContext), err + } + pm, err := patternmatcher.New(excludes) + if err != nil { + return nil, err + } + reader, writer := io.Pipe() + go func() { + writer.CloseWithError(writeFilteredBuildContext(writer, buildContext, pm, path.Clean(filepath.ToSlash(relDockerfile)))) + }() + return reader, nil +} + +func writeFilteredBuildContext(w io.Writer, buildContext io.Reader, pm *patternmatcher.PatternMatcher, relDockerfile string) error { + spool, err := os.CreateTemp("", "gitea-runner-build-") // excluded content, for a kept hardlink to it + if err != nil { + return err + } + defer os.Remove(spool.Name()) + defer spool.Close() + + reader := tar.NewReader(buildContext) + writer := tar.NewWriter(w) + buffer := make([]byte, 32*1024) + excluded := map[string]*excludedBuildContextEntry{} + for { + header, err := reader.Next() + if err == io.EOF { + return writer.Close() + } + if err != nil { + return err + } + name := path.Clean(header.Name) + var target *excludedBuildContextEntry + if header.Typeflag == tar.TypeLink { + target = excluded[path.Clean(header.Linkname)] + } + if name != ".dockerignore" && name != relDockerfile { + skip, err := pm.MatchesOrParentMatches(name) + if err != nil { + return err + } + if skip { + if target != nil { + excluded[name] = target + } else if header.Typeflag != tar.TypeDir { + offset, err := spool.Seek(0, io.SeekCurrent) + if err != nil { + return err + } + excluded[name] = &excludedBuildContextEntry{header: header, offset: offset} + if _, err := io.CopyBuffer(spool, reader, buffer); err != nil { + return err + } + } + continue + } + } + var content io.Reader = reader + if target != nil && target.keptName != "" { + header.Linkname = target.keptName + } else if target != nil { + header.Typeflag, header.Linkname, header.Size = target.header.Typeflag, target.header.Linkname, target.header.Size + content = io.NewSectionReader(spool, target.offset, header.Size) + target.keptName = header.Name + } + if err := writer.WriteHeader(header); err != nil { + return err + } + if _, err := io.CopyBuffer(writer, content, buffer); err != nil { + return err + } + } +} + +type excludedBuildContextEntry struct { + header *tar.Header + offset int64 + keptName string +} + func createBuildContext(ctx context.Context, contextDir, relDockerfile string) (io.ReadCloser, error) { common.Logger(ctx).Debugf("Creating archive for build context dir '%s' with relative dockerfile '%s'", contextDir, relDockerfile) diff --git a/act/container/docker_build_test.go b/act/container/docker_build_test.go index f7762d8e..4e1086ee 100644 --- a/act/container/docker_build_test.go +++ b/act/container/docker_build_test.go @@ -7,6 +7,8 @@ package container import ( "archive/tar" + "bytes" + "fmt" "io" "net/http" "net/http/httptest" @@ -35,7 +37,7 @@ func TestDockerBuildLabelsImageWithItsTagAndReportsDaemonError(t *testing.T) { t.Setenv("DOCKER_AUTH_CONFIG", "") require.ErrorContains(t, NewDockerBuildExecutor(NewDockerBuildExecutorInput{ - ImageTag: "action:test", Dockerfile: "Dockerfile", BuildContext: strings.NewReader(""), + ImageTag: "action:test", Dockerfile: "Dockerfile", BuildContext: strings.NewReader(""), Stdout: io.Discard, })(t.Context()), "build rejected") require.JSONEq(t, `{"com.gitea.runner.image":"action:test"}`, labels) } @@ -72,3 +74,56 @@ func TestCreateBuildContextFollowsDockerignoreAndSymlinkedRootButKeepsDockerfile require.Equal(t, []string{".dockerignore", "a.txt", "build/keep.txt", "empty/", "Dockerfile"}, names, contextDir) } } + +func TestFilterBuildContextAppliesDockerignoreAndMaterializesHardlinksToExcludedFiles(t *testing.T) { + const dockerignore = "*.log\nbuild\n!build/keep.txt\nActionfile\n.dockerignore\n" + for _, prefix := range []string{"", "./"} { + var archive bytes.Buffer + writer := tar.NewWriter(&archive) + for _, entry := range []struct { + name, body, link string + typeflag byte + }{ + {"Actionfile", "FROM scratch", "", tar.TypeReg}, + {"output.log", "log", "", tar.TypeReg}, + {"build/", "", "", tar.TypeDir}, + {"build/drop.txt", "drop", "", tar.TypeReg}, + {"build/keep.txt", "keep", "", tar.TypeReg}, + {"entrypoint", "", prefix + "output.log", tar.TypeLink}, + {"other-entrypoint", "", prefix + "output.log", tar.TypeLink}, + {".dockerignore", dockerignore, "", tar.TypeReg}, + } { + require.NoError(t, writer.WriteHeader(&tar.Header{Name: prefix + entry.name, Linkname: entry.link, Typeflag: entry.typeflag, Size: int64(len(entry.body)), Mode: 0o644})) + _, err := io.WriteString(writer, entry.body) + require.NoError(t, err) + } + require.NoError(t, writer.Close()) + + unfiltered, err := filterBuildContext(bytes.NewReader(archive.Bytes()), nil, "Actionfile") + require.NoError(t, err) + passedThrough, err := io.ReadAll(unfiltered) + require.NoError(t, err) + require.Equal(t, archive.Bytes(), passedThrough) + + filtered, err := filterBuildContext(bytes.NewReader(archive.Bytes()), []byte(dockerignore), "Actionfile") + require.NoError(t, err) + var entries []string + for reader := tar.NewReader(filtered); ; { + header, err := reader.Next() + if err == io.EOF { + break + } + require.NoError(t, err) + body, err := io.ReadAll(reader) + require.NoError(t, err) + entries = append(entries, fmt.Sprintf("%s %c %s %s", strings.TrimPrefix(header.Name, prefix), header.Typeflag, strings.TrimPrefix(header.Linkname, prefix), body)) + } + require.Equal(t, []string{ + "Actionfile 0 FROM scratch", + "build/keep.txt 0 keep", + "entrypoint 0 log", + "other-entrypoint 1 entrypoint ", + ".dockerignore 0 " + dockerignore, + }, entries, prefix) + } +} diff --git a/act/container/docker_images_test.go b/act/container/docker_images_test.go index b219f931..0c57e5e2 100644 --- a/act/container/docker_images_test.go +++ b/act/container/docker_images_test.go @@ -38,7 +38,7 @@ func loadScratchImage(t *testing.T, platform string) string { response, err := cli.ImageLoad(t.Context(), &archive) require.NoError(t, err) defer response.Close() - require.NoError(t, logDockerResponse(common.Logger(t.Context()), response, false)) + require.NoError(t, logDockerResponse(common.Logger(t.Context()), response, common.Logger(t.Context()).Debugf)) t.Cleanup(func() { _, _ = cli.ImageRemove(context.WithoutCancel(t.Context()), tag, mobyclient.ImageRemoveOptions{Force: true}) }) diff --git a/act/container/docker_logger.go b/act/container/docker_logger.go index 81bfaa0d..dbcd964f 100644 --- a/act/container/docker_logger.go +++ b/act/container/docker_logger.go @@ -8,6 +8,7 @@ package container import ( "bufio" + "cmp" "encoding/json/v2" "errors" "io" @@ -26,10 +27,7 @@ type dockerMessage struct { Progress string `json:"progress"` } -func logDockerResponse(logger *logging.Logger, dockerResponse io.ReadCloser, isError bool) error { - if dockerResponse == nil { - return nil - } +func logDockerResponse(logger *logging.Logger, dockerResponse io.ReadCloser, log func(string, ...any)) error { defer dockerResponse.Close() scanner := bufio.NewScanner(dockerResponse) @@ -38,41 +36,28 @@ func logDockerResponse(logger *logging.Logger, dockerResponse io.ReadCloser, isE line := scanner.Bytes() var msg dockerMessage if err := json.Unmarshal(line, &msg); err != nil { - writeLog(logger, false, "Unable to unmarshal line [%s] ==> %v", string(line), err) + logger.Debugf("Unable to unmarshal line [%s] ==> %v", string(line), err) continue } - if msg.Error != "" { - writeLog(logger, isError, "%s", msg.Error) - return errors.New(msg.Error) - } - - if msg.ErrorDetail.Message != "" { - writeLog(logger, isError, "%s", msg.ErrorDetail.Message) - return errors.New(msg.ErrorDetail.Message) + if message := cmp.Or(msg.Error, msg.ErrorDetail.Message); message != "" { + log("%s", message) + return errors.New(message) } switch { case msg.Status != "": if msg.Progress != "" { - writeLog(logger, isError, "%s :: %s :: %s\n", msg.Status, msg.ID, msg.Progress) + log("%s :: %s :: %s\n", msg.Status, msg.ID, msg.Progress) } else { - writeLog(logger, isError, "%s :: %s\n", msg.Status, msg.ID) + log("%s :: %s\n", msg.Status, msg.ID) } case msg.Stream != "": - writeLog(logger, isError, "%s", msg.Stream) + log("%s", msg.Stream) default: - writeLog(logger, false, "Unable to handle line: %s", string(line)) + logger.Debugf("Unable to handle line: %s", string(line)) } } return scanner.Err() } - -func writeLog(logger *logging.Logger, isError bool, format string, args ...any) { - if isError { - logger.Errorf(format, args...) - } else { - logger.Debugf(format, args...) - } -} diff --git a/act/container/docker_pull.go b/act/container/docker_pull.go index 87678a5a..00f81644 100644 --- a/act/container/docker_pull.go +++ b/act/container/docker_pull.go @@ -61,11 +61,10 @@ func NewDockerPullExecutor(input NewDockerPullExecutorInput) common.Executor { // stream rather than on the call itself, so both have to be checked pullOnce := func(opts client.ImagePullOptions) error { reader, err := cli.ImagePull(ctx, imageRef, opts) - streamErr := logDockerResponse(logger, reader, err != nil) if err != nil { return err } - return streamErr + return logDockerResponse(logger, reader, logger.Debugf) } err = pullOnce(imagePullOptions) diff --git a/act/container/docker_pull_test.go b/act/container/docker_pull_test.go index 55e73f52..18c1c38c 100644 --- a/act/container/docker_pull_test.go +++ b/act/container/docker_pull_test.go @@ -95,15 +95,16 @@ func TestIsPinnedImage(t *testing.T) { // The pull path reports a failure the daemon sent mid-stream, so it must carry the reason // whichever of the two shapes the daemon used. func TestLogDockerResponseError(t *testing.T) { - logger := logging.New(logtest.New()) + logger, hook := logtest.NewLogger() streamErr := func(line string) error { - return logDockerResponse(logger, io.NopCloser(strings.NewReader(line)), false) + return logDockerResponse(logger, io.NopCloser(strings.NewReader(line)), logger.Infof) } require.EqualError(t, streamErr(`{"error":"toomanyrequests: rate limit exceeded"}`), "toomanyrequests: rate limit exceeded") require.EqualError(t, streamErr(`{"errorDetail":{"message":"unexpected EOF"}}`), "unexpected EOF") require.NoError(t, streamErr(`{"status":"Downloading"}`)) require.NoError(t, streamErr(`{"stream":"`+strings.Repeat("x", 70000)+`"}`)) + assert.Equal(t, logging.LevelInfo, hook.LastRecord().Level) require.ErrorIs(t, logDockerResponse(logger, io.NopCloser(io.MultiReader( strings.NewReader(`{"status":"Downloading"}`+"\n"), iotest.ErrReader(io.ErrUnexpectedEOF), - )), false), io.ErrUnexpectedEOF) + )), logger.Infof), io.ErrUnexpectedEOF) } diff --git a/act/container/host_environment.go b/act/container/host_environment.go index fd40ffc4..8a88e473 100644 --- a/act/container/host_environment.go +++ b/act/container/host_environment.go @@ -283,6 +283,7 @@ func (e *HostEnvironment) exec(ctx context.Context, command []string, cmdline st // wait, so an orphan holding cmd's stdout pipe cannot hang cmd.Wait(). treeKill := process.NewTreeKill(cmd) treeKill.KeepLeftovers = !tracked + treeKill.GracefulCancel = true var ppty *os.File var tty *os.File diff --git a/act/runner/action.go b/act/runner/action.go index 05781287..6436841c 100644 --- a/act/runner/action.go +++ b/act/runner/action.go @@ -346,8 +346,10 @@ func execAsDocker(ctx context.Context, step actionStep, actionName, actionDir, b ImageTag: image, Platform: rc.Config.ContainerArchitecture, BuildArgs: rc.proxyBuildArgs(), + Stdout: common.NewLineWriter(rawLogLine(ctx)), } if localAction { + input.Dockerignore, _ = readSingleFileFromContainerArchive(ctx, rc.JobContainer, contextDir+"/.dockerignore", 1<<20) buildContext, err := rc.JobContainer.GetContainerArchive(ctx, contextDir+"/.") if err != nil { return err @@ -587,7 +589,8 @@ func shouldRunPreStep(step actionStep) common.Conditional { func hasPreStep(step actionStep) common.Conditional { return func(ctx context.Context) bool { action := step.getActionModel() - return action.Runs.Using.IsComposite() || + // a pre stage evaluates `with` before earlier steps run, so only a composite nesting actions gets one + return (action.Runs.Using.IsComposite() && slices.ContainsFunc(action.Runs.Steps, func(compositeStep model.Step) bool { return compositeStep.Uses != "" })) || (action.Runs.Using.IsNode() && action.Runs.Pre != "") || (action.Runs.Using.IsDocker() && diff --git a/act/runner/action_test.go b/act/runner/action_test.go index faa9903a..706282e1 100644 --- a/act/runner/action_test.go +++ b/act/runner/action_test.go @@ -751,12 +751,15 @@ func TestExecAsDockerStageEntrypoint(t *testing.T) { } } -func TestDockerActionHasPreAndPostStep(t *testing.T) { +func TestActionHasPreAndPostStep(t *testing.T) { newStep := func(runs model.ActionRuns) actionStep { return &stepActionRemote{action: &model.Action{Runs: runs}} } ctx := t.Context() + assert.False(t, hasPreStep(newStep(model.ActionRuns{Using: "composite", Steps: []model.Step{{Run: "true"}}}))(ctx)) + assert.True(t, hasPreStep(newStep(model.ActionRuns{Using: "composite", Steps: []model.Step{{Run: "true"}, {Uses: "actions/cache@v4"}}}))(ctx)) + assert.False(t, hasPreStep(newStep(model.ActionRuns{Using: "docker", Image: "Dockerfile"}))(ctx)) assert.False(t, hasPostStep(newStep(model.ActionRuns{Using: "docker", Image: "Dockerfile"}))(ctx)) diff --git a/act/runner/job_hooks.go b/act/runner/job_hooks.go index dc7df6d0..c95c1291 100644 --- a/act/runner/job_hooks.go +++ b/act/runner/job_hooks.go @@ -44,7 +44,11 @@ func (rc *RunContext) runJobHook(ctx context.Context, hookPath, name string) err rc.CurrentStep = "" defer func() { rc.CurrentStep = previousStep }() - cmd, shell := hookCommand(hookPath) + env := rc.mergeEnv(rc.caEnv, rc.containerSpec.Env, rc.GetEnv()) + rc.withGithubEnv(ctx, rc.getGithubContext(ctx), env) + rc.ApplyExtraPath(ctx, &env) + + cmd, shell := hookCommand(hookPath, func(preferred, fallback string) string { return rc.shellOrFallback(ctx, env, preferred, fallback) }) rawLogger := common.Logger(ctx).With(rawOutputField, true) rawLogger.Infof("A %s hook has been configured by the runner administrator", name) rawLogger.Infof("::group::Run '%s'", EscapeCommandData(hookPath)) @@ -53,10 +57,6 @@ func (rc *RunContext) runJobHook(ctx context.Context, hookPath, name string) err } rawLogger.Infof("::endgroup::") - env := rc.mergeEnv(rc.caEnv, rc.containerSpec.Env, rc.GetEnv()) - rc.withGithubEnv(ctx, rc.getGithubContext(ctx), env) - rc.ApplyExtraPath(ctx, &env) - err := rc.setupHookFileCommands(ctx, env) if err == nil { err = rc.runWithCommands(ctx, env, func(ctx context.Context) error { @@ -104,12 +104,14 @@ func (rc *RunContext) processHookFileCommands(ctx context.Context) error { // gives `run:` steps — a hook sets its own. See docs/adrs/1751-runner-job-hooks.md there. // The second return value is how the invocation is shown in the log, empty when the file is // executed directly. -func hookCommand(hookPath string) (cmd []string, shell string) { +func hookCommand(hookPath string, shellOrFallback func(preferred, fallback string) string) (cmd []string, shell string) { switch strings.ToLower(path.Ext(hookPath)) { case ".sh": - return []string{"bash", "-e", hookPath}, "bash -e {0}" + shellName := shellOrFallback("bash", "sh") + return []string{shellName, "-e", hookPath}, shellName + " -e {0}" case ".ps1": - return []string{"pwsh", "-command", ". '" + expressionStringEscape(hookPath) + "'"}, `pwsh -command ". '{0}'"` + shellName := shellOrFallback("pwsh", "powershell") + return []string{shellName, "-command", ". '" + expressionStringEscape(hookPath) + "'"}, shellName + ` -command ". '{0}'"` default: return []string{hookPath}, "" } diff --git a/act/runner/job_hooks_test.go b/act/runner/job_hooks_test.go index 56ab5ab5..e9c626ac 100644 --- a/act/runner/job_hooks_test.go +++ b/act/runner/job_hooks_test.go @@ -225,7 +225,7 @@ func TestRunJobHook(t *testing.T) { rc, ctx := newHookRunContext(jobContainer, &Config{JobStartedHook: "/hooks/started.sh"}) require.ErrorContains(t, rc.runJobStartedHook(ctx), "boom") - assert.Nil(t, jobContainer.cmd) + assert.NotContains(t, jobContainer.cmd, "/hooks/started.sh") assert.NotContains(t, rc.Env, "STALE") }) @@ -242,17 +242,26 @@ func TestRunJobHook(t *testing.T) { // actions/runner deliberately runs a hook without the flags it gives `run:` steps, and an // executable without a known extension speaks for itself through its shebang. func TestHookCommand(t *testing.T) { - for hookPath, want := range map[string]struct { - cmd []string - shell string + for _, tc := range []struct { + hookPath string + hasShell bool + cmd []string + shell string }{ - "/hooks/started.sh": {[]string{"bash", "-e", "/hooks/started.sh"}, "bash -e {0}"}, - "/hooks/started.PS1": {[]string{"pwsh", "-command", ". '/hooks/started.PS1'"}, `pwsh -command ". '{0}'"`}, - "/hooks/started": {[]string{"/hooks/started"}, ""}, - `C:\hooks\O'Brien\started.ps1`: {[]string{"pwsh", "-command", `. 'C:\hooks\O''Brien\started.ps1'`}, `pwsh -command ". '{0}'"`}, + {"/hooks/started.sh", true, []string{"bash", "-e", "/hooks/started.sh"}, "bash -e {0}"}, + {"/hooks/started.sh", false, []string{"sh", "-e", "/hooks/started.sh"}, "sh -e {0}"}, + {"/hooks/started.PS1", true, []string{"pwsh", "-command", ". '/hooks/started.PS1'"}, `pwsh -command ". '{0}'"`}, + {"/hooks/started.PS1", false, []string{"powershell", "-command", ". '/hooks/started.PS1'"}, `powershell -command ". '{0}'"`}, + {"/hooks/started", false, []string{"/hooks/started"}, ""}, + {`C:\hooks\O'Brien\started.ps1`, true, []string{"pwsh", "-command", `. 'C:\hooks\O''Brien\started.ps1'`}, `pwsh -command ". '{0}'"`}, } { - cmd, shell := hookCommand(hookPath) - assert.Equal(t, want.cmd, cmd, hookPath) - assert.Equal(t, want.shell, shell, hookPath) + cmd, shell := hookCommand(tc.hookPath, func(preferred, fallback string) string { + if tc.hasShell { + return preferred + } + return fallback + }) + assert.Equal(t, tc.cmd, cmd, tc.hookPath) + assert.Equal(t, tc.shell, shell, tc.hookPath) } } diff --git a/act/runner/run_context.go b/act/runner/run_context.go index d95f4d61..8575b72c 100644 --- a/act/runner/run_context.go +++ b/act/runner/run_context.go @@ -655,6 +655,11 @@ func (rc *RunContext) startJobContainer() common.Executor { rc.serviceContainers = append(rc.serviceContainers, &serviceContainer{name: serviceID, image: input.Image, container: newContainer(input)}) } + exposedPorts, portBindings, err := nat.ParsePortSpecs(rc.containerSpec.Ports) + if err != nil { + return fmt.Errorf("failed to parse container ports: %w", err) + } + ext := container.LinuxContainerEnvironmentExtensions{} containerInput := &container.NewContainerInput{ Entrypoint: []string{"sleep", strconv.FormatInt(int64(rc.Config.ContainerMaxLifetime.Round(time.Second).Seconds()), 10)}, @@ -666,6 +671,8 @@ func (rc *RunContext) startJobContainer() common.Executor { Env: envList, NetworkMode: networkName, NetworkAliases: []string{rc.Name}, + ExposedPorts: exposedPorts, + PortBindings: portBindings, Stdout: logWriter, Stderr: logWriter, Privileged: rc.Config.Privileged, @@ -1635,7 +1642,7 @@ func (rc *RunContext) getRunnerContext(ctx context.Context) map[string]any { runnerContext["name"] = rc.Config.RunnerName runnerContext["environment"] = "self-hosted" runnerContext["workspace"] = parentDir(rc.githubWorkspace()) - if rc.Config.RunnerDebug() { + if rc.topLevelRunContext().Config.RunnerDebug() { // a composite's config has no secrets runnerContext["debug"] = "1" } return runnerContext @@ -1679,7 +1686,9 @@ func (rc *RunContext) getGithubContext(ctx context.Context) *model.GithubContext Workspace: rc.githubWorkspace(), } if rc.JobContainer != nil { - ghc.EventPath = rc.JobContainer.GetActPath() + "/workflow/event.json" + actPath := rc.JobContainer.GetActPath() + ghc.EventPath = actPath + "/workflow/event.json" + ghc.Env, ghc.Path = container.JoinPath(actPath, envFileCommand), container.JoinPath(actPath, pathFileCommand) } ghc.RunnerTrackingID = rc.runnerTrackingID() @@ -1687,6 +1696,7 @@ func (rc *RunContext) getGithubContext(ctx context.Context) *model.GithubContext if preset := rc.Config.PresetGitHubContext; preset != nil { merged := *preset merged.Workflow, merged.Job, merged.Workspace, merged.EventPath = ghc.Workflow, ghc.Job, ghc.Workspace, ghc.EventPath + merged.Env, merged.Path = ghc.Env, ghc.Path merged.Action, merged.ActionPath, merged.ActionRepository, merged.ActionRef = ghc.Action, ghc.ActionPath, ghc.ActionRepository, ghc.ActionRef merged.RunnerPerflog, merged.RunnerTrackingID = ghc.RunnerPerflog, ghc.RunnerTrackingID *ghc = merged @@ -1841,7 +1851,7 @@ func (rc *RunContext) withGithubEnv(ctx context.Context, github *model.GithubCon if workspace := parentDir(github.Workspace); workspace != "" { env["RUNNER_WORKSPACE"] = workspace } - if rc.Config.RunnerDebug() { + if rc.topLevelRunContext().Config.RunnerDebug() { env["RUNNER_DEBUG"] = "1" } diff --git a/act/runner/run_context_test.go b/act/runner/run_context_test.go index d5aa046a..a672c4e3 100644 --- a/act/runner/run_context_test.go +++ b/act/runner/run_context_test.go @@ -342,6 +342,7 @@ jobs: container: image: node:20 credentials: {username: "${{ env.REG_USER }}", password: pass} + ports: ["8080:80"] services: redis: image: redis:latest @@ -351,6 +352,7 @@ jobs: `, &Config{Vars: map[string]string{"REG_USER": "user", "TEXT": "${{ 'literal' }}"}}) redis, job = inputs[0], inputs[1] require.Equal(t, "user", job.Username) + require.Equal(t, "8080", job.PortBindings["80/tcp"][0].HostPort) require.Equal(t, "user", redis.Username) require.Equal(t, []string{"TEXT=${{ 'literal' }}"}, redis.Env) require.Equal(t, "--volumes-from "+job.Name, redis.WorkflowOptions) @@ -1781,23 +1783,33 @@ func TestRunContextGetRunnerContext(t *testing.T) { assert.NotContains(t, runnerContext, "debug") }) - t.Run("reports debug when step debugging is on", func(t *testing.T) { + t.Run("reports debug from the secret, else the variable, also in composites", func(t *testing.T) { rc := createRunsOnRunContext(t, "ubuntu-latest") rc.Config.Secrets = map[string]string{"ACTIONS_STEP_DEBUG": "true"} - assert.Equal(t, "1", rc.getRunnerContext(ctx)["debug"]) + assert.Equal(t, "1", (&RunContext{Parent: rc, Config: &Config{}}).getRunnerContext(ctx)["debug"]) + + rc.Config.Secrets = map[string]string{} + rc.Config.Vars = map[string]string{"ACTIONS_STEP_DEBUG": "true"} + assert.Equal(t, "1", rc.getRunnerContext(ctx)["debug"]) + + rc.Config.Secrets = map[string]string{"ACTIONS_STEP_DEBUG": "false"} + assert.NotContains(t, rc.getRunnerContext(ctx), "debug") }) t.Run("keeps the execution environment values", func(t *testing.T) { rc := createRunsOnRunContext(t, "ubuntu-latest") rc.Config.Workdir = "/host/work/owner/repo" - rc.JobContainer = &container.HostEnvironment{Workdir: rc.Config.Workdir, Path: "/container/work/owner/repo", TmpDir: "/tmp/act", ToolCache: "/tmp/tool_cache"} + rc.JobContainer = &container.HostEnvironment{Workdir: rc.Config.Workdir, Path: "/container/work/owner/repo", TmpDir: "/tmp/act", ToolCache: "/tmp/tool_cache", ActPath: "/tmp/act-files"} runnerContext := rc.getRunnerContext(ctx) assert.Equal(t, "/tmp/act", runnerContext["temp"]) assert.Equal(t, "/tmp/tool_cache", runnerContext["tool_cache"]) assert.Equal(t, "/container/work/owner", runnerContext["workspace"]) - assert.Equal(t, "/container/work/owner/repo", rc.getGithubContext(ctx).Workspace) + githubContext := rc.getGithubContext(ctx) + assert.Equal(t, "/container/work/owner/repo", githubContext.Workspace) + assert.Equal(t, "/tmp/act-files/workflow/envs.txt", githubContext.Env) + assert.Equal(t, "/tmp/act-files/workflow/pathcmd.txt", githubContext.Path) assert.NotEmpty(t, runnerContext["os"]) }) } diff --git a/act/runner/runner.go b/act/runner/runner.go index db9e8c80..1a1cd261 100644 --- a/act/runner/runner.go +++ b/act/runner/runner.go @@ -100,7 +100,11 @@ type Config struct { // RunnerDebug reports whether debug logging is on, exposed as `runner.debug` and RUNNER_DEBUG. func (c Config) RunnerDebug() bool { - return c.Secrets["ACTIONS_STEP_DEBUG"] == "true" || c.Env["ACTIONS_STEP_DEBUG"] == "true" + stepDebug, ok := c.Secrets["ACTIONS_STEP_DEBUG"] + if !ok { + stepDebug = c.Vars["ACTIONS_STEP_DEBUG"] + } + return stepDebug == "true" || c.Env["ACTIONS_STEP_DEBUG"] == "true" } // GetToken: Adapt to Gitea diff --git a/act/runner/step.go b/act/runner/step.go index 86cb52b4..8581ee7f 100644 --- a/act/runner/step.go +++ b/act/runner/step.go @@ -21,6 +21,11 @@ import ( "gitea.dev/actionslib/pkg/model" ) +const ( + pathFileCommand = "workflow/pathcmd.txt" + envFileCommand = "workflow/envs.txt" +) + type step interface { pre() common.Executor main() common.Executor @@ -149,10 +154,7 @@ func runStepExecutor(step step, stage stepStage, executor common.Executor) commo stateFileCommand := path.Join("workflow", "statecmd.txt") (*step.getEnv())["GITHUB_STATE"] = container.JoinPath(actPath, stateFileCommand) - pathFileCommand := path.Join("workflow", "pathcmd.txt") (*step.getEnv())["GITHUB_PATH"] = container.JoinPath(actPath, pathFileCommand) - - envFileCommand := path.Join("workflow", "envs.txt") (*step.getEnv())["GITHUB_ENV"] = container.JoinPath(actPath, envFileCommand) // Per-step summary file. Composite sub-steps share the outer job step's index diff --git a/act/runner/step_run.go b/act/runner/step_run.go index 2dc65fc4..67f81516 100644 --- a/act/runner/step_run.go +++ b/act/runner/step_run.go @@ -10,7 +10,6 @@ import ( "fmt" "io" "maps" - "runtime" "slices" "strings" @@ -358,29 +357,29 @@ func (sr *stepRun) setupShell(ctx context.Context, eval *expressionEvaluator) (b implicitShell := step.Shell == "" if implicitShell { - shellWithFallback := []string{"bash", "sh"} + preferred, fallback := "bash", "sh" + if rc.isWindowsHost() { + preferred, fallback = "pwsh", "powershell" // Don't use bash on windows by default, if not using a docker container + } env := maps.Clone(sr.env) rc.ApplyExtraPath(ctx, &env) - if _, ok := rc.JobContainer.(*container.HostEnvironment); ok { - // Don't use bash on windows by default, if not using a docker container - if runtime.GOOS == "windows" { - shellWithFallback = []string{"pwsh", "powershell"} - } - step.Shell = shellWithFallback[0] - _, err := lookpath.LookPath2(shellWithFallback[0], env) - if err != nil { - step.Shell = shellWithFallback[1] - } - } else { - step.Shell = shellWithFallback[0] - if !rc.containerHasBash(ctx, env) { - step.Shell = shellWithFallback[1] - } - } + step.Shell = rc.shellOrFallback(ctx, env, preferred, fallback) } return implicitShell, nil } +// shellOrFallback probes a container only for bash. +func (rc *RunContext) shellOrFallback(ctx context.Context, env map[string]string, preferred, fallback string) string { + if _, ok := rc.JobContainer.(*container.HostEnvironment); ok { + if _, err := lookpath.LookPath2(preferred, env); err != nil { + return fallback + } + } else if preferred == "bash" && !rc.containerHasBash(ctx, env) { + return fallback + } + return preferred +} + // containerHasBash probes once per job, else every implicit-shell step pays for an exec. func (rc *RunContext) containerHasBash(ctx context.Context, env map[string]string) bool { top := rc.topLevelRunContext() diff --git a/docs/host-mode.md b/docs/host-mode.md index 8c8d194d..c33ac80e 100644 --- a/docs/host-mode.md +++ b/docs/host-mode.md @@ -12,7 +12,7 @@ steps: - Set the `RUNNER_TRACKING_ID` override in the step's `env` before it starts. Any value different from the job's ID opts out, including an empty string. On Linux and macOS a script can also set it before starting a process. - Opted-out processes and descendants that inherit the override survive normal job cleanup and expired output waits. Detach helper output because the runner closes leftover output pipes after a bounded wait. -- Cancelling or timing out an active step still terminates its process tree. +- Cancelling or timing out an active step still terminates its process tree, on Unix with SIGINT, then SIGTERM after 7.5 seconds and SIGKILL after 2.5 more. - The override does not preserve workspace or temporary files. Store persistent helper files outside those directories and arrange the helper's eventual shutdown. This follows [actions/runner's tracking ID and orphan cleanup](https://github.com/actions/runner/blob/main/src/Runner.Worker/JobExtension.cs). Container jobs follow their container's lifecycle. diff --git a/docs/job-hooks.md b/docs/job-hooks.md index 09281072..8aa31fac 100644 --- a/docs/job-hooks.md +++ b/docs/job-hooks.md @@ -28,8 +28,8 @@ The script is run according to its extension: | Extension | Command | | --- | --- | -| `.sh` | `bash -e ` | -| `.ps1` | `pwsh -command . ''` | +| `.sh` | `bash -e `, or `sh -e ` without bash | +| `.ps1` | `pwsh -command . ''`, or `powershell` on a host without pwsh | | anything else | the file itself, which needs its own shebang and executable bit | As on GitHub, the shell flags applied to `run:` steps are **not** applied to a hook — set `pipefail` or anything else you want inside the script. diff --git a/internal/pkg/process/killer_plan9.go b/internal/pkg/process/killer_plan9.go index 1249208e..8395bb18 100644 --- a/internal/pkg/process/killer_plan9.go +++ b/internal/pkg/process/killer_plan9.go @@ -26,4 +26,6 @@ func (k *Killer) Kill() error { return k.p.Kill() } +func (k *Killer) cancel(_process *os.Process, _graceful bool) error { return k.Kill() } + func (k *Killer) Close() error { return nil } diff --git a/internal/pkg/process/killer_unix.go b/internal/pkg/process/killer_unix.go index ef808415..5407132b 100644 --- a/internal/pkg/process/killer_unix.go +++ b/internal/pkg/process/killer_unix.go @@ -9,6 +9,7 @@ import ( "errors" "os" "syscall" + "time" ) // Killer terminates a started process together with its whole process group, @@ -52,5 +53,37 @@ func (k *Killer) Kill() error { return nil } +func (k *Killer) cancel(process *os.Process, graceful bool) error { + if graceful && k.pgid > 0 { + for _, stage := range []struct { + signal syscall.Signal + delay time.Duration + }{ + {syscall.SIGINT, 7500 * time.Millisecond}, + {syscall.SIGTERM, 2500 * time.Millisecond}, + } { + if process.Signal(syscall.Signal(0)) != nil || syscall.Kill(-k.pgid, stage.signal) != nil || waitForExit(process, stage.delay) { + break + } + } + } + return k.Kill() +} + +func waitForExit(process *os.Process, delay time.Duration) bool { + timer := time.NewTimer(delay) + defer timer.Stop() + ticker := time.NewTicker(10 * time.Millisecond) + defer ticker.Stop() + for !errors.Is(process.Signal(syscall.Signal(0)), os.ErrProcessDone) { + select { + case <-timer.C: + return false + case <-ticker.C: + } + } + return true +} + // Close is a no-op on Unix; there is no job handle to release. func (k *Killer) Close() error { return nil } diff --git a/internal/pkg/process/killer_unix_test.go b/internal/pkg/process/killer_unix_test.go index 0df771e8..9a05d468 100644 --- a/internal/pkg/process/killer_unix_test.go +++ b/internal/pkg/process/killer_unix_test.go @@ -6,9 +6,11 @@ package process import ( + "context" "fmt" "os" "os/exec" + "path/filepath" "strings" "syscall" "testing" @@ -95,3 +97,34 @@ func TestTreeKillCancelBeforeCaptureKillsTree(t *testing.T) { require.Error(t, cmd.Wait()) require.Eventually(t, func() bool { return !processAlive(childPID) }, 5*time.Second, 10*time.Millisecond) } + +func TestTreeKillCancelRunsInterruptTrap(t *testing.T) { + ctx, cancel := context.WithTimeout(t.Context(), 5*time.Second) + defer cancel() + marker := filepath.Join(t.TempDir(), "interrupted") + cmd := exec.CommandContext(ctx, "/bin/sh", "-c", `trap 'echo interrupted > "$1"; exit 0' INT; echo ready; read value`, "sh", marker) + cmd.SysProcAttr = SysProcAttr("", false) + input, err := cmd.StdinPipe() + require.NoError(t, err) + defer input.Close() + output, err := cmd.StdoutPipe() + require.NoError(t, err) + treeKill := NewTreeKill(cmd) + treeKill.GracefulCancel = true + require.NoError(t, cmd.Start()) + killer, err := treeKill.Capture(cmd.Process) + require.NoError(t, err) + defer killer.Close() + t.Cleanup(func() { + _ = killer.Kill() + _ = cmd.Wait() + }) + var ready string + _, err = fmt.Fscanln(output, &ready) + require.NoError(t, err) + require.Equal(t, "ready", ready) + + cancel() + require.ErrorIs(t, treeKill.Wait(ctx, cmd), context.Canceled) + require.FileExists(t, marker) +} diff --git a/internal/pkg/process/killer_windows.go b/internal/pkg/process/killer_windows.go index e9b643b6..7ed4020f 100644 --- a/internal/pkg/process/killer_windows.go +++ b/internal/pkg/process/killer_windows.go @@ -68,6 +68,8 @@ func (k *Killer) Kill() error { return windows.TerminateJobObject(k.job, 1) } +func (k *Killer) cancel(_process *os.Process, _graceful bool) error { return k.Kill() } + // Close releases the job handle. It does not terminate the processes. func (k *Killer) Close() error { if k == nil { diff --git a/internal/pkg/process/treekill.go b/internal/pkg/process/treekill.go index 5cbe1d10..85ef3476 100644 --- a/internal/pkg/process/treekill.go +++ b/internal/pkg/process/treekill.go @@ -32,8 +32,9 @@ const treeKillWaitDelay = 10 * time.Second // Callers still set cmd.SysProcAttr (via SysProcAttr) themselves, because the // value differs between the plain and PTY execution paths. type TreeKill struct { - killer atomic.Pointer[Killer] - KeepLeftovers bool + killer atomic.Pointer[Killer] + KeepLeftovers bool + GracefulCancel bool } // NewTreeKill sets cmd.Cancel, cmd.WaitDelay and CREATE_SUSPENDED. Call it after setting cmd.SysProcAttr, and Capture once after a successful Start. @@ -42,12 +43,12 @@ func NewTreeKill(cmd *exec.Cmd) *TreeKill { t := &TreeKill{} cmd.Cancel = func() error { if k := t.killer.Load(); k != nil { - return k.Kill() + return k.cancel(cmd.Process, t.GracefulCancel) } if cmd.Process != nil { if killer, err := NewKiller(cmd.Process); err == nil { defer killer.Close() - return killer.Kill() + return killer.cancel(cmd.Process, t.GracefulCancel) } return cmd.Process.Kill() } diff --git a/internal/pkg/process/treekill_test.go b/internal/pkg/process/treekill_test.go index e138e2c4..8007c452 100644 --- a/internal/pkg/process/treekill_test.go +++ b/internal/pkg/process/treekill_test.go @@ -45,6 +45,7 @@ func TestTreeKillWaitWithLeftoverHoldingOutput(t *testing.T) { cmd.Stdout = io.Discard cmd.SysProcAttr = SysProcAttr("", false) tk := NewTreeKill(cmd) + tk.GracefulCancel = true cmd.WaitDelay = 50 * time.Millisecond require.NoError(t, cmd.Start()) killer, err := tk.Capture(cmd.Process)