Repository: coder/envbuilder
Pull request #283 (2024-07-26 14:16:51)

chore: extract option defaults to options package
Part of https://github.com/coder/team-coconut/issues/11

Builds on top of https://github.com/coder/envbuilder/pull/282

More refactoring:
- Moves DefaultWorkspaceFolder to options package
- Extracts the option defaults logic to the options package
- DefaultWorkspaceFolder now returns EmptyWorkspaceDir if GitURL is invalid, meaning it no longer returns an error
- Extracts osfsWithChmod to its own package



Patch:
diff --git a/envbuilder.go b/envbuilder.go
index cafc4fc..2a00c84 100644
--- a/envbuilder.go
+++ b/envbuilder.go
@@ -32,7 +32,6 @@ import (
 	"github.com/GoogleContainerTools/kaniko/pkg/creds"
 	"github.com/GoogleContainerTools/kaniko/pkg/executor"
 	"github.com/GoogleContainerTools/kaniko/pkg/util"
-	giturls "github.com/chainguard-dev/git-urls"
 	"github.com/coder/envbuilder/devcontainer"
 	"github.com/coder/envbuilder/internal/ebutil"
 	"github.com/coder/envbuilder/internal/log"
@@ -42,8 +41,6 @@ import (
 	_ "github.com/distribution/distribution/v3/registry/storage/driver/filesystem"
 	"github.com/docker/cli/cli/config/configfile"
 	"github.com/fatih/color"
-	"github.com/go-git/go-billy/v5"
-	"github.com/go-git/go-billy/v5/osfs"
 	"github.com/go-git/go-git/v5/plumbing/transport"
 	v1 "github.com/google/go-containerregistry/pkg/v1"
 	"github.com/google/go-containerregistry/pkg/v1/remote"
@@ -62,23 +59,8 @@ type DockerConfig configfile.ConfigFile
 // Filesystem is the filesystem to use for all operations.
 // Defaults to the host filesystem.
 func Run(ctx context.Context, opts options.Options) error {
-	// Temporarily removed these from the default settings to prevent conflicts
-	// between current and legacy environment variables that add default values.
-	// Once the legacy environment variables are phased out, this can be
-	// reinstated to the previous default values.
-	if len(opts.IgnorePaths) == 0 {
-		opts.IgnorePaths = []string{
-			"/var/run",
-			// KinD adds these paths to pods, so ignore them by default.
-			"/product_uuid", "/product_name",
-		}
-	}
-	if opts.InitScript == "" {
-		opts.InitScript = "sleep infinity"
-	}
-	if opts.InitCommand == "" {
-		opts.InitCommand = "/bin/sh"
-	}
+	opts.SetDefaults()
+
 	if opts.CacheRepo == "" && opts.PushImage {
 		return fmt.Errorf("--cache-repo must be set when using --push-image")
 	}
@@ -91,16 +73,6 @@ func Run(ctx context.Context, opts options.Options) error {
 			return fmt.Errorf("parse init args: %w", err)
 		}
 	}
-	if opts.Filesystem == nil {
-		opts.Filesystem = &osfsWithChmod{osfs.New("/")}
-	}
-	if opts.WorkspaceFolder == "" {
-		f, err := DefaultWorkspaceFolder(opts.GitURL)
-		if err != nil {
-			return err
-		}
-		opts.WorkspaceFolder = f
-	}
 
 	stageNumber := 0
 	startStage := func(format string, args ...any) func(format string, args ...any) {
@@ -930,25 +902,6 @@ ENTRYPOINT [%q]`, exePath, exePath, exePath)
 	return nil
 }
 
-// DefaultWorkspaceFolder returns the default workspace folder
-// for a given repository URL.
-func DefaultWorkspaceFolder(repoURL string) (string, error) {
-	if repoURL == "" {
-		return constants.EmptyWorkspaceDir, nil
-	}
-	parsed, err := giturls.Parse(repoURL)
-	if err != nil {
-		return "", err
-	}
-	name := strings.Split(parsed.Path, "/")
-	hasOwnerAndRepo := len(name) >= 2
-	if !hasOwnerAndRepo {
-		return constants.EmptyWorkspaceDir, nil
-	}
-	repo := strings.TrimSuffix(name[len(name)-1], ".git")
-	return fmt.Sprintf("/workspaces/%s", repo), nil
-}
-
 type userInfo struct {
 	uid  int
 	gid  int
@@ -1077,14 +1030,6 @@ func newColor(value ...color.Attribute) *color.Color {
 	return c
 }
 
-type osfsWithChmod struct {
-	billy.Filesystem
-}
-
-func (fs *osfsWithChmod) Chmod(name string, mode os.FileMode) error {
-	return os.Chmod(name, mode)
-}
-
 func findDevcontainerJSON(options options.Options) (string, string, error) {
 	// 0. Check if custom devcontainer directory or path is provided.
 	if options.DevcontainerDir != "" || options.DevcontainerJSONPath != "" {
diff --git a/internal/chmodfs/chmodfs.go b/internal/chmodfs/chmodfs.go
new file mode 100644
index 0000000..1242417
--- /dev/null
+++ b/internal/chmodfs/chmodfs.go
@@ -0,0 +1,21 @@
+package chmodfs
+
+import (
+	"os"
+
+	"github.com/go-git/go-billy/v5"
+)
+
+func New(fs billy.Filesystem) billy.Filesystem {
+	return &osfsWithChmod{
+		Filesystem: fs,
+	}
+}
+
+type osfsWithChmod struct {
+	billy.Filesystem
+}
+
+func (fs *osfsWithChmod) Chmod(name string, mode os.FileMode) error {
+	return os.Chmod(name, mode)
+}
diff --git a/options/defaults.go b/options/defaults.go
new file mode 100644
index 0000000..18bf12a
--- /dev/null
+++ b/options/defaults.go
@@ -0,0 +1,58 @@
+package options
+
+import (
+	"fmt"
+	"strings"
+
+	"github.com/go-git/go-billy/v5/osfs"
+
+	giturls "github.com/chainguard-dev/git-urls"
+	"github.com/coder/envbuilder/constants"
+	"github.com/coder/envbuilder/internal/chmodfs"
+)
+
+// DefaultWorkspaceFolder returns the default workspace folder
+// for a given repository URL.
+func DefaultWorkspaceFolder(repoURL string) string {
+	if repoURL == "" {
+		return constants.EmptyWorkspaceDir
+	}
+	parsed, err := giturls.Parse(repoURL)
+	if err != nil {
+		return constants.EmptyWorkspaceDir
+	}
+	name := strings.Split(parsed.Path, "/")
+	hasOwnerAndRepo := len(name) >= 2
+	if !hasOwnerAndRepo {
+		return constants.EmptyWorkspaceDir
+	}
+	repo := strings.TrimSuffix(name[len(name)-1], ".git")
+	return fmt.Sprintf("/workspaces/%s", repo)
+}
+
+func (o *Options) SetDefaults() {
+	// Temporarily removed these from the default settings to prevent conflicts
+	// between current and legacy environment variables that add default values.
+	// Once the legacy environment variables are phased out, this can be
+	// reinstated to the previous default values.
+	if len(o.IgnorePaths) == 0 {
+		o.IgnorePaths = []string{
+			"/var/run",
+			// KinD adds these paths to pods, so ignore them by default.
+			"/product_uuid", "/product_name",
+		}
+	}
+	if o.InitScript == "" {
+		o.InitScript = "sleep infinity"
+	}
+	if o.InitCommand == "" {
+		o.InitCommand = "/bin/sh"
+	}
+
+	if o.Filesystem == nil {
+		o.Filesystem = chmodfs.New(osfs.New("/"))
+	}
+	if o.WorkspaceFolder == "" {
+		o.WorkspaceFolder = DefaultWorkspaceFolder(o.GitURL)
+	}
+}


Test patch:
diff --git a/envbuilder_test.go b/envbuilder_test.go
index 0545bfc..aa4205c 100644
--- a/envbuilder_test.go
+++ b/envbuilder_test.go
@@ -1,74 +1,1 @@
 package envbuilder_test
-
-import (
-	"testing"
-
-	"github.com/coder/envbuilder"
-	"github.com/coder/envbuilder/constants"
-
-	"github.com/stretchr/testify/require"
-)
-
-func TestDefaultWorkspaceFolder(t *testing.T) {
-	t.Parallel()
-
-	successTests := []struct {
-		name     string
-		gitURL   string
-		expected string
-	}{
-		{
-			name:     "HTTP",
-			gitURL:   "https://github.com/coder/envbuilder.git",
-			expected: "/workspaces/envbuilder",
-		},
-		{
-			name:     "SSH",
-			gitURL:   "git@github.com:coder/envbuilder.git",
-			expected: "/workspaces/envbuilder",
-		},
-		{
-			name:     "username and password",
-			gitURL:   "https://username:password@github.com/coder/envbuilder.git",
-			expected: "/workspaces/envbuilder",
-		},
-		{
-			name:     "fragment",
-			gitURL:   "https://github.com/coder/envbuilder.git#feature-branch",
-			expected: "/workspaces/envbuilder",
-		},
-		{
-			name:     "empty",
-			gitURL:   "",
-			expected: constants.EmptyWorkspaceDir,
-		},
-	}
-	for _, tt := range successTests {
-		t.Run(tt.name, func(t *testing.T) {
-			dir, err := envbuilder.DefaultWorkspaceFolder(tt.gitURL)
-			require.NoError(t, err)
-			require.Equal(t, tt.expected, dir)
-		})
-	}
-
-	invalidTests := []struct {
-		name       string
-		invalidURL string
-	}{
-		{
-			name:       "simple text",
-			invalidURL: "not a valid URL",
-		},
-		{
-			name:       "website URL",
-			invalidURL: "www.google.com",
-		},
-	}
-	for _, tt := range invalidTests {
-		t.Run(tt.name, func(t *testing.T) {
-			dir, err := envbuilder.DefaultWorkspaceFolder(tt.invalidURL)
-			require.NoError(t, err)
-			require.Equal(t, constants.EmptyWorkspaceDir, dir)
-		})
-	}
-}
diff --git a/options/defaults_test.go b/options/defaults_test.go
new file mode 100644
index 0000000..156ae16
--- /dev/null
+++ b/options/defaults_test.go
@@ -0,0 +1,93 @@
+package options_test
+
+import (
+	"testing"
+
+	"github.com/coder/envbuilder/internal/chmodfs"
+	"github.com/go-git/go-billy/v5/osfs"
+
+	"github.com/stretchr/testify/assert"
+
+	"github.com/coder/envbuilder/constants"
+	"github.com/coder/envbuilder/options"
+	"github.com/stretchr/testify/require"
+)
+
+func TestDefaultWorkspaceFolder(t *testing.T) {
+	t.Parallel()
+
+	successTests := []struct {
+		name     string
+		gitURL   string
+		expected string
+	}{
+		{
+			name:     "HTTP",
+			gitURL:   "https://github.com/coder/envbuilder.git",
+			expected: "/workspaces/envbuilder",
+		},
+		{
+			name:     "SSH",
+			gitURL:   "git@github.com:coder/envbuilder.git",
+			expected: "/workspaces/envbuilder",
+		},
+		{
+			name:     "username and password",
+			gitURL:   "https://username:password@github.com/coder/envbuilder.git",
+			expected: "/workspaces/envbuilder",
+		},
+		{
+			name:     "fragment",
+			gitURL:   "https://github.com/coder/envbuilder.git#feature-branch",
+			expected: "/workspaces/envbuilder",
+		},
+		{
+			name:     "empty",
+			gitURL:   "",
+			expected: constants.EmptyWorkspaceDir,
+		},
+	}
+	for _, tt := range successTests {
+		t.Run(tt.name, func(t *testing.T) {
+			dir := options.DefaultWorkspaceFolder(tt.gitURL)
+			require.Equal(t, tt.expected, dir)
+		})
+	}
+
+	invalidTests := []struct {
+		name       string
+		invalidURL string
+	}{
+		{
+			name:       "simple text",
+			invalidURL: "not a valid URL",
+		},
+		{
+			name:       "website URL",
+			invalidURL: "www.google.com",
+		},
+	}
+	for _, tt := range invalidTests {
+		t.Run(tt.name, func(t *testing.T) {
+			dir := options.DefaultWorkspaceFolder(tt.invalidURL)
+			require.Equal(t, constants.EmptyWorkspaceDir, dir)
+		})
+	}
+}
+
+func TestOptions_SetDefaults(t *testing.T) {
+	t.Parallel()
+
+	expected := options.Options{
+		InitScript:      "sleep infinity",
+		InitCommand:     "/bin/sh",
+		IgnorePaths:     []string{"/var/run", "/product_uuid", "/product_name"},
+		Filesystem:      chmodfs.New(osfs.New("/")),
+		GitURL:          "",
+		WorkspaceFolder: constants.EmptyWorkspaceDir,
+	}
+
+	var actual options.Options
+	actual.SetDefaults()
+	assert.Equal(t, expected, actual)
+}