create single fetch by digest client method

Signed-off-by: Meredith Lancaster <malancas@github.com>
This commit is contained in:
Meredith Lancaster 2025-03-24 18:28:27 -06:00
parent 5a895b9d72
commit a9cc7b481e
7 changed files with 58 additions and 86 deletions

View file

@ -48,8 +48,7 @@ type httpClient interface {
}
type Client interface {
GetByRepoAndDigest(params FetchParams) ([]*Attestation, error)
GetByOwnerAndDigest(params FetchParams) ([]*Attestation, error)
GetByDigest(params FetchParams) ([]*Attestation, error)
GetTrustDomain() (string, error)
}
@ -69,16 +68,28 @@ func NewLiveClient(hc *http.Client, host string, l *ioconfig.Handler) *LiveClien
}
}
// GetByRepoAndDigest fetches the attestation by repo and digest
func (c *LiveClient) GetByRepoAndDigest(params FetchParams) ([]*Attestation, error) {
url := fmt.Sprintf(GetAttestationByRepoAndSubjectDigestPath, params.Repo, params.Digest)
return c.getByURL(url, params)
}
// GetByOwnerAndDigest fetches attestation by owner and digest
func (c *LiveClient) GetByOwnerAndDigest(params FetchParams) ([]*Attestation, error) {
url := fmt.Sprintf(GetAttestationByOwnerAndSubjectDigestPath, params.Owner, params.Digest)
return c.getByURL(url, params)
// GetByDigest fetches the attestation by digest and either owner or repo
// depending on which is provided
func (c *LiveClient) GetByDigest(params FetchParams) ([]*Attestation, error) {
if params.Repo == "" && params.Owner == "" {
return nil, fmt.Errorf("owner or repo must be provided")
} else if params.Repo != "" {
// 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.
url := fmt.Sprintf(GetAttestationByRepoAndSubjectDigestPath, params.Repo, params.Digest)
attestations, err := c.getByURL(url, params)
if err != nil {
return nil, fmt.Errorf("failed to fetch attestations from %s: %w", params.Repo, err)
}
return attestations, nil
} else {
url := fmt.Sprintf(GetAttestationByOwnerAndSubjectDigestPath, params.Owner, params.Digest)
attestations, err := c.getByURL(url, params)
if err != nil {
return nil, fmt.Errorf("failed to fetch attestations from %s: %w", params.Owner, err)
}
return attestations, nil
}
}
func (c *LiveClient) getByURL(url string, params FetchParams) ([]*Attestation, error) {

View file

@ -51,7 +51,7 @@ var testFetchParams = FetchParams{
func TestGetByDigest(t *testing.T) {
c := NewClientWithMockGHClient(false)
testFetchParams.Repo = testRepo
attestations, err := c.GetByRepoAndDigest(testFetchParams)
attestations, err := c.GetByDigest(testFetchParams)
require.NoError(t, err)
require.Equal(t, 5, len(attestations))
@ -59,7 +59,7 @@ func TestGetByDigest(t *testing.T) {
require.Equal(t, bundle.GetMediaType(), "application/vnd.dev.sigstore.bundle.v0.3+json")
testFetchParams.Owner = testOwner
attestations, err = c.GetByOwnerAndDigest(testFetchParams)
attestations, err = c.GetByDigest(testFetchParams)
require.NoError(t, err)
require.Equal(t, 5, len(attestations))
@ -74,7 +74,7 @@ func TestGetByDigestGreaterThanLimit(t *testing.T) {
// The method should return five results when the limit is not set
testFetchParams.Limit = limit
testFetchParams.Repo = testRepo
attestations, err := c.GetByRepoAndDigest(testFetchParams)
attestations, err := c.GetByDigest(testFetchParams)
require.NoError(t, err)
require.Equal(t, 3, len(attestations))
@ -82,7 +82,7 @@ func TestGetByDigestGreaterThanLimit(t *testing.T) {
require.Equal(t, bundle.GetMediaType(), "application/vnd.dev.sigstore.bundle.v0.3+json")
testFetchParams.Owner = testOwner
attestations, err = c.GetByOwnerAndDigest(testFetchParams)
attestations, err = c.GetByDigest(testFetchParams)
require.NoError(t, err)
require.Equal(t, len(attestations), limit)
@ -94,7 +94,7 @@ func TestGetByDigestWithNextPage(t *testing.T) {
c := NewClientWithMockGHClient(true)
testFetchParams.Repo = testRepo
testFetchParams.Limit = 30
attestations, err := c.GetByRepoAndDigest(testFetchParams)
attestations, err := c.GetByDigest(testFetchParams)
require.NoError(t, err)
require.Equal(t, len(attestations), 10)
@ -102,7 +102,7 @@ func TestGetByDigestWithNextPage(t *testing.T) {
require.Equal(t, bundle.GetMediaType(), "application/vnd.dev.sigstore.bundle.v0.3+json")
testFetchParams.Owner = testOwner
attestations, err = c.GetByOwnerAndDigest(testFetchParams)
attestations, err = c.GetByDigest(testFetchParams)
require.NoError(t, err)
require.Equal(t, len(attestations), 10)
@ -117,7 +117,7 @@ func TestGetByDigestGreaterThanLimitWithNextPage(t *testing.T) {
// The method should return five results when the limit is not set
testFetchParams.Limit = limit
testFetchParams.Repo = testRepo
attestations, err := c.GetByRepoAndDigest(testFetchParams)
attestations, err := c.GetByDigest(testFetchParams)
require.NoError(t, err)
require.Equal(t, len(attestations), limit)
@ -125,7 +125,7 @@ func TestGetByDigestGreaterThanLimitWithNextPage(t *testing.T) {
require.Equal(t, bundle.GetMediaType(), "application/vnd.dev.sigstore.bundle.v0.3+json")
testFetchParams.Owner = testOwner
attestations, err = c.GetByOwnerAndDigest(testFetchParams)
attestations, err = c.GetByDigest(testFetchParams)
require.NoError(t, err)
require.Equal(t, len(attestations), limit)
@ -148,13 +148,13 @@ func TestGetByDigest_NoAttestationsFound(t *testing.T) {
}
testFetchParams.Repo = testRepo
attestations, err := c.GetByRepoAndDigest(testFetchParams)
attestations, err := c.GetByDigest(testFetchParams)
require.Error(t, err)
require.IsType(t, ErrNoAttestationsFound, err)
require.Nil(t, attestations)
testFetchParams.Owner = testOwner
attestations, err = c.GetByOwnerAndDigest(testFetchParams)
attestations, err = c.GetByDigest(testFetchParams)
require.Error(t, err)
require.IsType(t, ErrNoAttestationsFound, err)
require.Nil(t, attestations)
@ -173,12 +173,12 @@ func TestGetByDigest_Error(t *testing.T) {
}
testFetchParams.Repo = testRepo
attestations, err := c.GetByRepoAndDigest(testFetchParams)
attestations, err := c.GetByDigest(testFetchParams)
require.Error(t, err)
require.Nil(t, attestations)
testFetchParams.Owner = testOwner
attestations, err = c.GetByOwnerAndDigest(testFetchParams)
attestations, err = c.GetByDigest(testFetchParams)
require.Error(t, err)
require.Nil(t, attestations)
}
@ -385,7 +385,7 @@ func TestGetAttestationsRetries(t *testing.T) {
testFetchParams.Repo = testRepo
testFetchParams.Limit = 30
attestations, err := c.GetByRepoAndDigest(testFetchParams)
attestations, err := c.GetByDigest(testFetchParams)
require.NoError(t, err)
// assert the error path was executed; because this is a paged
@ -397,9 +397,9 @@ func TestGetAttestationsRetries(t *testing.T) {
bundle := (attestations)[0].Bundle
require.Equal(t, bundle.GetMediaType(), "application/vnd.dev.sigstore.bundle.v0.3+json")
// same test as above, but for GetByOwnerAndDigest:
// same test as above, but for GetByDigest:
testFetchParams.Owner = testOwner
attestations, err = c.GetByOwnerAndDigest(testFetchParams)
attestations, err = c.GetByDigest(testFetchParams)
require.NoError(t, err)
// because we haven't reset the mock, we have added 2 more failed requests
@ -426,7 +426,7 @@ func TestGetAttestationsMaxRetries(t *testing.T) {
}
testFetchParams.Repo = testRepo
_, err := c.GetByRepoAndDigest(testFetchParams)
_, err := c.GetByDigest(testFetchParams)
require.Error(t, err)
fetcher.AssertNumberOfCalls(t, "OnREST500Error", 4)

View file

@ -6,58 +6,44 @@ import (
"github.com/cli/cli/v2/pkg/cmd/attestation/test/data"
)
func makeTestAttestation() Attestation {
return Attestation{Bundle: data.SigstoreBundle(nil), BundleURL: "https://example.com"}
}
type MockClient struct {
OnGetByRepoAndDigest func(params FetchParams) ([]*Attestation, error)
OnGetByOwnerAndDigest func(params FetchParams) ([]*Attestation, error)
OnGetTrustDomain func() (string, error)
OnGetByDigest func(params FetchParams) ([]*Attestation, error)
OnGetTrustDomain func() (string, error)
}
func (m MockClient) GetByRepoAndDigest(params FetchParams) ([]*Attestation, error) {
return m.OnGetByRepoAndDigest(params)
}
func (m MockClient) GetByOwnerAndDigest(params FetchParams) ([]*Attestation, error) {
return m.OnGetByOwnerAndDigest(params)
func (m MockClient) GetByDigest(params FetchParams) ([]*Attestation, error) {
return m.OnGetByDigest(params)
}
func (m MockClient) GetTrustDomain() (string, error) {
return m.OnGetTrustDomain()
}
func makeTestAttestation() Attestation {
return Attestation{Bundle: data.SigstoreBundle(nil), BundleURL: "https://example.com"}
}
func OnGetByRepoAndDigestSuccess(params FetchParams) ([]*Attestation, error) {
func OnGetByDigestSuccess(params FetchParams) ([]*Attestation, error) {
att1 := makeTestAttestation()
att2 := makeTestAttestation()
return []*Attestation{&att1, &att2}, nil
}
func OnGetByRepoAndDigestFailure(params FetchParams) ([]*Attestation, error) {
return nil, fmt.Errorf("failed to fetch by repo and digest")
}
func OnGetByOwnerAndDigestSuccess(params FetchParams) ([]*Attestation, error) {
att1 := makeTestAttestation()
att2 := makeTestAttestation()
return []*Attestation{&att1, &att2}, nil
}
func OnGetByOwnerAndDigestFailure(params FetchParams) ([]*Attestation, error) {
func OnGetByDigestFailure(params FetchParams) ([]*Attestation, error) {
if params.Repo != "" {
return nil, fmt.Errorf("failed to fetch by repo and digest")
}
return nil, fmt.Errorf("failed to fetch by owner and digest")
}
func NewTestClient() *MockClient {
return &MockClient{
OnGetByRepoAndDigest: OnGetByRepoAndDigestSuccess,
OnGetByOwnerAndDigest: OnGetByOwnerAndDigestSuccess,
OnGetByDigest: OnGetByDigestSuccess,
}
}
func NewFailTestClient() *MockClient {
return &MockClient{
OnGetByRepoAndDigest: OnGetByRepoAndDigestFailure,
OnGetByOwnerAndDigest: OnGetByOwnerAndDigestFailure,
OnGetByDigest: OnGetByDigestFailure,
}
}

View file

@ -133,7 +133,7 @@ func runDownload(opts *Options) error {
Owner: opts.Owner,
Repo: opts.Repo,
}
attestations, err := verification.GetRemoteAttestations(opts.APIClient, params)
attestations, err := opts.APIClient.GetByDigest(params)
if err != nil {
if errors.Is(err, api.ErrNoAttestationsFound) {
fmt.Fprintf(opts.Logger.IO.Out, "No attestations found for %s\n", opts.ArtifactPath)

View file

@ -275,7 +275,7 @@ func TestRunDownload(t *testing.T) {
t.Run("no attestations found", func(t *testing.T) {
opts := baseOpts
opts.APIClient = api.MockClient{
OnGetByOwnerAndDigest: func(params api.FetchParams) ([]*api.Attestation, error) {
OnGetByDigest: func(params api.FetchParams) ([]*api.Attestation, error) {
return nil, api.ErrNoAttestationsFound
},
}
@ -291,7 +291,7 @@ func TestRunDownload(t *testing.T) {
t.Run("failed to fetch attestations", func(t *testing.T) {
opts := baseOpts
opts.APIClient = api.MockClient{
OnGetByOwnerAndDigest: func(params api.FetchParams) ([]*api.Attestation, error) {
OnGetByDigest: func(params api.FetchParams) ([]*api.Attestation, error) {
return nil, fmt.Errorf("failed to fetch attestations")
},
}

View file

@ -82,31 +82,6 @@ func loadBundlesFromJSONLinesFile(path string) ([]*api.Attestation, error) {
return attestations, nil
}
func GetRemoteAttestations(client api.Client, params api.FetchParams) ([]*api.Attestation, error) {
if client == nil {
return nil, fmt.Errorf("api client must be provided")
}
// 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.
var attestations []*api.Attestation
var err error
var owner string
if params.Repo != "" {
attestations, err = client.GetByRepoAndDigest(params)
owner = params.Repo
} else if params.Owner != "" {
attestations, err = client.GetByOwnerAndDigest(params)
owner = params.Owner
} else {
return nil, fmt.Errorf("owner or repo must be provided")
}
if err != nil {
return nil, fmt.Errorf("failed to fetch attestations from %s: %w", owner, err)
}
return attestations, err
}
func GetOCIAttestations(client oci.Client, artifact artifact.DigestedArtifact) ([]*api.Attestation, error) {
attestations, err := client.GetAttestations(artifact.NameRef(), artifact.DigestWithAlg())
if err != nil {

View file

@ -29,7 +29,7 @@ func getAttestations(o *Options, a artifact.DigestedArtifact) ([]*api.Attestatio
Repo: o.Repo,
}
attestations, err := verification.GetRemoteAttestations(o.APIClient, params)
attestations, err := o.APIClient.GetByDigest(params)
if err != nil {
msg := "✗ Loading attestations from GitHub API failed"
return nil, msg, err