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

ADD command #58

Merged
merged 6 commits into from
Apr 4, 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
7 changes: 6 additions & 1 deletion Gopkg.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions integration_tests/context/tars/file
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
normal file
Binary file added integration_tests/context/tars/file.bz2
Binary file not shown.
Binary file added integration_tests/context/tars/file.tar
Binary file not shown.
Binary file added integration_tests/context/tars/file.tar.gz
Binary file not shown.
18 changes: 18 additions & 0 deletions integration_tests/dockerfiles/Dockerfile_test_add
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
FROM gcr.io/google-appengine/debian9:latest
# First, try adding some regular files
ADD context/foo foo
ADD context/foo /foodir/
ADD context/bar/b* bar/
ADD . newdir
ADD ["context/foo", "/tmp/foo" ]

# Next, make sure environment replacement works
ENV contextenv ./context
ADD $contextenv/* /tmp/${contextenv}/

# Now, test extracting local tar archives
ADD context/tars/fil* /tars/
ADD context/tars/file.tar /tars_again

# 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/
12 changes: 12 additions & 0 deletions integration_tests/dockerfiles/config_test_add.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
[
{
"Image1": "gcr.io/kbuild-test/docker-test-add:latest",
"Image2": "gcr.io/kbuild-test/kbuild-test-add:latest",
"DiffType": "File",
"Diff": {
"Adds": null,
"Dels": null,
"Mods": null
}
}
]
8 changes: 8 additions & 0 deletions integration_tests/integration_test_yaml.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,14 @@ var fileTests = []struct {
kbuildContext: buildcontextPath,
repo: "test-workdir",
},
{
description: "test add",
dockerfilePath: "/workspace/integration_tests/dockerfiles/Dockerfile_test_add",
configPath: "/workspace/integration_tests/dockerfiles/config_test_add.json",
dockerContext: buildcontextPath,
kbuildContext: buildcontextPath,
repo: "test-add",
},
}

var structureTests = []struct {
Expand Down
140 changes: 140 additions & 0 deletions pkg/commands/add.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
/*
Copyright 2018 Google LLC

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package commands

import (
"github.com/GoogleCloudPlatform/k8s-container-builder/pkg/util"
"github.com/containers/image/manifest"
"github.com/docker/docker/builder/dockerfile/instructions"
"github.com/sirupsen/logrus"
"path/filepath"
"strings"
)

type AddCommand struct {
cmd *instructions.AddCommand
buildcontext string
snapshotFiles []string
}

// ExecuteCommand executes the ADD command
// Special stuff about ADD:
// 1. If <src> is a remote file URL:
// - destination will have permissions of 0600
// - If remote file has HTTP Last-Modified header, we set the mtime of the file to that timestamp
// - If dest doesn't end with a slash, the filepath is inferred to be <dest>/<filename>
// 2. If <src> is a local tar archive:
// -If <src> is a local tar archive, it is unpacked at the dest, as 'tar -x' would
func (a *AddCommand) ExecuteCommand(config *manifest.Schema2Config) error {
srcs := a.cmd.SourcesAndDest[:len(a.cmd.SourcesAndDest)-1]
dest := a.cmd.SourcesAndDest[len(a.cmd.SourcesAndDest)-1]

logrus.Infof("cmd: Add %s", srcs)
logrus.Infof("dest: %s", dest)

// First, resolve any environment replacement
resolvedEnvs, err := util.ResolveEnvironmentReplacementList(a.cmd.SourcesAndDest, config.Env, true)
if err != nil {
return err
}
dest = resolvedEnvs[len(resolvedEnvs)-1]
// Get a map of [src]:[files rooted at src]
srcMap, err := util.ResolveSources(resolvedEnvs, a.buildcontext)
if err != nil {
return err
}
// If any of the sources are local tar archives:
// 1. Unpack them to the specified destination
// 2. Remove it as a source that needs to be copied over
// If any of the sources is a remote file URL:
// 1. Download and copy it to the specifed dest
// 2. Remove it as a source that needs to be copied
for src, files := range srcMap {
for _, file := range files {
// If file is a local tar archive, then we unpack it to dest
filePath := filepath.Join(a.buildcontext, file)
isFilenameSource, err := isFilenameSource(srcMap, file)
if err != nil {
return err
}
if util.IsSrcRemoteFileURL(file) {
urlDest := util.URLDestinationFilepath(file, dest, config.WorkingDir)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Might be worth splitting these three cases into 3 helper functions to shorten ExecuteCommand a bit

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we need access to the file, srcMap, destination, and config I figured it would make more sense to keep it in this function, instead of passing all those in as arguments

logrus.Infof("Adding remote URL %s to %s", file, urlDest)
if err := util.DownloadFileToDest(file, urlDest); err != nil {
return err
}
a.snapshotFiles = append(a.snapshotFiles, urlDest)
delete(srcMap, src)
} else if isFilenameSource && util.IsFileLocalTarArchive(filePath) {
logrus.Infof("Unpacking local tar archive %s to %s", file, dest)
if err := util.UnpackLocalTarArchive(filePath, dest); err != nil {
return err
}
// Add the unpacked files to the snapshotter
filesAdded, err := util.Files(dest)
if err != nil {
return err
}
logrus.Debugf("Added %v from local tar archive %s", filesAdded, file)
a.snapshotFiles = append(a.snapshotFiles, filesAdded...)
delete(srcMap, src)
}
}
}
// With the remaining "normal" sources, create and execute a standard copy command
if len(srcMap) == 0 {
return nil
}
var regularSrcs []string
for src := range srcMap {
regularSrcs = append(regularSrcs, src)
}
copyCmd := CopyCommand{
cmd: &instructions.CopyCommand{
SourcesAndDest: append(regularSrcs, dest),
},
buildcontext: a.buildcontext,
}
if err := copyCmd.ExecuteCommand(config); err != nil {
return err
}
a.snapshotFiles = append(a.snapshotFiles, copyCmd.snapshotFiles...)
return nil
}

func isFilenameSource(srcMap map[string][]string, fileName string) (bool, error) {
for src := range srcMap {
matched, err := filepath.Match(src, fileName)
if err != nil {
return false, err
}
if matched || (src == fileName) {
return true, nil
}
}
return false, nil
}

// FilesToSnapshot should return an empty array if still nil; no files were changed
func (a *AddCommand) FilesToSnapshot() []string {
return a.snapshotFiles
}

// CreatedBy returns some information about the command for the image config
func (a *AddCommand) CreatedBy() string {
return strings.Join(a.cmd.SourcesAndDest, " ")
}
2 changes: 2 additions & 0 deletions pkg/commands/commands.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ func GetCommand(cmd instructions.Command, buildcontext string) (DockerCommand, e
return &EnvCommand{cmd: c}, nil
case *instructions.WorkdirCommand:
return &WorkdirCommand{cmd: c}, nil
case *instructions.AddCommand:
return &AddCommand{cmd: c, buildcontext: buildcontext}, nil
case *instructions.CmdCommand:
return &CmdCommand{cmd: c}, nil
case *instructions.EntrypointCommand:
Expand Down
45 changes: 44 additions & 1 deletion pkg/util/command_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ import (
"github.com/docker/docker/builder/dockerfile/shell"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
"net/http"
"net/url"
"os"
"path/filepath"
"strings"
Expand All @@ -31,6 +33,10 @@ import (
func ResolveEnvironmentReplacementList(values, envs []string, isFilepath bool) ([]string, error) {
var resolvedValues []string
for _, value := range values {
if IsSrcRemoteFileURL(value) {
resolvedValues = append(resolvedValues, value)
continue
}
resolved, err := ResolveEnvironmentReplacement(value, envs, isFilepath)
logrus.Debugf("Resolved %s to %s", value, resolved)
if err != nil {
Expand Down Expand Up @@ -106,13 +112,17 @@ func ResolveSources(srcsAndDest instructions.SourcesAndDest, root string) (map[s
func matchSources(srcs, files []string) ([]string, error) {
var matchedSources []string
for _, src := range srcs {
if IsSrcRemoteFileURL(src) {
matchedSources = append(matchedSources, src)
continue
}
src = filepath.Clean(src)
for _, file := range files {
matched, err := filepath.Match(src, file)
if err != nil {
return nil, err
}
if matched {
if matched || src == file {
matchedSources = append(matchedSources, file)
}
}
Expand Down Expand Up @@ -160,10 +170,31 @@ func DestinationFilepath(filename, srcName, dest, cwd, buildcontext string) (str
return filepath.Join(cwd, dest), nil
}

// URLDestinationFilepath gives the destination a file from a remote URL should be saved to
func URLDestinationFilepath(rawurl, dest, cwd string) string {
if !IsDestDir(dest) {
if !filepath.IsAbs(dest) {
return filepath.Join(cwd, dest)
}
return dest
}
urlBase := filepath.Base(rawurl)
destPath := filepath.Join(dest, urlBase)

if !filepath.IsAbs(dest) {
destPath = filepath.Join(cwd, destPath)
}
return destPath
}

// SourcesToFilesMap returns a map of [src]:[files rooted at source]
func SourcesToFilesMap(srcs []string, root string) (map[string][]string, error) {
srcMap := make(map[string][]string)
for _, src := range srcs {
if IsSrcRemoteFileURL(src) {
srcMap[src] = []string{src}
continue
}
src = filepath.Clean(src)
files, err := RelativeFiles(src, root)
if err != nil {
Expand Down Expand Up @@ -202,3 +233,15 @@ func IsSrcsValid(srcsAndDest instructions.SourcesAndDest, srcMap map[string][]st
}
return nil
}

func IsSrcRemoteFileURL(rawurl string) bool {
_, err := url.ParseRequestURI(rawurl)
if err != nil {
return false
}
_, err = http.Get(rawurl)
if err != nil {
return false
}
return true
}
Loading