From 774b66c9fe02d8736c0e1974db8672527403ff97 Mon Sep 17 00:00:00 2001 From: Riyaz Faizullabhoy Date: Thu, 21 Jan 2016 17:13:49 -0800 Subject: [PATCH] delegations CLI UX improvements Signed-off-by: Riyaz Faizullabhoy --- cmd/notary/delegations.go | 20 +++++++++++++------- cmd/notary/delegations_test.go | 12 ++++++++++++ cmd/notary/integration_test.go | 10 +++++----- cmd/notary/prettyprint.go | 4 ++-- cmd/notary/prettyprint_test.go | 6 +++--- 5 files changed, 35 insertions(+), 17 deletions(-) diff --git a/cmd/notary/delegations.go b/cmd/notary/delegations.go index f061942043..174340f181 100644 --- a/cmd/notary/delegations.go +++ b/cmd/notary/delegations.go @@ -32,9 +32,9 @@ var cmdDelegationRemoveTemplate = usageTemplate{ } var cmdDelegationAddTemplate = usageTemplate{ - Use: "add [ GUN ] [ Role ] ...", - Short: "Add a keys to delegation using the provided public key certificate PEMs.", - Long: "Add a keys to delegation using the provided public key certificate PEMs in a specific Global Unique Name.", + Use: "add [ GUN ] [ Role ] ...", + Short: "Add a keys to delegation using the provided public key X509 certificates.", + Long: "Add a keys to delegation using the provided public key PEM encoded X509 certificates in a specific Global Unique Name.", } var paths []string @@ -84,7 +84,7 @@ func (d *delegationCommander) delegationsList(cmd *cobra.Command, args []string) } cmd.Println("") - prettyPrintRoles(delegationRoles, cmd.Out()) + prettyPrintRoles(delegationRoles, cmd.Out(), "delegations") cmd.Println("") return nil } @@ -100,6 +100,11 @@ func (d *delegationCommander) delegationRemove(cmd *cobra.Command, args []string gun := args[0] role := args[1] + // Check if role is valid delegation name before requiring any user input + if !data.IsDelegation(role) { + return fmt.Errorf("invalid delegation name %s", role) + } + // If we're only given the gun and the role, attempt to remove all data for this delegation if len(args) == 2 && paths == nil { removeAll = true @@ -143,10 +148,11 @@ func (d *delegationCommander) delegationRemove(cmd *cobra.Command, args []string cmd.Println("") if removeAll { cmd.Printf("Forced removal (including all keys and paths) of delegation role %s to repository \"%s\" staged for next publish.\n", role, gun) + } else { + cmd.Printf( + "Removal of delegation role %s with keys %s and paths %s, to repository \"%s\" staged for next publish.\n", + role, keyIDs, paths, gun) } - cmd.Printf( - "Removal of delegation role %s with keys %s and paths %s, to repository \"%s\" staged for next publish.\n", - role, keyIDs, paths, gun) cmd.Println("") return nil diff --git a/cmd/notary/delegations_test.go b/cmd/notary/delegations_test.go index 9550965477..02e7c4607e 100644 --- a/cmd/notary/delegations_test.go +++ b/cmd/notary/delegations_test.go @@ -80,6 +80,18 @@ func TestRemoveInvalidDelegationName(t *testing.T) { assert.Error(t, err) } +func TestRemoveAllInvalidDelegationName(t *testing.T) { + // Cleanup after test + defer os.RemoveAll(testTrustDir) + + // Setup commander + commander := setup() + + // Should error due to invalid delegation name (should be prefixed by "targets/") + err := commander.delegationRemove(commander.GetCommand(), []string{"gun", "INVALID_NAME"}) + assert.Error(t, err) +} + func TestAddInvalidNumArgs(t *testing.T) { // Setup commander commander := setup() diff --git a/cmd/notary/integration_test.go b/cmd/notary/integration_test.go index eec3b1f52c..0cdf0ef9b0 100644 --- a/cmd/notary/integration_test.go +++ b/cmd/notary/integration_test.go @@ -189,7 +189,7 @@ func TestClientDelegationsInteraction(t *testing.T) { // list delegations - none yet output, err = runCommand(t, tempDir, "-s", server.URL, "delegation", "list", "gun") assert.NoError(t, err) - assert.Contains(t, output, "No such roles published in this repository.") + assert.Contains(t, output, "No delegations present in this repository.") // add new valid delegation with single new cert output, err = runCommand(t, tempDir, "delegation", "add", "gun", "targets/delegation", tempFile.Name()) @@ -204,7 +204,7 @@ func TestClientDelegationsInteraction(t *testing.T) { // list delegations - none yet because still unpublished output, err = runCommand(t, tempDir, "-s", server.URL, "delegation", "list", "gun") assert.NoError(t, err) - assert.Contains(t, output, "No such roles published in this repository.") + assert.Contains(t, output, "No delegations present in this repository.") // publish repo _, err = runCommand(t, tempDir, "-s", server.URL, "publish", "gun") @@ -281,7 +281,7 @@ func TestClientDelegationsInteraction(t *testing.T) { // list delegations - we should see no delegations output, err = runCommand(t, tempDir, "-s", server.URL, "delegation", "list", "gun") assert.NoError(t, err) - assert.Contains(t, output, "No such roles published in this repository.") + assert.Contains(t, output, "No delegations present in this repository.") // add delegation with multiple certs and multiple paths output, err = runCommand(t, tempDir, "delegation", "add", "gun", "targets/delegation", tempFile.Name(), tempFile2.Name(), "--paths", "path1,path2") @@ -350,7 +350,7 @@ func TestClientDelegationsInteraction(t *testing.T) { // remove by force to delete the delegation entirely output, err = runCommand(t, tempDir, "delegation", "remove", "gun", "targets/delegation", "-y") assert.NoError(t, err) - assert.Contains(t, output, "Removal of delegation role") + assert.Contains(t, output, "Forced removal (including all keys and paths) of delegation role") // publish repo _, err = runCommand(t, tempDir, "-s", server.URL, "publish", "gun") @@ -359,7 +359,7 @@ func TestClientDelegationsInteraction(t *testing.T) { // list delegations - we should see no delegations output, err = runCommand(t, tempDir, "-s", server.URL, "delegation", "list", "gun") assert.NoError(t, err) - assert.Contains(t, output, "No such roles published in this repository.") + assert.Contains(t, output, "No delegations present in this repository.") } // Splits a string into lines, and returns any lines that are not empty ( diff --git a/cmd/notary/prettyprint.go b/cmd/notary/prettyprint.go index 3cb870fb07..6adc619cf6 100644 --- a/cmd/notary/prettyprint.go +++ b/cmd/notary/prettyprint.go @@ -176,9 +176,9 @@ func prettyPrintTargets(ts []*client.TargetWithRole, writer io.Writer) { } // Pretty-prints the list of provided Roles -func prettyPrintRoles(rs []*data.Role, writer io.Writer) { +func prettyPrintRoles(rs []*data.Role, writer io.Writer, roleType string) { if len(rs) == 0 { - writer.Write([]byte("\nNo such roles published in this repository.\n\n")) + writer.Write([]byte(fmt.Sprintf("\nNo %s present in this repository.\n\n", roleType))) return } diff --git a/cmd/notary/prettyprint_test.go b/cmd/notary/prettyprint_test.go index f445703f7e..496b4c89c5 100644 --- a/cmd/notary/prettyprint_test.go +++ b/cmd/notary/prettyprint_test.go @@ -220,13 +220,13 @@ func generateCertificate(t *testing.T, gun string, expireInHours int64) *x509.Ce // are no roles. func TestPrettyPrintZeroRoles(t *testing.T) { var b bytes.Buffer - prettyPrintRoles([]*data.Role{}, &b) + prettyPrintRoles([]*data.Role{}, &b, "delegations") text, err := ioutil.ReadAll(&b) assert.NoError(t, err) lines := strings.Split(strings.TrimSpace(string(text)), "\n") assert.Len(t, lines, 1) - assert.Equal(t, "No such roles published in this repository.", lines[0]) + assert.Equal(t, "No delegations present in this repository.", lines[0]) } // Roles are sorted by name, and the name, paths, and KeyIDs are printed. @@ -241,7 +241,7 @@ func TestPrettyPrintSortedRoles(t *testing.T) { } var b bytes.Buffer - prettyPrintRoles(unsorted, &b) + prettyPrintRoles(unsorted, &b, "delegations") text, err := ioutil.ReadAll(&b) assert.NoError(t, err)