Merge pull request #1298 from crosbymichael/1246-auth-request

* Registry: Do not require login unless 401 is received on push

commands.go

@@ -30,7 +30,8 @@ import (
 const VERSION = "0.5.0-dev"
 
 var (
-	GITCOMMIT string
+	GITCOMMIT         string
+	AuthRequiredError = fmt.Errorf("Authentication is required.")
 )
 
 func (cli *DockerCli) getMethod(name string) (reflect.Method, bool) {
@@ -827,10 +828,6 @@ func (cli *DockerCli) CmdPush(args ...string) error {
 		return nil
 	}
 
-	if err := cli.checkIfLogged("push"); err != nil {
-		return err
-	}
-
 	// 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
@@ -839,13 +836,22 @@ func (cli *DockerCli) CmdPush(args ...string) error {
 		return fmt.Errorf("Impossible to push a \"root\" repository. Please rename your repository in <user>/<repo> (ex: %s/%s)", cli.configFile.Configs[auth.IndexServerAddress()].Username, name)
 	}
 
-	buf, err := json.Marshal(cli.configFile.Configs[auth.IndexServerAddress()])
-	if err != nil {
-		return err
+	v := url.Values{}
+	push := func() error {
+		buf, err := json.Marshal(cli.configFile.Configs[auth.IndexServerAddress()])
+		if err != nil {
+			return err
+		}
+
+		return cli.stream("POST", "/images/"+name+"/push?"+v.Encode(), bytes.NewBuffer(buf), cli.out)
 	}
 
-	v := url.Values{}
-	if err := cli.stream("POST", "/images/"+name+"/push?"+v.Encode(), bytes.NewBuffer(buf), cli.out); err != nil {
+	if err := push(); err != nil {
+		if err == AuthRequiredError {
+			if err = cli.checkIfLogged("push"); err == nil {
+				return push()
+			}
+		}
 		return err
 	}
 	return nil
@@ -1572,6 +1578,9 @@ func (cli *DockerCli) stream(method, path string, in io.Reader, out io.Writer) e
 			} else if err != nil {
 				return err
 			}
+			if jm.Error != nil && jm.Error.Code == 401 {
+				return AuthRequiredError
+			}
 			jm.Display(out)
 		}
 	} else {

registry/registry.go

@@ -147,7 +147,7 @@ func (r *Registry) GetRemoteHistory(imgID, registry string, token []string) ([]s
 	res, err := doWithCookies(r.client, req)
 	if err != nil || res.StatusCode != 200 {
 		if res != nil {
-			return nil, fmt.Errorf("Internal server error: %d trying to fetch remote history for %s", res.StatusCode, imgID)
+			return nil, utils.NewHTTPRequestError(fmt.Sprintf("Internal server error: %d trying to fetch remote history for %s", res.StatusCode, imgID), res)
 		}
 		return nil, err
 	}
@@ -197,7 +197,7 @@ func (r *Registry) GetRemoteImageJSON(imgID, registry string, token []string) ([
 	}
 	defer res.Body.Close()
 	if res.StatusCode != 200 {
-		return nil, -1, fmt.Errorf("HTTP code %d", res.StatusCode)
+		return nil, -1, utils.NewHTTPRequestError(fmt.Sprintf("HTTP code %d", res.StatusCode), res)
 	}
 
 	imageSize, err := strconv.Atoi(res.Header.Get("X-Docker-Size"))
@@ -289,12 +289,12 @@ func (r *Registry) GetRepositoryData(indexEp, remote string) (*RepositoryData, e
 	}
 	defer res.Body.Close()
 	if res.StatusCode == 401 {
-		return nil, fmt.Errorf("Please login first (HTTP code %d)", res.StatusCode)
+		return nil, utils.NewHTTPRequestError(fmt.Sprintf("Please login first (HTTP code %d)", res.StatusCode), res)
 	}
 	// TODO: Right now we're ignoring checksums in the response body.
 	// In the future, we need to use them to check image validity.
 	if res.StatusCode != 200 {
-		return nil, fmt.Errorf("HTTP code: %d", res.StatusCode)
+		return nil, utils.NewHTTPRequestError(fmt.Sprintf("HTTP code: %d", res.StatusCode), res)
 	}
 
 	var tokens []string
@@ -391,7 +391,7 @@ func (r *Registry) PushImageJSONRegistry(imgData *ImgData, jsonRaw []byte, regis
 	if res.StatusCode != 200 {
 		errBody, err := ioutil.ReadAll(res.Body)
 		if err != nil {
-			return fmt.Errorf("HTTP code %d while uploading metadata and error when trying to parse response body: %s", res.StatusCode, err)
+			return utils.NewHTTPRequestError(fmt.Sprint("HTTP code %d while uploading metadata and error when trying to parse response body: %s", res.StatusCode, err), res)
 		}
 		var jsonBody map[string]string
 		if err := json.Unmarshal(errBody, &jsonBody); err != nil {
@@ -399,7 +399,7 @@ func (r *Registry) PushImageJSONRegistry(imgData *ImgData, jsonRaw []byte, regis
 		} else if jsonBody["error"] == "Image already exists" {
 			return ErrAlreadyExists
 		}
-		return fmt.Errorf("HTTP code %d while uploading metadata: %s", res.StatusCode, errBody)
+		return utils.NewHTTPRequestError(fmt.Sprintf("HTTP code %d while uploading metadata: %s", res.StatusCode, errBody), res)
 	}
 	return nil
 }
@@ -427,9 +427,9 @@ func (r *Registry) PushImageLayerRegistry(imgID string, layer io.Reader, registr
 	if res.StatusCode != 200 {
 		errBody, err := ioutil.ReadAll(res.Body)
 		if err != nil {
-			return "", fmt.Errorf("HTTP code %d while uploading metadata and error when trying to parse response body: %s", res.StatusCode, err)
+			return "", utils.NewHTTPRequestError(fmt.Sprintf("HTTP code %d while uploading metadata and error when trying to parse response body: %s", res.StatusCode, err), res)
 		}
-		return "", fmt.Errorf("Received HTTP code %d while uploading layer: %s", res.StatusCode, errBody)
+		return "", utils.NewHTTPRequestError(fmt.Sprintf("Received HTTP code %d while uploading layer: %s", res.StatusCode, errBody), res)
 	}
 	return tarsumLayer.Sum(jsonRaw), nil
 }
@@ -463,7 +463,7 @@ func (r *Registry) PushRegistryTag(remote, revision, tag, registry string, token
 	}
 	res.Body.Close()
 	if res.StatusCode != 200 && res.StatusCode != 201 {
-		return fmt.Errorf("Internal server error: %d trying to push tag %s on %s", res.StatusCode, tag, remote)
+		return utils.NewHTTPRequestError(fmt.Sprintf("Internal server error: %d trying to push tag %s on %s", res.StatusCode, tag, remote), res)
 	}
 	return nil
 }
@@ -540,7 +540,7 @@ func (r *Registry) PushImageJSONIndex(indexEp, remote string, imgList []*ImgData
 			if err != nil {
 				return nil, err
 			}
-			return nil, fmt.Errorf("Error: Status %d trying to push repository %s: %s", res.StatusCode, remote, errBody)
+			return nil, utils.NewHTTPRequestError(fmt.Sprintf("Error: Status %d trying to push repository %s: %s", res.StatusCode, remote, errBody), res)
 		}
 		if res.Header.Get("X-Docker-Token") != "" {
 			tokens = res.Header["X-Docker-Token"]
@@ -564,7 +564,7 @@ func (r *Registry) PushImageJSONIndex(indexEp, remote string, imgList []*ImgData
 			if err != nil {
 				return nil, err
 			}
-			return nil, fmt.Errorf("Error: Status %d trying to push checksums %s: %s", res.StatusCode, remote, errBody)
+			return nil, utils.NewHTTPRequestError(fmt.Sprintf("Error: Status %d trying to push checksums %s: %s", res.StatusCode, remote, errBody), res)
 		}
 	}
 
@@ -586,7 +586,7 @@ func (r *Registry) SearchRepositories(term string) (*SearchResults, error) {
 	}
 	defer res.Body.Close()
 	if res.StatusCode != 200 {
-		return nil, fmt.Errorf("Unexepected status code %d", res.StatusCode)
+		return nil, utils.NewHTTPRequestError(fmt.Sprintf("Unexepected status code %d", res.StatusCode), res)
 	}
 	rawData, err := ioutil.ReadAll(res.Body)
 	if err != nil {

utils/utils.go

@@ -607,12 +607,29 @@ func NewWriteFlusher(w io.Writer) *WriteFlusher {
 	return &WriteFlusher{w: w, flusher: flusher}
 }
 
+type JSONError struct {
+	Code    int    `json:"code,omitempty"`
+	Message string `json:"message,omitempty"`
+}
+
 type JSONMessage struct {
-	Status   string `json:"status,omitempty"`
-	Progress string `json:"progress,omitempty"`
-	Error    string `json:"error,omitempty"`
-	ID       string `json:"id,omitempty"`
-	Time     int64  `json:"time,omitempty"`
+	Status       string     `json:"status,omitempty"`
+	Progress     string     `json:"progress,omitempty"`
+	ErrorMessage string     `json:"error,omitempty"` //deprecated
+	ID           string     `json:"id,omitempty"`
+	Time         int64      `json:"time,omitempty"`
+	Error        *JSONError `json:"errorDetail,omitempty"`
+}
+
+func (e *JSONError) Error() string {
+	return e.Message
+}
+
+func NewHTTPRequestError(msg string, res *http.Response) error {
+	return &JSONError{
+		Message: msg,
+		Code:    res.StatusCode,
+	}
 }
 
 func (jm *JSONMessage) Display(out io.Writer) error {
@@ -621,8 +638,8 @@ func (jm *JSONMessage) Display(out io.Writer) error {
 	}
 	if jm.Progress != "" {
 		fmt.Fprintf(out, "%s %s\r", jm.Status, jm.Progress)
-	} else if jm.Error != "" {
-		return fmt.Errorf(jm.Error)
+	} else if jm.Error != nil {
+		return jm.Error
 	} else if jm.ID != "" {
 		fmt.Fprintf(out, "%s: %s\n", jm.ID, jm.Status)
 	} else {
@@ -656,7 +673,11 @@ func (sf *StreamFormatter) FormatStatus(format string, a ...interface{}) []byte
 func (sf *StreamFormatter) FormatError(err error) []byte {
 	sf.used = true
 	if sf.json {
-		if b, err := json.Marshal(&JSONMessage{Error: err.Error()}); err == nil {
+		jsonError, ok := err.(*JSONError)
+		if !ok {
+			jsonError = &JSONError{Message: err.Error()}
+		}
+		if b, err := json.Marshal(&JSONMessage{Error: jsonError, ErrorMessage: err.Error()}); err == nil {
 			return b
 		}
 		return []byte("{\"error\":\"format error\"}")

utils_test.go

@@ -191,7 +191,7 @@ func TestMergeConfig(t *testing.T) {
 	if len(configUser.Volumes) != 3 {
 		t.Fatalf("Expected 3 volumes, /test1, /test2 and /test3, found %d", len(configUser.Volumes))
 	}
-	for v, _ := range configUser.Volumes {
+	for v := range configUser.Volumes {
 		if v != "/test1" && v != "/test2" && v != "/test3" {
 			t.Fatalf("Expected /test1 or /test2 or /test3, found %s", v)
 		}