Skip to content

Commit 1d5b5bc

Browse files
dgageotdgageot
committed
Improve Docker dependencies code
Fixes #386
1 parent 2d08841 commit 1d5b5bc

File tree

8 files changed

+101
-227
lines changed

8 files changed

+101
-227
lines changed

‎pkg/skaffold/bazel/bazel.go

+1-4
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ package bazel
1919
import (
2020
"fmt"
2121
"os/exec"
22-
"path/filepath"
2322
"strings"
2423

2524
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/v1alpha2"
@@ -52,9 +51,7 @@ func (*BazelDependencyResolver) GetDependencies(a *v1alpha2.Artifact) ([]string,
5251
continue
5352
}
5453

55-
dep := filepath.Join(a.Workspace, depToPath(l))
56-
57-
deps = append(deps, dep)
54+
deps = append(deps, depToPath(l))
5855
}
5956
return deps, nil
6057
}

‎pkg/skaffold/build/deps.go

+2-1
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ package build
1818

1919
import (
2020
"fmt"
21+
"path/filepath"
2122
"sort"
2223
"strings"
2324

@@ -119,7 +120,7 @@ func pathsForArtifact(a *v1alpha2.Artifact) ([]string, error) {
119120
logrus.Debugf("Ignoring %s for artifact dependencies", dep)
120121
continue
121122
}
122-
filteredDeps = append(filteredDeps, dep)
123+
filteredDeps = append(filteredDeps, filepath.Join(a.Workspace, dep))
123124
}
124125
return filteredDeps, nil
125126
}

‎pkg/skaffold/docker/context.go

+2-6
Original file line numberDiff line numberDiff line change
@@ -25,12 +25,8 @@ import (
2525
"github.com/pkg/errors"
2626
)
2727

28-
func contextTarPaths(dockerfilePath string, context string) ([]string, error) {
29-
return GetDockerfileDependencies(dockerfilePath, context)
30-
}
31-
3228
func CreateDockerTarContext(w io.Writer, dockerfilePath, context string) error {
33-
paths, err := contextTarPaths(dockerfilePath, context)
29+
paths, err := GetDockerfileDependencies(dockerfilePath, context)
3430
if err != nil {
3531
return errors.Wrap(err, "getting relative tar paths")
3632
}
@@ -41,7 +37,7 @@ func CreateDockerTarContext(w io.Writer, dockerfilePath, context string) error {
4137
}
4238

4339
func CreateDockerTarGzContext(w io.Writer, dockerfilePath, context string) error {
44-
paths, err := contextTarPaths(dockerfilePath, context)
40+
paths, err := GetDockerfileDependencies(dockerfilePath, context)
4541
if err != nil {
4642
return errors.Wrap(err, "getting relative tar paths")
4743
}

‎pkg/skaffold/docker/parse.go

+75-65
Original file line numberDiff line numberDiff line change
@@ -23,22 +23,18 @@ import (
2323
"io"
2424
"net/http"
2525
"os"
26-
"path"
2726
"path/filepath"
2827
"sort"
2928
"strings"
3029
"sync"
3130

3231
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/v1alpha2"
33-
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/util"
34-
"github.com/google/go-containerregistry/v1"
35-
32+
"github.com/docker/docker/builder/dockerignore"
33+
"github.com/docker/docker/pkg/fileutils"
3634
"github.com/google/go-containerregistry/authn"
3735
"github.com/google/go-containerregistry/name"
36+
"github.com/google/go-containerregistry/v1"
3837
"github.com/google/go-containerregistry/v1/remote"
39-
40-
"github.com/docker/docker/builder/dockerignore"
41-
"github.com/docker/docker/pkg/fileutils"
4238
"github.com/moby/moby/builder/dockerfile/parser"
4339
"github.com/moby/moby/builder/dockerfile/shell"
4440
"github.com/pkg/errors"
@@ -62,11 +58,9 @@ func (d *DockerfileDepResolver) GetDependencies(a *v1alpha2.Artifact) ([]string,
6258
return GetDockerfileDependencies(a.DockerArtifact.DockerfilePath, a.Workspace)
6359
}
6460

65-
// GetDockerfileDependencies parses a dockerfile and returns the full paths
66-
// of all the source files that the resulting docker image depends on.
67-
func GetDockerfileDependencies(dockerfilePath, workspace string) ([]string, error) {
61+
func readDockerfile(workspace, dockerfilePath string) ([]string, error) {
6862
path := filepath.Join(workspace, dockerfilePath)
69-
f, err := util.Fs.Open(path)
63+
f, err := os.Open(path)
7064
if err != nil {
7165
return nil, errors.Wrapf(err, "opening dockerfile: %s", path)
7266
}
@@ -96,7 +90,7 @@ func GetDockerfileDependencies(dockerfilePath, workspace string) ([]string, erro
9690
for _, value := range r.AST.Children {
9791
switch value.Value {
9892
case add, copy:
99-
processCopy(workspace, value, depMap, envs)
93+
processCopy(value, depMap, envs)
10094
case env:
10195
envs[value.Next.Value] = value.Next.Next.Value
10296
}
@@ -114,30 +108,86 @@ func GetDockerfileDependencies(dockerfilePath, workspace string) ([]string, erro
114108

115109
dispatchInstructions(res)
116110

117-
deps := []string{}
111+
var deps []string
118112
for dep := range depMap {
119113
deps = append(deps, dep)
120114
}
121115
logrus.Infof("Found dependencies for dockerfile %s", deps)
122116

123-
expandedDeps, err := util.ExpandPaths(workspace, deps)
117+
return deps, nil
118+
}
119+
120+
func GetDockerfileDependencies(dockerfilePath, workspace string) ([]string, error) {
121+
deps, err := readDockerfile(workspace, dockerfilePath)
124122
if err != nil {
125-
return nil, errors.Wrap(err, "expanding dockerfile paths")
123+
return nil, err
126124
}
127-
logrus.Infof("deps %s", expandedDeps)
128125

129-
if !util.StrSliceContains(expandedDeps, path) {
130-
expandedDeps = append(expandedDeps, path)
126+
// Read patterns to ignore
127+
var excludes []string
128+
dockerignorePath := filepath.Join(workspace, ".dockerignore")
129+
if _, err := os.Stat(dockerignorePath); !os.IsNotExist(err) {
130+
r, err := os.Open(dockerignorePath)
131+
if err != nil {
132+
return nil, err
133+
}
134+
defer r.Close()
135+
136+
excludes, err = dockerignore.ReadAll(r)
137+
if err != nil {
138+
return nil, err
139+
}
140+
}
141+
142+
// Walk the workspace
143+
files := make(map[string]bool)
144+
for _, dep := range deps {
145+
filepath.Walk(filepath.Join(workspace, dep), func(fpath string, info os.FileInfo, err error) error {
146+
if err != nil {
147+
return err
148+
}
149+
150+
relPath, err := filepath.Rel(workspace, fpath)
151+
if err != nil {
152+
return err
153+
}
154+
155+
ignored, err := fileutils.Matches(relPath, excludes)
156+
if err != nil {
157+
return err
158+
}
159+
160+
if info.IsDir() && ignored {
161+
return filepath.SkipDir
162+
}
163+
164+
if !info.IsDir() && !ignored {
165+
files[relPath] = true
166+
}
167+
168+
return nil
169+
})
131170
}
132171

133-
// Look for .dockerignore.
134-
ignorePath := filepath.Join(workspace, ".dockerignore")
135-
filteredDeps, err := ApplyDockerIgnore(expandedDeps, ignorePath)
172+
// Add dockerfile?
173+
m, err := fileutils.Matches(dockerfilePath, excludes)
136174
if err != nil {
137-
return nil, errors.Wrap(err, "applying dockerignore")
175+
return nil, err
176+
}
177+
if !m {
178+
files[dockerfilePath] = true
138179
}
139180

140-
return filteredDeps, nil
181+
// Ignore .dockerignore
182+
delete(files, ".dockerignore")
183+
184+
var dependencies []string
185+
for file := range files {
186+
dependencies = append(dependencies, file)
187+
}
188+
sort.Strings(dependencies)
189+
190+
return dependencies, nil
141191
}
142192

143193
func PortsFromDockerfile(r io.Reader) ([]string, error) {
@@ -256,7 +306,7 @@ func retrieveRemoteImage(image string) (*v1.ConfigFile, error) {
256306
return img.ConfigFile()
257307
}
258308

259-
func processCopy(workspace string, value *parser.Node, paths map[string]struct{}, envs map[string]string) error {
309+
func processCopy(value *parser.Node, paths map[string]struct{}, envs map[string]string) error {
260310
slex := shell.NewLex('\\')
261311
for {
262312
// Skip last node, since it is the destination, and stop if we arrive at a comment
@@ -273,8 +323,7 @@ func processCopy(workspace string, value *parser.Node, paths map[string]struct{}
273323
return nil
274324
}
275325
if !strings.HasPrefix(src, "http://") && !strings.HasPrefix(src, "https://") {
276-
dep := path.Join(workspace, src)
277-
paths[dep] = struct{}{}
326+
paths[src] = struct{}{}
278327
} else {
279328
logrus.Debugf("Skipping watch on remote dependency %s", src)
280329
}
@@ -300,42 +349,3 @@ func hasMultiStageFlag(flags []string) bool {
300349
}
301350
return false
302351
}
303-
304-
func ApplyDockerIgnore(paths []string, dockerIgnorePath string) ([]string, error) {
305-
absPaths, err := util.RelPathToAbsPath(paths)
306-
if err != nil {
307-
return nil, errors.Wrap(err, "getting absolute path of dependencies")
308-
}
309-
excludes := []string{}
310-
if _, err := util.Fs.Stat(dockerIgnorePath); !os.IsNotExist(err) {
311-
r, err := util.Fs.Open(dockerIgnorePath)
312-
if err != nil {
313-
return nil, err
314-
}
315-
defer r.Close()
316-
317-
excludes, err = dockerignore.ReadAll(r)
318-
if err != nil {
319-
return nil, err
320-
}
321-
excludes = append(excludes, ".dockerignore")
322-
}
323-
324-
absPathExcludes, err := util.RelPathToAbsPath(excludes)
325-
if err != nil {
326-
return nil, errors.Wrap(err, "getting absolute path of docker ignored paths")
327-
}
328-
329-
filteredDeps := []string{}
330-
for _, d := range absPaths {
331-
m, err := fileutils.Matches(d, absPathExcludes)
332-
if err != nil {
333-
return nil, err
334-
}
335-
if !m {
336-
filteredDeps = append(filteredDeps, d)
337-
}
338-
}
339-
sort.Strings(filteredDeps)
340-
return filteredDeps, nil
341-
}

‎pkg/skaffold/docker/parse_test.go

+20-13
Original file line numberDiff line numberDiff line change
@@ -18,15 +18,15 @@ package docker
1818

1919
import (
2020
"fmt"
21+
"io/ioutil"
22+
"os"
2123
"path/filepath"
2224
"strings"
2325
"testing"
2426

25-
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/util"
2627
"github.com/GoogleContainerTools/skaffold/testutil"
2728

2829
"github.com/google/go-containerregistry/v1"
29-
"github.com/spf13/afero"
3030
)
3131

3232
const copyDockerfile = `
@@ -159,7 +159,7 @@ func TestGetDockerfileDependencies(t *testing.T) {
159159
description: "add dependency",
160160
dockerfile: addDockerfile,
161161
workspace: "docker",
162-
expected: []string{"docker/Dockerfile", "docker/nginx.conf"},
162+
expected: []string{"Dockerfile", "nginx.conf"},
163163
},
164164
{
165165
description: "bad read",
@@ -204,11 +204,18 @@ func TestGetDockerfileDependencies(t *testing.T) {
204204
expected: []string{"Dockerfile", "file", "server.go", "test.conf", "worker.go"},
205205
},
206206
{
207-
description: "dockerignore with context in parent directory test",
207+
description: "dockerignore with non canonical workspace",
208208
dockerfile: contextDockerfile,
209209
workspace: "docker/../docker",
210210
dockerIgnore: true,
211-
expected: []string{},
211+
expected: []string{"Dockerfile", "nginx.conf"},
212+
},
213+
{
214+
description: "dockerignore with context in parent directory",
215+
dockerfile: contextDockerfile,
216+
workspace: "docker/..",
217+
dockerIgnore: true,
218+
expected: []string{"Dockerfile", "file", "server.go", "test.conf", "worker.go"},
212219
},
213220
{
214221
description: "onbuild test",
@@ -231,24 +238,24 @@ func TestGetDockerfileDependencies(t *testing.T) {
231238

232239
for _, test := range tests {
233240
t.Run(test.description, func(t *testing.T) {
234-
defer func(fs afero.Fs) { util.Fs = fs }(util.Fs)
235-
util.Fs = afero.NewMemMapFs()
241+
tmpDir, cleanup := testutil.TempDir(t)
242+
defer cleanup()
236243

237-
util.Fs.MkdirAll("docker", 0750)
244+
os.MkdirAll(filepath.Join(tmpDir, "docker"), 0750)
238245
for _, file := range []string{"docker/nginx.conf", "docker/bar", "server.go", "test.conf", "worker.go", "bar", "file"} {
239-
afero.WriteFile(util.Fs, file, []byte(""), 0644)
246+
ioutil.WriteFile(filepath.Join(tmpDir, file), []byte(""), 0644)
240247
}
241248

249+
workspace := filepath.Join(tmpDir, test.workspace)
242250
if !test.badReader {
243-
afero.WriteFile(util.Fs, filepath.Join(test.workspace, "Dockerfile"), []byte(test.dockerfile), 0644)
251+
ioutil.WriteFile(filepath.Join(workspace, "Dockerfile"), []byte(test.dockerfile), 0644)
244252
}
245253

246254
if test.dockerIgnore {
247-
afero.WriteFile(util.Fs, ".dockerignore", []byte(dockerIgnore), 0644)
248-
afero.WriteFile(util.Fs, filepath.Join("docker", ".dockerignore"), []byte(dockerIgnore), 0644)
255+
ioutil.WriteFile(filepath.Join(workspace, ".dockerignore"), []byte(dockerIgnore), 0644)
249256
}
250257

251-
deps, err := GetDockerfileDependencies("Dockerfile", test.workspace)
258+
deps, err := GetDockerfileDependencies("Dockerfile", workspace)
252259
testutil.CheckErrorAndDeepEqual(t, test.shouldErr, err, test.expected, deps)
253260
})
254261
}

‎pkg/skaffold/util/tar.go

+1-15
Original file line numberDiff line numberDiff line change
@@ -31,22 +31,8 @@ func CreateTar(w io.Writer, root string, paths []string) error {
3131
tw := tar.NewWriter(w)
3232
defer tw.Close()
3333

34-
absContext, err := filepath.Abs(root)
35-
if err != nil {
36-
return err
37-
}
3834
for _, p := range paths {
39-
absPath, err := filepath.Abs(p)
40-
if err != nil {
41-
return err
42-
}
43-
44-
tarPath, err := filepath.Rel(absContext, absPath)
45-
if err != nil {
46-
return err
47-
}
48-
49-
if err := addFileToTar(p, tarPath, tw); err != nil {
35+
if err := addFileToTar(filepath.Join(root, p), p, tw); err != nil {
5036
return err
5137
}
5238

0 commit comments

Comments
 (0)