Skip to content

chore: extract constants package #282

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 29, 2024
Merged

chore: extract constants package #282

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions constants/constants.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
package constants
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two alternatives, config and defaults. Not sure if/which would be better.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I personally dislike both of those options 😅 but I don't have a better proposal.


import (
"errors"
"path/filepath"
)

const (
// WorkspacesDir is the path to the directory where
// all workspaces are stored by default.
WorkspacesDir = "/workspaces"

// EmptyWorkspaceDir is the path to a workspace that has
// nothing going on... it's empty!
EmptyWorkspaceDir = WorkspacesDir + "/empty"

// MagicDir is where all envbuilder related files are stored.
// This is a special directory that must not be modified
// by the user or images.
MagicDir = "/.envbuilder"
)

var (
ErrNoFallbackImage = errors.New("no fallback image has been specified")

// MagicFile is a file that is created in the workspace
// when envbuilder has already been run. This is used
// to skip building when a container is restarting.
// e.g. docker stop -> docker start
MagicFile = filepath.Join(MagicDir, "built")
)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do feel like this and options belong in the same package, how about merging them under core or something? Not sure it's better description wise but still. Ideally I'd love for these and options to be in the package named envbuilder since that makes the most sense. But then we either lose the separation, or need to move the Run method elsewhere.

I suppose a build package could be appropriate for those, but then we'd kinda revert some of the splits you've already been doing.

58 changes: 17 additions & 41 deletions envbuilder.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"syscall"
"time"

"github.com/coder/envbuilder/constants"
"github.com/coder/envbuilder/git"
"github.com/coder/envbuilder/options"

Expand Down Expand Up @@ -53,31 +54,6 @@ import (
"golang.org/x/xerrors"
)

const (
// WorkspacesDir is the path to the directory where
// all workspaces are stored by default.
WorkspacesDir = "/workspaces"

// EmptyWorkspaceDir is the path to a workspace that has
// nothing going on... it's empty!
EmptyWorkspaceDir = WorkspacesDir + "/empty"

// MagicDir is where all envbuilder related files are stored.
// This is a special directory that must not be modified
// by the user or images.
MagicDir = "/.envbuilder"
)

var (
ErrNoFallbackImage = errors.New("no fallback image has been specified")

// MagicFile is a file that is created in the workspace
// when envbuilder has already been run. This is used
// to skip building when a container is restarting.
// e.g. docker stop -> docker start
MagicFile = filepath.Join(MagicDir, "built")
)

// DockerConfig represents the Docker configuration file.
type DockerConfig configfile.ConfigFile

Expand Down Expand Up @@ -171,7 +147,7 @@ func Run(ctx context.Context, opts options.Options) error {
if err != nil {
return fmt.Errorf("parse docker config: %w", err)
}
err = os.WriteFile(filepath.Join(MagicDir, "config.json"), decoded, 0o644)
err = os.WriteFile(filepath.Join(constants.MagicDir, "config.json"), decoded, 0o644)
if err != nil {
return fmt.Errorf("write docker config: %w", err)
}
Expand Down Expand Up @@ -237,19 +213,19 @@ func Run(ctx context.Context, opts options.Options) error {
}

defaultBuildParams := func() (*devcontainer.Compiled, error) {
dockerfile := filepath.Join(MagicDir, "Dockerfile")
dockerfile := filepath.Join(constants.MagicDir, "Dockerfile")
file, err := opts.Filesystem.OpenFile(dockerfile, os.O_CREATE|os.O_WRONLY, 0o644)
if err != nil {
return nil, err
}
defer file.Close()
if opts.FallbackImage == "" {
if fallbackErr != nil {
return nil, xerrors.Errorf("%s: %w", fallbackErr.Error(), ErrNoFallbackImage)
return nil, xerrors.Errorf("%s: %w", fallbackErr.Error(), constants.ErrNoFallbackImage)
}
// We can't use errors.Join here because our tests
// don't support parsing a multiline error.
return nil, ErrNoFallbackImage
return nil, constants.ErrNoFallbackImage
}
content := "FROM " + opts.FallbackImage
_, err = file.Write([]byte(content))
Expand All @@ -259,7 +235,7 @@ func Run(ctx context.Context, opts options.Options) error {
return &devcontainer.Compiled{
DockerfilePath: dockerfile,
DockerfileContent: content,
BuildContext: MagicDir,
BuildContext: constants.MagicDir,
}, nil
}

Expand Down Expand Up @@ -301,7 +277,7 @@ func Run(ctx context.Context, opts options.Options) error {
opts.Logger(log.LevelInfo, "No Dockerfile or image specified; falling back to the default image...")
fallbackDockerfile = defaultParams.DockerfilePath
}
buildParams, err = devContainer.Compile(opts.Filesystem, devcontainerDir, MagicDir, fallbackDockerfile, opts.WorkspaceFolder, false, os.LookupEnv)
buildParams, err = devContainer.Compile(opts.Filesystem, devcontainerDir, constants.MagicDir, fallbackDockerfile, opts.WorkspaceFolder, false, os.LookupEnv)
if err != nil {
return fmt.Errorf("compile devcontainer.json: %w", err)
}
Expand Down Expand Up @@ -399,7 +375,7 @@ func Run(ctx context.Context, opts options.Options) error {
// So we add them to the default ignore list. See:
// https://github.com/GoogleContainerTools/kaniko/blob/63be4990ca5a60bdf06ddc4d10aa4eca0c0bc714/cmd/executor/cmd/root.go#L136
ignorePaths := append([]string{
MagicDir,
constants.MagicDir,
opts.WorkspaceFolder,
// See: https://github.com/coder/envbuilder/issues/37
"/etc/resolv.conf",
Expand Down Expand Up @@ -441,7 +417,7 @@ ENTRYPOINT [%q]`, exePath, exePath, exePath)
}

// temp move of all ro mounts
tempRemountDest := filepath.Join("/", MagicDir, "mnt")
tempRemountDest := filepath.Join("/", constants.MagicDir, "mnt")
// ignorePrefixes is a superset of ignorePaths that we pass to kaniko's
// IgnoreList.
ignorePrefixes := append([]string{"/dev", "/proc", "/sys"}, ignorePaths...)
Expand All @@ -457,7 +433,7 @@ ENTRYPOINT [%q]`, exePath, exePath, exePath)

skippedRebuild := false
build := func() (v1.Image, error) {
_, err := opts.Filesystem.Stat(MagicFile)
_, err := opts.Filesystem.Stat(constants.MagicFile)
if err == nil && opts.SkipRebuild {
endStage := startStage("🏗️ Skipping build because of cache...")
imageRef, err := devcontainer.ImageFromDockerfile(buildParams.DockerfileContent)
Expand Down Expand Up @@ -640,7 +616,7 @@ ENTRYPOINT [%q]`, exePath, exePath, exePath)

// Create the magic file to indicate that this build
// has already been ran before!
file, err := opts.Filesystem.Create(MagicFile)
file, err := opts.Filesystem.Create(constants.MagicFile)
if err != nil {
return fmt.Errorf("create magic file: %w", err)
}
Expand Down Expand Up @@ -695,7 +671,7 @@ ENTRYPOINT [%q]`, exePath, exePath, exePath)

// Remove the Docker config secret file!
if opts.DockerConfigBase64 != "" {
c := filepath.Join(MagicDir, "config.json")
c := filepath.Join(constants.MagicDir, "config.json")
err = os.Remove(c)
if err != nil {
if !errors.Is(err, fs.ErrNotExist) {
Expand Down Expand Up @@ -855,7 +831,7 @@ ENTRYPOINT [%q]`, exePath, exePath, exePath)
opts.Logger(log.LevelInfo, "=== Running the setup command %q as the root user...", opts.SetupScript)

envKey := "ENVBUILDER_ENV"
envFile := filepath.Join("/", MagicDir, "environ")
envFile := filepath.Join("/", constants.MagicDir, "environ")
file, err := os.Create(envFile)
if err != nil {
return fmt.Errorf("create environ file: %w", err)
Expand Down Expand Up @@ -958,7 +934,7 @@ ENTRYPOINT [%q]`, exePath, exePath, exePath)
// for a given repository URL.
func DefaultWorkspaceFolder(repoURL string) (string, error) {
if repoURL == "" {
return EmptyWorkspaceDir, nil
return constants.EmptyWorkspaceDir, nil
}
parsed, err := giturls.Parse(repoURL)
if err != nil {
Expand All @@ -967,7 +943,7 @@ func DefaultWorkspaceFolder(repoURL string) (string, error) {
name := strings.Split(parsed.Path, "/")
hasOwnerAndRepo := len(name) >= 2
if !hasOwnerAndRepo {
return EmptyWorkspaceDir, nil
return constants.EmptyWorkspaceDir, nil
}
repo := strings.TrimSuffix(name[len(name)-1], ".git")
return fmt.Sprintf("/workspaces/%s", repo), nil
Expand Down Expand Up @@ -1180,7 +1156,7 @@ func findDevcontainerJSON(options options.Options) (string, string, error) {
// folks from unwittingly deleting their entire root directory.
func maybeDeleteFilesystem(logger log.Func, force bool) error {
kanikoDir, ok := os.LookupEnv("KANIKO_DIR")
if !ok || strings.TrimSpace(kanikoDir) != MagicDir {
if !ok || strings.TrimSpace(kanikoDir) != constants.MagicDir {
if force {
bailoutSecs := 10
logger(log.LevelWarn, "WARNING! BYPASSING SAFETY CHECK! THIS WILL DELETE YOUR ROOT FILESYSTEM!")
Expand All @@ -1190,7 +1166,7 @@ func maybeDeleteFilesystem(logger log.Func, force bool) error {
<-time.After(time.Second)
}
} else {
logger(log.LevelError, "KANIKO_DIR is not set to %s. Bailing!\n", MagicDir)
logger(log.LevelError, "KANIKO_DIR is not set to %s. Bailing!\n", constants.MagicDir)
logger(log.LevelError, "To bypass this check, set FORCE_SAFE=true.")
return errors.New("safety check failed")
}
Expand Down
6 changes: 4 additions & 2 deletions envbuilder_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import (
"testing"

"github.com/coder/envbuilder"
"github.com/coder/envbuilder/constants"

"github.com/stretchr/testify/require"
)

Expand Down Expand Up @@ -38,7 +40,7 @@ func TestDefaultWorkspaceFolder(t *testing.T) {
{
name: "empty",
gitURL: "",
expected: envbuilder.EmptyWorkspaceDir,
expected: constants.EmptyWorkspaceDir,
},
}
for _, tt := range successTests {
Expand Down Expand Up @@ -66,7 +68,7 @@ func TestDefaultWorkspaceFolder(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
dir, err := envbuilder.DefaultWorkspaceFolder(tt.invalidURL)
require.NoError(t, err)
require.Equal(t, envbuilder.EmptyWorkspaceDir, dir)
require.Equal(t, constants.EmptyWorkspaceDir, dir)
})
}
}
16 changes: 8 additions & 8 deletions integration/integration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,10 @@ import (
"testing"
"time"

"github.com/coder/envbuilder/options"

"github.com/coder/envbuilder"
"github.com/coder/envbuilder/constants"
"github.com/coder/envbuilder/devcontainer/features"
"github.com/coder/envbuilder/options"
"github.com/coder/envbuilder/testutil/gittest"
"github.com/coder/envbuilder/testutil/mwtest"
"github.com/coder/envbuilder/testutil/registrytest"
Expand Down Expand Up @@ -366,7 +366,7 @@ func TestBuildFromDockerfile(t *testing.T) {
require.Equal(t, "hello", strings.TrimSpace(output))

// Verify that the Docker configuration secret file is removed
output = execContainer(t, ctr, "stat "+filepath.Join(envbuilder.MagicDir, "config.json"))
output = execContainer(t, ctr, "stat "+filepath.Join(constants.MagicDir, "config.json"))
require.Contains(t, output, "No such file or directory")
}

Expand Down Expand Up @@ -592,7 +592,7 @@ func TestCloneFailsFallback(t *testing.T) {
_, err := runEnvbuilder(t, runOpts{env: []string{
envbuilderEnv("GIT_URL", "bad-value"),
}})
require.ErrorContains(t, err, envbuilder.ErrNoFallbackImage.Error())
require.ErrorContains(t, err, constants.ErrNoFallbackImage.Error())
})
}

Expand All @@ -610,7 +610,7 @@ func TestBuildFailsFallback(t *testing.T) {
envbuilderEnv("GIT_URL", srv.URL),
envbuilderEnv("DOCKERFILE_PATH", "Dockerfile"),
}})
require.ErrorContains(t, err, envbuilder.ErrNoFallbackImage.Error())
require.ErrorContains(t, err, constants.ErrNoFallbackImage.Error())
require.ErrorContains(t, err, "dockerfile parse error")
})
t.Run("FailsBuild", func(t *testing.T) {
Expand All @@ -626,7 +626,7 @@ RUN exit 1`,
envbuilderEnv("GIT_URL", srv.URL),
envbuilderEnv("DOCKERFILE_PATH", "Dockerfile"),
}})
require.ErrorContains(t, err, envbuilder.ErrNoFallbackImage.Error())
require.ErrorContains(t, err, constants.ErrNoFallbackImage.Error())
})
t.Run("BadDevcontainer", func(t *testing.T) {
t.Parallel()
Expand All @@ -639,7 +639,7 @@ RUN exit 1`,
_, err := runEnvbuilder(t, runOpts{env: []string{
envbuilderEnv("GIT_URL", srv.URL),
}})
require.ErrorContains(t, err, envbuilder.ErrNoFallbackImage.Error())
require.ErrorContains(t, err, constants.ErrNoFallbackImage.Error())
})
t.Run("NoImageOrDockerfile", func(t *testing.T) {
t.Parallel()
Expand Down Expand Up @@ -972,7 +972,7 @@ func setupPassthroughRegistry(t *testing.T, image string, opts *setupPassthrough

func TestNoMethodFails(t *testing.T) {
_, err := runEnvbuilder(t, runOpts{env: []string{}})
require.ErrorContains(t, err, envbuilder.ErrNoFallbackImage.Error())
require.ErrorContains(t, err, constants.ErrNoFallbackImage.Error())
}

func TestDockerfileBuildContext(t *testing.T) {
Expand Down