diff --git a/README.md b/README.md index a563186..a9e9a25 100644 --- a/README.md +++ b/README.md @@ -268,6 +268,12 @@ upaas fails the deploy instead of building. A Dockerfile that uses `RUN --network` needs Docker Engine 23.0 or later unless its `# syntax=` line names Dockerfile frontend 1.3 or later, such as `docker/dockerfile:1`. +The build context leaves out the files the app's ignore file names, as +`docker build` does: `.dockerignore` next to the app's Dockerfile if +there is one, otherwise `.dockerignore` at the root of the repository. The +Dockerfile and `.dockerignore` are always sent, even when the ignore file names +them. + Session secrets are automatically generated on first startup and persisted to `$UPAAS_DATA_DIR/session.key`. diff --git a/TODO.md b/TODO.md index 196aba3..6d40707 100644 --- a/TODO.md +++ b/TODO.md @@ -20,6 +20,13 @@ regress. # Completed Steps +- 2026-10-03: An app's build context leaves out the files its `.dockerignore` + names, such as `.git/config`, as `docker build` does; before, every file in + the clone was sent. An ignore file next to the Dockerfile, + `.dockerignore`, is read instead when there is one. The Dockerfile + and `.dockerignore` are always sent. An ignore file that cannot be read, or + holds a pattern Docker rejects, fails the build (#274). + - 2026-10-02: In a window too narrow for the top bar, such as 390 px, the New App and Logout buttons move to a second row instead of running into "by @sneak"; the bar keeps a gap between its two sides at every width (#272). diff --git a/go.mod b/go.mod index b2de996..6dc00c9 100644 --- a/go.mod +++ b/go.mod @@ -15,6 +15,7 @@ require ( github.com/joho/godotenv v1.5.1 github.com/mattn/go-sqlite3 v1.14.32 github.com/moby/buildkit v0.16.0 + github.com/moby/patternmatcher v0.6.0 github.com/oklog/ulid/v2 v2.1.1 github.com/prometheus/client_golang v1.23.2 github.com/spf13/viper v1.21.0 @@ -60,7 +61,6 @@ require ( github.com/klauspost/compress v1.18.2 // indirect github.com/moby/docker-image-spec v1.3.1 // indirect github.com/moby/locker v1.0.1 // indirect - github.com/moby/patternmatcher v0.6.0 // indirect github.com/moby/sys/sequential v0.6.0 // indirect github.com/moby/sys/signal v0.7.1 // indirect github.com/moby/sys/user v0.4.0 // indirect diff --git a/internal/docker/buildcontext.go b/internal/docker/buildcontext.go new file mode 100644 index 0000000..2ec9428 --- /dev/null +++ b/internal/docker/buildcontext.go @@ -0,0 +1,100 @@ +package docker + +import ( + "errors" + "fmt" + "io" + "io/fs" + "os" + "path/filepath" + "strings" + + "github.com/docker/docker/pkg/archive" + "github.com/moby/patternmatcher" + "github.com/moby/patternmatcher/ignorefile" +) + +// defaultDockerfileName is the Dockerfile Docker builds when none is named. +const defaultDockerfileName = "Dockerfile" + +// defaultDockerignoreName is the ignore file at the root of a build context, +// read when the Dockerfile has no ignore file of its own. +const defaultDockerignoreName = ".dockerignore" + +// tarBuildContext returns a tar of the build context in contextDir that leaves +// out the files the app's ignore file names, as docker build does; Docker does +// not apply the ignore file to a build context sent as a tar. dockerfile is +// the path of the Dockerfile inside contextDir. +func tarBuildContext(contextDir, dockerfile string) (io.ReadCloser, error) { + excludes, err := readDockerignore(contextDir, dockerfile) + if err != nil { + return nil, err + } + + return archive.TarWithOptions(contextDir, &archive.TarOptions{ + ExcludePatterns: excludes, + }) +} + +// readDockerignore returns the patterns of the files to leave out of the +// build context in contextDir, read as docker build reads them for the +// Dockerfile at dockerfile: from .dockerignore next to the +// Dockerfile if there is one, otherwise from .dockerignore at the root of the +// context. Without either file there are no patterns. +func readDockerignore(contextDir, dockerfile string) ([]string, error) { + // Docker reads the Dockerfile path as a path inside the build context: + // cleaned, with a leading / and any .. that would lead out of the context + // dropped, so ./Dockerfile and /Dockerfile both name the Dockerfile at the + // root. + dockerfile = strings.TrimPrefix(filepath.Join("/", dockerfile), "/") + if dockerfile == "" { + dockerfile = defaultDockerfileName + } + + // Reading through os.Root keeps the read inside the build context, even + // when the ignore file is a symlink. + root, err := os.OpenRoot(contextDir) + if err != nil { + return nil, err + } + + defer func() { _ = root.Close() }() + + name := dockerfile + defaultDockerignoreName + + file, err := root.Open(name) + if errors.Is(err, fs.ErrNotExist) { + name = defaultDockerignoreName + file, err = root.Open(name) + } + + if errors.Is(err, fs.ErrNotExist) { + return nil, nil + } + + if err != nil { + return nil, fmt.Errorf("failed to read %s: %w", name, err) + } + + defer func() { _ = file.Close() }() + + excludes, err := ignorefile.ReadAll(file) + if err != nil { + return nil, fmt.Errorf("failed to read %s: %w", name, err) + } + + // Like the docker command line, never leave out .dockerignore or the + // Dockerfile: Docker reads the Dockerfile from the context. + for _, keep := range []string{defaultDockerignoreName, filepath.ToSlash(dockerfile)} { + excluded, err := patternmatcher.MatchesOrParentMatches(keep, excludes) + if err != nil { + return nil, fmt.Errorf("invalid pattern in %s: %w", name, err) + } + + if excluded { + excludes = append(excludes, "!"+keep) + } + } + + return excludes, nil +} diff --git a/internal/docker/buildcontext_test.go b/internal/docker/buildcontext_test.go new file mode 100644 index 0000000..061efe8 --- /dev/null +++ b/internal/docker/buildcontext_test.go @@ -0,0 +1,269 @@ +package docker //nolint:testpackage // tests the unexported performBuild + +import ( + "archive/tar" + "errors" + "io" + "log/slog" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "slices" + "strings" + "testing" + + "github.com/docker/docker/client" +) + +// File names used in more than one test build context, as constants to +// satisfy the goconst linter. +const ( + testMainGo = "main.go" + testSecretFile = "secret.txt" + testDeployDockerfile = "deploy/Dockerfile" +) + +// TestPerformBuildFollowsDockerignore runs builds against a fake Docker API +// and checks which files the build context sent to it holds. +func TestPerformBuildFollowsDockerignore(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + dockerfile string + files map[string]string // path in the context: contents + want []string // files sent in the build context + }{ + { + name: "no ignore file", + dockerfile: defaultDockerfileName, + files: map[string]string{defaultDockerfileName: "", testMainGo: ""}, + want: []string{defaultDockerfileName, testMainGo}, + }, + { + name: "excludes and re-includes", + dockerfile: defaultDockerfileName, + files: map[string]string{ + defaultDockerignoreName: "secret.txt\n*.md\n!README.md\n", + defaultDockerfileName: "", + "NOTES.md": "", + "README.md": "", + testMainGo: "", + testSecretFile: "", + }, + want: []string{ + defaultDockerignoreName, defaultDockerfileName, "README.md", testMainGo, + }, + }, + { + name: "keeps the Dockerfile and .dockerignore", + dockerfile: defaultDockerfileName, + files: map[string]string{ + defaultDockerignoreName: "Dockerfile\n.dockerignore\nsecret.txt\n", + defaultDockerfileName: "", + testMainGo: "", + testSecretFile: "", + }, + want: []string{defaultDockerignoreName, defaultDockerfileName, testMainGo}, + }, + { + name: "an ignore file next to the Dockerfile wins over .dockerignore", + dockerfile: testDeployDockerfile, + files: map[string]string{ + defaultDockerignoreName: "main.go\n", + testDeployDockerfile: "", + "deploy/Dockerfile.dockerignore": "secret.txt\n", + testMainGo: "", + testSecretFile: "", + }, + want: []string{ + defaultDockerignoreName, + testDeployDockerfile, + "deploy/Dockerfile.dockerignore", + testMainGo, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + contextDir := t.TempDir() + writeFiles(t, contextDir, tt.files) + + got, err := buildContextFiles(t, contextDir, tt.dockerfile) + if err != nil { + t.Fatal(err) + } + + if !slices.Equal(got, tt.want) { + t.Errorf("build context holds %q, want %q", got, tt.want) + } + }) + } +} + +// TestPerformBuildReadsDockerfilePathInsideContext checks that ./Dockerfile +// and /Dockerfile name the Dockerfile at the root of the context, as Docker +// reads them, so an ignore file that names the Dockerfile does not leave it +// out. +func TestPerformBuildReadsDockerfilePathInsideContext(t *testing.T) { + t.Parallel() + + for _, dockerfile := range []string{"./Dockerfile", "/Dockerfile"} { + t.Run(dockerfile, func(t *testing.T) { + t.Parallel() + + contextDir := t.TempDir() + writeFiles(t, contextDir, map[string]string{ + defaultDockerignoreName: "Dockerfile\nsecret.txt\n", + defaultDockerfileName: "", + testMainGo: "", + testSecretFile: "", + }) + + got, err := buildContextFiles(t, contextDir, dockerfile) + if err != nil { + t.Fatal(err) + } + + want := []string{defaultDockerignoreName, defaultDockerfileName, testMainGo} + if !slices.Equal(got, want) { + t.Errorf("build context holds %q, want %q", got, want) + } + }) + } +} + +// TestPerformBuildFailsOnMalformedDockerignore checks that a pattern the +// docker command line would reject fails the build. +func TestPerformBuildFailsOnMalformedDockerignore(t *testing.T) { + t.Parallel() + + contextDir := t.TempDir() + writeFiles(t, contextDir, map[string]string{defaultDockerignoreName: "[\n"}) + + _, err := buildContextFiles(t, contextDir, defaultDockerfileName) + + want := "failed to create build context: " + + "invalid pattern in .dockerignore: syntax error in pattern" + if err == nil || err.Error() != want { + t.Errorf("got error %v, want %q", err, want) + } +} + +// TestPerformBuildFailsOnUnreadableDockerignore checks that an ignore file +// that cannot be read, here because it is a directory, fails the build. +func TestPerformBuildFailsOnUnreadableDockerignore(t *testing.T) { + t.Parallel() + + contextDir := t.TempDir() + + err := os.Mkdir(filepath.Join(contextDir, defaultDockerignoreName), 0o750) + if err != nil { + t.Fatal(err) + } + + _, err = buildContextFiles(t, contextDir, defaultDockerfileName) + + want := "failed to create build context: failed to read .dockerignore: " + if err == nil || !strings.HasPrefix(err.Error(), want) { + t.Errorf("got error %v, want one starting %q", err, want) + } +} + +// writeFiles writes files, a map of paths inside dir to their contents, +// creating the directories they are in. +func writeFiles(t *testing.T, dir string, files map[string]string) { + t.Helper() + + for name, contents := range files { + path := filepath.Join(dir, name) + + err := os.MkdirAll(filepath.Dir(path), 0o750) + if err != nil { + t.Fatal(err) + } + + err = os.WriteFile(path, []byte(contents), 0o600) + if err != nil { + t.Fatal(err) + } + } +} + +// buildContextFiles runs a build of contextDir against a fake Docker API and +// returns the names of the files in the build context sent to it, sorted. +func buildContextFiles(t *testing.T, contextDir, dockerfile string) ([]string, error) { + t.Helper() + + sent := make(chan []string, 1) + + srv := httptest.NewServer(http.HandlerFunc( + func(w http.ResponseWriter, r *http.Request) { + switch { + case strings.HasSuffix(r.URL.Path, "/version"): + _, _ = w.Write([]byte(`{"Version":"27.3.1","ApiVersion":"1.47"}`)) + case strings.HasSuffix(r.URL.Path, "/session"): + serveSession(t, w, r, make(chan string, 1)) + case strings.HasSuffix(r.URL.Path, "/build"): + sent <- tarFileNames(t, r.Body) + default: + t.Errorf("unexpected request to %s", r.URL.Path) + } + }, + )) + t.Cleanup(srv.Close) + + dockerAPI, err := client.NewClientWithOpts( + client.WithHost("tcp://" + srv.Listener.Addr().String()), + ) + if err != nil { + t.Fatal(err) + } + + c := &Client{docker: dockerAPI, log: slog.Default()} + + _, err = c.performBuild(t.Context(), BuildImageOptions{ + ContextDir: contextDir, + DockerfilePath: dockerfile, + }) + if err != nil { + return nil, err + } + + return <-sent, nil +} + +// tarFileNames returns the names of the regular files in the tar read from r, +// sorted. +func tarFileNames(t *testing.T, r io.Reader) []string { + t.Helper() + + var names []string + + reader := tar.NewReader(r) + + for { + header, err := reader.Next() + if errors.Is(err, io.EOF) { + break + } + + if err != nil { + t.Errorf("reading the build context: %v", err) + + return nil + } + + if header.Typeflag == tar.TypeReg { + names = append(names, header.Name) + } + } + + slices.Sort(names) + + return names +} diff --git a/internal/docker/client.go b/internal/docker/client.go index 1247e0d..f3b5305 100644 --- a/internal/docker/client.go +++ b/internal/docker/client.go @@ -25,7 +25,6 @@ import ( "github.com/docker/docker/api/types/network" "github.com/docker/docker/api/types/versions" "github.com/docker/docker/client" - "github.com/docker/docker/pkg/archive" "github.com/docker/docker/pkg/jsonmessage" "github.com/docker/docker/pkg/stdcopy" "github.com/docker/go-connections/nat" @@ -660,7 +659,7 @@ func (c *Client) performBuild( } // Create tar archive of build context - tarArchive, err := archive.TarWithOptions(opts.ContextDir, &archive.TarOptions{}) + tarArchive, err := tarBuildContext(opts.ContextDir, opts.DockerfilePath) if err != nil { return "", fmt.Errorf("failed to create build context: %w", err) }