Skip to content

Commit

Permalink
Fix .dockerignore resolution when using multiple Dockerfiles (#863)
Browse files Browse the repository at this point in the history
This pushes `.dockerignore` path resolution down into
`getIgnorePatterns` so it can juggle the multiple candidate ignore-files
correctly.

Resolution follows the behavior described [here](spec):

> ...the build client looks for a file named .dockerignore in the root
directory of the context.
>
> If you use multiple Dockerfiles, you can use different ignore-files
for each Dockerfile. [...] Place your ignore-file in the same directory
as the Dockerfile, and prefix the ignore-file with the name of the
Dockerfile.
>
> A Dockerfile-specific ignore-file takes precedence over the
.dockerignore file at the root of the build context if both exist.

In other words, we look for a `<Dockerfile>.dockerignore` file sitting
next to `<Dockerfile>`, and if that doesn't exist we fall back on the
`.dockerignore` at the root of our build context.

Fixes #675

[spec]:
https://docs.docker.com/build/building/context/#filename-and-location
  • Loading branch information
blampe authored Dec 11, 2023
1 parent ea5c8f4 commit 61de332
Show file tree
Hide file tree
Showing 17 changed files with 160 additions and 141 deletions.
34 changes: 3 additions & 31 deletions examples/dockerfile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,12 @@
package examples

import (
"github.com/pulumi/pulumi/pkg/v3/testing/integration"
"github.com/stretchr/testify/assert"
"os"
"path"
"testing"

"github.com/pulumi/pulumi/pkg/v3/testing/integration"
"github.com/stretchr/testify/assert"
)

func TestDockerfileDefaultYAML(t *testing.T) {
Expand Down Expand Up @@ -102,35 +103,6 @@ func TestDockerignoreSpecifiedYAML(t *testing.T) {
})
}

func TestDockerignoreDefaultFailYAML(t *testing.T) {
cwd, err := os.Getwd()
if !assert.NoError(t, err) {
t.FailNow()
}

integration.ProgramTest(t, &integration.ProgramTestOptions{
Dir: path.Join(cwd, "test-dockerfile", "dockerignore-default-fail"),
Quick: true,
SkipRefresh: true,
ExpectFailure: true,
})
}

func TestDockerignoreNoMappingYAML(t *testing.T) {
cwd, err := os.Getwd()
if !assert.NoError(t, err) {
t.FailNow()
}
// we expect this test to succeed, as we test that the ignore.txt file does in fact _not_ get ignored
// the ignore.txt file does not get ignored, as .dockerignore does not map to Mockerfile.
// The RUN command in Mockerfile therefore succeeds.
integration.ProgramTest(t, &integration.ProgramTestOptions{
Dir: path.Join(cwd, "test-dockerfile", "dockerignore-no-mapping"),
Quick: true,
SkipRefresh: true,
})
}

func TestDockerignoreWithExternalDockerfileYAML(t *testing.T) {
cwd, err := os.Getwd()
if !assert.NoError(t, err) {
Expand Down

This file was deleted.

8 changes: 0 additions & 8 deletions examples/test-dockerfile/dockerignore-default-fail/Dockerfile

This file was deleted.

13 changes: 0 additions & 13 deletions examples/test-dockerfile/dockerignore-default-fail/Pulumi.yaml

This file was deleted.

1 change: 0 additions & 1 deletion examples/test-dockerfile/dockerignore-default-fail/app.txt

This file was deleted.

This file was deleted.

6 changes: 5 additions & 1 deletion examples/test-dockerfile/dockerignore-default/Dockerfile
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
FROM scratch
FROM bash

COPY . /

RUN [ -f "app.txt" ]

RUN [ ! -f "ignore.txt" ]

This file was deleted.

7 changes: 0 additions & 7 deletions examples/test-dockerfile/dockerignore-no-mapping/Mockerfile

This file was deleted.

16 changes: 0 additions & 16 deletions examples/test-dockerfile/dockerignore-no-mapping/Pulumi.yaml

This file was deleted.

1 change: 0 additions & 1 deletion examples/test-dockerfile/dockerignore-no-mapping/app.txt

This file was deleted.

This file was deleted.

6 changes: 5 additions & 1 deletion examples/test-dockerfile/dockerignore-specified/Mockerfile
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
FROM --platform=linux/amd64 ubuntu
FROM bash

COPY . /

RUN [ -f "app.txt" ]

RUN [ ! -f "ignore.txt" ]
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
FROM --platform=linux/amd64 ubuntu
FROM bash

COPY . /

RUN cat app.txt
RUN [ -f "app.txt" ]

RUN [ ! -f "ignore.txt" ]
31 changes: 7 additions & 24 deletions provider/image.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,13 @@ import (
"github.com/opencontainers/go-digest"
"github.com/pulumi/pulumi/sdk/v3/go/common/resource"
"github.com/pulumi/pulumi/sdk/v3/go/common/resource/plugin"
"github.com/spf13/afero"
)

const defaultDockerfile = "Dockerfile"
const defaultBuilder = "2"
const (
defaultDockerfile = "Dockerfile"
defaultBuilder = "2"
)

type Image struct {
Name string
Expand Down Expand Up @@ -112,17 +115,13 @@ func (p *dockerNativeProvider) dockerBuild(ctx context.Context,
img.Build = build

docker, err := configureDockerClient(p.config, true)

if err != nil {
return "", nil, err
}

// make the build context and ensure to exclude dockerignore file patterns
// map the expected location for dockerignore
dockerignore := mapDockerignore(filepath.Base(build.Dockerfile))
dockerIgnorePath := filepath.Join(build.Context, dockerignore)

initialIgnorePatterns, err := getIgnore(dockerIgnorePath)
initialIgnorePatterns, err := getIgnorePatterns(afero.NewOsFs(), build.Dockerfile, build.Context)
if err != nil {
return "", nil, fmt.Errorf("error reading ignore file: %w", err)
}
Expand Down Expand Up @@ -245,7 +244,7 @@ func (p *dockerNativeProvider) dockerBuild(ctx context.Context,
AuthConfigs: authConfigs,
}

//Start a session for BuildKit
// Start a session for BuildKit
if build.BuilderVersion == defaultBuilder {
sess, err := session.NewSession(ctx, "pulumi-docker", identity.NewID())
if err != nil {
Expand Down Expand Up @@ -1157,19 +1156,3 @@ func defaultPooledTransport() *http.Transport {
}
return transport
}

func mapDockerignore(dockerfile string) string {
// Docker maps `Dockerfile` -> `.dockerignore`
// Nonstandard dockerfile names map to a file with a `.dockerignore` extension
// e.g. `Mockerfile` -> `Mockerfile.dockerignore`
// Note that we do not verify the existence of a .dockerignore file; we only map the name that it would have.

ignore := ".dockerignore"

// Add extension for nonstandardly named Dockerfiles
if dockerfile != defaultDockerfile {
ignore = dockerfile + ignore
}
// Return the default dockerignore name.
return ignore
}
114 changes: 100 additions & 14 deletions provider/image_test.go
Original file line number Diff line number Diff line change
@@ -1,13 +1,17 @@
package provider

import (
"bufio"
"fmt"
"runtime"
"strings"
"testing"

"github.com/docker/docker/api/types"
"github.com/pulumi/pulumi/sdk/v3/go/common/resource"
"github.com/spf13/afero"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestSetRegistry(t *testing.T) {
Expand Down Expand Up @@ -646,19 +650,101 @@ func TestConfigureDockerClient(t *testing.T) {
})
}

func TestMapDockerignore(t *testing.T) {

t.Run("Returns default .dockerignore", func(t *testing.T) {
expected := ".dockerignore"
input := defaultDockerfile
actual := mapDockerignore(input)
assert.Equal(t, expected, actual)
})
t.Run("Returns .dockerignore extension for nonstandard dockerfile names", func(t *testing.T) {
expected := "special.dockerfile.dockerignore"
input := "special.dockerfile"
actual := mapDockerignore(input)
assert.Equal(t, expected, actual)
})
func TestDockerIgnore(t *testing.T) {
tests := []struct {
name string

dockerfile string
context string
fs map[string]string

want []string
wantErr error
}{
{
name: "Dockerfile with root dockerignore",
dockerfile: "./foo/Dockerfile",
fs: map[string]string{
".dockerignore": "rootignore",
},
want: []string{"rootignore"},
},
{
name: "Dockerfile with root dockerignore and custom dockerignore",
dockerfile: "./foo/Dockerfile",
fs: map[string]string{
"foo/Dockerfile.dockerignore": "customignore",
".dockerignore": "rootignore",
},
want: []string{"customignore"},
},
{
name: "Dockerfile with root dockerignore and relative context",
dockerfile: "./foo/Dockerfile",
context: "../",
fs: map[string]string{
"../.dockerignore": "rootignore",
},
want: []string{"rootignore"},
},
{
name: "Dockerfile without root dockerignore",
dockerfile: "./foo/Dockerfile",
want: nil,
},
{
name: "Dockerfile with invalid root dockerignore",
dockerfile: "./foo/Dockerfile",
fs: map[string]string{
".dockerignore": strings.Repeat("*", bufio.MaxScanTokenSize),
},
wantErr: bufio.ErrTooLong,
},
{
name: "custom.Dockerfile without custom dockerignore and without root dockerignore",
dockerfile: "./foo/custom.Dockerfile",
want: nil,
},
{
name: "custom.Dockerfile with custom dockerignore and without root dockerignore",
dockerfile: "./foo/custom.Dockerfile",
fs: map[string]string{
"foo/custom.Dockerfile.dockerignore": "customignore",
},
want: []string{"customignore"},
},
{
name: "custom.Dockerfile with custom dockerignore and with root dockerignore",
dockerfile: "foo/custom.Dockerfile",
fs: map[string]string{
"foo/custom.Dockerfile.dockerignore": "customignore",
".dockerignore": "rootignore",
},
want: []string{"customignore"},
},
{
name: "custom.Dockerfile without custom dockerignore and with root dockerignore",
dockerfile: "foo/custom.Dockerfile",
fs: map[string]string{
".dockerignore": "rootignore",
},
want: []string{"rootignore"},
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
fs := afero.NewMemMapFs()
for fname, fdata := range tt.fs {
f, err := fs.Create(fname)
require.NoError(t, err)
_, err = f.Write([]byte(fdata))
require.NoError(t, err)
}
actual, err := getIgnorePatterns(fs, tt.dockerfile, tt.context)

assert.ErrorIs(t, err, tt.wantErr)
assert.Equal(t, tt.want, actual)
})
}
}
Loading

0 comments on commit 61de332

Please sign in to comment.