diff --git a/act/artifactcache/handler.go b/act/artifactcache/handler.go index a38a11a2..3354aea1 100644 --- a/act/artifactcache/handler.go +++ b/act/artifactcache/handler.go @@ -427,8 +427,10 @@ func (h *Handler) reserve(w http.ResponseWriter, r *http.Request, _ httprouter.P return } - cache := api.ToCache() - cache.Repo = cred.Repo + cache := &Cache{Repo: cred.Repo, Key: api.Key, Version: api.Version, Size: api.Size} + if cache.Size == 0 { + cache.Size = -1 + } db, err := h.openDB() if err != nil { h.responseJSON(w, r, 500, err) diff --git a/act/artifactcache/model.go b/act/artifactcache/model.go index c550f63b..c7df2479 100644 --- a/act/artifactcache/model.go +++ b/act/artifactcache/model.go @@ -10,23 +10,6 @@ type Request struct { Size int64 `json:"cacheSize"` } -func (c *Request) ToCache() *Cache { - if c == nil { - return nil - } - ret := &Cache{ - Key: c.Key, - Version: c.Version, - Size: c.Size, - } - if c.Size == 0 { - // So the request comes from old versions of actions, like `actions/cache@v2`. - // It doesn't send cache size. Set it to -1 to indicate that. - ret.Size = -1 - } - return ret -} - type Cache struct { ID uint64 `json:"id" boltholdKey:"ID"` Repo string `json:"repo" boltholdIndex:"Repo"` diff --git a/act/artifacts/server.go b/act/artifacts/server.go index fb4e8b6b..e6d27efd 100644 --- a/act/artifacts/server.go +++ b/act/artifacts/server.go @@ -50,65 +50,29 @@ type ResponseMessage struct { Message string `json:"message"` } -type WritableFile interface { - io.WriteCloser -} - -type WriteFS interface { - OpenWritable(name string) (WritableFile, error) - OpenAppendable(name string) (WritableFile, error) -} - -type readWriteFSImpl struct{} - -func (fwfs readWriteFSImpl) Open(name string) (fs.File, error) { - return os.Open(name) -} - -func (fwfs readWriteFSImpl) OpenWritable(name string) (WritableFile, error) { - if err := os.MkdirAll(filepath.Dir(name), os.ModePerm); err != nil { - return nil, err - } - return os.OpenFile(name, os.O_CREATE|os.O_RDWR|os.O_TRUNC, 0o644) -} - -func (fwfs readWriteFSImpl) OpenAppendable(name string) (WritableFile, error) { - if err := os.MkdirAll(filepath.Dir(name), os.ModePerm); err != nil { - return nil, err - } - file, err := os.OpenFile(name, os.O_CREATE|os.O_RDWR, 0o644) - if err != nil { - return nil, err - } - - _, err = file.Seek(0, io.SeekEnd) - if err != nil { - return nil, err - } - return file, nil -} - var gzipExtension = ".gz__" func safeResolve(baseDir, relPath string) string { return filepath.Join(baseDir, filepath.Clean(filepath.Join(string(os.PathSeparator), relPath))) } -func uploads(router *httprouter.Router, baseDir string, fsys WriteFS) { +func writeJSON(w http.ResponseWriter, value any) { + data, err := json.Marshal(value) + if err != nil { + panic(err) + } + if _, err := w.Write(data); err != nil { + panic(err) + } +} + +func uploads(router *httprouter.Router, baseDir string) { router.POST("/_apis/pipelines/workflows/:runId/artifacts", func(w http.ResponseWriter, req *http.Request, params httprouter.Params) { runID := params.ByName("runId") - json, err := json.Marshal(FileContainerResourceURL{ + writeJSON(w, FileContainerResourceURL{ FileContainerResourceURL: fmt.Sprintf("http://%s/upload/%s", req.Host, runID), }) - if err != nil { - panic(err) - } - - _, err = w.Write(json) - if err != nil { - panic(err) - } }) router.PUT("/upload/:runId", func(w http.ResponseWriter, req *http.Request, params httprouter.Params) { @@ -122,67 +86,47 @@ func uploads(router *httprouter.Router, baseDir string, fsys WriteFS) { safeRunPath := safeResolve(baseDir, runID) safePath := safeResolve(safeRunPath, itemPath) - file, err := func() (WritableFile, error) { - contentRange := req.Header.Get("Content-Range") - if contentRange != "" && !strings.HasPrefix(contentRange, "bytes 0-") { - return fsys.OpenAppendable(safePath) - } - return fsys.OpenWritable(safePath) - }() + if err := os.MkdirAll(filepath.Dir(safePath), os.ModePerm); err != nil { + panic(err) + } + flags := os.O_CREATE | os.O_WRONLY | os.O_TRUNC + appendUpload := req.Header.Get("Content-Range") + if appendUpload != "" && !strings.HasPrefix(appendUpload, "bytes 0-") { + flags = os.O_CREATE | os.O_WRONLY | os.O_APPEND + } + file, err := os.OpenFile(safePath, flags, 0o644) if err != nil { panic(err) } defer file.Close() - - writer, ok := file.(io.Writer) - if !ok { - panic(errors.New("File is not writable")) - } - if req.Body == nil { panic(errors.New("No body given")) } - _, err = io.Copy(writer, req.Body) + _, err = io.Copy(file, req.Body) if err != nil { panic(err) } - json, err := json.Marshal(ResponseMessage{ + writeJSON(w, ResponseMessage{ Message: "success", }) - if err != nil { - panic(err) - } - - _, err = w.Write(json) - if err != nil { - panic(err) - } }) router.PATCH("/_apis/pipelines/workflows/:runId/artifacts", func(w http.ResponseWriter, req *http.Request, params httprouter.Params) { - json, err := json.Marshal(ResponseMessage{ + writeJSON(w, ResponseMessage{ Message: "success", }) - if err != nil { - panic(err) - } - - _, err = w.Write(json) - if err != nil { - panic(err) - } }) } -func downloads(router *httprouter.Router, baseDir string, fsys fs.FS) { +func downloads(router *httprouter.Router, baseDir string) { router.GET("/_apis/pipelines/workflows/:runId/artifacts", func(w http.ResponseWriter, req *http.Request, params httprouter.Params) { runID := params.ByName("runId") safePath := safeResolve(baseDir, runID) - entries, err := fs.ReadDir(fsys, safePath) + entries, err := os.ReadDir(safePath) if err != nil { panic(err) } @@ -195,18 +139,10 @@ func downloads(router *httprouter.Router, baseDir string, fsys fs.FS) { }) } - json, err := json.Marshal(NamedFileContainerResourceURLResponse{ + writeJSON(w, NamedFileContainerResourceURLResponse{ Count: len(list), Value: list, }) - if err != nil { - panic(err) - } - - _, err = w.Write(json) - if err != nil { - panic(err) - } }) router.GET("/download/:container", func(w http.ResponseWriter, req *http.Request, params httprouter.Params) { @@ -215,7 +151,7 @@ func downloads(router *httprouter.Router, baseDir string, fsys fs.FS) { safePath := safeResolve(baseDir, filepath.Join(container, itemPath)) var files []ContainerItem - err := fs.WalkDir(fsys, safePath, func(path string, entry fs.DirEntry, err error) error { + err := filepath.WalkDir(safePath, func(path string, entry fs.DirEntry, err error) error { if !entry.IsDir() { rel, err := filepath.Rel(safePath, path) if err != nil { @@ -241,17 +177,9 @@ func downloads(router *httprouter.Router, baseDir string, fsys fs.FS) { panic(err) } - json, err := json.Marshal(ContainerItemResponse{ + writeJSON(w, ContainerItemResponse{ Value: files, }) - if err != nil { - panic(err) - } - - _, err = w.Write(json) - if err != nil { - panic(err) - } }) router.GET("/artifact/*path", func(w http.ResponseWriter, req *http.Request, params httprouter.Params) { @@ -259,15 +187,16 @@ func downloads(router *httprouter.Router, baseDir string, fsys fs.FS) { safePath := safeResolve(baseDir, path) - file, err := fsys.Open(safePath) + file, err := os.Open(safePath) if err != nil { // try gzip file - file, err = fsys.Open(safePath + gzipExtension) + file, err = os.Open(safePath + gzipExtension) if err != nil { panic(err) } w.Header().Add("Content-Encoding", "gzip") } + defer file.Close() _, err = io.Copy(w, file) if err != nil { @@ -287,9 +216,8 @@ func Serve(ctx context.Context, artifactPath, addr, port string) context.CancelF router := httprouter.New() logger.Debugf("Artifacts base path '%s'", artifactPath) - fsys := readWriteFSImpl{} - uploads(router, artifactPath, fsys) - downloads(router, artifactPath, fsys) + uploads(router, artifactPath) + downloads(router, artifactPath) server := &http.Server{ Addr: fmt.Sprintf("%s:%s", addr, port), diff --git a/act/artifacts/server_test.go b/act/artifacts/server_test.go index e7a56138..6db43d4a 100644 --- a/act/artifacts/server_test.go +++ b/act/artifacts/server_test.go @@ -8,7 +8,6 @@ import ( "bytes" "compress/gzip" "encoding/json" - "fmt" "io" "maps" "net/http" @@ -18,238 +17,18 @@ import ( "path/filepath" "strings" "testing" - "testing/fstest" "time" "github.com/julienschmidt/httprouter" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) -type writableMapFile struct { - fstest.MapFile -} - -func (f *writableMapFile) Write(data []byte) (int, error) { - f.Data = data - return len(data), nil -} - -func (f *writableMapFile) Close() error { - return nil -} - -type writeMapFS struct { - fstest.MapFS -} - -func (fsys writeMapFS) OpenWritable(name string) (WritableFile, error) { - file := &writableMapFile{ - MapFile: fstest.MapFile{ - Data: []byte("content2"), - }, - } - fsys.MapFS[name] = &file.MapFile - - return file, nil -} - -func (fsys writeMapFS) OpenAppendable(name string) (WritableFile, error) { - file := &writableMapFile{ - MapFile: fstest.MapFile{ - Data: []byte("content2"), - }, - } - fsys.MapFS[name] = &file.MapFile - - return file, nil -} - -func TestNewArtifactUploadPrepare(t *testing.T) { - assert := assert.New(t) - - memfs := fstest.MapFS(map[string]*fstest.MapFile{}) - - router := httprouter.New() - uploads(router, "artifact/server/path", writeMapFS{memfs}) - - req, _ := http.NewRequest(http.MethodPost, "http://localhost/_apis/pipelines/workflows/1/artifacts", nil) - rr := httptest.NewRecorder() - - router.ServeHTTP(rr, req) - - if status := rr.Code; status != http.StatusOK { - assert.Fail("Wrong status") - } - - response := FileContainerResourceURL{} - err := json.Unmarshal(rr.Body.Bytes(), &response) - if err != nil { - panic(err) - } - - assert.Equal("http://localhost/upload/1", response.FileContainerResourceURL) -} - -func TestArtifactUploadBlob(t *testing.T) { - assert := assert.New(t) - - memfs := fstest.MapFS(map[string]*fstest.MapFile{}) - - router := httprouter.New() - uploads(router, "artifact/server/path", writeMapFS{memfs}) - - req, _ := http.NewRequest(http.MethodPut, "http://localhost/upload/1?itemPath=some/file", strings.NewReader("content")) - rr := httptest.NewRecorder() - - router.ServeHTTP(rr, req) - - if status := rr.Code; status != http.StatusOK { - assert.Fail("Wrong status") - } - - response := ResponseMessage{} - err := json.Unmarshal(rr.Body.Bytes(), &response) - if err != nil { - panic(err) - } - - assert.Equal("success", response.Message) - assert.Equal("content", string(memfs["artifact/server/path/1/some/file"].Data)) -} - -func TestFinalizeArtifactUpload(t *testing.T) { - assert := assert.New(t) - - memfs := fstest.MapFS(map[string]*fstest.MapFile{}) - - router := httprouter.New() - uploads(router, "artifact/server/path", writeMapFS{memfs}) - - req, _ := http.NewRequest(http.MethodPatch, "http://localhost/_apis/pipelines/workflows/1/artifacts", nil) - rr := httptest.NewRecorder() - - router.ServeHTTP(rr, req) - - if status := rr.Code; status != http.StatusOK { - assert.Fail("Wrong status") - } - - response := ResponseMessage{} - err := json.Unmarshal(rr.Body.Bytes(), &response) - if err != nil { - panic(err) - } - - assert.Equal("success", response.Message) -} - -func TestListArtifacts(t *testing.T) { - assert := assert.New(t) - - memfs := fstest.MapFS(map[string]*fstest.MapFile{ - "artifact/server/path/1/file.txt": { - Data: []byte(""), - }, - }) - - router := httprouter.New() - downloads(router, "artifact/server/path", memfs) - - req, _ := http.NewRequest(http.MethodGet, "http://localhost/_apis/pipelines/workflows/1/artifacts", nil) - rr := httptest.NewRecorder() - - router.ServeHTTP(rr, req) - - if status := rr.Code; status != http.StatusOK { - assert.FailNow(fmt.Sprintf("Wrong status: %d", status)) - } - - response := NamedFileContainerResourceURLResponse{} - err := json.Unmarshal(rr.Body.Bytes(), &response) - if err != nil { - panic(err) - } - - assert.Equal(1, response.Count) - assert.Equal("file.txt", response.Value[0].Name) - assert.Equal("http://localhost/download/1", response.Value[0].FileContainerResourceURL) -} - -func TestListArtifactContainer(t *testing.T) { - assert := assert.New(t) - - memfs := fstest.MapFS(map[string]*fstest.MapFile{ - "artifact/server/path/1/some/file": { - Data: []byte(""), - }, - }) - - router := httprouter.New() - downloads(router, "artifact/server/path", memfs) - - req, _ := http.NewRequest(http.MethodGet, "http://localhost/download/1?itemPath=some/file", nil) - rr := httptest.NewRecorder() - - router.ServeHTTP(rr, req) - - if status := rr.Code; status != http.StatusOK { - assert.FailNow(fmt.Sprintf("Wrong status: %d", status)) - } - - response := ContainerItemResponse{} - err := json.Unmarshal(rr.Body.Bytes(), &response) - if err != nil { - panic(err) - } - - assert.Len(response.Value, 1) - assert.Equal("some/file", response.Value[0].Path) - assert.Equal("file", response.Value[0].ItemType) - assert.Equal("http://localhost/artifact/1/some/file/.", response.Value[0].ContentLocation) -} - -func TestDownloadArtifactFile(t *testing.T) { - assert := assert.New(t) - - memfs := fstest.MapFS(map[string]*fstest.MapFile{ - "artifact/server/path/1/some/file": { - Data: []byte("content"), - }, - }) - - router := httprouter.New() - downloads(router, "artifact/server/path", memfs) - - req, _ := http.NewRequest(http.MethodGet, "http://localhost/artifact/1/some/file", nil) - rr := httptest.NewRecorder() - - router.ServeHTTP(rr, req) - - if status := rr.Code; status != http.StatusOK { - assert.FailNow(fmt.Sprintf("Wrong status: %d", status)) - } - - data := rr.Body.Bytes() - - assert.Equal("content", string(data)) -} - -// TestArtifactFlow drives the real Serve() artifact server over a loopback socket, exercising -// the same upload -> finalize -> list -> download protocol the upload-artifact/download-artifact -// actions speak. Running it in-process (rather than from a job container) keeps it network-free -// and reachable everywhere, including when the CI job is itself a container. func TestArtifactFlow(t *testing.T) { artifactPath := t.TempDir() - // Serve the exact routes Serve() wires up, on a real loopback socket via httptest. httptest - // picks a free port and Close() tears the server down synchronously — avoiding both the - // port-rebind race and Serve()'s detached ListenAndServe goroutine, which logger.Fatal()s - // (process exit) on a bind error and can outlive the test's temp-dir cleanup. router := httprouter.New() - fsys := readWriteFSImpl{} - uploads(router, artifactPath, fsys) - downloads(router, artifactPath, fsys) + uploads(router, artifactPath) + downloads(router, artifactPath) server := httptest.NewServer(router) defer server.Close() @@ -257,8 +36,6 @@ func TestArtifactFlow(t *testing.T) { client := server.Client() client.Timeout = 5 * time.Second - // request performs one HTTP call and returns the status and body. The default transport adds - // Accept-Encoding: gzip and transparently decompresses, so gzipped downloads come back plain. request := func(t *testing.T, method, rawURL string, body io.Reader, header http.Header) (int, []byte) { t.Helper() req, err := http.NewRequest(method, rawURL, body) @@ -289,6 +66,8 @@ func TestArtifactFlow(t *testing.T) { status, data = request(t, http.MethodPatch, baseURL+"/_apis/pipelines/workflows/"+runID+"/artifacts", nil, nil) require.Equal(t, http.StatusOK, status, string(data)) + require.NoError(t, json.Unmarshal(data, &msg)) + require.Equal(t, "success", msg.Message) status, data = request(t, http.MethodGet, baseURL+"/_apis/pipelines/workflows/"+runID+"/artifacts", nil, nil) require.Equal(t, http.StatusOK, status, string(data)) @@ -314,6 +93,21 @@ func TestArtifactFlow(t *testing.T) { require.Equal(t, content, string(stored)) }) + t.Run("content-range", func(t *testing.T) { + const rawURL = "/upload/4?itemPath=chunks.txt" + status, data := request(t, http.MethodPut, baseURL+rawURL, strings.NewReader("first"), + http.Header{"Content-Range": []string{"bytes 0-4/11"}}) + require.Equal(t, http.StatusOK, status, string(data)) + + status, data = request(t, http.MethodPut, baseURL+rawURL, strings.NewReader("-second"), + http.Header{"Content-Range": []string{"bytes 5-11/11"}}) + require.Equal(t, http.StatusOK, status, string(data)) + + stored, err := os.ReadFile(filepath.Join(artifactPath, "4", "chunks.txt")) + require.NoError(t, err) + require.Equal(t, "first-second", string(stored)) + }) + t.Run("gzip-roundtrip", func(t *testing.T) { const runID, item, content = "2", "logs/app.log", "compressed payload\n" @@ -365,9 +159,7 @@ func TestArtifactFlow(t *testing.T) { }) } -func TestMkdirFsImplSafeResolve(t *testing.T) { - assert := assert.New(t) - +func TestSafeResolve(t *testing.T) { baseDir := "/foo/bar" tests := map[string]struct { @@ -385,97 +177,13 @@ func TestMkdirFsImplSafeResolve(t *testing.T) { for name, tc := range tests { t.Run(name, func(t *testing.T) { - assert.Equal(tc.want, safeResolve(baseDir, tc.input)) + require.Equal(t, tc.want, safeResolve(baseDir, tc.input)) }) } } -func TestReadWriteFSWritableAndAppendable(t *testing.T) { - fsys := readWriteFSImpl{} - name := filepath.Join(t.TempDir(), "nested", "artifact.txt") - - w, err := fsys.OpenWritable(name) - require.NoError(t, err) - _, err = w.Write([]byte("first")) - require.NoError(t, err) - require.NoError(t, w.Close()) - - w, err = fsys.OpenAppendable(name) - require.NoError(t, err) - _, err = w.Write([]byte("-second")) - require.NoError(t, err) - require.NoError(t, w.Close()) - - got, err := os.ReadFile(name) - require.NoError(t, err) - require.Equal(t, "first-second", string(got)) - - w, err = fsys.OpenWritable(name) - require.NoError(t, err) - _, err = w.Write([]byte("replaced")) - require.NoError(t, err) - require.NoError(t, w.Close()) - - got, err = os.ReadFile(name) - require.NoError(t, err) - require.Equal(t, "replaced", string(got)) -} - func TestServeEmptyArtifactPathReturnsCancelableNoop(t *testing.T) { cancel := Serve(t.Context(), "", "127.0.0.1", "0") require.NotNil(t, cancel) cancel() } - -func TestDownloadArtifactFileUnsafePath(t *testing.T) { - assert := assert.New(t) - - memfs := fstest.MapFS(map[string]*fstest.MapFile{ - "artifact/server/path/some/file": { - Data: []byte("content"), - }, - }) - - router := httprouter.New() - downloads(router, "artifact/server/path", memfs) - - req, _ := http.NewRequest(http.MethodGet, "http://localhost/artifact/2/../../some/file", nil) - rr := httptest.NewRecorder() - - router.ServeHTTP(rr, req) - - if status := rr.Code; status != http.StatusOK { - assert.FailNow(fmt.Sprintf("Wrong status: %d", status)) - } - - data := rr.Body.Bytes() - - assert.Equal("content", string(data)) -} - -func TestArtifactUploadBlobUnsafePath(t *testing.T) { - assert := assert.New(t) - - memfs := fstest.MapFS(map[string]*fstest.MapFile{}) - - router := httprouter.New() - uploads(router, "artifact/server/path", writeMapFS{memfs}) - - req, _ := http.NewRequest(http.MethodPut, "http://localhost/upload/1?itemPath=../../some/file", strings.NewReader("content")) - rr := httptest.NewRecorder() - - router.ServeHTTP(rr, req) - - if status := rr.Code; status != http.StatusOK { - assert.Fail("Wrong status") - } - - response := ResponseMessage{} - err := json.Unmarshal(rr.Body.Bytes(), &response) - if err != nil { - panic(err) - } - - assert.Equal("success", response.Message) - assert.Equal("content", string(memfs["artifact/server/path/1/some/file"].Data)) -} diff --git a/act/common/executor.go b/act/common/executor.go index cc44cde7..1e33c79b 100644 --- a/act/common/executor.go +++ b/act/common/executor.go @@ -54,22 +54,6 @@ func NewPipelineExecutor(executors ...Executor) Executor { return rtn } -// NewConditionalExecutor creates a new executor based on conditions -func NewConditionalExecutor(conditional Conditional, trueExecutor, falseExecutor Executor) Executor { - return func(ctx context.Context) error { - if conditional(ctx) { - if trueExecutor != nil { - return trueExecutor(ctx) - } - } else { - if falseExecutor != nil { - return falseExecutor(ctx) - } - } - return nil - } -} - // NewErrorExecutor creates a new executor that always errors out func NewErrorExecutor(err error) Executor { return func(ctx context.Context) error { @@ -192,10 +176,3 @@ func (e Executor) Finally(finally Executor) Executor { return err } } - -// Not return an inverted conditional -func (c Conditional) Not() Conditional { - return func(ctx context.Context) bool { - return !c(ctx) - } -} diff --git a/act/common/executor_test.go b/act/common/executor_test.go index fd45684b..c10a66c1 100644 --- a/act/common/executor_test.go +++ b/act/common/executor_test.go @@ -45,43 +45,6 @@ func TestNewWorkflow(t *testing.T) { assert.Equal(2, runcount) } -func TestNewConditionalExecutor(t *testing.T) { - assert := assert.New(t) - - ctx := context.Background() - - trueCount := 0 - falseCount := 0 - - err := NewConditionalExecutor(func(ctx context.Context) bool { - return false - }, func(ctx context.Context) error { - trueCount++ - return nil - }, func(ctx context.Context) error { - falseCount++ - return nil - })(ctx) - - assert.NoError(err) //nolint:testifylint // pre-existing issue from nektos/act - assert.Equal(0, trueCount) - assert.Equal(1, falseCount) - - err = NewConditionalExecutor(func(ctx context.Context) bool { - return true - }, func(ctx context.Context) error { - trueCount++ - return nil - }, func(ctx context.Context) error { - falseCount++ - return nil - })(ctx) - - assert.NoError(err) //nolint:testifylint // pre-existing issue from nektos/act - assert.Equal(1, trueCount) - assert.Equal(1, falseCount) -} - // concurrencyProbe returns an executor recording the peak number of concurrent copies. Copies // block until wantActive are in flight so the peak is exact without sleeping, and later copies // find the gate already open so the last one still finishes with no partner left. @@ -223,10 +186,3 @@ func TestExecutorFinallyReturnsFinallyErrorWithOriginal(t *testing.T) { t.Fatalf("finally error = %q, want both cleanup and original error", err) } } - -func TestConditionalNot(t *testing.T) { - cond := Conditional(func(context.Context) bool { return false }) - if !cond.Not()(context.Background()) { - t.Fatal("inverted conditional should be true") - } -} diff --git a/act/common/git/git.go b/act/common/git/git.go index 9e4ca9b6..974bf58f 100644 --- a/act/common/git/git.go +++ b/act/common/git/git.go @@ -36,7 +36,6 @@ var ( cloneLocks lock.Keyed[string] // key: clone target directory ErrShortRef = errors.New("short SHA references are not supported") - ErrNoRepo = errors.New("unable to find git repo") ) // AcquireCloneLock returns an unlock function after locking the per-directory mutex for dir. @@ -187,19 +186,16 @@ func FindGitRef(ctx context.Context, file string) (string, error) { } // FindGithubRepo get the repo -func FindGithubRepo(ctx context.Context, file, githubInstance, remoteName string) (string, error) { +func FindGithubRepo(ctx context.Context, file, githubInstance string) (string, error) { goGitMu.Lock() defer goGitMu.Unlock() - if remoteName == "" { - remoteName = "origin" - } - url, err := findGitRemoteURL(ctx, file, remoteName) + url, err := findGitRemoteURL(ctx, file, "origin") if err != nil { return "", err } - _, slug, err := findGitSlug(url, githubInstance) - return slug, err + _, slug := findGitSlug(url, githubInstance) + return slug, nil } func findGitRemoteURL(_ context.Context, file, remoteName string) (string, error) { @@ -226,25 +222,25 @@ func findGitRemoteURL(_ context.Context, file, remoteName string) (string, error return remote.Config().URLs[0], nil } -func findGitSlug(url, githubInstance string) (string, string, error) { //nolint:unparam // pre-existing issue from nektos/act +func findGitSlug(url, githubInstance string) (string, string) { if matches := codeCommitHTTPRegex.FindStringSubmatch(url); matches != nil { - return "CodeCommit", matches[2], nil + return "CodeCommit", matches[2] } else if matches := codeCommitSSHRegex.FindStringSubmatch(url); matches != nil { - return "CodeCommit", matches[2], nil + return "CodeCommit", matches[2] } else if matches := githubHTTPRegex.FindStringSubmatch(url); matches != nil { - return "GitHub", fmt.Sprintf("%s/%s", matches[1], matches[2]), nil + return "GitHub", fmt.Sprintf("%s/%s", matches[1], matches[2]) } else if matches := githubSSHRegex.FindStringSubmatch(url); matches != nil { - return "GitHub", fmt.Sprintf("%s/%s", matches[1], matches[2]), nil + return "GitHub", fmt.Sprintf("%s/%s", matches[1], matches[2]) } else if githubInstance != "github.com" { gheHTTPRegex := regexp.MustCompile(fmt.Sprintf(`^https?://%s/(.+)/(.+?)(?:.git)?$`, githubInstance)) gheSSHRegex := regexp.MustCompile(githubInstance + "[:/](.+)/(.+?)(?:.git)?$") if matches := gheHTTPRegex.FindStringSubmatch(url); matches != nil { - return "GitHubEnterprise", fmt.Sprintf("%s/%s", matches[1], matches[2]), nil + return "GitHubEnterprise", fmt.Sprintf("%s/%s", matches[1], matches[2]) } else if matches := gheSSHRegex.FindStringSubmatch(url); matches != nil { - return "GitHubEnterprise", fmt.Sprintf("%s/%s", matches[1], matches[2]), nil + return "GitHubEnterprise", fmt.Sprintf("%s/%s", matches[1], matches[2]) } } - return "", url, nil + return "", url } // NewGitCloneExecutorInput the input for the NewGitCloneExecutor diff --git a/act/common/git/git_test.go b/act/common/git/git_test.go index 40bbddf2..04f6481a 100644 --- a/act/common/git/git_test.go +++ b/act/common/git/git_test.go @@ -51,9 +51,7 @@ func TestFindGitSlug(t *testing.T) { } for _, tt := range slugTests { - provider, slug, err := findGitSlug(tt.url, "github.com") - - assert.NoError(err) //nolint:testifylint // pre-existing issue from nektos/act + provider, slug := findGitSlug(tt.url, "github.com") assert.Equal(tt.provider, provider) assert.Equal(tt.slug, slug) } @@ -87,45 +85,20 @@ func cleanGitHooks(dir string) error { return nil } -func TestFindGitRemoteURL(t *testing.T) { - assert := assert.New(t) - - basedir := t.TempDir() - err := gitCmd("init", basedir) - assert.NoError(err) //nolint:testifylint // pre-existing issue from nektos/act - err = cleanGitHooks(basedir) - assert.NoError(err) //nolint:testifylint // pre-existing issue from nektos/act - - remoteURL := "https://git-codecommit.us-east-1.amazonaws.com/v1/repos/my-repo-name" - err = gitCmd("-C", basedir, "remote", "add", "origin", remoteURL) - assert.NoError(err) //nolint:testifylint // pre-existing issue from nektos/act - - u, err := findGitRemoteURL(context.Background(), basedir, "origin") - assert.NoError(err) //nolint:testifylint // pre-existing issue from nektos/act - assert.Equal(remoteURL, u) - - remoteURL = "git@github.com/AwesomeOwner/MyAwesomeRepo.git" - err = gitCmd("-C", basedir, "remote", "add", "upstream", remoteURL) - assert.NoError(err) //nolint:testifylint // pre-existing issue from nektos/act - u, err = findGitRemoteURL(context.Background(), basedir, "upstream") - assert.NoError(err) //nolint:testifylint // pre-existing issue from nektos/act - assert.Equal(remoteURL, u) -} - -func TestFindGithubRepoUsesOriginAndCustomRemote(t *testing.T) { +func TestFindGithubRepoUsesOrigin(t *testing.T) { basedir := t.TempDir() + const remoteURL = "https://github.com/owner/repo.git" require.NoError(t, gitCmd("init", basedir)) require.NoError(t, cleanGitHooks(basedir)) - require.NoError(t, gitCmd("-C", basedir, "remote", "add", "origin", "https://github.com/owner/repo.git")) - require.NoError(t, gitCmd("-C", basedir, "remote", "add", "ghe", "git@git.example.com:team/project.git")) + require.NoError(t, gitCmd("-C", basedir, "remote", "add", "origin", remoteURL)) - slug, err := FindGithubRepo(context.Background(), basedir, "github.com", "") + url, err := findGitRemoteURL(context.Background(), basedir, "origin") + require.NoError(t, err) + require.Equal(t, remoteURL, url) + + slug, err := FindGithubRepo(context.Background(), basedir, "github.com") require.NoError(t, err) require.Equal(t, "owner/repo", slug) - - slug, err = FindGithubRepo(context.Background(), basedir, "git.example.com", "ghe") - require.NoError(t, err) - require.Equal(t, "team/project", slug) } func TestGitFindRef(t *testing.T) { diff --git a/act/container/container_types.go b/act/container/container_types.go index f4df9700..5e0829d3 100644 --- a/act/container/container_types.go +++ b/act/container/container_types.go @@ -88,9 +88,7 @@ type Info struct { // Container for managing docker run containers type Container interface { Create(capAdd, capDrop []string) common.Executor - ConnectToNetwork(name string) common.Executor Copy(destPath string, files ...*FileEntry) common.Executor - CopyTarStream(ctx context.Context, destPath string, tarStream io.Reader) error CopyDir(destPath, srcPath string, useGitIgnore bool) common.Executor GetContainerArchive(ctx context.Context, srcPath string) (io.ReadCloser, error) Inspect(ctx context.Context) (*Info, error) diff --git a/act/container/docker_run.go b/act/container/docker_run.go index 00e5ad40..8110cc61 100644 --- a/act/container/docker_run.go +++ b/act/container/docker_run.go @@ -65,29 +65,6 @@ func NewContainer(input *NewContainerInput) ExecutionsEnvironment { return cr } -func (cr *containerReference) ConnectToNetwork(name string) common.Executor { - return common. - NewDebugExecutor("docker network connect %s %s", name, cr.input.Name). - Then( - common.NewPipelineExecutor( - cr.connect(), - cr.connectToNetwork(name, cr.input.NetworkAliases), - ).IfNot(common.Dryrun), - ) -} - -func (cr *containerReference) connectToNetwork(name string, aliases []string) common.Executor { - return func(ctx context.Context) error { - _, err := cr.cli.NetworkConnect(ctx, name, client.NetworkConnectOptions{ - Container: cr.input.Name, - EndpointConfig: &network.EndpointSettings{ - Aliases: aliases, - }, - }) - return err - } -} - // supportsContainerImagePlatform reports whether the Docker server API version // is 1.41 and beyond func supportsContainerImagePlatform(ctx context.Context, cli client.APIClient) (bool, error) { @@ -944,42 +921,6 @@ func (cr *containerReference) waitForCommand(ctx context.Context, resp client.Hi } } -func (cr *containerReference) CopyTarStream(ctx context.Context, destPath string, tarStream io.Reader) error { - if cr.id == "" { - return cr.missingContainerError("copy to %s", destPath) - } - // Mkdir, with a path relative to the DestinationPath ("/") below. Docker 29.5+ - // rejects absolute tar entry names with "path escapes from parent". - buf := &bytes.Buffer{} - tw := tar.NewWriter(buf) - _ = tw.WriteHeader(&tar.Header{ - Name: strings.TrimPrefix(destPath, "/"), - Mode: 0o777, - Typeflag: tar.TypeDir, - }) - tw.Close() - _, err := cr.cli.CopyToContainer(ctx, cr.id, client.CopyToContainerOptions{ - DestinationPath: "/", - Content: buf, - }) - if err != nil { - return fmt.Errorf("failed to mkdir to copy content to container: %w", err) - } - // Copy Content - _, err = cr.cli.CopyToContainer(ctx, cr.id, client.CopyToContainerOptions{ - DestinationPath: destPath, - Content: tarStream, - }) - if err != nil { - return fmt.Errorf("failed to copy content to container: %w", err) - } - // If this fails, then folders have wrong permissions on non root container - if cr.UID != 0 || cr.GID != 0 { - _ = cr.Exec([]string{"chown", "-R", fmt.Sprintf("%d:%d", cr.UID, cr.GID), destPath}, nil, "0", "")(ctx) - } - return nil -} - func (cr *containerReference) copyDir(dstPath, srcPath string, useGitIgnore bool) common.Executor { return func(ctx context.Context) error { if cr.id == "" { @@ -1021,7 +962,6 @@ func (cr *containerReference) copyDir(dstPath, srcPath string, useGitIgnore bool } fc := &filecollector.FileCollector{ - Fs: &filecollector.DefaultFs{}, Ignorer: ignorer, SrcPath: srcPath, SrcPrefix: srcPrefix, diff --git a/act/container/docker_run_test.go b/act/container/docker_run_test.go index 8a1eaed8..02a335bf 100644 --- a/act/container/docker_run_test.go +++ b/act/container/docker_run_test.go @@ -5,7 +5,6 @@ package container import ( - "archive/tar" "bufio" "bytes" "context" @@ -342,116 +341,6 @@ func TestDockerWaitFailure(t *testing.T) { client.AssertExpectations(t) } -func TestDockerCopyTarStream(t *testing.T) { - ctx := context.Background() - - client := &mockDockerClient{} - client.On("CopyToContainer", ctx, "123", mock.MatchedBy(func(opts mobyclient.CopyToContainerOptions) bool { - return opts.DestinationPath == "/" && opts.Content != nil - })).Return(mobyclient.CopyToContainerResult{}, nil) - client.On("CopyToContainer", ctx, "123", mock.MatchedBy(func(opts mobyclient.CopyToContainerOptions) bool { - return opts.DestinationPath == "/var/run/act" && opts.Content != nil - })).Return(mobyclient.CopyToContainerResult{}, nil) - cr := &containerReference{ - id: "123", - cli: client, - input: &NewContainerInput{ - Image: "image", - }, - } - - _ = cr.CopyTarStream(ctx, "/var/run/act", &bytes.Buffer{}) - - client.AssertExpectations(t) -} - -// Docker 29.5+ rejects absolute names in the mkdir tarball with -// "path escapes from parent", since it is extracted relative to "/". -func TestDockerCopyTarStreamMkdirEntryIsRelative(t *testing.T) { - ctx := context.Background() - - var mkdirNames []string - client := &mockDockerClient{} - client.On("CopyToContainer", ctx, "123", mock.MatchedBy(func(opts mobyclient.CopyToContainerOptions) bool { - if opts.DestinationPath != "/" || opts.Content == nil { - return false - } - tr := tar.NewReader(opts.Content) - for { - hdr, err := tr.Next() - if err != nil { - break - } - mkdirNames = append(mkdirNames, hdr.Name) - } - return true - })).Return(mobyclient.CopyToContainerResult{}, nil) - client.On("CopyToContainer", ctx, "123", mock.MatchedBy(func(opts mobyclient.CopyToContainerOptions) bool { - return opts.DestinationPath == "/var/run/act" && opts.Content != nil - })).Return(mobyclient.CopyToContainerResult{}, nil) - cr := &containerReference{ - id: "123", - cli: client, - input: &NewContainerInput{ - Image: "image", - }, - } - - require.NoError(t, cr.CopyTarStream(ctx, "/var/run/act", &bytes.Buffer{})) - assert.Equal(t, []string{"var/run/act"}, mkdirNames) - - client.AssertExpectations(t) -} - -func TestDockerCopyTarStreamErrorInCopyFiles(t *testing.T) { - ctx := context.Background() - - merr := errors.New("Failure") - - client := &mockDockerClient{} - client.On("CopyToContainer", ctx, "123", mock.MatchedBy(func(opts mobyclient.CopyToContainerOptions) bool { - return opts.DestinationPath == "/" && opts.Content != nil - })).Return(mobyclient.CopyToContainerResult{}, merr) - cr := &containerReference{ - id: "123", - cli: client, - input: &NewContainerInput{ - Image: "image", - }, - } - - err := cr.CopyTarStream(ctx, "/var/run/act", &bytes.Buffer{}) - assert.ErrorIs(t, err, merr) //nolint:testifylint // pre-existing issue from nektos/act - - client.AssertExpectations(t) -} - -func TestDockerCopyTarStreamErrorInMkdir(t *testing.T) { - ctx := context.Background() - - merr := errors.New("Failure") - - client := &mockDockerClient{} - client.On("CopyToContainer", ctx, "123", mock.MatchedBy(func(opts mobyclient.CopyToContainerOptions) bool { - return opts.DestinationPath == "/" && opts.Content != nil - })).Return(mobyclient.CopyToContainerResult{}, nil) - client.On("CopyToContainer", ctx, "123", mock.MatchedBy(func(opts mobyclient.CopyToContainerOptions) bool { - return opts.DestinationPath == "/var/run/act" && opts.Content != nil - })).Return(mobyclient.CopyToContainerResult{}, merr) - cr := &containerReference{ - id: "123", - cli: client, - input: &NewContainerInput{ - Image: "image", - }, - } - - err := cr.CopyTarStream(ctx, "/var/run/act", &bytes.Buffer{}) - assert.ErrorIs(t, err, merr) //nolint:testifylint // pre-existing issue from nektos/act - - client.AssertExpectations(t) -} - // A remove that raced the daemon's AutoRemove teardown is not a failure and must not // be logged as one. func TestRemoveIgnoresAutoRemoveRace(t *testing.T) { @@ -582,7 +471,6 @@ func TestRejectsMissingContainer(t *testing.T) { } check("copyContent", cr.copyContent("/var/run/act", &FileEntry{Name: "x", Mode: 0o644})(ctx)) check("copyDir", cr.copyDir("/var/run/act", "/src", false)(ctx)) - check("CopyTarStream", cr.CopyTarStream(ctx, "/var/run/act", &bytes.Buffer{})) check("exec", cr.exec([]string{"echo"}, nil, "", "")(ctx)) _, err := cr.GetContainerArchive(ctx, "/var/run/act/x") check("GetContainerArchive", err) @@ -618,35 +506,6 @@ func TestPublicCopyPipelineHandlesStaleID(t *testing.T) { client.AssertExpectations(t) } -// TestDockerCopyToSymlinkPath is a regression test for gitea/runner#981. Most base images -// symlink /var/run to /run, so copying into /var/run/act traverses that symlink. The broken -// docker 29.5.1 daemon fails the extraction with "mkdirat var/run: file exists" (fixed in -// 29.5.2). Running against the daemon shipped in the dind image, this catches a bad bump. -func TestDockerCopyToSymlinkPath(t *testing.T) { - requireDocker(t) - ctx := context.Background() - - rc := NewContainer(&NewContainerInput{ - Image: "alpine:latest", - Entrypoint: []string{"sleep", "30"}, - Name: "act-test-symlink-" + time.Now().Format("20060102150405.000000"), - AutoRemove: true, - }) - require.NoError(t, rc.Pull(false)(ctx)) - require.NoError(t, rc.Create(nil, nil)(ctx)) - require.NoError(t, rc.Start(false)(ctx)) - t.Cleanup(func() { - _ = rc.Remove()(ctx) - _ = rc.Close()(ctx) - }) - - // CopyTarStream first creates the destination directory by extracting a tar at "/", - // which makes the daemon mkdir var, then var/run (the symlink), then act — the exact - // step that fails on the broken daemon. - err := rc.CopyTarStream(ctx, "/var/run/act/actions/", &bytes.Buffer{}) - require.NoError(t, err) -} - // Type assert containerReference implements ExecutionsEnvironment var _ ExecutionsEnvironment = &containerReference{} diff --git a/act/container/docker_socket.go b/act/container/docker_socket.go deleted file mode 100644 index 34490b8b..00000000 --- a/act/container/docker_socket.go +++ /dev/null @@ -1,138 +0,0 @@ -// Copyright 2024 The Gitea Authors. All rights reserved. -// Copyright 2024 The nektos/act Authors. All rights reserved. -// SPDX-License-Identifier: MIT - -package container - -import ( - "fmt" - "os" - "path/filepath" - "strings" - - log "github.com/sirupsen/logrus" -) - -var CommonSocketLocations = []string{ - "/var/run/docker.sock", - "/run/podman/podman.sock", - "$HOME/.colima/docker.sock", - "$XDG_RUNTIME_DIR/docker.sock", - "$XDG_RUNTIME_DIR/podman/podman.sock", - `\\.\pipe\docker_engine`, - "$HOME/.docker/run/docker.sock", -} - -// returns socket URI or false if not found any -func socketLocation() (string, bool) { - if dockerHost, exists := os.LookupEnv("DOCKER_HOST"); exists { - return dockerHost, true - } - - for _, p := range CommonSocketLocations { - if _, err := os.Lstat(os.ExpandEnv(p)); err == nil { - if strings.HasPrefix(p, `\\.\`) { - return "npipe://" + filepath.ToSlash(os.ExpandEnv(p)), true - } - return "unix://" + filepath.ToSlash(os.ExpandEnv(p)), true - } - } - - return "", false -} - -// This function, `isDockerHostURI`, takes a string argument `daemonPath`. It checks if the -// `daemonPath` is a valid Docker host URI. It does this by checking if the scheme of the URI (the -// part before "://") contains only alphabetic characters. If it does, the function returns true, -// indicating that the `daemonPath` is a Docker host URI. If it doesn't, or if the "://" delimiter -// is not found in the `daemonPath`, the function returns false. -func isDockerHostURI(daemonPath string) bool { - if before, _, ok := strings.Cut(daemonPath, "://"); ok { - scheme := before - if strings.IndexFunc(scheme, func(r rune) bool { - return (r < 'a' || r > 'z') && (r < 'A' || r > 'Z') - }) == -1 { - return true - } - } - return false -} - -type SocketAndHost struct { - Socket string - Host string -} - -func GetSocketAndHost(containerSocket string) (SocketAndHost, error) { - log.Debugf("Handling container host and socket") - - // Prefer DOCKER_HOST, don't override it - dockerHost, hasDockerHost := socketLocation() - socketHost := SocketAndHost{Socket: containerSocket, Host: dockerHost} - - // ** socketHost.Socket cases ** - // Case 1: User does _not_ want to mount a daemon socket (passes a dash) - // Case 2: User passes a filepath to the socket; is that even valid? - // Case 3: User passes a valid socket; do nothing - // Case 4: User omitted the flag; set a sane default - - // ** DOCKER_HOST cases ** - // Case A: DOCKER_HOST is set; use it, i.e. do nothing - // Case B: DOCKER_HOST is empty; use sane defaults - - // Set host for sanity's sake, when the socket isn't useful - if !hasDockerHost && (socketHost.Socket == "-" || !isDockerHostURI(socketHost.Socket) || socketHost.Socket == "") { - // Cases: 1B, 2B, 4B - socket, found := socketLocation() - socketHost.Host = socket - hasDockerHost = found - } - - // A - (dash) in socketHost.Socket means don't mount, preserve this value - // otherwise if socketHost.Socket is a filepath don't use it as socket - // Exit early if we're in an invalid state (e.g. when no DOCKER_HOST and user supplied "-", a dash or omitted) - if !hasDockerHost && socketHost.Socket != "" && !isDockerHostURI(socketHost.Socket) { - // Cases: 1B, 2B - // Should we early-exit here, since there is no host nor socket to talk to? - return SocketAndHost{}, fmt.Errorf("DOCKER_HOST was not set, couldn't be found in the usual locations, and the container daemon socket ('%s') is invalid", socketHost.Socket) - } - - // Default to DOCKER_HOST if set - if socketHost.Socket == "" && hasDockerHost { - // Cases: 4A - log.Debugf("Defaulting container socket to DOCKER_HOST") - socketHost.Socket = socketHost.Host - } - // Set sane default socket location if user omitted it - if socketHost.Socket == "" { - // Cases: 4B - socket, _ := socketLocation() - // socket is empty if it isn't found, so assignment here is at worst a no-op - log.Debugf("Defaulting container socket to default '%s'", socket) - socketHost.Socket = socket - } - - // Exit if both the DOCKER_HOST and socket are fulfilled - if hasDockerHost { - // Cases: 1A, 2A, 3A, 4A - if !isDockerHostURI(socketHost.Socket) { - // Cases: 1A, 2A - log.Debugf("DOCKER_HOST is set, but socket is invalid '%s'", socketHost.Socket) - } - return socketHost, nil - } - - // Set a sane DOCKER_HOST default if we can - if isDockerHostURI(socketHost.Socket) { - // Cases: 3B - log.Debugf("Setting DOCKER_HOST to container socket '%s'", socketHost.Socket) - socketHost.Host = socketHost.Socket - // Both DOCKER_HOST and container socket are valid; short-circuit exit - return socketHost, nil - } - - // Here there is no DOCKER_HOST _and_ the supplied container socket is not a valid URI (either invalid or a file path) - // Cases: 2B <- but is already handled at the top - // I.e. this path should never be taken - return SocketAndHost{}, fmt.Errorf("no DOCKER_HOST and an invalid container socket '%s'", socketHost.Socket) -} diff --git a/act/container/docker_socket_test.go b/act/container/docker_socket_test.go deleted file mode 100644 index 44252f47..00000000 --- a/act/container/docker_socket_test.go +++ /dev/null @@ -1,167 +0,0 @@ -// Copyright 2024 The Gitea Authors. All rights reserved. -// Copyright 2024 The nektos/act Authors. All rights reserved. -// SPDX-License-Identifier: MIT - -package container - -import ( - "os" - "testing" - - log "github.com/sirupsen/logrus" - assert "github.com/stretchr/testify/assert" -) - -func init() { - log.SetLevel(log.DebugLevel) -} - -var originalCommonSocketLocations = CommonSocketLocations - -func isolateSocketEnv(t *testing.T) { - t.Helper() - t.Cleanup(func() { CommonSocketLocations = originalCommonSocketLocations }) - if host, ok := os.LookupEnv("DOCKER_HOST"); ok { - t.Setenv("DOCKER_HOST", host) - } else { - t.Cleanup(func() { os.Unsetenv("DOCKER_HOST") }) - } -} - -func TestGetSocketAndHostWithSocket(t *testing.T) { - // Arrange - isolateSocketEnv(t) - dockerHost := "unix:///my/docker/host.sock" - socketURI := "/path/to/my.socket" - t.Setenv("DOCKER_HOST", dockerHost) - - // Act - ret, err := GetSocketAndHost(socketURI) - - // Assert - assert.NoError(t, err) //nolint:testifylint // pre-existing issue from nektos/act - assert.Equal(t, SocketAndHost{socketURI, dockerHost}, ret) -} - -func TestGetSocketAndHostNoSocket(t *testing.T) { - // Arrange - dockerHost := "unix:///my/docker/host.sock" - t.Setenv("DOCKER_HOST", dockerHost) - - // Act - ret, err := GetSocketAndHost("") - - // Assert - assert.NoError(t, err) //nolint:testifylint // pre-existing issue from nektos/act - assert.Equal(t, SocketAndHost{dockerHost, dockerHost}, ret) -} - -func TestGetSocketAndHostOnlySocket(t *testing.T) { - // Arrange - isolateSocketEnv(t) - socketURI := "/path/to/my.socket" - os.Unsetenv("DOCKER_HOST") - defaultSocket, defaultSocketFound := socketLocation() - - // Act - ret, err := GetSocketAndHost(socketURI) - - // Assert - assert.NoError(t, err, "Expected no error from GetSocketAndHost") //nolint:testifylint // pre-existing issue from nektos/act - assert.True(t, defaultSocketFound, "Expected to find default socket") - assert.Equal(t, socketURI, ret.Socket, "Expected socket to match common location") - assert.Equal(t, defaultSocket, ret.Host, "Expected ret.Host to match default socket location") -} - -func TestGetSocketAndHostDontMount(t *testing.T) { - // Arrange - isolateSocketEnv(t) - dockerHost := "unix:///my/docker/host.sock" - t.Setenv("DOCKER_HOST", dockerHost) - - // Act - ret, err := GetSocketAndHost("-") - - // Assert - assert.NoError(t, err) //nolint:testifylint // pre-existing issue from nektos/act - assert.Equal(t, SocketAndHost{"-", dockerHost}, ret) -} - -func TestGetSocketAndHostNoHostNoSocket(t *testing.T) { - // Arrange - isolateSocketEnv(t) - os.Unsetenv("DOCKER_HOST") - defaultSocket, found := socketLocation() - - // Act - ret, err := GetSocketAndHost("") - - // Assert - assert.True(t, found, "Expected a default socket to be found") - assert.NoError(t, err, "Expected no error from GetSocketAndHost") //nolint:testifylint // pre-existing issue from nektos/act - assert.Equal(t, SocketAndHost{defaultSocket, defaultSocket}, ret, "Expected to match default socket location") -} - -// Catch -// > Your code breaks setting DOCKER_HOST if shouldMount is false. -// > This happens if neither DOCKER_HOST nor --container-daemon-socket has a value, but socketLocation() returns a URI -func TestGetSocketAndHostNoHostNoSocketDefaultLocation(t *testing.T) { - // Arrange - isolateSocketEnv(t) - mySocketFile, tmpErr := os.CreateTemp(t.TempDir(), "act-*.sock") - mySocket := mySocketFile.Name() - unixSocket := "unix://" + mySocket - defer os.RemoveAll(mySocket) - assert.NoError(t, tmpErr) //nolint:testifylint // pre-existing issue from nektos/act - os.Unsetenv("DOCKER_HOST") - - CommonSocketLocations = []string{mySocket} - defaultSocket, found := socketLocation() - - // Act - ret, err := GetSocketAndHost("") - - // Assert - assert.Equal(t, unixSocket, defaultSocket, "Expected default socket to match common socket location") - assert.True(t, found, "Expected default socket to be found") - assert.NoError(t, err, "Expected no error from GetSocketAndHost") //nolint:testifylint // pre-existing issue from nektos/act - assert.Equal(t, SocketAndHost{unixSocket, unixSocket}, ret, "Expected to match default socket location") -} - -func TestGetSocketAndHostNoHostInvalidSocket(t *testing.T) { - // Arrange - isolateSocketEnv(t) - os.Unsetenv("DOCKER_HOST") - mySocket := "/my/socket/path.sock" - CommonSocketLocations = []string{"/unusual", "/socket", "/location"} - defaultSocket, found := socketLocation() - - // Act - ret, err := GetSocketAndHost(mySocket) - - // Assert - assert.False(t, found, "Expected no default socket to be found") - assert.Equal(t, "", defaultSocket, "Expected no default socket to be found") //nolint:testifylint // pre-existing issue from nektos/act - assert.Equal(t, SocketAndHost{}, ret, "Expected to match default socket location") - assert.Error(t, err, "Expected an error in invalid state") -} - -func TestGetSocketAndHostOnlySocketValidButUnusualLocation(t *testing.T) { - // Arrange - isolateSocketEnv(t) - socketURI := "unix:///path/to/my.socket" - CommonSocketLocations = []string{"/unusual", "/location"} - os.Unsetenv("DOCKER_HOST") - defaultSocket, found := socketLocation() - - // Act - ret, err := GetSocketAndHost(socketURI) - - // Assert - // Default socket locations - assert.Equal(t, "", defaultSocket, "Expect default socket location to be empty") //nolint:testifylint // pre-existing issue from nektos/act - assert.False(t, found, "Expected no default socket to be found") - // Sane default - assert.NoError(t, err, "Expect no error from GetSocketAndHost") //nolint:testifylint // pre-existing issue from nektos/act - assert.Equal(t, socketURI, ret.Host, "Expect host to default to unusual socket") -} diff --git a/act/container/host_environment.go b/act/container/host_environment.go index f963cb70..dd99064b 100644 --- a/act/container/host_environment.go +++ b/act/container/host_environment.go @@ -26,6 +26,7 @@ import ( "gitea.com/gitea/runner/act/lookpath" "gitea.com/gitea/runner/internal/pkg/process" + "github.com/creack/pty" "github.com/go-git/go-billy/v5/helper/polyfill" "github.com/go-git/go-billy/v5/osfs" "github.com/go-git/go-git/v5/plumbing/format/gitignore" @@ -71,12 +72,6 @@ func (e *HostEnvironment) Create(_, _ []string) common.Executor { } } -func (e *HostEnvironment) ConnectToNetwork(name string) common.Executor { - return func(ctx context.Context) error { - return nil - } -} - func (e *HostEnvironment) Close() common.Executor { return func(ctx context.Context) error { return nil @@ -97,33 +92,6 @@ func (e *HostEnvironment) Copy(destPath string, files ...*FileEntry) common.Exec } } -func (e *HostEnvironment) CopyTarStream(ctx context.Context, destPath string, tarStream io.Reader) error { - if err := os.RemoveAll(destPath); err != nil { - return err - } - tr := tar.NewReader(tarStream) - cp := &filecollector.CopyCollector{ - DstDir: destPath, - } - for { - ti, err := tr.Next() - if errors.Is(err, io.EOF) { - return nil - } else if err != nil { - return err - } - if ti.FileInfo().IsDir() { - continue - } - if ctx.Err() != nil { - return errors.New("CopyTarStream has been cancelled") - } - if err := cp.WriteFile(ti.Name, ti.FileInfo(), ti.Linkname, tr); err != nil { - return err - } - } -} - func (e *HostEnvironment) CopyDir(destPath, srcPath string, useGitIgnore bool) common.Executor { return func(ctx context.Context) error { logger := common.Logger(ctx) @@ -142,7 +110,6 @@ func (e *HostEnvironment) CopyDir(destPath, srcPath string, useGitIgnore bool) c ignorer = gitignore.NewMatcher(ps) } fc := &filecollector.FileCollector{ - Fs: &filecollector.DefaultFs{}, Ignorer: ignorer, SrcPath: srcPath, SrcPrefix: srcPrefix, @@ -180,7 +147,6 @@ func (e *HostEnvironment) GetContainerArchive(ctx context.Context, srcPath strin srcPrefix += string(filepath.Separator) } fc := &filecollector.FileCollector{ - Fs: &filecollector.DefaultFs{}, SrcPath: srcPath, SrcPrefix: srcPrefix, Handler: tc, @@ -246,24 +212,8 @@ func (w *ptyWriter) Write(buf []byte) (int, error) { return w.Out.Write(buf) } -type localEnv struct { - env map[string]string -} - -func (l *localEnv) Getenv(name string) string { - if runtime.GOOS == "windows" { - for k, v := range l.env { - if strings.EqualFold(name, k) { - return v - } - } - return "" - } - return l.env[name] -} - func lookupPathHost(cmd string, env map[string]string, writer io.Writer) (string, error) { - f, err := lookpath.LookPath2(cmd, &localEnv{env: env}) + f, err := lookpath.LookPath2(cmd, env) if err != nil { err := "Cannot find: " + cmd + " in PATH" if _, _err := writer.Write([]byte(err + "\n")); _err != nil { @@ -275,7 +225,7 @@ func lookupPathHost(cmd string, env map[string]string, writer io.Writer) (string } func setupPty(cmd *exec.Cmd, cmdline string) (*os.File, *os.File, error) { - ppty, tty, err := openPty() + ppty, tty, err := pty.Open() if err != nil { return nil, nil, err } diff --git a/act/container/util.go b/act/container/util.go deleted file mode 100644 index 1b606511..00000000 --- a/act/container/util.go +++ /dev/null @@ -1,17 +0,0 @@ -// Copyright 2022 The Gitea Authors. All rights reserved. -// Copyright 2022 The nektos/act Authors. All rights reserved. -// SPDX-License-Identifier: MIT - -//go:build (!windows && !plan9 && !openbsd) || (!windows && !plan9 && !mips64) - -package container - -import ( - "os" - - "github.com/creack/pty" -) - -func openPty() (*os.File, *os.File, error) { - return pty.Open() -} diff --git a/act/container/util_openbsd_mips64.go b/act/container/util_openbsd_mips64.go deleted file mode 100644 index 859dd027..00000000 --- a/act/container/util_openbsd_mips64.go +++ /dev/null @@ -1,14 +0,0 @@ -// Copyright 2022 The Gitea Authors. All rights reserved. -// Copyright 2022 The nektos/act Authors. All rights reserved. -// SPDX-License-Identifier: MIT - -package container - -import ( - "errors" - "os" -) - -func openPty() (*os.File, *os.File, error) { - return nil, nil, errors.New("Unsupported") -} diff --git a/act/container/util_plan9.go b/act/container/util_plan9.go deleted file mode 100644 index 859dd027..00000000 --- a/act/container/util_plan9.go +++ /dev/null @@ -1,14 +0,0 @@ -// Copyright 2022 The Gitea Authors. All rights reserved. -// Copyright 2022 The nektos/act Authors. All rights reserved. -// SPDX-License-Identifier: MIT - -package container - -import ( - "errors" - "os" -) - -func openPty() (*os.File, *os.File, error) { - return nil, nil, errors.New("Unsupported") -} diff --git a/act/container/util_windows.go b/act/container/util_windows.go deleted file mode 100644 index 859dd027..00000000 --- a/act/container/util_windows.go +++ /dev/null @@ -1,14 +0,0 @@ -// Copyright 2022 The Gitea Authors. All rights reserved. -// Copyright 2022 The nektos/act Authors. All rights reserved. -// SPDX-License-Identifier: MIT - -package container - -import ( - "errors" - "os" -) - -func openPty() (*os.File, *os.File, error) { - return nil, nil, errors.New("Unsupported") -} diff --git a/act/filecollector/file_collector.go b/act/filecollector/file_collector.go index 084c0804..c2e9e457 100644 --- a/act/filecollector/file_collector.go +++ b/act/filecollector/file_collector.go @@ -97,55 +97,25 @@ type FileCollector struct { Ignorer gitignore.Matcher SrcPath string SrcPrefix string - Fs Fs Handler Handler } -type Fs interface { - Walk(root string, fn filepath.WalkFunc) error - OpenGitIndex(path string) (*index.Index, error) - Open(path string) (io.ReadCloser, error) - Readlink(path string) (string, error) -} - -type DefaultFs struct{} - -func (*DefaultFs) Walk(root string, fn filepath.WalkFunc) error { - return filepath.Walk(root, fn) -} - -func (*DefaultFs) OpenGitIndex(path string) (*index.Index, error) { - r, err := git.PlainOpen(path) +func openGitIndex(path string) (*index.Index, error) { + repo, err := git.PlainOpen(path) if err != nil { return nil, err } - i, err := r.Storer.Index() - if err != nil { - return nil, err - } - return i, nil -} - -func (*DefaultFs) Open(path string) (io.ReadCloser, error) { - return os.Open(path) -} - -func (*DefaultFs) Readlink(path string) (string, error) { - return os.Readlink(path) + return repo.Storer.Index() } func (fc *FileCollector) CollectFiles(ctx context.Context, submodulePath []string) filepath.WalkFunc { - i, _ := fc.Fs.OpenGitIndex(path.Join(fc.SrcPath, path.Join(submodulePath...))) + i, _ := openGitIndex(path.Join(fc.SrcPath, path.Join(submodulePath...))) return func(file string, fi os.FileInfo, err error) error { if err != nil { return err } - if ctx != nil { - select { - case <-ctx.Done(): - return errors.New("copy cancelled") - default: - } + if ctx != nil && ctx.Err() != nil { + return errors.New("copy cancelled") } sansPrefix := strings.TrimPrefix(file, fc.SrcPrefix) @@ -175,7 +145,7 @@ func (fc *FileCollector) CollectFiles(ctx context.Context, submodulePath []strin } } if err == nil && entry.Mode == filemode.Submodule { - err = fc.Fs.Walk(file, fc.CollectFiles(ctx, split)) + err = filepath.Walk(file, fc.CollectFiles(ctx, split)) if err != nil { return err } @@ -185,7 +155,7 @@ func (fc *FileCollector) CollectFiles(ctx context.Context, submodulePath []strin // return on non-regular files (thanks to [kumo](https://medium.com/@komuw/just-like-you-did-fbdd7df829d3) for this suggested update) if fi.Mode()&os.ModeSymlink == os.ModeSymlink { - linkName, err := fc.Fs.Readlink(file) + linkName, err := os.Readlink(file) if err != nil { return fmt.Errorf("unable to readlink '%s': %w", file, err) } @@ -195,23 +165,15 @@ func (fc *FileCollector) CollectFiles(ctx context.Context, submodulePath []strin } // open file - f, err := fc.Fs.Open(file) + f, err := os.Open(file) if err != nil { return err } defer f.Close() if ctx != nil { - // make io.Copy cancellable by closing the file - cpctx, cpfinish := context.WithCancel(ctx) - defer cpfinish() - go func() { - select { - case <-cpctx.Done(): - case <-ctx.Done(): - f.Close() - } - }() + stop := context.AfterFunc(ctx, func() { _ = f.Close() }) + defer stop() } return fc.Handler.WriteFile(path, fi, "", f) diff --git a/act/filecollector/file_collector_test.go b/act/filecollector/file_collector_test.go index bdbece91..e7d11ea9 100644 --- a/act/filecollector/file_collector_test.go +++ b/act/filecollector/file_collector_test.go @@ -6,6 +6,7 @@ package filecollector import ( "archive/tar" + "bytes" "context" "io" "os" @@ -13,110 +14,41 @@ import ( "runtime" "strings" "testing" - "time" - "github.com/go-git/go-billy/v5" - "github.com/go-git/go-billy/v5/memfs" git "github.com/go-git/go-git/v5" - "github.com/go-git/go-git/v5/plumbing/cache" "github.com/go-git/go-git/v5/plumbing/format/gitignore" - "github.com/go-git/go-git/v5/plumbing/format/index" - "github.com/go-git/go-git/v5/storage/filesystem" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) -type memoryFs struct { - billy.Filesystem -} - -func (mfs *memoryFs) walk(root string, fn filepath.WalkFunc) error { - dir, err := mfs.ReadDir(root) - if err != nil { - return err - } - for i := range dir { - filename := filepath.Join(root, dir[i].Name()) - err = fn(filename, dir[i], nil) - if dir[i].IsDir() { - if err == filepath.SkipDir { - err = nil - } else if err := mfs.walk(filename, fn); err != nil { - return err - } - } - if err != nil { - return err - } - } - return nil -} - -func (mfs *memoryFs) Walk(root string, fn filepath.WalkFunc) error { - stat, err := mfs.Lstat(root) - if err != nil { - return err - } - err = fn(strings.Join([]string{root, "."}, string(filepath.Separator)), stat, nil) - if err != nil { - return err - } - return mfs.walk(root, fn) -} - -func (mfs *memoryFs) OpenGitIndex(path string) (*index.Index, error) { - f, _ := mfs.Filesystem.Chroot(filepath.Join(path, ".git")) //nolint:staticcheck // pre-existing issue from nektos/act - storage := filesystem.NewStorage(f, cache.NewObjectLRUDefault()) - i, err := storage.Index() - if err != nil { - return nil, err - } - return i, nil -} - -func (mfs *memoryFs) Open(path string) (io.ReadCloser, error) { - return mfs.Filesystem.Open(path) -} - -func (mfs *memoryFs) Readlink(path string) (string, error) { - return mfs.Filesystem.Readlink(path) -} - func TestIgnoredTrackedfile(t *testing.T) { - fs := memfs.New() - _ = fs.MkdirAll("mygitrepo/.git", 0o777) - dotgit, _ := fs.Chroot("mygitrepo/.git") - worktree, _ := fs.Chroot("mygitrepo") - repo, _ := git.Init(filesystem.NewStorage(dotgit, cache.NewObjectLRUDefault()), worktree) - f, _ := worktree.Create(".gitignore") - _, _ = f.Write([]byte(".*\n")) - f.Close() - // This file shouldn't be in the tar - f, _ = worktree.Create(".env") - _, _ = f.Write([]byte("test=val1\n")) - f.Close() - w, _ := repo.Worktree() - // .gitignore is in the tar after adding it to the index - _, _ = w.Add(".gitignore") + repoDir := filepath.Join(t.TempDir(), "mygitrepo") + repo, err := git.PlainInit(repoDir, false) + require.NoError(t, err) + require.NoError(t, os.WriteFile(filepath.Join(repoDir, ".gitignore"), []byte(".*\n"), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(repoDir, ".env"), []byte("test=val1\n"), 0o644)) + worktree, err := repo.Worktree() + require.NoError(t, err) + _, err = worktree.Add(".gitignore") + require.NoError(t, err) - tmpTar, _ := fs.Create("temp.tar") - tw := tar.NewWriter(tmpTar) - ps, _ := gitignore.ReadPatterns(worktree, []string{}) - ignorer := gitignore.NewMatcher(ps) + var archive bytes.Buffer + tw := tar.NewWriter(&archive) + patterns, err := gitignore.ReadPatterns(worktree.Filesystem, nil) + require.NoError(t, err) + ignorer := gitignore.NewMatcher(patterns) fc := &FileCollector{ - Fs: &memoryFs{Filesystem: fs}, Ignorer: ignorer, - SrcPath: "mygitrepo", - SrcPrefix: "mygitrepo" + string(filepath.Separator), + SrcPath: repoDir, + SrcPrefix: repoDir + string(filepath.Separator), Handler: &TarCollector{ TarWriter: tw, }, } - err := fc.Fs.Walk("mygitrepo", fc.CollectFiles(context.Background(), []string{})) - assert.NoError(t, err, "successfully collect files") //nolint:testifylint // pre-existing issue from nektos/act - tw.Close() - _, _ = tmpTar.Seek(0, io.SeekStart) - tr := tar.NewReader(tmpTar) + err = filepath.Walk(repoDir, fc.CollectFiles(context.Background(), nil)) + assert.NoError(t, err, "successfully collect files") + require.NoError(t, tw.Close()) + tr := tar.NewReader(&archive) h, err := tr.Next() assert.NoError(t, err, "tar must not be empty") //nolint:testifylint // pre-existing issue from nektos/act assert.Equal(t, ".gitignore", h.Name) @@ -125,47 +57,32 @@ func TestIgnoredTrackedfile(t *testing.T) { } func TestSymlinks(t *testing.T) { - fs := memfs.New() - _ = fs.MkdirAll("mygitrepo/.git", 0o777) - dotgit, _ := fs.Chroot("mygitrepo/.git") - worktree, _ := fs.Chroot("mygitrepo") - repo, _ := git.Init(filesystem.NewStorage(dotgit, cache.NewObjectLRUDefault()), worktree) - // This file shouldn't be in the tar - f, err := worktree.Create(".env") - assert.NoError(t, err) //nolint:testifylint // pre-existing issue from nektos/act - _, err = f.Write([]byte("test=val1\n")) - assert.NoError(t, err) //nolint:testifylint // pre-existing issue from nektos/act - f.Close() - err = worktree.Symlink(".env", "test.env") - assert.NoError(t, err) //nolint:testifylint // pre-existing issue from nektos/act + if runtime.GOOS == "windows" { + t.Skip("creating symlinks requires elevated privileges on Windows") + } + repoDir := filepath.Join(t.TempDir(), "mygitrepo") + repo, err := git.PlainInit(repoDir, false) + require.NoError(t, err) + require.NoError(t, os.WriteFile(filepath.Join(repoDir, ".env"), []byte("test=val1\n"), 0o644)) + require.NoError(t, os.Symlink(".env", filepath.Join(repoDir, "test.env"))) + worktree, err := repo.Worktree() + require.NoError(t, err) + _, err = worktree.Add("test.env") + require.NoError(t, err) - w, err := repo.Worktree() - assert.NoError(t, err) //nolint:testifylint // pre-existing issue from nektos/act - - // .gitignore is in the tar after adding it to the index - _, err = w.Add(".env") - assert.NoError(t, err) //nolint:testifylint // pre-existing issue from nektos/act - _, err = w.Add("test.env") - assert.NoError(t, err) //nolint:testifylint // pre-existing issue from nektos/act - - tmpTar, _ := fs.Create("temp.tar") - tw := tar.NewWriter(tmpTar) - ps, _ := gitignore.ReadPatterns(worktree, []string{}) - ignorer := gitignore.NewMatcher(ps) + var archive bytes.Buffer + tw := tar.NewWriter(&archive) fc := &FileCollector{ - Fs: &memoryFs{Filesystem: fs}, - Ignorer: ignorer, - SrcPath: "mygitrepo", - SrcPrefix: "mygitrepo" + string(filepath.Separator), + SrcPath: repoDir, + SrcPrefix: repoDir + string(filepath.Separator), Handler: &TarCollector{ TarWriter: tw, }, } - err = fc.Fs.Walk("mygitrepo", fc.CollectFiles(context.Background(), []string{})) - assert.NoError(t, err, "successfully collect files") //nolint:testifylint // pre-existing issue from nektos/act - tw.Close() - _, _ = tmpTar.Seek(0, io.SeekStart) - tr := tar.NewReader(tmpTar) + err = filepath.Walk(repoDir, fc.CollectFiles(context.Background(), nil)) + assert.NoError(t, err, "successfully collect files") + require.NoError(t, tw.Close()) + tr := tar.NewReader(&archive) h, err := tr.Next() files := map[string]tar.Header{} for err == nil { @@ -223,62 +140,14 @@ func TestCopyCollectorWriteFileOverwritesFileWithSymlink(t *testing.T) { assert.Equal(t, "target", resolved) } -func TestDefaultFsOpenReadlinkAndWalk(t *testing.T) { - if runtime.GOOS == "windows" { - t.Skip("creating symlinks requires elevated privileges on Windows") - } - - root := t.TempDir() - require.NoError(t, os.WriteFile(filepath.Join(root, "file.txt"), []byte("content"), 0o644)) - require.NoError(t, os.Symlink("file.txt", filepath.Join(root, "link.txt"))) - - fsys := &DefaultFs{} - var walked []string - require.NoError(t, fsys.Walk(root, func(path string, info os.FileInfo, err error) error { - require.NoError(t, err) - walked = append(walked, info.Name()) - return nil - })) - require.Contains(t, walked, "file.txt") - require.Contains(t, walked, "link.txt") - - file, err := fsys.Open(filepath.Join(root, "file.txt")) - require.NoError(t, err) - data, err := io.ReadAll(file) - require.NoError(t, err) - require.NoError(t, file.Close()) - require.Equal(t, "content", string(data)) - - link, err := fsys.Readlink(filepath.Join(root, "link.txt")) - require.NoError(t, err) - require.Equal(t, "file.txt", link) -} - func TestFileCollectorCancellationAndWalkError(t *testing.T) { - fc := &FileCollector{Fs: &memoryFs{Filesystem: memfs.New()}} - walk := fc.CollectFiles(cancelledContext(t), nil) + ctx, cancel := context.WithCancel(t.Context()) + cancel() + walk := (&FileCollector{}).CollectFiles(ctx, nil) - err := walk("file", fakeFileInfo{name: "file"}, nil) + err := walk("file", nil, nil) require.EqualError(t, err, "copy cancelled") - err = walk("file", fakeFileInfo{name: "file"}, os.ErrPermission) + err = walk("file", nil, os.ErrPermission) require.ErrorIs(t, err, os.ErrPermission) } - -func cancelledContext(t *testing.T) context.Context { - t.Helper() - ctx, cancel := context.WithCancel(context.Background()) - cancel() - return ctx -} - -type fakeFileInfo struct { - name string -} - -func (f fakeFileInfo) Name() string { return f.name } -func (f fakeFileInfo) Size() int64 { return 0 } -func (f fakeFileInfo) Mode() os.FileMode { return 0o644 } -func (f fakeFileInfo) ModTime() time.Time { return time.Time{} } -func (f fakeFileInfo) IsDir() bool { return false } -func (f fakeFileInfo) Sys() any { return nil } diff --git a/act/ghcontext/github_context.go b/act/ghcontext/github_context.go index 674b7d7f..6ade785e 100644 --- a/act/ghcontext/github_context.go +++ b/act/ghcontext/github_context.go @@ -25,32 +25,9 @@ var ( findGithubRepo = git.FindGithubRepo ) -func withDefaultBranch(ctx context.Context, b string, event map[string]any) map[string]any { - repoI, ok := event["repository"] - if !ok { - repoI = make(map[string]any) - } - - repo, ok := repoI.(map[string]any) - if !ok { - common.Logger(ctx).Warnf("unable to set default branch to %v", b) - return event - } - - // if the branch is already there return with no changes - if _, ok = repo["default_branch"]; ok { - return event - } - - repo["default_branch"] = b - event["repository"] = repo - - return event -} - // SetRef resolves the ref of the context from its event payload, falling back // to the ref checked out in repoPath. -func SetRef(ctx context.Context, ghc *model.GithubContext, defaultBranch, repoPath string) { +func SetRef(ctx context.Context, ghc *model.GithubContext, repoPath string) { logger := common.Logger(ctx) // https://docs.github.com/en/actions/learn-github-actions/events-that-trigger-workflows @@ -82,11 +59,15 @@ func SetRef(ctx context.Context, ghc *model.GithubContext, defaultBranch, repoPa ghc.Ref = ref } - // set the branch in the event data - if defaultBranch != "" { - ghc.Event = withDefaultBranch(ctx, defaultBranch, ghc.Event) - } else { - ghc.Event = withDefaultBranch(ctx, "master", ghc.Event) + repository, exists := ghc.Event["repository"] + if !exists { + repository = map[string]any{} + } + if repository, ok := repository.(map[string]any); !ok { + logger.Warn("unable to set default branch to master") + } else if _, exists := repository["default_branch"]; !exists { + repository["default_branch"] = "master" + ghc.Event["repository"] = repository } if ghc.Ref == "" { @@ -125,11 +106,11 @@ func SetSha(ctx context.Context, ghc *model.GithubContext, repoPath string) { // SetRepositoryAndOwner resolves the repository of the context from the git // remote in repoPath when it is not set yet, and derives its owner. -func SetRepositoryAndOwner(ctx context.Context, ghc *model.GithubContext, githubInstance, remoteName, repoPath string) { +func SetRepositoryAndOwner(ctx context.Context, ghc *model.GithubContext, githubInstance, repoPath string) { if ghc.Repository == "" { - repo, err := findGithubRepo(ctx, repoPath, githubInstance, remoteName) + repo, err := findGithubRepo(ctx, repoPath, githubInstance) if err != nil { - common.Logger(ctx).Warningf("unable to get git repo (githubInstance: %v; remoteName: %v, repoPath: %v): %v", githubInstance, remoteName, repoPath, err) + common.Logger(ctx).Warningf("unable to get git repo (githubInstance: %v, repoPath: %v): %v", githubInstance, repoPath, err) return } ghc.Repository = repo diff --git a/act/ghcontext/github_context_test.go b/act/ghcontext/github_context_test.go index 19044b3b..258ca0ad 100644 --- a/act/ghcontext/github_context_test.go +++ b/act/ghcontext/github_context_test.go @@ -104,7 +104,7 @@ func TestSetRef(t *testing.T) { Event: table.event, } - SetRef(context.Background(), ghc, "main", "/some/dir") + SetRef(context.Background(), ghc, "/some/dir") ghc.SetRefTypeAndName() assert.Equal(t, table.ref, ghc.Ref) @@ -122,7 +122,7 @@ func TestSetRef(t *testing.T) { Event: map[string]any{}, } - SetRef(context.Background(), ghc, "", "/some/dir") + SetRef(context.Background(), ghc, "/some/dir") assert.Equal(t, "refs/heads/master", ghc.Ref) }) diff --git a/act/lookpath/env.go b/act/lookpath/env.go index b492d997..254d4665 100644 --- a/act/lookpath/env.go +++ b/act/lookpath/env.go @@ -4,6 +4,18 @@ package lookpath -type Env interface { - Getenv(name string) string +import ( + "runtime" + "strings" +) + +func getenv(env map[string]string, name string) string { + if runtime.GOOS == "windows" { + for key, value := range env { + if strings.EqualFold(name, key) { + return value + } + } + } + return env[name] } diff --git a/act/lookpath/lp_js.go b/act/lookpath/lp_js.go index 560537a5..0ed96583 100644 --- a/act/lookpath/lp_js.go +++ b/act/lookpath/lp_js.go @@ -18,7 +18,7 @@ var ErrNotFound = errors.New("executable file not found in $PATH") // directories named by the PATH environment variable. // If file contains a slash, it is tried directly and the PATH is not consulted. // The result may be an absolute path or a path relative to the current directory. -func LookPath2(file string, lenv Env) (string, error) { +func LookPath2(file string, _ map[string]string) (string, error) { // Wasm can not execute processes, so act as if there are no executables at all. return "", &Error{file, ErrNotFound} } diff --git a/act/lookpath/lp_plan9.go b/act/lookpath/lp_plan9.go index 9e9eaebd..0de79796 100644 --- a/act/lookpath/lp_plan9.go +++ b/act/lookpath/lp_plan9.go @@ -32,7 +32,7 @@ func findExecutable(file string) error { // If file begins with "/", "#", "./", or "../", it is tried // directly and the path is not consulted. // The result may be an absolute path or a path relative to the current directory. -func LookPath2(file string, lenv Env) (string, error) { +func LookPath2(file string, env map[string]string) (string, error) { // skip the path lookup for these prefixes skip := []string{"/", "#", "./", "../"} @@ -46,7 +46,7 @@ func LookPath2(file string, lenv Env) (string, error) { } } - path := lenv.Getenv("path") + path := getenv(env, "path") for _, dir := range filepath.SplitList(path) { path := filepath.Join(dir, file) if err := findExecutable(path); err == nil { diff --git a/act/lookpath/lp_unix.go b/act/lookpath/lp_unix.go index 75405714..944d013a 100644 --- a/act/lookpath/lp_unix.go +++ b/act/lookpath/lp_unix.go @@ -33,7 +33,7 @@ func findExecutable(file string) error { // directories named by the PATH environment variable. // If file contains a slash, it is tried directly and the PATH is not consulted. // The result may be an absolute path or a path relative to the current directory. -func LookPath2(file string, lenv Env) (string, error) { +func LookPath2(file string, env map[string]string) (string, error) { // NOTE(rsc): I wish we could use the Plan 9 behavior here // (only bypass the path if file begins with / or ./ or ../) // but that would not match all the Unix shells. @@ -45,7 +45,7 @@ func LookPath2(file string, lenv Env) (string, error) { } return "", &Error{file, err} } - path := lenv.Getenv("PATH") + path := getenv(env, "PATH") for _, dir := range filepath.SplitList(path) { if dir == "" { // Unix shell semantics: path element "" means "." diff --git a/act/lookpath/lp_unix_test.go b/act/lookpath/lp_unix_test.go index ca64995d..3ade465f 100644 --- a/act/lookpath/lp_unix_test.go +++ b/act/lookpath/lp_unix_test.go @@ -13,12 +13,6 @@ import ( "testing" ) -type testEnv map[string]string - -func (e testEnv) Getenv(name string) string { - return e[name] -} - func TestLookPath2SearchesPathAndEmptyElement(t *testing.T) { dir := t.TempDir() exe := filepath.Join(dir, "tool") @@ -26,7 +20,7 @@ func TestLookPath2SearchesPathAndEmptyElement(t *testing.T) { t.Fatal(err) } - got, err := LookPath2("tool", testEnv{"PATH": string(filepath.ListSeparator) + dir}) + got, err := LookPath2("tool", map[string]string{"PATH": string(filepath.ListSeparator) + dir}) if err != nil { t.Fatal(err) } @@ -42,7 +36,7 @@ func TestLookPath2DirectPathDoesNotSearchPath(t *testing.T) { t.Fatal(err) } - got, err := LookPath2(exe, testEnv{"PATH": ""}) + got, err := LookPath2(exe, map[string]string{"PATH": ""}) if err != nil { t.Fatal(err) } @@ -58,7 +52,7 @@ func TestLookPath2ReportsPermissionAndNotFound(t *testing.T) { t.Fatal(err) } - _, err := LookPath2(file, testEnv{"PATH": dir}) + _, err := LookPath2(file, map[string]string{"PATH": dir}) var pathErr *Error if !errors.As(err, &pathErr) || !errors.Is(pathErr.Err, fs.ErrPermission) { t.Fatalf("LookPath2(non-executable) error = %v, want fs.ErrPermission wrapped in *Error", err) @@ -67,7 +61,7 @@ func TestLookPath2ReportsPermissionAndNotFound(t *testing.T) { t.Fatalf("Error() = %q, want %q", pathErr.Error(), fs.ErrPermission.Error()) } - _, err = LookPath2("missing", testEnv{"PATH": dir}) + _, err = LookPath2("missing", map[string]string{"PATH": dir}) if !errors.As(err, &pathErr) || !errors.Is(pathErr.Err, ErrNotFound) { t.Fatalf("LookPath2(missing) error = %v, want ErrNotFound wrapped in *Error", err) } diff --git a/act/lookpath/lp_windows.go b/act/lookpath/lp_windows.go index 7044496a..3fb56f6d 100644 --- a/act/lookpath/lp_windows.go +++ b/act/lookpath/lp_windows.go @@ -58,9 +58,9 @@ func findExecutable(file string, exts []string) (string, error) { // LookPath also uses PATHEXT environment variable to match // a suitable candidate. // The result may be an absolute path or a path relative to the current directory. -func LookPath2(file string, lenv Env) (string, error) { +func LookPath2(file string, env map[string]string) (string, error) { var exts []string - x := lenv.Getenv(`PATHEXT`) + x := getenv(env, `PATHEXT`) if x != "" { for e := range strings.SplitSeq(strings.ToLower(x), `;`) { if e == "" { @@ -85,7 +85,7 @@ func LookPath2(file string, lenv Env) (string, error) { if f, err := findExecutable(filepath.Join(".", file), exts); err == nil { return f, nil } - path := lenv.Getenv("path") + path := getenv(env, "path") for _, dir := range filepath.SplitList(path) { if f, err := findExecutable(filepath.Join(dir, file), exts); err == nil { return f, nil diff --git a/act/runner/action.go b/act/runner/action.go index 087a756b..a44c7cd5 100644 --- a/act/runner/action.go +++ b/act/runner/action.go @@ -124,21 +124,9 @@ func readActionImpl(ctx context.Context, step *model.Step, actionDir, actionPath defer closer.Close() action, err := model.ReadAction(reader) - // For Gitea, reduce log noise - // logger.Debugf("Read action %v from '%s'", action, "Unknown") return action, err } -// cachedActionTar returns the action's tree from the action cache, which only a remote action -// has an entry in. -func cachedActionTar(ctx context.Context, step actionStep, name, includePrefix string) (io.ReadCloser, error) { - remote, ok := step.(*stepActionRemote) - if !ok { - return nil, fmt.Errorf("action %q is a remote action but runs as %T", name, step) - } - return step.getRunContext().Config.ActionCache.GetTarArchive(ctx, remote.cacheDir, remote.resolvedSha, includePrefix) -} - func maybeCopyToActionDir(ctx context.Context, step actionStep, actionDir, actionPath, containerActionDir string) error { logger := common.Logger(ctx) rc := step.getRunContext() @@ -148,23 +136,13 @@ func maybeCopyToActionDir(ctx context.Context, step actionStep, actionDir, actio return nil } - var containerActionDirCopy string - containerActionDirCopy = strings.TrimSuffix(containerActionDir, actionPath) + containerActionDirCopy := strings.TrimSuffix(containerActionDir, actionPath) logger.Debug(containerActionDirCopy) if !strings.HasSuffix(containerActionDirCopy, `/`) { containerActionDirCopy += `/` } - if rc.Config != nil && rc.Config.ActionCache != nil { - ta, err := cachedActionTar(ctx, step, stepModel.Uses, "") - if err != nil { - return err - } - defer ta.Close() - return rc.JobContainer.CopyTarStream(ctx, containerActionDirCopy, ta) - } - defer git.AcquireCloneLock(actionDir)() if !rc.Config.NoActionPatch { @@ -191,13 +169,10 @@ func runActionImpl(step actionStep, actionDir string, remoteAction *remoteAction } action := step.getActionModel() - // For Gitea, reduce log noise - // logger.Debugf("About to run action %v", action) - err := setupActionEnv(ctx, step, remoteAction) - if err != nil { - return err - } + rc.withGithubEnv(ctx, step.getGithubContext(ctx), *step.getEnv()) + populateEnvsFromSavedState(step.getEnv(), step, rc) + populateEnvsFromInput(ctx, step.getEnv(), action, rc) actionLocation := path.Join(actionDir, actionPath) actionName, containerActionDir := getContainerActionPaths(stepModel, actionLocation, rc) @@ -215,7 +190,7 @@ func runActionImpl(step actionStep, actionDir string, remoteAction *remoteAction rc.ApplyExtraPath(ctx, step.getEnv()) - return rc.execJobContainer(containerArgs, *step.getEnv(), "", "")(ctx) + return rc.JobContainer.Exec(containerArgs, *step.getEnv(), "", "")(ctx) case x.IsDocker(): location := actionLocation if remoteAction == nil { @@ -240,8 +215,8 @@ func runActionImpl(step actionStep, actionDir string, remoteAction *remoteAction execArgs := []string{filepath.Join(containerActionDir, execFileName)} return common.NewPipelineExecutor( - rc.execJobContainer(buildArgs, *step.getEnv(), "", containerActionDir), - rc.execJobContainer(execArgs, *step.getEnv(), "", ""), + rc.JobContainer.Exec(buildArgs, *step.getEnv(), "", containerActionDir), + rc.JobContainer.Exec(execArgs, *step.getEnv(), "", ""), )(ctx) default: return fmt.Errorf("The runs.using key must be one of: %v, got %s", []string{ @@ -257,20 +232,6 @@ func runActionImpl(step actionStep, actionDir string, remoteAction *remoteAction } } -func setupActionEnv(ctx context.Context, step actionStep, _ *remoteAction) error { - rc := step.getRunContext() - - // A few fields in the environment (e.g. GITHUB_ACTION_REPOSITORY) - // are dependent on the action. That means we can complete the - // setup only after resolving the whole action model and cloning - // the action - rc.withGithubEnv(ctx, step.getGithubContext(ctx), *step.getEnv()) - populateEnvsFromSavedState(step.getEnv(), step, rc) - populateEnvsFromInput(ctx, step.getEnv(), step.getActionModel(), rc) - - return nil -} - // https://github.com/nektos/act/issues/228#issuecomment-629709055 // files in .gitignore are not copied in a Docker container // this causes issues with actions that ignore other important resources @@ -364,12 +325,6 @@ func execAsDocker(ctx context.Context, step actionStep, actionName, actionDir, b return err } defer buildContext.Close() - } else if rc.Config.ActionCache != nil { - buildContext, err = cachedActionTar(ctx, step, actionName, contextDir) - if err != nil { - return err - } - defer buildContext.Close() } prepImage = ContainerNewDockerBuildExecutor(container.NewDockerBuildExecutorInput{ ContextDir: contextDir, @@ -404,21 +359,19 @@ func execAsDocker(ctx context.Context, step actionStep, actionName, actionDir, b if err != nil { return err } - stepContainer := newStepContainer(ctx, step, image, cmd, entrypoint) + stepContainer := newStepContainer(ctx, step, image, cmd, entrypoint, rc.Config.ContainerOptions) return common.NewPipelineExecutor( prepImage, stepContainer.Pull(forcePull), - stepContainer.Remove().IfBool(!rc.Config.ReuseContainers), + stepContainer.Remove(), stepContainer.Create(rc.Config.ContainerCapAdd, rc.Config.ContainerCapDrop), stepContainer.Start(true), - ).Finally( - stepContainer.Remove().IfBool(!rc.Config.ReuseContainers && !rc.Config.AutoRemove), ).Finally(stepContainer.Close())(ctx) } // dockerEntrypoint returns the entrypoint the action's image runs with for the given // stage. Only the main stage honours the `entrypoint` input. -func dockerEntrypoint(ctx context.Context, step actionStep, eval ExpressionEvaluator, stage stepStage) ([]string, error) { +func dockerEntrypoint(ctx context.Context, step actionStep, eval *expressionEvaluator, stage stepStage) ([]string, error) { runs := step.getActionModel().Runs var entrypoint string @@ -469,18 +422,9 @@ func evalDockerArgs(ctx context.Context, step step, action *model.Action, cmd *[ } } -func newStepContainer(ctx context.Context, step step, image string, cmd, entrypoint []string) container.Container { +func newStepContainer(ctx context.Context, step step, image string, cmd, entrypoint []string, options string) container.Container { rc := step.getRunContext() - stepModel := step.getStepModel() - rawLogger := common.Logger(ctx).WithField("raw_output", true) - logWriter := common.NewLineWriter(rc.commandHandler(ctx), func(s string) bool { - if rc.Config.LogOutput { - rawLogger.Infof("%s", s) - } else { - rawLogger.Debugf("%s", s) - } - return true - }) + logWriter := rc.commandLogWriter(ctx) envList := make([]string, 0) for k, v := range *step.getEnv() { envList = append(envList, fmt.Sprintf("%s=%s", k, v)) @@ -493,12 +437,12 @@ func newStepContainer(ctx context.Context, step step, image string, cmd, entrypo if rc.IsHostEnv(ctx) { networkMode = "default" } - stepContainer := ContainerNewContainer(&container.NewContainerInput{ + return ContainerNewContainer(&container.NewContainerInput{ Cmd: cmd, Entrypoint: entrypoint, WorkingDir: rc.JobContainer.ToContainerPath(rc.Config.Workdir), Image: image, - Name: createContainerName(rc.jobContainerName(), "STEP-"+stepModel.ID), + Name: createContainerName(rc.jobContainerName(), "STEP-"+step.getStepModel().ID), Env: envList, Mounts: mounts, NetworkMode: networkMode, @@ -508,12 +452,11 @@ func newStepContainer(ctx context.Context, step step, image string, cmd, entrypo Privileged: rc.Config.Privileged, UsernsMode: rc.Config.UsernsMode, Platform: rc.Config.ContainerArchitecture, - Options: rc.Config.ContainerOptions, - AutoRemove: rc.Config.AutoRemove, + Options: options, + AutoRemove: true, ValidVolumes: rc.validVolumes(), AllocatePTY: rc.Config.AllocatePTY, }) - return stepContainer } func populateEnvsFromSavedState(env *map[string]string, step actionStep, rc *RunContext) { @@ -648,7 +591,7 @@ func runPreStep(step actionStep) common.Executor { rc.ApplyExtraPath(ctx, step.getEnv()) - return rc.execJobContainer(containerArgs, *step.getEnv(), "", "")(ctx) + return rc.JobContainer.Exec(containerArgs, *step.getEnv(), "", "")(ctx) case x.IsDocker(): // defaults in pre steps were missing, however provided inputs are available @@ -681,8 +624,8 @@ func runPreStep(step actionStep) common.Executor { execArgs := []string{filepath.Join(containerActionDir, execFileName)} return common.NewPipelineExecutor( - rc.execJobContainer(buildArgs, *step.getEnv(), "", containerActionDir), - rc.execJobContainer(execArgs, *step.getEnv(), "", ""), + rc.JobContainer.Exec(buildArgs, *step.getEnv(), "", containerActionDir), + rc.JobContainer.Exec(execArgs, *step.getEnv(), "", ""), )(ctx) default: return nil @@ -749,7 +692,7 @@ func runPostStep(step actionStep) common.Executor { rc.ApplyExtraPath(ctx, step.getEnv()) - return rc.execJobContainer(containerArgs, *step.getEnv(), "", "")(ctx) + return rc.JobContainer.Exec(containerArgs, *step.getEnv(), "", "")(ctx) case x.IsDocker(): populateEnvsFromSavedState(step.getEnv(), step, rc) @@ -775,8 +718,8 @@ func runPostStep(step actionStep) common.Executor { execArgs := []string{filepath.Join(containerActionDir, execFileName)} return common.NewPipelineExecutor( - rc.execJobContainer(buildArgs, *step.getEnv(), "", containerActionDir), - rc.execJobContainer(execArgs, *step.getEnv(), "", ""), + rc.JobContainer.Exec(buildArgs, *step.getEnv(), "", containerActionDir), + rc.JobContainer.Exec(execArgs, *step.getEnv(), "", ""), )(ctx) default: diff --git a/act/runner/action_cache.go b/act/runner/action_cache.go deleted file mode 100644 index 7e014fd7..00000000 --- a/act/runner/action_cache.go +++ /dev/null @@ -1,156 +0,0 @@ -// Copyright 2023 The Gitea Authors. All rights reserved. -// Copyright 2023 The nektos/act Authors. All rights reserved. -// SPDX-License-Identifier: MIT - -package runner - -import ( - "archive/tar" - "context" - "crypto/rand" - "encoding/hex" - "errors" - "io" - "io/fs" - "path" - "strings" - - git "github.com/go-git/go-git/v5" - config "github.com/go-git/go-git/v5/config" - "github.com/go-git/go-git/v5/plumbing" - "github.com/go-git/go-git/v5/plumbing/object" - "github.com/go-git/go-git/v5/plumbing/transport" - "github.com/go-git/go-git/v5/plumbing/transport/http" -) - -type ActionCache interface { - Fetch(ctx context.Context, cacheDir, url, ref, token string) (string, error) - GetTarArchive(ctx context.Context, cacheDir, sha, includePrefix string) (io.ReadCloser, error) -} - -type GoGitActionCache struct { - Path string -} - -func (c GoGitActionCache) Fetch(ctx context.Context, cacheDir, url, ref, token string) (string, error) { - gitPath := path.Join(c.Path, safeFilename(cacheDir)+".git") - gogitrepo, err := git.PlainInit(gitPath, true) - if errors.Is(err, git.ErrRepositoryAlreadyExists) { - gogitrepo, err = git.PlainOpen(gitPath) - } - if err != nil { - return "", err - } - tmpBranch := make([]byte, 12) - if _, err := rand.Read(tmpBranch); err != nil { - return "", err - } - branchName := hex.EncodeToString(tmpBranch) - - var auth transport.AuthMethod - if token != "" { - auth = &http.BasicAuth{ - Username: "token", - Password: token, - } - } - remote, err := gogitrepo.CreateRemoteAnonymous(&config.RemoteConfig{ - Name: "anonymous", - URLs: []string{ - url, - }, - }) - if err != nil { - return "", err - } - defer func() { - _ = gogitrepo.DeleteBranch(branchName) - }() - if err := remote.FetchContext(ctx, &git.FetchOptions{ - RefSpecs: []config.RefSpec{ - config.RefSpec(ref + ":" + branchName), - }, - Auth: auth, - Force: true, - }); err != nil { - return "", err - } - hash, err := gogitrepo.ResolveRevision(plumbing.Revision(branchName)) - if err != nil { - return "", err - } - return hash.String(), nil -} - -func (c GoGitActionCache) GetTarArchive(ctx context.Context, cacheDir, sha, includePrefix string) (io.ReadCloser, error) { - gitPath := path.Join(c.Path, safeFilename(cacheDir)+".git") - gogitrepo, err := git.PlainOpen(gitPath) - if err != nil { - return nil, err - } - commit, err := gogitrepo.CommitObject(plumbing.NewHash(sha)) - if err != nil { - return nil, err - } - files, err := commit.Files() - if err != nil { - return nil, err - } - rpipe, wpipe := io.Pipe() - // Interrupt io.Copy using ctx - ch := make(chan int, 1) - go func() { - select { - case <-ctx.Done(): - wpipe.CloseWithError(ctx.Err()) - case <-ch: - } - }() - go func() { - defer wpipe.Close() - defer close(ch) - tw := tar.NewWriter(wpipe) - cleanIncludePrefix := path.Clean(includePrefix) - wpipe.CloseWithError(files.ForEach(func(f *object.File) error { - if err := ctx.Err(); err != nil { - return err - } - name := f.Name - if strings.HasPrefix(name, cleanIncludePrefix+"/") { - name = name[len(cleanIncludePrefix)+1:] - } else if cleanIncludePrefix != "." && name != cleanIncludePrefix { - return nil - } - fmode, err := f.Mode.ToOSFileMode() - if err != nil { - return err - } - if fmode&fs.ModeSymlink == fs.ModeSymlink { - content, err := f.Contents() - if err != nil { - return err - } - return tw.WriteHeader(&tar.Header{ - Name: name, - Mode: int64(fmode), - Linkname: content, - }) - } - err = tw.WriteHeader(&tar.Header{ - Name: name, - Mode: int64(fmode), - Size: f.Size, - }) - if err != nil { - return err - } - reader, err := f.Reader() - if err != nil { - return err - } - _, err = io.Copy(tw, reader) - return err - })) - }() - return rpipe, err -} diff --git a/act/runner/action_cache_test.go b/act/runner/action_cache_test.go deleted file mode 100644 index bdd7fba0..00000000 --- a/act/runner/action_cache_test.go +++ /dev/null @@ -1,157 +0,0 @@ -// Copyright 2023 The Gitea Authors. All rights reserved. -// Copyright 2023 The nektos/act Authors. All rights reserved. -// SPDX-License-Identifier: MIT - -package runner - -import ( - "archive/tar" - "bytes" - "context" - "fmt" - "io" - "os" - "os/exec" - "path/filepath" - "strings" - "testing" - "time" - - "gitea.com/gitea/runner/act/common" - - "gitea.dev/actionslib/pkg/model" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func runGit(t *testing.T, dir string, args ...string) { - t.Helper() - if dir != "" { - args = append([]string{"-C", dir}, args...) - } - cmd := exec.Command("git", args...) - // Fixed identity and host-config isolation so commits succeed offline regardless of the - // host's git config (mirrors gitCmd in act/common/git). - cmd.Env = append(os.Environ(), - "GIT_AUTHOR_NAME=test", "GIT_AUTHOR_EMAIL=test@example.com", - "GIT_COMMITTER_NAME=test", "GIT_COMMITTER_EMAIL=test@example.com", - "GIT_CONFIG_GLOBAL=/dev/null", "GIT_CONFIG_SYSTEM=/dev/null", - ) - out, err := cmd.CombinedOutput() - require.NoError(t, err, string(out)) -} - -// TestShortShaActionRejected verifies a `uses` ref that is a shortened commit SHA is rejected -// with a clear error. The action is resolved from a local repo (via DefaultActionInstance) so -// this runs offline. -func TestShortShaActionRejected(t *testing.T) { - // a local "remote" action repo at /actions/hello-world-docker-action - actionRoot := t.TempDir() - repo := filepath.Join(actionRoot, "actions", "hello-world-docker-action") - require.NoError(t, os.MkdirAll(repo, 0o755)) - runGit(t, "", "init", "--initial-branch=main", repo) - require.NoError(t, os.WriteFile(filepath.Join(repo, "action.yml"), - []byte("name: hello\nruns:\n using: node24\n main: index.js\n"), 0o644)) - runGit(t, repo, "add", ".") - runGit(t, repo, "commit", "-m", "initial") - out, err := exec.Command("git", "-C", repo, "rev-parse", "HEAD").Output() - require.NoError(t, err) - shortSha := strings.TrimSpace(string(out))[:7] - - // a workflow that uses the action at the short SHA - wfDir := filepath.Join(t.TempDir(), "wf") - require.NoError(t, os.MkdirAll(wfDir, 0o755)) - wf := fmt.Sprintf("on: push\njobs:\n test:\n runs-on: ubuntu-latest\n steps:\n - uses: actions/hello-world-docker-action@%s\n", shortSha) - require.NoError(t, os.WriteFile(filepath.Join(wfDir, "push.yml"), []byte(wf), 0o644)) - - runner, err := New(&Config{ - Workdir: wfDir, - EventName: "push", - Platforms: map[string]string{"ubuntu-latest": baseImage}, - GitHubInstance: "github.com", - DefaultActionInstance: actionRoot, - ContainerMaxLifetime: time.Hour, - }) - require.NoError(t, err) - planner, err := model.NewWorkflowPlanner(wfDir, true) - require.NoError(t, err) - plan, err := planner.PlanEvent("push") - require.NoError(t, err) - - err = runner.NewPlanExecutor(plan)(common.WithDryrun(context.Background(), true)) - require.Error(t, err) - assert.Contains(t, err.Error(), "shortened version of a commit SHA") -} - -func TestActionCache(t *testing.T) { - a := assert.New(t) - ctx := context.Background() - - // Build a local bare repo with a `js` action dir so this runs offline (formerly cloned - // github.com/nektos/act-test-actions over the network). allowAnySHA1InWant lets the - // "Fetch Sha" case fetch a commit hash directly. - remoteDir := t.TempDir() - runGit(t, "", "init", "--bare", "--initial-branch=main", remoteDir) - runGit(t, remoteDir, "config", "uploadpack.allowAnySHA1InWant", "true") - - workDir := t.TempDir() - runGit(t, "", "clone", remoteDir, workDir) - require.NoError(t, os.MkdirAll(filepath.Join(workDir, "js"), 0o755)) - require.NoError(t, os.WriteFile(filepath.Join(workDir, "js", "action.yml"), - []byte("name: js\nruns:\n using: node24\n main: index.js\n"), 0o644)) - require.NoError(t, os.WriteFile(filepath.Join(workDir, "js", "index.js"), - []byte("console.log('hello');\n"), 0o644)) - runGit(t, workDir, "add", ".") - runGit(t, workDir, "commit", "-m", "initial") - runGit(t, workDir, "push", "-u", "origin", "main") - - out, err := exec.Command("git", "-C", workDir, "rev-parse", "main").Output() - require.NoError(t, err) - fullSha := strings.TrimSpace(string(out)) - - cache := &GoGitActionCache{ - Path: t.TempDir(), - } - cacheDir := "local/act-test-actions" - refs := []struct { - Name string - Ref string - }{ - {Name: "Fetch Branch Name", Ref: "main"}, - {Name: "Fetch Branch Name Absolutely", Ref: "refs/heads/main"}, - {Name: "Fetch HEAD", Ref: "HEAD"}, - {Name: "Fetch Sha", Ref: fullSha}, - } - for _, c := range refs { - t.Run(c.Name, func(t *testing.T) { - sha, err := cache.Fetch(ctx, cacheDir, remoteDir, c.Ref, "") - if !a.NoError(err) || !a.NotEmpty(sha) { //nolint:testifylint // pre-existing issue from nektos/act - return - } - atar, err := cache.GetTarArchive(ctx, cacheDir, sha, "js") - // NotNil, not NotEmpty: atar is a live io.PipeReader whose producer goroutine is - // writing concurrently; NotEmpty deep-reflects over its internals and races. - if !a.NoError(err) || !a.NotNil(atar) { //nolint:testifylint // pre-existing issue from nektos/act - return - } - // GetTarArchive streams from a background goroutine walking the shared repo. - // Drain and close so it finishes before the next subtest fetches into the same - // repo; otherwise the lingering walk races with that fetch. - defer func() { - _, _ = io.Copy(io.Discard, atar) - _ = atar.Close() - }() - mytar := tar.NewReader(atar) - th, err := mytar.Next() - if !a.NoError(err) || !a.NotEqual(0, th.Size) { //nolint:testifylint // pre-existing issue from nektos/act - return - } - buf := &bytes.Buffer{} - // G110: Potential DoS vulnerability via decompression bomb (gosec) - _, err = io.Copy(buf, mytar) - a.NoError(err) //nolint:testifylint // pre-existing issue from nektos/act - str := buf.String() - a.NotEmpty(str) - }) - } -} diff --git a/act/runner/action_composite.go b/act/runner/action_composite.go index 70983c2b..2be93c71 100644 --- a/act/runner/action_composite.go +++ b/act/runner/action_composite.go @@ -181,20 +181,7 @@ func (rc *RunContext) compositeExecutor(action *model.Action) *compositeSteps { stepPre := rc.newCompositeCommandExecutor(step.pre()) preSteps = append(preSteps, newCompositeStepLogExecutor(stepPre, stepID)) - steps = append(steps, func(ctx context.Context) error { - ctx = WithCompositeStepLogger(ctx, stepID) - logger := common.Logger(ctx) - err := rc.newCompositeCommandExecutor(step.main())(ctx) - - if err != nil { - logger.Errorf("##[error]%s", EscapeCommandData(err.Error())) - common.SetJobError(ctx, err) - } else if ctx.Err() != nil { - logger.Errorf("##[error]%s", EscapeCommandData(ctx.Err().Error())) - common.SetJobError(ctx, ctx.Err()) - } - return nil - }) + steps = append(steps, newCompositeStepLogExecutor(rc.newCompositeCommandExecutor(step.main()), stepID)) // run the post executor in reverse order if postExecutor != nil { @@ -222,19 +209,7 @@ func (rc *RunContext) newCompositeCommandExecutor(executor common.Executor) comm return func(ctx context.Context) error { ctx = WithCompositeLogger(ctx, &rc.Masks) - // We need to inject a composite RunContext related command - // handler into the current running job container - // We need this, to support scoping commands to the composite action - // executing. - rawLogger := common.Logger(ctx).WithField("raw_output", true) - logWriter := common.NewLineWriter(rc.commandHandler(ctx), func(s string) bool { - if rc.Config.LogOutput { - rawLogger.Infof("%s", s) - } else { - rawLogger.Debugf("%s", s) - } - return true - }) + logWriter := rc.commandLogWriter(ctx) oldout, olderr := rc.JobContainer.ReplaceLogWriter(logWriter, logWriter) defer rc.JobContainer.ReplaceLogWriter(oldout, olderr) diff --git a/act/runner/action_test.go b/act/runner/action_test.go index e358bef9..29e279a7 100644 --- a/act/runner/action_test.go +++ b/act/runner/action_test.go @@ -23,12 +23,10 @@ import ( "github.com/stretchr/testify/require" ) -type closerMock struct { - mock.Mock -} +type closerFunc func() -func (m *closerMock) Close() error { - m.Called() +func (close closerFunc) Close() error { + close() return nil } @@ -39,6 +37,15 @@ runs: using: 'node16' main: 'main.js' `, "\t", " ") + yamlAction := &model.Action{ + Name: "name", + Runs: model.ActionRuns{ + Using: "node16", + Main: "main.js", + PreIf: "always()", + PostIf: "always()", + }, + } table := []struct { name string @@ -52,30 +59,14 @@ runs: step: &model.Step{}, filename: "action.yml", fileContent: yaml, - expected: &model.Action{ - Name: "name", - Runs: model.ActionRuns{ - Using: "node16", - Main: "main.js", - PreIf: "always()", - PostIf: "always()", - }, - }, + expected: yamlAction, }, { name: "readActionYaml", step: &model.Step{}, filename: "action.yaml", fileContent: yaml, - expected: &model.Action{ - Name: "name", - Runs: model.ActionRuns{ - Using: "node16", - Main: "main.js", - PreIf: "always()", - PostIf: "always()", - }, - }, + expected: yamlAction, }, { name: "readDockerfile", @@ -121,14 +112,14 @@ runs: for _, tt := range table { t.Run(tt.name, func(t *testing.T) { - closerMock := &closerMock{} + closed := false readFile := func(filename string) (io.Reader, io.Closer, error) { if tt.filename != filename { return nil, nil, fs.ErrNotExist } - return strings.NewReader(tt.fileContent), closerMock, nil + return strings.NewReader(tt.fileContent), closerFunc(func() { closed = true }), nil } writeFile := func(filename string, data []byte, perm fs.FileMode) error { @@ -137,58 +128,16 @@ runs: return nil } - if tt.filename != "" { - closerMock.On("Close") - } - action, err := readActionImpl(context.Background(), tt.step, "actionDir", "actionPath", readFile, writeFile) assert.NoError(t, err) //nolint:testifylint // pre-existing issue from nektos/act assert.Equal(t, tt.expected, action) - closerMock.AssertExpectations(t) + assert.Equal(t, tt.filename != "", closed) }) } } -// With AutoRemove the daemon reaps the container on exit, so act must not remove it afterwards. -func TestExecAsDockerAutoRemove(t *testing.T) { - orig := ContainerNewContainer - defer func() { ContainerNewContainer = orig }() - - for _, tc := range []struct { - autoRemove bool - removes int - }{ - {false, 2}, // stale + post-run - {true, 1}, // post-run skipped - } { - cm := &containerMock{} - ContainerNewContainer = func(*container.NewContainerInput) container.ExecutionsEnvironment { return cm } - - step := &stepActionRemote{ - Step: &model.Step{ID: "1", Uses: "org/action@v1"}, - RunContext: &RunContext{ - Config: &Config{AutoRemove: tc.autoRemove}, - Run: &model.Run{JobID: "1", Workflow: &model.Workflow{Jobs: map[string]*model.Job{"1": {}}}}, - JobContainer: cm, - }, - action: &model.Action{Runs: model.ActionRuns{Using: "docker", Image: "docker://node:14"}}, - } - - removes := 0 - cm.On("Pull", false).Return(func(context.Context) error { return nil }) - cm.On("Remove").Return(func(context.Context) error { removes++; return nil }) - cm.On("Create", []string(nil), []string(nil)).Return(func(context.Context) error { return nil }) - cm.On("Start", true).Return(func(context.Context) error { return nil }) - cm.On("Close").Return(func(context.Context) error { return nil }) - - require.NoError(t, execAsDocker(context.Background(), step, "action", t.TempDir(), t.TempDir(), false, stepStageMain)) - cm.AssertExpectations(t) - assert.Equal(t, tc.removes, removes) - } -} - func TestActionRunner(t *testing.T) { table := []struct { name string @@ -337,11 +286,12 @@ func TestNewStepContainerDoesNotUseDockerSecrets(t *testing.T) { step.On("getStepModel").Return(&model.Step{ID: "action"}) step.On("getEnv").Return(&env) - _ = newStepContainer(ctx, step, "registry.example.com/action:tag", nil, nil) + _ = newStepContainer(ctx, step, "registry.example.com/action:tag", nil, nil, "") // DOCKER_USERNAME/DOCKER_PASSWORD should not be injected as pull credentials for docker action containers. assert.Empty(t, captured.Username) assert.Empty(t, captured.Password) + assert.True(t, captured.AutoRemove) step.AssertExpectations(t) } diff --git a/act/runner/command.go b/act/runner/command.go index ef8fc836..0c7d07a6 100644 --- a/act/runner/command.go +++ b/act/runner/command.go @@ -163,10 +163,6 @@ func (rc *RunContext) setOutput(ctx context.Context, kvPairs map[string]string, logger := common.Logger(ctx) stepID := rc.CurrentStep outputName := kvPairs["name"] - if outputMapping, ok := rc.OutputMappings[MappableOutput{StepID: stepID, OutputName: outputName}]; ok { - stepID = outputMapping.StepID - outputName = outputMapping.OutputName - } result, ok := rc.StepResults[stepID] if !ok { diff --git a/act/runner/container_mock_test.go b/act/runner/container_mock_test.go index 8a56b26f..890a526d 100644 --- a/act/runner/container_mock_test.go +++ b/act/runner/container_mock_test.go @@ -14,6 +14,8 @@ import ( "github.com/stretchr/testify/mock" ) +var noopExecutor = func(context.Context) error { return nil } + type containerMock struct { mock.Mock container.Container @@ -50,11 +52,6 @@ func (cm *containerMock) UpdateFromEnv(srcPath string, env *map[string]string) c return args.Get(0).(func(context.Context) error) } -func (cm *containerMock) UpdateFromImageEnv(env *map[string]string) common.Executor { - args := cm.Called(env) - return args.Get(0).(func(context.Context) error) -} - func (cm *containerMock) Copy(destPath string, files ...*container.FileEntry) common.Executor { args := cm.Called(destPath, files) return args.Get(0).(func(context.Context) error) diff --git a/act/runner/expression.go b/act/runner/expression.go index 08bfa459..b791cce0 100644 --- a/act/runner/expression.go +++ b/act/runner/expression.go @@ -26,20 +26,12 @@ import ( "go.yaml.in/yaml/v4" ) -// ExpressionEvaluator is the interface for evaluating expressions -type ExpressionEvaluator interface { - evaluate(context.Context, string, exprparser.DefaultStatusCheck) (any, error) - interpolate(context.Context, string) (string, error) - EvaluateYamlNode(context.Context, *yaml.Node) error - Interpolate(context.Context, string) string -} - // NewExpressionEvaluator creates a new evaluator -func (rc *RunContext) NewExpressionEvaluator(ctx context.Context) ExpressionEvaluator { +func (rc *RunContext) NewExpressionEvaluator(ctx context.Context) *ExpressionEvaluator { return rc.NewExpressionEvaluatorWithEnv(ctx, rc.GetEnv()) } -func (rc *RunContext) NewExpressionEvaluatorWithEnv(ctx context.Context, env map[string]string) ExpressionEvaluator { +func (rc *RunContext) NewExpressionEvaluatorWithEnv(ctx context.Context, env map[string]string) *ExpressionEvaluator { var workflowCallResult map[string]*model.WorkflowCallResult // todo: cleanup EvaluationEnvironment creation @@ -98,7 +90,7 @@ func (rc *RunContext) NewExpressionEvaluatorWithEnv(ctx context.Context, env map HashFiles: getHashFilesFunction(ctx, rc), } ee.Runner = rc.getRunnerContext(ctx) - return expressionEvaluator{ + return &expressionEvaluator{ interpreter: exprparser.NewInterpeter(ee, exprparser.Config{ Run: rc.Run, WorkingDir: rc.Config.Workdir, @@ -111,7 +103,7 @@ func (rc *RunContext) NewExpressionEvaluatorWithEnv(ctx context.Context, env map var hashfiles string // NewStepExpressionEvaluator creates a new evaluator -func (rc *RunContext) NewStepExpressionEvaluator(ctx context.Context, step step) ExpressionEvaluator { +func (rc *RunContext) NewStepExpressionEvaluator(ctx context.Context, step step) *ExpressionEvaluator { // todo: cleanup EvaluationEnvironment creation job := rc.Run.Job() strategy := make(map[string]any) @@ -150,7 +142,7 @@ func (rc *RunContext) NewStepExpressionEvaluator(ctx context.Context, step step) HashFiles: getHashFilesFunction(ctx, rc), } ee.Runner = rc.getRunnerContext(ctx) - return expressionEvaluator{ + return &expressionEvaluator{ interpreter: exprparser.NewInterpeter(ee, exprparser.Config{ Run: rc.Run, WorkingDir: rc.Config.Workdir, @@ -196,7 +188,7 @@ func getHashFilesFunction(ctx context.Context, rc *RunContext) func(v []reflect. Mode: 0o644, Body: hashfiles, }). - Then(rc.execJobContainer([]string{"node", path.Join(rc.JobContainer.GetActPath(), name)}, + Then(rc.JobContainer.Exec([]string{"node", path.Join(rc.JobContainer.GetActPath(), name)}, env, "", "")). Finally(func(context.Context) error { rc.JobContainer.ReplaceLogWriter(stdout, stderr) @@ -222,6 +214,8 @@ type expressionEvaluator struct { interpreter exprparser.Interpreter } +type ExpressionEvaluator = expressionEvaluator + func (ee expressionEvaluator) evaluate(ctx context.Context, in string, defaultStatusCheck exprparser.DefaultStatusCheck) (any, error) { logger := common.Logger(ctx) logger.Debugf("evaluating expression '%s'", in) @@ -261,7 +255,7 @@ func (ee expressionEvaluator) interpolate(ctx context.Context, in string) (strin // EvalBool evaluates an expression against given evaluator. An `if:` is an expression even without // `${{ }}`, while literal text around one makes the whole value a string. -func EvalBool(ctx context.Context, evaluator ExpressionEvaluator, expr string, defaultStatusCheck exprparser.DefaultStatusCheck) (bool, error) { +func EvalBool(ctx context.Context, evaluator *expressionEvaluator, expr string, defaultStatusCheck exprparser.DefaultStatusCheck) (bool, error) { return expreval.New(func(in string, dsc exprparser.DefaultStatusCheck) (any, error) { return evaluator.evaluate(ctx, in, dsc) }).EvalBool(expr, defaultStatusCheck) diff --git a/act/runner/job_executor.go b/act/runner/job_executor.go index 81bc84cd..222b50f9 100644 --- a/act/runner/job_executor.go +++ b/act/runner/job_executor.go @@ -240,43 +240,15 @@ func newJobExecutor(info jobInfo, sf stepFactory, rc *RunContext) common.Executo postExecutor = postExecutor.Finally(func(ctx context.Context) error { jobError := common.JobError(ctx) var err error - // jobError == nil keeps a failed job's container alive for post-mortem debugging when - // AutoRemove is off (the act-CLI --rm behavior; the shipped runner always sets - // AutoRemove). A cancelled run is not a failure to inspect, and the cancel-path post - // context now carries its own error container so a failing post step makes jobError - // non-nil — OR in rc.jobCancelled so cancellation still always tears the container down. - if rc.Config.AutoRemove || jobError == nil || rc.jobCancelled { - // always allow 1 min for stopping and removing the runner, even if we were cancelled - ctx, cancel := context.WithTimeout(common.WithLogger(context.Background(), common.Logger(ctx)), time.Minute) - defer cancel() + // always allow 1 min for stopping and removing the runner, even if we were cancelled + ctx, cancel := context.WithTimeout(common.WithLogger(context.Background(), common.Logger(ctx)), time.Minute) + defer cancel() - logger := common.Logger(ctx) - tryUploadJobSummary(ctx, rc) - // For Gitea - // We don't need to call `stopServiceContainers` here since it will be called by following `info.stopContainer` - // logger.Infof("Cleaning up services for job %s", rc.JobName) - // if err := rc.stopServiceContainers()(ctx); err != nil { - // logger.Errorf("Error while cleaning services: %v", err) - // } - - logger.Infof("Cleaning up container for job %s", rc.JobName) - if err = info.stopContainer()(ctx); err != nil { - logger.Errorf("##[error]%s", EscapeCommandData("Error while stop job container: "+err.Error())) - } - - // For Gitea - // We don't need to call `NewDockerNetworkRemoveExecutor` here since it is called by above `info.stopContainer` - // if !rc.IsHostEnv(ctx) && rc.Config.ContainerNetworkMode == "" { - // // clean network in docker mode only - // // if the value of `ContainerNetworkMode` is empty string, - // // it means that the network to which containers are connecting is created by `runner`, - // // so, we should remove the network at last. - // networkName, _ := rc.networkName() - // logger.Infof("Cleaning up network for job %s, and network name is: %s", rc.JobName, networkName) - // if err := container.NewDockerNetworkRemoveExecutor(networkName)(ctx); err != nil { - // logger.Errorf("Error while cleaning network: %v", err) - // } - // } + logger := common.Logger(ctx) + tryUploadJobSummary(ctx, rc) + logger.Infof("Cleaning up container for job %s", rc.JobName) + if err = info.stopContainer()(ctx); err != nil { + logger.Errorf("##[error]%s", EscapeCommandData("Error while stop job container: "+err.Error())) } setJobResult(ctx, info, rc, jobError == nil) setJobOutputs(ctx, rc) @@ -651,15 +623,7 @@ func useStepLogger(rc *RunContext, stepModel *model.Step, stage stepStage, execu return func(ctx context.Context) error { ctx = withStepLogger(ctx, stepModel.Number, stepModel.ID, rc.ExprEval.Interpolate(ctx, stepModel.String()), stage.String()) - rawLogger := common.Logger(ctx).WithField("raw_output", true) - logWriter := common.NewLineWriter(rc.commandHandler(ctx), func(s string) bool { - if rc.Config.LogOutput { - rawLogger.Infof("%s", s) - } else { - rawLogger.Debugf("%s", s) - } - return true - }) + logWriter := rc.commandLogWriter(ctx) oldout, olderr := rc.JobContainer.ReplaceLogWriter(logWriter, logWriter) defer rc.JobContainer.ReplaceLogWriter(oldout, olderr) diff --git a/act/runner/job_executor_test.go b/act/runner/job_executor_test.go index 6e7fc2b5..1a841bd6 100644 --- a/act/runner/job_executor_test.go +++ b/act/runner/job_executor_test.go @@ -336,6 +336,7 @@ func TestNewJobExecutor(t *testing.T) { executedSteps: []string{ "startContainer", "step1", + "stopContainer", "interpolateOutputs", "closeContainer", }, @@ -562,9 +563,8 @@ func TestNewJobExecutorRunsPostStepsAfterTimeout(t *testing.T) { jim.On("startContainer").Return(func(ctx context.Context) error { return nil }) jim.On("interpolateOutputs").Return(func(ctx context.Context) error { return nil }) jim.On("closeContainer").Return(func(ctx context.Context) error { return nil }) - // The job timed out, so it must be reported as failed. stopContainer is left - // unexpected on purpose: a timed-out (failed) job preserves its error state, so - // the graceful stop is skipped exactly like any other failure without AutoRemove. + // The job timed out, so it must be reported as failed and still cleaned up. + jim.On("stopContainer").Return(func(context.Context) error { return nil }) jim.On("result", "failure") sm := &stepMock{} @@ -984,12 +984,7 @@ func tarArchive(t *testing.T, entries ...tarEntry) []byte { func newTestRC(wf *model.Workflow, matrix map[string]any) *RunContext { return &RunContext{ - Config: &Config{ - Workdir: ".", - Platforms: map[string]string{ - "ubuntu-latest": "ubuntu-latest", - }, - }, + Config: &Config{Workdir: ".", PlatformPicker: func([]string) string { return "ubuntu-latest" }}, StepResults: map[string]*model.StepResult{}, Env: map[string]string{}, Matrix: matrix, diff --git a/act/runner/local_repository_cache.go b/act/runner/local_repository_cache.go deleted file mode 100644 index d8e1ebad..00000000 --- a/act/runner/local_repository_cache.go +++ /dev/null @@ -1,95 +0,0 @@ -// Copyright 2024 The Gitea Authors. All rights reserved. -// Copyright 2024 The nektos/act Authors. All rights reserved. -// SPDX-License-Identifier: MIT - -package runner - -import ( - "archive/tar" - "bytes" - "context" - "fmt" - "io" - "io/fs" - goURL "net/url" - "os" - "path/filepath" - "strings" - - "gitea.com/gitea/runner/act/filecollector" -) - -type LocalRepositoryCache struct { - Parent ActionCache - LocalRepositories map[string]string - CacheDirCache map[string]string -} - -func (l *LocalRepositoryCache) Fetch(ctx context.Context, cacheDir, url, ref, token string) (string, error) { - if dest, ok := l.LocalRepositories[fmt.Sprintf("%s@%s", url, ref)]; ok { - l.CacheDirCache[fmt.Sprintf("%s@%s", cacheDir, ref)] = dest - return ref, nil - } - if purl, err := goURL.Parse(url); err == nil { - if dest, ok := l.LocalRepositories[fmt.Sprintf("%s@%s", strings.TrimPrefix(purl.Path, "/"), ref)]; ok { - l.CacheDirCache[fmt.Sprintf("%s@%s", cacheDir, ref)] = dest - return ref, nil - } - } - return l.Parent.Fetch(ctx, cacheDir, url, ref, token) -} - -func (l *LocalRepositoryCache) GetTarArchive(ctx context.Context, cacheDir, sha, includePrefix string) (io.ReadCloser, error) { - // sha is mapped to ref in fetch if there is a local override - if dest, ok := l.CacheDirCache[fmt.Sprintf("%s@%s", cacheDir, sha)]; ok { - srcPath := filepath.Join(dest, includePrefix) - buf := &bytes.Buffer{} - tw := tar.NewWriter(buf) - defer tw.Close() - srcPath = filepath.Clean(srcPath) - fi, err := os.Lstat(srcPath) - if err != nil { - return nil, err - } - tc := &filecollector.TarCollector{ - TarWriter: tw, - } - if fi.IsDir() { - srcPrefix := srcPath - if !strings.HasSuffix(srcPrefix, string(filepath.Separator)) { - srcPrefix += string(filepath.Separator) - } - fc := &filecollector.FileCollector{ - Fs: &filecollector.DefaultFs{}, - SrcPath: srcPath, - SrcPrefix: srcPrefix, - Handler: tc, - } - err = filepath.Walk(srcPath, fc.CollectFiles(ctx, []string{})) - if err != nil { - return nil, err - } - } else { - var f io.ReadCloser - var linkname string - if fi.Mode()&fs.ModeSymlink != 0 { - linkname, err = os.Readlink(srcPath) - if err != nil { - return nil, err - } - } else { - f, err = os.Open(srcPath) - if err != nil { - return nil, err - } - defer f.Close() - } - err := tc.WriteFile(fi.Name(), fi, linkname, f) - if err != nil { - return nil, err - } - } - return io.NopCloser(buf), nil - } - return l.Parent.GetTarArchive(ctx, cacheDir, sha, includePrefix) -} diff --git a/act/runner/logger.go b/act/runner/logger.go index 615fc0e4..80affca0 100644 --- a/act/runner/logger.go +++ b/act/runner/logger.go @@ -99,10 +99,7 @@ func WithJobLogger(ctx context.Context, jobID, jobName string, config *Config, m mux.Lock() defer mux.Unlock() nextColor++ - formatter = &jobLogFormatter{ - color: colors[nextColor%len(colors)], - logPrefixJobID: config.LogPrefixJobID, - } + formatter = &jobLogFormatter{color: colors[nextColor%len(colors)]} } logger = logrus.New() @@ -338,8 +335,7 @@ func (f *maskedFormatter) Format(entry *logrus.Entry) ([]byte, error) { } type jobLogFormatter struct { - color int - logPrefixJobID bool + color int } func (f *jobLogFormatter) Format(entry *logrus.Entry) ([]byte, error) { @@ -363,12 +359,7 @@ func (f *jobLogFormatter) Format(entry *logrus.Entry) ([]byte, error) { func (f *jobLogFormatter) printColored(b *bytes.Buffer, entry *logrus.Entry) { entry.Message = strings.TrimSuffix(entry.Message, "\n") - var job any - if f.logPrefixJobID { - job = entry.Data["jobID"] - } else { - job = entry.Data["job"] - } + job := entry.Data["job"] debugFlag := "" if entry.Level == logrus.DebugLevel { @@ -391,12 +382,7 @@ func (f *jobLogFormatter) printColored(b *bytes.Buffer, entry *logrus.Entry) { func (f *jobLogFormatter) print(b *bytes.Buffer, entry *logrus.Entry) { entry.Message = strings.TrimSuffix(entry.Message, "\n") - var job any - if f.logPrefixJobID { - job = entry.Data["jobID"] - } else { - job = entry.Data["job"] - } + job := entry.Data["job"] debugFlag := "" if entry.Level == logrus.DebugLevel { diff --git a/act/runner/reusable_workflow.go b/act/runner/reusable_workflow.go index c90d19fe..2654ba5c 100644 --- a/act/runner/reusable_workflow.go +++ b/act/runner/reusable_workflow.go @@ -5,7 +5,6 @@ package runner import ( - "archive/tar" "context" "fmt" "net/url" @@ -78,10 +77,6 @@ func newRemoteReusableWorkflowExecutor(rc *RunContext) common.Executor { filename := fmt.Sprintf("%s/%s@%s", remoteReusableWorkflow.Org, remoteReusableWorkflow.Repo, remoteReusableWorkflow.Ref) workflowDir := fmt.Sprintf("%s/%s", rc.ActionCacheDir(), safeFilename(filename)) - if rc.Config.ActionCache != nil { - return newActionCacheReusableWorkflowExecutor(rc, filename, remoteReusableWorkflow) - } - token := getGitCloneToken(rc.Config, remoteReusableWorkflow.CloneURL()) return common.NewPipelineExecutor( @@ -90,41 +85,6 @@ func newRemoteReusableWorkflowExecutor(rc *RunContext) common.Executor { ) } -func newActionCacheReusableWorkflowExecutor(rc *RunContext, filename string, remoteReusableWorkflow *remoteReusableWorkflow) common.Executor { - return func(ctx context.Context) error { - ghctx := rc.getGithubContext(ctx) - remoteReusableWorkflow.URL = ghctx.ServerURL - sha, err := rc.Config.ActionCache.Fetch(ctx, filename, remoteReusableWorkflow.CloneURL(), remoteReusableWorkflow.Ref, ghctx.Token) - if err != nil { - return err - } - archive, err := rc.Config.ActionCache.GetTarArchive(ctx, filename, sha, ".github/workflows/"+remoteReusableWorkflow.Filename) - if err != nil { - return err - } - defer archive.Close() - treader := tar.NewReader(archive) - if _, err = treader.Next(); err != nil { - return err - } - planner, err := model.NewSingleWorkflowPlanner(remoteReusableWorkflow.Filename, treader) - if err != nil { - return err - } - plan, err := planner.PlanEvent("workflow_call") - if err != nil { - return err - } - - runner, err := NewReusableWorkflowRunner(rc) - if err != nil { - return err - } - - return runner.NewPlanExecutor(plan)(ctx) - } -} - // cloneRemoteReusableWorkflow always invokes the clone executor — moving refs // (branches, tags) must be re-resolved each run, matching GitHub Actions. // @@ -147,15 +107,12 @@ func cloneRemoteReusableWorkflow(rc *RunContext, cloneURL, ref, targetDirectory, } } -var modelNewWorkflowPlanner = model.NewWorkflowPlanner - func newReusableWorkflowExecutor(rc *RunContext, directory, workflow string) common.Executor { return func(ctx context.Context) error { - // Scoped to the yaml read so concurrent invocations don't serialize - // on the whole job run. + // Serialize workflow reads with cache updates. planner, err := func() (model.WorkflowPlanner, error) { defer git.AcquireCloneLock(directory)() - return modelNewWorkflowPlanner(path.Join(directory, workflow), true) + return model.NewWorkflowPlanner(path.Join(directory, workflow), true) }() if err != nil { return err @@ -166,12 +123,11 @@ func newReusableWorkflowExecutor(rc *RunContext, directory, workflow string) com return err } - runner, err := NewReusableWorkflowRunner(rc) + runner, err := newReusableWorkflowRunner(rc) if err != nil { return err } - // return runner.NewPlanExecutor(plan)(ctx) return common.NewPipelineExecutor( // For Gitea runner.NewPlanExecutor(plan), setReusedWorkflowCallerResult(rc, runner), @@ -179,7 +135,7 @@ func newReusableWorkflowExecutor(rc *RunContext, directory, workflow string) com } } -func NewReusableWorkflowRunner(rc *RunContext) (Runner, error) { +func newReusableWorkflowRunner(rc *RunContext) (*runnerImpl, error) { runner := &runnerImpl{ config: rc.Config, eventJSON: rc.EventJSON, @@ -255,16 +211,9 @@ func newRemoteReusableWorkflowFromAbsoluteURL(uses string) *remoteReusableWorkfl } // For Gitea -func setReusedWorkflowCallerResult(rc *RunContext, runner Runner) common.Executor { +func setReusedWorkflowCallerResult(rc *RunContext, runner *runnerImpl) common.Executor { return func(ctx context.Context) error { - logger := common.Logger(ctx) - - runnerImpl, ok := runner.(*runnerImpl) - if !ok { - logger.Warn("Failed to get caller from runner") - return nil - } - caller := runnerImpl.caller + caller := runner.caller allJobDone := true hasFailure := false @@ -294,7 +243,7 @@ func setReusedWorkflowCallerResult(rc *RunContext, runner Runner) common.Executo unlock := lockJob(rc.Run.Job()) rc.result(reusedWorkflowJobResult) unlock() - logger.WithField("jobResult", reusedWorkflowJobResult).Infof("Job %s", reusedWorkflowJobResultMessage) + common.Logger(ctx).WithField("jobResult", reusedWorkflowJobResult).Infof("Job %s", reusedWorkflowJobResultMessage) } } diff --git a/act/runner/reusable_workflow_test.go b/act/runner/reusable_workflow_test.go index 49e8840b..82661a09 100644 --- a/act/runner/reusable_workflow_test.go +++ b/act/runner/reusable_workflow_test.go @@ -5,7 +5,6 @@ package runner import ( "context" - "errors" "os" "os/exec" "path/filepath" @@ -77,19 +76,11 @@ func TestReusableWorkflowCachedBranchRefRefreshes(t *testing.T) { func TestNewReusableWorkflowExecutorHoldsCloneLock(t *testing.T) { workflowDir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(workflowDir, "reusable.yml"), []byte(":"), 0o644)) unlockOnce := sync.OnceFunc(git.AcquireCloneLock(workflowDir)) defer unlockOnce() - plannerCalled := make(chan struct{}) - - origPlanner := modelNewWorkflowPlanner - modelNewWorkflowPlanner = func(string, bool) (model.WorkflowPlanner, error) { - close(plannerCalled) - return nil, errors.New("stop") - } - defer func() { modelNewWorkflowPlanner = origPlanner }() - rc := &RunContext{ Config: &Config{}, Run: &model.Run{Workflow: &model.Workflow{Jobs: map[string]*model.Job{}}}, @@ -100,26 +91,18 @@ func TestNewReusableWorkflowExecutorHoldsCloneLock(t *testing.T) { go func() { done <- exec(context.Background()) }() select { - case <-plannerCalled: - t.Fatal("planner ran while clone lock was held") case err := <-done: - t.Fatalf("executor returned before planner was reached: %v", err) + t.Fatalf("executor returned while clone lock was held: %v", err) case <-time.After(50 * time.Millisecond): } unlockOnce() - select { - case <-plannerCalled: - case <-time.After(time.Second): - t.Fatal("planner not called after lock was released") - } - select { case err := <-done: require.Error(t, err) case <-time.After(time.Second): - t.Fatal("executor did not return after planner ran") + t.Fatal("executor did not return after lock was released") } } diff --git a/act/runner/run_context.go b/act/runner/run_context.go index e8946160..7510c321 100644 --- a/act/runner/run_context.go +++ b/act/runner/run_context.go @@ -56,10 +56,9 @@ type RunContext struct { CurrentStepIndex int StepResults map[string]*model.StepResult IntraActionState map[string]map[string]string - ExprEval ExpressionEvaluator + ExprEval *expressionEvaluator JobContainer container.ExecutionsEnvironment serviceContainers []*serviceContainer - OutputMappings map[MappableOutput]MappableOutput JobName string ActionPath string Parent *RunContext @@ -148,11 +147,6 @@ func (rc *RunContext) AddMask(mask string) { rc.Masks = append(rc.Masks, mask) } -type MappableOutput struct { - StepID string - OutputName string -} - func (rc *RunContext) String() string { name := fmt.Sprintf("%s/%s", rc.Run.Workflow.Name, rc.Name) if rc.caller != nil { @@ -341,16 +335,7 @@ func (rc *RunContext) GetBindsAndMounts() ([]string, map[string]string) { func (rc *RunContext) startHostEnvironment() common.Executor { return func(ctx context.Context) error { - logger := common.Logger(ctx) - rawLogger := logger.WithField(rawOutputField, true) - logWriter := common.NewLineWriter(rc.commandHandler(ctx), func(s string) bool { - if rc.Config.LogOutput { - rawLogger.Infof("%s", s) - } else { - rawLogger.Debugf("%s", s) - } - return true - }) + logWriter := rc.commandLogWriter(ctx) cacheDir := rc.ActionCacheDir() randBytes := make([]byte, 8) _, _ = rand.Read(randBytes) @@ -437,15 +422,7 @@ func (rc *RunContext) startJobContainer() common.Executor { return func(ctx context.Context) error { logger := common.Logger(ctx) image := rc.platformImage(ctx) - rawLogger := logger.WithField(rawOutputField, true) - logWriter := common.NewLineWriter(rc.commandHandler(ctx), func(s string) bool { - if rc.Config.LogOutput { - rawLogger.Infof("%s", s) - } else { - rawLogger.Debugf("%s", s) - } - return true - }) + logWriter := rc.commandLogWriter(ctx) username, password, err := rc.handleCredentials(ctx) if err != nil { @@ -497,7 +474,7 @@ func (rc *RunContext) startJobContainer() common.Executor { } // keep these local: reusing username/password would overwrite the // credentials the job container is pulled with further down - serviceUsername, servicePassword, err := rc.handleServiceCredentials(ctx, spec.Credentials) + serviceUsername, servicePassword, err := rc.interpolateCredentials(ctx, spec.Credentials, "") if err != nil { return fmt.Errorf("failed to handle service %s credentials: %w", serviceID, err) } @@ -568,7 +545,7 @@ func (rc *RunContext) startJobContainer() common.Executor { UsernsMode: rc.Config.UsernsMode, Platform: rc.Config.ContainerArchitecture, Options: rc.options(ctx), - AutoRemove: rc.Config.AutoRemove, + AutoRemove: true, ValidVolumes: rc.validVolumes(), AllocatePTY: rc.Config.AllocatePTY, }) @@ -604,12 +581,20 @@ func (rc *RunContext) startJobContainer() common.Executor { } } +func (rc *RunContext) commandLogWriter(ctx context.Context) io.Writer { + rawLogger := common.Logger(ctx).WithField(rawOutputField, true) + return common.NewLineWriter(rc.commandHandler(ctx), func(line string) bool { + rawLogger.Infof("%s", line) + return true + }) +} + // cleanupJobResources removes everything the job created, continuing past failures. // Only job container and volume errors are returned, the rest are logged. func (rc *RunContext) cleanupJobResources(networkName string, createAndDeleteNetwork bool) common.Executor { return func(ctx context.Context) error { logger := common.Logger(ctx) - removeJobContainer := rc.JobContainer != nil && !rc.Config.ReuseContainers + removeJobContainer := rc.JobContainer != nil var errs []error if removeJobContainer { @@ -639,12 +624,6 @@ func (rc *RunContext) cleanupJobResources(networkName string, createAndDeleteNet } } -func (rc *RunContext) execJobContainer(cmd []string, env map[string]string, user, workdir string) common.Executor { //nolint:unparam // pre-existing issue from nektos/act - return func(ctx context.Context) error { - return rc.JobContainer.Exec(cmd, env, user, workdir)(ctx) - } -} - func (rc *RunContext) ApplyExtraPath(ctx context.Context, env *map[string]string) { if len(rc.ExtraPath) > 0 { path := rc.JobContainer.GetPathVariableName() @@ -1078,19 +1057,9 @@ func (rc *RunContext) runsOnImage(ctx context.Context) string { runsOn[i] = rc.ExprEval.Interpolate(ctx, v) } - if pick := rc.Config.PlatformPicker; pick != nil { - if image := pick(runsOn); image != "" { - return image - } + if rc.Config.PlatformPicker != nil { + return rc.Config.PlatformPicker(runsOn) } - - for _, platformName := range rc.runsOnPlatformNames(ctx) { - image := rc.Config.Platforms[strings.ToLower(platformName)] - if image != "" { - return image - } - } - return "" } @@ -1367,9 +1336,9 @@ func (rc *RunContext) getGithubContext(ctx context.Context) *model.GithubContext ghc.SetBaseAndHeadRef() repoPath := rc.Config.Workdir - ghcontext.SetRepositoryAndOwner(ctx, ghc, rc.Config.GitHubInstance, rc.Config.RemoteName, repoPath) + ghcontext.SetRepositoryAndOwner(ctx, ghc, rc.Config.GitHubInstance, repoPath) if ghc.Ref == "" { - ghcontext.SetRef(ctx, ghc, rc.Config.DefaultBranch, repoPath) + ghcontext.SetRef(ctx, ghc, repoPath) } if ghc.Sha == "" { ghcontext.SetSha(ctx, ghc, repoPath) @@ -1585,53 +1554,30 @@ func (rc *RunContext) handleCredentials(ctx context.Context) (string, string, er return "", "", nil } - if len(container.Credentials) != 2 { - err := errors.New("invalid property count for key 'credentials:'") - return "", "", err + return rc.interpolateCredentials(ctx, container.Credentials, "container.") +} + +func (rc *RunContext) interpolateCredentials(ctx context.Context, credentials map[string]string, prefix string) (string, string, error) { + if credentials == nil { + return "", "", nil + } + if len(credentials) != 2 { + return "", "", errors.New("invalid property count for key 'credentials:'") } ee := rc.NewExpressionEvaluator(ctx) - var username, password string - if username = ee.Interpolate(ctx, container.Credentials["username"]); username == "" { - err := errors.New("failed to interpolate container.credentials.username") - return "", "", err + username := ee.Interpolate(ctx, credentials["username"]) + if username == "" { + return "", "", errors.New("failed to interpolate " + prefix + "credentials.username") } - if password = ee.Interpolate(ctx, container.Credentials["password"]); password == "" { - err := errors.New("failed to interpolate container.credentials.password") - return "", "", err - } - - if container.Credentials["username"] == "" || container.Credentials["password"] == "" { - err := errors.New("container.credentials cannot be empty") - return "", "", err + password := ee.Interpolate(ctx, credentials["password"]) + if password == "" { + return "", "", errors.New("failed to interpolate " + prefix + "credentials.password") } return username, password, nil } -func (rc *RunContext) handleServiceCredentials(ctx context.Context, creds map[string]string) (username, password string, err error) { - if creds == nil { - return username, password, err - } - if len(creds) != 2 { - err = errors.New("invalid property count for key 'credentials:'") - return username, password, err - } - - ee := rc.NewExpressionEvaluator(ctx) - if username = ee.Interpolate(ctx, creds["username"]); username == "" { - err = errors.New("failed to interpolate credentials.username") - return username, password, err - } - - if password = ee.Interpolate(ctx, creds["password"]); password == "" { - err = errors.New("failed to interpolate credentials.password") - return username, password, err - } - - return username, password, err -} - // GetServiceBindsAndMounts returns the binds and mounts for the service container, resolving paths as appopriate func (rc *RunContext) GetServiceBindsAndMounts(svcVolumes []string) ([]string, map[string]string) { binds, mounts, claimed := splitVolumes(svcVolumes) diff --git a/act/runner/run_context_test.go b/act/runner/run_context_test.go index 1e22b584..dbbef269 100644 --- a/act/runner/run_context_test.go +++ b/act/runner/run_context_test.go @@ -270,11 +270,8 @@ jobs: rc := &RunContext{ Name: "test", Config: &Config{ - Workdir: "/tmp", - // no daemon: an explicit network mode creates no network, and - // reusing containers short-circuits the volume cleanup executors + Workdir: "/tmp", ContainerNetworkMode: "host", - ReuseContainers: true, Env: map[string]string{}, Secrets: map[string]string{}, }, @@ -286,7 +283,8 @@ jobs: } rc.ExprEval = rc.NewExpressionEvaluator(t.Context()) - require.NoError(t, rc.startJobContainer()(t.Context())) + t.Setenv("DOCKER_HOST", "unix:///nonexistent.sock") + require.Error(t, rc.startJobContainer()(t.Context())) credentials := map[string][2]string{} for _, in := range inputs { @@ -335,7 +333,6 @@ jobs: Config: &Config{ Workdir: "/tmp", ContainerNetworkMode: "host", - ReuseContainers: true, Env: map[string]string{}, ProxyEnv: map[string]string{"http_proxy": "http://proxy:3128", "no_proxy": "internal.example"}, Secrets: map[string]string{}, @@ -348,7 +345,8 @@ jobs: } rc.ExprEval = rc.NewExpressionEvaluator(t.Context()) - require.NoError(t, rc.startJobContainer()(t.Context())) + t.Setenv("DOCKER_HOST", "unix:///nonexistent.sock") + require.Error(t, rc.startJobContainer()(t.Context())) env := map[string][]string{} for _, in := range inputs { @@ -662,13 +660,12 @@ func TestGetGitHubContext(t *testing.T) { Name: "GitHubContextTest", }, }, - Name: "GitHubContextTest", - CurrentStep: "step", - Matrix: map[string]any{}, - Env: map[string]string{}, - ExtraPath: []string{}, - StepResults: map[string]*model.StepResult{}, - OutputMappings: map[MappableOutput]MappableOutput{}, + Name: "GitHubContextTest", + CurrentStep: "step", + Matrix: map[string]any{}, + Env: map[string]string{}, + ExtraPath: []string{}, + StepResults: map[string]*model.StepResult{}, } rc.Run.JobID = "job1" @@ -745,13 +742,8 @@ func TestGetGithubContextRef(t *testing.T) { func createIfTestRunContext(jobs map[string]*model.Job) *RunContext { rc := &RunContext{ - Config: &Config{ - Workdir: ".", - Platforms: map[string]string{ - "ubuntu-latest": "ubuntu-latest", - }, - }, - Env: map[string]string{}, + Config: &Config{Workdir: ".", PlatformPicker: func([]string) string { return "ubuntu-latest" }}, + Env: map[string]string{}, Run: &model.Run{ JobID: "job1", Workflow: &model.Workflow{ @@ -1304,15 +1296,13 @@ func TestRunContextImageOS(t *testing.T) { t.Run("prefers the release in the resolved image tag", func(t *testing.T) { rc := createRunsOnRunContext(t, "ubuntu-latest") - rc.Config.Platforms = map[string]string{ - "ubuntu-latest": "docker.gitea.com/runner-images:ubuntu-24.04", - } + rc.Config.PlatformPicker = func([]string) string { return "docker.gitea.com/runner-images:ubuntu-24.04" } assert.Equal(t, "ubuntu24", rc.imageOS(ctx)) }) t.Run("falls back to the runs-on label", func(t *testing.T) { rc := createRunsOnRunContext(t, "ubuntu-22.04") - rc.Config.Platforms = map[string]string{"ubuntu-22.04": "some-image"} + rc.Config.PlatformPicker = func([]string) string { return "some-image" } assert.Equal(t, "ubuntu22", rc.imageOS(ctx)) }) diff --git a/act/runner/runner.go b/act/runner/runner.go index e216dee0..08919d1e 100644 --- a/act/runner/runner.go +++ b/act/runner/runner.go @@ -6,7 +6,6 @@ package runner import ( "context" - "encoding/json" "fmt" "maps" "os" @@ -22,61 +21,44 @@ import ( log "github.com/sirupsen/logrus" ) -// Runner provides capabilities to run GitHub actions -type Runner interface { - NewPlanExecutor(plan *model.Plan) common.Executor -} - // Config contains the config for a new runner type Config struct { - Actor string // the user that triggered the event - Workdir string // path to working directory - ActionCacheDir string // path used for caching action contents - ActionOfflineMode bool // when offline, use cached action contents - ActionCloneDepth int // limit history when cloning an action repo; 0 clones every branch in full - BindWorkdir bool // bind the workdir to the job container - EventName string // name of event to run - EventPath string // path to JSON file to use for event.json in containers - DefaultBranch string // name of the main branch for this repository - ReuseContainers bool // reuse containers to maintain state - ForcePull bool // force pulling of the image, even if already present - ForceRebuild bool // force rebuilding local docker image action - LogOutput bool // log the output from docker run - JSONLogger bool // use json or text logger - LogPrefixJobID bool // switches from the full job name to the job id - Env map[string]string // env for containers - Inputs map[string]string // manually passed action inputs - Secrets map[string]string // list of secrets - Vars map[string]string // list of vars - Token string // GitHub token - InsecureSecrets bool // switch hiding output when printing to terminal - Platforms map[string]string // list of platforms - Privileged bool // use privileged mode - UsernsMode string // user namespace to use - ContainerArchitecture string // Desired OS/architecture platform for running containers - ContainerDaemonSocket string // Path to Docker daemon socket - ContainerOptions string // Options for the job container - UseGitIgnore bool // controls if paths in .gitignore should not be copied into container, default true - GitHubInstance string // GitHub instance to use, default "github.com" - ContainerCapAdd []string // list of kernel capabilities to add to the containers - ContainerCapDrop []string // list of kernel capabilities to remove from the containers - AutoRemove bool // controls if the container is automatically removed upon workflow completion - ArtifactServerPath string // the path where the artifact server stores uploads - ArtifactServerAddr string // the address the artifact server binds to - ArtifactServerPort string // the port the artifact server binds to - NoSkipCheckout bool // do not skip actions/checkout - DisableActEnv bool // do not inject the ACT=true environment variable into jobs - RemoteName string // remote name in local git repo config - ReplaceGheActionWithGithubCom []string // Use actions from GitHub Enterprise instance to GitHub - ReplaceGheActionTokenWithGithubCom string // Token of private action repo on GitHub. - Matrix map[string]map[string]bool // Matrix config to run - ContainerNetworkMode docker_container.NetworkMode // the network mode of job containers (the value of --network) - ContainerNetworkCreateOptions container.NewDockerNetworkCreateExecutorInput // the default network create options - ActionCache ActionCache // Use a custom ActionCache Implementation - ProxyEnv map[string]string // the proxy variables the job runs with, also given to service containers and image builds - NoActionPatch bool // run actions exactly as published, applying no compatibility patches, see patch_actions.go + Actor string // the user that triggered the event + Workdir string // path to working directory + ActionCacheDir string // path used for caching action contents + ActionOfflineMode bool // when offline, use cached action contents + ActionCloneDepth int // limit history when cloning an action repo, 0 clones every branch in full + BindWorkdir bool // bind the workdir to the job container + EventName string // name of event to run + EventPath string // path to JSON file to use for event.json in containers + ForcePull bool // force pulling of the image, even if already present + ForceRebuild bool // force rebuilding local docker image action + JSONLogger bool // use json or text logger + Env map[string]string // env for containers + Secrets map[string]string // list of secrets + Vars map[string]string // list of vars + Token string // GitHub token + InsecureSecrets bool // switch hiding output when printing to terminal + Privileged bool // use privileged mode + UsernsMode string // user namespace to use + ContainerArchitecture string // Desired OS/architecture platform for running containers + ContainerDaemonSocket string // Path to Docker daemon socket + ContainerOptions string // Options for the job container + UseGitIgnore bool // controls if paths in .gitignore should not be copied into container, default true + GitHubInstance string // GitHub instance to use, default "github.com" + ContainerCapAdd []string // list of kernel capabilities to add to the containers + ContainerCapDrop []string // list of kernel capabilities to remove from the containers + ArtifactServerPath string // the path where the artifact server stores uploads + ArtifactServerAddr string // the address the artifact server binds to + ArtifactServerPort string // the port the artifact server binds to + NoSkipCheckout bool // do not skip actions/checkout + DisableActEnv bool // do not inject the ACT=true environment variable into jobs + ContainerNetworkMode docker_container.NetworkMode // the network mode of job containers (the value of --network) + ContainerNetworkCreateOptions container.NewDockerNetworkCreateExecutorInput // the default network create options + ProxyEnv map[string]string // the proxy variables the job runs with, also given to service containers and image builds + NoActionPatch bool // run actions exactly as published, applying no compatibility patches, see patch_actions.go - PresetGitHubContext *model.GithubContext // the preset github context, overrides some fields like DefaultBranch, Env, Secrets etc. + PresetGitHubContext *model.GithubContext // overrides actor, ref, repository, token and related context fields EventJSON string // the content of JSON file to use for event.json in containers, overrides EventPath ContainerNamePrefix string // the prefix of container name ContainerMaxLifetime time.Duration // the max lifetime of job containers @@ -88,17 +70,17 @@ type Config struct { // differ from GitHubInstance when the runner registered with a different hostname than // AppURL. It is never set for github.com or a GithubMirror, so the token stays on-instance. DefaultActionInstanceIsSelfHosted bool - PlatformPicker func(labels []string) string // platform picker, it will take precedence over Platforms if isn't nil - JobLoggerLevel *log.Level // the level of job logger - ValidVolumes []string // only volumes (and bind mounts) in this slice can be mounted on the job container or service containers - SharedToolCache bool // one tool cache for all jobs instead of one per job - InsecureSkipTLS bool // whether to skip verifying TLS certificate of the Gitea instance - MaxParallel int // max parallel jobs to run across all workflows (0 = no limit, uses CPU count) - AllocatePTY bool // allocate a pseudo-TTY for each step's process - ServiceReadyTimeout time.Duration // how long a job waits for its service containers to report healthy (0 uses the default) - RunnerName string // name this runner registered with, reported as `runner.name`, defaults to the hostname - JobStartedHook string // script run inside the job environment before the job's first step; ACTIONS_RUNNER_HOOK_JOB_STARTED is read from Env when empty - JobCompletedHook string // script run inside the job environment after the job's last step; ACTIONS_RUNNER_HOOK_JOB_COMPLETED is read from Env when empty + PlatformPicker func(labels []string) string + JobLoggerLevel *log.Level // the level of job logger + ValidVolumes []string // only volumes (and bind mounts) in this slice can be mounted on the job container or service containers + SharedToolCache bool // one tool cache for all jobs instead of one per job + InsecureSkipTLS bool // whether to skip verifying TLS certificate of the Gitea instance + MaxParallel int // max parallel jobs to run across all workflows (0 = no limit, uses CPU count) + AllocatePTY bool // allocate a pseudo-TTY for each step's process + ServiceReadyTimeout time.Duration // how long a job waits for its service containers to report healthy (0 uses the default) + RunnerName string // name this runner registered with, reported as `runner.name`, defaults to the hostname + JobStartedHook string // script run inside the job environment before the job's first step; ACTIONS_RUNNER_HOOK_JOB_STARTED is read from Env when empty + JobCompletedHook string // script run inside the job environment after the job's last step; ACTIONS_RUNNER_HOOK_JOB_COMPLETED is read from Env when empty } // RunnerDebug reports whether debug logging is on, exposed as `runner.debug` and @@ -141,8 +123,10 @@ type runnerImpl struct { caller *caller // the job calling this runner (caller of a reusable workflow) } +type Runner = runnerImpl + // New Creates a new Runner -func New(runnerConfig *Config) (Runner, error) { +func New(runnerConfig *Config) (*Runner, error) { runner := &runnerImpl{ config: runnerConfig, } @@ -150,7 +134,7 @@ func New(runnerConfig *Config) (Runner, error) { return runner.configure() } -func (runner *runnerImpl) configure() (Runner, error) { +func (runner *runnerImpl) configure() (*runnerImpl, error) { if runner.config.RunnerName == "" { // Callers that do not register, such as `exec`, still get a `runner.name`. runner.config.RunnerName, _ = os.Hostname() @@ -166,15 +150,6 @@ func (runner *runnerImpl) configure() (Runner, error) { return nil, err } runner.eventJSON = string(eventJSONBytes) - } else if len(runner.config.Inputs) != 0 { - eventMap := map[string]map[string]string{ - "inputs": runner.config.Inputs, - } - eventJSON, err := json.Marshal(eventMap) - if err != nil { - return nil, err - } - runner.eventJSON = string(eventJSON) } return runner, nil } @@ -213,7 +188,6 @@ func (runner *runnerImpl) NewPlanExecutor(plan *model.Plan) common.Executor { log.Debugf("Job.Outputs: %v", job.Outputs) log.Debugf("Job.Uses: %v", job.Uses) log.Debugf("Job.With: %v", job.With) - // log.Debugf("Job.RawSecrets: %v", job.RawSecrets) log.Debugf("Job.Result: %v", job.Result) if job.Strategy != nil { @@ -231,15 +205,11 @@ func (runner *runnerImpl) NewPlanExecutor(plan *model.Plan) common.Executor { } } - var matrixes []map[string]any - if m, err := job.GetMatrixes(); err != nil { + matrixes, err := job.GetMatrixes() + if err != nil { log.Errorf("Error while get job's matrix: %v", err) - } else { - log.Debugf("Job Matrices: %v", m) - log.Debugf("Runner Matrices: %v", runner.config.Matrix) - matrixes = selectMatrixes(m, runner.config.Matrix) } - log.Debugf("Final matrix after applying user inclusions '%v'", matrixes) + log.Debugf("Job Matrices: %v", matrixes) maxParallel := 4 if job.Strategy != nil { @@ -344,25 +314,6 @@ func handleFailure(plan *model.Plan) common.Executor { } } -func selectMatrixes(originalMatrixes []map[string]any, targetMatrixValues map[string]map[string]bool) []map[string]any { - matrixes := make([]map[string]any, 0) - for _, original := range originalMatrixes { - flag := true - for key, val := range original { - if allowedVals, ok := targetMatrixValues[key]; ok { - valToString := fmt.Sprintf("%v", val) - if _, ok := allowedVals[valToString]; !ok { - flag = false - } - } - } - if flag { - matrixes = append(matrixes, original) - } - } - return matrixes -} - func (runner *runnerImpl) newRunContext(ctx context.Context, run *model.Run, matrix map[string]any) *RunContext { rc := &RunContext{ Config: runner.config, diff --git a/act/runner/runner_test.go b/act/runner/runner_test.go index 19434a59..79e5c280 100644 --- a/act/runner/runner_test.go +++ b/act/runner/runner_test.go @@ -10,7 +10,6 @@ import ( "fmt" "io" "os" - "path" "path/filepath" "runtime" "slices" @@ -24,7 +23,6 @@ import ( "github.com/joho/godotenv" log "github.com/sirupsen/logrus" assert "github.com/stretchr/testify/assert" - "go.yaml.in/yaml/v4" ) var ( @@ -35,6 +33,17 @@ var ( secrets map[string]string ) +func mapPlatformPicker(platforms map[string]string) func([]string) string { + return func(labels []string) string { + for _, label := range labels { + if image := platforms[strings.ToLower(label)]; image != "" { + return image + } + } + return "" + } +} + func init() { if p := os.Getenv("ACT_TEST_IMAGE"); p != "" { baseImage = p @@ -189,29 +198,22 @@ func (j *TestJobFileInfo) runTest(ctx context.Context, t *testing.T, cfg *Config fullWorkflowPath := filepath.Join(workdir, j.workflowPath) runnerConfig := &Config{ - Workdir: workdir, - BindWorkdir: false, - EventName: j.eventName, - EventPath: cfg.EventPath, - Platforms: j.platforms, + Workdir: workdir, + BindWorkdir: false, + EventName: j.eventName, + EventPath: cfg.EventPath, + PlatformPicker: mapPlatformPicker(j.platforms), // fixtures reuse workflow and job names, so parallel tests would collide without this ContainerNamePrefix: strings.ReplaceAll(t.Name(), "/", "-"), - ReuseContainers: false, - // as the shipped runner does, else a fixture asserting a job failure keeps its - // container, and its network, on the daemon forever - AutoRemove: true, // 0 would run jobs runtime.NumCPU()-wide, making the network peak machine-dependent MaxParallel: 2, ForceRebuild: true, Env: cfg.Env, Secrets: cfg.Secrets, - Inputs: cfg.Inputs, GitHubInstance: "github.com", DefaultActionInstance: cfg.DefaultActionInstance, ContainerArchitecture: cfg.ContainerArchitecture, ContainerMaxLifetime: time.Hour, - Matrix: cfg.Matrix, - ActionCache: cfg.ActionCache, ValidVolumes: []string{"**"}, // allow workflow-declared volumes (e.g. container-volumes) } @@ -239,10 +241,6 @@ func (j *TestJobFileInfo) runTest(ctx context.Context, t *testing.T, cfg *Config fmt.Println("::endgroup::") //nolint:forbidigo // pre-existing issue from nektos/act } -type TestConfig struct { - LocalRepositories map[string]string `yaml:"local-repositories"` -} - func TestRunEvent(t *testing.T) { requireDocker(t) t.Parallel() @@ -323,9 +321,6 @@ func TestRunEvent(t *testing.T) { {workdir, "services", "push", "", platforms, secrets}, {workdir, "services-with-container", "push", "", platforms, secrets}, {workdir, "services-empty-image", "push", "", platforms, secrets}, - - // local remote action overrides - {workdir, "local-remote-action-overrides", "push", "", platforms, secrets}, } for _, table := range tables { @@ -347,22 +342,6 @@ func TestRunEvent(t *testing.T) { config.EventPath = eventFile } - testConfigFile := filepath.Join(workdir, table.workflowPath, "config.yml") - if file, err := os.ReadFile(testConfigFile); err == nil { - testConfig := &TestConfig{} - if yaml.Unmarshal(file, testConfig) == nil { - if testConfig.LocalRepositories != nil { - config.ActionCache = &LocalRepositoryCache{ - Parent: GoGitActionCache{ - path.Clean(path.Join(workdir, "cache")), - }, - LocalRepositories: testConfig.LocalRepositories, - CacheDirCache: map[string]string{}, - } - } - } - } - table.runTest(ctx, t, config) }) } @@ -584,8 +563,7 @@ func TestRunWithService(t *testing.T) { runnerConfig := &Config{ Workdir: workdir, EventName: eventName, - Platforms: platforms, - ReuseContainers: false, + PlatformPicker: mapPlatformPicker(platforms), ContainerMaxLifetime: time.Hour, // otherwise the job container is `sleep 0` and exits at once } runner, err := New(runnerConfig) @@ -601,26 +579,6 @@ func TestRunWithService(t *testing.T) { assert.NoError(t, err, workflowPath) } -func TestRunActionInputs(t *testing.T) { - t.Parallel() - requireDocker(t) - workflowPath := "input-from-cli" - - tjfi := TestJobFileInfo{ - workdir: workdir, - workflowPath: workflowPath, - eventName: "workflow_dispatch", - errorMessage: "", - platforms: platforms, - } - - inputs := map[string]string{ - "SOME_INPUT": "input", - } - - tjfi.runTest(context.Background(), t, &Config{Inputs: inputs}) -} - func TestRunEventPullRequest(t *testing.T) { t.Parallel() requireDocker(t) @@ -637,29 +595,3 @@ func TestRunEventPullRequest(t *testing.T) { tjfi.runTest(context.Background(), t, &Config{EventPath: filepath.Join(workdir, workflowPath, "event.json")}) } - -func TestRunMatrixWithUserDefinedInclusions(t *testing.T) { - t.Parallel() - requireDocker(t) - workflowPath := "matrix-with-user-inclusions" - - tjfi := TestJobFileInfo{ - workdir: workdir, - workflowPath: workflowPath, - eventName: "push", - errorMessage: "", - platforms: platforms, - } - - matrix := map[string]map[string]bool{ - "node": { - "8": true, - "8.x": true, - }, - "os": { - "ubuntu-18.04": true, - }, - } - - tjfi.runTest(context.Background(), t, &Config{Matrix: matrix}) -} diff --git a/act/runner/step.go b/act/runner/step.go index 619f878a..3f8b514e 100644 --- a/act/runner/step.go +++ b/act/runner/step.go @@ -85,10 +85,7 @@ func runStepExecutor(step step, stage stepStage, executor common.Executor) commo rc.StepResults[rc.CurrentStep] = stepResult } - err := setupEnv(ctx, step) - if err != nil { - return err - } + setupEnv(ctx, step) runStep, err := isStepEnabled(ctx, ifExpression, step, stage) if err != nil { @@ -232,7 +229,7 @@ func runStepExecutor(step step, stage stepStage, executor common.Executor) commo } } -func evaluateStepTimeout(ctx context.Context, exprEval ExpressionEvaluator, stepModel *model.Step) (context.Context, context.CancelFunc) { +func evaluateStepTimeout(ctx context.Context, exprEval *expressionEvaluator, stepModel *model.Step) (context.Context, context.CancelFunc) { timeout := exprEval.Interpolate(ctx, stepModel.TimeoutMinutes) if timeout != "" { if timeOutMinutes, err := strconv.ParseInt(timeout, 10, 64); err == nil { @@ -242,7 +239,7 @@ func evaluateStepTimeout(ctx context.Context, exprEval ExpressionEvaluator, step return ctx, func() {} } -func setupEnv(ctx context.Context, step step) error { //nolint:unparam // pre-existing issue from nektos/act +func setupEnv(ctx context.Context, step step) { rc := step.getRunContext() mergeEnv(ctx, step) @@ -263,11 +260,6 @@ func setupEnv(ctx context.Context, step step) error { //nolint:unparam // pre-ex (*step.getEnv())[k] = exprEval.Interpolate(ctx, v) } } - - // For Gitea, reduce log noise - // common.Logger(ctx).Debugf("setupEnv => %v", *step.getEnv()) - - return nil } func mergeEnv(ctx context.Context, step step) { diff --git a/act/runner/step_action_local.go b/act/runner/step_action_local.go index 77e08b34..49c7dfec 100644 --- a/act/runner/step_action_local.go +++ b/act/runner/step_action_local.go @@ -33,10 +33,7 @@ type stepActionLocal struct { func (sal *stepActionLocal) pre() common.Executor { sal.env = map[string]string{} - - return func(ctx context.Context) error { - return nil - } + return common.NewPipelineExecutor() } func (sal *stepActionLocal) main() common.Executor { @@ -50,39 +47,37 @@ 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) - localReader := func(ctx context.Context) actionYamlReader { - _, cpath := getContainerActionPaths(sal.Step, path.Join(actionDir, ""), sal.RunContext) - return func(filename string) (io.Reader, io.Closer, error) { - spath := path.Join(cpath, filename) - for range maxSymlinkDepth { - tars, err := sal.RunContext.JobContainer.GetContainerArchive(ctx, spath) - if errors.Is(err, fs.ErrNotExist) { - return nil, nil, err - } else if err != nil { - return nil, nil, fs.ErrNotExist - } - treader := tar.NewReader(tars) - header, err := treader.Next() - if errors.Is(err, io.EOF) { - return nil, nil, os.ErrNotExist - } else if err != nil { - return nil, nil, err - } - if header.FileInfo().Mode()&os.ModeSymlink == os.ModeSymlink { - spath, err = symlinkJoin(spath, header.Linkname, cpath) - if err != nil { - return nil, nil, err - } - } else { - return treader, tars, nil - } + localReader := func(filename string) (io.Reader, io.Closer, error) { + spath := path.Join(containerActionPath, filename) + for range maxSymlinkDepth { + tars, err := sal.RunContext.JobContainer.GetContainerArchive(ctx, spath) + if errors.Is(err, fs.ErrNotExist) { + return nil, nil, err + } else if err != nil { + return nil, nil, fs.ErrNotExist + } + treader := tar.NewReader(tars) + header, err := treader.Next() + if errors.Is(err, io.EOF) { + return nil, nil, os.ErrNotExist + } else if err != nil { + return nil, nil, err + } + if header.FileInfo().Mode()&os.ModeSymlink == os.ModeSymlink { + spath, err = symlinkJoin(spath, header.Linkname, containerActionPath) + if err != nil { + return nil, nil, err + } + } else { + return treader, tars, nil } - return nil, nil, fmt.Errorf("max depth %d of symlinks exceeded while reading %s", maxSymlinkDepth, spath) } + return nil, nil, fmt.Errorf("max depth %d of symlinks exceeded while reading %s", maxSymlinkDepth, spath) } - actionModel, err := sal.readAction(ctx, sal.Step, actionDir, "", localReader(ctx), os.WriteFile) + actionModel, err := sal.readAction(ctx, sal.Step, actionDir, "", localReader, os.WriteFile) if err != nil { return err } diff --git a/act/runner/step_action_local_test.go b/act/runner/step_action_local_test.go index c01d4868..1780d3d1 100644 --- a/act/runner/step_action_local_test.go +++ b/act/runner/step_action_local_test.go @@ -74,27 +74,17 @@ func TestStepActionLocalTest(t *testing.T) { salm.On("readAction", sal.Step, filepath.Clean("/tmp/path/to/action"), "", mock.Anything, mock.Anything). Return(&model.Action{}, nil) - cm.On("Copy", "/var/run/act", mock.AnythingOfType("[]*container.FileEntry")).Return(func(ctx context.Context) error { - return nil - }) + cm.On("Copy", "/var/run/act", mock.AnythingOfType("[]*container.FileEntry")).Return(noopExecutor) - cm.On("UpdateFromEnv", "/var/run/act/workflow/envs.txt", mock.AnythingOfType("*map[string]string")).Return(func(ctx context.Context) error { - return nil - }) + cm.On("UpdateFromEnv", "/var/run/act/workflow/envs.txt", mock.AnythingOfType("*map[string]string")).Return(noopExecutor) - cm.On("UpdateFromEnv", "/var/run/act/workflow/statecmd.txt", mock.AnythingOfType("*map[string]string")).Return(func(ctx context.Context) error { - return nil - }) + cm.On("UpdateFromEnv", "/var/run/act/workflow/statecmd.txt", mock.AnythingOfType("*map[string]string")).Return(noopExecutor) - cm.On("UpdateFromEnv", "/var/run/act/workflow/outputcmd.txt", mock.AnythingOfType("*map[string]string")).Return(func(ctx context.Context) error { - return nil - }) + cm.On("UpdateFromEnv", "/var/run/act/workflow/outputcmd.txt", mock.AnythingOfType("*map[string]string")).Return(noopExecutor) cm.On("GetContainerArchive", ctx, "/var/run/act/workflow/pathcmd.txt").Return(io.NopCloser(&bytes.Buffer{}), nil) - salm.On("runAction", sal, filepath.Clean("/tmp/path/to/action"), (*remoteAction)(nil)).Return(func(ctx context.Context) error { - return nil - }) + salm.On("runAction", sal, filepath.Clean("/tmp/path/to/action"), (*remoteAction)(nil)).Return(noopExecutor) err := sal.pre()(ctx) assert.NoError(t, err) //nolint:testifylint // pre-existing issue from nektos/act @@ -269,21 +259,13 @@ func TestStepActionLocalPost(t *testing.T) { } cm.On("Exec", suffixMatcher("runner/local/action/post.js"), sal.env, "", "").Return(func(ctx context.Context) error { return tt.err }) - cm.On("Copy", "/var/run/act", mock.AnythingOfType("[]*container.FileEntry")).Return(func(ctx context.Context) error { - return nil - }) + cm.On("Copy", "/var/run/act", mock.AnythingOfType("[]*container.FileEntry")).Return(noopExecutor) - cm.On("UpdateFromEnv", "/var/run/act/workflow/envs.txt", mock.AnythingOfType("*map[string]string")).Return(func(ctx context.Context) error { - return nil - }) + cm.On("UpdateFromEnv", "/var/run/act/workflow/envs.txt", mock.AnythingOfType("*map[string]string")).Return(noopExecutor) - cm.On("UpdateFromEnv", "/var/run/act/workflow/statecmd.txt", mock.AnythingOfType("*map[string]string")).Return(func(ctx context.Context) error { - return nil - }) + cm.On("UpdateFromEnv", "/var/run/act/workflow/statecmd.txt", mock.AnythingOfType("*map[string]string")).Return(noopExecutor) - cm.On("UpdateFromEnv", "/var/run/act/workflow/outputcmd.txt", mock.AnythingOfType("*map[string]string")).Return(func(ctx context.Context) error { - return nil - }) + cm.On("UpdateFromEnv", "/var/run/act/workflow/outputcmd.txt", mock.AnythingOfType("*map[string]string")).Return(noopExecutor) cm.On("GetContainerArchive", ctx, "/var/run/act/workflow/pathcmd.txt").Return(io.NopCloser(&bytes.Buffer{}), nil) } diff --git a/act/runner/step_action_remote.go b/act/runner/step_action_remote.go index 18438d03..3861d1c9 100644 --- a/act/runner/step_action_remote.go +++ b/act/runner/step_action_remote.go @@ -5,7 +5,6 @@ package runner import ( - "archive/tar" "context" "errors" "fmt" @@ -33,7 +32,6 @@ type stepActionRemote struct { action *model.Action env map[string]string remoteAction *remoteAction - cacheDir string resolvedSha string } @@ -69,58 +67,6 @@ func (sar *stepActionRemote) prepareActionExecutor() common.Executor { common.Logger(ctx).Debugf("Skipping local actions/checkout because workdir was already copied") return nil } - - for _, action := range sar.RunContext.Config.ReplaceGheActionWithGithubCom { - if strings.EqualFold(fmt.Sprintf("%s/%s", sar.remoteAction.Org, sar.remoteAction.Repo), action) { - sar.remoteAction.URL = "https://github.com" - github.Token = sar.RunContext.Config.ReplaceGheActionTokenWithGithubCom - } - } - // Actions served from the action cache are read out of a git object store rather than a - // directory, so they never reach the bundle patch below and keep to the v1 cache API. - if sar.RunContext.Config.ActionCache != nil { - cache := sar.RunContext.Config.ActionCache - - var err error - sar.cacheDir = fmt.Sprintf("%s/%s", sar.remoteAction.Org, sar.remoteAction.Repo) - repoURL := sar.remoteAction.URL + "/" + sar.cacheDir - repoRef := sar.remoteAction.Ref - sar.resolvedSha, err = cache.Fetch(ctx, sar.cacheDir, repoURL, repoRef, github.Token) - if err != nil { - return fmt.Errorf("failed to fetch \"%s\" version \"%s\": %w", repoURL, repoRef, err) - } - - remoteReader := func(ctx context.Context) actionYamlReader { - return func(filename string) (io.Reader, io.Closer, error) { - spath := path.Join(sar.remoteAction.Path, filename) - for range maxSymlinkDepth { - tars, err := cache.GetTarArchive(ctx, sar.cacheDir, sar.resolvedSha, spath) - if err != nil { - return nil, nil, os.ErrNotExist - } - treader := tar.NewReader(tars) - header, err := treader.Next() - if err != nil { - return nil, nil, os.ErrNotExist - } - if header.FileInfo().Mode()&os.ModeSymlink == os.ModeSymlink { - spath, err = symlinkJoin(spath, header.Linkname, ".") - if err != nil { - return nil, nil, err - } - } else { - return treader, tars, nil - } - } - return nil, nil, fmt.Errorf("max depth %d of symlinks exceeded while reading %s", maxSymlinkDepth, spath) - } - } - - actionModel, err := sar.readAction(ctx, sar.Step, sar.resolvedSha, sar.remoteAction.Path, remoteReader(ctx), os.WriteFile) - sar.action = actionModel - return err - } - actionDir := sar.actionDir() defaultActionURL := sar.RunContext.Config.DefaultActionURL() // For Gitea @@ -165,18 +111,16 @@ func (sar *stepActionRemote) prepareActionExecutor() common.Executor { sar.resolvedSha = sha } - remoteReader := func(ctx context.Context) actionYamlReader { //nolint:unparam // pre-existing issue from nektos/act - return func(filename string) (io.Reader, io.Closer, error) { - f, err := os.Open(filepath.Join(actionDir, sar.remoteAction.Path, filename)) - return f, f, err - } + remoteReader := func(filename string) (io.Reader, io.Closer, error) { + f, err := os.Open(filepath.Join(actionDir, sar.remoteAction.Path, filename)) + return f, f, err } return common.NewPipelineExecutor( ntErr, func(ctx context.Context) error { defer git.AcquireCloneLock(actionDir)() - actionModel, err := sar.readAction(ctx, sar.Step, actionDir, sar.remoteAction.Path, remoteReader(ctx), os.WriteFile) + actionModel, err := sar.readAction(ctx, sar.Step, actionDir, sar.remoteAction.Path, remoteReader, os.WriteFile) sar.action = actionModel return err }, diff --git a/act/runner/step_action_remote_test.go b/act/runner/step_action_remote_test.go index da5ba364..7de65387 100644 --- a/act/runner/step_action_remote_test.go +++ b/act/runner/step_action_remote_test.go @@ -31,6 +31,52 @@ 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 TestShortSHAActionRejected(t *testing.T) { + actionRoot := t.TempDir() + repo := filepath.Join(actionRoot, "actions", "hello-world-docker-action") + require.NoError(t, os.MkdirAll(repo, 0o755)) + gitMust(t, "", "init", "--initial-branch=main", repo) + gitMust(t, repo, "config", "user.email", "test@test") + gitMust(t, repo, "config", "user.name", "test") + require.NoError(t, os.WriteFile(filepath.Join(repo, "action.yml"), + []byte("name: hello\nruns:\n using: node24\n main: index.js\n"), 0o644)) + gitMust(t, repo, "add", ".") + gitMust(t, repo, "commit", "-m", "initial") + output, err := exec.Command("git", "-C", repo, "rev-parse", "--short=7", "HEAD").Output() + require.NoError(t, err) + + workflowDir := t.TempDir() + workflow := fmt.Sprintf("on: push\njobs:\n test:\n runs-on: ubuntu-latest\n steps:\n - uses: actions/hello-world-docker-action@%s\n", strings.TrimSpace(string(output))) + require.NoError(t, os.WriteFile(filepath.Join(workflowDir, "push.yml"), []byte(workflow), 0o644)) + + runner, err := New(&Config{ + Workdir: workflowDir, + EventName: "push", + GitHubInstance: "github.com", + DefaultActionInstance: actionRoot, + ContainerMaxLifetime: time.Hour, + PlatformPicker: func([]string) string { return baseImage }, + }) + require.NoError(t, err) + planner, err := model.NewWorkflowPlanner(workflowDir, true) + require.NoError(t, err) + plan, err := planner.PlanEvent("push") + require.NoError(t, err) + + err = runner.NewPlanExecutor(plan)(common.WithDryrun(t.Context(), true)) + require.ErrorContains(t, err, "shortened version of a commit SHA") +} + func (sarm *stepActionRemoteMocks) readAction(_ context.Context, step *model.Step, actionDir, actionPath string, readFile actionYamlReader, writeFile fileWriter) (*model.Action, error) { args := sarm.Called(step, actionDir, actionPath, readFile, writeFile) return args.Get(0).(*model.Action), args.Error(1) @@ -136,16 +182,12 @@ func TestStepActionRemote(t *testing.T) { clonedAction := false - origStepAtionRemoteNewCloneExecutor := stepActionRemoteNewCloneExecutor - stepActionRemoteNewCloneExecutor = func(input git.NewGitCloneExecutorInput) common.Executor { + setCloneExecutor(t, func(input git.NewGitCloneExecutorInput) common.Executor { return func(ctx context.Context) error { clonedAction = true return nil } - } - defer (func() { - stepActionRemoteNewCloneExecutor = origStepAtionRemoteNewCloneExecutor - })() + }) sar := &stepActionRemote{ RunContext: &RunContext{ @@ -170,33 +212,19 @@ func TestStepActionRemote(t *testing.T) { } sar.RunContext.ExprEval = sar.RunContext.NewExpressionEvaluator(ctx) - suffixMatcher := func(suffix string) any { - return mock.MatchedBy(func(actionDir string) bool { - return strings.HasSuffix(actionDir, suffix) - }) - } - if tt.mocks.read { - sarm.On("readAction", sar.Step, suffixMatcher(sar.Step.UsesHash()), "", mock.Anything, mock.Anything).Return(&model.Action{}, nil) + sarm.On("readAction", sar.Step, actionDirSuffix(sar.Step.UsesHash()), "", mock.Anything, mock.Anything).Return(&model.Action{}, nil) } if tt.mocks.run { - sarm.On("runAction", sar, suffixMatcher(sar.Step.UsesHash()), newRemoteAction(sar.Step.Uses)).Return(func(ctx context.Context) error { return tt.runError }) + sarm.On("runAction", sar, actionDirSuffix(sar.Step.UsesHash()), newRemoteAction(sar.Step.Uses)).Return(func(ctx context.Context) error { return tt.runError }) - cm.On("Copy", "/var/run/act", mock.AnythingOfType("[]*container.FileEntry")).Return(func(ctx context.Context) error { - return nil - }) + cm.On("Copy", "/var/run/act", mock.AnythingOfType("[]*container.FileEntry")).Return(noopExecutor) - cm.On("UpdateFromEnv", "/var/run/act/workflow/envs.txt", mock.AnythingOfType("*map[string]string")).Return(func(ctx context.Context) error { - return nil - }) + cm.On("UpdateFromEnv", "/var/run/act/workflow/envs.txt", mock.AnythingOfType("*map[string]string")).Return(noopExecutor) - cm.On("UpdateFromEnv", "/var/run/act/workflow/statecmd.txt", mock.AnythingOfType("*map[string]string")).Return(func(ctx context.Context) error { - return nil - }) + cm.On("UpdateFromEnv", "/var/run/act/workflow/statecmd.txt", mock.AnythingOfType("*map[string]string")).Return(noopExecutor) - cm.On("UpdateFromEnv", "/var/run/act/workflow/outputcmd.txt", mock.AnythingOfType("*map[string]string")).Return(func(ctx context.Context) error { - return nil - }) + cm.On("UpdateFromEnv", "/var/run/act/workflow/outputcmd.txt", mock.AnythingOfType("*map[string]string")).Return(noopExecutor) cm.On("GetContainerArchive", ctx, "/var/run/act/workflow/pathcmd.txt").Return(io.NopCloser(&bytes.Buffer{}), nil) } @@ -216,279 +244,43 @@ func TestStepActionRemote(t *testing.T) { } } -func TestStepActionRemotePre(t *testing.T) { - table := []struct { - name string - stepModel *model.Step +func TestStepActionRemotePrepare(t *testing.T) { + for _, test := range []struct { + name, uses, instance, actionPath, wantURL string }{ - { - name: "run-pre", - stepModel: &model.Step{ - Uses: "org/repo/path@ref", - }, - }, - } - - for _, tt := range table { - t.Run(tt.name, func(t *testing.T) { - ctx := context.Background() - - clonedAction := false - sarm := &stepActionRemoteMocks{} - - origStepAtionRemoteNewCloneExecutor := stepActionRemoteNewCloneExecutor - stepActionRemoteNewCloneExecutor = func(input git.NewGitCloneExecutorInput) common.Executor { - return func(ctx context.Context) error { - clonedAction = true - return nil - } - } - defer (func() { - stepActionRemoteNewCloneExecutor = origStepAtionRemoteNewCloneExecutor - })() - - sar := &stepActionRemote{ - Step: tt.stepModel, - RunContext: &RunContext{ - Config: &Config{ - GitHubInstance: "https://github.com", - ActionCacheDir: "/tmp/test-cache", - }, - Run: &model.Run{ - JobID: "1", - Workflow: &model.Workflow{ - Jobs: map[string]*model.Job{ - "1": {}, - }, - }, - }, - }, - readAction: sarm.readAction, - } - - suffixMatcher := func(suffix string) any { - return mock.MatchedBy(func(actionDir string) bool { - return strings.HasSuffix(actionDir, suffix) - }) - } - - sarm.On("readAction", sar.Step, suffixMatcher(sar.Step.UsesHash()), "path", mock.Anything, mock.Anything).Return(&model.Action{}, nil) - - err := sar.pre()(ctx) - - assert.NoError(t, err) //nolint:testifylint // pre-existing issue from nektos/act - assert.True(t, clonedAction) - - sarm.AssertExpectations(t) - }) - } -} - -func TestStepActionRemotePreThroughAction(t *testing.T) { - table := []struct { - name string - stepModel *model.Step - }{ - { - name: "run-pre", - stepModel: &model.Step{ - Uses: "org/repo/path@ref", - }, - }, - } - - for _, tt := range table { - t.Run(tt.name, func(t *testing.T) { - ctx := context.Background() - - clonedAction := false - sarm := &stepActionRemoteMocks{} - - origStepAtionRemoteNewCloneExecutor := stepActionRemoteNewCloneExecutor - stepActionRemoteNewCloneExecutor = func(input git.NewGitCloneExecutorInput) common.Executor { - return func(ctx context.Context) error { - if input.URL == "https://github.com/org/repo" { - clonedAction = true - } - return nil - } - } - defer (func() { - stepActionRemoteNewCloneExecutor = origStepAtionRemoteNewCloneExecutor - })() - - sar := &stepActionRemote{ - Step: tt.stepModel, - RunContext: &RunContext{ - Config: &Config{ - GitHubInstance: "https://enterprise.github.com", - ReplaceGheActionWithGithubCom: []string{"org/repo"}, - ActionCacheDir: "/tmp/test-cache", - }, - Run: &model.Run{ - JobID: "1", - Workflow: &model.Workflow{ - Jobs: map[string]*model.Job{ - "1": {}, - }, - }, - }, - }, - readAction: sarm.readAction, - } - - suffixMatcher := func(suffix string) any { - return mock.MatchedBy(func(actionDir string) bool { - return strings.HasSuffix(actionDir, suffix) - }) - } - - sarm.On("readAction", sar.Step, suffixMatcher(sar.Step.UsesHash()), "path", mock.Anything, mock.Anything).Return(&model.Action{}, nil) - - err := sar.pre()(ctx) - - assert.NoError(t, err) //nolint:testifylint // pre-existing issue from nektos/act - assert.True(t, clonedAction) - - sarm.AssertExpectations(t) - }) - } -} - -func TestStepActionRemotePreThroughActionToken(t *testing.T) { - table := []struct { - name string - stepModel *model.Step - }{ - { - name: "run-pre", - stepModel: &model.Step{ - Uses: "org/repo/path@ref", - }, - }, - } - - for _, tt := range table { - t.Run(tt.name, func(t *testing.T) { - ctx := context.Background() - + {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 - var actualToken string - sarm := &stepActionRemoteMocks{} - - origStepAtionRemoteNewCloneExecutor := stepActionRemoteNewCloneExecutor - stepActionRemoteNewCloneExecutor = func(input git.NewGitCloneExecutorInput) common.Executor { - return func(ctx context.Context) error { + setCloneExecutor(t, func(input git.NewGitCloneExecutorInput) common.Executor { + return func(context.Context) error { actualURL = input.URL - actualToken = input.Token return nil } - } - defer (func() { - stepActionRemoteNewCloneExecutor = origStepAtionRemoteNewCloneExecutor - })() + }) - // Use unique cache directory to ensure action gets cloned, not served from cache - uniqueCacheDir := fmt.Sprintf("/tmp/test-cache-token-%d", time.Now().UnixNano()) - - sar := &stepActionRemote{ - Step: tt.stepModel, + actionMocks := &stepActionRemoteMocks{} + action := &stepActionRemote{ + Step: &model.Step{Uses: test.uses}, RunContext: &RunContext{ - Config: &Config{ - GitHubInstance: "https://enterprise.github.com", - ReplaceGheActionWithGithubCom: []string{"org/repo"}, - ReplaceGheActionTokenWithGithubCom: "PRIVATE_ACTIONS_TOKEN_ON_GITHUB", - ActionCacheDir: uniqueCacheDir, - Token: "PRIVATE_ACTIONS_TOKEN_ON_GITHUB", - }, - Run: &model.Run{ - JobID: "1", - Workflow: &model.Workflow{ - Jobs: map[string]*model.Job{ - "1": {}, - }, - }, - }, + Config: &Config{GitHubInstance: test.instance, ActionCacheDir: t.TempDir()}, + Run: &model.Run{JobID: "1", Workflow: &model.Workflow{ + Jobs: map[string]*model.Job{"1": {}}, + }}, }, - readAction: sarm.readAction, + readAction: actionMocks.readAction, } + actionMocks.On("readAction", action.Step, actionDirSuffix(action.Step.UsesHash()), test.actionPath, + mock.Anything, mock.Anything).Return(&model.Action{}, nil) - suffixMatcher := func(suffix string) any { - return mock.MatchedBy(func(actionDir string) bool { - return strings.HasSuffix(actionDir, suffix) - }) - } - - sarm.On("readAction", sar.Step, suffixMatcher(sar.Step.UsesHash()), "path", mock.Anything, mock.Anything).Return(&model.Action{}, nil) - - err := sar.pre()(ctx) - - assert.NoError(t, err) //nolint:testifylint // pre-existing issue from nektos/act - // Verify that the clone was called (URL should be redirected to github.com) - assert.True(t, actualURL != "", "Expected clone to be called") //nolint:testifylint // pre-existing issue from nektos/act - assert.Equal(t, "https://github.com/org/repo", actualURL, "URL should be redirected to github.com") - // Note: Token might be empty because getGitCloneToken doesn't check ReplaceGheActionTokenWithGithubCom - // The important part is that the URL replacement works - if actualToken != "" { - assert.Equal(t, "PRIVATE_ACTIONS_TOKEN_ON_GITHUB", actualToken, "If token is set, it should be the replacement token") - } - - sarm.AssertExpectations(t) + require.NoError(t, action.prepareActionExecutor()(t.Context())) + assert.Equal(t, test.wantURL, actualURL) + actionMocks.AssertExpectations(t) }) } } -func TestStepActionRemoteUsesGitHubInstanceWhenDefaultActionInstanceEmpty(t *testing.T) { - ctx := context.Background() - - var actualURL string - sarm := &stepActionRemoteMocks{} - - origStepAtionRemoteNewCloneExecutor := stepActionRemoteNewCloneExecutor - stepActionRemoteNewCloneExecutor = func(input git.NewGitCloneExecutorInput) common.Executor { - return func(ctx context.Context) error { - actualURL = input.URL - return nil - } - } - defer func() { - stepActionRemoteNewCloneExecutor = origStepAtionRemoteNewCloneExecutor - }() - - sar := &stepActionRemote{ - Step: &model.Step{ - Uses: "actions/setup-go@v4", - }, - RunContext: &RunContext{ - Config: &Config{ - GitHubInstance: "gitea.example", - DefaultActionInstance: "", - ActionCacheDir: t.TempDir(), - }, - Run: &model.Run{ - JobID: "1", - Workflow: &model.Workflow{ - Jobs: map[string]*model.Job{ - "1": {}, - }, - }, - }, - }, - readAction: sarm.readAction, - } - - suffixMatcher := func(suffix string) any { - return mock.MatchedBy(func(actionDir string) bool { - return strings.HasSuffix(actionDir, suffix) - }) - } - sarm.On("readAction", sar.Step, suffixMatcher(sar.Step.UsesHash()), "", mock.Anything, mock.Anything).Return(&model.Action{}, nil) - - require.NoError(t, sar.prepareActionExecutor()(ctx)) - assert.Equal(t, "https://gitea.example/actions/setup-go", actualURL) - sarm.AssertExpectations(t) -} - func TestStepActionRemotePost(t *testing.T) { table := []struct { name string @@ -669,21 +461,13 @@ func TestStepActionRemotePost(t *testing.T) { cm.On("Exec", execMatcher, sar.env, "", "").Return(func(ctx context.Context) error { return tt.err }) - cm.On("Copy", "/var/run/act", mock.AnythingOfType("[]*container.FileEntry")).Return(func(ctx context.Context) error { - return nil - }) + cm.On("Copy", "/var/run/act", mock.AnythingOfType("[]*container.FileEntry")).Return(noopExecutor) - cm.On("UpdateFromEnv", "/var/run/act/workflow/envs.txt", mock.AnythingOfType("*map[string]string")).Return(func(ctx context.Context) error { - return nil - }) + cm.On("UpdateFromEnv", "/var/run/act/workflow/envs.txt", mock.AnythingOfType("*map[string]string")).Return(noopExecutor) - cm.On("UpdateFromEnv", "/var/run/act/workflow/statecmd.txt", mock.AnythingOfType("*map[string]string")).Return(func(ctx context.Context) error { - return nil - }) + cm.On("UpdateFromEnv", "/var/run/act/workflow/statecmd.txt", mock.AnythingOfType("*map[string]string")).Return(noopExecutor) - cm.On("UpdateFromEnv", "/var/run/act/workflow/outputcmd.txt", mock.AnythingOfType("*map[string]string")).Return(func(ctx context.Context) error { - return nil - }) + cm.On("UpdateFromEnv", "/var/run/act/workflow/outputcmd.txt", mock.AnythingOfType("*map[string]string")).Return(noopExecutor) cm.On("GetContainerArchive", ctx, "/var/run/act/workflow/pathcmd.txt").Return(io.NopCloser(&bytes.Buffer{}), nil) } @@ -1032,14 +816,10 @@ func TestStepActionRemoteCloneTokenSurvivesNilSecrets(t *testing.T) { ctx := context.Background() var capturedToken string - origStepAtionRemoteNewCloneExecutor := stepActionRemoteNewCloneExecutor - stepActionRemoteNewCloneExecutor = func(input git.NewGitCloneExecutorInput) common.Executor { + setCloneExecutor(t, func(input git.NewGitCloneExecutorInput) common.Executor { capturedToken = input.Token return func(ctx context.Context) error { return nil } - } - defer (func() { - stepActionRemoteNewCloneExecutor = origStepAtionRemoteNewCloneExecutor - })() + }) sarm := &stepActionRemoteMocks{} sar := &stepActionRemote{ @@ -1066,12 +846,7 @@ func TestStepActionRemoteCloneTokenSurvivesNilSecrets(t *testing.T) { } sar.RunContext.ExprEval = sar.RunContext.NewExpressionEvaluator(ctx) - suffixMatcher := func(suffix string) any { - return mock.MatchedBy(func(actionDir string) bool { - return strings.HasSuffix(actionDir, suffix) - }) - } - sarm.On("readAction", sar.Step, suffixMatcher(sar.Step.UsesHash()), "", mock.Anything, mock.Anything).Return(&model.Action{}, nil) + sarm.On("readAction", sar.Step, actionDirSuffix(sar.Step.UsesHash()), "", mock.Anything, mock.Anything).Return(&model.Action{}, nil) err := sar.prepareActionExecutor()(ctx) require.NoError(t, err) diff --git a/act/runner/step_docker.go b/act/runner/step_docker.go index b8efdfca..f61c44a0 100644 --- a/act/runner/step_docker.go +++ b/act/runner/step_docker.go @@ -6,7 +6,6 @@ package runner import ( "context" - "fmt" "strings" "gitea.com/gitea/runner/act/common" @@ -22,11 +21,7 @@ type stepDocker struct { env map[string]string } -func (sd *stepDocker) pre() common.Executor { - return func(ctx context.Context) error { - return nil - } -} +func (sd *stepDocker) pre() common.Executor { return common.NewPipelineExecutor() } func (sd *stepDocker) main() common.Executor { sd.env = map[string]string{} @@ -34,11 +29,7 @@ func (sd *stepDocker) main() common.Executor { return runStepExecutor(sd, stepStageMain, sd.runUsesContainer()) } -func (sd *stepDocker) post() common.Executor { - return func(ctx context.Context) error { - return nil - } -} +func (sd *stepDocker) post() common.Executor { return common.NewPipelineExecutor() } func (sd *stepDocker) getRunContext() *RunContext { return sd.RunContext @@ -77,64 +68,15 @@ func (sd *stepDocker) runUsesContainer() common.Executor { entrypoint = []string{entry} } - stepContainer := sd.newStepContainer(ctx, image, cmd, entrypoint) + stepContainer := newStepContainer(ctx, sd, image, cmd, entrypoint, "") return common.NewPipelineExecutor( stepContainer.Pull(rc.Config.ForcePull), - stepContainer.Remove().IfBool(!rc.Config.ReuseContainers), + stepContainer.Remove(), stepContainer.Create(rc.Config.ContainerCapAdd, rc.Config.ContainerCapDrop), stepContainer.Start(true), - ).Finally( - stepContainer.Remove().IfBool(!rc.Config.ReuseContainers && !rc.Config.AutoRemove), ).Finally(stepContainer.Close())(ctx) } } var ContainerNewContainer = container.NewContainer - -func (sd *stepDocker) newStepContainer(ctx context.Context, image string, cmd, entrypoint []string) container.Container { - rc := sd.RunContext - step := sd.Step - - rawLogger := common.Logger(ctx).WithField("raw_output", true) - logWriter := common.NewLineWriter(rc.commandHandler(ctx), func(s string) bool { - if rc.Config.LogOutput { - rawLogger.Infof("%s", s) - } else { - rawLogger.Debugf("%s", s) - } - return true - }) - envList := make([]string, 0) - for k, v := range sd.env { - envList = append(envList, fmt.Sprintf("%s=%s", k, v)) - } - - envList = append(envList, rc.runnerEnv(ctx)...) - - binds, mounts := rc.GetBindsAndMounts() - networkMode := "container:" + rc.jobContainerName() - if rc.IsHostEnv(ctx) { - networkMode = "default" - } - stepContainer := ContainerNewContainer(&container.NewContainerInput{ - Cmd: cmd, - Entrypoint: entrypoint, - WorkingDir: rc.JobContainer.ToContainerPath(rc.Config.Workdir), - Image: image, - Name: createContainerName(rc.jobContainerName(), "STEP-"+step.ID), - Env: envList, - Mounts: mounts, - NetworkMode: networkMode, - Binds: binds, - Stdout: logWriter, - Stderr: logWriter, - Privileged: rc.Config.Privileged, - UsernsMode: rc.Config.UsernsMode, - Platform: rc.Config.ContainerArchitecture, - AutoRemove: rc.Config.AutoRemove, - ValidVolumes: rc.validVolumes(), - AllocatePTY: rc.Config.AllocatePTY, - }) - return stepContainer -} diff --git a/act/runner/step_docker_test.go b/act/runner/step_docker_test.go index 3afe1731..18c31e1c 100644 --- a/act/runner/step_docker_test.go +++ b/act/runner/step_docker_test.go @@ -16,7 +16,6 @@ import ( "gitea.dev/actionslib/pkg/model" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" - "github.com/stretchr/testify/require" ) func TestStepDockerMain(t *testing.T) { @@ -69,41 +68,23 @@ func TestStepDockerMain(t *testing.T) { } sd.RunContext.ExprEval = sd.RunContext.NewExpressionEvaluator(ctx) - cm.On("Pull", false).Return(func(ctx context.Context) error { - return nil - }) + cm.On("Pull", false).Return(noopExecutor) - cm.On("Remove").Return(func(ctx context.Context) error { - return nil - }) + cm.On("Remove").Return(noopExecutor) - cm.On("Create", []string(nil), []string(nil)).Return(func(ctx context.Context) error { - return nil - }) + cm.On("Create", []string(nil), []string(nil)).Return(noopExecutor) - cm.On("Start", true).Return(func(ctx context.Context) error { - return nil - }) + cm.On("Start", true).Return(noopExecutor) - cm.On("Close").Return(func(ctx context.Context) error { - return nil - }) + cm.On("Close").Return(noopExecutor) - cm.On("Copy", "/var/run/act", mock.AnythingOfType("[]*container.FileEntry")).Return(func(ctx context.Context) error { - return nil - }) + cm.On("Copy", "/var/run/act", mock.AnythingOfType("[]*container.FileEntry")).Return(noopExecutor) - cm.On("UpdateFromEnv", "/var/run/act/workflow/envs.txt", mock.AnythingOfType("*map[string]string")).Return(func(ctx context.Context) error { - return nil - }) + cm.On("UpdateFromEnv", "/var/run/act/workflow/envs.txt", mock.AnythingOfType("*map[string]string")).Return(noopExecutor) - cm.On("UpdateFromEnv", "/var/run/act/workflow/statecmd.txt", mock.AnythingOfType("*map[string]string")).Return(func(ctx context.Context) error { - return nil - }) + cm.On("UpdateFromEnv", "/var/run/act/workflow/statecmd.txt", mock.AnythingOfType("*map[string]string")).Return(noopExecutor) - cm.On("UpdateFromEnv", "/var/run/act/workflow/outputcmd.txt", mock.AnythingOfType("*map[string]string")).Return(func(ctx context.Context) error { - return nil - }) + cm.On("UpdateFromEnv", "/var/run/act/workflow/outputcmd.txt", mock.AnythingOfType("*map[string]string")).Return(noopExecutor) cm.On("GetContainerArchive", ctx, "/var/run/act/workflow/pathcmd.txt").Return(io.NopCloser(&bytes.Buffer{}), nil) @@ -115,47 +96,11 @@ func TestStepDockerMain(t *testing.T) { // DOCKER_USERNAME/DOCKER_PASSWORD secrets should not be used as implicit pull credentials for docker:// action containers. assert.Empty(t, input.Username) assert.Empty(t, input.Password) + assert.True(t, input.AutoRemove) cm.AssertExpectations(t) } -// With AutoRemove the daemon reaps the container on exit, so act must not remove it afterwards. -func TestStepDockerAutoRemove(t *testing.T) { - orig := ContainerNewContainer - defer func() { ContainerNewContainer = orig }() - - for _, tc := range []struct { - autoRemove bool - removes int - }{ - {false, 2}, // stale + post-run - {true, 1}, // post-run skipped - } { - cm := &containerMock{} - ContainerNewContainer = func(*container.NewContainerInput) container.ExecutionsEnvironment { return cm } - - sd := &stepDocker{ - RunContext: &RunContext{ - Config: &Config{AutoRemove: tc.autoRemove}, - Run: &model.Run{JobID: "1", Workflow: &model.Workflow{Jobs: map[string]*model.Job{"1": {}}}}, - JobContainer: cm, - }, - Step: &model.Step{ID: "1", Uses: "docker://node:14"}, - } - - removes := 0 - cm.On("Pull", false).Return(func(context.Context) error { return nil }) - cm.On("Remove").Return(func(context.Context) error { removes++; return nil }) - cm.On("Create", []string(nil), []string(nil)).Return(func(context.Context) error { return nil }) - cm.On("Start", true).Return(func(context.Context) error { return nil }) - cm.On("Close").Return(func(context.Context) error { return nil }) - - require.NoError(t, sd.runUsesContainer()(context.Background())) - cm.AssertExpectations(t) - assert.Equal(t, tc.removes, removes) - } -} - func TestStepDockerNewStepContainerAllocatePTY(t *testing.T) { for _, tc := range []struct { name string @@ -199,23 +144,12 @@ func TestStepDockerNewStepContainerAllocatePTY(t *testing.T) { } sd.RunContext.ExprEval = sd.RunContext.NewExpressionEvaluator(ctx) - _ = sd.newStepContainer(ctx, "node:14", []string{"echo", "hi"}, nil) + _ = newStepContainer(ctx, sd, "node:14", []string{"echo", "hi"}, nil, "") assert.Equal(t, tc.allocPTY, captured.AllocatePTY) }) } } -func TestStepDockerPrePost(t *testing.T) { - ctx := context.Background() - sd := &stepDocker{} - - err := sd.pre()(ctx) - assert.NoError(t, err) //nolint:testifylint // pre-existing issue from nektos/act - - err = sd.post()(ctx) - assert.NoError(t, err) -} - func TestStepDockerNewStepContainerNetworkMode(t *testing.T) { cases := []struct { name string @@ -279,7 +213,7 @@ func TestStepDockerNewStepContainerNetworkMode(t *testing.T) { assert.Equal(t, tc.expectDefault, sd.RunContext.IsHostEnv(ctx), "IsHostEnv mismatch for platform %q", tc.platform) - _ = sd.newStepContainer(ctx, "alpine:3.20", []string{"echo", "hello"}, nil) + _ = newStepContainer(ctx, sd, "alpine:3.20", []string{"echo", "hello"}, nil, "") if tc.expectDefault { assert.Equal(t, "default", captured.NetworkMode, diff --git a/act/runner/step_run.go b/act/runner/step_run.go index 5ac470c7..a5ba14f2 100644 --- a/act/runner/step_run.go +++ b/act/runner/step_run.go @@ -33,11 +33,7 @@ type stepRun struct { shellCommand string } -func (sr *stepRun) pre() common.Executor { - return func(ctx context.Context) error { - return nil - } -} +func (sr *stepRun) pre() common.Executor { return common.NewPipelineExecutor() } func (sr *stepRun) main() common.Executor { sr.env = map[string]string{} @@ -202,11 +198,7 @@ func stepDeclaredEnvKeysInOrder(step *model.Step) []string { return keys } -func (sr *stepRun) post() common.Executor { - return func(ctx context.Context) error { - return nil - } -} +func (sr *stepRun) post() common.Executor { return common.NewPipelineExecutor() } func (sr *stepRun) getRunContext() *RunContext { return sr.RunContext @@ -306,22 +298,6 @@ func (sr *stepRun) setupShellCommand(ctx context.Context) (name, script string, return name, script, err } -type localEnv struct { - env map[string]string -} - -func (l *localEnv) Getenv(name string) string { - if runtime.GOOS == "windows" { - for k, v := range l.env { - if strings.EqualFold(name, k) { - return v - } - } - return "" - } - return l.env[name] -} - func (sr *stepRun) setupShell(ctx context.Context) { rc := sr.RunContext step := sr.Step @@ -344,10 +320,9 @@ func (sr *stepRun) setupShell(ctx context.Context) { shellWithFallback = []string{"pwsh", "powershell"} } step.Shell = shellWithFallback[0] - lenv := &localEnv{env: map[string]string{}} - maps.Copy(lenv.env, sr.env) - sr.getRunContext().ApplyExtraPath(ctx, &lenv.env) - _, err := lookpath.LookPath2(shellWithFallback[0], lenv) + env := maps.Clone(sr.env) + sr.getRunContext().ApplyExtraPath(ctx, &env) + _, err := lookpath.LookPath2(shellWithFallback[0], env) if err != nil { step.Shell = shellWithFallback[1] } diff --git a/act/runner/step_run_test.go b/act/runner/step_run_test.go index e16b2033..ba6c12cd 100644 --- a/act/runner/step_run_test.go +++ b/act/runner/step_run_test.go @@ -53,28 +53,16 @@ func TestStepRun(t *testing.T) { }, } - cm.On("Copy", "/var/run/act", []*container.FileEntry{fileEntry}).Return(func(ctx context.Context) error { - return nil - }) - cm.On("Exec", []string{"bash", "--noprofile", "--norc", "-e", "-o", "pipefail", "/var/run/act/workflow/1.sh"}, mock.AnythingOfType("map[string]string"), "", "workdir").Return(func(ctx context.Context) error { - return nil - }) + cm.On("Copy", "/var/run/act", []*container.FileEntry{fileEntry}).Return(noopExecutor) + cm.On("Exec", []string{"bash", "--noprofile", "--norc", "-e", "-o", "pipefail", "/var/run/act/workflow/1.sh"}, mock.AnythingOfType("map[string]string"), "", "workdir").Return(noopExecutor) - cm.On("Copy", "/var/run/act", mock.AnythingOfType("[]*container.FileEntry")).Return(func(ctx context.Context) error { - return nil - }) + cm.On("Copy", "/var/run/act", mock.AnythingOfType("[]*container.FileEntry")).Return(noopExecutor) - cm.On("UpdateFromEnv", "/var/run/act/workflow/envs.txt", mock.AnythingOfType("*map[string]string")).Return(func(ctx context.Context) error { - return nil - }) + cm.On("UpdateFromEnv", "/var/run/act/workflow/envs.txt", mock.AnythingOfType("*map[string]string")).Return(noopExecutor) - cm.On("UpdateFromEnv", "/var/run/act/workflow/statecmd.txt", mock.AnythingOfType("*map[string]string")).Return(func(ctx context.Context) error { - return nil - }) + cm.On("UpdateFromEnv", "/var/run/act/workflow/statecmd.txt", mock.AnythingOfType("*map[string]string")).Return(noopExecutor) - cm.On("UpdateFromEnv", "/var/run/act/workflow/outputcmd.txt", mock.AnythingOfType("*map[string]string")).Return(func(ctx context.Context) error { - return nil - }) + cm.On("UpdateFromEnv", "/var/run/act/workflow/outputcmd.txt", mock.AnythingOfType("*map[string]string")).Return(noopExecutor) ctx := context.Background() @@ -85,14 +73,3 @@ func TestStepRun(t *testing.T) { cm.AssertExpectations(t) } - -func TestStepRunPrePost(t *testing.T) { - ctx := context.Background() - sr := &stepRun{} - - err := sr.pre()(ctx) - assert.NoError(t, err) //nolint:testifylint // pre-existing issue from nektos/act - - err = sr.post()(ctx) - assert.NoError(t, err) -} diff --git a/act/runner/step_test.go b/act/runner/step_test.go index fa1fa326..e7536453 100644 --- a/act/runner/step_test.go +++ b/act/runner/step_test.go @@ -157,8 +157,7 @@ func TestSetupEnv(t *testing.T) { sm.On("getStepModel").Return(step) sm.On("getEnv").Return(&env) - err := setupEnv(context.Background(), sm) - assert.NoError(t, err) //nolint:testifylint // pre-existing issue from nektos/act + setupEnv(context.Background(), sm) // These are commit or system specific delete((env), "GITHUB_REF") @@ -213,12 +212,7 @@ func TestIsStepEnabled(t *testing.T) { return &stepRun{ RunContext: &RunContext{ - Config: &Config{ - Workdir: ".", - Platforms: map[string]string{ - "ubuntu-latest": "ubuntu-latest", - }, - }, + Config: &Config{Workdir: ".", PlatformPicker: func([]string) string { return "ubuntu-latest" }}, StepResults: map[string]*model.StepResult{}, Env: map[string]string{}, Run: &model.Run{ @@ -295,12 +289,7 @@ func TestIsContinueOnError(t *testing.T) { return &stepRun{ RunContext: &RunContext{ - Config: &Config{ - Workdir: ".", - Platforms: map[string]string{ - "ubuntu-latest": "ubuntu-latest", - }, - }, + Config: &Config{Workdir: ".", PlatformPicker: func([]string) string { return "ubuntu-latest" }}, StepResults: map[string]*model.StepResult{}, Env: map[string]string{}, Run: &model.Run{ diff --git a/act/runner/testdata/cache/nektos-test-override.git/HEAD b/act/runner/testdata/cache/nektos-test-override.git/HEAD deleted file mode 100644 index cb089cd8..00000000 --- a/act/runner/testdata/cache/nektos-test-override.git/HEAD +++ /dev/null @@ -1 +0,0 @@ -ref: refs/heads/master diff --git a/act/runner/testdata/cache/nektos-test-override.git/config b/act/runner/testdata/cache/nektos-test-override.git/config deleted file mode 100644 index cbdf39c0..00000000 --- a/act/runner/testdata/cache/nektos-test-override.git/config +++ /dev/null @@ -1,2 +0,0 @@ -[core] - bare = true diff --git a/act/runner/testdata/input-from-cli/input.yml b/act/runner/testdata/input-from-cli/input.yml deleted file mode 100644 index 42d3460f..00000000 --- a/act/runner/testdata/input-from-cli/input.yml +++ /dev/null @@ -1,21 +0,0 @@ -on: - workflow_dispatch: - inputs: - NAME: - description: "A random input name for the workflow" - type: string - required: true - SOME_VALUE: - description: "Some other input to pass" - type: string - required: true - -jobs: - test: - name: Test - runs-on: ubuntu-latest - - steps: - - name: Test with inputs - run: | - [ -z "${{ github.event.inputs.SOME_INPUT }}" ] && exit 1 || exit 0 diff --git a/act/runner/testdata/local-remote-action-overrides/config.yml b/act/runner/testdata/local-remote-action-overrides/config.yml deleted file mode 100644 index 46db82dd..00000000 --- a/act/runner/testdata/local-remote-action-overrides/config.yml +++ /dev/null @@ -1,3 +0,0 @@ -local-repositories: - https://github.com/nektos/test-override@a: testdata/actions/node24 - nektos/test-override@b: testdata/actions/node24 \ No newline at end of file diff --git a/act/runner/testdata/local-remote-action-overrides/push.yml b/act/runner/testdata/local-remote-action-overrides/push.yml deleted file mode 100644 index 76ca2dd9..00000000 --- a/act/runner/testdata/local-remote-action-overrides/push.yml +++ /dev/null @@ -1,9 +0,0 @@ -name: basic -on: push - -jobs: - build: - runs-on: ubuntu-latest - steps: - - uses: https://github.com/nektos/test-override@a - - uses: nektos/test-override@b \ No newline at end of file diff --git a/act/runner/testdata/matrix-with-user-inclusions/push.yml b/act/runner/testdata/matrix-with-user-inclusions/push.yml deleted file mode 100644 index 2fd19b4e..00000000 --- a/act/runner/testdata/matrix-with-user-inclusions/push.yml +++ /dev/null @@ -1,34 +0,0 @@ -name: matrix-with-user-inclusions -on: push - -jobs: - build: - name: PHP ${{ matrix.os }} ${{ matrix.node}} - runs-on: ubuntu-latest - steps: - - run: | - echo ${NODE_VERSION} | grep 8 - echo ${OS_VERSION} | grep ubuntu-18.04 - env: - NODE_VERSION: ${{ matrix.node }} - OS_VERSION: ${{ matrix.os }} - strategy: - matrix: - os: [ubuntu-18.04, macos-latest] - node: [4, 6, 8, 10] - exclude: - - os: macos-latest - node: 4 - include: - - os: ubuntu-16.04 - node: 10 - - test: - runs-on: ubuntu-latest - strategy: - matrix: - node: [8.x, 10.x, 12.x, 13.x] - steps: - - run: echo ${NODE_VERSION} | grep 8.x - env: - NODE_VERSION: ${{ matrix.node }} diff --git a/internal/app/cmd/exec.go b/internal/app/cmd/exec.go index db8ba18a..bf5c7340 100644 --- a/internal/app/cmd/exec.go +++ b/internal/app/cmd/exec.go @@ -446,10 +446,8 @@ func runExec(ctx context.Context, execArgs *executeArgs) func(cmd *cobra.Command config := &runner.Config{ Workdir: execArgs.Workdir(), BindWorkdir: false, - ReuseContainers: false, ForcePull: execArgs.forcePull, ForceRebuild: execArgs.forceRebuild, - LogOutput: true, JSONLogger: execArgs.jsonLogger, Env: env, ProxyEnv: proxyEnv, @@ -465,7 +463,6 @@ func runExec(ctx context.Context, execArgs *executeArgs) func(cmd *cobra.Command ContainerCapAdd: execArgs.containerCapAdd, ContainerCapDrop: execArgs.containerCapDrop, ContainerOptions: execArgs.containerOptions, - AutoRemove: true, ArtifactServerPath: execArgs.artifactServerPath, ArtifactServerPort: execArgs.artifactServerPort, ArtifactServerAddr: execArgs.artifactServerAddr, diff --git a/internal/app/run/runner.go b/internal/app/run/runner.go index 4a903e13..948eb5de 100644 --- a/internal/app/run/runner.go +++ b/internal/app/run/runner.go @@ -202,13 +202,6 @@ func (r *Runner) shouldRunIdleCleanup() bool { } } -// cleanupStaleTaskDirs reclaims stale bind-workdir per-task directories under -// workdirRoot. Retained as a thin wrapper so existing callers and tests keep a -// stable entry point. -func (r *Runner) cleanupStaleTaskDirs(ctx context.Context, workdirRoot string) { - r.cleanupStaleDirs(ctx, workdirRoot, isTaskIDDir) -} - // isTaskIDDir reports whether name is a per-task workspace dir (numeric task // ID). Any other directory is skipped to avoid deleting operator-managed data // under workdir_root. @@ -520,16 +513,13 @@ func (r *Runner) run(ctx context.Context, task *runnerv1.Task, reporter *report. ActionCloneDepth: actionCloneDepth, NoActionPatch: r.cfg.Runner.PatchActions != nil && !*r.cfg.Runner.PatchActions, - ReuseContainers: false, ForcePull: r.cfg.Container.ForcePull, ForceRebuild: r.cfg.Container.ForceRebuild, - LogOutput: true, JSONLogger: false, Env: envs, ProxyEnv: proxyEnv, Secrets: task.Secrets, GitHubInstance: strings.TrimSuffix(r.client.Address(), "/"), - AutoRemove: true, NoSkipCheckout: true, DisableActEnv: r.cfg.Runner.SetActEnv != nil && !*r.cfg.Runner.SetActEnv, PresetGitHubContext: preset, diff --git a/internal/app/run/runner_idle_cleanup_test.go b/internal/app/run/runner_idle_cleanup_test.go index 0a56b505..c19fb796 100644 --- a/internal/app/run/runner_idle_cleanup_test.go +++ b/internal/app/run/runner_idle_cleanup_test.go @@ -44,7 +44,7 @@ func TestRunnerCleanupStaleTaskDirs(t *testing.T) { now: func() time.Time { return now }, } - r.cleanupStaleTaskDirs(context.Background(), workdirRoot) + r.cleanupStaleDirs(context.Background(), workdirRoot, isTaskIDDir) assert.NoDirExists(t, oldTask) assert.DirExists(t, freshTask) @@ -111,7 +111,7 @@ func TestRunnerCleanupStaleTaskDirsMissingRoot(t *testing.T) { // Must be a silent no-op rather than a warning or panic when the root // has not yet been created (e.g. the runner has never executed a task). - r.cleanupStaleTaskDirs(context.Background(), filepath.Join(t.TempDir(), "missing")) + r.cleanupStaleDirs(context.Background(), filepath.Join(t.TempDir(), "missing"), isTaskIDDir) } func TestRunnerCleanupStaleTaskDirsHonorsContext(t *testing.T) { @@ -135,7 +135,7 @@ func TestRunnerCleanupStaleTaskDirsHonorsContext(t *testing.T) { now: func() time.Time { return now }, } - r.cleanupStaleTaskDirs(ctx, workdirRoot) + r.cleanupStaleDirs(ctx, workdirRoot, isTaskIDDir) for i := 1001; i <= 1003; i++ { assert.DirExists(t, filepath.Join(workdirRoot, strconv.Itoa(i))) diff --git a/internal/app/run/runner_test.go b/internal/app/run/runner_test.go index 81d2f3eb..109e7734 100644 --- a/internal/app/run/runner_test.go +++ b/internal/app/run/runner_test.go @@ -83,7 +83,7 @@ func TestNewRunnerInitializesLabelsAndEnvironment(t *testing.T) { Labels: []string{"ubuntu:host", "", "pool:e57e18d4"}, } cli := clientmocks.NewClient(t) - cli.On("Address").Return("https://gitea.example/").Maybe() + cli.AddressValue = "https://gitea.example/" r := NewRunner(cfg, reg, cli) @@ -137,7 +137,7 @@ func TestNewRunnerLeavesProxyToTheTask(t *testing.T) { cfg.Cache.ExternalServer = "http://cache.local:8088/" reg := &config.Registration{Name: "runner"} cli := clientmocks.NewClient(t) - cli.On("Address").Return("https://gitea.example/").Maybe() + cli.AddressValue = "https://gitea.example/" r := NewRunner(cfg, reg, cli) @@ -161,7 +161,7 @@ func TestNewRunnerCacheServiceV2(t *testing.T) { cfg := &config.Config{} cfg.Cache.Dir, cfg.Cache.Host = t.TempDir(), "127.0.0.1" cli := clientmocks.NewClient(t) - cli.On("Address").Return("https://gitea.example/").Maybe() + cli.AddressValue = "https://gitea.example/" r := NewRunner(cfg, &config.Registration{Name: "runner"}, cli) t.Cleanup(func() { _ = r.Close() }) @@ -202,7 +202,7 @@ func TestNewRunnerNormalizesTheExternalCacheServer(t *testing.T) { cfg := &config.Config{} cfg.Cache.ExternalServer = "http://cache.local:8088//" cli := clientmocks.NewClient(t) - cli.On("Address").Return("https://gitea.example/").Maybe() + cli.AddressValue = "https://gitea.example/" r := NewRunner(cfg, &config.Registration{Name: "runner"}, cli) diff --git a/internal/pkg/client/client.go b/internal/pkg/client/client.go index 88c269d8..a3a3fc80 100644 --- a/internal/pkg/client/client.go +++ b/internal/pkg/client/client.go @@ -4,16 +4,17 @@ package client import ( - "gitea.dev/actionslib/ping/v1/pingv1connect" - "gitea.dev/actionslib/runner/v1/runnerv1connect" + "context" + + "connectrpc.com/connect" + "gitea.dev/actionslib/runner/v1" ) // A Client manages communication with the runner. -// -//go:generate mockery --name Client type Client interface { - pingv1connect.PingServiceClient - runnerv1connect.RunnerServiceClient Address() string - Insecure() bool + Declare(context.Context, *connect.Request[runnerv1.DeclareRequest]) (*connect.Response[runnerv1.DeclareResponse], error) + FetchTask(context.Context, *connect.Request[runnerv1.FetchTaskRequest]) (*connect.Response[runnerv1.FetchTaskResponse], error) + UpdateLog(context.Context, *connect.Request[runnerv1.UpdateLogRequest]) (*connect.Response[runnerv1.UpdateLogResponse], error) + UpdateTask(context.Context, *connect.Request[runnerv1.UpdateTaskRequest]) (*connect.Response[runnerv1.UpdateTaskResponse], error) } diff --git a/internal/pkg/client/header.go b/internal/pkg/client/header.go deleted file mode 100644 index e76e3220..00000000 --- a/internal/pkg/client/header.go +++ /dev/null @@ -1,13 +0,0 @@ -// Copyright 2023 The Gitea Authors. All rights reserved. -// SPDX-License-Identifier: MIT - -package client - -import "gitea.dev/actionslib/pkg/protocol" - -// The headers are defined in the shared protocol package so that Gitea and the -// runner cannot drift apart. -const ( - UUIDHeader = protocol.UUIDHeader - TokenHeader = protocol.TokenHeader -) diff --git a/internal/pkg/client/http.go b/internal/pkg/client/http.go index ada7a48e..0368335b 100644 --- a/internal/pkg/client/http.go +++ b/internal/pkg/client/http.go @@ -14,6 +14,7 @@ import ( "connectrpc.com/connect" "gitea.dev/actionslib/ping/v1/pingv1connect" + "gitea.dev/actionslib/pkg/protocol" "gitea.dev/actionslib/runner/v1/runnerv1connect" ) @@ -42,10 +43,10 @@ func New(endpoint string, insecure bool, uuid, token string, timeout time.Durati return func(ctx context.Context, req connect.AnyRequest) (connect.AnyResponse, error) { req.Header().Set("User-Agent", "gitea-runner/"+ver.Version()) if uuid != "" { - req.Header().Set(UUIDHeader, uuid) + req.Header().Set(protocol.UUIDHeader, uuid) } if token != "" { - req.Header().Set(TokenHeader, token) + req.Header().Set(protocol.TokenHeader, token) } return next(ctx, req) } @@ -64,7 +65,6 @@ func New(endpoint string, insecure bool, uuid, token string, timeout time.Durati opts..., ), endpoint: endpoint, - insecure: insecure, } } @@ -72,10 +72,6 @@ func (c *HTTPClient) Address() string { return c.endpoint } -func (c *HTTPClient) Insecure() bool { - return c.insecure -} - var _ Client = (*HTTPClient)(nil) // An HTTPClient manages communication with the runner API. @@ -83,5 +79,4 @@ type HTTPClient struct { pingv1connect.PingServiceClient runnerv1connect.RunnerServiceClient endpoint string - insecure bool } diff --git a/internal/pkg/client/http_test.go b/internal/pkg/client/http_test.go index ef555dc0..b575c4c2 100644 --- a/internal/pkg/client/http_test.go +++ b/internal/pkg/client/http_test.go @@ -12,6 +12,7 @@ import ( "connectrpc.com/connect" pingv1 "gitea.dev/actionslib/ping/v1" + "gitea.dev/actionslib/pkg/protocol" "github.com/stretchr/testify/require" ) @@ -71,14 +72,13 @@ func TestNewSetsBaseURLAndHeaders(t *testing.T) { c := New(server.URL+"/", false, "the-uuid", "the-token", time.Minute) // Address returns the endpoint as supplied (untrimmed) require.Equal(t, server.URL+"/", c.Address()) - require.False(t, c.Insecure()) // the call is expected to fail (server returns 500), we only assert what was sent _, _ = c.Ping(t.Context(), connect.NewRequest(&pingv1.PingRequest{Data: "hi"})) require.True(t, strings.HasPrefix(gotPath, "/api/actions/"), "unexpected path %q", gotPath) - require.Equal(t, "the-uuid", gotHeaders.Get(UUIDHeader)) - require.Equal(t, "the-token", gotHeaders.Get(TokenHeader)) + require.Equal(t, "the-uuid", gotHeaders.Get(protocol.UUIDHeader)) + require.Equal(t, "the-token", gotHeaders.Get(protocol.TokenHeader)) } func TestNewOmitsEmptyHeaders(t *testing.T) { @@ -92,6 +92,6 @@ func TestNewOmitsEmptyHeaders(t *testing.T) { c := New(server.URL, false, "", "", time.Minute) _, _ = c.Ping(t.Context(), connect.NewRequest(&pingv1.PingRequest{Data: "hi"})) - require.Empty(t, gotHeaders.Get(UUIDHeader)) - require.Empty(t, gotHeaders.Get(TokenHeader)) + require.Empty(t, gotHeaders.Get(protocol.UUIDHeader)) + require.Empty(t, gotHeaders.Get(protocol.TokenHeader)) } diff --git a/internal/pkg/client/mocks/Client.go b/internal/pkg/client/mocks/Client.go index e56b5431..8f10850c 100644 --- a/internal/pkg/client/mocks/Client.go +++ b/internal/pkg/client/mocks/Client.go @@ -1,251 +1,65 @@ -// Code generated by mockery v2.42.1. DO NOT EDIT. +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT package mocks import ( - context "context" + "context" + "fmt" - connect "connectrpc.com/connect" - - mock "github.com/stretchr/testify/mock" - - pingv1 "gitea.dev/actionslib/ping/v1" - - runnerv1 "gitea.dev/actionslib/runner/v1" + "connectrpc.com/connect" + "gitea.dev/actionslib/runner/v1" + "github.com/stretchr/testify/mock" ) -// Client is an autogenerated mock type for the Client type type Client struct { mock.Mock + AddressValue string } -// Address provides a mock function with given fields: -func (_m *Client) Address() string { - ret := _m.Called() - - if len(ret) == 0 { - panic("no return value specified for Address") - } - - var r0 string - if rf, ok := ret.Get(0).(func() string); ok { - r0 = rf() - } else { - r0 = ret.Get(0).(string) - } - - return r0 +func (m *Client) Address() string { + return m.AddressValue } -// Declare provides a mock function with given fields: _a0, _a1 -func (_m *Client) Declare(_a0 context.Context, _a1 *connect.Request[runnerv1.DeclareRequest]) (*connect.Response[runnerv1.DeclareResponse], error) { - ret := _m.Called(_a0, _a1) - - if len(ret) == 0 { - panic("no return value specified for Declare") +func call[Request, Response any](ctx context.Context, m *Client, method string, request *connect.Request[Request]) (*connect.Response[Response], error) { + returns := m.MethodCalled(method, ctx, request) + if callback, ok := returns.Get(0).(func(context.Context, *connect.Request[Request]) (*connect.Response[Response], error)); ok { + return callback(ctx, request) } - - var r0 *connect.Response[runnerv1.DeclareResponse] - var r1 error - if rf, ok := ret.Get(0).(func(context.Context, *connect.Request[runnerv1.DeclareRequest]) (*connect.Response[runnerv1.DeclareResponse], error)); ok { - return rf(_a0, _a1) - } - if rf, ok := ret.Get(0).(func(context.Context, *connect.Request[runnerv1.DeclareRequest]) *connect.Response[runnerv1.DeclareResponse]); ok { - r0 = rf(_a0, _a1) - } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).(*connect.Response[runnerv1.DeclareResponse]) + var response *connect.Response[Response] + if value := returns.Get(0); value != nil { + var ok bool + response, ok = value.(*connect.Response[Response]) + if !ok { + panic(fmt.Sprintf("unexpected response type %T for %s", value, method)) } } - - if rf, ok := ret.Get(1).(func(context.Context, *connect.Request[runnerv1.DeclareRequest]) error); ok { - r1 = rf(_a0, _a1) - } else { - r1 = ret.Error(1) - } - - return r0, r1 + return response, returns.Error(1) } -// FetchTask provides a mock function with given fields: _a0, _a1 -func (_m *Client) FetchTask(_a0 context.Context, _a1 *connect.Request[runnerv1.FetchTaskRequest]) (*connect.Response[runnerv1.FetchTaskResponse], error) { - ret := _m.Called(_a0, _a1) - - if len(ret) == 0 { - panic("no return value specified for FetchTask") - } - - var r0 *connect.Response[runnerv1.FetchTaskResponse] - var r1 error - if rf, ok := ret.Get(0).(func(context.Context, *connect.Request[runnerv1.FetchTaskRequest]) (*connect.Response[runnerv1.FetchTaskResponse], error)); ok { - return rf(_a0, _a1) - } - if rf, ok := ret.Get(0).(func(context.Context, *connect.Request[runnerv1.FetchTaskRequest]) *connect.Response[runnerv1.FetchTaskResponse]); ok { - r0 = rf(_a0, _a1) - } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).(*connect.Response[runnerv1.FetchTaskResponse]) - } - } - - if rf, ok := ret.Get(1).(func(context.Context, *connect.Request[runnerv1.FetchTaskRequest]) error); ok { - r1 = rf(_a0, _a1) - } else { - r1 = ret.Error(1) - } - - return r0, r1 +func (m *Client) Declare(ctx context.Context, request *connect.Request[runnerv1.DeclareRequest]) (*connect.Response[runnerv1.DeclareResponse], error) { + return call[runnerv1.DeclareRequest, runnerv1.DeclareResponse](ctx, m, "Declare", request) } -// Insecure provides a mock function with given fields: -func (_m *Client) Insecure() bool { - ret := _m.Called() - - if len(ret) == 0 { - panic("no return value specified for Insecure") - } - - var r0 bool - if rf, ok := ret.Get(0).(func() bool); ok { - r0 = rf() - } else { - r0 = ret.Get(0).(bool) - } - - return r0 +func (m *Client) FetchTask(ctx context.Context, request *connect.Request[runnerv1.FetchTaskRequest]) (*connect.Response[runnerv1.FetchTaskResponse], error) { + return call[runnerv1.FetchTaskRequest, runnerv1.FetchTaskResponse](ctx, m, "FetchTask", request) } -// Ping provides a mock function with given fields: _a0, _a1 -func (_m *Client) Ping(_a0 context.Context, _a1 *connect.Request[pingv1.PingRequest]) (*connect.Response[pingv1.PingResponse], error) { - ret := _m.Called(_a0, _a1) - - if len(ret) == 0 { - panic("no return value specified for Ping") - } - - var r0 *connect.Response[pingv1.PingResponse] - var r1 error - if rf, ok := ret.Get(0).(func(context.Context, *connect.Request[pingv1.PingRequest]) (*connect.Response[pingv1.PingResponse], error)); ok { - return rf(_a0, _a1) - } - if rf, ok := ret.Get(0).(func(context.Context, *connect.Request[pingv1.PingRequest]) *connect.Response[pingv1.PingResponse]); ok { - r0 = rf(_a0, _a1) - } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).(*connect.Response[pingv1.PingResponse]) - } - } - - if rf, ok := ret.Get(1).(func(context.Context, *connect.Request[pingv1.PingRequest]) error); ok { - r1 = rf(_a0, _a1) - } else { - r1 = ret.Error(1) - } - - return r0, r1 +func (m *Client) UpdateLog(ctx context.Context, request *connect.Request[runnerv1.UpdateLogRequest]) (*connect.Response[runnerv1.UpdateLogResponse], error) { + return call[runnerv1.UpdateLogRequest, runnerv1.UpdateLogResponse](ctx, m, "UpdateLog", request) } -// Register provides a mock function with given fields: _a0, _a1 -func (_m *Client) Register(_a0 context.Context, _a1 *connect.Request[runnerv1.RegisterRequest]) (*connect.Response[runnerv1.RegisterResponse], error) { - ret := _m.Called(_a0, _a1) - - if len(ret) == 0 { - panic("no return value specified for Register") - } - - var r0 *connect.Response[runnerv1.RegisterResponse] - var r1 error - if rf, ok := ret.Get(0).(func(context.Context, *connect.Request[runnerv1.RegisterRequest]) (*connect.Response[runnerv1.RegisterResponse], error)); ok { - return rf(_a0, _a1) - } - if rf, ok := ret.Get(0).(func(context.Context, *connect.Request[runnerv1.RegisterRequest]) *connect.Response[runnerv1.RegisterResponse]); ok { - r0 = rf(_a0, _a1) - } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).(*connect.Response[runnerv1.RegisterResponse]) - } - } - - if rf, ok := ret.Get(1).(func(context.Context, *connect.Request[runnerv1.RegisterRequest]) error); ok { - r1 = rf(_a0, _a1) - } else { - r1 = ret.Error(1) - } - - return r0, r1 +func (m *Client) UpdateTask(ctx context.Context, request *connect.Request[runnerv1.UpdateTaskRequest]) (*connect.Response[runnerv1.UpdateTaskResponse], error) { + return call[runnerv1.UpdateTaskRequest, runnerv1.UpdateTaskResponse](ctx, m, "UpdateTask", request) } -// UpdateLog provides a mock function with given fields: _a0, _a1 -func (_m *Client) UpdateLog(_a0 context.Context, _a1 *connect.Request[runnerv1.UpdateLogRequest]) (*connect.Response[runnerv1.UpdateLogResponse], error) { - ret := _m.Called(_a0, _a1) - - if len(ret) == 0 { - panic("no return value specified for UpdateLog") - } - - var r0 *connect.Response[runnerv1.UpdateLogResponse] - var r1 error - if rf, ok := ret.Get(0).(func(context.Context, *connect.Request[runnerv1.UpdateLogRequest]) (*connect.Response[runnerv1.UpdateLogResponse], error)); ok { - return rf(_a0, _a1) - } - if rf, ok := ret.Get(0).(func(context.Context, *connect.Request[runnerv1.UpdateLogRequest]) *connect.Response[runnerv1.UpdateLogResponse]); ok { - r0 = rf(_a0, _a1) - } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).(*connect.Response[runnerv1.UpdateLogResponse]) - } - } - - if rf, ok := ret.Get(1).(func(context.Context, *connect.Request[runnerv1.UpdateLogRequest]) error); ok { - r1 = rf(_a0, _a1) - } else { - r1 = ret.Error(1) - } - - return r0, r1 -} - -// UpdateTask provides a mock function with given fields: _a0, _a1 -func (_m *Client) UpdateTask(_a0 context.Context, _a1 *connect.Request[runnerv1.UpdateTaskRequest]) (*connect.Response[runnerv1.UpdateTaskResponse], error) { - ret := _m.Called(_a0, _a1) - - if len(ret) == 0 { - panic("no return value specified for UpdateTask") - } - - var r0 *connect.Response[runnerv1.UpdateTaskResponse] - var r1 error - if rf, ok := ret.Get(0).(func(context.Context, *connect.Request[runnerv1.UpdateTaskRequest]) (*connect.Response[runnerv1.UpdateTaskResponse], error)); ok { - return rf(_a0, _a1) - } - if rf, ok := ret.Get(0).(func(context.Context, *connect.Request[runnerv1.UpdateTaskRequest]) *connect.Response[runnerv1.UpdateTaskResponse]); ok { - r0 = rf(_a0, _a1) - } else { - if ret.Get(0) != nil { - r0 = ret.Get(0).(*connect.Response[runnerv1.UpdateTaskResponse]) - } - } - - if rf, ok := ret.Get(1).(func(context.Context, *connect.Request[runnerv1.UpdateTaskRequest]) error); ok { - r1 = rf(_a0, _a1) - } else { - r1 = ret.Error(1) - } - - return r0, r1 -} - -// NewClient creates a new instance of Client. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. -// The first argument is typically a *testing.T value. func NewClient(t interface { mock.TestingT Cleanup(func()) }, ) *Client { - mock := &Client{} - mock.Mock.Test(t) - - t.Cleanup(func() { mock.AssertExpectations(t) }) - - return mock + client := &Client{} + client.Test(t) + t.Cleanup(func() { client.AssertExpectations(t) }) + return client }