Blob Blame History Raw
From 0d3577e545a067205fc92070064875a52361d0a5 Mon Sep 17 00:00:00 2001
From: Michal Minar <miminar@redhat.com>
Date: Mon, 4 May 2015 17:39:31 +0200
Subject: [PATCH] Fixed push of unqualified registry

Local image with unqualified name had to be fully qualified before a
push to default registry.

Moved check for official repository to daemon side because client
doesn't know which registry is the default.

Signed-off-by: Michal Minar <miminar@redhat.com>
---
 api/client/commands.go                  |  13 ---
 graph/push.go                           |  30 +++++-
 integration-cli/docker_cli_push_test.go | 156 ++++++++++++++++++++++++++------
 3 files changed, 156 insertions(+), 43 deletions(-)

diff --git a/api/client/commands.go b/api/client/commands.go
index f145fc4..5099dba 100644
--- a/api/client/commands.go
+++ b/api/client/commands.go
@@ -1311,19 +1311,6 @@ func (cli *DockerCli) CmdPush(args ...string) error {
 	if err != nil {
 		return err
 	}
-	// Resolve the Auth config relevant for this server
-	authConfig := cli.configFile.ResolveAuthConfig(repoInfo.Index)
-	// If we're not using a custom registry, we know the restrictions
-	// applied to repository names and can warn the user in advance.
-	// Custom repositories can have different rules, and we must also
-	// allow pushing by image ID.
-	if repoInfo.Official {
-		username := authConfig.Username
-		if username == "" {
-			username = "<user>"
-		}
-		return fmt.Errorf("You cannot push a \"root\" repository. Please rename your repository to <user>/<repo> (ex: %s/%s)", username, repoInfo.LocalName)
-	}
 
 	v := url.Values{}
 	v.Set("tag", tag)
diff --git a/graph/push.go b/graph/push.go
index f61fb43..fa348cd 100644
--- a/graph/push.go
+++ b/graph/push.go
@@ -494,6 +494,7 @@ func (s *TagStore) CmdPush(job *engine.Job) engine.Status {
 		sf          = utils.NewStreamFormatter(job.GetenvBool("json"))
 		authConfig  = &registry.AuthConfig{}
 		metaHeaders map[string][]string
+		localRepo   Repository
 	)
 
 	// Resolve the Repository name from fqn to RepositoryInfo
@@ -513,6 +514,23 @@ func (s *TagStore) CmdPush(job *engine.Job) engine.Status {
 		log.Infof("Push of %s to official registry has been forced", localName)
 	}
 
+	// If we're not using a custom registry, we know the restrictions
+	// applied to repository names and can warn the user in advance.
+	// Custom repositories can have different rules, and we must also
+	// allow pushing by image ID.
+	if repoInfo.Official {
+		username := authConfig.Username
+		if username == "" {
+			username = "<user>"
+		}
+		name := localName
+		parts := strings.Split(repoInfo.LocalName, "/")
+		if len(parts) > 0 {
+			name = parts[len(parts)-1]
+		}
+		return job.Errorf("You cannot push a \"root\" repository. Please rename your repository to <user>/<repo> (ex: %s/%s)", username, name)
+	}
+
 	if _, err := s.poolAdd("push", repoInfo.LocalName); err != nil {
 		return job.Error(err)
 	}
@@ -533,10 +551,14 @@ func (s *TagStore) CmdPush(job *engine.Job) engine.Status {
 		reposLen = len(s.Repositories[repoInfo.LocalName])
 	}
 	job.Stdout.Write(sf.FormatStatus("", "The push refers to a repository [%s] (len: %d)", repoInfo.CanonicalName, reposLen))
-	// If it fails, try to get the repository
-	localRepo, exists := s.Repositories[repoInfo.LocalName]
-	if !exists {
-		return job.Errorf("Repository does not exist: %s", repoInfo.LocalName)
+	matching := s.getRepositoryList(localName)
+	for _, namedRepo := range matching {
+		for _, localRepo = range namedRepo {
+			break
+		}
+	}
+	if localRepo == nil {
+		return job.Errorf("Repository does not exist: %s", localName)
 	}
 
 	if repoInfo.Index.Official || endpoint.Version == registry.APIVersion2 {
diff --git a/integration-cli/docker_cli_push_test.go b/integration-cli/docker_cli_push_test.go
index a84e298..87821c0 100644
--- a/integration-cli/docker_cli_push_test.go
+++ b/integration-cli/docker_cli_push_test.go
@@ -7,6 +7,7 @@ import (
 	"io/ioutil"
 	"os"
 	"os/exec"
+	"regexp"
 	"strings"
 	"testing"
 	"time"
@@ -14,6 +15,11 @@ import (
 	"github.com/docker/docker/vendor/src/code.google.com/p/go/src/pkg/archive/tar"
 )
 
+const (
+	confirmText  = "want to push to public registry? [y/n]"
+	farewellText = "nothing pushed."
+)
+
 // pulling an image from the central registry should work
 func TestPushBusyboxImage(t *testing.T) {
 	defer setupRegistry(t)()
@@ -159,11 +165,34 @@ func TestPushEmptyLayer(t *testing.T) {
 	logDone("push - empty layer config to private registry")
 }
 
+func readConfirmText(t *testing.T, out *bufio.Reader) {
+	done := make(chan struct{})
+	go func() {
+		line, err := out.ReadBytes(']')
+		if err != nil {
+			t.Fatalf("Failed to read a confirmation text for a push: %v", err)
+		}
+		if !strings.HasSuffix(strings.ToLower(string(line)), confirmText) {
+			t.Fatalf("Expected confirmation text %q, not: %q", confirmText, line)
+		}
+		buf := make([]byte, 4)
+		n, err := out.Read(buf)
+		if err != nil {
+			t.Fatalf("Failed to read confirmation text for a push: %v", err)
+		}
+		if n > 2 || n < 1 || buf[0] != ':' {
+			t.Fatalf("Got unexpected line ending: %q", string(buf))
+		}
+		close(done)
+	}()
+	select {
+	case <-done:
+	case <-time.After(4 * time.Second):
+		t.Fatalf("Timeout while waiting on confirmation text.")
+	}
+}
+
 func TestPushToPublicRegistry(t *testing.T) {
-	const (
-		confirmText  = "want to push to public registry? [y/n]"
-		farewellText = "nothing pushed."
-	)
 	repoName := "docker.io/dockercli/busybox"
 	// tag the image to upload it to the private registry
 	tagCmd := exec.Command(dockerBinary, "tag", "busybox", repoName)
@@ -193,33 +222,16 @@ func TestPushToPublicRegistry(t *testing.T) {
 
 		outReader := bufio.NewReader(stdout)
 
-		readConfirmText := func(out *bufio.Reader) {
-			line, err := out.ReadBytes(']')
-			if err != nil {
-				t.Fatalf("Failed to read a confirmation text for a push: %v", err)
-			}
-			if !strings.HasSuffix(strings.ToLower(string(line)), confirmText) {
-				t.Fatalf("Expected confirmation text %q, not: %q", confirmText, line)
-			}
-			buf := make([]byte, 4)
-			n, err := out.Read(buf)
-			if err != nil {
-				t.Fatalf("Failed to read confirmation text for a push: %v", err)
-			}
-			if n > 2 || n < 1 || buf[0] != ':' {
-				t.Fatalf("Got unexpected line ending: %q", string(buf))
-			}
-		}
-		readConfirmText(outReader)
+		readConfirmText(t, outReader)
 
 		stdin.Write([]byte("\n"))
-		readConfirmText(outReader)
+		readConfirmText(t, outReader)
 		stdin.Write([]byte("  \n"))
-		readConfirmText(outReader)
+		readConfirmText(t, outReader)
 		stdin.Write([]byte("foo\n"))
-		readConfirmText(outReader)
+		readConfirmText(t, outReader)
 		stdin.Write([]byte("no\n"))
-		readConfirmText(outReader)
+		readConfirmText(t, outReader)
 		if sayNo {
 			stdin.Write([]byte(" n \n"))
 		} else {
@@ -282,3 +294,95 @@ func TestPushToPublicRegistry(t *testing.T) {
 
 	logDone("push - to public registry")
 }
+
+func TestPushToAdditionalRegistry(t *testing.T) {
+	reg := setupAndGetRegistryAt(t, privateRegistryURLs[0])
+	defer reg.Close()
+	d := NewDaemon(t)
+	if err := d.StartWithBusybox("--add-registry=" + reg.url); err != nil {
+		t.Fatalf("We should have been able to start the daemon with passing add-registry=%s: %v", reg.url, err)
+	}
+	defer d.Stop()
+
+	busyboxId := d.getAndTestImageEntry(t, 1, "busybox", "").id
+
+	// push busybox to additional registry as "library/busybox" and remove all local images
+	if out, err := d.Cmd("tag", "busybox", "library/busybox"); err != nil {
+		t.Fatalf("Failed to tag image %s: error %v, output %q", "busybox", err, out)
+	}
+	if out, err := d.Cmd("push", "library/busybox"); err != nil {
+		t.Fatalf("Failed to push image library/busybox: error %v, output %q", err, out)
+	}
+	toRemove := []string{"busybox", "library/busybox"}
+	if out, err := d.Cmd("rmi", toRemove...); err != nil {
+		t.Fatalf("Failed to remove images %v: %v, output: %s", toRemove, err, out)
+	}
+	d.getAndTestImageEntry(t, 0, "", "")
+
+	// pull it from additional registry
+	if _, err := d.Cmd("pull", "library/busybox"); err != nil {
+		t.Fatalf("We should have been able to pull library/busybox from %q: %v", reg.url, err)
+	}
+	d.getAndTestImageEntry(t, 1, reg.url+"/library/busybox", busyboxId)
+
+	logDone("push - to additional registry")
+}
+
+func TestPushOfficialImage(t *testing.T) {
+	var reErr = regexp.MustCompile(`rename your repository to[^:]*:\s*<user>/busybox\b`)
+
+	// push busybox to public registry as "library/busybox"
+	cmd := exec.Command(dockerBinary, "push", "library/busybox")
+	stdin, err := cmd.StdinPipe()
+	if err != nil {
+		t.Fatalf("Failed to get stdin pipe for process: %v", err)
+	}
+	stdout, err := cmd.StdoutPipe()
+	if err != nil {
+		t.Fatalf("Failed to get stdout pipe for process: %v", err)
+	}
+	stderr, err := cmd.StderrPipe()
+	if err != nil {
+		t.Fatalf("Failed to get stderr pipe for process: %v", err)
+	}
+	if err := cmd.Start(); err != nil {
+		t.Fatalf("Failed to start pushing to public registry: %v", err)
+	}
+	outReader := bufio.NewReader(stdout)
+	readConfirmText(t, outReader)
+	stdin.Write([]byte{'Y', '\n'})
+
+	errReader := bufio.NewReader(stderr)
+	line, isPrefix, err := errReader.ReadLine()
+	if err != nil {
+		t.Fatalf("Failed to read farewell: %v", err)
+	}
+	if isPrefix {
+		t.Errorf("Got unexpectedly long output.")
+	}
+	if !reErr.Match(line) {
+		t.Errorf("Got unexpected output %q", line)
+	}
+	if line, _, err = outReader.ReadLine(); err != io.EOF {
+		t.Errorf("Expected EOF, not: %q", line)
+	}
+	for ; err != io.EOF; line, _, err = errReader.ReadLine() {
+		t.Errorf("Expected no message on stderr, got: %q", string(line))
+	}
+
+	// Wait for command to finish with short timeout.
+	finish := make(chan struct{})
+	go func() {
+		if err := cmd.Wait(); err == nil {
+			t.Error("Push command should have failed.")
+		}
+		close(finish)
+	}()
+	select {
+	case <-finish:
+	case <-time.After(1 * time.Second):
+		t.Fatalf("Docker push failed to exit.")
+	}
+
+	logDone("push - official image")
+}
-- 
2.1.0