Merge pull request #934 from n4ss/refactor-trust-inspect

Refactor trust view command into a --pretty flag on trust inspect

cli/command/trust/cmd.go

@@ -16,7 +16,6 @@ func NewTrustCommand(dockerCli command.Cli) *cobra.Command {
 		Annotations: map[string]string{"experimentalCLI": ""},
 	}
 	cmd.AddCommand(
-		newViewCommand(dockerCli),
 		newRevokeCommand(dockerCli),
 		newSignCommand(dockerCli),
 		newTrustKeyCommand(dockerCli),

cli/command/trust/inspect.go

@@ -2,6 +2,7 @@ package trust
 
 import (
 	"encoding/json"
+	"fmt"
 	"sort"
 
 	"github.com/docker/cli/cli"
@@ -11,24 +12,55 @@ import (
 	"github.com/theupdateframework/notary/tuf/data"
 )
 
+type inspectOptions struct {
+	remotes []string
+	// FIXME(n4ss): this is consistent with `docker service inspect` but we should provide
+	// a `--format` flag too. (format and pretty-print should be exclusive)
+	prettyPrint bool
+}
+
 func newInspectCommand(dockerCli command.Cli) *cobra.Command {
+	options := inspectOptions{}
 	cmd := &cobra.Command{
 		Use:   "inspect IMAGE[:TAG] [IMAGE[:TAG]...]",
 		Short: "Return low-level information about keys and signatures",
 		Args:  cli.RequiresMinArgs(1),
 		RunE: func(cmd *cobra.Command, args []string) error {
-			return runInspect(dockerCli, args)
+			options.remotes = args
+
+			return runInspect(dockerCli, options)
 		},
 	}
+
+	flags := cmd.Flags()
+	flags.BoolVar(&options.prettyPrint, "pretty", false, "Print the information in a human friendly format")
+
 	return cmd
 }
 
-func runInspect(dockerCli command.Cli, remotes []string) error {
+func runInspect(dockerCli command.Cli, opts inspectOptions) error {
+	if opts.prettyPrint {
+		var err error
+
+		for index, remote := range opts.remotes {
+			if err = prettyPrintTrustInfo(dockerCli, remote); err != nil {
+				return err
+			}
+
+			// Additional separator between the inspection output of each image
+			if index < len(opts.remotes)-1 {
+				fmt.Fprint(dockerCli.Out(), "\n\n")
+			}
+		}
+
+		return err
+	}
+
 	getRefFunc := func(ref string) (interface{}, []byte, error) {
 		i, err := getRepoTrustInfo(dockerCli, ref)
 		return nil, i, err
 	}
-	return inspect.Inspect(dockerCli.Out(), remotes, "", getRefFunc)
+	return inspect.Inspect(dockerCli.Out(), opts.remotes, "", getRefFunc)
 }
 
 func getRepoTrustInfo(cli command.Cli, remote string) ([]byte, error) {

cli/command/trust/inspect_pretty.go

@@ -4,34 +4,21 @@ import (
 	"fmt"
 	"io"
 	"sort"
-	"strings"
 
-	"github.com/docker/cli/cli"
 	"github.com/docker/cli/cli/command"
 	"github.com/docker/cli/cli/command/formatter"
-	"github.com/spf13/cobra"
 	"github.com/theupdateframework/notary/client"
 )
 
-func newViewCommand(dockerCli command.Cli) *cobra.Command {
-	cmd := &cobra.Command{
-		Use:   "view IMAGE[:TAG]",
-		Short: "Display detailed information about keys and signatures",
-		Args:  cli.ExactArgs(1),
-		RunE: func(cmd *cobra.Command, args []string) error {
-			return viewTrustInfo(dockerCli, args[0])
-		},
-	}
-	return cmd
-}
-
-func viewTrustInfo(cli command.Cli, remote string) error {
+func prettyPrintTrustInfo(cli command.Cli, remote string) error {
 	signatureRows, adminRolesWithSigs, delegationRoles, err := lookupTrustInfo(cli, remote)
 	if err != nil {
 		return err
 	}
 
 	if len(signatureRows) > 0 {
+		fmt.Fprintf(cli.Out(), "\nSignatures for %s\n\n", remote)
+
 		if err := printSignatures(cli.Out(), signatureRows); err != nil {
 			return err
 		}
@@ -42,14 +29,14 @@ func viewTrustInfo(cli command.Cli, remote string) error {
 
 	// If we do not have additional signers, do not display
 	if len(signerRoleToKeyIDs) > 0 {
-		fmt.Fprintf(cli.Out(), "\nList of signers and their keys for %s:\n\n", strings.Split(remote, ":")[0])
+		fmt.Fprintf(cli.Out(), "\nList of signers and their keys for %s\n\n", remote)
 		if err := printSignerInfo(cli.Out(), signerRoleToKeyIDs); err != nil {
 			return err
 		}
 	}
 
 	// This will always have the root and targets information
-	fmt.Fprintf(cli.Out(), "\nAdministrative keys for %s:\n", strings.Split(remote, ":")[0])
+	fmt.Fprintf(cli.Out(), "\nAdministrative keys for %s\n\n", remote)
 	printSortedAdminKeys(cli.Out(), adminRolesWithSigs)
 	return nil
 }
@@ -57,7 +44,9 @@ func viewTrustInfo(cli command.Cli, remote string) error {
 func printSortedAdminKeys(out io.Writer, adminRoles []client.RoleWithSignatures) {
 	sort.Slice(adminRoles, func(i, j int) bool { return adminRoles[i].Name > adminRoles[j].Name })
 	for _, adminRole := range adminRoles {
-		fmt.Fprintf(out, "%s", formatAdminRole(adminRole))
+		if formattedAdminRole := formatAdminRole(adminRole); formattedAdminRole != "" {
+			fmt.Fprintf(out, "  %s", formattedAdminRole)
+		}
 	}
 }
 

cli/command/trust/inspect_pretty_test.go

@@ -17,11 +17,13 @@ import (
 	"github.com/theupdateframework/notary/tuf/data"
 )
 
+// TODO(n4ss): remove common tests with the regular inspect command
+
 type fakeClient struct {
 	dockerClient.Client
 }
 
-func TestTrustViewCommandErrors(t *testing.T) {
+func TestTrustInspectPrettyCommandErrors(t *testing.T) {
 	testCases := []struct {
 		name          string
 		args          []string
@@ -29,12 +31,7 @@ func TestTrustViewCommandErrors(t *testing.T) {
 	}{
 		{
 			name:          "not-enough-args",
-			expectedError: "requires exactly 1 argument",
-		},
-		{
-			name:          "too-many-args",
-			args:          []string{"remote1", "remote2"},
-			expectedError: "requires exactly 1 argument",
+			expectedError: "requires at least 1 argument",
 		},
 		{
 			name:          "sha-reference",
@@ -48,104 +45,115 @@ func TestTrustViewCommandErrors(t *testing.T) {
 		},
 	}
 	for _, tc := range testCases {
-		cmd := newViewCommand(
+		cmd := newInspectCommand(
 			test.NewFakeCli(&fakeClient{}))
 		cmd.SetArgs(tc.args)
 		cmd.SetOutput(ioutil.Discard)
+		cmd.Flags().Set("pretty", "true")
 		assert.ErrorContains(t, cmd.Execute(), tc.expectedError)
 	}
 }
 
-func TestTrustViewCommandOfflineErrors(t *testing.T) {
+func TestTrustInspectPrettyCommandOfflineErrors(t *testing.T) {
 	cli := test.NewFakeCli(&fakeClient{})
 	cli.SetNotaryClient(notaryfake.GetOfflineNotaryRepository)
-	cmd := newViewCommand(cli)
+	cmd := newInspectCommand(cli)
+	cmd.Flags().Set("pretty", "true")
 	cmd.SetArgs([]string{"nonexistent-reg-name.io/image"})
 	cmd.SetOutput(ioutil.Discard)
 	assert.ErrorContains(t, cmd.Execute(), "No signatures or cannot access nonexistent-reg-name.io/image")
 
 	cli = test.NewFakeCli(&fakeClient{})
 	cli.SetNotaryClient(notaryfake.GetOfflineNotaryRepository)
-	cmd = newViewCommand(cli)
+	cmd = newInspectCommand(cli)
+	cmd.Flags().Set("pretty", "true")
 	cmd.SetArgs([]string{"nonexistent-reg-name.io/image:tag"})
 	cmd.SetOutput(ioutil.Discard)
 	assert.ErrorContains(t, cmd.Execute(), "No signatures or cannot access nonexistent-reg-name.io/image")
 }
 
-func TestTrustViewCommandUninitializedErrors(t *testing.T) {
+func TestTrustInspectPrettyCommandUninitializedErrors(t *testing.T) {
 	cli := test.NewFakeCli(&fakeClient{})
 	cli.SetNotaryClient(notaryfake.GetUninitializedNotaryRepository)
-	cmd := newViewCommand(cli)
+	cmd := newInspectCommand(cli)
+	cmd.Flags().Set("pretty", "true")
 	cmd.SetArgs([]string{"reg/unsigned-img"})
 	cmd.SetOutput(ioutil.Discard)
 	assert.ErrorContains(t, cmd.Execute(), "No signatures or cannot access reg/unsigned-img")
 
 	cli = test.NewFakeCli(&fakeClient{})
 	cli.SetNotaryClient(notaryfake.GetUninitializedNotaryRepository)
-	cmd = newViewCommand(cli)
+	cmd = newInspectCommand(cli)
+	cmd.Flags().Set("pretty", "true")
 	cmd.SetArgs([]string{"reg/unsigned-img:tag"})
 	cmd.SetOutput(ioutil.Discard)
 	assert.ErrorContains(t, cmd.Execute(), "No signatures or cannot access reg/unsigned-img:tag")
 }
 
-func TestTrustViewCommandEmptyNotaryRepoErrors(t *testing.T) {
+func TestTrustInspectPrettyCommandEmptyNotaryRepoErrors(t *testing.T) {
 	cli := test.NewFakeCli(&fakeClient{})
 	cli.SetNotaryClient(notaryfake.GetEmptyTargetsNotaryRepository)
-	cmd := newViewCommand(cli)
+	cmd := newInspectCommand(cli)
+	cmd.Flags().Set("pretty", "true")
 	cmd.SetArgs([]string{"reg/img:unsigned-tag"})
 	cmd.SetOutput(ioutil.Discard)
 	assert.NilError(t, cmd.Execute())
 	assert.Check(t, is.Contains(cli.OutBuffer().String(), "No signatures for reg/img:unsigned-tag"))
-	assert.Check(t, is.Contains(cli.OutBuffer().String(), "Administrative keys for reg/img:"))
+	assert.Check(t, is.Contains(cli.OutBuffer().String(), "Administrative keys for reg/img"))
 
 	cli = test.NewFakeCli(&fakeClient{})
 	cli.SetNotaryClient(notaryfake.GetEmptyTargetsNotaryRepository)
-	cmd = newViewCommand(cli)
+	cmd = newInspectCommand(cli)
+	cmd.Flags().Set("pretty", "true")
 	cmd.SetArgs([]string{"reg/img"})
 	cmd.SetOutput(ioutil.Discard)
 	assert.NilError(t, cmd.Execute())
 	assert.Check(t, is.Contains(cli.OutBuffer().String(), "No signatures for reg/img"))
-	assert.Check(t, is.Contains(cli.OutBuffer().String(), "Administrative keys for reg/img:"))
+	assert.Check(t, is.Contains(cli.OutBuffer().String(), "Administrative keys for reg/img"))
 }
 
-func TestTrustViewCommandFullRepoWithoutSigners(t *testing.T) {
+func TestTrustInspectPrettyCommandFullRepoWithoutSigners(t *testing.T) {
 	cli := test.NewFakeCli(&fakeClient{})
 	cli.SetNotaryClient(notaryfake.GetLoadedWithNoSignersNotaryRepository)
-	cmd := newViewCommand(cli)
+	cmd := newInspectCommand(cli)
+	cmd.Flags().Set("pretty", "true")
 	cmd.SetArgs([]string{"signed-repo"})
 	assert.NilError(t, cmd.Execute())
 
-	golden.Assert(t, cli.OutBuffer().String(), "trust-view-full-repo-no-signers.golden")
+	golden.Assert(t, cli.OutBuffer().String(), "trust-inspect-pretty-full-repo-no-signers.golden")
 }
 
-func TestTrustViewCommandOneTagWithoutSigners(t *testing.T) {
+func TestTrustInspectPrettyCommandOneTagWithoutSigners(t *testing.T) {
 	cli := test.NewFakeCli(&fakeClient{})
 	cli.SetNotaryClient(notaryfake.GetLoadedWithNoSignersNotaryRepository)
-	cmd := newViewCommand(cli)
+	cmd := newInspectCommand(cli)
+	cmd.Flags().Set("pretty", "true")
 	cmd.SetArgs([]string{"signed-repo:green"})
 	assert.NilError(t, cmd.Execute())
 
-	golden.Assert(t, cli.OutBuffer().String(), "trust-view-one-tag-no-signers.golden")
+	golden.Assert(t, cli.OutBuffer().String(), "trust-inspect-pretty-one-tag-no-signers.golden")
 }
 
-func TestTrustViewCommandFullRepoWithSigners(t *testing.T) {
+func TestTrustInspectPrettyCommandFullRepoWithSigners(t *testing.T) {
 	cli := test.NewFakeCli(&fakeClient{})
 	cli.SetNotaryClient(notaryfake.GetLoadedNotaryRepository)
-	cmd := newViewCommand(cli)
+	cmd := newInspectCommand(cli)
+	cmd.Flags().Set("pretty", "true")
 	cmd.SetArgs([]string{"signed-repo"})
 	assert.NilError(t, cmd.Execute())
 
-	golden.Assert(t, cli.OutBuffer().String(), "trust-view-full-repo-with-signers.golden")
+	golden.Assert(t, cli.OutBuffer().String(), "trust-inspect-pretty-full-repo-with-signers.golden")
 }
 
-func TestTrustViewCommandUnsignedTagInSignedRepo(t *testing.T) {
+func TestTrustInspectPrettyCommandUnsignedTagInSignedRepo(t *testing.T) {
 	cli := test.NewFakeCli(&fakeClient{})
 	cli.SetNotaryClient(notaryfake.GetLoadedNotaryRepository)
-	cmd := newViewCommand(cli)
+	cmd := newInspectCommand(cli)
+	cmd.Flags().Set("pretty", "true")
 	cmd.SetArgs([]string{"signed-repo:unsigned"})
 	assert.NilError(t, cmd.Execute())
 
-	golden.Assert(t, cli.OutBuffer().String(), "trust-view-unsigned-tag-with-signers.golden")
+	golden.Assert(t, cli.OutBuffer().String(), "trust-inspect-pretty-unsigned-tag-with-signers.golden")
 }
 
 func TestNotaryRoleToSigner(t *testing.T) {

cli/command/trust/inspect_test.go

@@ -18,12 +18,7 @@ func TestTrustInspectCommandErrors(t *testing.T) {
 	}{
 		{
 			name:          "not-enough-args",
-			expectedError: "requires exactly 1 argument",
-		},
-		{
-			name:          "too-many-args",
-			args:          []string{"remote1", "remote2"},
-			expectedError: "requires exactly 1 argument",
+			expectedError: "requires at least 1 argument",
 		},
 		{
 			name:          "sha-reference",
@@ -37,8 +32,9 @@ func TestTrustInspectCommandErrors(t *testing.T) {
 		},
 	}
 	for _, tc := range testCases {
-		cmd := newViewCommand(
+		cmd := newInspectCommand(
 			test.NewFakeCli(&fakeClient{}))
+		cmd.Flags().Set("pretty", "true")
 		cmd.SetArgs(tc.args)
 		cmd.SetOutput(ioutil.Discard)
 		assert.ErrorContains(t, cmd.Execute(), tc.expectedError)

cli/command/trust/testdata/trust-inspect-pretty-full-repo-no-signers.golden

@@ -1,6 +1,10 @@
+
+Signatures for signed-repo
+
 SIGNED TAG          DIGEST                     SIGNERS
 green               677265656e2d646967657374   (Repo Admin)
 
-Administrative keys for signed-repo:
-Repository Key:	targetsID
-Root Key:	rootID
+Administrative keys for signed-repo
+
+  Repository Key:	targetsID
+  Root Key:	rootID

cli/command/trust/testdata/trust-inspect-pretty-full-repo-with-signers.golden

@@ -1,14 +1,18 @@
+
+Signatures for signed-repo
+
 SIGNED TAG          DIGEST                     SIGNERS
 blue                626c75652d646967657374     alice
 green               677265656e2d646967657374   (Repo Admin)
 red                 7265642d646967657374       alice, bob
 
-List of signers and their keys for signed-repo:
+List of signers and their keys for signed-repo
 
 SIGNER              KEYS
 alice               A
 bob                 B
 
-Administrative keys for signed-repo:
-Repository Key:	targetsID
-Root Key:	rootID
+Administrative keys for signed-repo
+
+  Repository Key:	targetsID
+  Root Key:	rootID

cli/command/trust/testdata/trust-inspect-pretty-one-tag-no-signers.golden

@@ -0,0 +1,10 @@
+
+Signatures for signed-repo:green
+
+SIGNED TAG          DIGEST                     SIGNERS
+green               677265656e2d646967657374   (Repo Admin)
+
+Administrative keys for signed-repo:green
+
+  Repository Key:	targetsID
+  Root Key:	rootID

cli/command/trust/testdata/trust-inspect-pretty-unsigned-tag-with-signers.golden

@@ -0,0 +1,14 @@
+
+No signatures for signed-repo:unsigned
+
+
+List of signers and their keys for signed-repo:unsigned
+
+SIGNER              KEYS
+alice               A
+bob                 B
+
+Administrative keys for signed-repo:unsigned
+
+  Repository Key:	targetsID
+  Root Key:	rootID

cli/command/trust/testdata/trust-view-one-tag-no-signers.golden

@@ -1,6 +0,0 @@
-SIGNED TAG          DIGEST                     SIGNERS
-green               677265656e2d646967657374   (Repo Admin)
-
-Administrative keys for signed-repo:
-Repository Key:	targetsID
-Root Key:	rootID

cli/command/trust/testdata/trust-view-unsigned-tag-with-signers.golden

@@ -1,13 +0,0 @@
-
-No signatures for signed-repo:unsigned
-
-
-List of signers and their keys for signed-repo:
-
-SIGNER              KEYS
-alice               A
-bob                 B
-
-Administrative keys for signed-repo:
-Repository Key:	targetsID
-Root Key:	rootID