fix(sync): ping func should not try to read response body (#1757)

closes: #1703

Signed-off-by: Petu Eusebiu <peusebiu@cisco.com>

examples/config-popular-registries.json

@@ -17,7 +17,7 @@
         "registries": [
         {
           "urls": [
-            "https://docker.io/library"
+            "https://index.docker.io"
           ],
           "content": [
           {

examples/config-sync.json

@@ -63,7 +63,7 @@
 				},
 				{
 					"urls": [
-						"https://docker.io/library"
+						"https://index.docker.io"
 					],
 					"onDemand": true,
 					"tlsVerify": true,

pkg/common/http_client.go

@@ -6,7 +6,6 @@ import (
 	"crypto/x509"
 	"encoding/json"
 	"errors"
-	"fmt"
 	"io"
 	"net/http"
 	"os"
@@ -138,6 +137,8 @@ func MakeHTTPGetRequest(ctx context.Context, httpClient *http.Client,
 		return nil, "", -1, err
 	}
 
+	defer resp.Body.Close()
+
 	body, err := io.ReadAll(resp.Body)
 	if err != nil {
 		log.Error().Str("errorType", TypeOf(err)).
@@ -146,12 +147,7 @@ func MakeHTTPGetRequest(ctx context.Context, httpClient *http.Client,
 		return nil, "", resp.StatusCode, err
 	}
 
-	defer resp.Body.Close()
-
 	if resp.StatusCode != http.StatusOK {
-		log.Error().Str("status code", fmt.Sprint(resp.StatusCode)).
-			Err(err).Str("blobURL", blobURL).Msg("couldn't get blob")
-
 		return nil, "", resp.StatusCode, errors.New(string(body)) //nolint:goerr113
 	}
 
@@ -159,9 +155,6 @@ func MakeHTTPGetRequest(ctx context.Context, httpClient *http.Client,
 	if len(body) > 0 {
 		err = json.Unmarshal(body, &resultPtr)
 		if err != nil {
-			log.Error().Str("errorType", TypeOf(err)).Str("blobURL", blobURL).
-				Err(err).Msg("couldn't unmarshal remote blob")
-
 			return body, "", resp.StatusCode, err
 		}
 	}

pkg/extensions/sync/httpclient/client.go

@@ -2,6 +2,7 @@ package client
 
 import (
 	"context"
+	"io"
 	"net/http"
 	"net/url"
 	"sync"
@@ -71,13 +72,42 @@ func (httpClient *Client) SetConfig(config Config) error {
 	return nil
 }
 
-func (httpClient *Client) IsAvailable() bool {
+func (httpClient *Client) Ping() bool {
-	_, _, statusCode, err := httpClient.MakeGetRequest(context.Background(), nil, "", "/v2/")
+	pingURL := *httpClient.url
-	if err != nil || statusCode != http.StatusOK {
+
+	pingURL = *pingURL.JoinPath("/v2/")
+
+	req, err := http.NewRequest(http.MethodGet, pingURL.String(), nil) //nolint
+	if err != nil {
 		return false
 	}
 
-	return true
+	resp, err := httpClient.client.Do(req)
+	if err != nil {
+		httpClient.log.Error().Err(err).Str("url", pingURL.String()).
+			Msg("sync: failed to ping registry")
+
+		return false
+	}
+
+	defer resp.Body.Close()
+
+	if resp.StatusCode == http.StatusOK || resp.StatusCode == http.StatusUnauthorized {
+		return true
+	}
+
+	body, err := io.ReadAll(resp.Body)
+	if err != nil {
+		httpClient.log.Error().Err(err).Str("url", pingURL.String()).
+			Msg("failed to read body while pinging registry")
+
+		return false
+	}
+
+	httpClient.log.Error().Str("url", pingURL.String()).Str("body", string(body)).Int("statusCode", resp.StatusCode).
+		Msg("sync: failed to ping registry")
+
+	return false
 }
 
 func (httpClient *Client) MakeGetRequest(ctx context.Context, resultPtr interface{}, mediaType string,

pkg/extensions/sync/references/cosign.go

@@ -198,10 +198,6 @@ func (ref CosignReference) getManifest(ctx context.Context, repo, cosignTag stri
 			return nil, nil, zerr.ErrSyncReferrerNotFound
 		}
 
-		ref.log.Error().Str("errorType", common.TypeOf(err)).
-			Str("repository", repo).Str("tag", cosignTag).Int("statusCode", statusCode).
-			Err(err).Msg("couldn't get cosign manifest for image")
-
 		return nil, nil, err
 	}
 

pkg/extensions/sync/references/oci.go

@@ -184,10 +184,6 @@ func (ref OciReferences) getIndex(ctx context.Context, repo, subjectDigestStr st
 			return index, zerr.ErrSyncReferrerNotFound
 		}
 
-		ref.log.Error().Str("errorType", common.TypeOf(err)).
-			Err(err).Str("repository", repo).Str("subject", subjectDigestStr).Int("statusCode", statusCode).
-			Msg("couldn't get oci reference for image")
-
 		return index, err
 	}
 

pkg/extensions/sync/references/oras.go

@@ -186,9 +186,6 @@ func (ref ORASReferences) getReferenceList(ctx context.Context, repo, subjectDig
 			return referrers, zerr.ErrSyncReferrerNotFound
 		}
 
-		ref.log.Error().Err(err).Str("repository", repo).Str("subject", subjectDigestStr).
-			Msg("couldn't get ORAS artifacts for image")
-
 		return referrers, err
 	}
 

pkg/extensions/sync/references/references.go

@@ -79,7 +79,7 @@ func (refs References) syncAll(ctx context.Context, localRepo, upstreamRepo,
 	for _, ref := range refs.referenceList {
 		syncedRefsDigests, err = ref.SyncReferences(ctx, localRepo, upstreamRepo, subjectDigestStr)
 		if err != nil {
-			refs.log.Debug().Err(err).
+			refs.log.Error().Err(err).
 				Str("reference type", ref.Name()).
 				Str("image", fmt.Sprintf("%s:%s", upstreamRepo, subjectDigestStr)).
 				Msg("couldn't sync image referrer")

pkg/extensions/sync/service.go

@@ -95,7 +95,7 @@ func New(opts syncconf.RegistryConfig, credentialsFilepath string,
 }
 
 func (service *BaseService) SetNextAvailableClient() error {
-	if service.client != nil && service.client.IsAvailable() {
+	if service.client != nil && service.client.Ping() {
 		return nil
 	}
 
@@ -125,10 +125,12 @@ func (service *BaseService) SetNextAvailableClient() error {
 		}
 
 		if err != nil {
+			service.log.Error().Err(err).Str("url", url).Msg("sync: failed to initialize http client")
+
 			continue
 		}
 
-		if !service.client.IsAvailable() {
+		if !service.client.Ping() {
 			continue
 		}
 	}
@@ -252,9 +254,6 @@ func (service *BaseService) SyncImage(ctx context.Context, repo, reference strin
 
 	err = service.references.SyncAll(ctx, repo, remoteRepo, manifestDigest.String())
 	if err != nil && !errors.Is(err, zerr.ErrSyncReferrerNotFound) {
-		service.log.Error().Err(err).Str("remote", remoteURL).Str("repo", repo).Str("reference", reference).
-			Msg("error while syncing references for image")
-
 		return err
 	}
 

test/blackbox/sync_docker.bats

@@ -54,7 +54,7 @@ function setup_file() {
             "registries": [
                 {
                     "urls": [
-                        "https://docker.io/library"
+                        "https://index.docker.io"
                     ],
                     "content": [
                         {
@@ -330,7 +330,7 @@ function teardown_file() {
     [ $(echo "${lines[-1]}" | jq '.tags[]') = '"v2.0.0-rc5"' ]
 }
 
-@test "run docker with image synced from docker.io/library" {
+@test "run docker with image synced from docker.io" {
     local zot_root_dir=${BATS_FILE_TMPDIR}/zot
     run rm -rf ${zot_root_dir}
     [ "$status" -eq 0 ]