From b8565dc0fde5b232cab6a858acbce62074578c86 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Mon, 22 Apr 2024 14:31:38 +0200 Subject: [PATCH 1/6] feat: locate devcontainer.json in multiple places --- envbuilder.go | 63 ++++++++++++++++++++++++--------- integration/integration_test.go | 24 +++++++++++++ 2 files changed, 71 insertions(+), 16 deletions(-) diff --git a/envbuilder.go b/envbuilder.go index 7ae8936f..3ac79123 100644 --- a/envbuilder.go +++ b/envbuilder.go @@ -118,8 +118,8 @@ type Options struct { // DevcontainerDir is a path to the folder containing // the devcontainer.json file that will be used to build the // workspace and can either be an absolute path or a path - // relative to the workspace folder. If not provided, defaults to - // `.devcontainer`. + // relative to the workspace folder. + // If not provided, it defaults to `.devcontainer`. DevcontainerDir string `env:"DEVCONTAINER_DIR"` // DevcontainerJSONPath is a path to a devcontainer.json file @@ -127,6 +127,11 @@ type Options struct { // DevcontainerDir. This can be used in cases where one wants // to substitute an edited devcontainer.json file for the one // that exists in the repo. + // If neither `DevcontainerDir` nor `DevcontainerJSONPath` is provided, + // envbuilder will browse following directories to locate it: + // 1. `.devcontainer/devcontainer.json` + // 2. `.devcontainer.json` + // 3. `.devcontainer//devcontainer.json` DevcontainerJSONPath string `env:"DEVCONTAINER_JSON_PATH"` // DockerfilePath is a relative path to the Dockerfile that @@ -422,20 +427,7 @@ func Run(ctx context.Context, options Options) error { if options.DockerfilePath == "" { // Only look for a devcontainer if a Dockerfile wasn't specified. // devcontainer is a standard, so it's reasonable to be the default. - devcontainerDir := options.DevcontainerDir - if devcontainerDir == "" { - devcontainerDir = ".devcontainer" - } - if !filepath.IsAbs(devcontainerDir) { - devcontainerDir = filepath.Join(options.WorkspaceFolder, devcontainerDir) - } - devcontainerPath := options.DevcontainerJSONPath - if devcontainerPath == "" { - devcontainerPath = "devcontainer.json" - } - if !filepath.IsAbs(devcontainerPath) { - devcontainerPath = filepath.Join(devcontainerDir, devcontainerPath) - } + devcontainerPath, devcontainerDir := findDevcontainerJSON(options) _, err := options.Filesystem.Stat(devcontainerPath) if err == nil { // We know a devcontainer exists. @@ -1201,3 +1193,42 @@ type osfsWithChmod struct { func (fs *osfsWithChmod) Chmod(name string, mode os.FileMode) error { return os.Chmod(name, mode) } + +func findDevcontainerJSON(options Options) (string, string) { + // 0. Check provided options + if options.DevcontainerDir != "" || options.DevcontainerJSONPath != "" { + // Locate .devcontainer directory. + devcontainerDir := options.DevcontainerDir + if devcontainerDir == "" { + devcontainerDir = ".devcontainer" + } + // If `devcontainerDir` is not an absolute path, assume it is relative to the workspace folder. + if !filepath.IsAbs(devcontainerDir) { + devcontainerDir = filepath.Join(options.WorkspaceFolder, devcontainerDir) + } + + // Locate devcontainer.json manifest. + devcontainerPath := options.DevcontainerJSONPath + + // An absolute location always takes a precedence. + if filepath.IsAbs(devcontainerPath) { + return options.DevcontainerJSONPath, devcontainerDir + } + + // If an override is not provided, assume it is just `devcontainer.json`. + if devcontainerPath == "" { + devcontainerPath = "devcontainer.json" + } + + if !filepath.IsAbs(devcontainerPath) { + devcontainerPath = filepath.Join(devcontainerDir, devcontainerPath) + } + return devcontainerDir, devcontainerPath + } + + // 1. Check `options.WorkspaceFolder`/.devcontainer/devcontainer.json + // 2. Check `options.WorkspaceFolder`/devcontainer.json + // 3. Check every folder: `options.WorkspaceFolder`//devcontainer.json + + panic("not implemented yet") +} diff --git a/integration/integration_test.go b/integration/integration_test.go index 2bab8e0c..84c364d3 100644 --- a/integration/integration_test.go +++ b/integration/integration_test.go @@ -273,6 +273,30 @@ func TestBuildFromDevcontainerInCustomPath(t *testing.T) { require.Equal(t, "hello", strings.TrimSpace(output)) } +func TestBuildFromDevcontainerInRoot(t *testing.T) { + t.Parallel() + + // Ensures that a Git repository with a devcontainer.json is cloned and built. + url := createGitServer(t, gitServerOptions{ + files: map[string]string{ + "devcontainer.json": `{ + "name": "Test", + "build": { + "dockerfile": "Dockerfile" + }, + }`, + "Dockerfile": "FROM ubuntu", + }, + }) + ctr, err := runEnvbuilder(t, options{env: []string{ + "GIT_URL=" + url, + }}) + require.NoError(t, err) + + output := execContainer(t, ctr, "echo hello") + require.Equal(t, "hello", strings.TrimSpace(output)) +} + func TestBuildCustomCertificates(t *testing.T) { srv := httptest.NewTLSServer(createGitHandler(t, gitServerOptions{ files: map[string]string{ From c273fefba0333883e5c333ac7d3a96472bb0fed5 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Mon, 22 Apr 2024 14:46:22 +0200 Subject: [PATCH 2/6] two options --- envbuilder.go | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/envbuilder.go b/envbuilder.go index 3ac79123..12fcc52e 100644 --- a/envbuilder.go +++ b/envbuilder.go @@ -1195,7 +1195,7 @@ func (fs *osfsWithChmod) Chmod(name string, mode os.FileMode) error { } func findDevcontainerJSON(options Options) (string, string) { - // 0. Check provided options + // 0. Check provided options. if options.DevcontainerDir != "" || options.DevcontainerJSONPath != "" { // Locate .devcontainer directory. devcontainerDir := options.DevcontainerDir @@ -1226,9 +1226,21 @@ func findDevcontainerJSON(options Options) (string, string) { return devcontainerDir, devcontainerPath } - // 1. Check `options.WorkspaceFolder`/.devcontainer/devcontainer.json - // 2. Check `options.WorkspaceFolder`/devcontainer.json - // 3. Check every folder: `options.WorkspaceFolder`//devcontainer.json + // 1. Check `options.WorkspaceFolder`/.devcontainer/devcontainer.json. + location := filepath.Join(options.WorkspaceFolder, ".devcontainer", "devcontainer.json") + _, err := options.Filesystem.Stat(location) + if err == nil { + return filepath.Dir(location), location + } + + // 2. Check `options.WorkspaceFolder`/devcontainer.json. + location = filepath.Join(options.WorkspaceFolder, "devcontainer.json") + _, err = options.Filesystem.Stat(location) + if err == nil { + return filepath.Dir(location), location + } + + // 3. Check every folder: `options.WorkspaceFolder`//devcontainer.json. panic("not implemented yet") } From 34c4a0080ff857ff640db1063721049e2aa34ce2 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Mon, 22 Apr 2024 14:51:00 +0200 Subject: [PATCH 3/6] tdd subfolder --- envbuilder.go | 6 +++--- integration/integration_test.go | 23 +++++++++++++++++++++++ 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/envbuilder.go b/envbuilder.go index 12fcc52e..459c57f0 100644 --- a/envbuilder.go +++ b/envbuilder.go @@ -1223,21 +1223,21 @@ func findDevcontainerJSON(options Options) (string, string) { if !filepath.IsAbs(devcontainerPath) { devcontainerPath = filepath.Join(devcontainerDir, devcontainerPath) } - return devcontainerDir, devcontainerPath + return devcontainerPath, devcontainerDir } // 1. Check `options.WorkspaceFolder`/.devcontainer/devcontainer.json. location := filepath.Join(options.WorkspaceFolder, ".devcontainer", "devcontainer.json") _, err := options.Filesystem.Stat(location) if err == nil { - return filepath.Dir(location), location + return location, filepath.Dir(location) } // 2. Check `options.WorkspaceFolder`/devcontainer.json. location = filepath.Join(options.WorkspaceFolder, "devcontainer.json") _, err = options.Filesystem.Stat(location) if err == nil { - return filepath.Dir(location), location + return location, filepath.Dir(location) } // 3. Check every folder: `options.WorkspaceFolder`//devcontainer.json. diff --git a/integration/integration_test.go b/integration/integration_test.go index 84c364d3..d1f0ba3a 100644 --- a/integration/integration_test.go +++ b/integration/integration_test.go @@ -273,6 +273,29 @@ func TestBuildFromDevcontainerInCustomPath(t *testing.T) { require.Equal(t, "hello", strings.TrimSpace(output)) } +func TestBuildFromDevcontainerInSubfolder(t *testing.T) { + t.Parallel() + + // Ensures that a Git repository with a devcontainer.json is cloned and built. + url := createGitServer(t, gitServerOptions{ + files: map[string]string{ + "./devcontainer/subfolder/devcontainer.json": `{ + "name": "Test", + "build": { + "dockerfile": "Dockerfile" + }, + }`, + "Dockerfile": "FROM ubuntu", + }, + }) + ctr, err := runEnvbuilder(t, options{env: []string{ + "GIT_URL=" + url, + }}) + require.NoError(t, err) + + output := execContainer(t, ctr, "echo hello") + require.Equal(t, "hello", strings.TrimSpace(output)) +} func TestBuildFromDevcontainerInRoot(t *testing.T) { t.Parallel() From 6cde3b29ba75970bebbc4fd4edd3ca3e42184785 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Mon, 22 Apr 2024 15:48:44 +0200 Subject: [PATCH 4/6] Finish? --- envbuilder.go | 45 +++++++++++++++++++++++++-------- integration/integration_test.go | 4 +-- 2 files changed, 37 insertions(+), 12 deletions(-) diff --git a/envbuilder.go b/envbuilder.go index 459c57f0..4f858f67 100644 --- a/envbuilder.go +++ b/envbuilder.go @@ -427,9 +427,11 @@ func Run(ctx context.Context, options Options) error { if options.DockerfilePath == "" { // Only look for a devcontainer if a Dockerfile wasn't specified. // devcontainer is a standard, so it's reasonable to be the default. - devcontainerPath, devcontainerDir := findDevcontainerJSON(options) - _, err := options.Filesystem.Stat(devcontainerPath) - if err == nil { + devcontainerPath, devcontainerDir, err := findDevcontainerJSON(options) + if err != nil { + logf(codersdk.LogLevelError, "Failed to locate devcontainer.json: %s", err.Error()) + logf(codersdk.LogLevelError, "Falling back to the default image...") + } else { // We know a devcontainer exists. // Let's parse it and use it! file, err := options.Filesystem.Open(devcontainerPath) @@ -1194,7 +1196,7 @@ func (fs *osfsWithChmod) Chmod(name string, mode os.FileMode) error { return os.Chmod(name, mode) } -func findDevcontainerJSON(options Options) (string, string) { +func findDevcontainerJSON(options Options) (string, string, error) { // 0. Check provided options. if options.DevcontainerDir != "" || options.DevcontainerJSONPath != "" { // Locate .devcontainer directory. @@ -1212,7 +1214,7 @@ func findDevcontainerJSON(options Options) (string, string) { // An absolute location always takes a precedence. if filepath.IsAbs(devcontainerPath) { - return options.DevcontainerJSONPath, devcontainerDir + return options.DevcontainerJSONPath, devcontainerDir, nil } // If an override is not provided, assume it is just `devcontainer.json`. @@ -1223,24 +1225,47 @@ func findDevcontainerJSON(options Options) (string, string) { if !filepath.IsAbs(devcontainerPath) { devcontainerPath = filepath.Join(devcontainerDir, devcontainerPath) } - return devcontainerPath, devcontainerDir + return devcontainerPath, devcontainerDir, nil } // 1. Check `options.WorkspaceFolder`/.devcontainer/devcontainer.json. location := filepath.Join(options.WorkspaceFolder, ".devcontainer", "devcontainer.json") _, err := options.Filesystem.Stat(location) if err == nil { - return location, filepath.Dir(location) + return location, filepath.Dir(location), nil } // 2. Check `options.WorkspaceFolder`/devcontainer.json. location = filepath.Join(options.WorkspaceFolder, "devcontainer.json") _, err = options.Filesystem.Stat(location) if err == nil { - return location, filepath.Dir(location) + return location, filepath.Dir(location), nil } - // 3. Check every folder: `options.WorkspaceFolder`//devcontainer.json. + // 3. Check every folder: `options.WorkspaceFolder`/.devcontainer//devcontainer.json. + devcontainerDir := filepath.Join(options.WorkspaceFolder, ".devcontainer") + + fileInfos, err := options.Filesystem.ReadDir(devcontainerDir) + if err != nil { + return "", "", err + } + + logf := options.Logger + for _, fileInfo := range fileInfos { + if !fileInfo.IsDir() { + logf(codersdk.LogLevelDebug, `%s is a file`, fileInfo.Name()) + continue + } + + location := filepath.Join(devcontainerDir, fileInfo.Name(), "devcontainer.json") + _, err := options.Filesystem.Stat(location) + if err != nil { + logf(codersdk.LogLevelDebug, `stat %s failed: %s`, location, err.Error()) + continue + } + + return location, filepath.Dir(location), nil + } - panic("not implemented yet") + return "", "", errors.New("can't find devcontainer.json, is it a correct spec?") } diff --git a/integration/integration_test.go b/integration/integration_test.go index d1f0ba3a..53439025 100644 --- a/integration/integration_test.go +++ b/integration/integration_test.go @@ -279,13 +279,13 @@ func TestBuildFromDevcontainerInSubfolder(t *testing.T) { // Ensures that a Git repository with a devcontainer.json is cloned and built. url := createGitServer(t, gitServerOptions{ files: map[string]string{ - "./devcontainer/subfolder/devcontainer.json": `{ + ".devcontainer/subfolder/devcontainer.json": `{ "name": "Test", "build": { "dockerfile": "Dockerfile" }, }`, - "Dockerfile": "FROM ubuntu", + ".devcontainer/subfolder/Dockerfile": "FROM ubuntu", }, }) ctr, err := runEnvbuilder(t, options{env: []string{ From 4352446fca7ee2ba8f450781f158ca56659e350e Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Mon, 22 Apr 2024 16:11:31 +0200 Subject: [PATCH 5/6] fix --- envbuilder.go | 21 +++++++-------------- 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/envbuilder.go b/envbuilder.go index 4f858f67..5a85f306 100644 --- a/envbuilder.go +++ b/envbuilder.go @@ -118,8 +118,8 @@ type Options struct { // DevcontainerDir is a path to the folder containing // the devcontainer.json file that will be used to build the // workspace and can either be an absolute path or a path - // relative to the workspace folder. - // If not provided, it defaults to `.devcontainer`. + // relative to the workspace folder. If not provided, defaults to + // `.devcontainer`. DevcontainerDir string `env:"DEVCONTAINER_DIR"` // DevcontainerJSONPath is a path to a devcontainer.json file @@ -1197,9 +1197,8 @@ func (fs *osfsWithChmod) Chmod(name string, mode os.FileMode) error { } func findDevcontainerJSON(options Options) (string, string, error) { - // 0. Check provided options. + // 0. Check if custom devcontainer directory or path is provided. if options.DevcontainerDir != "" || options.DevcontainerJSONPath != "" { - // Locate .devcontainer directory. devcontainerDir := options.DevcontainerDir if devcontainerDir == "" { devcontainerDir = ".devcontainer" @@ -1209,14 +1208,11 @@ func findDevcontainerJSON(options Options) (string, string, error) { devcontainerDir = filepath.Join(options.WorkspaceFolder, devcontainerDir) } - // Locate devcontainer.json manifest. - devcontainerPath := options.DevcontainerJSONPath - // An absolute location always takes a precedence. + devcontainerPath := options.DevcontainerJSONPath if filepath.IsAbs(devcontainerPath) { return options.DevcontainerJSONPath, devcontainerDir, nil } - // If an override is not provided, assume it is just `devcontainer.json`. if devcontainerPath == "" { devcontainerPath = "devcontainer.json" @@ -1230,15 +1226,13 @@ func findDevcontainerJSON(options Options) (string, string, error) { // 1. Check `options.WorkspaceFolder`/.devcontainer/devcontainer.json. location := filepath.Join(options.WorkspaceFolder, ".devcontainer", "devcontainer.json") - _, err := options.Filesystem.Stat(location) - if err == nil { + if _, err := options.Filesystem.Stat(location); err == nil { return location, filepath.Dir(location), nil } // 2. Check `options.WorkspaceFolder`/devcontainer.json. location = filepath.Join(options.WorkspaceFolder, "devcontainer.json") - _, err = options.Filesystem.Stat(location) - if err == nil { + if _, err := options.Filesystem.Stat(location); err == nil { return location, filepath.Dir(location), nil } @@ -1258,8 +1252,7 @@ func findDevcontainerJSON(options Options) (string, string, error) { } location := filepath.Join(devcontainerDir, fileInfo.Name(), "devcontainer.json") - _, err := options.Filesystem.Stat(location) - if err != nil { + if _, err := options.Filesystem.Stat(location); err != nil { logf(codersdk.LogLevelDebug, `stat %s failed: %s`, location, err.Error()) continue } From d97bf2095df7021872881da8645570de5d5ca7b2 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Tue, 23 Apr 2024 11:00:33 +0200 Subject: [PATCH 6/6] internal test for findDevcontainersJSON --- envbuilder_internal_test.go | 154 ++++++++++++++++++++++++++++++++++++ 1 file changed, 154 insertions(+) create mode 100644 envbuilder_internal_test.go diff --git a/envbuilder_internal_test.go b/envbuilder_internal_test.go new file mode 100644 index 00000000..d9fd3cb9 --- /dev/null +++ b/envbuilder_internal_test.go @@ -0,0 +1,154 @@ +package envbuilder + +import ( + "testing" + + "github.com/go-git/go-billy/v5/memfs" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestFindDevcontainerJSON(t *testing.T) { + t.Parallel() + + t.Run("empty filesystem", func(t *testing.T) { + t.Parallel() + + // given + fs := memfs.New() + + // when + _, _, err := findDevcontainerJSON(Options{ + Filesystem: fs, + WorkspaceFolder: "/workspace", + }) + + // then + require.Error(t, err) + }) + + t.Run("devcontainers.json is missing", func(t *testing.T) { + t.Parallel() + + // given + fs := memfs.New() + err := fs.MkdirAll("/workspace/.devcontainer", 0600) + require.NoError(t, err) + + // when + _, _, err = findDevcontainerJSON(Options{ + Filesystem: fs, + WorkspaceFolder: "/workspace", + }) + + // then + require.Error(t, err) + }) + + t.Run("default configuration", func(t *testing.T) { + t.Parallel() + + // given + fs := memfs.New() + err := fs.MkdirAll("/workspace/.devcontainer", 0600) + require.NoError(t, err) + fs.Create("/workspace/.devcontainer/devcontainer.json") + + // when + devcontainerPath, devcontainerDir, err := findDevcontainerJSON(Options{ + Filesystem: fs, + WorkspaceFolder: "/workspace", + }) + + // then + require.NoError(t, err) + assert.Equal(t, "/workspace/.devcontainer/devcontainer.json", devcontainerPath) + assert.Equal(t, "/workspace/.devcontainer", devcontainerDir) + }) + + t.Run("overridden .devcontainer directory", func(t *testing.T) { + t.Parallel() + + // given + fs := memfs.New() + err := fs.MkdirAll("/workspace/experimental-devcontainer", 0600) + require.NoError(t, err) + fs.Create("/workspace/experimental-devcontainer/devcontainer.json") + + // when + devcontainerPath, devcontainerDir, err := findDevcontainerJSON(Options{ + Filesystem: fs, + WorkspaceFolder: "/workspace", + DevcontainerDir: "experimental-devcontainer", + }) + + // then + require.NoError(t, err) + assert.Equal(t, "/workspace/experimental-devcontainer/devcontainer.json", devcontainerPath) + assert.Equal(t, "/workspace/experimental-devcontainer", devcontainerDir) + }) + + t.Run("overridden devcontainer.json path", func(t *testing.T) { + t.Parallel() + + // given + fs := memfs.New() + err := fs.MkdirAll("/workspace/.devcontainer", 0600) + require.NoError(t, err) + fs.Create("/workspace/.devcontainer/experimental.json") + + // when + devcontainerPath, devcontainerDir, err := findDevcontainerJSON(Options{ + Filesystem: fs, + WorkspaceFolder: "/workspace", + DevcontainerJSONPath: "experimental.json", + }) + + // then + require.NoError(t, err) + assert.Equal(t, "/workspace/.devcontainer/experimental.json", devcontainerPath) + assert.Equal(t, "/workspace/.devcontainer", devcontainerDir) + }) + + t.Run("devcontainer.json in workspace root", func(t *testing.T) { + t.Parallel() + + // given + fs := memfs.New() + err := fs.MkdirAll("/workspace", 0600) + require.NoError(t, err) + fs.Create("/workspace/devcontainer.json") + + // when + devcontainerPath, devcontainerDir, err := findDevcontainerJSON(Options{ + Filesystem: fs, + WorkspaceFolder: "/workspace", + }) + + // then + require.NoError(t, err) + assert.Equal(t, "/workspace/devcontainer.json", devcontainerPath) + assert.Equal(t, "/workspace", devcontainerDir) + }) + + t.Run("devcontainer.json in subfolder of .devcontainer", func(t *testing.T) { + t.Parallel() + + // given + fs := memfs.New() + err := fs.MkdirAll("/workspace/.devcontainer/sample", 0600) + require.NoError(t, err) + fs.Create("/workspace/.devcontainer/sample/devcontainer.json") + + // when + devcontainerPath, devcontainerDir, err := findDevcontainerJSON(Options{ + Filesystem: fs, + WorkspaceFolder: "/workspace", + }) + + // then + require.NoError(t, err) + assert.Equal(t, "/workspace/.devcontainer/sample/devcontainer.json", devcontainerPath) + assert.Equal(t, "/workspace/.devcontainer/sample", devcontainerDir) + }) +}