From 2270f59173378acd0e464043302e49c5397fdfc6 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Fri, 26 Jul 2024 13:53:00 +0100 Subject: [PATCH 1/3] move DefaultWorkspaceFolder to options package Signed-off-by: Cian Johnston --- envbuilder.go | 26 +---------------- envbuilder_test.go | 73 ---------------------------------------------- 2 files changed, 1 insertion(+), 98 deletions(-) diff --git a/envbuilder.go b/envbuilder.go index cafc4fcd..6115ea99 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" @@ -95,11 +94,7 @@ func Run(ctx context.Context, opts options.Options) error { opts.Filesystem = &osfsWithChmod{osfs.New("/")} } if opts.WorkspaceFolder == "" { - f, err := DefaultWorkspaceFolder(opts.GitURL) - if err != nil { - return err - } - opts.WorkspaceFolder = f + opts.WorkspaceFolder = options.DefaultWorkspaceFolder(opts.GitURL) } stageNumber := 0 @@ -930,25 +925,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 diff --git a/envbuilder_test.go b/envbuilder_test.go index 0545bfce..aa4205c7 100644 --- a/envbuilder_test.go +++ b/envbuilder_test.go @@ -1,74 +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) - }) - } -} From 47331e6a1a4a64519efc075b53c1a7518cb30bd6 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Fri, 26 Jul 2024 13:55:07 +0100 Subject: [PATCH 2/3] extract osfsWithChmod to its own package Signed-off-by: Cian Johnston --- envbuilder.go | 12 ++---------- internal/chmodfs/chmodfs.go | 21 +++++++++++++++++++++ 2 files changed, 23 insertions(+), 10 deletions(-) create mode 100644 internal/chmodfs/chmodfs.go diff --git a/envbuilder.go b/envbuilder.go index 6115ea99..59d14874 100644 --- a/envbuilder.go +++ b/envbuilder.go @@ -26,6 +26,7 @@ import ( "github.com/coder/envbuilder/constants" "github.com/coder/envbuilder/git" + "github.com/coder/envbuilder/internal/chmodfs" "github.com/coder/envbuilder/options" "github.com/GoogleContainerTools/kaniko/pkg/config" @@ -41,7 +42,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" @@ -91,7 +91,7 @@ func Run(ctx context.Context, opts options.Options) error { } } if opts.Filesystem == nil { - opts.Filesystem = &osfsWithChmod{osfs.New("/")} + opts.Filesystem = chmodfs.New(osfs.New("/")) } if opts.WorkspaceFolder == "" { opts.WorkspaceFolder = options.DefaultWorkspaceFolder(opts.GitURL) @@ -1053,14 +1053,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 00000000..1242417a --- /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) +} From fd6ef176a2313d72bd0d9d1d311bef89a5ede194 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Fri, 26 Jul 2024 14:52:44 +0100 Subject: [PATCH 3/3] extract option defaults logic Signed-off-by: Cian Johnston --- envbuilder.go | 27 +----------- options/defaults.go | 58 +++++++++++++++++++++++++ options/defaults_test.go | 93 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 153 insertions(+), 25 deletions(-) create mode 100644 options/defaults.go create mode 100644 options/defaults_test.go diff --git a/envbuilder.go b/envbuilder.go index 59d14874..2a00c84c 100644 --- a/envbuilder.go +++ b/envbuilder.go @@ -26,7 +26,6 @@ import ( "github.com/coder/envbuilder/constants" "github.com/coder/envbuilder/git" - "github.com/coder/envbuilder/internal/chmodfs" "github.com/coder/envbuilder/options" "github.com/GoogleContainerTools/kaniko/pkg/config" @@ -42,7 +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/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" @@ -61,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") } @@ -90,12 +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 = chmodfs.New(osfs.New("/")) - } - if opts.WorkspaceFolder == "" { - opts.WorkspaceFolder = options.DefaultWorkspaceFolder(opts.GitURL) - } stageNumber := 0 startStage := func(format string, args ...any) func(format string, args ...any) { diff --git a/options/defaults.go b/options/defaults.go new file mode 100644 index 00000000..18bf12a8 --- /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) + } +} diff --git a/options/defaults_test.go b/options/defaults_test.go new file mode 100644 index 00000000..156ae16b --- /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) +}