From c5554f811b385f84e57d57bf89d97d4ec1efe1e3 Mon Sep 17 00:00:00 2001 From: Nassim 'Nass' Eddequiouaq Date: Fri, 9 Mar 2018 18:37:43 +0100 Subject: [PATCH] Refactor trust view command into a --pretty flag on trust inspect Signed-off-by: Nassim 'Nass' Eddequiouaq --- cli/command/trust/cmd.go | 1 - cli/command/trust/inspect.go | 39 ++++++++++- .../trust/{view.go => inspect_pretty.go} | 27 +++----- .../{view_test.go => inspect_pretty_test.go} | 66 +++++++++++-------- cli/command/trust/inspect_test.go | 10 +-- ...nspect-pretty-full-repo-no-signers.golden} | 10 ++- ...pect-pretty-full-repo-with-signers.golden} | 12 ++-- ...t-inspect-pretty-one-tag-no-signers.golden | 10 +++ ...ct-pretty-unsigned-tag-with-signers.golden | 14 ++++ .../trust-view-one-tag-no-signers.golden | 6 -- ...rust-view-unsigned-tag-with-signers.golden | 13 ---- 11 files changed, 123 insertions(+), 85 deletions(-) rename cli/command/trust/{view.go => inspect_pretty.go} (78%) rename cli/command/trust/{view_test.go => inspect_pretty_test.go} (90%) rename cli/command/trust/testdata/{trust-view-full-repo-no-signers.golden => trust-inspect-pretty-full-repo-no-signers.golden} (50%) rename cli/command/trust/testdata/{trust-view-full-repo-with-signers.golden => trust-inspect-pretty-full-repo-with-signers.golden} (65%) create mode 100644 cli/command/trust/testdata/trust-inspect-pretty-one-tag-no-signers.golden create mode 100644 cli/command/trust/testdata/trust-inspect-pretty-unsigned-tag-with-signers.golden delete mode 100644 cli/command/trust/testdata/trust-view-one-tag-no-signers.golden delete mode 100644 cli/command/trust/testdata/trust-view-unsigned-tag-with-signers.golden diff --git a/cli/command/trust/cmd.go b/cli/command/trust/cmd.go index 6ffc57d05f..3496ce65ee 100644 --- a/cli/command/trust/cmd.go +++ b/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), diff --git a/cli/command/trust/inspect.go b/cli/command/trust/inspect.go index 772c95e888..ae481c1ab7 100644 --- a/cli/command/trust/inspect.go +++ b/cli/command/trust/inspect.go @@ -2,6 +2,7 @@ package trust import ( "encoding/json" + "fmt" "sort" "github.com/docker/cli/cli" @@ -11,24 +12,56 @@ 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 = nil + + 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) { diff --git a/cli/command/trust/view.go b/cli/command/trust/inspect_pretty.go similarity index 78% rename from cli/command/trust/view.go rename to cli/command/trust/inspect_pretty.go index 16e7f13f72..af146d20d2 100644 --- a/cli/command/trust/view.go +++ b/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) + } } } diff --git a/cli/command/trust/view_test.go b/cli/command/trust/inspect_pretty_test.go similarity index 90% rename from cli/command/trust/view_test.go rename to cli/command/trust/inspect_pretty_test.go index 73dfec6215..f97b6239ac 100644 --- a/cli/command/trust/view_test.go +++ b/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,50 +45,56 @@ 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()) @@ -100,7 +103,8 @@ func TestTrustViewCommandEmptyNotaryRepoErrors(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"}) cmd.SetOutput(ioutil.Discard) assert.NilError(t, cmd.Execute()) @@ -108,44 +112,48 @@ func TestTrustViewCommandEmptyNotaryRepoErrors(t *testing.T) { 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) { diff --git a/cli/command/trust/inspect_test.go b/cli/command/trust/inspect_test.go index f72f84374c..6e0e402bbd 100644 --- a/cli/command/trust/inspect_test.go +++ b/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) diff --git a/cli/command/trust/testdata/trust-view-full-repo-no-signers.golden b/cli/command/trust/testdata/trust-inspect-pretty-full-repo-no-signers.golden similarity index 50% rename from cli/command/trust/testdata/trust-view-full-repo-no-signers.golden rename to cli/command/trust/testdata/trust-inspect-pretty-full-repo-no-signers.golden index 6fb3b5b34b..9f3ada0843 100644 --- a/cli/command/trust/testdata/trust-view-full-repo-no-signers.golden +++ b/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 diff --git a/cli/command/trust/testdata/trust-view-full-repo-with-signers.golden b/cli/command/trust/testdata/trust-inspect-pretty-full-repo-with-signers.golden similarity index 65% rename from cli/command/trust/testdata/trust-view-full-repo-with-signers.golden rename to cli/command/trust/testdata/trust-inspect-pretty-full-repo-with-signers.golden index 7e73f06726..49b1efd2b0 100644 --- a/cli/command/trust/testdata/trust-view-full-repo-with-signers.golden +++ b/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 diff --git a/cli/command/trust/testdata/trust-inspect-pretty-one-tag-no-signers.golden b/cli/command/trust/testdata/trust-inspect-pretty-one-tag-no-signers.golden new file mode 100644 index 0000000000..b58572890e --- /dev/null +++ b/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 diff --git a/cli/command/trust/testdata/trust-inspect-pretty-unsigned-tag-with-signers.golden b/cli/command/trust/testdata/trust-inspect-pretty-unsigned-tag-with-signers.golden new file mode 100644 index 0000000000..302a6b5e66 --- /dev/null +++ b/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 diff --git a/cli/command/trust/testdata/trust-view-one-tag-no-signers.golden b/cli/command/trust/testdata/trust-view-one-tag-no-signers.golden deleted file mode 100644 index 6fb3b5b34b..0000000000 --- a/cli/command/trust/testdata/trust-view-one-tag-no-signers.golden +++ /dev/null @@ -1,6 +0,0 @@ -SIGNED TAG DIGEST SIGNERS -green 677265656e2d646967657374 (Repo Admin) - -Administrative keys for signed-repo: -Repository Key: targetsID -Root Key: rootID diff --git a/cli/command/trust/testdata/trust-view-unsigned-tag-with-signers.golden b/cli/command/trust/testdata/trust-view-unsigned-tag-with-signers.golden deleted file mode 100644 index c00a9feecf..0000000000 --- a/cli/command/trust/testdata/trust-view-unsigned-tag-with-signers.golden +++ /dev/null @@ -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