Merge pull request #3109 from bainsy88/issue_3076

Add code to return errors from registries

remotes/docker/fetcher.go

@@ -18,6 +18,7 @@ package docker
 
 import (
 	"context"
+	"encoding/json"
 	"fmt"
 	"io"
 	"io/ioutil"
@@ -28,6 +29,7 @@ import (
 	"github.com/containerd/containerd/errdefs"
 	"github.com/containerd/containerd/images"
 	"github.com/containerd/containerd/log"
+	"github.com/docker/distribution/registry/api/errcode"
 	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
@@ -102,10 +104,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 tests to test new error cases
+func TestDockerFetcherOpen(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())
+
+			}
+
+		})
+	}
+}