Change the CLI for rotate key to require a role type

Signed-off-by: Ying Li <ying.li@docker.com>

cmd/notary/integration_test.go

@@ -585,7 +585,7 @@ func TestClientDelegationsPublishing(t *testing.T) {
 	assertNumKeys(t, tempDir, 1, 2, true)
 
 	// rotate the snapshot key to server
-	output, err = runCommand(t, tempDir, "-s", server.URL, "key", "rotate", "gun", "-r", "--key-type", "snapshot")
+	output, err = runCommand(t, tempDir, "-s", server.URL, "key", "rotate", "gun", "snapshot", "-r")
 	assert.NoError(t, err)
 
 	// publish repo
@@ -866,7 +866,9 @@ func TestClientKeyGenerationRotation(t *testing.T) {
 	assertSuccessfullyPublish(t, tempDir, server.URL, "gun", target, tempfiles[0])
 
 	// rotate the signing keys
-	_, err = runCommand(t, tempDir, "key", "rotate", "gun")
+	_, err = runCommand(t, tempDir, "key", "rotate", "gun", data.CanonicalSnapshotRole)
+	assert.NoError(t, err)
+	_, err = runCommand(t, tempDir, "key", "rotate", "gun", data.CanonicalTargetsRole)
 	assert.NoError(t, err)
 	root, sign := assertNumKeys(t, tempDir, 1, 4, true)
 	assert.Equal(t, origRoot[0], root[0])

cmd/notary/keys.go

@@ -16,11 +16,12 @@ import (
 	"github.com/docker/notary/passphrase"
 	"github.com/docker/notary/trustmanager"
 
+	"io/ioutil"
+
 	"github.com/docker/notary"
 	"github.com/docker/notary/tuf/data"
 	"github.com/spf13/cobra"
 	"github.com/spf13/viper"
-	"io/ioutil"
 )
 
 var cmdKeyTemplate = usageTemplate{
@@ -36,9 +37,9 @@ var cmdKeyListTemplate = usageTemplate{
 }
 
 var cmdRotateKeyTemplate = usageTemplate{
-	Use:   "rotate [ GUN ]",
-	Short: "Rotate the signing (non-root) keys for the given Globally Unique Name.",
-	Long:  "Removes old signing (non-root) keys for the given Globally Unique Name, and generates new ones.  If rotating to a server-managed key, the key rotation is automatically published.  If rotating to locally-managed key(s), only local, non-online changes are made - please use then `notary publish` to push the key rotation changes to the remote server.",
+	Use:   "rotate [ GUN ] [ key role ]",
+	Short: "Rotate a signing (non-root) key of the given type for the given Globally Unique Name and role.",
+	Long:  "Generates a new signing key (non-root) for the given Globally Unique Name and role.  Rotating to a server-managed key is an online-only operation: a new key is requested from the server rather than generated, and if successful, the key rotation is immediately published.  Rotating to a locally-managed key is an offline operation only: `notary publish` must be executed manually afterward to publish to the remote server.\nThe role must be one of \"snapshot\", \"targets\", or \"timestamp\".",
 }
 
 var cmdKeyGenerateRootKeyTemplate = usageTemplate{
@@ -127,11 +128,7 @@ func (k *keyCommander) GetCommand() *cobra.Command {
 	cmdRotateKey.Flags().BoolVarP(&k.rotateKeyServerManaged, "server-managed", "r",
 		false, "Signing and key management will be handled by the remote server. "+
 			"(no key will be generated or stored locally) "+
-			"Can only be used in conjunction with --key-type.")
-	cmdRotateKey.Flags().StringVarP(&k.rotateKeyRole, "key-type", "t", "",
-		`Key type to rotate.  Supported values: "targets", "snapshot". `+
-			`If not provided, both targets and snapshot keys will be rotated, `+
-			`and the new keys will be locally generated and stored.`)
+			"Can only be used for timestamp and snapshot roles, and MUST be used for the timestamp role.")
 	cmd.AddCommand(cmdRotateKey)
 
 	return cmd
@@ -406,24 +403,9 @@ func (k *keyCommander) keysImport(cmd *cobra.Command, args []string) error {
 }
 
 func (k *keyCommander) keysRotate(cmd *cobra.Command, args []string) error {
-	if len(args) < 1 {
+	if len(args) < 2 {
 		cmd.Usage()
-		return fmt.Errorf("Must specify a GUN")
-	}
-	rotateKeyRole := strings.ToLower(k.rotateKeyRole)
-
-	var rolesToRotate []string
-	switch rotateKeyRole {
-	case "":
-		rolesToRotate = []string{data.CanonicalSnapshotRole, data.CanonicalTargetsRole}
-	case data.CanonicalSnapshotRole:
-		rolesToRotate = []string{data.CanonicalSnapshotRole}
-	case data.CanonicalTargetsRole:
-		rolesToRotate = []string{data.CanonicalTargetsRole}
-	case data.CanonicalTimestampRole:
-		rolesToRotate = []string{data.CanonicalTimestampRole}
-	default:
-		return fmt.Errorf("key rotation not supported for %s keys", k.rotateKeyRole)
+		return fmt.Errorf("Must specify a GUN and a key role to rotate")
 	}
 
 	config, err := k.configGetter()
@@ -432,6 +414,8 @@ func (k *keyCommander) keysRotate(cmd *cobra.Command, args []string) error {
 	}
 
 	gun := args[0]
+	rotateKeyRole := args[1]
+
 	var rt http.RoundTripper
 	if k.rotateKeyServerManaged {
 		// this does not actually push the changes, just creates the keys, but
@@ -447,12 +431,7 @@ func (k *keyCommander) keysRotate(cmd *cobra.Command, args []string) error {
 	if err != nil {
 		return err
 	}
-	for _, role := range rolesToRotate {
-		if err := nRepo.RotateKey(role, k.rotateKeyServerManaged); err != nil {
-			return err
-		}
-	}
-	return nil
+	return nRepo.RotateKey(rotateKeyRole, k.rotateKeyServerManaged)
 }
 
 func removeKeyInteractively(keyStores []trustmanager.KeyStore, keyID string,

cmd/notary/keys_test.go

@@ -248,10 +248,14 @@ func TestRotateKeyInvalidRoles(t *testing.T) {
 				rotateKeyRole:          role,
 				rotateKeyServerManaged: serverManaged,
 			}
-			err := k.keysRotate(&cobra.Command{}, []string{"gun"})
+			commands := []string{"gun", role}
+			if serverManaged {
+				commands = append(commands, "-r")
+			}
+			err := k.keysRotate(&cobra.Command{}, commands)
 			assert.Error(t, err)
 			assert.Contains(t, err.Error(),
-				fmt.Sprintf("key rotation not supported for %s keys", role))
+				fmt.Sprintf("does not currently permit rotating the %s key", role))
 		}
 	}
 }
@@ -264,7 +268,7 @@ func TestRotateKeyTargetCannotBeServerManaged(t *testing.T) {
 		rotateKeyRole:          data.CanonicalTargetsRole,
 		rotateKeyServerManaged: true,
 	}
-	err := k.keysRotate(&cobra.Command{}, []string{"gun"})
+	err := k.keysRotate(&cobra.Command{}, []string{"gun", data.CanonicalTargetsRole})
 	assert.Error(t, err)
 	assert.IsType(t, client.ErrInvalidRemoteRole{}, err)
 }
@@ -277,7 +281,7 @@ func TestRotateKeyTimestampCannotBeLocallyManaged(t *testing.T) {
 		rotateKeyRole:          data.CanonicalTimestampRole,
 		rotateKeyServerManaged: false,
 	}
-	err := k.keysRotate(&cobra.Command{}, []string{"gun"})
+	err := k.keysRotate(&cobra.Command{}, []string{"gun", data.CanonicalTimestampRole})
 	assert.Error(t, err)
 	assert.IsType(t, client.ErrInvalidLocalRole{}, err)
 }
@@ -353,11 +357,9 @@ func TestRotateKeyRemoteServerManagesKey(t *testing.T) {
 				return v, nil
 			},
 			getRetriever:           func() passphrase.Retriever { return ret },
-			rotateKeyRole:          role,
 			rotateKeyServerManaged: true,
 		}
-		err = k.keysRotate(&cobra.Command{}, []string{gun})
-		assert.NoError(t, err)
+		assert.NoError(t, k.keysRotate(&cobra.Command{}, []string{gun, role}))
 
 		repo, err := client.NewNotaryRepository(tempBaseDir, gun, ts.URL, http.DefaultTransport, ret)
 		assert.NoError(t, err, "error creating repo: %s", err)
@@ -391,8 +393,7 @@ func TestRotateKeyRemoteServerManagesKey(t *testing.T) {
 }
 
 // The command line uses NotaryRepository's RotateKey - this is just testing
-// that the correct config variables are passed for the client to rotate
-// both the targets and snapshot key, and create them locally
+// that multiple keys can be rotated at once locally
 func TestRotateKeyBothKeys(t *testing.T) {
 	// Temporary directory where test files will be created
 	tempBaseDir, err := ioutil.TempDir("/tmp", "notary-test-")
@@ -415,8 +416,8 @@ func TestRotateKeyBothKeys(t *testing.T) {
 		},
 		getRetriever: func() passphrase.Retriever { return ret },
 	}
-	err = k.keysRotate(&cobra.Command{}, []string{gun})
-	assert.NoError(t, err)
+	assert.NoError(t, k.keysRotate(&cobra.Command{}, []string{gun, data.CanonicalTargetsRole}))
+	assert.NoError(t, k.keysRotate(&cobra.Command{}, []string{gun, data.CanonicalSnapshotRole}))
 
 	repo, err := client.NewNotaryRepository(tempBaseDir, gun, ts.URL, nil, ret)
 	assert.NoError(t, err, "error creating repo: %s", err)

cmd/notary/main_test.go

@@ -105,7 +105,7 @@ var exampleValidCommands = []string{
 	"add repo v1 somefile",
 	"verify repo v1",
 	"key list",
-	"key rotate repo",
+	"key rotate repo snapshot",
 	"key generate rsa",
 	"key backup tempfile.zip",
 	"key export e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855 backup.pem",