Repository: google/go-containerregistry
Pull request #551 (2019-09-24 16:20:18)

transport: Fix ping code to wrap errors in a transport.Error
I added a high level test that the operation returns an error for a
server which only returns one status code. I'll probably add a few more
cases to help find other pathways that are swallowing the error and
contained status code.

codecov-io: # [Codecov](https://codecov.io/gh/google/go-containerregistry/pull/551?src=pr&el=h1) Report
> Merging [#551](https://codecov.io/gh/google/go-containerregistry/pull/551?src=pr&el=desc) into [master](https://codecov.io/gh/google/go-containerregistry/commit/53e1ac5a7d185672cfa09b847b4fa32d229899d1?src=pr&el=desc) will **increase** coverage by `0.14%`.
> The diff coverage is `100%`.

[![Impacted file tree graph](https://codecov.io/gh/google/go-containerregistry/pull/551/graphs/tree.svg?width=650&token=wG7iiDCdZ4&height=150&src=pr)](https://codecov.io/gh/google/go-containerregistry/pull/551?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master     #551      +/-   ##
==========================================
+ Coverage   72.34%   72.48%   +0.14%     
==========================================
  Files          95       95              
  Lines        4245     4245              
==========================================
+ Hits         3071     3077       +6     
+ Misses        777      774       -3     
+ Partials      397      394       -3
```


| [Impacted Files](https://codecov.io/gh/google/go-containerregistry/pull/551?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pkg/v1/remote/transport/ping.go](https://codecov.io/gh/google/go-containerregistry/pull/551/diff?src=pr&el=tree#diff-cGtnL3YxL3JlbW90ZS90cmFuc3BvcnQvcGluZy5nbw==) | `100% <100%> (ø)` | :arrow_up: |
| [pkg/v1/remote/image.go](https://codecov.io/gh/google/go-containerregistry/pull/551/diff?src=pr&el=tree#diff-cGtnL3YxL3JlbW90ZS9pbWFnZS5nbw==) | `83.72% <0%> (+2.32%)` | :arrow_up: |
| [pkg/v1/remote/descriptor.go](https://codecov.io/gh/google/go-containerregistry/pull/551/diff?src=pr&el=tree#diff-cGtnL3YxL3JlbW90ZS9kZXNjcmlwdG9yLmdv) | `83.08% <0%> (+2.94%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/google/go-containerregistry/pull/551?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/google/go-containerregistry/pull/551?src=pr&el=footer). Last update [53e1ac5...52ecc34](https://codecov.io/gh/google/go-containerregistry/pull/551?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/pkg/v1/remote/transport/ping.go b/pkg/v1/remote/transport/ping.go
index 3fc4aa15..d999ebcf 100644
--- a/pkg/v1/remote/transport/ping.go
+++ b/pkg/v1/remote/transport/ping.go
@@ -108,7 +108,7 @@ func ping(reg name.Registry, t http.RoundTripper) (*pingResp, error) {
 				scheme:    scheme,
 			}, nil
 		default:
-			return nil, fmt.Errorf("unrecognized HTTP status: %v", resp.Status)
+			return nil, CheckError(resp, http.StatusOK, http.StatusUnauthorized)
 		}
 	}
 	return nil, connErr


Test patch:
diff --git a/pkg/v1/remote/error_roundtrip_test.go b/pkg/v1/remote/error_roundtrip_test.go
new file mode 100644
index 00000000..f5c9b25a
--- /dev/null
+++ b/pkg/v1/remote/error_roundtrip_test.go
@@ -0,0 +1,128 @@
+// Copyright 2019 Google LLC All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package remote_test
+
+import (
+	"fmt"
+	"log"
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+
+	"github.com/google/go-containerregistry/pkg/name"
+	"github.com/google/go-containerregistry/pkg/registry"
+	"github.com/google/go-containerregistry/pkg/v1/random"
+	"github.com/google/go-containerregistry/pkg/v1/remote"
+	"github.com/google/go-containerregistry/pkg/v1/remote/transport"
+)
+
+func TestStatusCodeReturned(t *testing.T) {
+
+	tcs := []struct {
+		Description string
+		Handler     http.Handler
+	}{{
+		Description: "Only returns teapot status",
+		Handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			w.WriteHeader(http.StatusTeapot)
+		}),
+	}, {
+		Description: "Handle v2, returns teapot status else",
+		Handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			log.Print(r.URL.Path)
+			if r.URL.Path == "/v2/" {
+				return
+			}
+			w.WriteHeader(http.StatusTeapot)
+		}),
+	}}
+
+	for _, tc := range tcs {
+		t.Run(tc.Description, func(t *testing.T) {
+			o := httptest.NewServer(tc.Handler)
+			defer o.Close()
+
+			ref, err := name.NewDigest(strings.TrimPrefix(o.URL+"/foo:@sha256:53b27244ffa2f585799adbfaf79fba5a5af104597751b289c8b235e7b8f7ebf5", "http://"))
+
+			if err != nil {
+				t.Fatalf("Unable to parse digest: %v", err)
+			}
+
+			_, err = remote.Image(ref)
+			terr, ok := err.(*transport.Error)
+			if !ok {
+				t.Fatalf("Unable to cast error to transport error: %v", err)
+			}
+			if terr.StatusCode != http.StatusTeapot {
+				t.Errorf("Incorrect status code received, got %v, wanted %v", terr.StatusCode, http.StatusTeapot)
+			}
+		})
+	}
+}
+
+func TestBlobStatusCodeReturned(t *testing.T) {
+	reg := registry.New()
+	rh := httptest.NewServer(reg)
+	defer rh.Close()
+	i, _ := random.Image(1024, 16)
+	tag := strings.TrimPrefix(fmt.Sprintf("%s/foo:bar", rh.URL), "http://")
+	d, _ := name.NewTag(tag)
+	if err := remote.Write(d, i); err != nil {
+		t.Fatalf("Unable to write empty image: %v", err)
+	}
+
+	handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		log.Print(r.URL.Path)
+		if strings.Contains(r.URL.Path, "blob") {
+			w.WriteHeader(http.StatusTeapot)
+			return
+		}
+		reg.ServeHTTP(w, r)
+	})
+
+	o := httptest.NewServer(handler)
+	defer o.Close()
+
+	ref, err := name.NewTag(strings.TrimPrefix(fmt.Sprintf("%s/foo:bar", o.URL), "http://"))
+	if err != nil {
+		t.Fatalf("Unable to parse digest: %v", err)
+	}
+
+	ri, err := remote.Image(ref)
+	if err != nil {
+		t.Fatalf("Unable to fetch manifest: %v", err)
+	}
+	l, err := ri.Layers()
+	if err != nil {
+		t.Fatalf("Unable to fetch layers: %v", err)
+	}
+	_, err = l[0].Compressed()
+	terr, ok := err.(*transport.Error)
+	if !ok {
+		t.Fatalf("Unable to cast error to transport error: %v", err)
+	}
+	if terr.StatusCode != http.StatusTeapot {
+		t.Errorf("Incorrect status code received, got %v, wanted %v", terr.StatusCode, http.StatusTeapot)
+	}
+	_, err = l[0].Uncompressed()
+	terr, ok = err.(*transport.Error)
+	if !ok {
+		t.Fatalf("Unable to cast error to transport error: %v", err)
+	}
+	if terr.StatusCode != http.StatusTeapot {
+		t.Errorf("Incorrect status code received, got %v, wanted %v", terr.StatusCode, http.StatusTeapot)
+	}
+}