Skip to content

Commit

Permalink
fix add command bug when adding remote URLs
Browse files Browse the repository at this point in the history
  • Loading branch information
Priya Wadhwa committed Aug 7, 2018
1 parent 53b5fb4 commit 39708c3
Show file tree
Hide file tree
Showing 7 changed files with 34 additions and 5 deletions.
2 changes: 1 addition & 1 deletion deploy/Dockerfile_debug
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ RUN make -C /go/src/github.com/awslabs/amazon-ecr-credential-helper linux-amd64
FROM gcr.io/cloud-builders/bazel:latest
RUN git clone https://github.com/GoogleContainerTools/distroless.git
WORKDIR /distroless
RUN bazel build //experimental/busybox:busybox.tar
RUN bazel build //experimental/busybox:busybox_tar
RUN tar -C /distroless/bazel-genfiles/experimental/busybox/ -xf /distroless/bazel-genfiles/experimental/busybox/busybox.tar

FROM scratch
Expand Down
1 change: 1 addition & 0 deletions integration/dockerfiles/Dockerfile_test_add
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,4 @@ COPY $file /arg

# Finally, test adding a remote URL, concurrently with a normal file
ADD https://github.com/GoogleCloudPlatform/docker-credential-gcr/releases/download/v1.4.3/docker-credential-gcr_linux_386-1.4.3.tar.gz context/foo /test/all/
ADD https://github.com/GoogleCloudPlatform/docker-credential-gcr/releases/download/v1.4.3-static/docker-credential-gcr_linux_amd64-1.4.3.tar.gz /destination
2 changes: 1 addition & 1 deletion integration/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ func TestRun(t *testing.T) {

func TestLayers(t *testing.T) {
offset := map[string]int{
"Dockerfile_test_add": 9,
"Dockerfile_test_add": 10,
"Dockerfile_test_scratch": 3,
// the Docker built image combined some of the dirs defined by separate VOLUME commands into one layer
// which is why this offset exists
Expand Down
6 changes: 5 additions & 1 deletion pkg/constants/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,11 @@ const (
)

// KanikoBuildFiles is the list of files required to build kaniko
var KanikoBuildFiles = []string{"/kaniko/executor", "/kaniko/ssl/certs/ca-certificates.crt"}
var KanikoBuildFiles = []string{"/kaniko/executor",
"/kaniko/ssl/certs/ca-certificates.crt",
"/kaniko/docker-credential-gcr",
"/kaniko/docker-credential-ecr-login",
"/kaniko/.docker/config.json"}

// ScratchEnvVars are the default environment variables needed for a scratch image.
var ScratchEnvVars = []string{"PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"}
14 changes: 12 additions & 2 deletions pkg/snapshot/snapshot.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"os"
"path/filepath"

"github.com/GoogleContainerTools/kaniko/pkg/constants"
"github.com/GoogleContainerTools/kaniko/pkg/util"
"github.com/sirupsen/logrus"
)
Expand Down Expand Up @@ -94,8 +95,8 @@ func (s *Snapshotter) snapshotFiles(f io.Writer, files []string) (bool, error) {
if val, ok := snapshottedFiles[file]; ok && val {
continue
}
if util.CheckWhitelist(file) {
logrus.Debugf("Not adding %s to layer, as it's whitelisted", file)
if util.CheckWhitelist(file) && !isBuildFile(file) {
logrus.Infof("Not adding %s to layer, as it's whitelisted", file)
continue
}
snapshottedFiles[file] = true
Expand All @@ -118,6 +119,15 @@ func (s *Snapshotter) snapshotFiles(f io.Writer, files []string) (bool, error) {
return filesAdded, nil
}

func isBuildFile(file string) bool {
for _, buildFile := range constants.KanikoBuildFiles {
if file == buildFile {
return true
}
}
return false
}

func (s *Snapshotter) snapShotFS(f io.Writer) (bool, error) {
logrus.Info("Taking snapshot of full filesystem...")
s.hardlinks = map[uint64]string{}
Expand Down
4 changes: 4 additions & 0 deletions pkg/util/command_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,11 @@ func IsSrcsValid(srcsAndDest instructions.SourcesAndDest, resolvedSources []stri
}
}

// If there is only one source and it's a directory, docker assumes the dest is a directory
if len(resolvedSources) == 1 {
if IsSrcRemoteFileURL(resolvedSources[0]) {
return nil
}
fi, err := os.Lstat(filepath.Join(root, resolvedSources[0]))
if err != nil {
return err
Expand Down
10 changes: 10 additions & 0 deletions pkg/util/command_util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,16 @@ var isSrcValidTests = []struct {
},
shouldErr: false,
},
{
srcsAndDest: []string{
testUrl,
"dest",
},
resolvedSources: []string{
testUrl,
},
shouldErr: false,
},
}

func Test_IsSrcsValid(t *testing.T) {
Expand Down

0 comments on commit 39708c3

Please sign in to comment.