From bfd140c0e5f11f7fcbcb4ddbbc2ba4e0624604c5 Mon Sep 17 00:00:00 2001
From: Meredith Lancaster <malancas@github.com>
Date: Wed, 6 Nov 2024 07:57:18 -0700
Subject: [PATCH 01/30] initial pass at fetching bundles with sas urls

Signed-off-by: Meredith Lancaster <malancas@github.com>
---
 go.mod                                        |  1 +
 go.sum                                        |  2 +
 pkg/cmd/attestation/api/attestation.go        |  1 +
 pkg/cmd/attestation/api/client.go             | 49 +++++++++++++++++++
 pkg/cmd/attestation/api/mock_client.go        |  4 ++
 .../attestation/verification/attestation.go   | 16 +++++-
 6 files changed, 71 insertions(+), 2 deletions(-)

diff --git a/go.mod b/go.mod
index 77be0c19d..9a2e53d38 100644
--- a/go.mod
+++ b/go.mod
@@ -92,6 +92,7 @@ require (
 	github.com/go-openapi/swag v0.23.0 // indirect
 	github.com/go-openapi/validate v0.24.0 // indirect
 	github.com/godbus/dbus/v5 v5.1.0 // indirect
+	github.com/golang/snappy v0.0.4 // indirect
 	github.com/google/certificate-transparency-go v1.2.1 // indirect
 	github.com/google/uuid v1.6.0 // indirect
 	github.com/gorilla/css v1.0.0 // indirect
diff --git a/go.sum b/go.sum
index fe7adc3e7..cc92ed459 100644
--- a/go.sum
+++ b/go.sum
@@ -198,6 +198,8 @@ github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da h1:oI5xCqsCo564l
 github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da/go.mod h1:cIg4eruTrX1D+g88fzRXU5OdNfaM+9IcxsU14FzY7Hc=
 github.com/golang/protobuf v1.5.4 h1:i7eJL8qZTpSEXOPTxNKhASYpMn+8e5Q6AdndVa1dWek=
 github.com/golang/protobuf v1.5.4/go.mod h1:lnTiLA8Wa4RWRcIUkrtSVa5nRhsEGBg48fD6rSs7xps=
+github.com/golang/snappy v0.0.4 h1:yAGX7huGHXlcLOEtBnF4w7FQwA26wojNCwOYAEhLjQM=
+github.com/golang/snappy v0.0.4/go.mod h1:/XxbfmMg8lxefKM7IXC3fBNl/7bRcc72aCRzEWrmP2Q=
 github.com/google/certificate-transparency-go v1.2.1 h1:4iW/NwzqOqYEEoCBEFP+jPbBXbLqMpq3CifMyOnDUME=
 github.com/google/certificate-transparency-go v1.2.1/go.mod h1:bvn/ytAccv+I6+DGkqpvSsEdiVGramgaSC6RD3tEmeE=
 github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI=
diff --git a/pkg/cmd/attestation/api/attestation.go b/pkg/cmd/attestation/api/attestation.go
index ea055b293..d5c444ee2 100644
--- a/pkg/cmd/attestation/api/attestation.go
+++ b/pkg/cmd/attestation/api/attestation.go
@@ -26,6 +26,7 @@ func newErrNoAttestations(name, digest string) ErrNoAttestations {
 
 type Attestation struct {
 	Bundle *bundle.Bundle `json:"bundle"`
+	SASUrl string         `json:"signedAccessSignatureUrl"`
 }
 
 type AttestationsResponse struct {
diff --git a/pkg/cmd/attestation/api/client.go b/pkg/cmd/attestation/api/client.go
index f47b5f759..f2f9f48f8 100644
--- a/pkg/cmd/attestation/api/client.go
+++ b/pkg/cmd/attestation/api/client.go
@@ -1,16 +1,21 @@
 package api
 
 import (
+	"encoding/json"
 	"errors"
 	"fmt"
 	"io"
 	"net/http"
+	"net/url"
 	"strings"
 	"time"
 
 	"github.com/cenkalti/backoff/v4"
 	"github.com/cli/cli/v2/api"
 	ioconfig "github.com/cli/cli/v2/pkg/cmd/attestation/io"
+	"github.com/golang/snappy"
+	v1 "github.com/sigstore/protobuf-specs/gen/pb-go/bundle/v1"
+	"github.com/sigstore/sigstore-go/pkg/bundle"
 )
 
 const (
@@ -25,6 +30,7 @@ type apiClient interface {
 }
 
 type Client interface {
+	FetchAttestationsWithSASURL(attestations []*Attestation) ([]*Attestation, error)
 	GetByRepoAndDigest(repo, digest string, limit int) ([]*Attestation, error)
 	GetByOwnerAndDigest(owner, digest string, limit int) ([]*Attestation, error)
 	GetTrustDomain() (string, error)
@@ -130,6 +136,49 @@ func (c *LiveClient) getAttestations(url, name, digest string, limit int) ([]*At
 	return attestations, nil
 }
 
+func (c *LiveClient) FetchAttestationsWithSASURL(attestations []*Attestation) ([]*Attestation, error) {
+	fetched := make([]*Attestation, len(attestations))
+	for i, a := range attestations {
+		b, err := c.fetchAttestationWithSASURL(a)
+		if err != nil {
+			return nil, fmt.Errorf("failed to fetch attestation with SAS URL: %w", err)
+		}
+		fetched[i] = &Attestation{
+			Bundle: b,
+		}
+	}
+	return fetched, nil
+}
+
+func (c *LiveClient) fetchAttestationWithSASURL(a *Attestation) (*bundle.Bundle, error) {
+	if a.SASUrl == "" {
+		return a.Bundle, nil
+	}
+
+	parsed, err := url.Parse(a.SASUrl)
+	if err != nil {
+		return nil, err
+	}
+
+	var resp []byte
+	if err = c.api.REST(parsed.Host, http.MethodGet, parsed.Path, nil, &resp); err != nil {
+		return nil, err
+	}
+
+	var decompressedBytes []byte
+	_, err = snappy.Decode(decompressedBytes, resp)
+	if err != nil {
+		return nil, err
+	}
+
+	var pbBundle *v1.Bundle
+	if err = json.Unmarshal(decompressedBytes, pbBundle); err != nil {
+		return nil, err
+	}
+
+	return bundle.NewBundle(pbBundle)
+}
+
 func shouldRetry(err error) bool {
 	var httpError api.HTTPError
 	if errors.As(err, &httpError) {
diff --git a/pkg/cmd/attestation/api/mock_client.go b/pkg/cmd/attestation/api/mock_client.go
index bcb51c414..17bfdadec 100644
--- a/pkg/cmd/attestation/api/mock_client.go
+++ b/pkg/cmd/attestation/api/mock_client.go
@@ -12,6 +12,10 @@ type MockClient struct {
 	OnGetTrustDomain      func() (string, error)
 }
 
+func (m MockClient) FetchAttestationsWithSASURL(attestations []*Attestation) ([]*Attestation, error) {
+	return nil, nil
+}
+
 func (m MockClient) GetByRepoAndDigest(repo, digest string, limit int) ([]*Attestation, error) {
 	return m.OnGetByRepoAndDigest(repo, digest, limit)
 }
diff --git a/pkg/cmd/attestation/verification/attestation.go b/pkg/cmd/attestation/verification/attestation.go
index 0ea91c2f7..a4548a72d 100644
--- a/pkg/cmd/attestation/verification/attestation.go
+++ b/pkg/cmd/attestation/verification/attestation.go
@@ -127,13 +127,25 @@ func GetRemoteAttestations(c FetchAttestationsConfig) ([]*api.Attestation, error
 		if err != nil {
 			return nil, fmt.Errorf("failed to fetch attestations from %s: %w", c.Repo, err)
 		}
-		return attestations, nil
+
+		fetched, err := c.APIClient.FetchAttestationsWithSASURL(attestations)
+		if err != nil {
+			return nil, fmt.Errorf("failed to fetch bundles from SAS URL: %w", err)
+		}
+
+		return fetched, nil
 	} else if c.Owner != "" {
 		attestations, err := c.APIClient.GetByOwnerAndDigest(c.Owner, c.Digest, c.Limit)
 		if err != nil {
 			return nil, fmt.Errorf("failed to fetch attestations from %s: %w", c.Owner, err)
 		}
-		return attestations, nil
+
+		fetched, err := c.APIClient.FetchAttestationsWithSASURL(attestations)
+		if err != nil {
+			return nil, fmt.Errorf("failed to fetch bundles from SAS URL: %w", err)
+		}
+
+		return fetched, nil
 	}
 	return nil, fmt.Errorf("owner or repo must be provided")
 }

From 5a6a7968a30b70e50dbc8521f2bcaf5d23c6102b Mon Sep 17 00:00:00 2001
From: Meredith Lancaster <malancas@github.com>
Date: Mon, 16 Dec 2024 11:25:43 -0700
Subject: [PATCH 02/30] fetch bundles with sas url

Signed-off-by: Meredith Lancaster <malancas@github.com>
---
 pkg/cmd/attestation/api/attestation.go        |  4 +-
 pkg/cmd/attestation/api/client.go             | 83 +++++++++++++------
 pkg/cmd/attestation/api/mock_client.go        |  7 +-
 .../attestation/verification/attestation.go   | 20 ++---
 4 files changed, 69 insertions(+), 45 deletions(-)

diff --git a/pkg/cmd/attestation/api/attestation.go b/pkg/cmd/attestation/api/attestation.go
index d5c444ee2..16b062a96 100644
--- a/pkg/cmd/attestation/api/attestation.go
+++ b/pkg/cmd/attestation/api/attestation.go
@@ -25,8 +25,8 @@ func newErrNoAttestations(name, digest string) ErrNoAttestations {
 }
 
 type Attestation struct {
-	Bundle *bundle.Bundle `json:"bundle"`
-	SASUrl string         `json:"signedAccessSignatureUrl"`
+	Bundle    *bundle.Bundle `json:"bundle"`
+	BundleURL string         `json:"bundle_url"`
 }
 
 type AttestationsResponse struct {
diff --git a/pkg/cmd/attestation/api/client.go b/pkg/cmd/attestation/api/client.go
index f2f9f48f8..acc4a1014 100644
--- a/pkg/cmd/attestation/api/client.go
+++ b/pkg/cmd/attestation/api/client.go
@@ -1,12 +1,10 @@
 package api
 
 import (
-	"encoding/json"
 	"errors"
 	"fmt"
 	"io"
 	"net/http"
-	"net/url"
 	"strings"
 	"time"
 
@@ -16,6 +14,7 @@ import (
 	"github.com/golang/snappy"
 	v1 "github.com/sigstore/protobuf-specs/gen/pb-go/bundle/v1"
 	"github.com/sigstore/sigstore-go/pkg/bundle"
+	"google.golang.org/protobuf/encoding/protojson"
 )
 
 const (
@@ -24,13 +23,16 @@ const (
 	maxLimitForFetch = 100
 )
 
+// Allow injecting backoff interval in tests.
+var getAttestationRetryInterval = time.Millisecond * 200
+
 type apiClient interface {
 	REST(hostname, method, p string, body io.Reader, data interface{}) error
 	RESTWithNext(hostname, method, p string, body io.Reader, data interface{}) (string, error)
 }
 
 type Client interface {
-	FetchAttestationsWithSASURL(attestations []*Attestation) ([]*Attestation, error)
+	// FetchAttestationsWithSASURL(attestations []*Attestation) ([]*Attestation, error)
 	GetByRepoAndDigest(repo, digest string, limit int) ([]*Attestation, error)
 	GetByOwnerAndDigest(owner, digest string, limit int) ([]*Attestation, error)
 	GetTrustDomain() (string, error)
@@ -58,7 +60,18 @@ func (c *LiveClient) BuildRepoAndDigestURL(repo, digest string) string {
 // GetByRepoAndDigest fetches the attestation by repo and digest
 func (c *LiveClient) GetByRepoAndDigest(repo, digest string, limit int) ([]*Attestation, error) {
 	url := c.BuildRepoAndDigestURL(repo, digest)
-	return c.getAttestations(url, repo, digest, limit)
+	attestations, err := c.getAttestations(url, repo, digest, limit)
+	if err != nil {
+		return nil, fmt.Errorf("failed to fetch attestation by repo and digest: %w", err)
+	}
+
+	compressedBundles, err := c.fetchBundlesWithSASURL(attestations)
+	if err != nil {
+		return nil, fmt.Errorf("failed to fetch attestation with SAS URL: %w", err)
+	}
+
+	// decompress before returning
+	return compressedBundles, nil
 }
 
 func (c *LiveClient) BuildOwnerAndDigestURL(owner, digest string) string {
@@ -69,7 +82,18 @@ func (c *LiveClient) BuildOwnerAndDigestURL(owner, digest string) string {
 // GetByOwnerAndDigest fetches attestation by owner and digest
 func (c *LiveClient) GetByOwnerAndDigest(owner, digest string, limit int) ([]*Attestation, error) {
 	url := c.BuildOwnerAndDigestURL(owner, digest)
-	return c.getAttestations(url, owner, digest, limit)
+	attestations, err := c.getAttestations(url, owner, digest, limit)
+	if err != nil {
+		return nil, fmt.Errorf("failed to fetch attestation by repo and digest: %w", err)
+	}
+
+	compressedBundles, err := c.fetchBundlesWithSASURL(attestations)
+	if err != nil {
+		return nil, fmt.Errorf("failed to fetch attestation with SAS URL: %w", err)
+	}
+
+	// decompress before returning
+	return compressedBundles, nil
 }
 
 // GetTrustDomain returns the current trust domain. If the default is used
@@ -78,9 +102,6 @@ func (c *LiveClient) GetTrustDomain() (string, error) {
 	return c.getTrustDomain(MetaPath)
 }
 
-// Allow injecting backoff interval in tests.
-var getAttestationRetryInterval = time.Millisecond * 200
-
 func (c *LiveClient) getAttestations(url, name, digest string, limit int) ([]*Attestation, error) {
 	c.logger.VerbosePrintf("Fetching attestations for artifact digest %s\n\n", digest)
 
@@ -136,12 +157,12 @@ func (c *LiveClient) getAttestations(url, name, digest string, limit int) ([]*At
 	return attestations, nil
 }
 
-func (c *LiveClient) FetchAttestationsWithSASURL(attestations []*Attestation) ([]*Attestation, error) {
+func (c *LiveClient) fetchBundlesWithSASURL(attestations []*Attestation) ([]*Attestation, error) {
 	fetched := make([]*Attestation, len(attestations))
 	for i, a := range attestations {
-		b, err := c.fetchAttestationWithSASURL(a)
+		b, err := c.fetchBundleWithSASURL(a)
 		if err != nil {
-			return nil, fmt.Errorf("failed to fetch attestation with SAS URL: %w", err)
+			return nil, fmt.Errorf("failed to fetch bundle with SAS URL: %w", err)
 		}
 		fetched[i] = &Attestation{
 			Bundle: b,
@@ -150,33 +171,45 @@ func (c *LiveClient) FetchAttestationsWithSASURL(attestations []*Attestation) ([
 	return fetched, nil
 }
 
-func (c *LiveClient) fetchAttestationWithSASURL(a *Attestation) (*bundle.Bundle, error) {
-	if a.SASUrl == "" {
-		return a.Bundle, nil
+func (c *LiveClient) fetchBundleWithSASURL(a *Attestation) (*bundle.Bundle, error) {
+	if a.BundleURL == "" {
+		//return a.Bundle, nil
+		return nil, fmt.Errorf("SAS URL is empty")
 	}
 
-	parsed, err := url.Parse(a.SASUrl)
+	c.logger.VerbosePrintf("Fetching attestation bundle\n\n")
+
+	httpClient := http.DefaultClient
+	r, err := httpClient.Get(a.BundleURL)
 	if err != nil {
 		return nil, err
 	}
 
-	var resp []byte
-	if err = c.api.REST(parsed.Host, http.MethodGet, parsed.Path, nil, &resp); err != nil {
-		return nil, err
+	defer r.Body.Close()
+	if r.StatusCode != http.StatusOK {
+		return nil, fmt.Errorf("unexpected status code: %d", r.StatusCode)
 	}
 
-	var decompressedBytes []byte
-	_, err = snappy.Decode(decompressedBytes, resp)
+	// parse the URL to get the host and path
+	resp, err := io.ReadAll(r.Body)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("failed to read blob storage response body: %w", err)
 	}
 
-	var pbBundle *v1.Bundle
-	if err = json.Unmarshal(decompressedBytes, pbBundle); err != nil {
-		return nil, err
+	var out []byte
+	decompressed, err := snappy.Decode(out, resp)
+	if err != nil {
+		return nil, fmt.Errorf("failed to decompress with snappy: %w", err)
 	}
 
-	return bundle.NewBundle(pbBundle)
+	var pbBundle v1.Bundle
+	if err = protojson.Unmarshal(decompressed, &pbBundle); err != nil {
+		return nil, fmt.Errorf("failed to unmarshal to bundle: %w", err)
+	}
+
+	c.logger.VerbosePrintf("Successfully fetched bundle\n\n")
+
+	return bundle.NewBundle(&pbBundle)
 }
 
 func shouldRetry(err error) bool {
diff --git a/pkg/cmd/attestation/api/mock_client.go b/pkg/cmd/attestation/api/mock_client.go
index 17bfdadec..63dd3d244 100644
--- a/pkg/cmd/attestation/api/mock_client.go
+++ b/pkg/cmd/attestation/api/mock_client.go
@@ -7,9 +7,10 @@ import (
 )
 
 type MockClient struct {
-	OnGetByRepoAndDigest  func(repo, digest string, limit int) ([]*Attestation, error)
-	OnGetByOwnerAndDigest func(owner, digest string, limit int) ([]*Attestation, error)
-	OnGetTrustDomain      func() (string, error)
+	OnGetByRepoAndDigest          func(repo, digest string, limit int) ([]*Attestation, error)
+	OnGetByOwnerAndDigest         func(owner, digest string, limit int) ([]*Attestation, error)
+	OnGetTrustDomain              func() (string, error)
+	OnFetchAttestationsWithSASURL func(attestations []*Attestation) ([]*Attestation, error)
 }
 
 func (m MockClient) FetchAttestationsWithSASURL(attestations []*Attestation) ([]*Attestation, error) {
diff --git a/pkg/cmd/attestation/verification/attestation.go b/pkg/cmd/attestation/verification/attestation.go
index 9755e46f5..41da71010 100644
--- a/pkg/cmd/attestation/verification/attestation.go
+++ b/pkg/cmd/attestation/verification/attestation.go
@@ -102,29 +102,19 @@ func GetRemoteAttestations(client api.Client, params FetchRemoteAttestationsPara
 	// check if Repo is set first because if Repo has been set, Owner will be set using the value of Repo.
 	// If Repo is not set, the field will remain empty. It will not be populated using the value of Owner.
 	if params.Repo != "" {
-		attestations, err := client.GetByRepoAndDigest(params.Repo, params.Digest, params.Limit)
+		bundles, err := client.GetByRepoAndDigest(params.Repo, params.Digest, params.Limit)
 		if err != nil {
-			return nil, fmt.Errorf("failed to fetch attestations from %s: %w", params.Repo, err)
+			return nil, fmt.Errorf("failed to fetch attestation bundles from %s: %w", params.Repo, err)
 		}
 
-		fetched, err := client.FetchAttestationsWithSASURL(attestations)
-		if err != nil {
-			return nil, fmt.Errorf("failed to fetch bundles from SAS URL: %w", err)
-		}
-
-		return fetched, nil
+		return bundles, nil
 	} else if params.Owner != "" {
-		attestations, err := client.GetByOwnerAndDigest(params.Owner, params.Digest, params.Limit)
+		bundles, err := client.GetByOwnerAndDigest(params.Owner, params.Digest, params.Limit)
 		if err != nil {
 			return nil, fmt.Errorf("failed to fetch attestations from %s: %w", params.Owner, err)
 		}
 
-		fetched, err := client.FetchAttestationsWithSASURL(attestations)
-		if err != nil {
-			return nil, fmt.Errorf("failed to fetch bundles from SAS URL: %w", err)
-		}
-
-		return fetched, nil
+		return bundles, nil
 	}
 	return nil, fmt.Errorf("owner or repo must be provided")
 }

From e51b4efaa9008ebeaf373b91deed654987489c79 Mon Sep 17 00:00:00 2001
From: Meredith Lancaster <malancas@github.com>
Date: Mon, 16 Dec 2024 11:50:46 -0700
Subject: [PATCH 03/30] remove unused method

Signed-off-by: Meredith Lancaster <malancas@github.com>
---
 pkg/cmd/attestation/api/client.go      | 15 ++++++---------
 pkg/cmd/attestation/api/mock_client.go | 11 +++--------
 2 files changed, 9 insertions(+), 17 deletions(-)

diff --git a/pkg/cmd/attestation/api/client.go b/pkg/cmd/attestation/api/client.go
index acc4a1014..47cff27ed 100644
--- a/pkg/cmd/attestation/api/client.go
+++ b/pkg/cmd/attestation/api/client.go
@@ -32,7 +32,6 @@ type apiClient interface {
 }
 
 type Client interface {
-	// FetchAttestationsWithSASURL(attestations []*Attestation) ([]*Attestation, error)
 	GetByRepoAndDigest(repo, digest string, limit int) ([]*Attestation, error)
 	GetByOwnerAndDigest(owner, digest string, limit int) ([]*Attestation, error)
 	GetTrustDomain() (string, error)
@@ -65,35 +64,33 @@ func (c *LiveClient) GetByRepoAndDigest(repo, digest string, limit int) ([]*Atte
 		return nil, fmt.Errorf("failed to fetch attestation by repo and digest: %w", err)
 	}
 
-	compressedBundles, err := c.fetchBundlesWithSASURL(attestations)
+	bundles, err := c.fetchBundlesWithSASURL(attestations)
 	if err != nil {
 		return nil, fmt.Errorf("failed to fetch attestation with SAS URL: %w", err)
 	}
 
-	// decompress before returning
-	return compressedBundles, nil
+	return bundles, nil
 }
 
-func (c *LiveClient) BuildOwnerAndDigestURL(owner, digest string) string {
+func (c *LiveClient) buildOwnerAndDigestURL(owner, digest string) string {
 	owner = strings.Trim(owner, "/")
 	return fmt.Sprintf(GetAttestationByOwnerAndSubjectDigestPath, owner, digest)
 }
 
 // GetByOwnerAndDigest fetches attestation by owner and digest
 func (c *LiveClient) GetByOwnerAndDigest(owner, digest string, limit int) ([]*Attestation, error) {
-	url := c.BuildOwnerAndDigestURL(owner, digest)
+	url := c.buildOwnerAndDigestURL(owner, digest)
 	attestations, err := c.getAttestations(url, owner, digest, limit)
 	if err != nil {
 		return nil, fmt.Errorf("failed to fetch attestation by repo and digest: %w", err)
 	}
 
-	compressedBundles, err := c.fetchBundlesWithSASURL(attestations)
+	bundles, err := c.fetchBundlesWithSASURL(attestations)
 	if err != nil {
 		return nil, fmt.Errorf("failed to fetch attestation with SAS URL: %w", err)
 	}
 
-	// decompress before returning
-	return compressedBundles, nil
+	return bundles, nil
 }
 
 // GetTrustDomain returns the current trust domain. If the default is used
diff --git a/pkg/cmd/attestation/api/mock_client.go b/pkg/cmd/attestation/api/mock_client.go
index 63dd3d244..bcb51c414 100644
--- a/pkg/cmd/attestation/api/mock_client.go
+++ b/pkg/cmd/attestation/api/mock_client.go
@@ -7,14 +7,9 @@ import (
 )
 
 type MockClient struct {
-	OnGetByRepoAndDigest          func(repo, digest string, limit int) ([]*Attestation, error)
-	OnGetByOwnerAndDigest         func(owner, digest string, limit int) ([]*Attestation, error)
-	OnGetTrustDomain              func() (string, error)
-	OnFetchAttestationsWithSASURL func(attestations []*Attestation) ([]*Attestation, error)
-}
-
-func (m MockClient) FetchAttestationsWithSASURL(attestations []*Attestation) ([]*Attestation, error) {
-	return nil, nil
+	OnGetByRepoAndDigest  func(repo, digest string, limit int) ([]*Attestation, error)
+	OnGetByOwnerAndDigest func(owner, digest string, limit int) ([]*Attestation, error)
+	OnGetTrustDomain      func() (string, error)
 }
 
 func (m MockClient) GetByRepoAndDigest(repo, digest string, limit int) ([]*Attestation, error) {

From 6b95175363ccceb31259f88a216763a3dc02d345 Mon Sep 17 00:00:00 2001
From: Meredith Lancaster <malancas@github.com>
Date: Mon, 16 Dec 2024 11:57:42 -0700
Subject: [PATCH 04/30] add httpClient field to LiveClient struct

Signed-off-by: Meredith Lancaster <malancas@github.com>
---
 pkg/cmd/attestation/api/client.go      | 34 ++++++++++++++------------
 pkg/cmd/attestation/api/client_test.go | 18 +++++++-------
 2 files changed, 28 insertions(+), 24 deletions(-)

diff --git a/pkg/cmd/attestation/api/client.go b/pkg/cmd/attestation/api/client.go
index 47cff27ed..34be1a6bb 100644
--- a/pkg/cmd/attestation/api/client.go
+++ b/pkg/cmd/attestation/api/client.go
@@ -26,11 +26,15 @@ const (
 // Allow injecting backoff interval in tests.
 var getAttestationRetryInterval = time.Millisecond * 200
 
-type apiClient interface {
+type githubApiClient interface {
 	REST(hostname, method, p string, body io.Reader, data interface{}) error
 	RESTWithNext(hostname, method, p string, body io.Reader, data interface{}) (string, error)
 }
 
+type httpClient interface {
+	Get(url string) (*http.Response, error)
+}
+
 type Client interface {
 	GetByRepoAndDigest(repo, digest string, limit int) ([]*Attestation, error)
 	GetByOwnerAndDigest(owner, digest string, limit int) ([]*Attestation, error)
@@ -38,27 +42,29 @@ type Client interface {
 }
 
 type LiveClient struct {
-	api    apiClient
-	host   string
-	logger *ioconfig.Handler
+	githubAPI  githubApiClient
+	httpClient httpClient
+	host       string
+	logger     *ioconfig.Handler
 }
 
 func NewLiveClient(hc *http.Client, host string, l *ioconfig.Handler) *LiveClient {
 	return &LiveClient{
-		api:    api.NewClientFromHTTP(hc),
-		host:   strings.TrimSuffix(host, "/"),
-		logger: l,
+		githubAPI:  api.NewClientFromHTTP(hc),
+		host:       strings.TrimSuffix(host, "/"),
+		httpClient: hc,
+		logger:     l,
 	}
 }
 
-func (c *LiveClient) BuildRepoAndDigestURL(repo, digest string) string {
+func (c *LiveClient) buildRepoAndDigestURL(repo, digest string) string {
 	repo = strings.Trim(repo, "/")
 	return fmt.Sprintf(GetAttestationByRepoAndSubjectDigestPath, repo, digest)
 }
 
 // GetByRepoAndDigest fetches the attestation by repo and digest
 func (c *LiveClient) GetByRepoAndDigest(repo, digest string, limit int) ([]*Attestation, error) {
-	url := c.BuildRepoAndDigestURL(repo, digest)
+	url := c.buildRepoAndDigestURL(repo, digest)
 	attestations, err := c.getAttestations(url, repo, digest, limit)
 	if err != nil {
 		return nil, fmt.Errorf("failed to fetch attestation by repo and digest: %w", err)
@@ -121,7 +127,7 @@ func (c *LiveClient) getAttestations(url, name, digest string, limit int) ([]*At
 	// if no attestation or less than limit, then keep fetching
 	for url != "" && len(attestations) < limit {
 		err := backoff.Retry(func() error {
-			newURL, restErr := c.api.RESTWithNext(c.host, http.MethodGet, url, nil, &resp)
+			newURL, restErr := c.githubAPI.RESTWithNext(c.host, http.MethodGet, url, nil, &resp)
 
 			if restErr != nil {
 				if shouldRetry(restErr) {
@@ -170,14 +176,12 @@ func (c *LiveClient) fetchBundlesWithSASURL(attestations []*Attestation) ([]*Att
 
 func (c *LiveClient) fetchBundleWithSASURL(a *Attestation) (*bundle.Bundle, error) {
 	if a.BundleURL == "" {
-		//return a.Bundle, nil
-		return nil, fmt.Errorf("SAS URL is empty")
+		return nil, fmt.Errorf("bundle URL is empty")
 	}
 
 	c.logger.VerbosePrintf("Fetching attestation bundle\n\n")
 
-	httpClient := http.DefaultClient
-	r, err := httpClient.Get(a.BundleURL)
+	r, err := c.httpClient.Get(a.BundleURL)
 	if err != nil {
 		return nil, err
 	}
@@ -225,7 +229,7 @@ func (c *LiveClient) getTrustDomain(url string) (string, error) {
 
 	bo := backoff.NewConstantBackOff(getAttestationRetryInterval)
 	err := backoff.Retry(func() error {
-		restErr := c.api.REST(c.host, http.MethodGet, url, nil, &resp)
+		restErr := c.githubAPI.REST(c.host, http.MethodGet, url, nil, &resp)
 		if restErr != nil {
 			if shouldRetry(restErr) {
 				return restErr
diff --git a/pkg/cmd/attestation/api/client_test.go b/pkg/cmd/attestation/api/client_test.go
index adac00598..fb3b338bf 100644
--- a/pkg/cmd/attestation/api/client_test.go
+++ b/pkg/cmd/attestation/api/client_test.go
@@ -22,7 +22,7 @@ func NewClientWithMockGHClient(hasNextPage bool) Client {
 
 	if hasNextPage {
 		return &LiveClient{
-			api: mockAPIClient{
+			githubAPI: mockAPIClient{
 				OnRESTWithNext: fetcher.OnRESTSuccessWithNextPage,
 			},
 			logger: l,
@@ -30,7 +30,7 @@ func NewClientWithMockGHClient(hasNextPage bool) Client {
 	}
 
 	return &LiveClient{
-		api: mockAPIClient{
+		githubAPI: mockAPIClient{
 			OnRESTWithNext: fetcher.OnRESTSuccess,
 		},
 		logger: l,
@@ -50,7 +50,7 @@ func TestGetURL(t *testing.T) {
 	}
 
 	for _, data := range testData {
-		s := c.BuildRepoAndDigestURL(data.repo, data.digest)
+		s := c.buildRepoAndDigestURL(data.repo, data.digest)
 		require.Equal(t, data.expected, s)
 	}
 }
@@ -135,7 +135,7 @@ func TestGetByDigest_NoAttestationsFound(t *testing.T) {
 	}
 
 	c := LiveClient{
-		api: mockAPIClient{
+		githubAPI: mockAPIClient{
 			OnRESTWithNext: fetcher.OnRESTWithNextNoAttestations,
 		},
 		logger: io.NewTestHandler(),
@@ -158,7 +158,7 @@ func TestGetByDigest_Error(t *testing.T) {
 	}
 
 	c := LiveClient{
-		api: mockAPIClient{
+		githubAPI: mockAPIClient{
 			OnRESTWithNext: fetcher.OnRESTWithNextError,
 		},
 		logger: io.NewTestHandler(),
@@ -180,7 +180,7 @@ func TestGetTrustDomain(t *testing.T) {
 
 	t.Run("with returned trust domain", func(t *testing.T) {
 		c := LiveClient{
-			api: mockAPIClient{
+			githubAPI: mockAPIClient{
 				OnREST: fetcher.OnREST,
 			},
 			logger: io.NewTestHandler(),
@@ -193,7 +193,7 @@ func TestGetTrustDomain(t *testing.T) {
 
 	t.Run("with error", func(t *testing.T) {
 		c := LiveClient{
-			api: mockAPIClient{
+			githubAPI: mockAPIClient{
 				OnREST: fetcher.OnRESTError,
 			},
 			logger: io.NewTestHandler(),
@@ -213,7 +213,7 @@ func TestGetAttestationsRetries(t *testing.T) {
 	}
 
 	c := &LiveClient{
-		api: mockAPIClient{
+		githubAPI: mockAPIClient{
 			OnRESTWithNext: fetcher.FlakyOnRESTSuccessWithNextPageHandler(),
 		},
 		logger: io.NewTestHandler(),
@@ -252,7 +252,7 @@ func TestGetAttestationsMaxRetries(t *testing.T) {
 	}
 
 	c := &LiveClient{
-		api: mockAPIClient{
+		githubAPI: mockAPIClient{
 			OnRESTWithNext: fetcher.OnREST500ErrorHandler(),
 		},
 		logger: io.NewTestHandler(),

From 45121f6674345a7b56ec8ef1153f7b2aee8c1a6f Mon Sep 17 00:00:00 2001
From: Meredith Lancaster <malancas@github.com>
Date: Mon, 16 Dec 2024 12:02:42 -0700
Subject: [PATCH 05/30] go mod tidy

Signed-off-by: Meredith Lancaster <malancas@github.com>
---
 go.mod | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/go.mod b/go.mod
index d2179290d..5a3188875 100644
--- a/go.mod
+++ b/go.mod
@@ -21,6 +21,7 @@ require (
 	github.com/distribution/reference v0.5.0
 	github.com/gabriel-vasile/mimetype v1.4.7
 	github.com/gdamore/tcell/v2 v2.5.4
+	github.com/golang/snappy v0.0.4
 	github.com/google/go-cmp v0.6.0
 	github.com/google/go-containerregistry v0.20.2
 	github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510
@@ -92,7 +93,6 @@ require (
 	github.com/go-openapi/swag v0.23.0 // indirect
 	github.com/go-openapi/validate v0.24.0 // indirect
 	github.com/godbus/dbus/v5 v5.1.0 // indirect
-	github.com/golang/snappy v0.0.4 // indirect
 	github.com/google/certificate-transparency-go v1.2.1 // indirect
 	github.com/google/uuid v1.6.0 // indirect
 	github.com/gorilla/css v1.0.0 // indirect

From 8f5d7100f5056cc375a6c58dabc9dfaf55424769 Mon Sep 17 00:00:00 2001
From: Meredith Lancaster <malancas@github.com>
Date: Mon, 16 Dec 2024 12:02:52 -0700
Subject: [PATCH 06/30] var naming

Signed-off-by: Meredith Lancaster <malancas@github.com>
---
 pkg/cmd/attestation/api/client.go | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/pkg/cmd/attestation/api/client.go b/pkg/cmd/attestation/api/client.go
index 34be1a6bb..7646b04e1 100644
--- a/pkg/cmd/attestation/api/client.go
+++ b/pkg/cmd/attestation/api/client.go
@@ -181,24 +181,24 @@ func (c *LiveClient) fetchBundleWithSASURL(a *Attestation) (*bundle.Bundle, erro
 
 	c.logger.VerbosePrintf("Fetching attestation bundle\n\n")
 
-	r, err := c.httpClient.Get(a.BundleURL)
+	resp, err := c.httpClient.Get(a.BundleURL)
 	if err != nil {
 		return nil, err
 	}
 
-	defer r.Body.Close()
-	if r.StatusCode != http.StatusOK {
-		return nil, fmt.Errorf("unexpected status code: %d", r.StatusCode)
+	defer resp.Body.Close()
+	if resp.StatusCode != http.StatusOK {
+		return nil, fmt.Errorf("unexpected status code: %d", resp.StatusCode)
 	}
 
 	// parse the URL to get the host and path
-	resp, err := io.ReadAll(r.Body)
+	body, err := io.ReadAll(resp.Body)
 	if err != nil {
 		return nil, fmt.Errorf("failed to read blob storage response body: %w", err)
 	}
 
 	var out []byte
-	decompressed, err := snappy.Decode(out, resp)
+	decompressed, err := snappy.Decode(out, body)
 	if err != nil {
 		return nil, fmt.Errorf("failed to decompress with snappy: %w", err)
 	}

From fb020f2a79aefa878f1e84d33410e18c8bb4e5f1 Mon Sep 17 00:00:00 2001
From: Meredith Lancaster <malancas@github.com>
Date: Mon, 16 Dec 2024 12:13:22 -0700
Subject: [PATCH 07/30] update error messages

Signed-off-by: Meredith Lancaster <malancas@github.com>
---
 pkg/cmd/attestation/api/client.go | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/pkg/cmd/attestation/api/client.go b/pkg/cmd/attestation/api/client.go
index 7646b04e1..fc5e06f7b 100644
--- a/pkg/cmd/attestation/api/client.go
+++ b/pkg/cmd/attestation/api/client.go
@@ -70,9 +70,9 @@ func (c *LiveClient) GetByRepoAndDigest(repo, digest string, limit int) ([]*Atte
 		return nil, fmt.Errorf("failed to fetch attestation by repo and digest: %w", err)
 	}
 
-	bundles, err := c.fetchBundlesWithSASURL(attestations)
+	bundles, err := c.fetchBundlesByURL(attestations)
 	if err != nil {
-		return nil, fmt.Errorf("failed to fetch attestation with SAS URL: %w", err)
+		return nil, fmt.Errorf("failed to fetch bundle with URL: %w", err)
 	}
 
 	return bundles, nil
@@ -91,9 +91,9 @@ func (c *LiveClient) GetByOwnerAndDigest(owner, digest string, limit int) ([]*At
 		return nil, fmt.Errorf("failed to fetch attestation by repo and digest: %w", err)
 	}
 
-	bundles, err := c.fetchBundlesWithSASURL(attestations)
+	bundles, err := c.fetchBundlesByURL(attestations)
 	if err != nil {
-		return nil, fmt.Errorf("failed to fetch attestation with SAS URL: %w", err)
+		return nil, fmt.Errorf("failed to fetch bundle with URL: %w", err)
 	}
 
 	return bundles, nil
@@ -160,12 +160,12 @@ func (c *LiveClient) getAttestations(url, name, digest string, limit int) ([]*At
 	return attestations, nil
 }
 
-func (c *LiveClient) fetchBundlesWithSASURL(attestations []*Attestation) ([]*Attestation, error) {
+func (c *LiveClient) fetchBundlesByURL(attestations []*Attestation) ([]*Attestation, error) {
 	fetched := make([]*Attestation, len(attestations))
 	for i, a := range attestations {
-		b, err := c.fetchBundleWithSASURL(a)
+		b, err := c.fetchBundleByURL(a)
 		if err != nil {
-			return nil, fmt.Errorf("failed to fetch bundle with SAS URL: %w", err)
+			return nil, fmt.Errorf("failed to fetch bundle with URL: %w", err)
 		}
 		fetched[i] = &Attestation{
 			Bundle: b,
@@ -174,7 +174,7 @@ func (c *LiveClient) fetchBundlesWithSASURL(attestations []*Attestation) ([]*Att
 	return fetched, nil
 }
 
-func (c *LiveClient) fetchBundleWithSASURL(a *Attestation) (*bundle.Bundle, error) {
+func (c *LiveClient) fetchBundleByURL(a *Attestation) (*bundle.Bundle, error) {
 	if a.BundleURL == "" {
 		return nil, fmt.Errorf("bundle URL is empty")
 	}

From e4431a3f55de6c9a20394e31aaa91f5f40a8d191 Mon Sep 17 00:00:00 2001
From: Meredith Lancaster <malancas@github.com>
Date: Mon, 16 Dec 2024 12:22:20 -0700
Subject: [PATCH 08/30] add mock http client

Signed-off-by: Meredith Lancaster <malancas@github.com>
---
 pkg/cmd/attestation/api/client_test.go        |  6 +++++
 pkg/cmd/attestation/api/mock_client.go        |  2 +-
 ...t_test.go => mock_githubApiClient_test.go} |  0
 .../attestation/api/mock_httpClient_test.go   | 25 +++++++++++++++++++
 4 files changed, 32 insertions(+), 1 deletion(-)
 rename pkg/cmd/attestation/api/{mock_apiClient_test.go => mock_githubApiClient_test.go} (100%)
 create mode 100644 pkg/cmd/attestation/api/mock_httpClient_test.go

diff --git a/pkg/cmd/attestation/api/client_test.go b/pkg/cmd/attestation/api/client_test.go
index fb3b338bf..9e47870cf 100644
--- a/pkg/cmd/attestation/api/client_test.go
+++ b/pkg/cmd/attestation/api/client_test.go
@@ -25,6 +25,9 @@ func NewClientWithMockGHClient(hasNextPage bool) Client {
 			githubAPI: mockAPIClient{
 				OnRESTWithNext: fetcher.OnRESTSuccessWithNextPage,
 			},
+			httpClient: mockHttpClient{
+				OnGet: fetcher.OnGetSuccess,
+			},
 			logger: l,
 		}
 	}
@@ -33,6 +36,9 @@ func NewClientWithMockGHClient(hasNextPage bool) Client {
 		githubAPI: mockAPIClient{
 			OnRESTWithNext: fetcher.OnRESTSuccess,
 		},
+		httpClient: mockHttpClient{
+			OnGet: fetcher.OnGetSuccess,
+		},
 		logger: l,
 	}
 }
diff --git a/pkg/cmd/attestation/api/mock_client.go b/pkg/cmd/attestation/api/mock_client.go
index bcb51c414..b2fd334c0 100644
--- a/pkg/cmd/attestation/api/mock_client.go
+++ b/pkg/cmd/attestation/api/mock_client.go
@@ -25,7 +25,7 @@ func (m MockClient) GetTrustDomain() (string, error) {
 }
 
 func makeTestAttestation() Attestation {
-	return Attestation{Bundle: data.SigstoreBundle(nil)}
+	return Attestation{Bundle: data.SigstoreBundle(nil), BundleURL: "https://example.com"}
 }
 
 func OnGetByRepoAndDigestSuccess(repo, digest string, limit int) ([]*Attestation, error) {
diff --git a/pkg/cmd/attestation/api/mock_apiClient_test.go b/pkg/cmd/attestation/api/mock_githubApiClient_test.go
similarity index 100%
rename from pkg/cmd/attestation/api/mock_apiClient_test.go
rename to pkg/cmd/attestation/api/mock_githubApiClient_test.go
diff --git a/pkg/cmd/attestation/api/mock_httpClient_test.go b/pkg/cmd/attestation/api/mock_httpClient_test.go
new file mode 100644
index 000000000..70eceeea5
--- /dev/null
+++ b/pkg/cmd/attestation/api/mock_httpClient_test.go
@@ -0,0 +1,25 @@
+package api
+
+import (
+	"io"
+	"net/http"
+	"strings"
+
+	"github.com/cli/cli/v2/pkg/cmd/attestation/test/data"
+)
+
+type mockHttpClient struct {
+	OnGet func(url string) (*http.Response, error)
+}
+
+func (m mockHttpClient) Get(url string) (*http.Response, error) {
+	return m.OnGet(url)
+}
+
+func (m *mockDataGenerator) OnGetSuccess(url string) (*http.Response, error) {
+	bundle := data.SigstoreBundle(nil)
+	return &http.Response{
+		StatusCode: 200,
+		Body:       io.NopCloser(strings.NewReader(bundle.String())),
+	}, nil
+}

From ab4912ff48f8aea8ec48e96079e46ea4af8c546f Mon Sep 17 00:00:00 2001
From: Meredith Lancaster <malancas@github.com>
Date: Mon, 16 Dec 2024 12:40:13 -0700
Subject: [PATCH 09/30] fix failing tests

Signed-off-by: Meredith Lancaster <malancas@github.com>
---
 pkg/cmd/attestation/api/client.go               | 6 +++++-
 pkg/cmd/attestation/api/client_test.go          | 6 ++++++
 pkg/cmd/attestation/api/mock_httpClient_test.go | 7 ++++---
 pkg/cmd/attestation/verification/attestation.go | 2 +-
 4 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/pkg/cmd/attestation/api/client.go b/pkg/cmd/attestation/api/client.go
index fc5e06f7b..2519aa9e4 100644
--- a/pkg/cmd/attestation/api/client.go
+++ b/pkg/cmd/attestation/api/client.go
@@ -88,7 +88,11 @@ func (c *LiveClient) GetByOwnerAndDigest(owner, digest string, limit int) ([]*At
 	url := c.buildOwnerAndDigestURL(owner, digest)
 	attestations, err := c.getAttestations(url, owner, digest, limit)
 	if err != nil {
-		return nil, fmt.Errorf("failed to fetch attestation by repo and digest: %w", err)
+		return nil, err
+	}
+
+	if len(attestations) == 0 {
+		return nil, newErrNoAttestations(owner, digest)
 	}
 
 	bundles, err := c.fetchBundlesByURL(attestations)
diff --git a/pkg/cmd/attestation/api/client_test.go b/pkg/cmd/attestation/api/client_test.go
index 9e47870cf..540ae1366 100644
--- a/pkg/cmd/attestation/api/client_test.go
+++ b/pkg/cmd/attestation/api/client_test.go
@@ -144,6 +144,9 @@ func TestGetByDigest_NoAttestationsFound(t *testing.T) {
 		githubAPI: mockAPIClient{
 			OnRESTWithNext: fetcher.OnRESTWithNextNoAttestations,
 		},
+		httpClient: mockHttpClient{
+			OnGet: fetcher.OnGetSuccess,
+		},
 		logger: io.NewTestHandler(),
 	}
 
@@ -222,6 +225,9 @@ func TestGetAttestationsRetries(t *testing.T) {
 		githubAPI: mockAPIClient{
 			OnRESTWithNext: fetcher.FlakyOnRESTSuccessWithNextPageHandler(),
 		},
+		httpClient: mockHttpClient{
+			OnGet: fetcher.OnGetSuccess,
+		},
 		logger: io.NewTestHandler(),
 	}
 
diff --git a/pkg/cmd/attestation/api/mock_httpClient_test.go b/pkg/cmd/attestation/api/mock_httpClient_test.go
index 70eceeea5..89584874f 100644
--- a/pkg/cmd/attestation/api/mock_httpClient_test.go
+++ b/pkg/cmd/attestation/api/mock_httpClient_test.go
@@ -1,11 +1,12 @@
 package api
 
 import (
+	"bytes"
 	"io"
 	"net/http"
-	"strings"
 
 	"github.com/cli/cli/v2/pkg/cmd/attestation/test/data"
+	"github.com/golang/snappy"
 )
 
 type mockHttpClient struct {
@@ -17,9 +18,9 @@ func (m mockHttpClient) Get(url string) (*http.Response, error) {
 }
 
 func (m *mockDataGenerator) OnGetSuccess(url string) (*http.Response, error) {
-	bundle := data.SigstoreBundle(nil)
+	compressed := snappy.Encode(nil, data.SigstoreBundleRaw)
 	return &http.Response{
 		StatusCode: 200,
-		Body:       io.NopCloser(strings.NewReader(bundle.String())),
+		Body:       io.NopCloser(bytes.NewReader(compressed)),
 	}, nil
 }
diff --git a/pkg/cmd/attestation/verification/attestation.go b/pkg/cmd/attestation/verification/attestation.go
index 41da71010..8022a4943 100644
--- a/pkg/cmd/attestation/verification/attestation.go
+++ b/pkg/cmd/attestation/verification/attestation.go
@@ -104,7 +104,7 @@ func GetRemoteAttestations(client api.Client, params FetchRemoteAttestationsPara
 	if params.Repo != "" {
 		bundles, err := client.GetByRepoAndDigest(params.Repo, params.Digest, params.Limit)
 		if err != nil {
-			return nil, fmt.Errorf("failed to fetch attestation bundles from %s: %w", params.Repo, err)
+			return nil, fmt.Errorf("failed to fetch attestations from %s: %w", params.Repo, err)
 		}
 
 		return bundles, nil

From 9051da39fc559ee73ffb1d870863c9100fd5c9f6 Mon Sep 17 00:00:00 2001
From: Meredith Lancaster <malancas@github.com>
Date: Mon, 6 Jan 2025 10:19:47 -0700
Subject: [PATCH 10/30] provide additional logging and fallback

Signed-off-by: Meredith Lancaster <malancas@github.com>
---
 pkg/cmd/attestation/api/client.go | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/pkg/cmd/attestation/api/client.go b/pkg/cmd/attestation/api/client.go
index 2519aa9e4..51b67d33d 100644
--- a/pkg/cmd/attestation/api/client.go
+++ b/pkg/cmd/attestation/api/client.go
@@ -26,11 +26,13 @@ const (
 // Allow injecting backoff interval in tests.
 var getAttestationRetryInterval = time.Millisecond * 200
 
+// githubApiClient makes REST calls to the GitHub API
 type githubApiClient interface {
 	REST(hostname, method, p string, body io.Reader, data interface{}) error
 	RESTWithNext(hostname, method, p string, body io.Reader, data interface{}) (string, error)
 }
 
+// httpClient makes HTTP calls to all non-GitHub API endpoints
 type httpClient interface {
 	Get(url string) (*http.Response, error)
 }
@@ -180,7 +182,8 @@ func (c *LiveClient) fetchBundlesByURL(attestations []*Attestation) ([]*Attestat
 
 func (c *LiveClient) fetchBundleByURL(a *Attestation) (*bundle.Bundle, error) {
 	if a.BundleURL == "" {
-		return nil, fmt.Errorf("bundle URL is empty")
+		c.logger.VerbosePrintf("Bundle URL is empty. Falling back to bundle field\n\n")
+		return a.Bundle, nil
 	}
 
 	c.logger.VerbosePrintf("Fetching attestation bundle\n\n")

From 311f2b2e236dc0f5a60026e1a5e920e8b063799e Mon Sep 17 00:00:00 2001
From: Meredith Lancaster <malancas@github.com>
Date: Mon, 6 Jan 2025 10:39:40 -0700
Subject: [PATCH 11/30] return fetch attestations err directly

Signed-off-by: Meredith Lancaster <malancas@github.com>
---
 pkg/cmd/attestation/api/client.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/pkg/cmd/attestation/api/client.go b/pkg/cmd/attestation/api/client.go
index 51b67d33d..d681be446 100644
--- a/pkg/cmd/attestation/api/client.go
+++ b/pkg/cmd/attestation/api/client.go
@@ -69,7 +69,7 @@ func (c *LiveClient) GetByRepoAndDigest(repo, digest string, limit int) ([]*Atte
 	url := c.buildRepoAndDigestURL(repo, digest)
 	attestations, err := c.getAttestations(url, repo, digest, limit)
 	if err != nil {
-		return nil, fmt.Errorf("failed to fetch attestation by repo and digest: %w", err)
+		return nil, err
 	}
 
 	bundles, err := c.fetchBundlesByURL(attestations)

From 070b67e5a42c68b7db066e6dd6118113eb16e7d3 Mon Sep 17 00:00:00 2001
From: Meredith Lancaster <malancas@github.com>
Date: Mon, 6 Jan 2025 10:44:55 -0700
Subject: [PATCH 12/30] fetch bundles in parallel

Signed-off-by: Meredith Lancaster <malancas@github.com>
---
 pkg/cmd/attestation/api/client.go | 24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/pkg/cmd/attestation/api/client.go b/pkg/cmd/attestation/api/client.go
index d681be446..1a29e3044 100644
--- a/pkg/cmd/attestation/api/client.go
+++ b/pkg/cmd/attestation/api/client.go
@@ -14,6 +14,7 @@ import (
 	"github.com/golang/snappy"
 	v1 "github.com/sigstore/protobuf-specs/gen/pb-go/bundle/v1"
 	"github.com/sigstore/sigstore-go/pkg/bundle"
+	"golang.org/x/sync/errgroup"
 	"google.golang.org/protobuf/encoding/protojson"
 )
 
@@ -168,15 +169,24 @@ func (c *LiveClient) getAttestations(url, name, digest string, limit int) ([]*At
 
 func (c *LiveClient) fetchBundlesByURL(attestations []*Attestation) ([]*Attestation, error) {
 	fetched := make([]*Attestation, len(attestations))
+	g := errgroup.Group{}
 	for i, a := range attestations {
-		b, err := c.fetchBundleByURL(a)
-		if err != nil {
-			return nil, fmt.Errorf("failed to fetch bundle with URL: %w", err)
-		}
-		fetched[i] = &Attestation{
-			Bundle: b,
-		}
+		g.Go(func() error {
+			b, err := c.fetchBundleByURL(a)
+			if err != nil {
+				return fmt.Errorf("failed to fetch bundle with URL: %w", err)
+			}
+			fetched[i] = &Attestation{
+				Bundle: b,
+			}
+			return nil
+		})
 	}
+
+	if err := g.Wait(); err != nil {
+		return nil, err
+	}
+
 	return fetched, nil
 }
 

From e03a36ea3c44e59bf0a1cf5e5a4fe082da13c734 Mon Sep 17 00:00:00 2001
From: Meredith Lancaster <malancas@github.com>
Date: Mon, 6 Jan 2025 12:12:26 -0700
Subject: [PATCH 13/30] add tests for bundle url fetch and fallback

Signed-off-by: Meredith Lancaster <malancas@github.com>
---
 pkg/cmd/attestation/api/client.go             |  2 +-
 pkg/cmd/attestation/api/client_test.go        | 53 ++++++++++++++++---
 .../attestation/api/mock_httpClient_test.go   |  8 +--
 3 files changed, 51 insertions(+), 12 deletions(-)

diff --git a/pkg/cmd/attestation/api/client.go b/pkg/cmd/attestation/api/client.go
index 1a29e3044..d0ef3f21b 100644
--- a/pkg/cmd/attestation/api/client.go
+++ b/pkg/cmd/attestation/api/client.go
@@ -196,7 +196,7 @@ func (c *LiveClient) fetchBundleByURL(a *Attestation) (*bundle.Bundle, error) {
 		return a.Bundle, nil
 	}
 
-	c.logger.VerbosePrintf("Fetching attestation bundle\n\n")
+	c.logger.VerbosePrintf("Fetching attestation bundle with bundle URL\n\n")
 
 	resp, err := c.httpClient.Get(a.BundleURL)
 	if err != nil {
diff --git a/pkg/cmd/attestation/api/client_test.go b/pkg/cmd/attestation/api/client_test.go
index 540ae1366..62b1c40cf 100644
--- a/pkg/cmd/attestation/api/client_test.go
+++ b/pkg/cmd/attestation/api/client_test.go
@@ -4,6 +4,7 @@ import (
 	"testing"
 
 	"github.com/cli/cli/v2/pkg/cmd/attestation/io"
+	"github.com/cli/cli/v2/pkg/cmd/attestation/test/data"
 
 	"github.com/stretchr/testify/require"
 )
@@ -25,8 +26,8 @@ func NewClientWithMockGHClient(hasNextPage bool) Client {
 			githubAPI: mockAPIClient{
 				OnRESTWithNext: fetcher.OnRESTSuccessWithNextPage,
 			},
-			httpClient: mockHttpClient{
-				OnGet: fetcher.OnGetSuccess,
+			httpClient: &mockHttpClient{
+				OnGet: OnGetSuccess,
 			},
 			logger: l,
 		}
@@ -36,8 +37,8 @@ func NewClientWithMockGHClient(hasNextPage bool) Client {
 		githubAPI: mockAPIClient{
 			OnRESTWithNext: fetcher.OnRESTSuccess,
 		},
-		httpClient: mockHttpClient{
-			OnGet: fetcher.OnGetSuccess,
+		httpClient: &mockHttpClient{
+			OnGet: OnGetSuccess,
 		},
 		logger: l,
 	}
@@ -144,8 +145,8 @@ func TestGetByDigest_NoAttestationsFound(t *testing.T) {
 		githubAPI: mockAPIClient{
 			OnRESTWithNext: fetcher.OnRESTWithNextNoAttestations,
 		},
-		httpClient: mockHttpClient{
-			OnGet: fetcher.OnGetSuccess,
+		httpClient: &mockHttpClient{
+			OnGet: OnGetSuccess,
 		},
 		logger: io.NewTestHandler(),
 	}
@@ -182,6 +183,42 @@ func TestGetByDigest_Error(t *testing.T) {
 	require.Nil(t, attestations)
 }
 
+func TestFetchBundleByURL(t *testing.T) {
+	t.Run("fetch by bundle URL successfully", func(t *testing.T) {
+		httpClient := mockHttpClient{
+			OnGet: OnGetSuccess,
+		}
+		c := &LiveClient{
+			httpClient: &httpClient,
+			logger:     io.NewTestHandler(),
+		}
+
+		attestation := makeTestAttestation()
+		bundle, err := c.fetchBundleByURL(&attestation)
+		require.NoError(t, err)
+		require.Equal(t, "application/vnd.dev.sigstore.bundle.v0.3+json", bundle.GetMediaType())
+		require.True(t, httpClient.called)
+	})
+
+	t.Run("fallback to bundle field when BundleURL field is empty", func(t *testing.T) {
+		httpClient := mockHttpClient{
+			OnGet: OnGetSuccess,
+		}
+		c := &LiveClient{
+			httpClient: &mockHttpClient{
+				OnGet: OnGetSuccess,
+			},
+			logger: io.NewTestHandler(),
+		}
+
+		attestation := Attestation{Bundle: data.SigstoreBundle(t)}
+		bundle, err := c.fetchBundleByURL(&attestation)
+		require.NoError(t, err)
+		require.Equal(t, "application/vnd.dev.sigstore.bundle.v0.3+json", bundle.GetMediaType())
+		require.False(t, httpClient.called)
+	})
+}
+
 func TestGetTrustDomain(t *testing.T) {
 	fetcher := mockMetaGenerator{
 		TrustDomain: "foo",
@@ -225,8 +262,8 @@ func TestGetAttestationsRetries(t *testing.T) {
 		githubAPI: mockAPIClient{
 			OnRESTWithNext: fetcher.FlakyOnRESTSuccessWithNextPageHandler(),
 		},
-		httpClient: mockHttpClient{
-			OnGet: fetcher.OnGetSuccess,
+		httpClient: &mockHttpClient{
+			OnGet: OnGetSuccess,
 		},
 		logger: io.NewTestHandler(),
 	}
diff --git a/pkg/cmd/attestation/api/mock_httpClient_test.go b/pkg/cmd/attestation/api/mock_httpClient_test.go
index 89584874f..365619ac1 100644
--- a/pkg/cmd/attestation/api/mock_httpClient_test.go
+++ b/pkg/cmd/attestation/api/mock_httpClient_test.go
@@ -10,14 +10,16 @@ import (
 )
 
 type mockHttpClient struct {
-	OnGet func(url string) (*http.Response, error)
+	called bool
+	OnGet  func(url string) (*http.Response, error)
 }
 
-func (m mockHttpClient) Get(url string) (*http.Response, error) {
+func (m *mockHttpClient) Get(url string) (*http.Response, error) {
+	m.called = true
 	return m.OnGet(url)
 }
 
-func (m *mockDataGenerator) OnGetSuccess(url string) (*http.Response, error) {
+func OnGetSuccess(url string) (*http.Response, error) {
 	compressed := snappy.Encode(nil, data.SigstoreBundleRaw)
 	return &http.Response{
 		StatusCode: 200,

From 0202ca8df5b645dfc92330a08bac63a111d8ace9 Mon Sep 17 00:00:00 2001
From: Meredith Lancaster <malancas@github.com>
Date: Mon, 6 Jan 2025 12:58:09 -0700
Subject: [PATCH 14/30] add test case for bundle url fetch failure

Signed-off-by: Meredith Lancaster <malancas@github.com>
---
 pkg/cmd/attestation/api/client_test.go        | 71 +++++++++++--------
 .../attestation/api/mock_httpClient_test.go   |  7 ++
 2 files changed, 48 insertions(+), 30 deletions(-)

diff --git a/pkg/cmd/attestation/api/client_test.go b/pkg/cmd/attestation/api/client_test.go
index 62b1c40cf..4fb0f539a 100644
--- a/pkg/cmd/attestation/api/client_test.go
+++ b/pkg/cmd/attestation/api/client_test.go
@@ -184,39 +184,50 @@ func TestGetByDigest_Error(t *testing.T) {
 }
 
 func TestFetchBundleByURL(t *testing.T) {
-	t.Run("fetch by bundle URL successfully", func(t *testing.T) {
-		httpClient := mockHttpClient{
-			OnGet: OnGetSuccess,
-		}
-		c := &LiveClient{
-			httpClient: &httpClient,
-			logger:     io.NewTestHandler(),
-		}
+	httpClient := mockHttpClient{
+		OnGet: OnGetSuccess,
+	}
+	c := &LiveClient{
+		httpClient: &httpClient,
+		logger:     io.NewTestHandler(),
+	}
 
-		attestation := makeTestAttestation()
-		bundle, err := c.fetchBundleByURL(&attestation)
-		require.NoError(t, err)
-		require.Equal(t, "application/vnd.dev.sigstore.bundle.v0.3+json", bundle.GetMediaType())
-		require.True(t, httpClient.called)
-	})
+	attestation := makeTestAttestation()
+	bundle, err := c.fetchBundleByURL(&attestation)
+	require.NoError(t, err)
+	require.Equal(t, "application/vnd.dev.sigstore.bundle.v0.3+json", bundle.GetMediaType())
+	require.True(t, httpClient.called)
+}
+func TestFetchBundleByURL_FetchByURLFail(t *testing.T) {
+	httpClient := mockHttpClient{
+		OnGet: OnGetFail,
+	}
+	c := &LiveClient{
+		httpClient: &httpClient,
+		logger:     io.NewTestHandler(),
+	}
 
-	t.Run("fallback to bundle field when BundleURL field is empty", func(t *testing.T) {
-		httpClient := mockHttpClient{
-			OnGet: OnGetSuccess,
-		}
-		c := &LiveClient{
-			httpClient: &mockHttpClient{
-				OnGet: OnGetSuccess,
-			},
-			logger: io.NewTestHandler(),
-		}
+	attestation := makeTestAttestation()
+	bundle, err := c.fetchBundleByURL(&attestation)
+	require.Error(t, err)
+	require.Nil(t, bundle)
+	require.True(t, httpClient.called)
+}
 
-		attestation := Attestation{Bundle: data.SigstoreBundle(t)}
-		bundle, err := c.fetchBundleByURL(&attestation)
-		require.NoError(t, err)
-		require.Equal(t, "application/vnd.dev.sigstore.bundle.v0.3+json", bundle.GetMediaType())
-		require.False(t, httpClient.called)
-	})
+func TestFetchBundleByURL_FallbackToBundleField(t *testing.T) {
+	httpClient := mockHttpClient{
+		OnGet: OnGetSuccess,
+	}
+	c := &LiveClient{
+		httpClient: &httpClient,
+		logger:     io.NewTestHandler(),
+	}
+
+	attestation := Attestation{Bundle: data.SigstoreBundle(t)}
+	bundle, err := c.fetchBundleByURL(&attestation)
+	require.NoError(t, err)
+	require.Equal(t, "application/vnd.dev.sigstore.bundle.v0.3+json", bundle.GetMediaType())
+	require.False(t, httpClient.called)
 }
 
 func TestGetTrustDomain(t *testing.T) {
diff --git a/pkg/cmd/attestation/api/mock_httpClient_test.go b/pkg/cmd/attestation/api/mock_httpClient_test.go
index 365619ac1..b89c101fe 100644
--- a/pkg/cmd/attestation/api/mock_httpClient_test.go
+++ b/pkg/cmd/attestation/api/mock_httpClient_test.go
@@ -2,6 +2,7 @@ package api
 
 import (
 	"bytes"
+	"fmt"
 	"io"
 	"net/http"
 
@@ -26,3 +27,9 @@ func OnGetSuccess(url string) (*http.Response, error) {
 		Body:       io.NopCloser(bytes.NewReader(compressed)),
 	}, nil
 }
+
+func OnGetFail(url string) (*http.Response, error) {
+	return &http.Response{
+		StatusCode: 500,
+	}, fmt.Errorf("failed to fetch with %s", url)
+}

From 69865117ab92abc2a1cb075b38f24302185de03e Mon Sep 17 00:00:00 2001
From: Meredith Lancaster <malancas@github.com>
Date: Mon, 6 Jan 2025 13:14:02 -0700
Subject: [PATCH 15/30] add mutex for test field

Signed-off-by: Meredith Lancaster <malancas@github.com>
---
 pkg/cmd/attestation/api/mock_httpClient_test.go | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/pkg/cmd/attestation/api/mock_httpClient_test.go b/pkg/cmd/attestation/api/mock_httpClient_test.go
index b89c101fe..2914d7887 100644
--- a/pkg/cmd/attestation/api/mock_httpClient_test.go
+++ b/pkg/cmd/attestation/api/mock_httpClient_test.go
@@ -5,18 +5,22 @@ import (
 	"fmt"
 	"io"
 	"net/http"
+	"sync"
 
 	"github.com/cli/cli/v2/pkg/cmd/attestation/test/data"
 	"github.com/golang/snappy"
 )
 
 type mockHttpClient struct {
+	mutex  sync.RWMutex
 	called bool
 	OnGet  func(url string) (*http.Response, error)
 }
 
 func (m *mockHttpClient) Get(url string) (*http.Response, error) {
+	m.mutex.Lock()
 	m.called = true
+	m.mutex.Unlock()
 	return m.OnGet(url)
 }
 

From 9ecd90c26ceb564e0dea8f492f9f42da952ab6c8 Mon Sep 17 00:00:00 2001
From: Meredith Lancaster <malancas@github.com>
Date: Tue, 7 Jan 2025 10:24:42 -0700
Subject: [PATCH 16/30] setup testing struct for test cases

Signed-off-by: Meredith Lancaster <malancas@github.com>
---
 pkg/cmd/attestation/api/client_test.go        | 89 ++++++++++++-------
 .../attestation/api/mock_httpClient_test.go   | 26 +++---
 2 files changed, 71 insertions(+), 44 deletions(-)

diff --git a/pkg/cmd/attestation/api/client_test.go b/pkg/cmd/attestation/api/client_test.go
index 4fb0f539a..0f91aff7e 100644
--- a/pkg/cmd/attestation/api/client_test.go
+++ b/pkg/cmd/attestation/api/client_test.go
@@ -21,15 +21,15 @@ func NewClientWithMockGHClient(hasNextPage bool) Client {
 	}
 	l := io.NewTestHandler()
 
+	mockHTTPClient := &mockHttpClient{}
+
 	if hasNextPage {
 		return &LiveClient{
 			githubAPI: mockAPIClient{
 				OnRESTWithNext: fetcher.OnRESTSuccessWithNextPage,
 			},
-			httpClient: &mockHttpClient{
-				OnGet: OnGetSuccess,
-			},
-			logger: l,
+			httpClient: mockHTTPClient,
+			logger:     l,
 		}
 	}
 
@@ -37,10 +37,8 @@ func NewClientWithMockGHClient(hasNextPage bool) Client {
 		githubAPI: mockAPIClient{
 			OnRESTWithNext: fetcher.OnRESTSuccess,
 		},
-		httpClient: &mockHttpClient{
-			OnGet: OnGetSuccess,
-		},
-		logger: l,
+		httpClient: mockHTTPClient,
+		logger:     l,
 	}
 }
 
@@ -145,10 +143,8 @@ func TestGetByDigest_NoAttestationsFound(t *testing.T) {
 		githubAPI: mockAPIClient{
 			OnRESTWithNext: fetcher.OnRESTWithNextNoAttestations,
 		},
-		httpClient: &mockHttpClient{
-			OnGet: OnGetSuccess,
-		},
-		logger: io.NewTestHandler(),
+		httpClient: &mockHttpClient{},
+		logger:     io.NewTestHandler(),
 	}
 
 	attestations, err := c.GetByRepoAndDigest(testRepo, testDigest, DefaultLimit)
@@ -183,12 +179,46 @@ func TestGetByDigest_Error(t *testing.T) {
 	require.Nil(t, attestations)
 }
 
-func TestFetchBundleByURL(t *testing.T) {
-	httpClient := mockHttpClient{
-		OnGet: OnGetSuccess,
+func TestFetchBundlesByURL(t *testing.T) {
+	mockHTTPClient := &mockHttpClient{}
+	client := LiveClient{
+		httpClient: mockHTTPClient,
+		logger:     io.NewTestHandler(),
 	}
+
+	att1 := makeTestAttestation()
+	att2 := makeTestAttestation()
+	attestations := []*Attestation{&att1, &att2}
+	fetched, err := client.fetchBundlesByURL(attestations)
+	require.NoError(t, err)
+	require.Len(t, fetched, 2)
+	require.Equal(t, "application/vnd.dev.sigstore.bundle.v0.3+json", fetched[0].Bundle.GetMediaType())
+	require.Equal(t, 2, mockHTTPClient.currNumCalls)
+}
+
+func TestFetchBundlesByURL_Fail(t *testing.T) {
+	mockHTTPClient := &mockHttpClient{
+		failAfterNumCalls: 2,
+	}
+
 	c := &LiveClient{
-		httpClient: &httpClient,
+		httpClient: mockHTTPClient,
+		logger:     io.NewTestHandler(),
+	}
+
+	att1 := makeTestAttestation()
+	att2 := makeTestAttestation()
+	attestations := []*Attestation{&att1, &att2}
+	fetched, err := c.fetchBundlesByURL(attestations)
+	require.Error(t, err)
+	require.Nil(t, fetched)
+}
+
+func TestFetchBundleByURL(t *testing.T) {
+	mockHTTPClient := &mockHttpClient{}
+
+	c := &LiveClient{
+		httpClient: mockHTTPClient,
 		logger:     io.NewTestHandler(),
 	}
 
@@ -196,14 +226,16 @@ func TestFetchBundleByURL(t *testing.T) {
 	bundle, err := c.fetchBundleByURL(&attestation)
 	require.NoError(t, err)
 	require.Equal(t, "application/vnd.dev.sigstore.bundle.v0.3+json", bundle.GetMediaType())
-	require.True(t, httpClient.called)
+	require.Equal(t, 1, mockHTTPClient.currNumCalls)
 }
+
 func TestFetchBundleByURL_FetchByURLFail(t *testing.T) {
-	httpClient := mockHttpClient{
-		OnGet: OnGetFail,
+	mockHTTPClient := &mockHttpClient{
+		failAfterNumCalls: 1,
 	}
+
 	c := &LiveClient{
-		httpClient: &httpClient,
+		httpClient: mockHTTPClient,
 		logger:     io.NewTestHandler(),
 	}
 
@@ -211,15 +243,14 @@ func TestFetchBundleByURL_FetchByURLFail(t *testing.T) {
 	bundle, err := c.fetchBundleByURL(&attestation)
 	require.Error(t, err)
 	require.Nil(t, bundle)
-	require.True(t, httpClient.called)
+	require.Equal(t, 1, mockHTTPClient.currNumCalls)
 }
 
 func TestFetchBundleByURL_FallbackToBundleField(t *testing.T) {
-	httpClient := mockHttpClient{
-		OnGet: OnGetSuccess,
-	}
+	mockHTTPClient := &mockHttpClient{}
+
 	c := &LiveClient{
-		httpClient: &httpClient,
+		httpClient: mockHTTPClient,
 		logger:     io.NewTestHandler(),
 	}
 
@@ -227,7 +258,7 @@ func TestFetchBundleByURL_FallbackToBundleField(t *testing.T) {
 	bundle, err := c.fetchBundleByURL(&attestation)
 	require.NoError(t, err)
 	require.Equal(t, "application/vnd.dev.sigstore.bundle.v0.3+json", bundle.GetMediaType())
-	require.False(t, httpClient.called)
+	require.Equal(t, 0, mockHTTPClient.currNumCalls)
 }
 
 func TestGetTrustDomain(t *testing.T) {
@@ -273,10 +304,8 @@ func TestGetAttestationsRetries(t *testing.T) {
 		githubAPI: mockAPIClient{
 			OnRESTWithNext: fetcher.FlakyOnRESTSuccessWithNextPageHandler(),
 		},
-		httpClient: &mockHttpClient{
-			OnGet: OnGetSuccess,
-		},
-		logger: io.NewTestHandler(),
+		httpClient: &mockHttpClient{},
+		logger:     io.NewTestHandler(),
 	}
 
 	attestations, err := c.GetByRepoAndDigest(testRepo, testDigest, DefaultLimit)
diff --git a/pkg/cmd/attestation/api/mock_httpClient_test.go b/pkg/cmd/attestation/api/mock_httpClient_test.go
index 2914d7887..3218ed508 100644
--- a/pkg/cmd/attestation/api/mock_httpClient_test.go
+++ b/pkg/cmd/attestation/api/mock_httpClient_test.go
@@ -12,28 +12,26 @@ import (
 )
 
 type mockHttpClient struct {
-	mutex  sync.RWMutex
-	called bool
-	OnGet  func(url string) (*http.Response, error)
+	mutex             sync.RWMutex
+	currNumCalls      int
+	failAfterNumCalls int
 }
 
 func (m *mockHttpClient) Get(url string) (*http.Response, error) {
 	m.mutex.Lock()
-	m.called = true
+	m.currNumCalls++
 	m.mutex.Unlock()
-	return m.OnGet(url)
-}
 
-func OnGetSuccess(url string) (*http.Response, error) {
-	compressed := snappy.Encode(nil, data.SigstoreBundleRaw)
+	if m.failAfterNumCalls > 0 && m.currNumCalls >= m.failAfterNumCalls {
+		return &http.Response{
+			StatusCode: 500,
+		}, fmt.Errorf("failed to fetch with %s", url)
+	}
+
+	var compressed []byte
+	compressed = snappy.Encode(compressed, data.SigstoreBundleRaw)
 	return &http.Response{
 		StatusCode: 200,
 		Body:       io.NopCloser(bytes.NewReader(compressed)),
 	}, nil
 }
-
-func OnGetFail(url string) (*http.Response, error) {
-	return &http.Response{
-		StatusCode: 500,
-	}, fmt.Errorf("failed to fetch with %s", url)
-}

From e34e188ee2c38f345203cbfa1eeb000313e1763e Mon Sep 17 00:00:00 2001
From: Meredith Lancaster <malancas@github.com>
Date: Tue, 7 Jan 2025 10:43:24 -0700
Subject: [PATCH 17/30] add http client test constructors

Signed-off-by: Meredith Lancaster <malancas@github.com>
---
 pkg/cmd/attestation/api/client_test.go        | 37 +++++++++----------
 .../attestation/api/mock_httpClient_test.go   | 23 +++++++++++-
 2 files changed, 39 insertions(+), 21 deletions(-)

diff --git a/pkg/cmd/attestation/api/client_test.go b/pkg/cmd/attestation/api/client_test.go
index 0f91aff7e..23dcb77d9 100644
--- a/pkg/cmd/attestation/api/client_test.go
+++ b/pkg/cmd/attestation/api/client_test.go
@@ -21,14 +21,14 @@ func NewClientWithMockGHClient(hasNextPage bool) Client {
 	}
 	l := io.NewTestHandler()
 
-	mockHTTPClient := &mockHttpClient{}
+	mockHTTPClient := SuccessHTTPClient()
 
 	if hasNextPage {
 		return &LiveClient{
 			githubAPI: mockAPIClient{
 				OnRESTWithNext: fetcher.OnRESTSuccessWithNextPage,
 			},
-			httpClient: mockHTTPClient,
+			httpClient: &mockHTTPClient,
 			logger:     l,
 		}
 	}
@@ -37,7 +37,7 @@ func NewClientWithMockGHClient(hasNextPage bool) Client {
 		githubAPI: mockAPIClient{
 			OnRESTWithNext: fetcher.OnRESTSuccess,
 		},
-		httpClient: mockHTTPClient,
+		httpClient: &mockHTTPClient,
 		logger:     l,
 	}
 }
@@ -139,11 +139,12 @@ func TestGetByDigest_NoAttestationsFound(t *testing.T) {
 		NumAttestations: 5,
 	}
 
+	httpClient := SuccessHTTPClient()
 	c := LiveClient{
 		githubAPI: mockAPIClient{
 			OnRESTWithNext: fetcher.OnRESTWithNextNoAttestations,
 		},
-		httpClient: &mockHttpClient{},
+		httpClient: &httpClient,
 		logger:     io.NewTestHandler(),
 	}
 
@@ -193,16 +194,14 @@ func TestFetchBundlesByURL(t *testing.T) {
 	require.NoError(t, err)
 	require.Len(t, fetched, 2)
 	require.Equal(t, "application/vnd.dev.sigstore.bundle.v0.3+json", fetched[0].Bundle.GetMediaType())
-	require.Equal(t, 2, mockHTTPClient.currNumCalls)
+	require.Equal(t, 2, mockHTTPClient.TimesCalled())
 }
 
 func TestFetchBundlesByURL_Fail(t *testing.T) {
-	mockHTTPClient := &mockHttpClient{
-		failAfterNumCalls: 2,
-	}
+	mockHTTPClient := HTTPClientFailsAfterNumCalls(1)
 
 	c := &LiveClient{
-		httpClient: mockHTTPClient,
+		httpClient: &mockHTTPClient,
 		logger:     io.NewTestHandler(),
 	}
 
@@ -215,10 +214,10 @@ func TestFetchBundlesByURL_Fail(t *testing.T) {
 }
 
 func TestFetchBundleByURL(t *testing.T) {
-	mockHTTPClient := &mockHttpClient{}
+	mockHTTPClient := SuccessHTTPClient()
 
 	c := &LiveClient{
-		httpClient: mockHTTPClient,
+		httpClient: &mockHTTPClient,
 		logger:     io.NewTestHandler(),
 	}
 
@@ -226,16 +225,14 @@ func TestFetchBundleByURL(t *testing.T) {
 	bundle, err := c.fetchBundleByURL(&attestation)
 	require.NoError(t, err)
 	require.Equal(t, "application/vnd.dev.sigstore.bundle.v0.3+json", bundle.GetMediaType())
-	require.Equal(t, 1, mockHTTPClient.currNumCalls)
+	require.Equal(t, 1, mockHTTPClient.TimesCalled())
 }
 
 func TestFetchBundleByURL_FetchByURLFail(t *testing.T) {
-	mockHTTPClient := &mockHttpClient{
-		failAfterNumCalls: 1,
-	}
+	mockHTTPClient := FailHTTPClient()
 
 	c := &LiveClient{
-		httpClient: mockHTTPClient,
+		httpClient: &mockHTTPClient,
 		logger:     io.NewTestHandler(),
 	}
 
@@ -243,14 +240,14 @@ func TestFetchBundleByURL_FetchByURLFail(t *testing.T) {
 	bundle, err := c.fetchBundleByURL(&attestation)
 	require.Error(t, err)
 	require.Nil(t, bundle)
-	require.Equal(t, 1, mockHTTPClient.currNumCalls)
+	require.Equal(t, 1, mockHTTPClient.TimesCalled())
 }
 
 func TestFetchBundleByURL_FallbackToBundleField(t *testing.T) {
-	mockHTTPClient := &mockHttpClient{}
+	mockHTTPClient := SuccessHTTPClient()
 
 	c := &LiveClient{
-		httpClient: mockHTTPClient,
+		httpClient: &mockHTTPClient,
 		logger:     io.NewTestHandler(),
 	}
 
@@ -258,7 +255,7 @@ func TestFetchBundleByURL_FallbackToBundleField(t *testing.T) {
 	bundle, err := c.fetchBundleByURL(&attestation)
 	require.NoError(t, err)
 	require.Equal(t, "application/vnd.dev.sigstore.bundle.v0.3+json", bundle.GetMediaType())
-	require.Equal(t, 0, mockHTTPClient.currNumCalls)
+	require.Equal(t, 0, mockHTTPClient.TimesCalled())
 }
 
 func TestGetTrustDomain(t *testing.T) {
diff --git a/pkg/cmd/attestation/api/mock_httpClient_test.go b/pkg/cmd/attestation/api/mock_httpClient_test.go
index 3218ed508..345d2c5c2 100644
--- a/pkg/cmd/attestation/api/mock_httpClient_test.go
+++ b/pkg/cmd/attestation/api/mock_httpClient_test.go
@@ -14,6 +14,7 @@ import (
 type mockHttpClient struct {
 	mutex             sync.RWMutex
 	currNumCalls      int
+	alwaysFail        bool
 	failAfterNumCalls int
 }
 
@@ -22,7 +23,7 @@ func (m *mockHttpClient) Get(url string) (*http.Response, error) {
 	m.currNumCalls++
 	m.mutex.Unlock()
 
-	if m.failAfterNumCalls > 0 && m.currNumCalls >= m.failAfterNumCalls {
+	if m.alwaysFail || (m.failAfterNumCalls > 0 && m.currNumCalls > m.failAfterNumCalls) {
 		return &http.Response{
 			StatusCode: 500,
 		}, fmt.Errorf("failed to fetch with %s", url)
@@ -35,3 +36,23 @@ func (m *mockHttpClient) Get(url string) (*http.Response, error) {
 		Body:       io.NopCloser(bytes.NewReader(compressed)),
 	}, nil
 }
+
+func (m *mockHttpClient) TimesCalled() int {
+	return m.currNumCalls
+}
+
+func FailHTTPClient() mockHttpClient {
+	return mockHttpClient{
+		alwaysFail: true,
+	}
+}
+
+func SuccessHTTPClient() mockHttpClient {
+	return mockHttpClient{}
+}
+
+func HTTPClientFailsAfterNumCalls(numCalls int) mockHttpClient {
+	return mockHttpClient{
+		failAfterNumCalls: numCalls,
+	}
+}

From ecf55c6c16f50279e98526b2a953630832e77a31 Mon Sep 17 00:00:00 2001
From: Meredith Lancaster <malancas@github.com>
Date: Tue, 7 Jan 2025 10:54:17 -0700
Subject: [PATCH 18/30] use mock to assert number of http calls

Signed-off-by: Meredith Lancaster <malancas@github.com>
---
 pkg/cmd/attestation/api/client_test.go          |  8 ++++----
 pkg/cmd/attestation/api/mock_httpClient_test.go | 11 +++++++----
 2 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/pkg/cmd/attestation/api/client_test.go b/pkg/cmd/attestation/api/client_test.go
index 23dcb77d9..3572689f8 100644
--- a/pkg/cmd/attestation/api/client_test.go
+++ b/pkg/cmd/attestation/api/client_test.go
@@ -194,7 +194,7 @@ func TestFetchBundlesByURL(t *testing.T) {
 	require.NoError(t, err)
 	require.Len(t, fetched, 2)
 	require.Equal(t, "application/vnd.dev.sigstore.bundle.v0.3+json", fetched[0].Bundle.GetMediaType())
-	require.Equal(t, 2, mockHTTPClient.TimesCalled())
+	mockHTTPClient.AssertNumberOfCalls(t, "OnGet", 2)
 }
 
 func TestFetchBundlesByURL_Fail(t *testing.T) {
@@ -225,7 +225,7 @@ func TestFetchBundleByURL(t *testing.T) {
 	bundle, err := c.fetchBundleByURL(&attestation)
 	require.NoError(t, err)
 	require.Equal(t, "application/vnd.dev.sigstore.bundle.v0.3+json", bundle.GetMediaType())
-	require.Equal(t, 1, mockHTTPClient.TimesCalled())
+	mockHTTPClient.AssertNumberOfCalls(t, "OnGet", 1)
 }
 
 func TestFetchBundleByURL_FetchByURLFail(t *testing.T) {
@@ -240,7 +240,7 @@ func TestFetchBundleByURL_FetchByURLFail(t *testing.T) {
 	bundle, err := c.fetchBundleByURL(&attestation)
 	require.Error(t, err)
 	require.Nil(t, bundle)
-	require.Equal(t, 1, mockHTTPClient.TimesCalled())
+	mockHTTPClient.AssertNumberOfCalls(t, "OnGet", 1)
 }
 
 func TestFetchBundleByURL_FallbackToBundleField(t *testing.T) {
@@ -255,7 +255,7 @@ func TestFetchBundleByURL_FallbackToBundleField(t *testing.T) {
 	bundle, err := c.fetchBundleByURL(&attestation)
 	require.NoError(t, err)
 	require.Equal(t, "application/vnd.dev.sigstore.bundle.v0.3+json", bundle.GetMediaType())
-	require.Equal(t, 0, mockHTTPClient.TimesCalled())
+	mockHTTPClient.AssertNotCalled(t, "OnGet")
 }
 
 func TestGetTrustDomain(t *testing.T) {
diff --git a/pkg/cmd/attestation/api/mock_httpClient_test.go b/pkg/cmd/attestation/api/mock_httpClient_test.go
index 345d2c5c2..dd0c139d5 100644
--- a/pkg/cmd/attestation/api/mock_httpClient_test.go
+++ b/pkg/cmd/attestation/api/mock_httpClient_test.go
@@ -9,26 +9,33 @@ import (
 
 	"github.com/cli/cli/v2/pkg/cmd/attestation/test/data"
 	"github.com/golang/snappy"
+	"github.com/stretchr/testify/mock"
 )
 
 type mockHttpClient struct {
+	mock.Mock
 	mutex             sync.RWMutex
 	currNumCalls      int
 	alwaysFail        bool
 	failAfterNumCalls int
+	OnGet             func(url string) (*http.Response, error)
 }
 
 func (m *mockHttpClient) Get(url string) (*http.Response, error) {
+	m.On("OnGet").Return()
 	m.mutex.Lock()
 	m.currNumCalls++
 	m.mutex.Unlock()
 
 	if m.alwaysFail || (m.failAfterNumCalls > 0 && m.currNumCalls > m.failAfterNumCalls) {
+		m.MethodCalled("OnGet")
 		return &http.Response{
 			StatusCode: 500,
 		}, fmt.Errorf("failed to fetch with %s", url)
 	}
 
+	m.MethodCalled("OnGet")
+
 	var compressed []byte
 	compressed = snappy.Encode(compressed, data.SigstoreBundleRaw)
 	return &http.Response{
@@ -37,10 +44,6 @@ func (m *mockHttpClient) Get(url string) (*http.Response, error) {
 	}, nil
 }
 
-func (m *mockHttpClient) TimesCalled() int {
-	return m.currNumCalls
-}
-
 func FailHTTPClient() mockHttpClient {
 	return mockHttpClient{
 		alwaysFail: true,

From 9d88ca8cf8e14b86275f4c5612fae576ac597572 Mon Sep 17 00:00:00 2001
From: Meredith Lancaster <malancas@github.com>
Date: Tue, 7 Jan 2025 11:32:49 -0700
Subject: [PATCH 19/30] simplify mock http client

Signed-off-by: Meredith Lancaster <malancas@github.com>
---
 pkg/cmd/attestation/api/client_test.go        | 15 +++--
 .../attestation/api/mock_httpClient_test.go   | 67 ++++++++++++-------
 2 files changed, 49 insertions(+), 33 deletions(-)

diff --git a/pkg/cmd/attestation/api/client_test.go b/pkg/cmd/attestation/api/client_test.go
index 3572689f8..feba1a942 100644
--- a/pkg/cmd/attestation/api/client_test.go
+++ b/pkg/cmd/attestation/api/client_test.go
@@ -181,9 +181,9 @@ func TestGetByDigest_Error(t *testing.T) {
 }
 
 func TestFetchBundlesByURL(t *testing.T) {
-	mockHTTPClient := &mockHttpClient{}
+	mockHTTPClient := SuccessHTTPClient()
 	client := LiveClient{
-		httpClient: mockHTTPClient,
+		httpClient: &mockHTTPClient,
 		logger:     io.NewTestHandler(),
 	}
 
@@ -194,11 +194,11 @@ func TestFetchBundlesByURL(t *testing.T) {
 	require.NoError(t, err)
 	require.Len(t, fetched, 2)
 	require.Equal(t, "application/vnd.dev.sigstore.bundle.v0.3+json", fetched[0].Bundle.GetMediaType())
-	mockHTTPClient.AssertNumberOfCalls(t, "OnGet", 2)
+	mockHTTPClient.AssertNumberOfCalls(t, "OnGetSuccess", 2)
 }
 
 func TestFetchBundlesByURL_Fail(t *testing.T) {
-	mockHTTPClient := HTTPClientFailsAfterNumCalls(1)
+	mockHTTPClient := FailsAfterNumCallsHTTPClient(1)
 
 	c := &LiveClient{
 		httpClient: &mockHTTPClient,
@@ -211,6 +211,7 @@ func TestFetchBundlesByURL_Fail(t *testing.T) {
 	fetched, err := c.fetchBundlesByURL(attestations)
 	require.Error(t, err)
 	require.Nil(t, fetched)
+	mockHTTPClient.AssertNumberOfCalls(t, "OnGetFailAfterOneCall", 2)
 }
 
 func TestFetchBundleByURL(t *testing.T) {
@@ -225,7 +226,7 @@ func TestFetchBundleByURL(t *testing.T) {
 	bundle, err := c.fetchBundleByURL(&attestation)
 	require.NoError(t, err)
 	require.Equal(t, "application/vnd.dev.sigstore.bundle.v0.3+json", bundle.GetMediaType())
-	mockHTTPClient.AssertNumberOfCalls(t, "OnGet", 1)
+	mockHTTPClient.AssertNumberOfCalls(t, "OnGetSuccess", 1)
 }
 
 func TestFetchBundleByURL_FetchByURLFail(t *testing.T) {
@@ -240,7 +241,7 @@ func TestFetchBundleByURL_FetchByURLFail(t *testing.T) {
 	bundle, err := c.fetchBundleByURL(&attestation)
 	require.Error(t, err)
 	require.Nil(t, bundle)
-	mockHTTPClient.AssertNumberOfCalls(t, "OnGet", 1)
+	mockHTTPClient.AssertNumberOfCalls(t, "OnGetFail", 1)
 }
 
 func TestFetchBundleByURL_FallbackToBundleField(t *testing.T) {
@@ -255,7 +256,7 @@ func TestFetchBundleByURL_FallbackToBundleField(t *testing.T) {
 	bundle, err := c.fetchBundleByURL(&attestation)
 	require.NoError(t, err)
 	require.Equal(t, "application/vnd.dev.sigstore.bundle.v0.3+json", bundle.GetMediaType())
-	mockHTTPClient.AssertNotCalled(t, "OnGet")
+	mockHTTPClient.AssertNotCalled(t, "OnGetSuccess")
 }
 
 func TestGetTrustDomain(t *testing.T) {
diff --git a/pkg/cmd/attestation/api/mock_httpClient_test.go b/pkg/cmd/attestation/api/mock_httpClient_test.go
index dd0c139d5..8cbc91755 100644
--- a/pkg/cmd/attestation/api/mock_httpClient_test.go
+++ b/pkg/cmd/attestation/api/mock_httpClient_test.go
@@ -5,7 +5,6 @@ import (
 	"fmt"
 	"io"
 	"net/http"
-	"sync"
 
 	"github.com/cli/cli/v2/pkg/cmd/attestation/test/data"
 	"github.com/golang/snappy"
@@ -14,27 +13,11 @@ import (
 
 type mockHttpClient struct {
 	mock.Mock
-	mutex             sync.RWMutex
-	currNumCalls      int
-	alwaysFail        bool
-	failAfterNumCalls int
-	OnGet             func(url string) (*http.Response, error)
 }
 
 func (m *mockHttpClient) Get(url string) (*http.Response, error) {
-	m.On("OnGet").Return()
-	m.mutex.Lock()
-	m.currNumCalls++
-	m.mutex.Unlock()
-
-	if m.alwaysFail || (m.failAfterNumCalls > 0 && m.currNumCalls > m.failAfterNumCalls) {
-		m.MethodCalled("OnGet")
-		return &http.Response{
-			StatusCode: 500,
-		}, fmt.Errorf("failed to fetch with %s", url)
-	}
-
-	m.MethodCalled("OnGet")
+	m.On("OnGetSuccess").Return()
+	m.MethodCalled("OnGetSuccess")
 
 	var compressed []byte
 	compressed = snappy.Encode(compressed, data.SigstoreBundleRaw)
@@ -44,18 +27,50 @@ func (m *mockHttpClient) Get(url string) (*http.Response, error) {
 	}, nil
 }
 
-func FailHTTPClient() mockHttpClient {
-	return mockHttpClient{
-		alwaysFail: true,
+type failHttpClient struct {
+	mock.Mock
+}
+
+func (m *failHttpClient) Get(url string) (*http.Response, error) {
+	m.On("OnGetFail").Return()
+	m.MethodCalled("OnGetFail")
+
+	return &http.Response{
+		StatusCode: 500,
+	}, fmt.Errorf("failed to fetch with %s", url)
+}
+
+type failAfterOneCallHttpClient struct {
+	mock.Mock
+}
+
+func (m *failAfterOneCallHttpClient) Get(url string) (*http.Response, error) {
+	m.On("OnGetFailAfterOneCall").Return()
+
+	if len(m.Calls) >= 1 {
+		m.MethodCalled("OnGetFailAfterOneCall")
+		return &http.Response{
+			StatusCode: 500,
+		}, fmt.Errorf("failed to fetch with %s", url)
 	}
+
+	m.MethodCalled("OnGetFailAfterOneCall")
+	var compressed []byte
+	compressed = snappy.Encode(compressed, data.SigstoreBundleRaw)
+	return &http.Response{
+		StatusCode: 200,
+		Body:       io.NopCloser(bytes.NewReader(compressed)),
+	}, nil
+}
+
+func FailHTTPClient() failHttpClient {
+	return failHttpClient{}
 }
 
 func SuccessHTTPClient() mockHttpClient {
 	return mockHttpClient{}
 }
 
-func HTTPClientFailsAfterNumCalls(numCalls int) mockHttpClient {
-	return mockHttpClient{
-		failAfterNumCalls: numCalls,
-	}
+func FailsAfterNumCallsHTTPClient(numCalls int) failAfterOneCallHttpClient {
+	return failAfterOneCallHttpClient{}
 }

From 7838e912b657f71371b96326790c7d5a0a84ba39 Mon Sep 17 00:00:00 2001
From: Meredith Lancaster <malancas@github.com>
Date: Tue, 7 Jan 2025 11:37:02 -0700
Subject: [PATCH 20/30] more mock http client cleanup

Signed-off-by: Meredith Lancaster <malancas@github.com>
---
 pkg/cmd/attestation/api/client_test.go        | 36 +++++++++----------
 .../attestation/api/mock_httpClient_test.go   | 12 -------
 2 files changed, 18 insertions(+), 30 deletions(-)

diff --git a/pkg/cmd/attestation/api/client_test.go b/pkg/cmd/attestation/api/client_test.go
index feba1a942..9b8dabc00 100644
--- a/pkg/cmd/attestation/api/client_test.go
+++ b/pkg/cmd/attestation/api/client_test.go
@@ -21,14 +21,14 @@ func NewClientWithMockGHClient(hasNextPage bool) Client {
 	}
 	l := io.NewTestHandler()
 
-	mockHTTPClient := SuccessHTTPClient()
+	httpClient := &mockHttpClient{}
 
 	if hasNextPage {
 		return &LiveClient{
 			githubAPI: mockAPIClient{
 				OnRESTWithNext: fetcher.OnRESTSuccessWithNextPage,
 			},
-			httpClient: &mockHTTPClient,
+			httpClient: httpClient,
 			logger:     l,
 		}
 	}
@@ -37,7 +37,7 @@ func NewClientWithMockGHClient(hasNextPage bool) Client {
 		githubAPI: mockAPIClient{
 			OnRESTWithNext: fetcher.OnRESTSuccess,
 		},
-		httpClient: &mockHTTPClient,
+		httpClient: httpClient,
 		logger:     l,
 	}
 }
@@ -139,12 +139,12 @@ func TestGetByDigest_NoAttestationsFound(t *testing.T) {
 		NumAttestations: 5,
 	}
 
-	httpClient := SuccessHTTPClient()
+	httpClient := &mockHttpClient{}
 	c := LiveClient{
 		githubAPI: mockAPIClient{
 			OnRESTWithNext: fetcher.OnRESTWithNextNoAttestations,
 		},
-		httpClient: &httpClient,
+		httpClient: httpClient,
 		logger:     io.NewTestHandler(),
 	}
 
@@ -181,9 +181,9 @@ func TestGetByDigest_Error(t *testing.T) {
 }
 
 func TestFetchBundlesByURL(t *testing.T) {
-	mockHTTPClient := SuccessHTTPClient()
+	httpClient := &mockHttpClient{}
 	client := LiveClient{
-		httpClient: &mockHTTPClient,
+		httpClient: httpClient,
 		logger:     io.NewTestHandler(),
 	}
 
@@ -194,14 +194,14 @@ func TestFetchBundlesByURL(t *testing.T) {
 	require.NoError(t, err)
 	require.Len(t, fetched, 2)
 	require.Equal(t, "application/vnd.dev.sigstore.bundle.v0.3+json", fetched[0].Bundle.GetMediaType())
-	mockHTTPClient.AssertNumberOfCalls(t, "OnGetSuccess", 2)
+	httpClient.AssertNumberOfCalls(t, "OnGetSuccess", 2)
 }
 
 func TestFetchBundlesByURL_Fail(t *testing.T) {
-	mockHTTPClient := FailsAfterNumCallsHTTPClient(1)
+	httpClient := &failAfterOneCallHttpClient{}
 
 	c := &LiveClient{
-		httpClient: &mockHTTPClient,
+		httpClient: httpClient,
 		logger:     io.NewTestHandler(),
 	}
 
@@ -211,14 +211,14 @@ func TestFetchBundlesByURL_Fail(t *testing.T) {
 	fetched, err := c.fetchBundlesByURL(attestations)
 	require.Error(t, err)
 	require.Nil(t, fetched)
-	mockHTTPClient.AssertNumberOfCalls(t, "OnGetFailAfterOneCall", 2)
+	httpClient.AssertNumberOfCalls(t, "OnGetFailAfterOneCall", 2)
 }
 
 func TestFetchBundleByURL(t *testing.T) {
-	mockHTTPClient := SuccessHTTPClient()
+	httpClient := &mockHttpClient{}
 
 	c := &LiveClient{
-		httpClient: &mockHTTPClient,
+		httpClient: httpClient,
 		logger:     io.NewTestHandler(),
 	}
 
@@ -226,14 +226,14 @@ func TestFetchBundleByURL(t *testing.T) {
 	bundle, err := c.fetchBundleByURL(&attestation)
 	require.NoError(t, err)
 	require.Equal(t, "application/vnd.dev.sigstore.bundle.v0.3+json", bundle.GetMediaType())
-	mockHTTPClient.AssertNumberOfCalls(t, "OnGetSuccess", 1)
+	httpClient.AssertNumberOfCalls(t, "OnGetSuccess", 1)
 }
 
 func TestFetchBundleByURL_FetchByURLFail(t *testing.T) {
-	mockHTTPClient := FailHTTPClient()
+	mockHTTPClient := &failHttpClient{}
 
 	c := &LiveClient{
-		httpClient: &mockHTTPClient,
+		httpClient: mockHTTPClient,
 		logger:     io.NewTestHandler(),
 	}
 
@@ -245,10 +245,10 @@ func TestFetchBundleByURL_FetchByURLFail(t *testing.T) {
 }
 
 func TestFetchBundleByURL_FallbackToBundleField(t *testing.T) {
-	mockHTTPClient := SuccessHTTPClient()
+	mockHTTPClient := &mockHttpClient{}
 
 	c := &LiveClient{
-		httpClient: &mockHTTPClient,
+		httpClient: mockHTTPClient,
 		logger:     io.NewTestHandler(),
 	}
 
diff --git a/pkg/cmd/attestation/api/mock_httpClient_test.go b/pkg/cmd/attestation/api/mock_httpClient_test.go
index 8cbc91755..4dc34fbf8 100644
--- a/pkg/cmd/attestation/api/mock_httpClient_test.go
+++ b/pkg/cmd/attestation/api/mock_httpClient_test.go
@@ -62,15 +62,3 @@ func (m *failAfterOneCallHttpClient) Get(url string) (*http.Response, error) {
 		Body:       io.NopCloser(bytes.NewReader(compressed)),
 	}, nil
 }
-
-func FailHTTPClient() failHttpClient {
-	return failHttpClient{}
-}
-
-func SuccessHTTPClient() mockHttpClient {
-	return mockHttpClient{}
-}
-
-func FailsAfterNumCallsHTTPClient(numCalls int) failAfterOneCallHttpClient {
-	return failAfterOneCallHttpClient{}
-}

From 3c0280c4d7d1621412093f78e6870b0129fbfb4c Mon Sep 17 00:00:00 2001
From: Meredith Lancaster <malancas@github.com>
Date: Tue, 7 Jan 2025 11:51:23 -0700
Subject: [PATCH 21/30] undo name change for now

Signed-off-by: Meredith Lancaster <malancas@github.com>
---
 pkg/cmd/attestation/verification/attestation.go | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/pkg/cmd/attestation/verification/attestation.go b/pkg/cmd/attestation/verification/attestation.go
index 8022a4943..5bca39700 100644
--- a/pkg/cmd/attestation/verification/attestation.go
+++ b/pkg/cmd/attestation/verification/attestation.go
@@ -102,19 +102,19 @@ func GetRemoteAttestations(client api.Client, params FetchRemoteAttestationsPara
 	// check if Repo is set first because if Repo has been set, Owner will be set using the value of Repo.
 	// If Repo is not set, the field will remain empty. It will not be populated using the value of Owner.
 	if params.Repo != "" {
-		bundles, err := client.GetByRepoAndDigest(params.Repo, params.Digest, params.Limit)
+		attestations, err := client.GetByRepoAndDigest(params.Repo, params.Digest, params.Limit)
 		if err != nil {
 			return nil, fmt.Errorf("failed to fetch attestations from %s: %w", params.Repo, err)
 		}
 
-		return bundles, nil
+		return attestations, nil
 	} else if params.Owner != "" {
-		bundles, err := client.GetByOwnerAndDigest(params.Owner, params.Digest, params.Limit)
+		attestations, err := client.GetByOwnerAndDigest(params.Owner, params.Digest, params.Limit)
 		if err != nil {
 			return nil, fmt.Errorf("failed to fetch attestations from %s: %w", params.Owner, err)
 		}
 
-		return bundles, nil
+		return attestations, nil
 	}
 	return nil, fmt.Errorf("owner or repo must be provided")
 }

From 37e0969f20cc6881ab7ccf3b4c675d13ea3a5dbd Mon Sep 17 00:00:00 2001
From: Meredith Lancaster <malancas@github.com>
Date: Tue, 7 Jan 2025 11:51:51 -0700
Subject: [PATCH 22/30] remove spaces

Signed-off-by: Meredith Lancaster <malancas@github.com>
---
 pkg/cmd/attestation/verification/attestation.go | 2 --
 1 file changed, 2 deletions(-)

diff --git a/pkg/cmd/attestation/verification/attestation.go b/pkg/cmd/attestation/verification/attestation.go
index 5bca39700..fdfd667bd 100644
--- a/pkg/cmd/attestation/verification/attestation.go
+++ b/pkg/cmd/attestation/verification/attestation.go
@@ -106,14 +106,12 @@ func GetRemoteAttestations(client api.Client, params FetchRemoteAttestationsPara
 		if err != nil {
 			return nil, fmt.Errorf("failed to fetch attestations from %s: %w", params.Repo, err)
 		}
-
 		return attestations, nil
 	} else if params.Owner != "" {
 		attestations, err := client.GetByOwnerAndDigest(params.Owner, params.Digest, params.Limit)
 		if err != nil {
 			return nil, fmt.Errorf("failed to fetch attestations from %s: %w", params.Owner, err)
 		}
-
 		return attestations, nil
 	}
 	return nil, fmt.Errorf("owner or repo must be provided")

From 0a602fae073326f882834987492d7433c88a7f36 Mon Sep 17 00:00:00 2001
From: Meredith Lancaster <malancas@github.com>
Date: Tue, 7 Jan 2025 11:54:02 -0700
Subject: [PATCH 23/30] undo other name change

Signed-off-by: Meredith Lancaster <malancas@github.com>
---
 pkg/cmd/attestation/api/client.go      | 4 ++--
 pkg/cmd/attestation/api/client_test.go | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/pkg/cmd/attestation/api/client.go b/pkg/cmd/attestation/api/client.go
index d0ef3f21b..dc3ab2cb4 100644
--- a/pkg/cmd/attestation/api/client.go
+++ b/pkg/cmd/attestation/api/client.go
@@ -60,14 +60,14 @@ func NewLiveClient(hc *http.Client, host string, l *ioconfig.Handler) *LiveClien
 	}
 }
 
-func (c *LiveClient) buildRepoAndDigestURL(repo, digest string) string {
+func (c *LiveClient) BuildRepoAndDigestURL(repo, digest string) string {
 	repo = strings.Trim(repo, "/")
 	return fmt.Sprintf(GetAttestationByRepoAndSubjectDigestPath, repo, digest)
 }
 
 // GetByRepoAndDigest fetches the attestation by repo and digest
 func (c *LiveClient) GetByRepoAndDigest(repo, digest string, limit int) ([]*Attestation, error) {
-	url := c.buildRepoAndDigestURL(repo, digest)
+	url := c.BuildRepoAndDigestURL(repo, digest)
 	attestations, err := c.getAttestations(url, repo, digest, limit)
 	if err != nil {
 		return nil, err
diff --git a/pkg/cmd/attestation/api/client_test.go b/pkg/cmd/attestation/api/client_test.go
index 9b8dabc00..7a01809cf 100644
--- a/pkg/cmd/attestation/api/client_test.go
+++ b/pkg/cmd/attestation/api/client_test.go
@@ -55,7 +55,7 @@ func TestGetURL(t *testing.T) {
 	}
 
 	for _, data := range testData {
-		s := c.buildRepoAndDigestURL(data.repo, data.digest)
+		s := c.BuildRepoAndDigestURL(data.repo, data.digest)
 		require.Equal(t, data.expected, s)
 	}
 }

From 258c69cd266e725eb26de266281aa737241d4fbc Mon Sep 17 00:00:00 2001
From: Meredith Lancaster <malancas@github.com>
Date: Tue, 7 Jan 2025 11:56:05 -0700
Subject: [PATCH 24/30] undo more name chanages

Signed-off-by: Meredith Lancaster <malancas@github.com>
---
 pkg/cmd/attestation/api/client.go | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/pkg/cmd/attestation/api/client.go b/pkg/cmd/attestation/api/client.go
index dc3ab2cb4..02b4d6758 100644
--- a/pkg/cmd/attestation/api/client.go
+++ b/pkg/cmd/attestation/api/client.go
@@ -81,14 +81,14 @@ func (c *LiveClient) GetByRepoAndDigest(repo, digest string, limit int) ([]*Atte
 	return bundles, nil
 }
 
-func (c *LiveClient) buildOwnerAndDigestURL(owner, digest string) string {
+func (c *LiveClient) BuildOwnerAndDigestURL(owner, digest string) string {
 	owner = strings.Trim(owner, "/")
 	return fmt.Sprintf(GetAttestationByOwnerAndSubjectDigestPath, owner, digest)
 }
 
 // GetByOwnerAndDigest fetches attestation by owner and digest
 func (c *LiveClient) GetByOwnerAndDigest(owner, digest string, limit int) ([]*Attestation, error) {
-	url := c.buildOwnerAndDigestURL(owner, digest)
+	url := c.BuildOwnerAndDigestURL(owner, digest)
 	attestations, err := c.getAttestations(url, owner, digest, limit)
 	if err != nil {
 		return nil, err

From f46cccbab4ecfdcfd9dd61743f2b53d19cf375ea Mon Sep 17 00:00:00 2001
From: Meredith Lancaster <malancas@github.com>
Date: Tue, 7 Jan 2025 12:03:49 -0700
Subject: [PATCH 25/30] comment

Signed-off-by: Meredith Lancaster <malancas@github.com>
---
 pkg/cmd/attestation/api/client.go | 1 +
 1 file changed, 1 insertion(+)

diff --git a/pkg/cmd/attestation/api/client.go b/pkg/cmd/attestation/api/client.go
index 02b4d6758..20df51165 100644
--- a/pkg/cmd/attestation/api/client.go
+++ b/pkg/cmd/attestation/api/client.go
@@ -191,6 +191,7 @@ func (c *LiveClient) fetchBundlesByURL(attestations []*Attestation) ([]*Attestat
 }
 
 func (c *LiveClient) fetchBundleByURL(a *Attestation) (*bundle.Bundle, error) {
+	// for now, we fallback to the bundle field if the bundle URL is empty
 	if a.BundleURL == "" {
 		c.logger.VerbosePrintf("Bundle URL is empty. Falling back to bundle field\n\n")
 		return a.Bundle, nil

From 42cb2547cdcb8bf089bbdd055f7a0ac8114cc37f Mon Sep 17 00:00:00 2001
From: Meredith Lancaster <malancas@github.com>
Date: Tue, 7 Jan 2025 14:13:50 -0700
Subject: [PATCH 26/30] remove old comment

Signed-off-by: Meredith Lancaster <malancas@github.com>
---
 pkg/cmd/attestation/api/client.go | 1 -
 1 file changed, 1 deletion(-)

diff --git a/pkg/cmd/attestation/api/client.go b/pkg/cmd/attestation/api/client.go
index 20df51165..9c5979d46 100644
--- a/pkg/cmd/attestation/api/client.go
+++ b/pkg/cmd/attestation/api/client.go
@@ -209,7 +209,6 @@ func (c *LiveClient) fetchBundleByURL(a *Attestation) (*bundle.Bundle, error) {
 		return nil, fmt.Errorf("unexpected status code: %d", resp.StatusCode)
 	}
 
-	// parse the URL to get the host and path
 	body, err := io.ReadAll(resp.Body)
 	if err != nil {
 		return nil, fmt.Errorf("failed to read blob storage response body: %w", err)

From 51a74aed1daa4cff4452d847a79a4842ce17ec86 Mon Sep 17 00:00:00 2001
From: Meredith Lancaster <malancas@users.noreply.github.com>
Date: Tue, 7 Jan 2025 15:14:23 -0700
Subject: [PATCH 27/30] Update pkg/cmd/attestation/api/client.go

Co-authored-by: Phill MV <phillmv@github.com>
---
 pkg/cmd/attestation/api/client.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/pkg/cmd/attestation/api/client.go b/pkg/cmd/attestation/api/client.go
index 9c5979d46..ca6c0f3d5 100644
--- a/pkg/cmd/attestation/api/client.go
+++ b/pkg/cmd/attestation/api/client.go
@@ -172,7 +172,7 @@ func (c *LiveClient) fetchBundlesByURL(attestations []*Attestation) ([]*Attestat
 	g := errgroup.Group{}
 	for i, a := range attestations {
 		g.Go(func() error {
-			b, err := c.fetchBundleByURL(a)
+			b, err := c.GetBundle(url)
 			if err != nil {
 				return fmt.Errorf("failed to fetch bundle with URL: %w", err)
 			}

From 8d89dd97fdb487f8ad64d386de1d25d6ff3d36c8 Mon Sep 17 00:00:00 2001
From: Meredith Lancaster <malancas@users.noreply.github.com>
Date: Tue, 7 Jan 2025 15:14:53 -0700
Subject: [PATCH 28/30] Update pkg/cmd/attestation/api/client.go

Co-authored-by: Phill MV <phillmv@github.com>
---
 pkg/cmd/attestation/api/client.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/pkg/cmd/attestation/api/client.go b/pkg/cmd/attestation/api/client.go
index ca6c0f3d5..bf49756be 100644
--- a/pkg/cmd/attestation/api/client.go
+++ b/pkg/cmd/attestation/api/client.go
@@ -167,7 +167,7 @@ func (c *LiveClient) getAttestations(url, name, digest string, limit int) ([]*At
 	return attestations, nil
 }
 
-func (c *LiveClient) fetchBundlesByURL(attestations []*Attestation) ([]*Attestation, error) {
+func (c *LiveClient) fetchBundleFromAttestations(attestations []*Attestation) ([]*Attestation, error) {
 	fetched := make([]*Attestation, len(attestations))
 	g := errgroup.Group{}
 	for i, a := range attestations {

From 33d0002d214c4a94b063ba3cad36aa6f0ec24aaf Mon Sep 17 00:00:00 2001
From: Meredith Lancaster <malancas@github.com>
Date: Tue, 7 Jan 2025 15:22:02 -0700
Subject: [PATCH 29/30] update tests to use new function name

Signed-off-by: Meredith Lancaster <malancas@github.com>
---
 pkg/cmd/attestation/api/client.go      | 25 +++++++++--------
 pkg/cmd/attestation/api/client_test.go | 37 +++++++++-----------------
 2 files changed, 26 insertions(+), 36 deletions(-)

diff --git a/pkg/cmd/attestation/api/client.go b/pkg/cmd/attestation/api/client.go
index bf49756be..f8d0509ee 100644
--- a/pkg/cmd/attestation/api/client.go
+++ b/pkg/cmd/attestation/api/client.go
@@ -73,7 +73,7 @@ func (c *LiveClient) GetByRepoAndDigest(repo, digest string, limit int) ([]*Atte
 		return nil, err
 	}
 
-	bundles, err := c.fetchBundlesByURL(attestations)
+	bundles, err := c.fetchBundleFromAttestations(attestations)
 	if err != nil {
 		return nil, fmt.Errorf("failed to fetch bundle with URL: %w", err)
 	}
@@ -98,7 +98,7 @@ func (c *LiveClient) GetByOwnerAndDigest(owner, digest string, limit int) ([]*At
 		return nil, newErrNoAttestations(owner, digest)
 	}
 
-	bundles, err := c.fetchBundlesByURL(attestations)
+	bundles, err := c.fetchBundleFromAttestations(attestations)
 	if err != nil {
 		return nil, fmt.Errorf("failed to fetch bundle with URL: %w", err)
 	}
@@ -172,7 +172,16 @@ func (c *LiveClient) fetchBundleFromAttestations(attestations []*Attestation) ([
 	g := errgroup.Group{}
 	for i, a := range attestations {
 		g.Go(func() error {
-			b, err := c.GetBundle(url)
+			// for now, we fallback to the bundle field if the bundle URL is empty
+			if a.BundleURL == "" {
+				c.logger.VerbosePrintf("Bundle URL is empty. Falling back to bundle field\n\n")
+				fetched[i] = &Attestation{
+					Bundle: a.Bundle,
+				}
+				return nil
+			}
+
+			b, err := c.GetBundle(a.BundleURL)
 			if err != nil {
 				return fmt.Errorf("failed to fetch bundle with URL: %w", err)
 			}
@@ -190,16 +199,10 @@ func (c *LiveClient) fetchBundleFromAttestations(attestations []*Attestation) ([
 	return fetched, nil
 }
 
-func (c *LiveClient) fetchBundleByURL(a *Attestation) (*bundle.Bundle, error) {
-	// for now, we fallback to the bundle field if the bundle URL is empty
-	if a.BundleURL == "" {
-		c.logger.VerbosePrintf("Bundle URL is empty. Falling back to bundle field\n\n")
-		return a.Bundle, nil
-	}
-
+func (c *LiveClient) GetBundle(url string) (*bundle.Bundle, error) {
 	c.logger.VerbosePrintf("Fetching attestation bundle with bundle URL\n\n")
 
-	resp, err := c.httpClient.Get(a.BundleURL)
+	resp, err := c.httpClient.Get(url)
 	if err != nil {
 		return nil, err
 	}
diff --git a/pkg/cmd/attestation/api/client_test.go b/pkg/cmd/attestation/api/client_test.go
index 7a01809cf..fcceb33d7 100644
--- a/pkg/cmd/attestation/api/client_test.go
+++ b/pkg/cmd/attestation/api/client_test.go
@@ -180,7 +180,7 @@ func TestGetByDigest_Error(t *testing.T) {
 	require.Nil(t, attestations)
 }
 
-func TestFetchBundlesByURL(t *testing.T) {
+func TestFetchBundleFromAttestations(t *testing.T) {
 	httpClient := &mockHttpClient{}
 	client := LiveClient{
 		httpClient: httpClient,
@@ -190,14 +190,14 @@ func TestFetchBundlesByURL(t *testing.T) {
 	att1 := makeTestAttestation()
 	att2 := makeTestAttestation()
 	attestations := []*Attestation{&att1, &att2}
-	fetched, err := client.fetchBundlesByURL(attestations)
+	fetched, err := client.fetchBundleFromAttestations(attestations)
 	require.NoError(t, err)
 	require.Len(t, fetched, 2)
 	require.Equal(t, "application/vnd.dev.sigstore.bundle.v0.3+json", fetched[0].Bundle.GetMediaType())
 	httpClient.AssertNumberOfCalls(t, "OnGetSuccess", 2)
 }
 
-func TestFetchBundlesByURL_Fail(t *testing.T) {
+func TestFetchBundleFromAttestations_Fail(t *testing.T) {
 	httpClient := &failAfterOneCallHttpClient{}
 
 	c := &LiveClient{
@@ -208,28 +208,13 @@ func TestFetchBundlesByURL_Fail(t *testing.T) {
 	att1 := makeTestAttestation()
 	att2 := makeTestAttestation()
 	attestations := []*Attestation{&att1, &att2}
-	fetched, err := c.fetchBundlesByURL(attestations)
+	fetched, err := c.fetchBundleFromAttestations(attestations)
 	require.Error(t, err)
 	require.Nil(t, fetched)
 	httpClient.AssertNumberOfCalls(t, "OnGetFailAfterOneCall", 2)
 }
 
-func TestFetchBundleByURL(t *testing.T) {
-	httpClient := &mockHttpClient{}
-
-	c := &LiveClient{
-		httpClient: httpClient,
-		logger:     io.NewTestHandler(),
-	}
-
-	attestation := makeTestAttestation()
-	bundle, err := c.fetchBundleByURL(&attestation)
-	require.NoError(t, err)
-	require.Equal(t, "application/vnd.dev.sigstore.bundle.v0.3+json", bundle.GetMediaType())
-	httpClient.AssertNumberOfCalls(t, "OnGetSuccess", 1)
-}
-
-func TestFetchBundleByURL_FetchByURLFail(t *testing.T) {
+func TestFetchBundleFromAttestations_FetchByURLFail(t *testing.T) {
 	mockHTTPClient := &failHttpClient{}
 
 	c := &LiveClient{
@@ -237,8 +222,9 @@ func TestFetchBundleByURL_FetchByURLFail(t *testing.T) {
 		logger:     io.NewTestHandler(),
 	}
 
-	attestation := makeTestAttestation()
-	bundle, err := c.fetchBundleByURL(&attestation)
+	a := makeTestAttestation()
+	attestations := []*Attestation{&a}
+	bundle, err := c.fetchBundleFromAttestations(attestations)
 	require.Error(t, err)
 	require.Nil(t, bundle)
 	mockHTTPClient.AssertNumberOfCalls(t, "OnGetFail", 1)
@@ -252,10 +238,11 @@ func TestFetchBundleByURL_FallbackToBundleField(t *testing.T) {
 		logger:     io.NewTestHandler(),
 	}
 
-	attestation := Attestation{Bundle: data.SigstoreBundle(t)}
-	bundle, err := c.fetchBundleByURL(&attestation)
+	a := Attestation{Bundle: data.SigstoreBundle(t)}
+	attestations := []*Attestation{&a}
+	fetched, err := c.fetchBundleFromAttestations(attestations)
 	require.NoError(t, err)
-	require.Equal(t, "application/vnd.dev.sigstore.bundle.v0.3+json", bundle.GetMediaType())
+	require.Equal(t, "application/vnd.dev.sigstore.bundle.v0.3+json", fetched[0].Bundle.GetMediaType())
 	mockHTTPClient.AssertNotCalled(t, "OnGetSuccess")
 }
 

From 8ad877b188762e7a613ddc68ce220f4e7144a856 Mon Sep 17 00:00:00 2001
From: Meredith Lancaster <malancas@github.com>
Date: Wed, 8 Jan 2025 08:38:43 -0700
Subject: [PATCH 30/30] add check for invalid attestation

Signed-off-by: Meredith Lancaster <malancas@github.com>
---
 pkg/cmd/attestation/api/client.go      |  5 +++++
 pkg/cmd/attestation/api/client_test.go | 14 ++++++++++++++
 2 files changed, 19 insertions(+)

diff --git a/pkg/cmd/attestation/api/client.go b/pkg/cmd/attestation/api/client.go
index f8d0509ee..37579b7bc 100644
--- a/pkg/cmd/attestation/api/client.go
+++ b/pkg/cmd/attestation/api/client.go
@@ -172,6 +172,10 @@ func (c *LiveClient) fetchBundleFromAttestations(attestations []*Attestation) ([
 	g := errgroup.Group{}
 	for i, a := range attestations {
 		g.Go(func() error {
+			if a.Bundle == nil && a.BundleURL == "" {
+				return fmt.Errorf("attestation has no bundle or bundle URL")
+			}
+
 			// for now, we fallback to the bundle field if the bundle URL is empty
 			if a.BundleURL == "" {
 				c.logger.VerbosePrintf("Bundle URL is empty. Falling back to bundle field\n\n")
@@ -181,6 +185,7 @@ func (c *LiveClient) fetchBundleFromAttestations(attestations []*Attestation) ([
 				return nil
 			}
 
+			// otherwise fetch the bundle with the provided URL
 			b, err := c.GetBundle(a.BundleURL)
 			if err != nil {
 				return fmt.Errorf("failed to fetch bundle with URL: %w", err)
diff --git a/pkg/cmd/attestation/api/client_test.go b/pkg/cmd/attestation/api/client_test.go
index fcceb33d7..65f8d59ca 100644
--- a/pkg/cmd/attestation/api/client_test.go
+++ b/pkg/cmd/attestation/api/client_test.go
@@ -197,6 +197,20 @@ func TestFetchBundleFromAttestations(t *testing.T) {
 	httpClient.AssertNumberOfCalls(t, "OnGetSuccess", 2)
 }
 
+func TestFetchBundleFromAttestations_InvalidAttestation(t *testing.T) {
+	httpClient := &mockHttpClient{}
+	client := LiveClient{
+		httpClient: httpClient,
+		logger:     io.NewTestHandler(),
+	}
+
+	att1 := Attestation{}
+	attestations := []*Attestation{&att1}
+	fetched, err := client.fetchBundleFromAttestations(attestations)
+	require.Error(t, err)
+	require.Nil(t, fetched, 2)
+}
+
 func TestFetchBundleFromAttestations_Fail(t *testing.T) {
 	httpClient := &failAfterOneCallHttpClient{}