diff --git a/core/pkg/executor/action_reference.go b/core/pkg/executor/action_reference.go index a17c655b..0c8cd15d 100644 --- a/core/pkg/executor/action_reference.go +++ b/core/pkg/executor/action_reference.go @@ -12,6 +12,7 @@ import ( "errors" "fmt" "io" + "io/fs" "net/url" "path/filepath" @@ -24,7 +25,6 @@ import ( "drassi.run/core/pkg/store/git" "drassi.run/core/util/dig" "drassi.run/core/util/otel" - "github.com/go-git/go-git/v5/plumbing/object" "go.opentelemetry.io/otel/trace" "go.uber.org/dig" "gopkg.in/yaml.v3" @@ -75,7 +75,12 @@ func (spec *ReferenceActionSpec) CreateExecutor( token = "" } - if rev, err := store.Fetch(ctx, spec.Repo, token); err != nil { + var fetchOpts []gitstore.FetchOption + if token != "" { + fetchOpts = append(fetchOpts, gitstore.WithToken(token)) + } + + if rev, err := store.Fetch(ctx, spec.Repo, fetchOpts...); err != nil { return nil, err } else { s.Writef("Download action repository %q (SHA:%s)", gitstore.Location(spec.Repo), rev) @@ -97,13 +102,13 @@ func (spec *ReferenceActionSpec) loadAction(ctx context.Context, s *scribe.Scrib // 1. First, try reading "action.yml" or "action.yaml" file for _, f := range []string{"action.yml", "action.yaml"} { path := filepath.Join(spec.Repo.Path, f) - if r, err := store.File(ctx, spec.Repo, spec.rev, path); err == nil { + if r, err := store.Read(ctx, spec.Repo, spec.rev, gitstore.WithFile(path)); err == nil { span.AddEvent("Loaded Action", trace.WithAttributes(xotel.ActionPath(path)), ) s.Debugf("Loading %q for action", path) return spec.loadActionManifest(r) - } else if !errors.Is(err, object.ErrFileNotFound) { + } else if !errors.Is(err, fs.ErrNotExist) { return nil, err } } @@ -111,14 +116,14 @@ func (spec *ReferenceActionSpec) loadAction(ctx context.Context, s *scribe.Scrib // 2. Second, try reading "Dockerfile" or "dockerfile" for _, f := range []string{"Dockerfile", "dockerfile"} { path := filepath.Join(spec.Repo.Path, f) - if r, err := store.File(ctx, spec.Repo, spec.rev, path); err == nil { + if r, err := store.Read(ctx, spec.Repo, spec.rev, gitstore.WithFile(path)); err == nil { r.Close() span.AddEvent("Loaded Action", trace.WithAttributes(xotel.ActionPath(path)), ) s.Debugf("Loading %q for action", path) return spec.createDockerfileAction(path) - } else if !errors.Is(err, object.ErrFileNotFound) { + } else if !errors.Is(err, fs.ErrNotExist) { return nil, err } } @@ -154,8 +159,7 @@ func (spec *ReferenceActionSpec) createDockerfileAction(dockerfile string) (Acti } func (spec *ReferenceActionSpec) transferAction(ctx context.Context, store gitstore.Manager, sandbox sandboxer.Sandbox) error { - location := gitstore.FullName(spec.Repo) + "@" + spec.Repo.Ref - r, err := store.Read(ctx, spec.Repo, spec.rev, location) + r, err := store.Read(ctx, spec.Repo, spec.rev, gitstore.WithSubpath(spec.Repo.Path)) if err != nil { return err } diff --git a/core/pkg/store/git/manager.go b/core/pkg/store/git/manager.go index dfaa83ea..2fc0bc2a 100644 --- a/core/pkg/store/git/manager.go +++ b/core/pkg/store/git/manager.go @@ -29,8 +29,6 @@ import ( "github.com/go-git/go-git/v5/plumbing" "github.com/go-git/go-git/v5/plumbing/cache" "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" "github.com/go-git/go-git/v5/storage" "github.com/go-git/go-git/v5/storage/filesystem" "github.com/hashicorp/golang-lru/v2/expirable" @@ -39,9 +37,8 @@ import ( ) type Manager interface { - Fetch(ctx context.Context, repo *RepoReference, token string) (rev string, err error) - Read(ctx context.Context, repo *RepoReference, rev, dir string) (io.ReadCloser, error) - File(ctx context.Context, repo *RepoReference, rev, path string) (io.ReadCloser, error) + Fetch(ctx context.Context, repo *RepoReference, opts ...FetchOption) (rev string, err error) + Read(ctx context.Context, repo *RepoReference, rev string, opts ...ReadOption) (io.ReadCloser, error) Close() error } @@ -87,7 +84,7 @@ func (m *manager) onEvict(_ string, repo *git.Repository) { } } -func (m *manager) Fetch(ctx context.Context, repo *RepoReference, token string) (string, error) { +func (m *manager) Fetch(ctx context.Context, repo *RepoReference, opts ...FetchOption) (string, error) { key := Location(repo) v, err, _ := m.sf.Do(key, func() (any, error) { repoPath, err := m.ensureDir(repo) @@ -103,7 +100,7 @@ func (m *manager) Fetch(ctx context.Context, repo *RepoReference, token string) tmpBranch := rand.String(12) defer gitRepo.DeleteBranch(tmpBranch) - err = m.fetch(ctx, gitRepo, repo, token, tmpBranch) + err = m.fetch(ctx, gitRepo, repo, tmpBranch, opts...) if err != nil { return "", err } @@ -120,7 +117,16 @@ func (m *manager) Fetch(ctx context.Context, repo *RepoReference, token string) return v.(string), nil } -func (m *manager) Read(ctx context.Context, repo *RepoReference, rev string, dir string) (io.ReadCloser, error) { +func (m *manager) Read(ctx context.Context, repo *RepoReference, rev string, opts ...ReadOption) (io.ReadCloser, error) { + ro := new(readOptions) + for _, opt := range opts { + opt(ro) + } + + if ro.file != "" && ro.subpath != "" { + return nil, errors.New("cannot specify both file and subpath") + } + gitRepo, err := m.getRepo(repo) if err != nil { return nil, err @@ -130,6 +136,39 @@ func (m *manager) Read(ctx context.Context, repo *RepoReference, rev string, dir if err != nil { return nil, err } + + if ro.file != "" { + return m.readFile(commit, ro.file) + } + return m.readArchive(ctx, commit, ro.subpath) +} + +func (m *manager) readFile(commit *object.Commit, filePath string) (io.ReadCloser, error) { + tree, err := commit.Tree() + if err != nil { + return nil, err + } + + cleanPath := strings.TrimPrefix(path.Clean(filePath), "/") + entry, err := tree.FindEntry(cleanPath) + if err != nil { + if notFoundErr(err) { + return nil, fs.ErrNotExist + } + return nil, err + } + if !entry.Mode.IsFile() { + return nil, fmt.Errorf("%q is not a (regular) file", filePath) + } + + file, err := tree.TreeEntryFile(entry) + if err != nil { + return nil, err + } + return file.Reader() +} + +func (m *manager) readArchive(ctx context.Context, commit *object.Commit, subpath string) (io.ReadCloser, error) { files, err := commit.Files() if err != nil { return nil, err @@ -149,7 +188,7 @@ func (m *manager) Read(ctx context.Context, repo *RepoReference, rev string, dir defer close(ch) tw := tar.NewWriter(writer) - handler := newTarHandler(tw, dir) + handler := newTarHandler(tw, subpath) err := files.ForEach(handler) err = cmp.Or(err, tw.Close()) @@ -158,41 +197,6 @@ func (m *manager) Read(ctx context.Context, repo *RepoReference, rev string, dir return reader, nil } -func (m *manager) File(ctx context.Context, repo *RepoReference, rev, filePath string) (io.ReadCloser, error) { - gitRepo, err := m.getRepo(repo) - if err != nil { - return nil, err - } - - commit, err := gitRepo.CommitObject(plumbing.NewHash(rev)) - if err != nil { - return nil, err - } - - tree, err := commit.Tree() - if err != nil { - return nil, err - } - - cleanPath := strings.TrimPrefix(path.Clean(filePath), "/") - entry, err := tree.FindEntry(cleanPath) - if err != nil { - if notFoundErr(err) { - return nil, fs.ErrNotExist - } - return nil, err - } - if !entry.Mode.IsFile() { - return nil, fmt.Errorf("%q is not a (regular) file", filePath) - } - - if file, err := tree.TreeEntryFile(entry); err != nil { - return nil, err - } else { - return file.Reader() - } -} - func (m *manager) getRepo(repo *RepoReference) (*git.Repository, error) { id := FullName(repo) if gitRepo, ok := m.repos.Get(id); ok { @@ -215,13 +219,10 @@ func (m *manager) getRepo(repo *RepoReference) (*git.Repository, error) { return gitRepo, nil } -func (m *manager) fetch(ctx context.Context, gitRepo *git.Repository, repo *RepoReference, token, branch string) error { - var auth transport.AuthMethod - if token != "" { - auth = &http.BasicAuth{ - Username: "token", - Password: token, - } +func (m *manager) fetch(ctx context.Context, gitRepo *git.Repository, repo *RepoReference, branch string, opts ...FetchOption) error { + fo := new(fetchOptions) + for _, opt := range opts { + opt(fo) } remoteConfig := &config.RemoteConfig{ @@ -235,18 +236,18 @@ func (m *manager) fetch(ctx context.Context, gitRepo *git.Repository, repo *Repo // TODO: using treeless clone when go-git implement it // https://github.blog/2020-12-21-get-up-to-speed-with-partial-clone-and-shallow-clone/ - fetchOptions := &git.FetchOptions{ + gfo := &git.FetchOptions{ RefSpecs: []config.RefSpec{ config.RefSpec(fmt.Sprintf("+%s:refs/heads/%s", repo.Ref, branch)), }, - Auth: auth, + Auth: fo.auth, Tags: git.NoTags, Force: true, Prune: true, } - return remote.FetchContext(ctx, fetchOptions) + return remote.FetchContext(ctx, gfo) } func (m *manager) ensureDir(repo *RepoReference) (string, error) { diff --git a/core/pkg/store/git/manager_test.go b/core/pkg/store/git/manager_test.go index 0ccd2d60..655b4427 100644 --- a/core/pkg/store/git/manager_test.go +++ b/core/pkg/store/git/manager_test.go @@ -242,7 +242,7 @@ func (s *ManagerTestSuite) TestFetch() { s.Run("fetch default branch (HEAD)", func() { ref := s.makeLocalRepoRef(repoInfo.RepoDir, "HEAD") - rev, err := s.mgr.Fetch(s.T().Context(), ref, "") + rev, err := s.mgr.Fetch(s.T().Context(), ref) s.Require().NoError(err) s.Assert().Equal(repoInfo.MainCommitHash, rev) }) @@ -250,7 +250,7 @@ func (s *ManagerTestSuite) TestFetch() { s.Run("fetch feature branch", func() { ref := s.makeLocalRepoRef(repoInfo.RepoDir, repoInfo.BranchName) - rev, err := s.mgr.Fetch(s.T().Context(), ref, "") + rev, err := s.mgr.Fetch(s.T().Context(), ref) s.Require().NoError(err) s.Assert().Equal(repoInfo.FeatureCommit, rev) }) @@ -258,7 +258,7 @@ func (s *ManagerTestSuite) TestFetch() { s.Run("fetch tag", func() { ref := s.makeLocalRepoRef(repoInfo.RepoDir, repoInfo.TagName) - rev, err := s.mgr.Fetch(s.T().Context(), ref, "") + rev, err := s.mgr.Fetch(s.T().Context(), ref) s.Require().NoError(err) s.Assert().Equal(repoInfo.MainCommitHash, rev) }) @@ -266,7 +266,7 @@ func (s *ManagerTestSuite) TestFetch() { s.Run("fetch non-existent ref returns error", func() { ref := s.makeLocalRepoRef(repoInfo.RepoDir, "nonexistent-branch-404") - rev, err := s.mgr.Fetch(s.T().Context(), ref, "") + rev, err := s.mgr.Fetch(s.T().Context(), ref) s.Assert().Error(err) s.Assert().Empty(rev) }) @@ -287,7 +287,7 @@ func (s *ManagerTestSuite) TestFetch() { } const testToken = "secret-token-123" - _, err := s.mgr.Fetch(s.T().Context(), ref, testToken) + _, err := s.mgr.Fetch(s.T().Context(), ref, WithToken(testToken)) s.Assert().Error(err) expectedAuth := "Basic " + base64.StdEncoding.EncodeToString([]byte("token:"+testToken)) s.Assert().Equal(expectedAuth, receivedAuth) @@ -296,7 +296,7 @@ func (s *ManagerTestSuite) TestFetch() { s.Run("fetch non-existent repository returns error", func() { ref := s.makeLocalRepoRef(filepath.Join(s.T().TempDir(), "does-not-exist"), "HEAD") - rev, err := s.mgr.Fetch(s.T().Context(), ref, "") + rev, err := s.mgr.Fetch(s.T().Context(), ref) s.Assert().Error(err) s.Assert().Empty(rev) }) @@ -304,9 +304,10 @@ func (s *ManagerTestSuite) TestFetch() { func (s *ManagerTestSuite) TestRead() { files := map[string]string{ - "action.yml": "name: test-action\ndescription: test", - "src/index.js": "console.log('test');", - "nested/dir/file.md": "# Docs", + "action.yml": "name: test-action\ndescription: test", + "src/index.js": "console.log('test');", + "nested/dir/file.md": "# Docs", + "nested/sub/data.json": `{"key": "value"}`, } symlinks := map[string]string{ "src/alias.js": "index.js", @@ -314,19 +315,20 @@ func (s *ManagerTestSuite) TestRead() { repoInfo := s.initTestGitRepo(files, symlinks) ref := s.makeLocalRepoRef(repoInfo.RepoDir, "HEAD") - rev, err := s.mgr.Fetch(s.T().Context(), ref, "") + rev, err := s.mgr.Fetch(s.T().Context(), ref) s.Require().NoError(err) s.Run("read full archive with directory prefix", func() { prefix := "actions/my-action@v1" - rc, err := s.mgr.Read(s.T().Context(), ref, rev, prefix) + rc, err := s.mgr.Read(s.T().Context(), ref, rev, WithSubpath(prefix)) s.Require().NoError(err) defer rc.Close() expectedEntries := map[string]string{ - "actions/my-action@v1/action.yml": files["action.yml"], - "actions/my-action@v1/src/index.js": files["src/index.js"], - "actions/my-action@v1/nested/dir/file.md": files["nested/dir/file.md"], + "actions/my-action@v1/action.yml": files["action.yml"], + "actions/my-action@v1/src/index.js": files["src/index.js"], + "actions/my-action@v1/nested/dir/file.md": files["nested/dir/file.md"], + "actions/my-action@v1/nested/sub/data.json": files["nested/sub/data.json"], } expectedSymlinks := map[string]string{ "actions/my-action@v1/src/alias.js": "index.js", @@ -336,7 +338,7 @@ func (s *ManagerTestSuite) TestRead() { }) s.Run("read full archive with empty prefix", func() { - rc, err := s.mgr.Read(s.T().Context(), ref, rev, "") + rc, err := s.mgr.Read(s.T().Context(), ref, rev) s.Require().NoError(err) defer rc.Close() @@ -345,7 +347,7 @@ func (s *ManagerTestSuite) TestRead() { s.Run("read non-existent revision error", func() { nonExistentRev := strings.Repeat("0", 40) - rc, err := s.mgr.Read(s.T().Context(), ref, nonExistentRev, "") + rc, err := s.mgr.Read(s.T().Context(), ref, nonExistentRev) s.Assert().Error(err) s.Assert().Nil(rc) }) @@ -356,44 +358,13 @@ func (s *ManagerTestSuite) TestRead() { Name: "unknown/repo", Ref: "main", } - rc, err := s.mgr.Read(s.T().Context(), unknownRef, rev, "") + rc, err := s.mgr.Read(s.T().Context(), unknownRef, rev) s.Assert().Error(err) s.Assert().Nil(rc) }) - s.Run("context cancellation during read", func() { - cancelMgr, _ := s.newTestManager() - - cancelRev, err := cancelMgr.Fetch(s.T().Context(), ref, "") - s.Require().NoError(err) - - ctx, cancel := context.WithCancel(s.T().Context()) - cancel() - - rc, err := cancelMgr.Read(ctx, ref, cancelRev, "") - s.Require().NoError(err) - defer rc.Close() - - _, readErr := io.ReadAll(rc) - s.Assert().Error(readErr) - s.Assert().True(errors.Is(readErr, context.Canceled) || errors.Is(readErr, io.ErrClosedPipe)) - }) -} - -func (s *ManagerTestSuite) TestFile() { - files := map[string]string{ - "action.yml": "name: my-action", - "src/index.js": "console.log('ok')", - "nested/sub/data.json": `{"key": "value"}`, - } - repoInfo := s.initTestGitRepo(files, nil) - - ref := s.makeLocalRepoRef(repoInfo.RepoDir, "HEAD") - rev, err := s.mgr.Fetch(s.T().Context(), ref, "") - s.Require().NoError(err) - s.Run("read existing regular file", func() { - rc, err := s.mgr.File(s.T().Context(), ref, rev, "action.yml") + rc, err := s.mgr.Read(s.T().Context(), ref, rev, WithFile("action.yml")) s.Require().NoError(err) defer rc.Close() @@ -410,7 +381,7 @@ func (s *ManagerTestSuite) TestFile() { "nested/../nested/sub/data.json", } for _, p := range testPaths { - rc, err := s.mgr.File(s.T().Context(), ref, rev, p) + rc, err := s.mgr.Read(s.T().Context(), ref, rev, WithFile(p)) s.Require().NoError(err, "path: %s", p) content, err := io.ReadAll(rc) s.Require().NoError(err) @@ -420,14 +391,14 @@ func (s *ManagerTestSuite) TestFile() { }) s.Run("read non-existent file error", func() { - rc, err := s.mgr.File(s.T().Context(), ref, rev, "does-not-exist.txt") + rc, err := s.mgr.Read(s.T().Context(), ref, rev, WithFile("does-not-exist.txt")) s.Assert().Error(err) s.Assert().Nil(rc) s.Assert().True(os.IsNotExist(err) || notFoundErr(err)) }) s.Run("read directory as file error", func() { - rc, err := s.mgr.File(s.T().Context(), ref, rev, "src") + rc, err := s.mgr.Read(s.T().Context(), ref, rev, WithFile("src")) s.Assert().Error(err) s.Assert().Nil(rc) s.Assert().Contains(err.Error(), "not a (regular) file") @@ -435,7 +406,7 @@ func (s *ManagerTestSuite) TestFile() { s.Run("read file with invalid revision error", func() { invalidRev := strings.Repeat("a", 40) - rc, err := s.mgr.File(s.T().Context(), ref, invalidRev, "action.yml") + rc, err := s.mgr.Read(s.T().Context(), ref, invalidRev, WithFile("action.yml")) s.Assert().Error(err) s.Assert().Nil(rc) }) @@ -446,9 +417,34 @@ func (s *ManagerTestSuite) TestFile() { Name: "unknown/repo", Ref: "main", } - rc, err := s.mgr.File(s.T().Context(), unknownRef, rev, "action.yml") + rc, err := s.mgr.Read(s.T().Context(), unknownRef, rev, WithFile("action.yml")) + s.Assert().Error(err) + s.Assert().Nil(rc) + }) + + s.Run("read with both file and subpath returns error", func() { + rc, err := s.mgr.Read(s.T().Context(), ref, rev, WithFile("action.yml"), WithSubpath("actions")) s.Assert().Error(err) s.Assert().Nil(rc) + s.Assert().Contains(err.Error(), "cannot specify both file and subpath") + }) + + s.Run("context cancellation during read", func() { + cancelMgr, _ := s.newTestManager() + + cancelRev, err := cancelMgr.Fetch(s.T().Context(), ref) + s.Require().NoError(err) + + ctx, cancel := context.WithCancel(s.T().Context()) + cancel() + + rc, err := cancelMgr.Read(ctx, ref, cancelRev) + s.Require().NoError(err) + defer rc.Close() + + _, readErr := io.ReadAll(rc) + s.Assert().Error(readErr) + s.Assert().True(errors.Is(readErr, context.Canceled) || errors.Is(readErr, io.ErrClosedPipe)) }) } @@ -464,7 +460,7 @@ func (s *ManagerTestSuite) TestPersistence() { mgr1, err := New(rootDir) s.Require().NoError(err) ref := s.makeLocalRepoRef(repoInfo.RepoDir, "HEAD") - rev, err := mgr1.Fetch(s.T().Context(), ref, "") + rev, err := mgr1.Fetch(s.T().Context(), ref) s.Require().NoError(err) // Manager 2: Created with the same rootDir without calling Fetch @@ -472,7 +468,7 @@ func (s *ManagerTestSuite) TestPersistence() { s.Require().NoError(err) // File lookup should succeed by reading from disk - rc, err := mgr2.File(s.T().Context(), ref, rev, "action.yml") + rc, err := mgr2.Read(s.T().Context(), ref, rev, WithFile("action.yml")) s.Require().NoError(err) defer rc.Close() @@ -481,7 +477,7 @@ func (s *ManagerTestSuite) TestPersistence() { s.Assert().Equal(files["action.yml"], string(content)) // Read archive should also succeed - tarRc, err := mgr2.Read(s.T().Context(), ref, rev, "") + tarRc, err := mgr2.Read(s.T().Context(), ref, rev) s.Require().NoError(err) defer tarRc.Close() s.assertTarEntries(tarRc, files, nil) @@ -506,11 +502,11 @@ func (s *ManagerTestSuite) TestConcurrency() { ctx := s.T().Context() // Concurrently fetch - rev, err := s.mgr.Fetch(ctx, ref, "") + rev, err := s.mgr.Fetch(ctx, ref) s.Require().NoError(err) // Concurrently read file - rc, err := s.mgr.File(ctx, ref, rev, "file1.txt") + rc, err := s.mgr.Read(ctx, ref, rev, WithFile("file1.txt")) s.Require().NoError(err) content, err := io.ReadAll(rc) s.Require().NoError(err) @@ -518,7 +514,7 @@ func (s *ManagerTestSuite) TestConcurrency() { _ = rc.Close() // Concurrently read tar - tarRc, err := s.mgr.Read(ctx, ref, rev, "") + tarRc, err := s.mgr.Read(ctx, ref, rev) s.Require().NoError(err) s.assertTarEntries(tarRc, files, nil) _ = tarRc.Close() @@ -537,7 +533,7 @@ func (s *ManagerTestSuite) TestClose() { ref := s.makeLocalRepoRef(repoInfo.RepoDir, "HEAD") // Fetch repository to ensure it is opened and stored in manager - _, err := s.mgr.Fetch(s.T().Context(), ref, "") + _, err := s.mgr.Fetch(s.T().Context(), ref) s.Require().NoError(err) // Close the manager diff --git a/core/pkg/store/git/options.go b/core/pkg/store/git/options.go index ba750f7e..75d1eebe 100644 --- a/core/pkg/store/git/options.go +++ b/core/pkg/store/git/options.go @@ -6,7 +6,12 @@ package gitstore -import "time" +import ( + "time" + + "github.com/go-git/go-git/v5/plumbing/transport" + "github.com/go-git/go-git/v5/plumbing/transport/http" +) const ( defaultSize = 100 @@ -30,3 +35,35 @@ func WithTTL(ttl time.Duration) Option { o.ttl = ttl } } + +type FetchOption func(*fetchOptions) +type fetchOptions struct { + auth transport.AuthMethod +} + +func WithToken(token string) FetchOption { + return func(o *fetchOptions) { + o.auth = &http.BasicAuth{ + Username: "token", + Password: token, + } + } +} + +type ReadOption func(*readOptions) +type readOptions struct { + subpath string + file string +} + +func WithSubpath(subpath string) ReadOption { + return func(o *readOptions) { + o.subpath = subpath + } +} + +func WithFile(file string) ReadOption { + return func(o *readOptions) { + o.file = file + } +}