add testing around OCI and API client

Signed-off-by: Meredith Lancaster <malancas@github.com>

pkg/cmd/attestation/artifact/artifact.go

@@ -51,7 +51,7 @@ func normalizeReference(reference string, pathSeparator rune) (normalized string
 	return filepath.Clean(reference), fileArtifactType, nil
 }
 
-func NewDigestedArtifact(client oci.Client, reference, digestAlg string) (artifact *DigestedArtifact, err error) {
+func NewDigestedArtifact(client *oci.Client, reference, digestAlg string) (artifact *DigestedArtifact, err error) {
 	normalized, artifactType, err := normalizeReference(reference, os.PathSeparator)
 	if err != nil {
 		return nil, err

pkg/cmd/attestation/artifact/image.go

@@ -7,7 +7,10 @@ import (
 	"github.com/distribution/reference"
 )
 
-func digestContainerImageArtifact(url string, client oci.Client) (*DigestedArtifact, error) {
+func digestContainerImageArtifact(url string, client *oci.Client) (*DigestedArtifact, error) {
+	if client == nil {
+		return nil, fmt.Errorf("missing OCI client")
+	}
 	// try to parse the url as a valid registry reference
 	named, err := reference.Parse(url)
 	if err != nil {

pkg/cmd/attestation/artifact/image_test.go

@@ -19,23 +19,34 @@ func TestDigestContainerImageArtifact(t *testing.T) {
 	assert.Equal(t, "sha256", digestedArtifact.digestAlg)
 }
 
-func TestFetchImageFailure(t *testing.T) {
+func TestParseImageRefFailure(t *testing.T) {
 	client := oci.NewReferenceFailClient()
 	url := "example.com/repo:tag"
 	_, err := digestContainerImageArtifact(url, client)
 	assert.Error(t, err)
 }
 
-func TestRegistryAuthFailure(t *testing.T) {
-	client := oci.NewAuthFailClient()
-	url := "example.com/repo:tag"
-	_, err := digestContainerImageArtifact(url, client)
-	assert.ErrorIs(t, err, oci.ErrRegistryAuthz)
-}
+func TestFetchImageFailure(t *testing.T) {
+	testcase := []struct {
+		name        string
+		client      *oci.Client
+		expectedErr error
+	}{
+		{
+			name:        "Fail to authorize with registry",
+			client:      oci.NewAuthFailClient(),
+			expectedErr: oci.ErrRegistryAuthz,
+		},
+		{
+			name:        "Fail to fetch image due to denial",
+			client:      oci.NewDeniedClient(),
+			expectedErr: oci.ErrDenied,
+		},
+	}
 
-func TestDeniedFailure(t *testing.T) {
-	client := oci.NewDeniedClient()
-	url := "example.com/repo:tag"
-	_, err := digestContainerImageArtifact(url, client)
-	assert.ErrorIs(t, err, oci.ErrDenied)
+	for _, tc := range testcase {
+		url := "example.com/repo:tag"
+		_, err := digestContainerImageArtifact(url, tc.client)
+		assert.ErrorIs(t, err, tc.expectedErr)
+	}
 }

pkg/cmd/attestation/artifact/oci/oci.go

@@ -52,15 +52,15 @@ func (c Client) GetImageDigest(imgName string) (*v1.Hash, error) {
 	return &desc.Digest, nil
 }
 
-func NewLiveClient() Client {
-	return Client{
+func NewLiveClient() *Client {
+	return &Client{
 		ParseReference: name.ParseReference,
 		Get:            remote.Get,
 	}
 }
 
-func NewMockClient() Client {
-	return Client{
+func NewMockClient() *Client {
+	return &Client{
 		ParseReference: func(string, ...name.Option) (name.Reference, error) {
 			return name.Tag{}, nil
 		},
@@ -76,8 +76,8 @@ func NewMockClient() Client {
 	}
 }
 
-func NewReferenceFailClient() Client {
-	return Client{
+func NewReferenceFailClient() *Client {
+	return &Client{
 		ParseReference: func(string, ...name.Option) (name.Reference, error) {
 			return nil, fmt.Errorf("failed to parse reference")
 		},
@@ -87,8 +87,8 @@ func NewReferenceFailClient() Client {
 	}
 }
 
-func NewAuthFailClient() Client {
-	return Client{
+func NewAuthFailClient() *Client {
+	return &Client{
 		ParseReference: func(string, ...name.Option) (name.Reference, error) {
 			return name.Tag{}, nil
 		},
@@ -98,8 +98,8 @@ func NewAuthFailClient() Client {
 	}
 }
 
-func NewDeniedClient() Client {
-	return Client{
+func NewDeniedClient() *Client {
+	return &Client{
 		ParseReference: func(string, ...name.Option) (name.Reference, error) {
 			return name.Tag{}, nil
 		},

pkg/cmd/attestation/attestation.go

@@ -16,6 +16,7 @@ func NewCmdAttestation(f *cmdutil.Factory) *cobra.Command {
 		Use:     "attestation [subcommand]",
 		Short:   "Work with attestations",
 		Aliases: []string{"at"},
+		Hidden:  true,
 		Long: heredoc.Docf(`
 		Work with attestations that represent trusted metadata about artifacts and images.
 

pkg/cmd/attestation/download/download.go

@@ -102,6 +102,9 @@ func NewDownloadCmd(f *cmdutil.Factory) *cobra.Command {
 }
 
 func RunDownload(opts *Options) error {
+	if opts.APIClient == nil {
+		return fmt.Errorf("missing API client")
+	}
 	artifact, err := artifact.NewDigestedArtifact(opts.OCIClient, opts.ArtifactPath, opts.DigestAlgorithm)
 	if err != nil {
 		return fmt.Errorf("failed to digest artifact: %w", err)

pkg/cmd/attestation/download/download_test.go

@@ -108,6 +108,19 @@ func TestRunDownload(t *testing.T) {
 		assert.Error(t, err)
 		assert.ErrorContains(t, err, "failed to digest artifact")
 	})
+
+	t.Run("with missing OCI client", func(t *testing.T) {
+		customOpts := baseOpts
+		customOpts.ArtifactPath = "oci://ghcr.io/github/test"
+		customOpts.OCIClient = nil
+		assert.Error(t, RunDownload(&customOpts))
+	})
+
+	t.Run("with missing API client", func(t *testing.T) {
+		customOpts := baseOpts
+		customOpts.APIClient = nil
+		assert.Error(t, RunDownload(&customOpts))
+	})
 }
 
 func TestCreateJSONLinesFilePath(t *testing.T) {

pkg/cmd/attestation/download/options.go

@@ -10,12 +10,12 @@ import (
 )
 
 type Options struct {
+	APIClient       api.Client
 	ArtifactPath    string
 	DigestAlgorithm string
-	APIClient       api.Client
 	Logger          *logging.Logger
 	Limit           int
-	OCIClient       oci.Client
+	OCIClient       *oci.Client
 	OutputPath      string
 	Owner           string
 	Verbose         bool

pkg/cmd/attestation/inspect/inspect_test.go

@@ -3,6 +3,7 @@ package inspect
 import (
 	"testing"
 
+	"github.com/cli/cli/v2/pkg/cmd/attestation/artifact/oci"
 	"github.com/cli/cli/v2/pkg/cmd/attestation/logging"
 	"github.com/cli/cli/v2/pkg/cmd/attestation/test"
 
@@ -30,6 +31,7 @@ func TestRunInspect(t *testing.T) {
 		BundlePath:      bundlePath,
 		DigestAlgorithm: "sha512",
 		Logger:          logger,
+		OCIClient:       oci.NewMockClient(),
 	}
 
 	t.Run("with valid artifact and bundle", func(t *testing.T) {
@@ -63,4 +65,11 @@ func TestRunInspect(t *testing.T) {
 		customOpts.BundlePath = "../test/data/sigstore-js-2.1.0_with_2_bundles.jsonl"
 		assert.Nil(t, RunInspect(&customOpts))
 	})
+
+	t.Run("with missing OCI client", func(t *testing.T) {
+		customOpts := opts
+		customOpts.ArtifactPath = "oci://ghcr.io/github/test"
+		customOpts.OCIClient = nil
+		assert.Error(t, RunInspect(&customOpts))
+	})
 }

pkg/cmd/attestation/inspect/options.go

@@ -18,7 +18,7 @@ type Options struct {
 	Verbose         bool
 	Quiet           bool
 	Logger          *logging.Logger
-	OCIClient       oci.Client
+	OCIClient       *oci.Client
 }
 
 // Clean cleans the file path option values

pkg/cmd/attestation/tufrootverify/tuf-root-verify.go

@@ -18,10 +18,9 @@ func NewTUFRootVerifyCmd(f *cmdutil.Factory) *cobra.Command {
 	var mirror string
 	var root string
 	var cmd = cobra.Command{
-		Use:    "tuf-root-verify --mirror <mirror-url> --root <root.json>",
-		Args:   cobra.ExactArgs(0),
-		Hidden: true,
-		Short:  "Verify the TUF repository from a provided TUF root",
+		Use:   "tuf-root-verify --mirror <mirror-url> --root <root.json>",
+		Args:  cobra.ExactArgs(0),
+		Short: "Verify the TUF repository from a provided TUF root",
 		Long: heredoc.Docf(`
 			Verify a TUF repository with a local TUF root.
 

pkg/cmd/attestation/verification/attestation.go

@@ -93,6 +93,9 @@ func loadBundlesFromJSONLinesFile(path string) ([]*api.Attestation, error) {
 }
 
 func getRemoteAttestations(c FetchAttestationsConfig) ([]*api.Attestation, error) {
+	if c.APIClient == 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 != "" {

pkg/cmd/attestation/verify/options.go

@@ -30,7 +30,7 @@ type Options struct {
 	APIClient            api.Client
 	Logger               *logging.Logger
 	Limit                int
-	OCIClient            oci.Client
+	OCIClient            *oci.Client
 }
 
 // Clean cleans the file path option values

pkg/cmd/attestation/verify/verify_test.go

@@ -162,6 +162,20 @@ func TestRunVerify(t *testing.T) {
 		opts.BundlePath = "../test/data/sigstore-js-2.1.0_with_2_bundles.jsonl"
 		assert.Nil(t, RunVerify(&opts))
 	})
+
+	t.Run("with missing OCI client", func(t *testing.T) {
+		customOpts := publicGoodOpts
+		customOpts.ArtifactPath = "oci://ghcr.io/github/test"
+		customOpts.OCIClient = nil
+		assert.Error(t, RunVerify(&customOpts))
+	})
+
+	t.Run("with missing API client", func(t *testing.T) {
+		customOpts := publicGoodOpts
+		customOpts.APIClient = nil
+		customOpts.BundlePath = ""
+		assert.Error(t, RunVerify(&customOpts))
+	})
 }
 
 func TestVerifySLSAPredicateType_InvalidPredicate(t *testing.T) {