Add code to return message field of returned registry errors

Docker registries return errors in a know format so this change now checks for these
errors and returns the message field. If the error is not in the expected format fall
back to the original behaviour.

https://github.com/containerd/containerd/issues/3076

Signed-off-by: Jack Baines <jack.baines@uk.ibm.com>

remotes/docker/fetcher.go

@@ -18,6 +18,7 @@ package docker
 
 import (
 	"context"
+	"encoding/json"
 	"fmt"
 	"io"
 	"io/ioutil"
@@ -25,6 +26,8 @@ import (
 	"path"
 	"strings"
 
+	"github.com/docker/distribution/registry/api/errcode"
+
 	"github.com/containerd/containerd/errdefs"
 	"github.com/containerd/containerd/images"
 	"github.com/containerd/containerd/log"
@@ -102,10 +105,18 @@ func (r dockerFetcher) open(ctx context.Context, u, mediatype string, offset int
 		// can discard the bytes, hiding the seek behavior from the
 		// implementation.
 
-		resp.Body.Close()
+		defer resp.Body.Close()
 		if resp.StatusCode == http.StatusNotFound {
 			return nil, errors.Wrapf(errdefs.ErrNotFound, "content at %v not found", u)
 		}
+		body, err := ioutil.ReadAll(resp.Body)
+		if err == nil {
+			dockerErr := errcode.Errors{}
+			err := json.Unmarshal(body, &dockerErr)
+			if err == nil && dockerErr.Len() > 0 {
+				return nil, errors.Errorf("unexpected status code %v: %s - Server message: %s", u, resp.Status, dockerErr.Error())
+			}
+		}
 		return nil, errors.Errorf("unexpected status code %v: %v", u, resp.Status)
 	}
 	if offset > 0 {

remotes/docker/fetcher_test.go

@@ -18,12 +18,18 @@ package docker
 
 import (
 	"context"
+	"encoding/json"
 	"fmt"
+	"io"
 	"io/ioutil"
 	"math/rand"
 	"net/http"
 	"net/http/httptest"
 	"testing"
+
+	"github.com/docker/distribution/registry/api/errcode"
+	"github.com/pkg/errors"
+	"gotest.tools/assert"
 )
 
 func TestFetcherOpen(t *testing.T) {
@@ -92,3 +98,66 @@ func TestFetcherOpen(t *testing.T) {
 		t.Fatal("expected error opening with invalid server response")
 	}
 }
+
+// New set of test to test new error cases
+func Test_dockerFetcher_open(t *testing.T) {
+	tests := []struct {
+		name                   string
+		mockedStatus           int
+		mockedErr              error
+		want                   io.ReadCloser
+		wantErr                bool
+		wantServerMessageError bool
+		wantPlainError         bool
+	}{
+		{
+			name:         "should return status and error.message if it exists if the registry request fails",
+			mockedStatus: 500,
+			mockedErr: errcode.Errors{errcode.Error{
+				Code:    errcode.ErrorCodeUnknown,
+				Message: "Test Error",
+			}},
+			want:                   nil,
+			wantErr:                true,
+			wantServerMessageError: true,
+		},
+		{
+			name:           "should return just status if the registry request fails and does not return a docker error",
+			mockedStatus:   500,
+			mockedErr:      fmt.Errorf("Non-docker error"),
+			want:           nil,
+			wantErr:        true,
+			wantPlainError: true,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+
+			s := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
+				rw.WriteHeader(tt.mockedStatus)
+				bytes, _ := json.Marshal(tt.mockedErr)
+				rw.Write(bytes)
+			}))
+			defer s.Close()
+
+			r := dockerFetcher{&dockerBase{
+				client: s.Client(),
+			}}
+
+			got, err := r.open(context.TODO(), s.URL, "", 0)
+			assert.Equal(t, tt.wantErr, (err != nil))
+			assert.Equal(t, tt.want, got)
+			if tt.wantErr {
+				var expectedError error
+				if tt.wantServerMessageError {
+					expectedError = errors.Errorf("unexpected status code %v: %v %s - Server message: %s", s.URL, tt.mockedStatus, http.StatusText(tt.mockedStatus), tt.mockedErr.Error())
+				} else if tt.wantPlainError {
+					expectedError = errors.Errorf("unexpected status code %v: %v %s", s.URL, tt.mockedStatus, http.StatusText(tt.mockedStatus))
+				}
+				assert.Equal(t, expectedError.Error(), err.Error())
+
+			}
+
+		})
+	}
+}