From b769a9765f2258350cf5f536e08acbfdd2ac49c0 Mon Sep 17 00:00:00 2001 From: silverwind Date: Thu, 15 Oct 2026 12:49:37 +0000 Subject: [PATCH] perf: download actions as archives once per job (#1316) Each step refreshed its own clone of an action, so a composite using one action repository ten times fetched it ten times per job, and a single network error failed the job. Like actions/runner, a job now downloads each action repository and ref once, as an archive extracted into a directory per commit. Each action still gets its own copy in the job container. This restores the per-job dedupe of https://gitea.com/gitea/runner/pulls/1178, which only reached a since-deleted branch. How a download works: 1. The action URL is taken after `insteadOf` rewriting, as git would use it. 2. The archive comes from `/archive/.tar.gz`, or from codeload.github.com for github.com, as for actions/runner. 3. A cached tag or branch is revalidated with its ETag, which GitHub and Gitea 28+ answer with a 304. A cached commit pin needs no request. 4. Transient failures (5xx, 408, 429 honouring `Retry-After`, dropped transfers) are retried three times with a 10 to 30 second backoff. 5. Git takes over, cloning and exporting the commit into the same directory layout, when the URL is not HTTP(S), the ref is a short commit, the host serves no usable archive, the host cannot be connected to, or the retries run out. Median of 3 runs, git (shallow clone, then fetch) against archives, on a cold and a warm cache: | Action | git cold | git warm | archive cold | archive warm | |---|---|---|---|---| | github.com actions/checkout@v4 | 937ms | 726ms | 286ms | 171ms | | github.com actions/cache@v4 | 1006ms | 801ms | 516ms | 161ms | | github.com actions/setup-node@v4 | 1046ms | 786ms | 463ms | 156ms | | gitea.com actions/checkout@v4 | 2007ms | 1165ms | 667ms | 222ms | The action cache layout changes, so existing caches download once more. *Written by Claude.* Fixes: https://gitea.com/gitea/runner/issues/1220 Reviewed-on: https://gitea.com/gitea/runner/pulls/1316 Reviewed-by: bircni Co-authored-by: silverwind --- README.md | 2 +- act/common/git/fetch_action.go | 238 ++++++++++++++++++++++++ act/common/git/fetch_action_test.go | 225 ++++++++++++++++++++++ act/common/git/git.go | 52 +++--- act/common/git/git_test.go | 40 ---- act/runner/action.go | 37 ++-- act/runner/action_composite_test.go | 6 +- act/runner/action_test.go | 13 +- act/runner/job_executor.go | 12 +- act/runner/job_executor_test.go | 20 +- act/runner/patch_actions_test.go | 7 +- act/runner/reusable_workflow.go | 4 +- act/runner/reusable_workflow_test.go | 4 + act/runner/run_context.go | 5 +- act/runner/step_action_local.go | 5 +- act/runner/step_action_remote.go | 96 +++++----- act/runner/step_action_remote_test.go | 158 ++++++++-------- internal/pkg/config/config.example.yaml | 5 +- internal/pkg/config/config.go | 2 +- 19 files changed, 664 insertions(+), 267 deletions(-) create mode 100644 act/common/git/fetch_action.go create mode 100644 act/common/git/fetch_action_test.go diff --git a/README.md b/README.md index 429b4067..d1399d42 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@ On the runner: -- `git` 2.34.1 or newer on `PATH`, to download actions and reusable workflows. +- `git` 2.34.1 or newer on `PATH`, to clone reusable workflows, and actions whose host serves no archive of them. - `docker` or `podman` with `podman.socket` enabled, for docker mode, Docker actions and `docker://` steps. To install Docker CE, follow the official [install instructions](https://docs.docker.com/engine/install/). Where steps run, in the job image or on the runner in host mode: diff --git a/act/common/git/fetch_action.go b/act/common/git/fetch_action.go new file mode 100644 index 00000000..858cab3f --- /dev/null +++ b/act/common/git/fetch_action.go @@ -0,0 +1,238 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package git + +import ( + "archive/tar" + "bytes" + "compress/gzip" + "context" + "errors" + "io" + "math/rand/v2" + "net" + "net/http" + "net/url" + "os" + "path/filepath" + "runtime" + "strconv" + "strings" + "time" + + "gitea.com/gitea/runner/act/common" +) + +var archiveRetryDelay = func(retryAfter string) time.Duration { // as actions/runner backs off + if seconds, err := strconv.Atoi(retryAfter); err == nil { + return min(max(time.Duration(seconds)*time.Second, 10*time.Second), 10*time.Minute) + } + return time.Duration(10+rand.IntN(21)) * time.Second +} + +func FetchAction(ctx context.Context, input NewGitCloneExecutorInput) (string, string, error) { // input.Dir is one clone URL and ref's, holding a directory per commit + unlock, err := AcquireCloneLock(ctx, input.Dir) + if err != nil { + return "", "", err + } + defer unlock() + if err := os.MkdirAll(input.Dir, 0o755); err != nil { + return "", "", err + } + + tree := func(sha string) string { return filepath.Join(input.Dir, sha) } + record := filepath.Join(input.Dir, "ref") + data, _ := os.ReadFile(record) + sha, etag, _ := strings.Cut(string(data), "\n") + if !isFullObjectID(sha) || !exists(tree(sha)) { + sha, etag = "", "" + } else if input.OfflineMode || isFullObjectID(input.Ref) { + return tree(sha), sha, nil + } + + newSha, newEtag, err := downloadArchive(ctx, input, sha, etag) + if err != nil && ctx.Err() == nil { + common.Logger(ctx).Debugf("Cloning %s@%s: %v", input.URL, input.Ref, err) + newEtag = "" + newSha, err = exportClone(ctx, input) + } + if err != nil { + return "", "", err + } + if newSha != sha || newEtag != etag { + if err := os.WriteFile(record, []byte(newSha+"\n"+newEtag), 0o600); err != nil { + return "", "", err + } + } + return tree(newSha), newSha, nil +} + +func downloadArchive(ctx context.Context, input NewGitCloneExecutorInput, sha, etag string) (string, string, error) { + rewritten, err := gitOutput(ctx, "", "ls-remote", "--get-url", input.URL) + if err != nil { + return "", "", err + } + source, err := url.Parse(rewritten) + if err != nil || source.Scheme != "http" && source.Scheme != "https" || hexRefRegex.MatchString(input.Ref) && !isFullObjectID(input.Ref) { // only git tells a short commit from a branch + return "", "", errors.New("no archive") + } + archive := source.JoinPath("archive", input.Ref+".tar.gz") + if source.Host == "github.com" { + source.Host = "codeload.github.com" + archive = source.JoinPath("tar.gz", input.Ref) + } + client := &http.Client{Transport: input.Transport, Timeout: 5 * time.Minute, CheckRedirect: func(request *http.Request, via []*http.Request) error { + if len(via) >= 10 { + return errors.New("stopped after 10 redirects") + } + if request.URL.Host != via[0].URL.Host || request.URL.Scheme != via[0].URL.Scheme { // the token stays on its origin + request.Header.Del("Authorization") + } + return nil + }} + for attempt := 1; ; attempt++ { + newSha, newEtag, retryAfter, err := downloadArchiveOnce(ctx, client, archive.String(), input.Token, sha, etag, input.Dir) + if retryAfter == 0 || attempt == 3 || ctx.Err() != nil { + return newSha, newEtag, err + } + common.Logger(ctx).Warnf("Failed to download action '%s'. Error: %v. Back off %s before retry.", archive.Redacted(), err, retryAfter) + select { + case <-ctx.Done(): + return "", "", ctx.Err() + case <-time.After(retryAfter): + } + } +} + +func downloadArchiveOnce(ctx context.Context, client *http.Client, archive, token, sha, etag, dir string) (string, string, time.Duration, error) { + request, err := http.NewRequestWithContext(ctx, http.MethodGet, archive, nil) + if err != nil { + return "", "", 0, err + } + if etag != "" { + request.Header.Set("If-None-Match", etag) + } + if token != "" { + request.SetBasicAuth("token", token) + } + response, err := client.Do(request) + if err != nil { + var opErr *net.OpError + if errors.As(err, &opErr) && opErr.Op == "dial" { + return "", "", 0, err + } + return "", "", archiveRetryDelay(""), err + } + defer response.Body.Close() + switch status := response.StatusCode; { + case status == http.StatusNotModified && sha != "": + return sha, etag, 0, nil + case status == http.StatusOK: + newEtag := response.Header.Get("ETag") + gz, err := gzip.NewReader(response.Body) + if err == nil { + sha, err = extractTree(gz, dir, !strings.HasSuffix(newEtag, `-false"`)) // Gitea's mark of an unprefixed archive + } + var netErr net.Error + if errors.Is(err, io.ErrUnexpectedEOF) || errors.As(err, &netErr) { + return "", "", archiveRetryDelay(""), err + } + return sha, newEtag, 0, err + case status == http.StatusRequestTimeout || status == http.StatusTooManyRequests || status >= http.StatusInternalServerError: + return "", "", archiveRetryDelay(response.Header.Get("Retry-After")), errors.New(response.Status) + default: + return "", "", 0, errors.New(response.Status) + } +} + +func exportClone(ctx context.Context, input NewGitCloneExecutorInput) (string, error) { + clone := input + clone.Dir = filepath.Join(input.Dir, "git") + sha, _, err := resolveClone(ctx, clone) + if err != nil || exists(filepath.Join(input.Dir, sha)) { + return sha, err + } + archive, err := runGit(ctx, clone.Dir, nil, "archive", "--format=tar", sha) + if err != nil { + return "", err + } + _, err = extractTree(bytes.NewReader(archive), input.Dir, false) + return sha, err +} + +func extractTree(archive io.Reader, dir string, strip bool) (string, error) { // into dir/, strip dropping the directory wrapping the files + temp, err := os.MkdirTemp(dir, "tmp-") + if err != nil { + return "", err + } + defer os.RemoveAll(temp) + root, err := os.OpenRoot(temp) + if err != nil { + return "", err + } + defer root.Close() + + sha := "" + reader := tar.NewReader(archive) + for { + header, err := reader.Next() + if errors.Is(err, io.EOF) { + break + } + if err != nil { + return "", err + } + if header.Typeflag == tar.TypeXGlobalHeader { + if sha = header.PAXRecords["comment"]; isFullObjectID(sha) && exists(filepath.Join(dir, sha)) { + return sha, nil + } + continue + } + name := header.Name + if strip { + _, name, _ = strings.Cut(name, "/") + } + if name = strings.TrimSuffix(name, "/"); name == "" { + continue + } + switch header.Typeflag { + case tar.TypeDir: + err = root.MkdirAll(name, 0o755) + case tar.TypeReg: + err = writeTreeFile(root, name, header.Mode, reader) + case tar.TypeSymlink: + if err = root.Symlink(header.Linkname, name); err != nil && runtime.GOOS == "windows" { + err = writeTreeFile(root, name, 0, strings.NewReader(header.Linkname)) + } + } + if err != nil { + return "", err + } + } + if !isFullObjectID(sha) { + return "", errors.New("the archive names no commit") + } + if err := root.Close(); err != nil { + return "", err + } + return sha, os.Rename(temp, filepath.Join(dir, sha)) +} + +func writeTreeFile(root *os.Root, name string, mode int64, content io.Reader) error { + perm := os.FileMode(0o644) + if mode&0o111 != 0 { + perm = 0o755 + } + file, err := root.OpenFile(name, os.O_CREATE|os.O_EXCL|os.O_WRONLY, perm) + if err != nil { + return err + } + _, err = io.Copy(file, content) + return errors.Join(err, file.Close()) +} + +func exists(name string) bool { + _, err := os.Stat(name) + return err == nil +} diff --git a/act/common/git/fetch_action_test.go b/act/common/git/fetch_action_test.go new file mode 100644 index 00000000..c71987b9 --- /dev/null +++ b/act/common/git/fetch_action_test.go @@ -0,0 +1,225 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package git + +import ( + "archive/tar" + "bytes" + "encoding/base64" + "fmt" + "net/http" + "net/http/cgi" + "net/http/httptest" + "os" + "os/exec" + "path/filepath" + "strings" + "sync" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestFetchAction(t *testing.T) { + original := archiveRetryDelay + archiveRetryDelay = func(_retryAfter string) time.Duration { return time.Nanosecond } + t.Cleanup(func() { archiveRetryDelay = original }) + + root := t.TempDir() + remote := filepath.Join(root, "org", "action") + require.NoError(t, gitCmd("init", "--initial-branch=main", remote)) + require.NoError(t, os.WriteFile(filepath.Join(remote, "action.yml"), []byte("name: action\n"), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(remote, "run.sh"), []byte("#!/bin/sh\n"), 0o755)) + require.NoError(t, os.Symlink("action.yml", filepath.Join(remote, "link"))) + require.NoError(t, gitCmd("-C", remote, "add", ".")) + require.NoError(t, gitCmd("-C", remote, "commit", "-m", "initial")) + require.NoError(t, gitCmd("-C", remote, "tag", "v1")) + + gitPath, err := exec.LookPath("git") + require.NoError(t, err) + backend := &cgi.Handler{Path: gitPath, Args: []string{"http-backend"}, Env: []string{"GIT_PROJECT_ROOT=" + root, "GIT_HTTP_EXPORT_ALL=1"}} + var mu sync.Mutex + var archiveRequests []http.Header + var mirror *httptest.Server + requests, archives, failures := 0, "", 0 + handler := http.HandlerFunc(func(writer http.ResponseWriter, request *http.Request) { + mu.Lock() + defer mu.Unlock() + requests++ + ref, ok := strings.CutPrefix(request.URL.Path, "/org/action/archive/") + if !ok { + backend.ServeHTTP(writer, request) + return + } + archiveRequests = append(archiveRequests, request.Header.Clone()) + switch { + case failures > 0: + failures-- + writer.WriteHeader(http.StatusServiceUnavailable) + return + case archives == "none": + http.NotFound(writer, request) + return + case archives == "unreachable": + http.Redirect(writer, request, "http://127.0.0.1:1"+request.URL.Path, http.StatusFound) + return + case archives == "redirect" && request.Host != mirror.Listener.Addr().String(): + http.Redirect(writer, request, mirror.URL+request.URL.Path, http.StatusFound) + return + } + sha, _ := gitOutput(request.Context(), remote, "rev-parse", strings.TrimSuffix(ref, ".tar.gz")) + etag := fmt.Sprintf(`W/"%s-tar.gz-%t"`, sha, archives != "unprefixed") + if request.Header.Get("If-None-Match") == etag { + writer.WriteHeader(http.StatusNotModified) + return + } + args := []string{"archive", "--format=tar.gz", sha} + if archives != "unprefixed" { + args = append(args, "--prefix=action/") + } + archive, _ := runGit(request.Context(), remote, nil, args...) + writer.Header().Set("ETag", etag) + _, _ = writer.Write(archive) + }) + server := httptest.NewServer(handler) + mirror = httptest.NewServer(handler) + t.Cleanup(server.Close) + t.Cleanup(mirror.Close) + serve := func(mode string, failing int) { + mu.Lock() + defer mu.Unlock() + requests, archives, failures, archiveRequests = 0, mode, failing, nil + } + served := func() (int, []http.Header) { + mu.Lock() + defer mu.Unlock() + return requests, append([]http.Header(nil), archiveRequests...) + } + newInput := func(ref string) NewGitCloneExecutorInput { + return NewGitCloneExecutorInput{URL: server.URL + "/org/action", Ref: ref, Dir: t.TempDir(), Token: "secret"} + } + + t.Run("downloads, revalidates and follows a moved tag", func(t *testing.T) { + serve("prefixed", 0) + input := newInput("v1") + tree, sha, err := FetchAction(t.Context(), input) + require.NoError(t, err) + assert.Equal(t, gitRevParse(t, remote, "v1"), sha) + assert.Equal(t, filepath.Join(input.Dir, sha), tree) + assert.NoDirExists(t, filepath.Join(tree, ".git")) + info, err := os.Stat(filepath.Join(tree, "run.sh")) + require.NoError(t, err) + assert.Equal(t, os.FileMode(0o755), info.Mode().Perm()) + link, err := os.Readlink(filepath.Join(tree, "link")) + require.NoError(t, err) + assert.Equal(t, "action.yml", link) + + again, _, err := FetchAction(t.Context(), input) + require.NoError(t, err) + assert.Equal(t, tree, again) + + require.NoError(t, gitCmd("-C", remote, "commit", "--allow-empty", "-m", "moved")) + require.NoError(t, gitCmd("-C", remote, "tag", "--force", "v1")) + moved, movedSha, err := FetchAction(t.Context(), input) + require.NoError(t, err) + assert.Equal(t, gitRevParse(t, remote, "v1"), movedSha) + assert.NotEqual(t, tree, moved) + assert.FileExists(t, filepath.Join(tree, "action.yml")) + + input.OfflineMode = true + _, offlineSha, err := FetchAction(t.Context(), input) + require.NoError(t, err) + assert.Equal(t, movedSha, offlineSha) + + requests, headers := served() + assert.Equal(t, 3, requests) + require.Len(t, headers, 3) + assert.Equal(t, "Basic "+base64.StdEncoding.EncodeToString([]byte("token:secret")), headers[0].Get("Authorization")) + assert.Empty(t, headers[0].Get("If-None-Match")) + assert.Equal(t, fmt.Sprintf(`W/"%s-tar.gz-true"`, sha), headers[1].Get("If-None-Match")) + assert.Equal(t, headers[1].Get("If-None-Match"), headers[2].Get("If-None-Match")) + }) + + t.Run("downloads a pinned commit once and rejects a short one", func(t *testing.T) { + serve("prefixed", 0) + sha := gitRevParse(t, remote, "main") + input := newInput(sha) + for range 2 { + tree, got, err := FetchAction(t.Context(), input) + require.NoError(t, err) + assert.Equal(t, sha, got) + assert.FileExists(t, filepath.Join(tree, "action.yml")) + } + _, _, err := FetchAction(t.Context(), newInput(sha[:7])) + require.ErrorIs(t, err, ErrShortRef) + _, headers := served() + assert.Len(t, headers, 1) + assert.NoDirExists(t, filepath.Join(input.Dir, "git")) + }) + + t.Run("retries a failing download and drops the token on a redirect to another origin", func(t *testing.T) { + serve("redirect", 2) + input := newInput("main") + _, sha, err := FetchAction(t.Context(), input) + require.NoError(t, err) + assert.Equal(t, gitRevParse(t, remote, "main"), sha) + assert.NoDirExists(t, filepath.Join(input.Dir, "git")) + _, headers := served() + require.Len(t, headers, 4) + assert.NotEmpty(t, headers[2].Get("Authorization")) + assert.Empty(t, headers[3].Get("Authorization")) + }) + + t.Run("keeps an unprefixed archive whole, downloads where insteadOf points and exports a clone without an archive or from an unreachable host", func(t *testing.T) { + for _, mode := range []string{"unprefixed", "none", "unreachable"} { + serve(mode, 0) + tree, sha, err := FetchAction(t.Context(), newInput("main")) + require.NoError(t, err, mode) + assert.Equal(t, gitRevParse(t, remote, "main"), sha, mode) + assert.FileExists(t, filepath.Join(tree, "action.yml"), mode) + assert.NoDirExists(t, filepath.Join(tree, ".git"), mode) + _, headers := served() + assert.Len(t, headers, 1, mode) + } + + serve("prefixed", 0) + config := filepath.Join(t.TempDir(), "gitconfig") + require.NoError(t, os.WriteFile(config, []byte("[url \""+server.URL+"/org/\"]\n\tinsteadOf = https://example.invalid/org/\n"), 0o644)) + t.Setenv("GIT_CONFIG_GLOBAL", config) + input := newInput("main") + input.URL = "https://example.invalid/org/action" + _, sha, err := FetchAction(t.Context(), input) + require.NoError(t, err) + assert.Equal(t, gitRevParse(t, remote, "main"), sha) + _, headers := served() + assert.Len(t, headers, 1) + assert.NoDirExists(t, filepath.Join(input.Dir, "git")) + }) +} + +func TestExtractTreeConfinesEntries(t *testing.T) { + outside := t.TempDir() + for name, entries := range map[string][]*tar.Header{ + "parent path": {{Name: "../escape", Typeflag: tar.TypeReg}}, + "through a symlink": {{Name: "out", Typeflag: tar.TypeSymlink, Linkname: outside}, {Name: "out/escape", Typeflag: tar.TypeReg}}, + } { + t.Run(name, func(t *testing.T) { + var archive bytes.Buffer + writer := tar.NewWriter(&archive) + require.NoError(t, writer.WriteHeader(&tar.Header{Typeflag: tar.TypeXGlobalHeader, PAXRecords: map[string]string{"comment": strings.Repeat("a", 40)}})) + for _, entry := range entries { + require.NoError(t, writer.WriteHeader(entry)) + } + require.NoError(t, writer.Close()) + + dir := t.TempDir() + _, err := extractTree(&archive, dir, false) + require.Error(t, err) + assert.NoFileExists(t, filepath.Join(dir, "escape")) + assert.NoFileExists(t, filepath.Join(outside, "escape")) + }) + } +} diff --git a/act/common/git/git.go b/act/common/git/git.go index 2b31367e..214c32b8 100644 --- a/act/common/git/git.go +++ b/act/common/git/git.go @@ -10,6 +10,7 @@ import ( "encoding/base64" "errors" "fmt" + "net/http" "net/url" "os" "os/exec" @@ -178,13 +179,10 @@ type NewGitCloneExecutorInput struct { // 0 for full clone. Depth int - // Quiet drops the informational clone line to debug level, for callers that log their own - // download summary (the setup section's action report). - Quiet bool - // For Gitea InsecureSkipTLS bool - ClientCertFile, ClientKeyFile string // PEM, absolute as git runs in other directories + ClientCertFile, ClientKeyFile string // PEM, absolute as git runs in other directories + Transport http.RoundTripper // for FetchAction's archive downloads, nil for http.DefaultTransport } // CloneIfRequired reports whether an existing local clone was reused. @@ -221,11 +219,7 @@ func CloneIfRequired(ctx context.Context, input NewGitCloneExecutorInput, logger func NewGitCloneExecutor(input NewGitCloneExecutorInput) common.Executor { return func(ctx context.Context) error { logger := common.Logger(ctx) - if input.Quiet { - logger.Debugf("git clone '%s' # ref=%s", input.URL, input.Ref) - } else { - logger.Infof("git clone '%s' # ref=%s", input.URL, input.Ref) - } + logger.Infof("git clone '%s' # ref=%s", input.URL, input.Ref) logger.Debugf(" cloning %s to %s", input.URL, input.Dir) unlock, err := AcquireCloneLock(ctx, input.Dir) if err != nil { @@ -233,24 +227,10 @@ func NewGitCloneExecutor(input NewGitCloneExecutorInput) common.Executor { } defer unlock() - reused, err := CloneIfRequired(ctx, input, logger) + resolved, reused, err := resolveClone(ctx, input) if err != nil { return err } - resolved, err := resolveRef(ctx, input.Dir, input.Ref) - if errors.Is(err, ErrShortRef) || (err != nil && input.OfflineMode) { - return err - } - if !input.OfflineMode && (err != nil || (reused && !isFullHash(input.Ref, resolved))) { - if err := fetch(ctx, input); err != nil { - return err - } - resolved, err = resolveRef(ctx, input.Dir, input.Ref) - } - if err != nil { - logger.Errorf("Unable to resolve %s: %v", input.Ref, err) - return err - } if _, err := runGit(ctx, input.Dir, nil, "checkout", "--force", "--detach", resolved); err != nil { return fmt.Errorf("checkout %s: %w", resolved, err) } @@ -264,6 +244,28 @@ func NewGitCloneExecutor(input NewGitCloneExecutorInput) common.Executor { } } +func resolveClone(ctx context.Context, input NewGitCloneExecutorInput) (string, bool, error) { // the caller holds input.Dir's lock + logger := common.Logger(ctx) + reused, err := CloneIfRequired(ctx, input, logger) + if err != nil { + return "", false, err + } + resolved, err := resolveRef(ctx, input.Dir, input.Ref) + if errors.Is(err, ErrShortRef) || (err != nil && input.OfflineMode) { + return "", false, err + } + if !input.OfflineMode && (err != nil || (reused && !isFullHash(input.Ref, resolved))) { + if err := fetch(ctx, input); err != nil { + return "", false, err + } + resolved, err = resolveRef(ctx, input.Dir, input.Ref) + } + if err != nil { + logger.Errorf("Unable to resolve %s: %v", input.Ref, err) + } + return resolved, reused, err +} + func clone(ctx context.Context, input NewGitCloneExecutorInput) error { if input.Depth > 0 && isFullObjectID(input.Ref) { if err := shallowPinnedClone(ctx, input); err == nil { diff --git a/act/common/git/git_test.go b/act/common/git/git_test.go index 5f74edf0..789ab2c0 100644 --- a/act/common/git/git_test.go +++ b/act/common/git/git_test.go @@ -21,7 +21,6 @@ import ( "testing/synctest" "time" - "gitea.com/gitea/runner/act/common" "gitea.com/gitea/runner/act/common/logging" "gitea.com/gitea/runner/act/common/logging/logtest" "gitea.com/gitea/runner/internal/pkg/lock" @@ -426,45 +425,6 @@ func TestGitCloneExecutorOfflineMode(t *testing.T) { }) } -func TestGitCloneExecutorQuietDemotesCloneLine(t *testing.T) { - t.Parallel() - remoteDir := t.TempDir() - require.NoError(t, gitCmd("init", "--bare", "--initial-branch=main", remoteDir)) - workDir := t.TempDir() - require.NoError(t, gitCmd("clone", remoteDir, workDir)) - require.NoError(t, gitCmd("-C", workDir, "checkout", "-b", "main")) - require.NoError(t, gitCmd("-C", workDir, "commit", "--allow-empty", "-m", "initial")) - require.NoError(t, gitCmd("-C", workDir, "push", "-u", "origin", "main")) - - // Quiet callers report the download themselves, so the clone line must not reach the job log. - for name, quiet := range map[string]bool{"quiet": true, "not quiet": false} { - t.Run(name, func(t *testing.T) { - hook := logtest.New() - logger := logging.New(logging.NewLevelHandler(hook, logging.LevelInfo)) - ctx := common.WithLogger(t.Context(), logger.With("job", "j1")) - - require.NoError(t, NewGitCloneExecutor(NewGitCloneExecutorInput{ - URL: remoteDir, - Ref: "main", - Dir: t.TempDir(), - Quiet: quiet, - })(ctx)) - - var cloneLines int - for _, entry := range hook.AllRecords() { - if strings.HasPrefix(entry.Message, "git clone ") { - cloneLines++ - } - } - if quiet { - assert.Zero(t, cloneLines) - } else { - assert.Equal(t, 1, cloneLines) - } - }) - } -} - func TestGitCloneExecutorShallow(t *testing.T) { t.Parallel() // Build a local "remote" with several commits on main plus a tag, so a full clone would pull noticeably more history than a shallow one. diff --git a/act/runner/action.go b/act/runner/action.go index 8be1cc32..dee56e54 100644 --- a/act/runner/action.go +++ b/act/runner/action.go @@ -91,7 +91,7 @@ func readActionImpl(ctx context.Context, step *model.Step, actionDir, actionPath if b, err = trampoline.ReadFile("res/trampoline.js"); err != nil { return nil, err } - err2 := writeFile(filepath.Join(actionDir, actionPath, "trampoline.js"), b, 0o400) + err2 := writeFile(filepath.Join(actionDir, actionPath, "trampoline.js"), b, 0o644) if err2 != nil { return nil, err2 } @@ -152,11 +152,11 @@ func maybeCopyToActionDir(ctx context.Context, step actionStep, actionDir, actio defer unlock() if !rc.Config.NoActionPatch { - // A concurrent job's prepare resets this directory, so patch under the copy's lock. + // Other jobs share the directory, so patch under the copy's lock. defer patchActions(ctx, actionScriptPaths(filepath.Join(actionDir, actionPath), step.getActionModel()))() } - return rc.JobContainer.CopyDir(containerActionDirCopy, actionDir+"/", rc.Config.UseGitIgnore, true)(ctx) + return rc.JobContainer.CopyDir(containerActionDirCopy, actionDir+"/", false, true)(ctx) // an exported commit has no .git to apply gitignore with } func runActionImpl(step actionStep, actionDir string, remoteAction *remoteAction) common.Executor { @@ -179,7 +179,7 @@ func runActionImpl(step actionStep, actionDir string, remoteAction *remoteAction } actionLocation := path.Join(actionDir, actionPath) - actionName, containerActionDir := getContainerActionPaths(stepModel, actionLocation, rc) + actionName, containerActionDir := getContainerActionPaths(step) logger.Debugf("type=%v actionDir=%s actionPath=%s workdir=%s actionCacheDir=%s actionName=%s containerActionDir=%s", stepModel.Type(), actionDir, actionPath, rc.Config.Workdir, rc.ActionCacheDir(), actionName, containerActionDir) @@ -509,25 +509,14 @@ func populateEnvsFromInput(ctx context.Context, env *map[string]string, action * return nil } -func getContainerActionPaths(step *model.Step, actionDir string, rc *RunContext) (string, string) { - actionName := "" - containerActionDir := "." - if step.Type() != model.StepTypeUsesActionRemote { - actionName = getOsSafeRelativePath(actionDir, rc.Config.Workdir) - containerActionDir = rc.JobContainer.ToContainerPath(rc.Config.Workdir) + "/" + actionName - actionName = "./" + actionName - } else if step.Type() == model.StepTypeUsesActionRemote { - actionName = getOsSafeRelativePath(actionDir, rc.ActionCacheDir()) - containerActionDir = rc.JobContainer.GetActPath() + "/actions/" + actionName +func getContainerActionPaths(step actionStep) (string, string) { + rc := step.getRunContext() + if sar, ok := step.(*stepActionRemote); ok { // a copy per action, as its repository's directory is shared + actionName := path.Join(sar.containerKey(), sar.remoteAction.Path) + return actionName, rc.JobContainer.GetActPath() + "/actions/" + actionName } - - if actionName == "" { - actionName = filepath.Base(actionDir) - if runtime.GOOS == "windows" { - actionName = strings.ReplaceAll(actionName, "\\", "/") - } - } - return actionName, containerActionDir + actionName := getOsSafeRelativePath(filepath.Join(rc.Config.Workdir, step.getStepModel().Uses), rc.Config.Workdir) + return "./" + actionName, rc.JobContainer.ToContainerPath(rc.Config.Workdir) + "/" + actionName } func getOsSafeRelativePath(s, prefix string) string { @@ -573,13 +562,13 @@ func actionStagePaths(step actionStep) (actionDir, actionPath, actionName, conta stepModel := step.getStepModel() if sar, ok := step.(*stepActionRemote); ok { - actionDir = sar.actionDir() + actionDir = sar.actionDir actionPath = sar.remoteAction.Path } else { actionDir = filepath.Join(rc.Config.Workdir, stepModel.Uses) } - actionName, containerActionDir = getContainerActionPaths(stepModel, path.Join(actionDir, actionPath), rc) + actionName, containerActionDir = getContainerActionPaths(step) return actionDir, actionPath, actionName, containerActionDir } diff --git a/act/runner/action_composite_test.go b/act/runner/action_composite_test.go index 948f5e5c..9d223fc3 100644 --- a/act/runner/action_composite_test.go +++ b/act/runner/action_composite_test.go @@ -4,9 +4,9 @@ package runner import ( + "context" "testing" - "gitea.com/gitea/runner/act/common" "gitea.com/gitea/runner/act/common/git" "gitea.dev/actionslib/pkg/model" @@ -50,7 +50,9 @@ func TestCompositeActionParity(t *testing.T) { }) t.Run("propagates pre failures", func(t *testing.T) { - setCloneExecutor(t, func(git.NewGitCloneExecutorInput) common.Executor { return common.NewErrorExecutor(assert.AnError) }) + setFetch(t, func(_ctx context.Context, _input git.NewGitCloneExecutorInput) (string, string, error) { + return "", "", assert.AnError + }) rc := &RunContext{ Config: &Config{GitHubInstance: "github.com", ActionCacheDir: t.TempDir()}, Run: &model.Run{JobID: "job", Workflow: &model.Workflow{Jobs: map[string]*model.Job{"job": {}}}}, diff --git a/act/runner/action_test.go b/act/runner/action_test.go index 94222d59..795dd70d 100644 --- a/act/runner/action_test.go +++ b/act/runner/action_test.go @@ -128,7 +128,7 @@ runs: writeFile := func(filename string, data []byte, perm fs.FileMode) error { assert.Equal(t, "actionDir/actionPath/trampoline.js", filename) - assert.Equal(t, fs.FileMode(0o400), perm) + assert.Equal(t, fs.FileMode(0o644), perm) return nil } @@ -178,7 +178,8 @@ func TestActionRunner(t *testing.T) { Using: "node16", }, }, - env: map[string]string{}, + env: map[string]string{}, + remoteAction: mustNewRemoteAction(t, "org/repo/path@ref"), }, expectedEnv: map[string]string{"INPUT_KEY": "default value"}, }, @@ -217,7 +218,8 @@ func TestActionRunner(t *testing.T) { Using: "node16", }, }, - env: map[string]string{}, + env: map[string]string{}, + remoteAction: mustNewRemoteAction(t, "org/repo/path@ref"), }, expectedEnv: map[string]string{"STATE_name": "state value"}, }, @@ -228,7 +230,8 @@ func TestActionRunner(t *testing.T) { ctx := t.Context() cm := &containerMock{} - cm.On("CopyDir", "/var/run/act/actions/dir/", "dir/", false, true).Return(func(ctx context.Context) error { return nil }) + containerActionDir := "/var/run/act/actions/" + model.UsesHash("/org/repo/path@ref") + "/" + cm.On("CopyDir", containerActionDir, "dir/", false, true).Return(func(ctx context.Context) error { return nil }) envMatcher := mock.MatchedBy(func(env map[string]string) bool { for k, v := range tt.expectedEnv { @@ -239,7 +242,7 @@ func TestActionRunner(t *testing.T) { return true }) - cm.On("Exec", []string{"node", "--preserve-symlinks-main", "/var/run/act/actions/dir/path"}, envMatcher, "", "").Return(func(ctx context.Context) error { return nil }) + cm.On("Exec", []string{"node", "--preserve-symlinks-main", containerActionDir + "path"}, envMatcher, "", "").Return(func(ctx context.Context) error { return nil }) tt.step.getRunContext().JobContainer = cm diff --git a/act/runner/job_executor.go b/act/runner/job_executor.go index 354bf246..e067c7b3 100644 --- a/act/runner/job_executor.go +++ b/act/runner/job_executor.go @@ -80,7 +80,7 @@ func reportStepError(ctx context.Context, rc *RunContext, err error) { // executor can fetch all of them up front. type actionPreparer interface { prepareActionExecutor() common.Executor - actionDownloadInfo() (reference, sha string, ok bool) + actionDownloadInfo() (reference, sha string) } // printPrepareActions downloads every action the job uses before its first step runs and reports @@ -101,15 +101,11 @@ func printPrepareActions(rc *RunContext, preparers []actionPreparer) common.Exec reportStepError(ctx, rc, err) return setupError{err} } - reference, sha, ok := preparer.actionDownloadInfo() - if !ok { + reference, sha := preparer.actionDownloadInfo() + if sha == "" { continue } - if sha == "" { - rawLogger.Infof("Download action repository '%s'", reference) - } else { - rawLogger.Infof("Download action repository '%s' (SHA:%s)", reference, sha) - } + rawLogger.Infof("Download action repository '%s' (SHA:%s)", reference, sha) } return nil } diff --git a/act/runner/job_executor_test.go b/act/runner/job_executor_test.go index 65c6ca67..8e0ed908 100644 --- a/act/runner/job_executor_test.go +++ b/act/runner/job_executor_test.go @@ -118,7 +118,6 @@ func (sfm *stepFactoryMock) newStep(model *model.Step, rc *RunContext) (step, er type actionPreparerMock struct { reference string sha string - ok bool err error prepared int } @@ -130,8 +129,8 @@ func (apm *actionPreparerMock) prepareActionExecutor() common.Executor { } } -func (apm *actionPreparerMock) actionDownloadInfo() (string, string, bool) { - return apm.reference, apm.sha, apm.ok +func (apm *actionPreparerMock) actionDownloadInfo() (string, string) { + return apm.reference, apm.sha } func TestPrintPrepareActionsGolden(t *testing.T) { @@ -140,18 +139,15 @@ func TestPrintPrepareActionsGolden(t *testing.T) { ctx := common.WithLogger(t.Context(), logger.With("job", "j1")) preparers := []actionPreparer{ - &actionPreparerMock{reference: "actions/checkout@v7", sha: "9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0", ok: true}, - // A resolved commit is best effort; the ref alone is reported when it is unknown. - &actionPreparerMock{reference: "actions/setup-go@v6", ok: true}, + &actionPreparerMock{reference: "actions/checkout@v7", sha: "9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0"}, // A step that downloads nothing, such as the checkout of the workflow's own repository. - &actionPreparerMock{ok: false}, + &actionPreparerMock{}, } require.NoError(t, printPrepareActions(&RunContext{}, preparers)(ctx)) want := strings.Join([]string{ "[j1] | Prepare all required actions", "[j1] | Download action repository 'actions/checkout@v7' (SHA:9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0)", - "[j1] | Download action repository 'actions/setup-go@v6'", "", }, "\n") assert.Equal(t, want, buf.String()) @@ -173,7 +169,7 @@ func TestPrintPrepareActionsFailsJobOnDownloadError(t *testing.T) { downloadErr := errors.New("failed to fetch \"actions/checkout\"") rc := &RunContext{} - remaining := &actionPreparerMock{reference: "actions/setup-go@v6", ok: true} + remaining := &actionPreparerMock{reference: "actions/setup-go@v6", sha: "sha"} err := printPrepareActions(rc, []actionPreparer{ &actionPreparerMock{err: downloadErr}, @@ -267,7 +263,7 @@ func TestNewJobExecutorDownloadsAllActionsBeforeTheFirstStep(t *testing.T) { for _, stepModel := range steps { sm := &stepMock{} - apm := &actionPreparerMock{reference: "actions/checkout@v" + stepModel.ID, ok: true} + apm := &actionPreparerMock{reference: "actions/checkout@v" + stepModel.ID, sha: "sha" + stepModel.ID} sfm.On("newStep", stepModel, rc).Return(&actionStepMock{stepMock: sm, actionPreparerMock: apm}, nil) sm.On("pre").Return(func(ctx context.Context) error { @@ -304,8 +300,8 @@ func TestNewJobExecutorDownloadsAllActionsBeforeTheFirstStep(t *testing.T) { } assert.Equal(t, []string{ "Prepare all required actions", - "Download action repository 'actions/checkout@v1'", - "Download action repository 'actions/checkout@v2'", + "Download action repository 'actions/checkout@v1' (SHA:sha1)", + "Download action repository 'actions/checkout@v2' (SHA:sha2)", "Complete job name: test", "Run Pre 1", "Run Pre 2", diff --git a/act/runner/patch_actions_test.go b/act/runner/patch_actions_test.go index bc7e8e4f..8d194b3c 100644 --- a/act/runner/patch_actions_test.go +++ b/act/runner/patch_actions_test.go @@ -223,8 +223,6 @@ func TestActionScriptPaths(t *testing.T) { assert.Nil(t, actionScriptPaths("/a", escaping)) } -// The bundle has to be patched whatever state the shared action directory is in, because a -// concurrent job's prepare checks the action out again and resets it. func TestPatchActionsAtTheContainerCopy(t *testing.T) { copiedBundle := func(t *testing.T, noPatch bool) string { t.Helper() @@ -234,12 +232,13 @@ func TestPatchActionsAtTheContainerCopy(t *testing.T) { Step: &model.Step{Uses: "owner/repo/sub@v1"}, remoteAction: &remoteAction{Org: "owner", Repo: "repo", Path: "sub", Ref: "v1"}, action: &model.Action{Runs: model.ActionRuns{Using: "node20", Main: "index.js"}}, + actionDir: t.TempDir(), RunContext: &RunContext{ Config: &Config{ActionCacheDir: t.TempDir(), NoActionPatch: noPatch}, JobContainer: cm, }, } - script := filepath.Join(sar.actionDir(), "sub", "index.js") + script := filepath.Join(sar.actionDir, "sub", "index.js") require.NoError(t, os.MkdirAll(filepath.Dir(script), 0o755)) require.NoError(t, os.WriteFile(script, []byte(gateTSC), 0o600)) @@ -250,7 +249,7 @@ func TestPatchActionsAtTheContainerCopy(t *testing.T) { copied = string(body) return nil }) - require.NoError(t, maybeCopyToActionDir(t.Context(), sar, sar.actionDir(), "sub", "/var/run/act/actions/repo/sub")) + require.NoError(t, maybeCopyToActionDir(t.Context(), sar, sar.actionDir, "sub", "/var/run/act/actions/repo/sub")) source, err := os.ReadFile(script) require.NoError(t, err) assert.Equal(t, gateTSC, string(source), "the shared copy is left as it was found") diff --git a/act/runner/reusable_workflow.go b/act/runner/reusable_workflow.go index bafa247b..b115343c 100644 --- a/act/runner/reusable_workflow.go +++ b/act/runner/reusable_workflow.go @@ -209,7 +209,9 @@ func (c Config) withInstanceAuth(input git.NewGitCloneExecutorInput, token strin return input } input.ClientCertFile, input.ClientKeyFile = c.ClientCertFile, c.ClientKeyFile - input.InsecureSkipTLS = c.InsecureSkipTLS && onInstanceHost(u.Host, c.GitHubInstance) + if onInstanceHost(u.Host, c.GitHubInstance) { + input.InsecureSkipTLS, input.Transport = c.InsecureSkipTLS, c.InstanceTransport + } if u.User == nil { input.Token = token } diff --git a/act/runner/reusable_workflow_test.go b/act/runner/reusable_workflow_test.go index 7df7a2a1..ee12d543 100644 --- a/act/runner/reusable_workflow_test.go +++ b/act/runner/reusable_workflow_test.go @@ -185,8 +185,12 @@ func TestInstanceAuthAndTransportOnlyApplyToTheInstance(t *testing.T) { InsecureSkipTLS: true, ClientCertFile: "/cert", ClientKeyFile: "/key", + InstanceTransport: &http.Transport{}, } want := git.NewGitCloneExecutorInput{URL: tt.cloneURL, InsecureSkipTLS: tt.insecure} + if tt.insecure { + want.Transport = conf.InstanceTransport + } if tt.token { want.Token = "token" } diff --git a/act/runner/run_context.go b/act/runner/run_context.go index 37dd629d..e0afdbd8 100644 --- a/act/runner/run_context.go +++ b/act/runner/run_context.go @@ -83,8 +83,9 @@ type RunContext struct { platformImage string // container.image or the runs-on pick, resolved once by isEnabled jobIndex, jobTotal int // strategy.job-index and job-total, total 0 when unknown - jobEnv map[string]string // workflow and job env as evaluated for the container and service specs - dockerActionImages map[*model.Step]string // Dockerfile action images built for this job, removed when it ends + jobEnv map[string]string // workflow and job env as evaluated for the container and service specs + dockerActionImages map[*model.Step]string // Dockerfile action images built for this job, removed when it ends + actionDownloads map[string]actionDownload // remote action repositories by clone URL and ref, fetched once per job // summaryFileInitialized tracks which per-step summary files (workflow/step-summary-N.md) // have already been created on the JobContainer. The runner sets up file-command files diff --git a/act/runner/step_action_local.go b/act/runner/step_action_local.go index ec715f8f..ad02e981 100644 --- a/act/runner/step_action_local.go +++ b/act/runner/step_action_local.go @@ -47,7 +47,7 @@ func (sal *stepActionLocal) main() common.Executor { defer rawLogger.Infof("::endgroup::") actionDir := filepath.Join(sal.getRunContext().Config.Workdir, sal.Step.Uses) - _, containerActionPath := getContainerActionPaths(sal.Step, path.Join(actionDir, ""), sal.RunContext) + _, containerActionPath := getContainerActionPaths(sal) localReader := func(filename string) (io.Reader, io.Closer, error) { spath := path.Join(containerActionPath, filename) @@ -127,8 +127,7 @@ func (sal *stepActionLocal) getActionModel() *model.Action { func (sal *stepActionLocal) getCompositeRunContext(ctx context.Context) (*RunContext, error) { if sal.compositeRunContext == nil { - actionDir := filepath.Join(sal.RunContext.Config.Workdir, sal.Step.Uses) - _, containerActionDir := getContainerActionPaths(sal.getStepModel(), actionDir, sal.RunContext) + _, containerActionDir := getContainerActionPaths(sal) compositeRunContext, err := newCompositeRunContext(ctx, sal.RunContext, sal, containerActionDir) if err != nil { diff --git a/act/runner/step_action_remote.go b/act/runner/step_action_remote.go index d13d1686..051dd7e2 100644 --- a/act/runner/step_action_remote.go +++ b/act/runner/step_action_remote.go @@ -30,13 +30,16 @@ type stepActionRemote struct { action *model.Action env map[string]string remoteAction *remoteAction - resolvedSha string + actionDir string // the repository at the resolved commit, shared by the job's actions in it + resolvedSha string // set on the step that made the job's download of the repository } -var stepActionRemoteNewCloneExecutor = git.NewGitCloneExecutor +type actionDownload struct { + dir string + err error +} -// selfRepoPrefix introduces a self-repository reference: the action lives in the repo holding the file that wrote the `uses:`. -const selfRepoPrefix = "$/" +var stepActionRemoteFetch = git.FetchAction func (sar *stepActionRemote) prepareActionExecutor() common.Executor { return func(ctx context.Context) error { @@ -64,61 +67,57 @@ func (sar *stepActionRemote) prepareActionExecutor() common.Executor { common.Logger(ctx).Debugf("Skipping local actions/checkout because workdir was already copied") return nil } - actionDir := sar.actionDir() - defaultActionURL := sar.RunContext.Config.DefaultActionURL() - // For Gitea - // github.Token, unlike Config.GetToken, survives the composite config copy, which nils Config.Secrets. - gitClone := stepActionRemoteNewCloneExecutor(sar.RunContext.Config.withInstanceAuth(git.NewGitCloneExecutorInput{ - URL: sar.remoteAction.CloneURL(defaultActionURL), - Ref: sar.remoteAction.Ref, - Dir: actionDir, - OfflineMode: sar.RunContext.Config.ActionOfflineMode, - Depth: sar.RunContext.Config.ActionCloneDepth, - // printPrepareActions reports the download with its resolved commit. - Quiet: true, - }, github.Token)) - if err := gitClone(ctx); err != nil { - var refErr *git.Error - switch { - case errors.As(err, &refErr) && errors.Is(err, git.ErrShortRef): - return fmt.Errorf("unable to resolve action `%s`, the provided ref `%s` is the shortened version of a commit SHA, which is not supported. Please use the full commit SHA `%s` instead", - sar.Step.Uses, sar.remoteAction.Ref, refErr.Commit()) - default: - return err + cloneURL := sar.remoteAction.CloneURL(sar.RunContext.Config.DefaultActionURL()) + job := sar.RunContext.topLevelRunContext() + key := cloneURL + "@" + sar.remoteAction.Ref + download, ok := job.actionDownloads[key] + if !ok { + // For Gitea + // github.Token, unlike Config.GetToken, survives the composite config copy, which nils Config.Secrets. + download.dir, sar.resolvedSha, download.err = stepActionRemoteFetch(ctx, sar.RunContext.Config.withInstanceAuth(git.NewGitCloneExecutorInput{ + URL: cloneURL, + Ref: sar.remoteAction.Ref, + Dir: filepath.Join(sar.RunContext.ActionCacheDir(), model.UsesHash(key)), + OfflineMode: sar.RunContext.Config.ActionOfflineMode, + Depth: sar.RunContext.Config.ActionCloneDepth, + }, github.Token)) + if job.actionDownloads == nil { + job.actionDownloads = map[string]actionDownload{} } + job.actionDownloads[key] = download } + var refErr *git.Error + switch { + case errors.As(download.err, &refErr) && errors.Is(download.err, git.ErrShortRef): + return fmt.Errorf("unable to resolve action `%s`, the provided ref `%s` is the shortened version of a commit SHA, which is not supported. Please use the full commit SHA `%s` instead", + sar.Step.Uses, sar.remoteAction.Ref, refErr.Commit()) + case download.err != nil: + return download.err + } + sar.actionDir = download.dir - unlock, err := git.AcquireCloneLock(ctx, actionDir) + unlock, err := git.AcquireCloneLock(ctx, sar.actionDir) if err != nil { return err } defer unlock() - // Best effort: the download report falls back to the ref alone when the commit is unknown. - if _, sha, err := git.FindGitRevision(ctx, actionDir); err != nil { - common.Logger(ctx).Debugf("unable to resolve the commit of %s: %v", sar.remoteAction.Reference(), err) - } else { - sar.resolvedSha = sha - } - remoteReader := func(filename string) (io.Reader, io.Closer, error) { - f, err := os.Open(filepath.Join(actionDir, sar.remoteAction.Path, filename)) + f, err := os.Open(filepath.Join(sar.actionDir, sar.remoteAction.Path, filename)) return f, f, err } - actionModel, err := sar.readAction(ctx, sar.Step, actionDir, sar.remoteAction.Path, remoteReader, os.WriteFile) + actionModel, err := sar.readAction(ctx, sar.Step, sar.actionDir, sar.remoteAction.Path, remoteReader, os.WriteFile) sar.action = actionModel return err } } -// actionDownloadInfo reports the action this step downloaded and the commit it resolved to. ok is -// false when nothing was fetched, as for the local checkout of the workflow's own repository. -func (sar *stepActionRemote) actionDownloadInfo() (reference, sha string, ok bool) { - if sar.remoteAction == nil || sar.action == nil { - return "", "", false +func (sar *stepActionRemote) actionDownloadInfo() (reference, sha string) { // once per job, as actions/runner reports + if sar.resolvedSha == "" { + return "", "" } - return sar.remoteAction.Reference(), sar.resolvedSha, true + return fmt.Sprintf("%s/%s@%s", sar.remoteAction.Org, sar.remoteAction.Repo, sar.remoteAction.Ref), sar.resolvedSha } func (sar *stepActionRemote) pre() common.Executor { @@ -147,7 +146,7 @@ func (sar *stepActionRemote) main() common.Executor { return sar.RunContext.JobContainer.CopyDir(copyToPath, sar.RunContext.Config.Workdir+string(filepath.Separator)+".", sar.RunContext.Config.UseGitIgnore, false)(ctx) } - return sar.runAction(sar, sar.actionDir(), sar.remoteAction)(ctx) + return sar.runAction(sar, sar.actionDir, sar.remoteAction)(ctx) }), ) } @@ -156,13 +155,8 @@ func (sar *stepActionRemote) post() common.Executor { return runStepExecutor(sar, stepStagePost, runPostStep(sar)).If(hasPostStep(sar)).If(shouldRunPostStep(sar)) } -func (sar *stepActionRemote) actionDir() string { - uses := sar.Step.Uses - if strings.HasPrefix(uses, selfRepoPrefix) { - // The same `$/x` names a different action per enclosing repo, so key the cache on what it resolved to. - uses = sar.remoteAction.URL + "/" + sar.remoteAction.Reference() - } - return fmt.Sprintf("%s/%s", sar.RunContext.ActionCacheDir(), model.UsesHash(uses)) +func (sar *stepActionRemote) containerKey() string { + return model.UsesHash(sar.remoteAction.URL + "/" + sar.remoteAction.Reference()) } func (sar *stepActionRemote) getRunContext() *RunContext { @@ -213,9 +207,7 @@ func (sar *stepActionRemote) getActionModel() *model.Action { func (sar *stepActionRemote) getCompositeRunContext(ctx context.Context) (*RunContext, error) { if sar.compositeRunContext == nil { - actionDir := sar.actionDir() - actionLocation := path.Join(actionDir, sar.remoteAction.Path) - _, containerActionDir := getContainerActionPaths(sar.getStepModel(), actionLocation, sar.RunContext) + _, containerActionDir := getContainerActionPaths(sar) compositeRunContext, err := newCompositeRunContext(ctx, sar.RunContext, sar, containerActionDir) if err != nil { diff --git a/act/runner/step_action_remote_test.go b/act/runner/step_action_remote_test.go index 95b0fcc2..1c40368c 100644 --- a/act/runner/step_action_remote_test.go +++ b/act/runner/step_action_remote_test.go @@ -12,6 +12,7 @@ import ( "io" "os" "os/exec" + "path" "path/filepath" "strings" "testing" @@ -31,14 +32,10 @@ type stepActionRemoteMocks struct { mock.Mock } -func actionDirSuffix(suffix string) any { - return mock.MatchedBy(func(actionDir string) bool { return strings.HasSuffix(actionDir, suffix) }) -} - -func setCloneExecutor(t *testing.T, executor func(git.NewGitCloneExecutorInput) common.Executor) { - original := stepActionRemoteNewCloneExecutor - stepActionRemoteNewCloneExecutor = executor - t.Cleanup(func() { stepActionRemoteNewCloneExecutor = original }) +func setFetch(t *testing.T, fetch func(context.Context, git.NewGitCloneExecutorInput) (string, string, error)) { + original := stepActionRemoteFetch + stepActionRemoteFetch = fetch + t.Cleanup(func() { stepActionRemoteFetch = original }) } func TestShortSHAActionRejected(t *testing.T) { @@ -181,12 +178,10 @@ func TestStepActionRemote(t *testing.T) { sarm := &stepActionRemoteMocks{} clonedAction := false - - setCloneExecutor(t, func(input git.NewGitCloneExecutorInput) common.Executor { - return func(ctx context.Context) error { - clonedAction = true - return nil - } + actionDir := t.TempDir() + setFetch(t, func(_ctx context.Context, _input git.NewGitCloneExecutorInput) (string, string, error) { + clonedAction = true + return actionDir, "", nil }) sar := &stepActionRemote{ @@ -213,10 +208,10 @@ func TestStepActionRemote(t *testing.T) { sar.RunContext.ExprEval = sar.RunContext.NewExpressionEvaluator(ctx) if tt.mocks.read { - sarm.On("readAction", sar.Step, actionDirSuffix(sar.Step.UsesHash()), "", mock.Anything, mock.Anything).Return(&model.Action{}, nil) + sarm.On("readAction", sar.Step, actionDir, "", mock.Anything, mock.Anything).Return(&model.Action{}, nil) } if tt.mocks.run { - sarm.On("runAction", sar, actionDirSuffix(sar.Step.UsesHash()), mustNewRemoteAction(t, sar.Step.Uses)).Return(func(ctx context.Context) error { return tt.runError }) + sarm.On("runAction", sar, actionDir, mustNewRemoteAction(t, sar.Step.Uses)).Return(func(ctx context.Context) error { return tt.runError }) cm.On("Copy", "/var/run/act", mock.AnythingOfType("[]*container.FileEntry")).Return(noopExecutor) @@ -263,40 +258,60 @@ func TestStepActionRemote(t *testing.T) { } func TestStepActionRemotePrepare(t *testing.T) { - for _, test := range []struct { - name, uses, instance, actionPath, wantURL string - }{ - {name: "nested action", uses: "org/repo/path@ref", instance: "https://github.com", actionPath: "path", wantURL: "https://github.com/org/repo"}, - {name: "instance fallback", uses: "actions/setup-go@v4", instance: "gitea.example", wantURL: "https://gitea.example/actions/setup-go"}, - } { - t.Run(test.name, func(t *testing.T) { - var actualURL string - setCloneExecutor(t, func(input git.NewGitCloneExecutorInput) common.Executor { - return func(context.Context) error { - actualURL = input.URL - return nil - } - }) - - actionMocks := &stepActionRemoteMocks{} - action := &stepActionRemote{ - Step: &model.Step{Uses: test.uses}, - RunContext: &RunContext{ - Config: &Config{GitHubInstance: test.instance, ActionCacheDir: t.TempDir()}, - Run: &model.Run{JobID: "1", Workflow: &model.Workflow{ - Jobs: map[string]*model.Job{"1": {}}, - }}, - }, - readAction: actionMocks.readAction, - } - actionMocks.On("readAction", action.Step, actionDirSuffix(action.Step.UsesHash()), test.actionPath, - mock.Anything, mock.Anything).Return(&model.Action{}, nil) - - require.NoError(t, action.prepareActionExecutor()(t.Context())) - assert.Equal(t, test.wantURL, actualURL) - actionMocks.AssertExpectations(t) - }) + var fetched, read []string + setFetch(t, func(_ctx context.Context, input git.NewGitCloneExecutorInput) (string, string, error) { + fetched = append(fetched, input.URL+"@"+input.Ref) + if input.Ref == "missing" { + return "", "", assert.AnError + } + return filepath.Join(input.Dir, "commit"), "sha-" + input.Ref, nil + }) + newJob := func(instance string) *RunContext { + return &RunContext{ + Config: &Config{GitHubInstance: instance, ActionCacheDir: t.TempDir()}, + Run: &model.Run{JobID: "1", Workflow: &model.Workflow{Jobs: map[string]*model.Job{"1": {}}}}, + JobContainer: &jobContainerMock{}, + } } + prepare := func(rc *RunContext, uses string) (*stepActionRemote, error) { + sar := &stepActionRemote{Step: &model.Step{Uses: uses}, RunContext: rc, readAction: func(_ctx context.Context, _step *model.Step, _actionDir, actionPath string, _readFile actionYamlReader, _writeFile fileWriter) (*model.Action, error) { + read = append(read, actionPath) + return &model.Action{}, nil + }} + return sar, sar.prepareActionExecutor()(t.Context()) + } + + job := newJob("https://github.com") + restore, err := prepare(job, "org/repo/restore@v1") + require.NoError(t, err) + root, err := prepare(job, "org/repo@v1") + require.NoError(t, err) + nested, err := prepare(&RunContext{Config: job.Config, Run: job.Run, JobContainer: job.JobContainer, Parent: job}, "org/repo/save@v1") + require.NoError(t, err) + other, err := prepare(job, "org/repo@v2") + require.NoError(t, err) + _, err = prepare(newJob("gitea.example"), "org/repo@v1") + require.NoError(t, err) + for range 2 { + _, err = prepare(job, "org/repo@missing") + require.ErrorIs(t, err, assert.AnError) + } + + assert.Equal(t, []string{"https://github.com/org/repo@v1", "https://github.com/org/repo@v2", "https://gitea.example/org/repo@v1", "https://github.com/org/repo@missing"}, fetched) + assert.Equal(t, []string{"restore", "", "save", "", ""}, read) + assert.Equal(t, root.actionDir, restore.actionDir) + assert.Equal(t, root.actionDir, nested.actionDir) + assert.NotEqual(t, root.actionDir, other.actionDir) + + reference, sha := restore.actionDownloadInfo() + assert.Equal(t, []string{"org/repo@v1", "sha-v1"}, []string{reference, sha}) + _, sha = root.actionDownloadInfo() + assert.Empty(t, sha) + + _, rootContainerDir := getContainerActionPaths(root) + _, restoreContainerDir := getContainerActionPaths(restore) + assert.NotEqual(t, path.Dir(rootContainerDir), path.Dir(restoreContainerDir)) + assert.Equal(t, "restore", path.Base(restoreContainerDir)) } func TestStepActionRemotePost(t *testing.T) { @@ -701,17 +716,15 @@ func Test_newRemoteActionSelfRepo(t *testing.T) { } } -func Test_stepActionRemoteSelfRepoActionDir(t *testing.T) { - dirFor := func(repo string) string { +func Test_stepActionRemoteSelfRepoContainerKeyDiffersPerRepo(t *testing.T) { + keyFor := func(repo string) string { sar := &stepActionRemote{ Step: &model.Step{Uses: "$/.gitea/actions/build"}, - RunContext: &RunContext{Config: &Config{ActionCacheDir: "/cache"}}, remoteAction: &remoteAction{Org: "owner", Repo: repo, Ref: "v1"}, } - return sar.actionDir() + return sar.containerKey() } - // The same `$/x` in two repos must not share a cache directory. - assert.NotEqual(t, dirFor("one"), dirFor("two")) + assert.NotEqual(t, keyFor("one"), keyFor("two")) } func Test_remoteActionReference(t *testing.T) { @@ -774,37 +787,11 @@ func TestStepActionRemotePreResolvesDownloadedCommit(t *testing.T) { require.NoError(t, sar.prepareActionExecutor()(t.Context())) - reference, sha, ok := sar.actionDownloadInfo() - assert.True(t, ok) + reference, sha := sar.actionDownloadInfo() assert.Equal(t, "actions/setup-go@main", reference) assert.Equal(t, wantSha, sha) } -func TestStepActionRemoteActionDownloadInfo(t *testing.T) { - t.Run("reports the action and its resolved commit", func(t *testing.T) { - sar := &stepActionRemote{ - remoteAction: mustNewRemoteAction(t, "actions/checkout@v7"), - action: &model.Action{}, - resolvedSha: "9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0", - } - - reference, sha, ok := sar.actionDownloadInfo() - - assert.True(t, ok) - assert.Equal(t, "actions/checkout@v7", reference) - assert.Equal(t, "9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0", sha) - }) - - t.Run("reports nothing when no action was downloaded", func(t *testing.T) { - // The local checkout of the workflow's own repository resolves no action. - sar := &stepActionRemote{remoteAction: mustNewRemoteAction(t, "actions/checkout@v7")} - - _, _, ok := sar.actionDownloadInfo() - - assert.False(t, ok) - }) -} - func Test_safeFilename(t *testing.T) { tests := []struct { s string @@ -856,9 +843,10 @@ func TestStepActionRemoteCloneTokenSurvivesNilSecrets(t *testing.T) { ctx := t.Context() var capturedToken string - setCloneExecutor(t, func(input git.NewGitCloneExecutorInput) common.Executor { + actionDir := t.TempDir() + setFetch(t, func(_ctx context.Context, input git.NewGitCloneExecutorInput) (string, string, error) { capturedToken = input.Token - return func(ctx context.Context) error { return nil } + return actionDir, "", nil }) sarm := &stepActionRemoteMocks{} @@ -886,7 +874,7 @@ func TestStepActionRemoteCloneTokenSurvivesNilSecrets(t *testing.T) { } sar.RunContext.ExprEval = sar.RunContext.NewExpressionEvaluator(ctx) - sarm.On("readAction", sar.Step, actionDirSuffix(sar.Step.UsesHash()), "", mock.Anything, mock.Anything).Return(&model.Action{}, nil) + sarm.On("readAction", sar.Step, actionDir, "", mock.Anything, mock.Anything).Return(&model.Action{}, nil) err := sar.prepareActionExecutor()(ctx) require.NoError(t, err) diff --git a/internal/pkg/config/config.example.yaml b/internal/pkg/config/config.example.yaml index b91a49d2..1b74c098 100644 --- a/internal/pkg/config/config.example.yaml +++ b/internal/pkg/config/config.example.yaml @@ -91,11 +91,12 @@ runner: # it replaces https://github.com with the value here, which is useful for some special network environments. #github_mirror: '' # Extra HTTP headers for runner API requests to Gitea, e.g. reverse proxy authentication. - # Not used for action clones, artifacts, job summaries or jobs. Runner headers take precedence. + # Not used for action downloads, artifacts, job summaries or jobs. Runner headers take precedence. # Dropped on redirects to another hostname, or to HTTP if the instance URL uses HTTPS. #extra_headers: # X-Proxy-Token: 'example' - # When true (the default), fetch only the requested ref of an action repository (e.g. actions/checkout@v4) at depth 1 instead of cloning every branch's full history. + # When true (the default), clone only the requested ref at depth 1 instead of every branch's full history. Actions download as + # archives, so this applies to reusable workflows and to actions whose host serves no archive of them. # Set to false to clone the full history. #action_shallow_clone: true # When true (the default), inject the ACT=true environment variable into jobs. diff --git a/internal/pkg/config/config.go b/internal/pkg/config/config.go index 0ff35ff2..8bbc67e1 100644 --- a/internal/pkg/config/config.go +++ b/internal/pkg/config/config.go @@ -81,7 +81,7 @@ type Runner struct { ReportCloseTimeout time.Duration `yaml:"report_close_timeout"` // ReportCloseTimeout caps each RPC attempt when flushing the final logs and task state at job completion, on a detached context so a server cancel can't block the acknowledgement. Labels []string `yaml:"labels"` // Labels specify the labels of the runner. Labels are declared on each startup GithubMirror string `yaml:"github_mirror"` // GithubMirror defines what mirrors should be used when using github - ActionShallowClone *bool `yaml:"action_shallow_clone"` // ActionShallowClone fetches only the requested ref of an action repository at depth 1 instead of cloning every branch's full history. It is a pointer to distinguish between false and not set; if not set, it defaults to true. + ActionShallowClone *bool `yaml:"action_shallow_clone"` // ActionShallowClone clones only the requested ref at depth 1 instead of every branch's full history, for reusable workflows and actions without an archive. It is a pointer to distinguish between false and not set; if not set, it defaults to true. SetActEnv *bool `yaml:"set_act_env"` // SetActEnv controls whether the ACT=true environment variable is injected into jobs. It is a pointer to distinguish between false and not set; if not set, it defaults to true. Set it to false so workflows gated on `if: ${{ !env.ACT }}` behave like on GitHub. PatchActions *bool `yaml:"patch_actions"` // PatchActions applies compatibility patches to the actions a job runs, so actions written for GitHub work against Gitea, see act/runner/patch_actions.go. It is a pointer to distinguish between false and not set; if not set, it defaults to true. Set it to false to run every action exactly as published, at the price of the artifact actions refusing. AllocatePTY bool `yaml:"allocate_pty"` // AllocatePTY allocates a pseudo-TTY for each step's process. Default is false, matching GitHub's actions/runner. Enable only for jobs that need an interactive terminal; tools like docker build emit redrawing progress frames into the captured log when a TTY is present. Applies to both host and docker backends.