From bac2d78b9ddd34226fc88b64cd23ef9136f808b6 Mon Sep 17 00:00:00 2001 From: Riyaz Faizullabhoy Date: Thu, 28 Jan 2016 18:06:33 -0800 Subject: [PATCH] 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) }