diff --git a/task_test.go b/task_test.go index 711d672416..6504988a2a 100644 --- a/task_test.go +++ b/task_test.go @@ -702,6 +702,7 @@ func TestIncludesRemote(t *testing.T) { enableExperimentForTest(t, &experiments.RemoteTaskfiles, 1) dir := "testdata/includes_remote" + os.RemoveAll(filepath.Join(dir, ".task", "remote")) srv := httptest.NewServer(http.FileServer(http.Dir(dir))) defer srv.Close() @@ -777,8 +778,8 @@ func TestIncludesRemote(t *testing.T) { }, } - for j, e := range executors { - t.Run(fmt.Sprint(j), func(t *testing.T) { + for _, e := range executors { + t.Run(e.name, func(t *testing.T) { require.NoError(t, e.executor.Setup()) for k, taskCall := range taskCalls { diff --git a/taskfile/node_base.go b/taskfile/node_base.go index 8dd60f831a..c712de6c46 100644 --- a/taskfile/node_base.go +++ b/taskfile/node_base.go @@ -1,19 +1,19 @@ package taskfile type ( - NodeOption func(*BaseNode) - // BaseNode is a generic node that implements the Parent() methods of the + NodeOption func(*baseNode) + // baseNode is a generic node that implements the Parent() methods of the // NodeReader interface. It does not implement the Read() method and it // designed to be embedded in other node types so that this boilerplate code // does not need to be repeated. - BaseNode struct { + baseNode struct { parent Node dir string } ) -func NewBaseNode(dir string, opts ...NodeOption) *BaseNode { - node := &BaseNode{ +func NewBaseNode(dir string, opts ...NodeOption) *baseNode { + node := &baseNode{ parent: nil, dir: dir, } @@ -27,15 +27,15 @@ func NewBaseNode(dir string, opts ...NodeOption) *BaseNode { } func WithParent(parent Node) NodeOption { - return func(node *BaseNode) { + return func(node *baseNode) { node.parent = parent } } -func (node *BaseNode) Parent() Node { +func (node *baseNode) Parent() Node { return node.parent } -func (node *BaseNode) Dir() string { +func (node *baseNode) Dir() string { return node.dir } diff --git a/taskfile/node_cache.go b/taskfile/node_cache.go index b489161b23..0dac811429 100644 --- a/taskfile/node_cache.go +++ b/taskfile/node_cache.go @@ -11,13 +11,13 @@ import ( const remoteCacheDir = "remote" type CacheNode struct { - *BaseNode + *baseNode source RemoteNode } func NewCacheNode(source RemoteNode, dir string) *CacheNode { return &CacheNode{ - BaseNode: &BaseNode{ + baseNode: &baseNode{ dir: filepath.Join(dir, remoteCacheDir), }, source: source, diff --git a/taskfile/node_file.go b/taskfile/node_file.go index 79b7a9572e..aa35437cb6 100644 --- a/taskfile/node_file.go +++ b/taskfile/node_file.go @@ -13,8 +13,8 @@ import ( // A FileNode is a node that reads a taskfile from the local filesystem. type FileNode struct { - *BaseNode - Entrypoint string + *baseNode + entrypoint string } func NewFileNode(entrypoint, dir string, opts ...NodeOption) (*FileNode, error) { @@ -25,13 +25,13 @@ func NewFileNode(entrypoint, dir string, opts ...NodeOption) (*FileNode, error) return nil, err } return &FileNode{ - BaseNode: base, - Entrypoint: entrypoint, + baseNode: base, + entrypoint: entrypoint, }, nil } func (node *FileNode) Location() string { - return node.Entrypoint + return node.entrypoint } func (node *FileNode) Read() ([]byte, error) { @@ -63,7 +63,7 @@ func (node *FileNode) ResolveEntrypoint(entrypoint string) (string, error) { // NOTE: Uses the directory of the entrypoint (Taskfile), not the current working directory // This means that files are included relative to one another - entrypointDir := filepath.Dir(node.Entrypoint) + entrypointDir := filepath.Dir(node.entrypoint) return filepathext.SmartJoin(entrypointDir, path), nil } @@ -79,6 +79,6 @@ func (node *FileNode) ResolveDir(dir string) (string, error) { // NOTE: Uses the directory of the entrypoint (Taskfile), not the current working directory // This means that files are included relative to one another - entrypointDir := filepath.Dir(node.Entrypoint) + entrypointDir := filepath.Dir(node.entrypoint) return filepathext.SmartJoin(entrypointDir, path), nil } diff --git a/taskfile/node_git.go b/taskfile/node_git.go index fd15ce4e8a..5152ea7e76 100644 --- a/taskfile/node_git.go +++ b/taskfile/node_git.go @@ -21,8 +21,8 @@ import ( // An GitNode is a node that reads a Taskfile from a remote location via Git. type GitNode struct { - *BaseNode - URL *url.URL + *baseNode + url *url.URL rawUrl string ref string path string @@ -40,23 +40,20 @@ func NewGitNode( return nil, err } - basePath, path := func() (string, string) { - x := strings.Split(u.Path, "//") - return x[0], x[1] - }() + basePath, path := splitURLOnDoubleSlash(u) ref := u.Query().Get("ref") - rawUrl := u.String() + rawUrl := u.Redacted() u.RawQuery = "" u.Path = basePath if u.Scheme == "http" && !insecure { - return nil, &errors.TaskfileNotSecureError{URI: entrypoint} + return nil, &errors.TaskfileNotSecureError{URI: u.Redacted()} } return &GitNode{ - BaseNode: base, - URL: u, + baseNode: base, + url: u, rawUrl: rawUrl, ref: ref, path: path, @@ -79,7 +76,7 @@ func (node *GitNode) ReadContext(_ context.Context) ([]byte, error) { fs := memfs.New() storer := memory.NewStorage() _, err := git.Clone(storer, fs, &git.CloneOptions{ - URL: node.URL.String(), + URL: node.url.String(), ReferenceName: plumbing.ReferenceName(node.ref), SingleBranch: true, Depth: 1, @@ -102,7 +99,7 @@ func (node *GitNode) ReadContext(_ context.Context) ([]byte, error) { func (node *GitNode) ResolveEntrypoint(entrypoint string) (string, error) { dir, _ := filepath.Split(node.path) - resolvedEntrypoint := fmt.Sprintf("%s//%s", node.URL, filepath.Join(dir, entrypoint)) + resolvedEntrypoint := fmt.Sprintf("%s//%s", node.url, filepath.Join(dir, entrypoint)) if node.ref != "" { return fmt.Sprintf("%s?ref=%s", resolvedEntrypoint, node.ref), nil } @@ -127,11 +124,23 @@ func (node *GitNode) ResolveDir(dir string) (string, error) { func (node *GitNode) CacheKey() string { checksum := strings.TrimRight(checksum([]byte(node.Location())), "=") - prefix := filepath.Base(filepath.Dir(node.path)) - lastDir := filepath.Base(node.path) + lastDir := filepath.Base(filepath.Dir(node.path)) + prefix := filepath.Base(node.path) // Means it's not "", nor "." nor "/", so it's a valid directory if len(lastDir) > 1 { - prefix = fmt.Sprintf("%s-%s", lastDir, prefix) + prefix = fmt.Sprintf("%s.%s", lastDir, prefix) + } + return fmt.Sprintf("git.%s.%s.%s", node.url.Host, prefix, checksum) +} + +func splitURLOnDoubleSlash(u *url.URL) (string, string) { + x := strings.Split(u.Path, "//") + switch len(x) { + case 0: + return "", "" + case 1: + return x[0], "" + default: + return x[0], x[1] } - return fmt.Sprintf("%s.%s", prefix, checksum) } diff --git a/taskfile/node_git_test.go b/taskfile/node_git_test.go index 1b88a083a9..c359ae81be 100644 --- a/taskfile/node_git_test.go +++ b/taskfile/node_git_test.go @@ -4,6 +4,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestGitNode_ssh(t *testing.T) { @@ -13,8 +14,8 @@ func TestGitNode_ssh(t *testing.T) { assert.NoError(t, err) assert.Equal(t, "main", node.ref) assert.Equal(t, "Taskfile.yml", node.path) - assert.Equal(t, "ssh://git@github.com/foo/bar.git//Taskfile.yml?ref=main", node.rawUrl) - assert.Equal(t, "ssh://git@github.com/foo/bar.git", node.URL.String()) + assert.Equal(t, "ssh://git@github.com/foo/bar.git//Taskfile.yml?ref=main", node.Location()) + assert.Equal(t, "ssh://git@github.com/foo/bar.git", node.url.String()) entrypoint, err := node.ResolveEntrypoint("common.yml") assert.NoError(t, err) assert.Equal(t, "ssh://git@github.com/foo/bar.git//common.yml?ref=main", entrypoint) @@ -27,8 +28,8 @@ func TestGitNode_sshWithDir(t *testing.T) { assert.NoError(t, err) assert.Equal(t, "main", node.ref) assert.Equal(t, "directory/Taskfile.yml", node.path) - assert.Equal(t, "ssh://git@github.com/foo/bar.git//directory/Taskfile.yml?ref=main", node.rawUrl) - assert.Equal(t, "ssh://git@github.com/foo/bar.git", node.URL.String()) + assert.Equal(t, "ssh://git@github.com/foo/bar.git//directory/Taskfile.yml?ref=main", node.Location()) + assert.Equal(t, "ssh://git@github.com/foo/bar.git", node.url.String()) entrypoint, err := node.ResolveEntrypoint("common.yml") assert.NoError(t, err) assert.Equal(t, "ssh://git@github.com/foo/bar.git//directory/common.yml?ref=main", entrypoint) @@ -41,8 +42,8 @@ func TestGitNode_https(t *testing.T) { assert.NoError(t, err) assert.Equal(t, "main", node.ref) assert.Equal(t, "Taskfile.yml", node.path) - assert.Equal(t, "https://github.com/foo/bar.git//Taskfile.yml?ref=main", node.rawUrl) - assert.Equal(t, "https://github.com/foo/bar.git", node.URL.String()) + assert.Equal(t, "https://github.com/foo/bar.git//Taskfile.yml?ref=main", node.Location()) + assert.Equal(t, "https://github.com/foo/bar.git", node.url.String()) entrypoint, err := node.ResolveEntrypoint("common.yml") assert.NoError(t, err) assert.Equal(t, "https://github.com/foo/bar.git//common.yml?ref=main", entrypoint) @@ -55,8 +56,8 @@ func TestGitNode_httpsWithDir(t *testing.T) { assert.NoError(t, err) assert.Equal(t, "main", node.ref) assert.Equal(t, "directory/Taskfile.yml", node.path) - assert.Equal(t, "https://github.com/foo/bar.git//directory/Taskfile.yml?ref=main", node.rawUrl) - assert.Equal(t, "https://github.com/foo/bar.git", node.URL.String()) + assert.Equal(t, "https://github.com/foo/bar.git//directory/Taskfile.yml?ref=main", node.Location()) + assert.Equal(t, "https://github.com/foo/bar.git", node.url.String()) entrypoint, err := node.ResolveEntrypoint("common.yml") assert.NoError(t, err) assert.Equal(t, "https://github.com/foo/bar.git//directory/common.yml?ref=main", entrypoint) @@ -65,18 +66,28 @@ func TestGitNode_httpsWithDir(t *testing.T) { func TestGitNode_CacheKey(t *testing.T) { t.Parallel() - node, err := NewGitNode("https://github.com/foo/bar.git//directory/Taskfile.yml?ref=main", "", false) - assert.NoError(t, err) - key := node.CacheKey() - assert.Equal(t, "Taskfile.yml-directory.f1ddddac425a538870230a3e38fc0cded4ec5da250797b6cab62c82477718fbb", key) + tests := []struct { + entrypoint string + expectedKey string + }{ + { + entrypoint: "https://github.com/foo/bar.git//directory/Taskfile.yml?ref=main", + expectedKey: "git.github.com.directory.Taskfile.yml.f1ddddac425a538870230a3e38fc0cded4ec5da250797b6cab62c82477718fbb", + }, + { + entrypoint: "https://github.com/foo/bar.git//Taskfile.yml?ref=main", + expectedKey: "git.github.com.Taskfile.yml.39d28c1ff36f973705ae188b991258bbabaffd6d60bcdde9693d157d00d5e3a4", + }, + { + entrypoint: "https://github.com/foo/bar.git//multiple/directory/Taskfile.yml?ref=main", + expectedKey: "git.github.com.directory.Taskfile.yml.1b6d145e01406dcc6c0aa572e5a5d1333be1ccf2cae96d18296d725d86197d31", + }, + } - node, err = NewGitNode("https://github.com/foo/bar.git//Taskfile.yml?ref=main", "", false) - assert.NoError(t, err) - key = node.CacheKey() - assert.Equal(t, "Taskfile.yml-..39d28c1ff36f973705ae188b991258bbabaffd6d60bcdde9693d157d00d5e3a4", key) - - node, err = NewGitNode("https://github.com/foo/bar.git//multiple/directory/Taskfile.yml?ref=main", "", false) - assert.NoError(t, err) - key = node.CacheKey() - assert.Equal(t, "Taskfile.yml-directory.1b6d145e01406dcc6c0aa572e5a5d1333be1ccf2cae96d18296d725d86197d31", key) + for _, tt := range tests { + node, err := NewGitNode(tt.entrypoint, "", false) + require.NoError(t, err) + key := node.CacheKey() + assert.Equal(t, tt.expectedKey, key) + } } diff --git a/taskfile/node_http.go b/taskfile/node_http.go index 16e0ee40c9..faa6616db4 100644 --- a/taskfile/node_http.go +++ b/taskfile/node_http.go @@ -16,9 +16,8 @@ import ( // An HTTPNode is a node that reads a Taskfile from a remote location via HTTP. type HTTPNode struct { - *BaseNode - URL *url.URL // stores url pointing actual remote file. (e.g. with Taskfile.yml) - entrypoint string // stores entrypoint url. used for building graph vertices. + *baseNode + url *url.URL // stores url pointing actual remote file. (e.g. with Taskfile.yml) } func NewHTTPNode( @@ -33,18 +32,16 @@ func NewHTTPNode( return nil, err } if url.Scheme == "http" && !insecure { - return nil, &errors.TaskfileNotSecureError{URI: entrypoint} + return nil, &errors.TaskfileNotSecureError{URI: url.Redacted()} } - return &HTTPNode{ - BaseNode: base, - URL: url, - entrypoint: entrypoint, + baseNode: base, + url: url, }, nil } func (node *HTTPNode) Location() string { - return node.entrypoint + return node.url.Redacted() } func (node *HTTPNode) Read() ([]byte, error) { @@ -52,14 +49,13 @@ func (node *HTTPNode) Read() ([]byte, error) { } func (node *HTTPNode) ReadContext(ctx context.Context) ([]byte, error) { - url, err := RemoteExists(ctx, node.URL) + url, err := RemoteExists(ctx, *node.url) if err != nil { return nil, err } - node.URL = url - req, err := http.NewRequest("GET", node.URL.String(), nil) + req, err := http.NewRequest("GET", url.String(), nil) if err != nil { - return nil, errors.TaskfileFetchFailedError{URI: node.URL.String()} + return nil, errors.TaskfileFetchFailedError{URI: node.Location()} } resp, err := http.DefaultClient.Do(req.WithContext(ctx)) @@ -67,12 +63,12 @@ func (node *HTTPNode) ReadContext(ctx context.Context) ([]byte, error) { if ctx.Err() != nil { return nil, err } - return nil, errors.TaskfileFetchFailedError{URI: node.URL.String()} + return nil, errors.TaskfileFetchFailedError{URI: node.Location()} } defer resp.Body.Close() if resp.StatusCode != http.StatusOK { return nil, errors.TaskfileFetchFailedError{ - URI: node.URL.String(), + URI: node.Location(), HTTPStatusCode: resp.StatusCode, } } @@ -91,7 +87,7 @@ func (node *HTTPNode) ResolveEntrypoint(entrypoint string) (string, error) { if err != nil { return "", err } - return node.URL.ResolveReference(ref).String(), nil + return node.url.ResolveReference(ref).String(), nil } func (node *HTTPNode) ResolveDir(dir string) (string, error) { @@ -116,12 +112,12 @@ func (node *HTTPNode) ResolveDir(dir string) (string, error) { func (node *HTTPNode) CacheKey() string { checksum := strings.TrimRight(checksum([]byte(node.Location())), "=") - dir, filename := filepath.Split(node.entrypoint) + dir, filename := filepath.Split(node.url.Path) lastDir := filepath.Base(dir) prefix := filename // Means it's not "", nor "." nor "/", so it's a valid directory if len(lastDir) > 1 { - prefix = fmt.Sprintf("%s-%s", lastDir, filename) + prefix = fmt.Sprintf("%s.%s", lastDir, filename) } - return fmt.Sprintf("%s.%s", prefix, checksum) + return fmt.Sprintf("http.%s.%s.%s", node.url.Host, prefix, checksum) } diff --git a/taskfile/node_http_test.go b/taskfile/node_http_test.go new file mode 100644 index 0000000000..ade7c905b9 --- /dev/null +++ b/taskfile/node_http_test.go @@ -0,0 +1,49 @@ +package taskfile + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestHTTPNode_CacheKey(t *testing.T) { + t.Parallel() + + tests := []struct { + entrypoint string + expectedKey string + }{ + { + entrypoint: "https://github.com", + expectedKey: "http.github.com..996e1f714b08e971ec79e3bea686287e66441f043177999a13dbc546d8fe402a", + }, + { + entrypoint: "https://github.com/Taskfile.yml", + expectedKey: "http.github.com.Taskfile.yml.85b3c3ad71b78dc74e404c7b4390fc13672925cb644a4d26c21b9f97c17b5fc0", + }, + { + entrypoint: "https://github.com/foo", + expectedKey: "http.github.com.foo.df3158dafc823e6847d9bcaf79328446c4877405e79b100723fa6fd545ed3e2b", + }, + { + entrypoint: "https://github.com/foo/Taskfile.yml", + expectedKey: "http.github.com.foo.Taskfile.yml.aea946ea7eb6f6bb4e159e8b840b6b50975927778b2e666df988c03bbf10c4c4", + }, + { + entrypoint: "https://github.com/foo/bar", + expectedKey: "http.github.com.foo.bar.d3514ad1d4daedf9cc2825225070b49ebc8db47fa5177951b2a5b9994597570c", + }, + { + entrypoint: "https://github.com/foo/bar/Taskfile.yml", + expectedKey: "http.github.com.bar.Taskfile.yml.b9cf01e01e47c0e96ea536e1a8bd7b3a6f6c1f1881bad438990d2bfd4ccd0ac0", + }, + } + + for _, tt := range tests { + node, err := NewHTTPNode(tt.entrypoint, "", false) + require.NoError(t, err) + key := node.CacheKey() + assert.Equal(t, tt.expectedKey, key) + } +} diff --git a/taskfile/node_stdin.go b/taskfile/node_stdin.go index 387f50fe83..b09a779538 100644 --- a/taskfile/node_stdin.go +++ b/taskfile/node_stdin.go @@ -12,12 +12,12 @@ import ( // A StdinNode is a node that reads a taskfile from the standard input stream. type StdinNode struct { - *BaseNode + *baseNode } func NewStdinNode(dir string) (*StdinNode, error) { return &StdinNode{ - BaseNode: NewBaseNode(dir), + baseNode: NewBaseNode(dir), }, nil } diff --git a/taskfile/taskfile.go b/taskfile/taskfile.go index d08e74ec6a..e209444acc 100644 --- a/taskfile/taskfile.go +++ b/taskfile/taskfile.go @@ -36,11 +36,11 @@ var ( // at the given URL with any of the default Taskfile files names. If any of // these match a file, the first matching path will be returned. If no files are // found, an error will be returned. -func RemoteExists(ctx context.Context, u *url.URL) (*url.URL, error) { +func RemoteExists(ctx context.Context, u url.URL) (*url.URL, error) { // Create a new HEAD request for the given URL to check if the resource exists req, err := http.NewRequestWithContext(ctx, "HEAD", u.String(), nil) if err != nil { - return nil, errors.TaskfileFetchFailedError{URI: u.String()} + return nil, errors.TaskfileFetchFailedError{URI: u.Redacted()} } // Request the given URL @@ -49,7 +49,7 @@ func RemoteExists(ctx context.Context, u *url.URL) (*url.URL, error) { if ctx.Err() != nil { return nil, fmt.Errorf("checking remote file: %w", ctx.Err()) } - return nil, errors.TaskfileFetchFailedError{URI: u.String()} + return nil, errors.TaskfileFetchFailedError{URI: u.Redacted()} } defer resp.Body.Close() @@ -61,7 +61,7 @@ func RemoteExists(ctx context.Context, u *url.URL) (*url.URL, error) { if resp.StatusCode == http.StatusOK && slices.ContainsFunc(allowedContentTypes, func(s string) bool { return strings.Contains(contentType, s) }) { - return u, nil + return &u, nil } // If the request was not successful, append the default Taskfile names to @@ -78,7 +78,7 @@ func RemoteExists(ctx context.Context, u *url.URL) (*url.URL, error) { // Try the alternative URL resp, err = http.DefaultClient.Do(req) if err != nil { - return nil, errors.TaskfileFetchFailedError{URI: u.String()} + return nil, errors.TaskfileFetchFailedError{URI: u.Redacted()} } defer resp.Body.Close() @@ -88,5 +88,5 @@ func RemoteExists(ctx context.Context, u *url.URL) (*url.URL, error) { } } - return nil, errors.TaskfileNotFoundError{URI: u.String(), Walk: false} + return nil, errors.TaskfileNotFoundError{URI: u.Redacted(), Walk: false} }