From 6ffde51d897210e0e2c53f2b97ef804f6f9fcdc1 Mon Sep 17 00:00:00 2001 From: Riyaz Faizullabhoy Date: Wed, 27 Jan 2016 17:01:23 -0800 Subject: [PATCH 1/5] Ensure empty string path is properly handled, make default for adding delegation Signed-off-by: Riyaz Faizullabhoy --- cmd/notary/delegations.go | 23 ++++++++++++++--------- cmd/notary/integration_test.go | 6 ++++-- cmd/notary/prettyprint.go | 15 ++++++++++++++- 3 files changed, 32 insertions(+), 12 deletions(-) diff --git a/cmd/notary/delegations.go b/cmd/notary/delegations.go index d47f87b34a..1838dbaed2 100644 --- a/cmd/notary/delegations.go +++ b/cmd/notary/delegations.go @@ -42,8 +42,8 @@ type delegationCommander struct { configGetter func() (*viper.Viper, error) retriever passphrase.Retriever - paths []string - removeAll, removeYes bool + paths []string + removeAll, forceYes bool } func (d *delegationCommander) GetCommand() *cobra.Command { @@ -53,7 +53,7 @@ func (d *delegationCommander) GetCommand() *cobra.Command { cmdRemDelg := cmdDelegationRemoveTemplate.ToCommand(d.delegationRemove) cmdRemDelg.Flags().StringSliceVar(&d.paths, "paths", nil, "List of paths to remove") cmdRemDelg.Flags().BoolVarP( - &d.removeYes, "yes", "y", false, "Answer yes to the removal question (no confirmation)") + &d.forceYes, "yes", "y", false, "Answer yes to the removal question (no confirmation)") cmd.AddCommand(cmdRemDelg) cmdAddDelg := cmdDelegationAddTemplate.ToCommand(d.delegationAdd) @@ -146,7 +146,7 @@ func (d *delegationCommander) delegationRemove(cmd *cobra.Command, args []string if d.removeAll { cmd.Println("\nAre you sure you want to remove all data for this delegation? (yes/no)") // Ask for confirmation before force removing delegation - if !d.removeYes { + if !d.forceYes { confirmed := askConfirm() if !confirmed { fatalf("Aborting action.") @@ -171,9 +171,14 @@ func (d *delegationCommander) delegationRemove(cmd *cobra.Command, args []string if d.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, d.paths, gun) + removingItems := "" + if len(keyIDs) > 0 { + removingItems = removingItems + fmt.Sprintf("with keys %s, ", keyIDs) + } + if d.paths != nil { + removingItems = removingItems + fmt.Sprintf("with paths [%s], ", prettyPrintPaths(d.paths)) + } + cmd.Printf("Removal of delegation role %s %sto repository \"%s\" staged for next publish.\n", role, removingItems, gun) } cmd.Println("") @@ -240,8 +245,8 @@ func (d *delegationCommander) delegationAdd(cmd *cobra.Command, args []string) e cmd.Println("") cmd.Printf( - "Addition of delegation role %s with keys %s and paths %s, to repository \"%s\" staged for next publish.\n", - role, pubKeyIDs, d.paths, gun) + "Addition of delegation role %s with keys %s to paths [%s], to repository \"%s\" staged for next publish.\n", + role, pubKeyIDs, prettyPrintPaths(d.paths), gun) cmd.Println("") return nil } diff --git a/cmd/notary/integration_test.go b/cmd/notary/integration_test.go index 292b8ce9b9..df9eefb9fa 100644 --- a/cmd/notary/integration_test.go +++ b/cmd/notary/integration_test.go @@ -195,10 +195,11 @@ func TestClientDelegationsInteraction(t *testing.T) { assert.NoError(t, err) assert.Contains(t, output, "No delegations present in this repository.") - // add new valid delegation with single new cert + // add new valid delegation with single new cert, and "" path output, err = runCommand(t, tempDir, "delegation", "add", "gun", "targets/delegation", tempFile.Name()) assert.NoError(t, err) assert.Contains(t, output, "Addition of delegation role") + assert.Contains(t, output, "\"\"") // check status - see delegation output, err = runCommand(t, tempDir, "status", "gun") @@ -224,6 +225,7 @@ func TestClientDelegationsInteraction(t *testing.T) { assert.NoError(t, err) assert.Contains(t, output, "targets/delegation") assert.Contains(t, output, keyID) + assert.Contains(t, output, "\"\"") // Setup another certificate tempFile2, err := ioutil.TempFile("", "pemfile2") @@ -246,7 +248,7 @@ func TestClientDelegationsInteraction(t *testing.T) { keyID2, err := utils.CanonicalKeyID(parsedPubKey2) assert.NoError(t, err) - // add to the delegation by specifying the same role, this time add another key and path + // add to the delegation by specifying the same role, this time add a scoped path output, err = runCommand(t, tempDir, "delegation", "add", "gun", "targets/delegation", tempFile2.Name(), "--paths", "path") assert.NoError(t, err) assert.Contains(t, output, "Addition of delegation role") diff --git a/cmd/notary/prettyprint.go b/cmd/notary/prettyprint.go index 6adc619cf6..be750fd2b6 100644 --- a/cmd/notary/prettyprint.go +++ b/cmd/notary/prettyprint.go @@ -190,7 +190,7 @@ func prettyPrintRoles(rs []*data.Role, writer io.Writer, roleType string) { for _, r := range rs { table.Append([]string{ r.Name, - strings.Join(r.Paths, ","), + prettyPrintPaths(r.Paths), strings.Join(r.KeyIDs, ","), fmt.Sprintf("%v", r.Threshold), }) @@ -198,6 +198,19 @@ func prettyPrintRoles(rs []*data.Role, writer io.Writer, roleType string) { table.Render() } +// Pretty-prints a list of delegation paths, and ensures the empty string is printed as "" in the console +func prettyPrintPaths(paths []string) string { + prettyPaths := []string{} + for _, path := range paths { + // manually escape "" + if path == "" { + path = "\"\"" + } + prettyPaths = append(prettyPaths, path) + } + return strings.Join(prettyPaths, ",") +} + // --- pretty printing certs --- // cert by repo name then expiry time. Don't bother sorting by fingerprint. From bac2d78b9ddd34226fc88b64cd23ef9136f808b6 Mon Sep 17 00:00:00 2001 From: Riyaz Faizullabhoy Date: Thu, 28 Jan 2016 18:06:33 -0800 Subject: [PATCH 2/5] Adds --all-paths flag (requires new TUF delegation key for removes), also print in addition to "" on CLI Signed-off-by: Riyaz Faizullabhoy --- cmd/notary/delegations.go | 54 +++++++++++++---- cmd/notary/integration_test.go | 104 ++++++++++++++++++++++++++++++++- cmd/notary/prettyprint.go | 4 +- 3 files changed, 145 insertions(+), 17 deletions(-) diff --git a/cmd/notary/delegations.go b/cmd/notary/delegations.go index 1838dbaed2..124d63e26e 100644 --- a/cmd/notary/delegations.go +++ b/cmd/notary/delegations.go @@ -42,8 +42,8 @@ type delegationCommander struct { configGetter func() (*viper.Viper, error) retriever passphrase.Retriever - paths []string - removeAll, forceYes bool + paths []string + allPaths, removeAll, forceYes bool } func (d *delegationCommander) GetCommand() *cobra.Command { @@ -52,12 +52,13 @@ func (d *delegationCommander) GetCommand() *cobra.Command { cmdRemDelg := cmdDelegationRemoveTemplate.ToCommand(d.delegationRemove) cmdRemDelg.Flags().StringSliceVar(&d.paths, "paths", nil, "List of paths to remove") - cmdRemDelg.Flags().BoolVarP( - &d.forceYes, "yes", "y", false, "Answer yes to the removal question (no confirmation)") + cmdRemDelg.Flags().BoolVarP(&d.forceYes, "yes", "y", false, "Answer yes to the removal question (no confirmation)") + cmdRemDelg.Flags().BoolVar(&d.allPaths, "all-paths", false, "Remove all paths from this delegation") cmd.AddCommand(cmdRemDelg) cmdAddDelg := cmdDelegationAddTemplate.ToCommand(d.delegationAdd) cmdAddDelg.Flags().StringSliceVar(&d.paths, "paths", nil, "List of paths to add") + cmdAddDelg.Flags().BoolVar(&d.allPaths, "all-paths", false, "Add all paths to this delegation") cmd.AddCommand(cmdAddDelg) return cmd } @@ -121,20 +122,21 @@ func (d *delegationCommander) delegationRemove(cmd *cobra.Command, args []string } // If we're only given the gun and the role, attempt to remove all data for this delegation - if len(args) == 2 && d.paths == nil { + if len(args) == 2 && d.paths == nil && !d.allPaths { d.removeAll = true } keyIDs := []string{} - // Change nil paths to empty slice for TUF - if d.paths == nil { - d.paths = []string{} - } if len(args) > 2 { keyIDs = args[2:] } + // If the user passes --all-paths, don't use any of the passed in --paths + if d.allPaths { + d.paths = nil + } + // no online operations are performed by add so the transport argument // should be nil nRepo, err := notaryclient.NewNotaryRepository( @@ -160,6 +162,12 @@ func (d *delegationCommander) delegationRemove(cmd *cobra.Command, args []string return fmt.Errorf("failed to remove delegation: %v", err) } } else { + if d.allPaths { + err = nRepo.ClearDelegationPaths(role) + if err != nil { + return fmt.Errorf("failed to remove delegation: %v", err) + } + } // Remove any keys or paths that we passed in err = nRepo.RemoveDelegationKeysAndPaths(role, keyIDs, d.paths) if err != nil { @@ -175,6 +183,9 @@ func (d *delegationCommander) delegationRemove(cmd *cobra.Command, args []string if len(keyIDs) > 0 { removingItems = removingItems + fmt.Sprintf("with keys %s, ", keyIDs) } + if d.allPaths { + removingItems = removingItems + "with all paths," + } if d.paths != nil { removingItems = removingItems + fmt.Sprintf("with paths [%s], ", prettyPrintPaths(d.paths)) } @@ -187,7 +198,7 @@ func (d *delegationCommander) delegationRemove(cmd *cobra.Command, args []string // delegationAdd creates a new delegation by adding a public key from a certificate to a specific role in a GUN func (d *delegationCommander) delegationAdd(cmd *cobra.Command, args []string) error { - if len(args) < 2 || len(args) < 3 && d.paths == nil { + if len(args) < 2 || len(args) < 3 && d.paths == nil && !d.allPaths { cmd.Usage() return fmt.Errorf("must specify the Global Unique Name and the role of the delegation along with the public key certificate paths and/or a list of paths to add") } @@ -219,6 +230,18 @@ func (d *delegationCommander) delegationAdd(cmd *cobra.Command, args []string) e } } + for _, path := range d.paths { + if path == "" { + d.allPaths = true + break + } + } + + // If the user passes --all-paths (or gave the "" path in --paths), give the "" path + if d.allPaths { + d.paths = []string{""} + } + // no online operations are performed by add so the transport argument // should be nil nRepo, err := notaryclient.NewNotaryRepository( @@ -244,9 +267,16 @@ func (d *delegationCommander) delegationAdd(cmd *cobra.Command, args []string) e } cmd.Println("") + addingItems := "" + if len(pubKeyIDs) > 0 { + addingItems = addingItems + fmt.Sprintf("with keys %s, ", pubKeys) + } + if d.paths != nil || d.allPaths { + addingItems = addingItems + fmt.Sprintf("with paths [%s], ", prettyPrintPaths(d.paths)) + } cmd.Printf( - "Addition of delegation role %s with keys %s to paths [%s], to repository \"%s\" staged for next publish.\n", - role, pubKeyIDs, prettyPrintPaths(d.paths), gun) + "Addition of delegation role %s %s to repository \"%s\" staged for next publish.\n", + role, addingItems, gun) cmd.Println("") return nil } diff --git a/cmd/notary/integration_test.go b/cmd/notary/integration_test.go index df9eefb9fa..ccb589b4c1 100644 --- a/cmd/notary/integration_test.go +++ b/cmd/notary/integration_test.go @@ -195,11 +195,11 @@ func TestClientDelegationsInteraction(t *testing.T) { assert.NoError(t, err) assert.Contains(t, output, "No delegations present in this repository.") - // add new valid delegation with single new cert, and "" path + // add new valid delegation with single new cert, and no path output, err = runCommand(t, tempDir, "delegation", "add", "gun", "targets/delegation", tempFile.Name()) assert.NoError(t, err) assert.Contains(t, output, "Addition of delegation role") - assert.Contains(t, output, "\"\"") + assert.NotContains(t, output, "path") // check status - see delegation output, err = runCommand(t, tempDir, "status", "gun") @@ -220,12 +220,30 @@ func TestClientDelegationsInteraction(t *testing.T) { assert.NoError(t, err) assert.Contains(t, output, "No unpublished changes for gun") - // list delegations - we should see our added delegation + // list delegations - we should see our added delegation, with no paths output, err = runCommand(t, tempDir, "-s", server.URL, "delegation", "list", "gun") assert.NoError(t, err) assert.Contains(t, output, "targets/delegation") assert.Contains(t, output, keyID) + assert.NotContains(t, output, "\"\"") + + // add all paths to this delegation + output, err = runCommand(t, tempDir, "delegation", "add", "gun", "targets/delegation", "--all-paths") + assert.NoError(t, err) + assert.Contains(t, output, "Addition of delegation role") assert.Contains(t, output, "\"\"") + assert.Contains(t, output, "") + + // publish repo + _, err = runCommand(t, tempDir, "-s", server.URL, "publish", "gun") + assert.NoError(t, err) + + // list delegations - we should see our added delegation, with no paths + output, err = runCommand(t, tempDir, "-s", server.URL, "delegation", "list", "gun") + assert.NoError(t, err) + assert.Contains(t, output, "targets/delegation") + assert.Contains(t, output, "\"\"") + assert.Contains(t, output, "") // Setup another certificate tempFile2, err := ioutil.TempFile("", "pemfile2") @@ -364,6 +382,86 @@ func TestClientDelegationsInteraction(t *testing.T) { assert.Contains(t, output, keyID) assert.Contains(t, output, keyID2) + // Add a bunch of individual paths so we can test a delegation remove --all-paths + output, err = runCommand(t, tempDir, "delegation", "add", "gun", "targets/delegation", "--paths", "abcdef,123456") + assert.NoError(t, err) + + // Add more individual paths so we can test a delegation remove --all-paths + output, err = runCommand(t, tempDir, "delegation", "add", "gun", "targets/delegation", "--paths", "banana,apple,orange,kiwi") + assert.NoError(t, err) + + // publish repo + _, err = runCommand(t, tempDir, "-s", server.URL, "publish", "gun") + assert.NoError(t, err) + + // list delegations - we should see all of our paths + output, err = runCommand(t, tempDir, "-s", server.URL, "delegation", "list", "gun") + assert.NoError(t, err) + assert.Contains(t, output, "abcdef") + assert.Contains(t, output, "123456") + assert.Contains(t, output, "banana") + assert.Contains(t, output, "apple") + assert.Contains(t, output, "orange") + assert.Contains(t, output, "kiwi") + + // Try adding "", and check that adding it with other paths clears out the others + output, err = runCommand(t, tempDir, "delegation", "add", "gun", "targets/delegation", "--paths", "\"\",grapefruit,pomegranate") + assert.NoError(t, err) + + // publish repo + _, err = runCommand(t, tempDir, "-s", server.URL, "publish", "gun") + assert.NoError(t, err) + + // list delegations - we should see all of our old paths, and "" + output, err = runCommand(t, tempDir, "-s", server.URL, "delegation", "list", "gun") + assert.NoError(t, err) + assert.Contains(t, output, "abcdef") + assert.Contains(t, output, "123456") + assert.Contains(t, output, "banana") + assert.Contains(t, output, "apple") + assert.Contains(t, output, "orange") + assert.Contains(t, output, "kiwi") + assert.Contains(t, output, "\"\"") + assert.NotContains(t, output, "grapefruit") + assert.NotContains(t, output, "pomegranate") + + // Try removing just "" + output, err = runCommand(t, tempDir, "delegation", "remove", "gun", "targets/delegation", "--paths", "\"\"") + assert.NoError(t, err) + + // publish repo + _, err = runCommand(t, tempDir, "-s", server.URL, "publish", "gun") + assert.NoError(t, err) + + // list delegations - we should see all of our old paths without "" + output, err = runCommand(t, tempDir, "-s", server.URL, "delegation", "list", "gun") + assert.NoError(t, err) + assert.Contains(t, output, "abcdef") + assert.Contains(t, output, "123456") + assert.Contains(t, output, "banana") + assert.Contains(t, output, "apple") + assert.Contains(t, output, "orange") + assert.Contains(t, output, "kiwi") + assert.NotContains(t, output, "\"\"") + + // Remove --all-paths to clear out all paths from this delegation + output, err = runCommand(t, tempDir, "delegation", "remove", "gun", "targets/delegation", "--all-paths") + assert.NoError(t, err) + + // publish repo + _, err = runCommand(t, tempDir, "-s", server.URL, "publish", "gun") + assert.NoError(t, err) + + // list delegations - we should see all of our paths + output, err = runCommand(t, tempDir, "-s", server.URL, "delegation", "list", "gun") + assert.NoError(t, err) + assert.NotContains(t, output, "abcdef") + assert.NotContains(t, output, "123456") + assert.NotContains(t, output, "banana") + assert.NotContains(t, output, "apple") + assert.NotContains(t, output, "orange") + assert.NotContains(t, output, "kiwi") + // remove by force to delete the delegation entirely output, err = runCommand(t, tempDir, "delegation", "remove", "gun", "targets/delegation", "-y") assert.NoError(t, err) diff --git a/cmd/notary/prettyprint.go b/cmd/notary/prettyprint.go index be750fd2b6..673d8617b0 100644 --- a/cmd/notary/prettyprint.go +++ b/cmd/notary/prettyprint.go @@ -202,9 +202,9 @@ func prettyPrintRoles(rs []*data.Role, writer io.Writer, roleType string) { func prettyPrintPaths(paths []string) string { prettyPaths := []string{} for _, path := range paths { - // manually escape "" + // manually escape "" and designate that it is all paths with an extra print if path == "" { - path = "\"\"" + path = "\"\" " } prettyPaths = append(prettyPaths, path) } From f6c703e44d002b2f99d3c2dfd9e93e782fdebaf3 Mon Sep 17 00:00:00 2001 From: Riyaz Faizullabhoy Date: Fri, 29 Jan 2016 11:47:09 -0800 Subject: [PATCH 3/5] Rename to ClearAllPaths, add comment for delegationAdd Signed-off-by: Riyaz Faizullabhoy --- cmd/notary/delegations.go | 1 + 1 file changed, 1 insertion(+) diff --git a/cmd/notary/delegations.go b/cmd/notary/delegations.go index 124d63e26e..8aec9fd5d4 100644 --- a/cmd/notary/delegations.go +++ b/cmd/notary/delegations.go @@ -198,6 +198,7 @@ func (d *delegationCommander) delegationRemove(cmd *cobra.Command, args []string // delegationAdd creates a new delegation by adding a public key from a certificate to a specific role in a GUN func (d *delegationCommander) delegationAdd(cmd *cobra.Command, args []string) error { + // We must have at least the gun and role name, and at least one key or path (or the --all-paths flag) to add if len(args) < 2 || len(args) < 3 && d.paths == nil && !d.allPaths { cmd.Usage() return fmt.Errorf("must specify the Global Unique Name and the role of the delegation along with the public key certificate paths and/or a list of paths to add") From 7d2b174098abc9c2535a7f1f50eec9c286c6c206 Mon Sep 17 00:00:00 2001 From: Riyaz Faizullabhoy Date: Wed, 3 Feb 2016 15:23:14 -0800 Subject: [PATCH 4/5] adding more complex paths to test Signed-off-by: Riyaz Faizullabhoy --- cmd/notary/delegations.go | 6 +++--- cmd/notary/integration_test.go | 26 +++++++++++++------------- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/cmd/notary/delegations.go b/cmd/notary/delegations.go index 8aec9fd5d4..a61b75cb77 100644 --- a/cmd/notary/delegations.go +++ b/cmd/notary/delegations.go @@ -184,7 +184,7 @@ func (d *delegationCommander) delegationRemove(cmd *cobra.Command, args []string removingItems = removingItems + fmt.Sprintf("with keys %s, ", keyIDs) } if d.allPaths { - removingItems = removingItems + "with all paths," + removingItems = removingItems + "with all paths, " } if d.paths != nil { removingItems = removingItems + fmt.Sprintf("with paths [%s], ", prettyPrintPaths(d.paths)) @@ -198,7 +198,7 @@ func (d *delegationCommander) delegationRemove(cmd *cobra.Command, args []string // delegationAdd creates a new delegation by adding a public key from a certificate to a specific role in a GUN func (d *delegationCommander) delegationAdd(cmd *cobra.Command, args []string) error { - // We must have at least the gun and role name, and at least one key or path (or the --all-paths flag) to add + // We must have at least the gun and role name, and at least one key or path (or the --all-paths flag) to add if len(args) < 2 || len(args) < 3 && d.paths == nil && !d.allPaths { cmd.Usage() return fmt.Errorf("must specify the Global Unique Name and the role of the delegation along with the public key certificate paths and/or a list of paths to add") @@ -276,7 +276,7 @@ func (d *delegationCommander) delegationAdd(cmd *cobra.Command, args []string) e addingItems = addingItems + fmt.Sprintf("with paths [%s], ", prettyPrintPaths(d.paths)) } cmd.Printf( - "Addition of delegation role %s %s to repository \"%s\" staged for next publish.\n", + "Addition of delegation role %s %sto repository \"%s\" staged for next publish.\n", role, addingItems, gun) cmd.Println("") return nil diff --git a/cmd/notary/integration_test.go b/cmd/notary/integration_test.go index ccb589b4c1..b16448f789 100644 --- a/cmd/notary/integration_test.go +++ b/cmd/notary/integration_test.go @@ -387,7 +387,7 @@ func TestClientDelegationsInteraction(t *testing.T) { assert.NoError(t, err) // Add more individual paths so we can test a delegation remove --all-paths - output, err = runCommand(t, tempDir, "delegation", "add", "gun", "targets/delegation", "--paths", "banana,apple,orange,kiwi") + output, err = runCommand(t, tempDir, "delegation", "add", "gun", "targets/delegation", "--paths", "banana/split,apple/crumble/pie,orange.peel,kiwi") assert.NoError(t, err) // publish repo @@ -399,9 +399,9 @@ func TestClientDelegationsInteraction(t *testing.T) { assert.NoError(t, err) assert.Contains(t, output, "abcdef") assert.Contains(t, output, "123456") - assert.Contains(t, output, "banana") - assert.Contains(t, output, "apple") - assert.Contains(t, output, "orange") + assert.Contains(t, output, "banana/split") + assert.Contains(t, output, "apple/crumble/pie") + assert.Contains(t, output, "orange.peel") assert.Contains(t, output, "kiwi") // Try adding "", and check that adding it with other paths clears out the others @@ -417,9 +417,9 @@ func TestClientDelegationsInteraction(t *testing.T) { assert.NoError(t, err) assert.Contains(t, output, "abcdef") assert.Contains(t, output, "123456") - assert.Contains(t, output, "banana") - assert.Contains(t, output, "apple") - assert.Contains(t, output, "orange") + assert.Contains(t, output, "banana/split") + assert.Contains(t, output, "apple/crumble/pie") + assert.Contains(t, output, "orange.peel") assert.Contains(t, output, "kiwi") assert.Contains(t, output, "\"\"") assert.NotContains(t, output, "grapefruit") @@ -438,9 +438,9 @@ func TestClientDelegationsInteraction(t *testing.T) { assert.NoError(t, err) assert.Contains(t, output, "abcdef") assert.Contains(t, output, "123456") - assert.Contains(t, output, "banana") - assert.Contains(t, output, "apple") - assert.Contains(t, output, "orange") + assert.Contains(t, output, "banana/split") + assert.Contains(t, output, "apple/crumble/pie") + assert.Contains(t, output, "orange.peel") assert.Contains(t, output, "kiwi") assert.NotContains(t, output, "\"\"") @@ -457,9 +457,9 @@ func TestClientDelegationsInteraction(t *testing.T) { assert.NoError(t, err) assert.NotContains(t, output, "abcdef") assert.NotContains(t, output, "123456") - assert.NotContains(t, output, "banana") - assert.NotContains(t, output, "apple") - assert.NotContains(t, output, "orange") + assert.NotContains(t, output, "banana/split") + assert.NotContains(t, output, "apple/crumble/pie") + assert.NotContains(t, output, "orange.peel") assert.NotContains(t, output, "kiwi") // remove by force to delete the delegation entirely From f654216b066c9a372f740f80e989138e482c4e5d Mon Sep 17 00:00:00 2001 From: Riyaz Faizullabhoy Date: Wed, 3 Feb 2016 16:45:13 -0800 Subject: [PATCH 5/5] sort paths, more tests with all paths Signed-off-by: Riyaz Faizullabhoy --- cmd/notary/integration_test.go | 38 ++++++++++++++++++++++++++++++++++ cmd/notary/prettyprint.go | 2 ++ cmd/notary/prettyprint_test.go | 2 +- 3 files changed, 41 insertions(+), 1 deletion(-) diff --git a/cmd/notary/integration_test.go b/cmd/notary/integration_test.go index b16448f789..e176556d7e 100644 --- a/cmd/notary/integration_test.go +++ b/cmd/notary/integration_test.go @@ -462,6 +462,44 @@ func TestClientDelegationsInteraction(t *testing.T) { assert.NotContains(t, output, "orange.peel") assert.NotContains(t, output, "kiwi") + // Check that we ignore other --paths if we pass in --all-paths on an add + output, err = runCommand(t, tempDir, "delegation", "add", "gun", "targets/delegation", "--all-paths", "--paths", "grapefruit,pomegranate") + assert.NoError(t, err) + + // publish repo + _, err = runCommand(t, tempDir, "-s", server.URL, "publish", "gun") + assert.NoError(t, err) + + // list delegations - we should only see "", and not the other paths specified + output, err = runCommand(t, tempDir, "-s", server.URL, "delegation", "list", "gun") + assert.NoError(t, err) + assert.Contains(t, output, "\"\"") + assert.NotContains(t, output, "grapefruit") + assert.NotContains(t, output, "pomegranate") + + // Add those extra paths we ignored to set up the next test + output, err = runCommand(t, tempDir, "delegation", "add", "gun", "targets/delegation", "--paths", "grapefruit,pomegranate") + assert.NoError(t, err) + + // publish repo + _, err = runCommand(t, tempDir, "-s", server.URL, "publish", "gun") + assert.NoError(t, err) + + // Check that we ignore other --paths if we pass in --all-paths on a remove + output, err = runCommand(t, tempDir, "delegation", "remove", "gun", "targets/delegation", "--all-paths", "--paths", "pomegranate") + assert.NoError(t, err) + + // publish repo + _, err = runCommand(t, tempDir, "-s", server.URL, "publish", "gun") + assert.NoError(t, err) + + // list delegations - we should see no paths + output, err = runCommand(t, tempDir, "-s", server.URL, "delegation", "list", "gun") + assert.NoError(t, err) + assert.NotContains(t, output, "\"\"") + assert.NotContains(t, output, "grapefruit") + assert.NotContains(t, output, "pomegranate") + // remove by force to delete the delegation entirely output, err = runCommand(t, tempDir, "delegation", "remove", "gun", "targets/delegation", "-y") assert.NoError(t, err) diff --git a/cmd/notary/prettyprint.go b/cmd/notary/prettyprint.go index 673d8617b0..a1d917dcca 100644 --- a/cmd/notary/prettyprint.go +++ b/cmd/notary/prettyprint.go @@ -200,6 +200,8 @@ func prettyPrintRoles(rs []*data.Role, writer io.Writer, roleType string) { // Pretty-prints a list of delegation paths, and ensures the empty string is printed as "" in the console func prettyPrintPaths(paths []string) string { + // sort paths first + sort.Strings(paths) prettyPaths := []string{} for _, path := range paths { // manually escape "" and designate that it is all paths with an extra print diff --git a/cmd/notary/prettyprint_test.go b/cmd/notary/prettyprint_test.go index 496b4c89c5..b47275b5bb 100644 --- a/cmd/notary/prettyprint_test.go +++ b/cmd/notary/prettyprint_test.go @@ -249,7 +249,7 @@ func TestPrettyPrintSortedRoles(t *testing.T) { {"targets/aardvark/unicorn/pony", "rainbows", "135", "1"}, {"targets/bee", "honey", "246", "1"}, {"targets/bee/wasp", "honey/sting", "246,468", "1"}, - {"targets/zebra", "stripes,black,white", "101", "1"}, + {"targets/zebra", "black,stripes,white", "101", "1"}, } lines := strings.Split(strings.TrimSpace(string(text)), "\n")