From 63f37eb36996fbf496673052386be215b14b992d Mon Sep 17 00:00:00 2001
From: Meredith Lancaster <malancas@github.com>
Date: Mon, 18 Nov 2024 08:24:25 -0700
Subject: [PATCH] pr feedback

Signed-off-by: Meredith Lancaster <malancas@github.com>
---
 pkg/cmd/attestation/download/download.go      | 13 +++++----
 .../attestation/verification/attestation.go   | 27 +++++++++----------
 pkg/cmd/attestation/verify/attestation.go     | 13 +++++----
 3 files changed, 25 insertions(+), 28 deletions(-)

diff --git a/pkg/cmd/attestation/download/download.go b/pkg/cmd/attestation/download/download.go
index a79af5935..143912308 100644
--- a/pkg/cmd/attestation/download/download.go
+++ b/pkg/cmd/attestation/download/download.go
@@ -122,14 +122,13 @@ func runDownload(opts *Options) error {
 
 	opts.Logger.VerbosePrintf("Downloading trusted metadata for artifact %s\n\n", opts.ArtifactPath)
 
-	c := verification.FetchRemoteAttestationsParams{
-		APIClient: opts.APIClient,
-		Digest:    artifact.DigestWithAlg(),
-		Limit:     opts.Limit,
-		Owner:     opts.Owner,
-		Repo:      opts.Repo,
+	params := verification.FetchRemoteAttestationsParams{
+		Digest: artifact.DigestWithAlg(),
+		Limit:  opts.Limit,
+		Owner:  opts.Owner,
+		Repo:   opts.Repo,
 	}
-	attestations, err := verification.GetRemoteAttestations(c)
+	attestations, err := verification.GetRemoteAttestations(opts.APIClient, params)
 	if err != nil {
 		if errors.Is(err, api.ErrNoAttestations{}) {
 			fmt.Fprintf(opts.Logger.IO.Out, "No attestations found for %s\n", opts.ArtifactPath)
diff --git a/pkg/cmd/attestation/verification/attestation.go b/pkg/cmd/attestation/verification/attestation.go
index bdee3a014..07083a5c0 100644
--- a/pkg/cmd/attestation/verification/attestation.go
+++ b/pkg/cmd/attestation/verification/attestation.go
@@ -20,12 +20,11 @@ const SLSAPredicateV1 = "https://slsa.dev/provenance/v1"
 var ErrUnrecognisedBundleExtension = errors.New("bundle file extension not supported, must be json or jsonl")
 var ErrEmptyBundleFile = errors.New("provided bundle file is empty")
 
-type FetchRemoteAttestations struct {
-	APIClient api.Client
-	Digest    string
-	Limit     int
-	Owner     string
-	Repo      string
+type FetchRemoteAttestationsParams struct {
+	Digest string
+	Limit  int
+	Owner  string
+	Repo   string
 }
 
 // GetLocalAttestations returns a slice of attestations read from a local bundle file.
@@ -96,22 +95,22 @@ func loadBundlesFromJSONLinesFile(path string) ([]*api.Attestation, error) {
 	return attestations, nil
 }
 
-func GetRemoteAttestations(c FetchRemoteAttestations) ([]*api.Attestation, error) {
-	if c.APIClient == nil {
+func GetRemoteAttestations(client api.Client, params FetchRemoteAttestationsParams) ([]*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.
-	if c.Repo != "" {
-		attestations, err := c.APIClient.GetByRepoAndDigest(c.Repo, c.Digest, c.Limit)
+	if params.Repo != "" {
+		attestations, err := client.GetByRepoAndDigest(params.Repo, params.Digest, params.Limit)
 		if err != nil {
-			return nil, fmt.Errorf("failed to fetch attestations from %s: %w", c.Repo, err)
+			return nil, fmt.Errorf("failed to fetch attestations from %s: %w", params.Repo, err)
 		}
 		return attestations, nil
-	} else if c.Owner != "" {
-		attestations, err := c.APIClient.GetByOwnerAndDigest(c.Owner, c.Digest, c.Limit)
+	} 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", c.Owner, err)
+			return nil, fmt.Errorf("failed to fetch attestations from %s: %w", params.Owner, err)
 		}
 		return attestations, nil
 	}
diff --git a/pkg/cmd/attestation/verify/attestation.go b/pkg/cmd/attestation/verify/attestation.go
index c067afe9b..f3f2792c4 100644
--- a/pkg/cmd/attestation/verify/attestation.go
+++ b/pkg/cmd/attestation/verify/attestation.go
@@ -32,15 +32,14 @@ func getAttestations(o *Options, a artifact.DigestedArtifact) ([]*api.Attestatio
 		return attestations, msg, nil
 	}
 
-	c := verification.FetchRemoteAttestations{
-		APIClient: o.APIClient,
-		Digest:    a.DigestWithAlg(),
-		Limit:     o.Limit,
-		Owner:     o.Owner,
-		Repo:      o.Repo,
+	params := verification.FetchRemoteAttestationsParams{
+		Digest: a.DigestWithAlg(),
+		Limit:  o.Limit,
+		Owner:  o.Owner,
+		Repo:   o.Repo,
 	}
 
-	attestations, err := verification.GetRemoteAttestations(c)
+	attestations, err := verification.GetRemoteAttestations(o.APIClient, params)
 	if err != nil {
 		msg := "✗ Loading attestations from GitHub API failed"
 		return nil, msg, err