delegations CLI UX improvements

Signed-off-by: Riyaz Faizullabhoy <riyaz.faizullabhoy@docker.com>

cmd/notary/delegations.go

@@ -32,9 +32,9 @@ var cmdDelegationRemoveTemplate = usageTemplate{
 }
 
 var cmdDelegationAddTemplate = usageTemplate{
-	Use:   "add [ GUN ] [ Role ] <PEM file path 1> ...",
-	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 ] <X509 file path 1> ...",
+	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.Println("")
 
 	return nil

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()

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 (

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
 	}
 

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)