Skip to content

Commit 25d70ce

Browse files
DanielleMaywoodDanielleMaywood
authored
fix(agent/agentcontainers): respect ignore files (#19016)
Closes #19011 We now use [go-git](https://pkg.go.dev/github.com/go-git/go-git/v5@v5.16.2/plumbing/format/gitignore)'s `gitignore` plumbing implementation to parse the `.gitignore` files and match against the patterns generated. We use this to ignore any ignored files in the git repository. Unfortunately I've had to slightly re-implement some of the interface exposed by `go-git` because they use `billy.Filesystem` instead of `afero.Fs`.
1 parent 5c1bf1d commit 25d70ce

File tree

6 files changed

+323
-7
lines changed

6 files changed

+323
-7
lines changed

‎agent/agentcontainers/api.go

Lines changed: 41 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,13 @@ import (
2121

2222
"github.com/fsnotify/fsnotify"
2323
"github.com/go-chi/chi/v5"
24+
"github.com/go-git/go-git/v5/plumbing/format/gitignore"
2425
"github.com/google/uuid"
2526
"github.com/spf13/afero"
2627
"golang.org/x/xerrors"
2728

2829
"cdr.dev/slog"
30+
"github.com/coder/coder/v2/agent/agentcontainers/ignore"
2931
"github.com/coder/coder/v2/agent/agentcontainers/watcher"
3032
"github.com/coder/coder/v2/agent/agentexec"
3133
"github.com/coder/coder/v2/agent/usershell"
@@ -469,13 +471,49 @@ func (api *API) discoverDevcontainerProjects() error {
469471
}
470472

471473
func (api *API) discoverDevcontainersInProject(projectPath string) error {
474+
logger := api.logger.
475+
Named("project-discovery").
476+
With(slog.F("project_path", projectPath))
477+
478+
globalPatterns, err := ignore.LoadGlobalPatterns(api.fs)
479+
if err != nil {
480+
return xerrors.Errorf("read global git ignore patterns: %w", err)
481+
}
482+
483+
patterns, err := ignore.ReadPatterns(api.ctx, logger, api.fs, projectPath)
484+
if err != nil {
485+
return xerrors.Errorf("read git ignore patterns: %w", err)
486+
}
487+
488+
matcher := gitignore.NewMatcher(append(globalPatterns, patterns...))
489+
472490
devcontainerConfigPaths := []string{
473491
"/.devcontainer/devcontainer.json",
474492
"/.devcontainer.json",
475493
}
476494

477-
return afero.Walk(api.fs, projectPath, func(path string, info fs.FileInfo, _ error) error {
495+
return afero.Walk(api.fs, projectPath, func(path string, info fs.FileInfo, err error) error {
496+
if err != nil {
497+
logger.Error(api.ctx, "encountered error while walking for dev container projects",
498+
slog.F("path", path),
499+
slog.Error(err))
500+
return nil
501+
}
502+
503+
pathParts := ignore.FilePathToParts(path)
504+
505+
// We know that a directory entry cannot be a `devcontainer.json` file, so we
506+
// always skip processing directories. If the directory happens to be ignored
507+
// by git then we'll make sure to ignore all of the children of that directory.
478508
if info.IsDir() {
509+
if matcher.Match(pathParts, true) {
510+
return fs.SkipDir
511+
}
512+
513+
return nil
514+
}
515+
516+
if matcher.Match(pathParts, false) {
479517
return nil
480518
}
481519

@@ -486,11 +524,11 @@ func (api *API) discoverDevcontainersInProject(projectPath string) error {
486524

487525
workspaceFolder := strings.TrimSuffix(path, relativeConfigPath)
488526

489-
api.logger.Debug(api.ctx, "discovered dev container project", slog.F("workspace_folder", workspaceFolder))
527+
logger.Debug(api.ctx, "discovered dev container project", slog.F("workspace_folder", workspaceFolder))
490528

491529
api.mu.Lock()
492530
if _, found := api.knownDevcontainers[workspaceFolder]; !found {
493-
api.logger.Debug(api.ctx, "adding dev container project", slog.F("workspace_folder", workspaceFolder))
531+
logger.Debug(api.ctx, "adding dev container project", slog.F("workspace_folder", workspaceFolder))
494532

495533
dc := codersdk.WorkspaceAgentDevcontainer{
496534
ID: uuid.New(),

‎agent/agentcontainers/api_test.go

Lines changed: 112 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"net/http/httptest"
1010
"os"
1111
"os/exec"
12+
"path/filepath"
1213
"runtime"
1314
"slices"
1415
"strings"
@@ -3211,6 +3212,9 @@ func TestDevcontainerDiscovery(t *testing.T) {
32113212
// repositories to find any `.devcontainer/devcontainer.json`
32123213
// files. These tests are to validate that behavior.
32133214

3215+
homeDir, err := os.UserHomeDir()
3216+
require.NoError(t, err)
3217+
32143218
tests := []struct {
32153219
name string
32163220
agentDir string
@@ -3345,6 +3349,113 @@ func TestDevcontainerDiscovery(t *testing.T) {
33453349
},
33463350
},
33473351
},
3352+
{
3353+
name: "RespectGitIgnore",
3354+
agentDir: "/home/coder",
3355+
fs: map[string]string{
3356+
"/home/coder/coder/.git/HEAD": "",
3357+
"/home/coder/coder/.gitignore": "y/",
3358+
"/home/coder/coder/.devcontainer.json": "",
3359+
"/home/coder/coder/x/y/.devcontainer.json": "",
3360+
},
3361+
expected: []codersdk.WorkspaceAgentDevcontainer{
3362+
{
3363+
WorkspaceFolder: "/home/coder/coder",
3364+
ConfigPath: "/home/coder/coder/.devcontainer.json",
3365+
Status: codersdk.WorkspaceAgentDevcontainerStatusStopped,
3366+
},
3367+
},
3368+
},
3369+
{
3370+
name: "RespectNestedGitIgnore",
3371+
agentDir: "/home/coder",
3372+
fs: map[string]string{
3373+
"/home/coder/coder/.git/HEAD": "",
3374+
"/home/coder/coder/.devcontainer.json": "",
3375+
"/home/coder/coder/y/.devcontainer.json": "",
3376+
"/home/coder/coder/x/.gitignore": "y/",
3377+
"/home/coder/coder/x/y/.devcontainer.json": "",
3378+
},
3379+
expected: []codersdk.WorkspaceAgentDevcontainer{
3380+
{
3381+
WorkspaceFolder: "/home/coder/coder",
3382+
ConfigPath: "/home/coder/coder/.devcontainer.json",
3383+
Status: codersdk.WorkspaceAgentDevcontainerStatusStopped,
3384+
},
3385+
{
3386+
WorkspaceFolder: "/home/coder/coder/y",
3387+
ConfigPath: "/home/coder/coder/y/.devcontainer.json",
3388+
Status: codersdk.WorkspaceAgentDevcontainerStatusStopped,
3389+
},
3390+
},
3391+
},
3392+
{
3393+
name: "RespectGitInfoExclude",
3394+
agentDir: "/home/coder",
3395+
fs: map[string]string{
3396+
"/home/coder/coder/.git/HEAD": "",
3397+
"/home/coder/coder/.git/info/exclude": "y/",
3398+
"/home/coder/coder/.devcontainer.json": "",
3399+
"/home/coder/coder/x/y/.devcontainer.json": "",
3400+
},
3401+
expected: []codersdk.WorkspaceAgentDevcontainer{
3402+
{
3403+
WorkspaceFolder: "/home/coder/coder",
3404+
ConfigPath: "/home/coder/coder/.devcontainer.json",
3405+
Status: codersdk.WorkspaceAgentDevcontainerStatusStopped,
3406+
},
3407+
},
3408+
},
3409+
{
3410+
name: "RespectHomeGitConfig",
3411+
agentDir: homeDir,
3412+
fs: map[string]string{
3413+
"/tmp/.gitignore": "node_modules/",
3414+
filepath.Join(homeDir, ".gitconfig"): `
3415+
[core]
3416+
excludesFile = /tmp/.gitignore
3417+
`,
3418+
3419+
filepath.Join(homeDir, ".git/HEAD"): "",
3420+
filepath.Join(homeDir, ".devcontainer.json"): "",
3421+
filepath.Join(homeDir, "node_modules/y/.devcontainer.json"): "",
3422+
},
3423+
expected: []codersdk.WorkspaceAgentDevcontainer{
3424+
{
3425+
WorkspaceFolder: homeDir,
3426+
ConfigPath: filepath.Join(homeDir, ".devcontainer.json"),
3427+
Status: codersdk.WorkspaceAgentDevcontainerStatusStopped,
3428+
},
3429+
},
3430+
},
3431+
{
3432+
name: "IgnoreNonsenseDevcontainerNames",
3433+
agentDir: "/home/coder",
3434+
fs: map[string]string{
3435+
"/home/coder/.git/HEAD": "",
3436+
3437+
"/home/coder/.devcontainer/devcontainer.json.bak": "",
3438+
"/home/coder/.devcontainer/devcontainer.json.old": "",
3439+
"/home/coder/.devcontainer/devcontainer.json~": "",
3440+
"/home/coder/.devcontainer/notdevcontainer.json": "",
3441+
"/home/coder/.devcontainer/devcontainer.json.swp": "",
3442+
3443+
"/home/coder/foo/.devcontainer.json.bak": "",
3444+
"/home/coder/foo/.devcontainer.json.old": "",
3445+
"/home/coder/foo/.devcontainer.json~": "",
3446+
"/home/coder/foo/.notdevcontainer.json": "",
3447+
"/home/coder/foo/.devcontainer.json.swp": "",
3448+
3449+
"/home/coder/bar/.devcontainer.json": "",
3450+
},
3451+
expected: []codersdk.WorkspaceAgentDevcontainer{
3452+
{
3453+
WorkspaceFolder: "/home/coder/bar",
3454+
ConfigPath: "/home/coder/bar/.devcontainer.json",
3455+
Status: codersdk.WorkspaceAgentDevcontainerStatusStopped,
3456+
},
3457+
},
3458+
},
33483459
}
33493460

33503461
initFS := func(t *testing.T, files map[string]string) afero.Fs {
@@ -3397,7 +3508,7 @@ func TestDevcontainerDiscovery(t *testing.T) {
33973508
err := json.NewDecoder(rec.Body).Decode(&got)
33983509
require.NoError(t, err)
33993510

3400-
return len(got.Devcontainers) == len(tt.expected)
3511+
return len(got.Devcontainers) >= len(tt.expected)
34013512
}, testutil.WaitShort, testutil.IntervalFast, "dev containers never found")
34023513

34033514
// Now projects have been discovered, we'll allow the updater loop

‎agent/agentcontainers/ignore/dir.go

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,124 @@
1+
package ignore
2+
3+
import (
4+
"bytes"
5+
"context"
6+
"errors"
7+
"io/fs"
8+
"os"
9+
"path/filepath"
10+
"strings"
11+
12+
"github.com/go-git/go-git/v5/plumbing/format/config"
13+
"github.com/go-git/go-git/v5/plumbing/format/gitignore"
14+
"github.com/spf13/afero"
15+
"golang.org/x/xerrors"
16+
17+
"cdr.dev/slog"
18+
)
19+
20+
const (
21+
gitconfigFile = ".gitconfig"
22+
gitignoreFile = ".gitignore"
23+
gitInfoExcludeFile = ".git/info/exclude"
24+
)
25+
26+
func FilePathToParts(path string) []string {
27+
components := []string{}
28+
29+
if path == "" {
30+
return components
31+
}
32+
33+
for segment := range strings.SplitSeq(filepath.Clean(path), string(filepath.Separator)) {
34+
if segment != "" {
35+
components = append(components, segment)
36+
}
37+
}
38+
39+
return components
40+
}
41+
42+
func readIgnoreFile(fileSystem afero.Fs, path, ignore string) ([]gitignore.Pattern, error) {
43+
var ps []gitignore.Pattern
44+
45+
data, err := afero.ReadFile(fileSystem, filepath.Join(path, ignore))
46+
if err != nil && !errors.Is(err, os.ErrNotExist) {
47+
return nil, err
48+
}
49+
50+
for s := range strings.SplitSeq(string(data), "\n") {
51+
if !strings.HasPrefix(s, "#") && len(strings.TrimSpace(s)) > 0 {
52+
ps = append(ps, gitignore.ParsePattern(s, FilePathToParts(path)))
53+
}
54+
}
55+
56+
return ps, nil
57+
}
58+
59+
func ReadPatterns(ctx context.Context, logger slog.Logger, fileSystem afero.Fs, path string) ([]gitignore.Pattern, error) {
60+
var ps []gitignore.Pattern
61+
62+
subPs, err := readIgnoreFile(fileSystem, path, gitInfoExcludeFile)
63+
if err != nil {
64+
return nil, err
65+
}
66+
67+
ps = append(ps, subPs...)
68+
69+
if err := afero.Walk(fileSystem, path, func(path string, info fs.FileInfo, err error) error {
70+
if err != nil {
71+
logger.Error(ctx, "encountered error while walking for git ignore files",
72+
slog.F("path", path),
73+
slog.Error(err))
74+
return nil
75+
}
76+
77+
if !info.IsDir() {
78+
return nil
79+
}
80+
81+
subPs, err := readIgnoreFile(fileSystem, path, gitignoreFile)
82+
if err != nil {
83+
return err
84+
}
85+
86+
ps = append(ps, subPs...)
87+
88+
return nil
89+
}); err != nil {
90+
return nil, err
91+
}
92+
93+
return ps, nil
94+
}
95+
96+
func loadPatterns(fileSystem afero.Fs, path string) ([]gitignore.Pattern, error) {
97+
data, err := afero.ReadFile(fileSystem, path)
98+
if err != nil && !errors.Is(err, os.ErrNotExist) {
99+
return nil, err
100+
}
101+
102+
decoder := config.NewDecoder(bytes.NewBuffer(data))
103+
104+
conf := config.New()
105+
if err := decoder.Decode(conf); err != nil {
106+
return nil, xerrors.Errorf("decode config: %w", err)
107+
}
108+
109+
excludes := conf.Section("core").Options.Get("excludesfile")
110+
if excludes == "" {
111+
return nil, nil
112+
}
113+
114+
return readIgnoreFile(fileSystem, "", excludes)
115+
}
116+
117+
func LoadGlobalPatterns(fileSystem afero.Fs) ([]gitignore.Pattern, error) {
118+
home, err := os.UserHomeDir()
119+
if err != nil {
120+
return nil, err
121+
}
122+
123+
return loadPatterns(fileSystem, filepath.Join(home, gitconfigFile))
124+
}

‎agent/agentcontainers/ignore/dir_test.go

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
package ignore_test
2+
3+
import (
4+
"fmt"
5+
"testing"
6+
7+
"github.com/stretchr/testify/require"
8+
9+
"github.com/coder/coder/v2/agent/agentcontainers/ignore"
10+
)
11+
12+
func TestFilePathToParts(t *testing.T) {
13+
t.Parallel()
14+
15+
tests := []struct {
16+
path string
17+
expected []string
18+
}{
19+
{"", []string{}},
20+
{"/", []string{}},
21+
{"foo", []string{"foo"}},
22+
{"/foo", []string{"foo"}},
23+
{"./foo/bar", []string{"foo", "bar"}},
24+
{"../foo/bar", []string{"..", "foo", "bar"}},
25+
{"foo/bar/baz", []string{"foo", "bar", "baz"}},
26+
{"/foo/bar/baz", []string{"foo", "bar", "baz"}},
27+
{"foo/../bar", []string{"bar"}},
28+
}
29+
30+
for _, tt := range tests {
31+
t.Run(fmt.Sprintf("`%s`", tt.path), func(t *testing.T) {
32+
t.Parallel()
33+
34+
parts := ignore.FilePathToParts(tt.path)
35+
require.Equal(t, tt.expected, parts)
36+
})
37+
}
38+
}

0 commit comments

Comments
 (0)