Merge pull request #1158 from cespare/1142-docker-add-fix

*Buildfile: determine a filename from a URL if the destination is a directory

buildfile.go

@@ -7,6 +7,7 @@ import (
 	"github.com/dotcloud/docker/utils"
 	"io"
 	"io/ioutil"
+	"net/url"
 	"os"
 	"path"
 	"reflect"
@@ -201,6 +202,24 @@ func (b *buildFile) addRemote(container *Container, orig, dest string) error {
 	}
 	defer file.Body.Close()
 
+	// If the destination is a directory, figure out the filename.
+	if strings.HasSuffix(dest, "/") {
+		u, err := url.Parse(orig)
+		if err != nil {
+			return err
+		}
+		path := u.Path
+		if strings.HasSuffix(path, "/") {
+			path = path[:len(path)-1]
+		}
+		parts := strings.Split(path, "/")
+		filename := parts[len(parts)-1]
+		if filename == "" {
+			return fmt.Errorf("cannot determine filename from url: %s", u)
+		}
+		dest = dest + filename
+	}
+
 	return container.Inject(file.Body, dest)
 }
 
@@ -208,7 +227,7 @@ func (b *buildFile) addContext(container *Container, orig, dest string) error {
 	origPath := path.Join(b.context, orig)
 	destPath := path.Join(container.RootfsPath(), dest)
 	// Preserve the trailing '/'
-	if dest[len(dest)-1] == '/' {
+	if strings.HasSuffix(dest, "/") {
 		destPath = destPath + "/"
 	}
 	fi, err := os.Stat(origPath)

buildfile_test.go

@@ -3,13 +3,17 @@ package docker
 import (
 	"fmt"
 	"io/ioutil"
+	"net"
+	"net/http"
+	"net/http/httptest"
+	"strings"
 	"testing"
 )
 
 // mkTestContext generates a build context from the contents of the provided dockerfile.
 // This context is suitable for use as an argument to BuildFile.Build()
 func mkTestContext(dockerfile string, files [][2]string, t *testing.T) Archive {
-	context, err := mkBuildContext(fmt.Sprintf(dockerfile, unitTestImageID), files)
+	context, err := mkBuildContext(dockerfile, files)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -22,6 +26,8 @@ type testContextTemplate struct {
 	dockerfile string
 	// Additional files in the context, eg [][2]string{"./passwd", "gordon"}
 	files [][2]string
+	// Additional remote files to host on a local HTTP server.
+	remoteFiles [][2]string
 }
 
 // A table of all the contexts to build and test.
@@ -29,27 +35,31 @@ type testContextTemplate struct {
 var testContexts = []testContextTemplate{
 	{
 		`
-from   %s
+from   {IMAGE}
 run    sh -c 'echo root:testpass > /tmp/passwd'
 run    mkdir -p /var/run/sshd
 run    [ "$(cat /tmp/passwd)" = "root:testpass" ]
 run    [ "$(ls -d /var/run/sshd)" = "/var/run/sshd" ]
 `,
 		nil,
+		nil,
 	},
 
 	{
 		`
-from %s
+from {IMAGE}
 add foo /usr/lib/bla/bar
-run [ "$(cat /usr/lib/bla/bar)" = 'hello world!' ]
+run [ "$(cat /usr/lib/bla/bar)" = 'hello' ]
+add http://{SERVERADDR}/baz /usr/lib/baz/quux
+run [ "$(cat /usr/lib/baz/quux)" = 'world!' ]
 `,
-		[][2]string{{"foo", "hello world!"}},
+		[][2]string{{"foo", "hello"}},
+		[][2]string{{"/baz", "world!"}},
 	},
 
 	{
 		`
-from %s
+from {IMAGE}
 add f /
 run [ "$(cat /f)" = "hello" ]
 add f /abc
@@ -71,38 +81,86 @@ run [ "$(cat /somewheeeere/over/the/rainbooow/ga)" = "bu" ]
 			{"f", "hello"},
 			{"d/ga", "bu"},
 		},
+		nil,
 	},
 
 	{
 		`
-from %s
+from {IMAGE}
+add http://{SERVERADDR}/x /a/b/c
+run [ "$(cat /a/b/c)" = "hello" ]
+add http://{SERVERADDR}/x?foo=bar /
+run [ "$(cat /x)" = "hello" ]
+add http://{SERVERADDR}/x /d/
+run [ "$(cat /d/x)" = "hello" ]
+add http://{SERVERADDR} /e
+run [ "$(cat /e)" = "blah" ]
+`,
+		nil,
+		[][2]string{{"/x", "hello"}, {"/", "blah"}},
+	},
+
+	{
+		`
+from   {IMAGE}
 env    FOO BAR
 run    [ "$FOO" = "BAR" ]
 `,
 		nil,
-	},
-
-	{
-		`
-from %s
-ENTRYPOINT /bin/echo
-CMD Hello world
-`,
 		nil,
 	},
 
 	{
 		`
-from %s
+from {IMAGE}
+ENTRYPOINT /bin/echo
+CMD Hello world
+`,
+		nil,
+		nil,
+	},
+
+	{
+		`
+from {IMAGE}
 VOLUME /test
 CMD Hello world
 `,
 		nil,
+		nil,
 	},
 }
 
 // FIXME: test building with 2 successive overlapping ADD commands
 
+func constructDockerfile(template string, ip net.IP, port string) string {
+	serverAddr := fmt.Sprintf("%s:%s", ip, port)
+	replacer := strings.NewReplacer("{IMAGE}", unitTestImageID, "{SERVERADDR}", serverAddr)
+	return replacer.Replace(template)
+}
+
+func mkTestingFileServer(files [][2]string) (*httptest.Server, error) {
+	mux := http.NewServeMux()
+	for _, file := range files {
+		name, contents := file[0], file[1]
+		mux.HandleFunc(name, func(w http.ResponseWriter, r *http.Request) {
+			w.Write([]byte(contents))
+		})
+	}
+
+	// This is how httptest.NewServer sets up a net.Listener, except that our listener must accept remote
+	// connections (from the container).
+	listener, err := net.Listen("tcp", ":0")
+	if err != nil {
+		return nil, err
+	}
+
+	s := httptest.NewUnstartedServer(mux)
+	s.Listener = listener
+	s.Start()
+	return s, nil
+}
+
 func TestBuild(t *testing.T) {
 	for _, ctx := range testContexts {
 		buildImage(ctx, t)
@@ -121,9 +179,24 @@ func buildImage(context testContextTemplate, t *testing.T) *Image {
 		pullingPool: make(map[string]struct{}),
 		pushingPool: make(map[string]struct{}),
 	}
-	buildfile := NewBuildFile(srv, ioutil.Discard, false)
 
-	id, err := buildfile.Build(mkTestContext(context.dockerfile, context.files, t))
+	httpServer, err := mkTestingFileServer(context.remoteFiles)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer httpServer.Close()
+
+	idx := strings.LastIndex(httpServer.URL, ":")
+	if idx < 0 {
+		t.Fatalf("could not get port from test http server address %s", httpServer.URL)
+	}
+	port := httpServer.URL[idx+1:]
+
+	ip := runtime.networkManager.bridgeNetwork.IP
+	dockerfile := constructDockerfile(context.dockerfile, ip, port)
+
+	buildfile := NewBuildFile(srv, ioutil.Discard, false)
+	id, err := buildfile.Build(mkTestContext(dockerfile, context.files, t))
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -137,10 +210,10 @@ func buildImage(context testContextTemplate, t *testing.T) *Image {
 
 func TestVolume(t *testing.T) {
 	img := buildImage(testContextTemplate{`
-        from %s
+        from {IMAGE}
         volume /test
         cmd Hello world
-    `, nil}, t)
+    `, nil, nil}, t)
 
 	if len(img.Config.Volumes) == 0 {
 		t.Fail()
@@ -154,9 +227,9 @@ func TestVolume(t *testing.T) {
 
 func TestBuildMaintainer(t *testing.T) {
 	img := buildImage(testContextTemplate{`
-        from %s
+        from {IMAGE}
         maintainer dockerio
-    `, nil}, t)
+    `, nil, nil}, t)
 
 	if img.Author != "dockerio" {
 		t.Fail()
@@ -165,10 +238,10 @@ func TestBuildMaintainer(t *testing.T) {
 
 func TestBuildEnv(t *testing.T) {
 	img := buildImage(testContextTemplate{`
-        from %s
+        from {IMAGE}
         env port 4243
         `,
-		nil}, t)
+		nil, nil}, t)
 
 	if img.Config.Env[0] != "port=4243" {
 		t.Fail()
@@ -177,10 +250,10 @@ func TestBuildEnv(t *testing.T) {
 
 func TestBuildCmd(t *testing.T) {
 	img := buildImage(testContextTemplate{`
-        from %s
+        from {IMAGE}
         cmd ["/bin/echo", "Hello World"]
         `,
-		nil}, t)
+		nil, nil}, t)
 
 	if img.Config.Cmd[0] != "/bin/echo" {
 		t.Log(img.Config.Cmd[0])
@@ -194,10 +267,10 @@ func TestBuildCmd(t *testing.T) {
 
 func TestBuildExpose(t *testing.T) {
 	img := buildImage(testContextTemplate{`
-        from %s
+        from {IMAGE}
         expose 4243
         `,
-		nil}, t)
+		nil, nil}, t)
 
 	if img.Config.PortSpecs[0] != "4243" {
 		t.Fail()
@@ -206,10 +279,10 @@ func TestBuildExpose(t *testing.T) {
 
 func TestBuildEntrypoint(t *testing.T) {
 	img := buildImage(testContextTemplate{`
-        from %s
+        from {IMAGE}
         entrypoint ["/bin/echo"]
         `,
-		nil}, t)
+		nil, nil}, t)
 
 	if img.Config.Entrypoint[0] != "/bin/echo" {
 	}

docs/sources/use/builder.rst

@@ -152,6 +152,14 @@ destination container.
 
 The copy obeys the following rules:
 
+* If ``<src>`` is a URL and ``<dest>`` does not end with a trailing slash,
+  then a file is downloaded from the URL and copied to ``<dest>``.
+* If ``<src>`` is a URL and ``<dest>`` does end with a trailing slash,
+  then the filename is inferred from the URL and the file is downloaded to
+  ``<dest>/<filename>``. For instance, ``ADD http://example.com/foobar /``
+  would create the file ``/foobar``. The URL must have a nontrivial path
+  so that an appropriate filename can be discovered in this case
+  (``http://example.com`` will not work).
 * If ``<src>`` is a directory, the entire directory is copied,
   including filesystem metadata.
 * If ``<src>``` is a tar archive in a recognized compression format