fix: preserve response body in retryError for structured error messages

The retryError function was consuming the HTTP response body without
restoring it for subsequent readers. This caused structured registry
error messages to be lost during retries, falling back to generic
HTTP status messages instead.

This change buffers the response body and creates a new readable stream
so that both retryError and subsequent error handling can access the
response content.

Before:
- Without retry: MANIFEST_UNKNOWN: manifest unknown; unknown tag=v1.0.0
- With retry: unexpected status code 404 Not Found

After:
- Both cases: MANIFEST_UNKNOWN: manifest unknown; unknown tag=v1.0.0

Fixes #2125

Author: ryanbrainard

pkg/v1/remote/error_roundtrip_test.go

@@ -125,3 +125,52 @@ func TestBlobStatusCodeReturned(t *testing.T) {
 		t.Errorf("Incorrect status code received, got %v, wanted %v", terr.StatusCode, http.StatusTeapot)
 	}
 }
+
+func TestRetryPreservesStructuredErrors(t *testing.T) {
+	// Test that structured registry errors are preserved when retries are enabled
+	handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.URL.Path == "/v2/" {
+			return
+		}
+		// Return a structured registry error
+		w.Header().Set("Content-Type", "application/json")
+		w.WriteHeader(http.StatusNotFound)
+		w.Write([]byte(`{"errors":[{"code":"MANIFEST_UNKNOWN","message":"manifest unknown","detail":"unknown tag=v1.0.0"}]}`))
+	})
+
+	server := httptest.NewServer(handler)
+	defer server.Close()
+
+	ref, err := name.NewTag(strings.TrimPrefix(server.URL+"/test:v1.0.0", "http://"))
+	if err != nil {
+		t.Fatalf("Unable to parse tag: %v", err)
+	}
+
+	// Test without retry - should get structured error
+	_, err = remote.Image(ref)
+	if err == nil {
+		t.Fatal("Expected error, got nil")
+	}
+	withoutRetryMsg := err.Error()
+
+	// Test with retry - should still get structured error (not generic 404)
+	_, err = remote.Image(ref, remote.WithRetryStatusCodes(http.StatusNotFound))
+	if err == nil {
+		t.Fatal("Expected error, got nil")
+	}
+	withRetryMsg := err.Error()
+
+	// Both should contain the structured error message
+	expectedSubstring := "MANIFEST_UNKNOWN: manifest unknown; unknown tag=v1.0.0"
+	if !strings.Contains(withoutRetryMsg, expectedSubstring) {
+		t.Errorf("Without retry error %q should contain %q", withoutRetryMsg, expectedSubstring)
+	}
+	if !strings.Contains(withRetryMsg, expectedSubstring) {
+		t.Errorf("With retry error %q should contain %q", withRetryMsg, expectedSubstring)
+	}
+
+	// The retry case should NOT contain generic "unexpected status code 404"
+	if strings.Contains(withRetryMsg, "unexpected status code 404 Not Found") {
+		t.Errorf("With retry error should not contain generic 404 message, got: %q", withRetryMsg)
+	}
+}

pkg/v1/remote/transport/error.go

@@ -15,6 +15,7 @@
 package transport
 
 import (
+	"bytes"
 	"encoding/json"
 	"fmt"
 	"io"
@@ -189,6 +190,8 @@ func retryError(resp *http.Response) error {
 	if err != nil {
 		return err
 	}
+	_ = resp.Body.Close()
+	resp.Body = io.NopCloser(bytes.NewReader(b))
 
 	rerr := makeError(resp, b)
 	rerr.temporary = true

pkg/v1/remote/transport/error_test.go

@@ -234,3 +234,85 @@ func (e *errReadCloser) Read(_ []byte) (int, error) {
 func (e *errReadCloser) Close() error {
 	return e.err
 }
+
+func TestRetryError(t *testing.T) {
+	tests := []struct {
+		name      string
+		status    int
+		body      string
+		wantError string
+	}{
+		{
+			name:      "returned error",
+			status:    http.StatusInternalServerError,
+			body:      "boom",
+			wantError: "unexpected status code 500 Internal Server Error: boom",
+		},
+		{
+			name:      "empty body",
+			status:    http.StatusInternalServerError,
+			body:      "",
+			wantError: "unexpected status code 500 Internal Server Error",
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			resp := &http.Response{
+				StatusCode: test.status,
+				Body:       io.NopCloser(bytes.NewBufferString(test.body)),
+			}
+
+			// Call retryError
+			err := retryError(resp)
+			if err == nil {
+				t.Fatal("retryError() = nil, wanted error")
+			}
+
+			// Verify the error is marked as temporary
+			var terr *Error
+			if !errors.As(err, &terr) {
+				t.Fatalf("retryError() = %T, wanted *Error", err)
+			}
+			if !terr.Temporary() {
+				t.Error("retryError() should return temporary error")
+			}
+
+			// Verify error message
+			if terr.Error() != test.wantError {
+				t.Errorf("retryError().Error() = %q, want %q", terr.Error(), test.wantError)
+			}
+
+			// Verify the response body can still be read
+			body, err := io.ReadAll(resp.Body)
+			if err != nil {
+				t.Fatalf("Failed to read response body after retryError: %v", err)
+			}
+			if string(body) != test.body {
+				t.Errorf("Response body after retryError = %q, want %q", string(body), test.body)
+			}
+
+			// Verify we can read it again (body should be rewindable)
+			body2, err := io.ReadAll(resp.Body)
+			if err != nil {
+				t.Fatalf("Failed to read response body second time: %v", err)
+			}
+			if string(body2) != "" {
+				t.Errorf("Second read should be empty, got %q", string(body2))
+			}
+		})
+	}
+}
+
+func TestRetryErrorReadError(t *testing.T) {
+	expectedErr := errors.New("read error")
+	resp := &http.Response{
+		StatusCode: http.StatusInternalServerError,
+		Body:       &errReadCloser{expectedErr},
+	}
+
+	err := retryError(resp)
+	if !errors.Is(err, expectedErr) {
+		t.Errorf("retryError() = %v, want %v", err, expectedErr)
+	}
+}