From 9537a3a1497efa32f9ec691b9787883179cadd2c Mon Sep 17 00:00:00 2001 From: Alberto Rodriguez Peon <alberto.rodriguez.peon@cern.ch> Date: Mon, 2 Jul 2018 11:43:00 +0200 Subject: [PATCH 1/7] Do not crash when adding a symlink --- docker.go | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/docker.go b/docker.go index bc1619b..2d1c9ea 100644 --- a/docker.go +++ b/docker.go @@ -106,7 +106,7 @@ func makeTarReader(workdir string) *bytes.Reader { defer tw.Close() log.Debug("Preparing to make Tar contents for Docker build") - filepath.Walk(workdir, func(file string, fi os.FileInfo, err error) error { + if err := filepath.Walk(workdir, func(file string, fi os.FileInfo, err error) error { // return on any error if err != nil { return err @@ -131,21 +131,25 @@ func makeTarReader(workdir string) *bytes.Reader { return nil } - // open files for taring - log.Debugf("Adding file %s to Tar contents", file) - f, err := os.Open(file) - defer f.Close() - if err != nil { - return err - } + // If it is a normal file e.g. not a symlink, add the file to the tar + if fi.Mode().IsRegular() { + log.Debugf("Adding file %s to Tar contents", file) + f, err := os.Open(file) + defer f.Close() + if err != nil { + return err + } - // copy file data into tar writer - if _, err := io.Copy(tw, f); err != nil { - return err + // copy file data into tar writer + if _, err := io.Copy(tw, f); err != nil { + return err + } } return nil - }) + }); err != nil { + log.Fatalf("Error generating tar file: %s", err) + } return bytes.NewReader(buffer.Bytes()) } -- GitLab From 6291af880215278399ad911bf9df52976196195e Mon Sep 17 00:00:00 2001 From: Alberto Rodriguez Peon <alberto.rodriguez.peon@cern.ch> Date: Mon, 2 Jul 2018 11:52:09 +0200 Subject: [PATCH 2/7] Add symlink test --- .gitlab-ci.yml | 16 ++++++++++++++++ test/symlinks/Dockerfile | 3 +++ test/symlinks/dest/testsymlink | 1 + test/symlinks/fromsymlink | 1 + 4 files changed, 21 insertions(+) create mode 100644 test/symlinks/Dockerfile create mode 100644 test/symlinks/dest/testsymlink create mode 120000 test/symlinks/fromsymlink diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index ba6ff9f..7bfcee3 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -137,6 +137,22 @@ test_with_artifact_dockerfile: - cat /folder/testfile - test $(cat /folder/testfile) == "testcontent" +# Build image with a symlink +build_symlinks_dockerfile: + <<: *build_test_image + variables: + TO: ${CI_REGISTRY_IMAGE}/test/symlinks:test_${CI_COMMIT_REF_NAME} + CONTEXT_DIR: test/symlinks + DOCKER_DEBUG: "true" + +test_symlinks_dockerfile: + <<: *verify_test_image + image: ${CI_REGISTRY_IMAGE}/test/symlinks:test_${CI_COMMIT_REF_NAME} + script: + - grep -qi alpine /etc/os-release + - test $(cat /test/fromsymlink) == "symlink" + - test -h /test/fromsymlink + # If all tests pass and we are running on master, retag the image to latest. This image will be used for user docker builds from now on. build_stable_image: stage: promote_tested_image_to_latest diff --git a/test/symlinks/Dockerfile b/test/symlinks/Dockerfile new file mode 100644 index 0000000..a3d2aa8 --- /dev/null +++ b/test/symlinks/Dockerfile @@ -0,0 +1,3 @@ +FROM alpine:latest + +COPY . /test diff --git a/test/symlinks/dest/testsymlink b/test/symlinks/dest/testsymlink new file mode 100644 index 0000000..d933faa --- /dev/null +++ b/test/symlinks/dest/testsymlink @@ -0,0 +1 @@ +symlink diff --git a/test/symlinks/fromsymlink b/test/symlinks/fromsymlink new file mode 120000 index 0000000..f6ea8d4 --- /dev/null +++ b/test/symlinks/fromsymlink @@ -0,0 +1 @@ +dest/testsymlink \ No newline at end of file -- GitLab From 09685594576b73d66bee3801958e19534b862f8b Mon Sep 17 00:00:00 2001 From: Alberto Rodriguez Peon <alberto.rodriguez.peon@cern.ch> Date: Mon, 2 Jul 2018 14:33:27 +0200 Subject: [PATCH 3/7] Support symlinks --- docker.go | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/docker.go b/docker.go index 2d1c9ea..2af9031 100644 --- a/docker.go +++ b/docker.go @@ -112,8 +112,20 @@ func makeTarReader(workdir string) *bytes.Reader { return err } + var link string + // If the file is a symlink, the link attribute is the destination + if fi.Mode()&os.ModeSymlink != 0 { + log.Debugf("Adding symlink %s to Tar contents", file) + link, err = os.Readlink(file) + if err != nil { + return err + } + } else { + // When a normal file, link is just the name of the file + link = fi.Name() + } // create a new dir/file header - header, err := tar.FileInfoHeader(fi, fi.Name()) + header, err := tar.FileInfoHeader(fi, link) if err != nil { return err } -- GitLab From 91f4478d057979f2f99ecdb667a1ccc0607f6186 Mon Sep 17 00:00:00 2001 From: Alberto Rodriguez Peon <alberto.rodriguez.peon@cern.ch> Date: Wed, 4 Jul 2018 15:16:00 +0200 Subject: [PATCH 4/7] Rewrite using new library --- docker.go | 121 ++++++++++++++++++++++-------------------------------- 1 file changed, 50 insertions(+), 71 deletions(-) diff --git a/docker.go b/docker.go index 2af9031..5e2c8e6 100644 --- a/docker.go +++ b/docker.go @@ -3,9 +3,6 @@ package main import ( "archive/tar" "bytes" - "context" - "encoding/base64" - "encoding/json" "fmt" "io" "io/ioutil" @@ -14,11 +11,8 @@ import ( "path/filepath" "strings" + docker "github.com/fsouza/go-dockerclient" log "github.com/sirupsen/logrus" - - "github.com/docker/docker/api/types" - "github.com/docker/docker/client" - "github.com/docker/docker/pkg/jsonmessage" ) const ( @@ -26,74 +20,50 @@ const ( dockerDebugEnvName = "DOCKER_DEBUG" ) -func buildImage(options map[string]*string, loginInfo map[string]types.AuthConfig, buildArgs map[string]*string) error { - cli, err := client.NewEnvClient() - if err != nil { - log.Printf("Unable to create docker client: %v", err) - return err - } - defer cli.Close() +func buildImage(client *docker.Client, options map[string]*string, loginInfo docker.AuthConfigurations, buildArgs []docker.BuildArg) error { noCache := options["NO_CACHE"] != nil && *options["NO_CACHE"] == "" log.Printf("Preparing local tar file of current dir '%s' for Docker daemon ...", *options["CONTEXT_DIR"]) - buildOptions := types.ImageBuildOptions{ - ForceRemove: true, - PullParent: true, - Dockerfile: *options["DOCKER_FILE"], - NoCache: noCache, - BuildArgs: buildArgs, - Tags: []string{*options["TO"]}, - AuthConfigs: loginInfo, - } - resp, err := cli.ImageBuild(context.Background(), makeTarReader(*options["CONTEXT_DIR"]), buildOptions) - if err != nil { - return err + + buildOptions := docker.BuildImageOptions{ + Name: *options["TO"], + Dockerfile: *options["DOCKER_FILE"], + Pull: true, // Always pull images + NoCache: noCache, + BuildArgs: buildArgs, + AuthConfigs: loginInfo, + OutputStream: os.Stdout, + ContextDir: *options["CONTEXT_DIR"], } - defer resp.Body.Close() - err = jsonmessage.DisplayJSONMessagesStream(resp.Body, os.Stdout, 0, true, nil) - if err != nil { + // resp, err := cli.ImageBuild(context.Background(), makeTarReader(*options["CONTEXT_DIR"]), buildOptions) + if err := client.BuildImage(buildOptions); err != nil { return err } - return nil } -func pushImage(options map[string]*string) error { - cli, err := client.NewEnvClient() +func pushImage(client *docker.Client, options map[string]*string, loginInfo docker.AuthConfigurations) error { - if err != nil { - log.Printf("Unable to create docker client: %v", err) - return err - } - defer cli.Close() + // Obtain registry name to pushImage + destinationImageFields := strings.Split(*options["TO"], "/") + registry := destinationImageFields[0] - // Prepare login to push the image - authObj := map[string]string{ - "username": *options["DOCKER_LOGIN_USERNAME"], - "password": *options["DOCKER_LOGIN_PASSWORD"], - } - json, err := json.Marshal(authObj) - if err != nil { - log.Fatal(err) - } - encodedAuth := base64.StdEncoding.EncodeToString(json) - - pushOptions := types.ImagePushOptions{ - RegistryAuth: encodedAuth, + pushOptions := docker.PushImageOptions{ + Name: *options["TO"], + Registry: registry, + OutputStream: os.Stdout, } - resp, err := cli.ImagePush(context.Background(), *options["TO"], pushOptions) - if err != nil { - return err + var authInfo docker.AuthConfiguration + if val, ok := loginInfo.Configs[registry]; ok { + authInfo = val + } else { + authInfo = docker.AuthConfiguration{} } - defer resp.Close() + client.PushImage(pushOptions, authInfo) - err = jsonmessage.DisplayJSONMessagesStream(resp, os.Stdout, 0, true, nil) - if err != nil { - return err - } return nil } @@ -212,17 +182,20 @@ func parseEnvVariables(defaults map[string]*string) map[string]*string { } // Obtain a types.AuthConfig object that can be use to build using a private image -func loginWithRegistry(username, password, server *string) map[string]types.AuthConfig { +func loginWithRegistry(username, password, server *string) docker.AuthConfigurations { // If no username and password, ignore auth - if username == nil || password == nil || server == nil { - return nil + authConfigs := docker.AuthConfigurations{ + Configs: make(map[string]docker.AuthConfiguration), } - loginInfo := make(map[string]types.AuthConfig) - loginInfo[*server] = types.AuthConfig{ - Username: *username, - Password: *password, + + // If no username and password, ignore auth + if !(username == nil || password == nil || server == nil) { + authConfigs.Configs[*server] = docker.AuthConfiguration{ + Username: *username, + Password: *password, + } } - return loginInfo + return authConfigs } //Prints useful information for the build @@ -271,9 +244,9 @@ func overrideFrom(dockerfile, newFrom *string) error { } // Obtain build args from environment variables starting with 'BUILD_ARG' -func parseBuildArgs() (map[string]*string, error) { +func parseBuildArgs() ([]docker.BuildArg, error) { - buildArgs := make(map[string]*string) + buildArgs := make([]docker.BuildArg, 0) // os.Environ returns a copy of strings representing the environment, // in the form "key=value". for _, env := range os.Environ() { @@ -282,7 +255,10 @@ func parseBuildArgs() (map[string]*string, error) { if len(comps) != 3 { return buildArgs, fmt.Errorf("Build arg '%s' is malformed. The value should be a key value pair", env) } - buildArgs[comps[1]] = &comps[2] + buildArgs = append(buildArgs, docker.BuildArg{ + Name: comps[1], + Value: comps[2], + }) } } return buildArgs, nil @@ -329,8 +305,11 @@ func main() { log.Fatal(err) } + endpoint := "unix:///var/run/docker.sock" + client, err := docker.NewClient(endpoint) + // Proceed to build the image - if err := buildImage(options, loginInfo, buildArgs); err != nil { + if err := buildImage(client, options, loginInfo, buildArgs); err != nil { log.Fatal(err) } log.Println("Build successfully completed!") @@ -338,7 +317,7 @@ func main() { // If all good building the image, push it to the registry if options["TO"] != nil { log.Printf("Pushing the built image to '%s'", *options["TO"]) - if err := pushImage(options); err != nil { + if err := pushImage(client, options, loginInfo); err != nil { log.Fatal(err) } log.Printf("Image successfully pushed to '%s'", *options["TO"]) -- GitLab From 33117b9daf45d6b32a43ca8291e2f03217f9be28 Mon Sep 17 00:00:00 2001 From: Alberto Rodriguez Peon <alberto.rodriguez.peon@cern.ch> Date: Wed, 4 Jul 2018 15:39:29 +0200 Subject: [PATCH 5/7] Add test for .dockerimage --- .gitlab-ci.yml | 18 +++++++ docker.go | 75 ---------------------------- test/with_dockerignore/.dockerignore | 1 + test/with_dockerignore/Dockerfile | 3 ++ test/with_dockerignore/ignore | 0 test/with_dockerignore/notignored | 1 + 6 files changed, 23 insertions(+), 75 deletions(-) create mode 100644 test/with_dockerignore/.dockerignore create mode 100644 test/with_dockerignore/Dockerfile create mode 100644 test/with_dockerignore/ignore create mode 100644 test/with_dockerignore/notignored diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 7bfcee3..c111767 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -153,6 +153,24 @@ test_symlinks_dockerfile: - test $(cat /test/fromsymlink) == "symlink" - test -h /test/fromsymlink +# Build image with a .dockerignore file +build_dockerignore_dockerfile: + <<: *build_test_image + variables: + TO: ${CI_REGISTRY_IMAGE}/test/dockerignore:test_${CI_COMMIT_REF_NAME} + CONTEXT_DIR: test/with_dockerignore + DOCKER_DEBUG: "true" + +test_dockerignore_dockerfile: + <<: *verify_test_image + image: ${CI_REGISTRY_IMAGE}/test/dockerignore:test_${CI_COMMIT_REF_NAME} + script: + - grep -qi alpine /etc/os-release + - test -e /test/notignored + - test $(cat /test/notignored) == "file" + # Verify the file in the .dockerignore file is not present + - test ! -e /test/ignored + # If all tests pass and we are running on master, retag the image to latest. This image will be used for user docker builds from now on. build_stable_image: stage: promote_tested_image_to_latest diff --git a/docker.go b/docker.go index 5e2c8e6..0263f2e 100644 --- a/docker.go +++ b/docker.go @@ -1,14 +1,10 @@ package main import ( - "archive/tar" - "bytes" "fmt" - "io" "io/ioutil" "os" "path" - "path/filepath" "strings" docker "github.com/fsouza/go-dockerclient" @@ -24,8 +20,6 @@ func buildImage(client *docker.Client, options map[string]*string, loginInfo doc noCache := options["NO_CACHE"] != nil && *options["NO_CACHE"] == "" - log.Printf("Preparing local tar file of current dir '%s' for Docker daemon ...", *options["CONTEXT_DIR"]) - buildOptions := docker.BuildImageOptions{ Name: *options["TO"], Dockerfile: *options["DOCKER_FILE"], @@ -67,75 +61,6 @@ func pushImage(client *docker.Client, options map[string]*string, loginInfo dock return nil } -// makeTarReader - Making memory tar for image build context -// Heavily based on https://github.com/da4nik/ssci/blob/42e0b9c6d93bda94284a2d73343d22dbc9a7e75a/ci/build.go#L48 -func makeTarReader(workdir string) *bytes.Reader { - buffer := new(bytes.Buffer) - - tw := tar.NewWriter(buffer) - defer tw.Close() - - log.Debug("Preparing to make Tar contents for Docker build") - if err := filepath.Walk(workdir, func(file string, fi os.FileInfo, err error) error { - // return on any error - if err != nil { - return err - } - - var link string - // If the file is a symlink, the link attribute is the destination - if fi.Mode()&os.ModeSymlink != 0 { - log.Debugf("Adding symlink %s to Tar contents", file) - link, err = os.Readlink(file) - if err != nil { - return err - } - } else { - // When a normal file, link is just the name of the file - link = fi.Name() - } - // create a new dir/file header - header, err := tar.FileInfoHeader(fi, link) - if err != nil { - return err - } - - // update the name to correctly reflect the desired destination when untaring - header.Name = strings.TrimPrefix(strings.Replace(file, workdir, "", -1), string(filepath.Separator)) - - // write the header - if err = tw.WriteHeader(header); err != nil { - return err - } - - // return on directories since there will be no content to tar - if fi.Mode().IsDir() { - return nil - } - - // If it is a normal file e.g. not a symlink, add the file to the tar - if fi.Mode().IsRegular() { - log.Debugf("Adding file %s to Tar contents", file) - f, err := os.Open(file) - defer f.Close() - if err != nil { - return err - } - - // copy file data into tar writer - if _, err := io.Copy(tw, f); err != nil { - return err - } - } - - return nil - }); err != nil { - log.Fatalf("Error generating tar file: %s", err) - } - - return bytes.NewReader(buffer.Bytes()) -} - // Helper function to convert a string into a pointer of a string func getPointer(s string) *string { return &s diff --git a/test/with_dockerignore/.dockerignore b/test/with_dockerignore/.dockerignore new file mode 100644 index 0000000..8485e98 --- /dev/null +++ b/test/with_dockerignore/.dockerignore @@ -0,0 +1 @@ +ignore \ No newline at end of file diff --git a/test/with_dockerignore/Dockerfile b/test/with_dockerignore/Dockerfile new file mode 100644 index 0000000..a3d2aa8 --- /dev/null +++ b/test/with_dockerignore/Dockerfile @@ -0,0 +1,3 @@ +FROM alpine:latest + +COPY . /test diff --git a/test/with_dockerignore/ignore b/test/with_dockerignore/ignore new file mode 100644 index 0000000..e69de29 diff --git a/test/with_dockerignore/notignored b/test/with_dockerignore/notignored new file mode 100644 index 0000000..1a010b1 --- /dev/null +++ b/test/with_dockerignore/notignored @@ -0,0 +1 @@ +file \ No newline at end of file -- GitLab From c3e4b001ce33e2347fc31d280fa300120bf338d8 Mon Sep 17 00:00:00 2001 From: Alberto Rodriguez Peon <alberto.rodriguez.peon@cern.ch> Date: Wed, 4 Jul 2018 16:21:47 +0200 Subject: [PATCH 6/7] Allow override of docker host via env variable --- docker.go | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/docker.go b/docker.go index 0263f2e..463bdf4 100644 --- a/docker.go +++ b/docker.go @@ -14,6 +14,7 @@ import ( const ( flagFile = "./runonce" dockerDebugEnvName = "DOCKER_DEBUG" + dockerSocket = "unix:///var/run/docker.sock" ) func buildImage(client *docker.Client, options map[string]*string, loginInfo docker.AuthConfigurations, buildArgs []docker.BuildArg) error { @@ -230,8 +231,13 @@ func main() { log.Fatal(err) } - endpoint := "unix:///var/run/docker.sock" - client, err := docker.NewClient(endpoint) + // Allow users to override the docker host via DOCKER_HOST environment variable + // This would allow to work with DinD in a service container for instance + dockerHost := dockerSocket + if env, present := os.LookupEnv("DOCKER_HOST"); present { + dockerHost = env + } + client, err := docker.NewClient(dockerHost) // Proceed to build the image if err := buildImage(client, options, loginInfo, buildArgs); err != nil { -- GitLab From 1cf454c276425e5856e1ac6d4287cce5ee7cd5b7 Mon Sep 17 00:00:00 2001 From: Alberto Rodriguez Peon <alberto.rodriguez.peon@cern.ch> Date: Wed, 4 Jul 2018 16:35:00 +0200 Subject: [PATCH 7/7] Improve comment about requirement to always pull images --- docker.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/docker.go b/docker.go index 463bdf4..c4ccb4c 100644 --- a/docker.go +++ b/docker.go @@ -22,9 +22,11 @@ func buildImage(client *docker.Client, options map[string]*string, loginInfo doc noCache := options["NO_CACHE"] != nil && *options["NO_CACHE"] == "" buildOptions := docker.BuildImageOptions{ - Name: *options["TO"], - Dockerfile: *options["DOCKER_FILE"], - Pull: true, // Always pull images + Name: *options["TO"], + Dockerfile: *options["DOCKER_FILE"], + // Always pull image even if locally accessible. This is absolutely necessary to protect + // private source images from being reused if present already. + Pull: true, NoCache: noCache, BuildArgs: buildArgs, AuthConfigs: loginInfo, -- GitLab