Leave out of the build context what the app's .dockerignore names (closes #274)
Check / check (pull_request) Skipped
Check / check (pull_request) Skipped
Docker does not apply .dockerignore to a build context sent as a tar, so upaas sent every file in the clone. The build now reads the ignore file as docker build does, with the ignorefile reader of moby/patternmatcher: <Dockerfile>.dockerignore next to the Dockerfile if there is one, otherwise .dockerignore at the root. As Docker does, the Dockerfile path is read as a path inside the clone, and when a Dockerfile named Dockerfile is missing, a lowercase dockerfile next to it is used instead. As the docker command line does, the Dockerfile and .dockerignore are never left out. The ignore file is read through os.Root. An ignore file that cannot be read, or a malformed pattern, fails the build. Model: opus-5-5
This commit is contained in:
@@ -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: `<Dockerfile>.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`.
|
||||
|
||||
|
||||
@@ -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,
|
||||
`<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-03: `make test`, and so `docker build .`, fits a machine with 4 GiB of
|
||||
memory: tests hash passwords with 1 MiB instead of upaasd's 64 MiB, so
|
||||
`GOMAXPROCS=4 make test` peaks at about 1.4 GiB instead of 2.7 GiB. upaasd
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -0,0 +1,114 @@
|
||||
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 <dockerfile>.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() }()
|
||||
|
||||
// When a Dockerfile named Dockerfile is missing, Docker builds a
|
||||
// lowercase dockerfile in the same directory instead, and docker build
|
||||
// then reads dockerfile.dockerignore as its ignore file.
|
||||
if filepath.Base(dockerfile) == defaultDockerfileName {
|
||||
lowercase := filepath.Join(filepath.Dir(dockerfile), "dockerfile")
|
||||
|
||||
_, dockerfileErr := root.Lstat(dockerfile)
|
||||
_, lowercaseErr := root.Lstat(lowercase)
|
||||
|
||||
if errors.Is(dockerfileErr, fs.ErrNotExist) && lowercaseErr == nil {
|
||||
dockerfile = lowercase
|
||||
}
|
||||
}
|
||||
|
||||
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
|
||||
}
|
||||
@@ -0,0 +1,296 @@
|
||||
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"
|
||||
testLowercaseDockerfile = "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: "keeps a lowercase dockerfile built in place of a missing Dockerfile",
|
||||
dockerfile: defaultDockerfileName,
|
||||
files: map[string]string{
|
||||
defaultDockerignoreName: "*\n",
|
||||
testLowercaseDockerfile: "",
|
||||
},
|
||||
want: []string{defaultDockerignoreName, testLowercaseDockerfile},
|
||||
},
|
||||
{
|
||||
name: "reads a lowercase dockerfile's own ignore file",
|
||||
dockerfile: defaultDockerfileName,
|
||||
files: map[string]string{
|
||||
defaultDockerignoreName: "main.go\n",
|
||||
testLowercaseDockerfile: "",
|
||||
"dockerfile.dockerignore": "secret.txt\n",
|
||||
testMainGo: "",
|
||||
testSecretFile: "",
|
||||
},
|
||||
want: []string{
|
||||
defaultDockerignoreName,
|
||||
testLowercaseDockerfile,
|
||||
"dockerfile.dockerignore",
|
||||
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
|
||||
}
|
||||
@@ -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)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user