Merge pull request #514 from docker/delg-empty-path

Ensure empty string path is properly handled, add --all-paths flag
This commit is contained in:
Diogo Mónica 2016-02-03 17:11:11 -08:00
commit 78dda3d16d
4 changed files with 210 additions and 21 deletions

View File

@ -42,8 +42,8 @@ type delegationCommander struct {
configGetter func() (*viper.Viper, error)
retriever passphrase.Retriever
paths []string
removeAll, removeYes 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.removeYes, "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(
@ -146,7 +148,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.")
@ -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 {
@ -171,9 +179,17 @@ 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.allPaths {
removingItems = removingItems + "with all paths, "
}
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("")
@ -182,7 +198,8 @@ 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 {
// 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")
}
@ -214,6 +231,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(
@ -239,9 +268,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 and paths %s, to repository \"%s\" staged for next publish.\n",
role, pubKeyIDs, d.paths, gun)
"Addition of delegation role %s %sto repository \"%s\" staged for next publish.\n",
role, addingItems, gun)
cmd.Println("")
return nil
}

View File

@ -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 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.NotContains(t, output, "path")
// check status - see delegation
output, err = runCommand(t, tempDir, "status", "gun")
@ -219,11 +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, "<all paths>")
// 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, "<all paths>")
// Setup another certificate
tempFile2, err := ioutil.TempFile("", "pemfile2")
@ -246,7 +266,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")
@ -362,6 +382,124 @@ 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/split,apple/crumble/pie,orange.peel,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/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
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/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")
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/split")
assert.Contains(t, output, "apple/crumble/pie")
assert.Contains(t, output, "orange.peel")
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/split")
assert.NotContains(t, output, "apple/crumble/pie")
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)

View File

@ -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,21 @@ 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 {
// 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 <all paths>
if path == "" {
path = "\"\" <all paths>"
}
prettyPaths = append(prettyPaths, path)
}
return strings.Join(prettyPaths, ",")
}
// --- pretty printing certs ---
// cert by repo name then expiry time. Don't bother sorting by fingerprint.

View File

@ -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")