Skip to content
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.

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

Don’t always fail if some COPY patterns don't match any file #744

Merged
merged 1 commit into from
Jun 25, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
65 changes: 40 additions & 25 deletions pkg/skaffold/docker/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,8 @@ func readDockerfile(workspace, dockerfilePath string) ([]string, error) {
return nil, errors.Wrap(err, "parsing dockerfile")
}

var copied [][]string
envs := map[string]string{}
depMap := map[string]struct{}{}
// First process onbuilds, if present.
onbuildsImages := [][]string{}
for _, value := range res.AST.Children {
Expand All @@ -77,7 +77,10 @@ func readDockerfile(workspace, dockerfilePath string) ([]string, error) {
for _, value := range r.AST.Children {
switch value.Value {
case add, copy:
processCopy(value, depMap, envs)
files, _ := processCopy(value, envs)
if len(files) > 0 {
copied = append(copied, files)
}
case env:
envs[value.Next.Value] = value.Next.Next.Value
}
Expand All @@ -96,29 +99,38 @@ func readDockerfile(workspace, dockerfilePath string) ([]string, error) {
dispatchInstructions(res)

expandedPaths := make(map[string]bool)
for p := range depMap {
path := filepath.Join(workspace, p)
for _, files := range copied {
matchesOne := false

for _, p := range files {
path := filepath.Join(workspace, p)
if _, err := os.Stat(path); err == nil {
expandedPaths[p] = true
matchesOne = true
continue
}

if _, err := os.Stat(path); err == nil {
expandedPaths[p] = true
continue
}
files, err := filepath.Glob(path)
if err != nil {
return nil, errors.Wrap(err, "invalid glob pattern")
}
if files == nil {
continue
}

files, err := filepath.Glob(path)
if err != nil {
return nil, errors.Wrap(err, "invalid glob pattern")
}
if files == nil {
return nil, fmt.Errorf("file pattern must match at least one file %s", path)
}
for _, f := range files {
rel, err := filepath.Rel(workspace, f)
if err != nil {
return nil, fmt.Errorf("getting relative path of %s", f)
}

for _, f := range files {
rel, err := filepath.Rel(workspace, f)
if err != nil {
return nil, fmt.Errorf("getting relative path of %s", f)
expandedPaths[rel] = true
}
matchesOne = true
}

expandedPaths[rel] = true
if !matchesOne {
return nil, fmt.Errorf("file pattern %s must match at least one file", files)
}
}

Expand Down Expand Up @@ -262,7 +274,9 @@ func retrieveRemoteConfig(identifier string) (*v1.ConfigFile, error) {
return img.ConfigFile()
}

func processCopy(value *parser.Node, paths map[string]struct{}, envs map[string]string) error {
func processCopy(value *parser.Node, envs map[string]string) ([]string, error) {
var copied []string

slex := shell.NewLex('\\')
for {
// Skip last node, since it is the destination, and stop if we arrive at a comment
Expand All @@ -271,22 +285,23 @@ func processCopy(value *parser.Node, paths map[string]struct{}, envs map[string]
}
src, err := processShellWord(slex, value.Next.Value, envs)
if err != nil {
return errors.Wrap(err, "processing word")
return nil, errors.Wrap(err, "processing word")
}
// If the --from flag is provided, we are dealing with a multi-stage dockerfile
// Adding a dependency from a different stage does not imply a source dependency
if hasMultiStageFlag(value.Flags) {
return nil
return nil, nil
}
if !strings.HasPrefix(src, "http://") && !strings.HasPrefix(src, "https://") {
paths[src] = struct{}{}
copied = append(copied, src)
} else {
logrus.Debugf("Skipping watch on remote dependency %s", src)
}

value = value.Next
}
return nil

return copied, nil
}

func processShellWord(lex *shell.Lex, word string, envs map[string]string) (string, error) {
Expand Down
11 changes: 11 additions & 0 deletions pkg/skaffold/docker/parse_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,11 @@ FROM nginx
ADD *.none /tmp
`

const oneWilcardMatchesNone = `
FROM nginx
ADD *.go *.none /tmp
`

const multiStageDockerfile = `
FROM golang:1.9.2
WORKDIR /go/src/github.com/r2d4/leeroy/
Expand Down Expand Up @@ -178,6 +183,12 @@ func TestGetDependencies(t *testing.T) {
workspace: ".",
shouldErr: true,
},
{
description: "one wilcard matches none",
dockerfile: oneWilcardMatchesNone,
workspace: ".",
expected: []string{"Dockerfile", "server.go", "worker.go"},
},
{
description: "bad read",
badReader: true,
Expand Down