Merge pull request #1723 from smartcontractkit/fix-cli-delete

Fix keystore delete confirmation

Author: connorwstein

keystore/cli/cli.go

@@ -177,7 +177,7 @@ func NewDeleteCmd() *cobra.Command {
 				}
 				reader := bufio.NewReader(cmd.InOrStdin())
 				line, _ := reader.ReadString('\n')
-				if strings.TrimSpace(line) != "yes\n" {
+				if strings.TrimSpace(line) != "yes" {
 					return errors.New("delete aborted by user")
 				}
 			}

keystore/cli/cli_test.go

@@ -15,7 +15,6 @@ import (
 )
 
 func TestCLI(t *testing.T) {
-	cmd := cli.NewRootCmd()
 	tempDir := t.TempDir()
 	defer os.RemoveAll(tempDir)
 	keystoreFile := filepath.Join(tempDir, "keystore.json")
@@ -26,70 +25,58 @@ func TestCLI(t *testing.T) {
 	os.Setenv("KEYSTORE_PASSWORD", "testpassword")
 
 	// No error just listing help.
-	buf := new(bytes.Buffer)
-	cmd.SetOutput(buf)
-	cmd.SetErr(buf)
-	cmd.SetArgs([]string{})
-	require.NoError(t, cmd.ExecuteContext(t.Context()))
+	_, err = runCommand(t, nil, "")
+	require.NoError(t, err)
 
 	// Create a key.
-	buf.Reset()
-	cmd.SetArgs([]string{"create", "-d", `{"Keys": [{"KeyName": "testkey", "KeyType": "X25519"}]}`})
-	require.NoError(t, cmd.ExecuteContext(t.Context()))
+	_, err = runCommand(t, nil, "create", "-d", `{"Keys": [{"KeyName": "testkey", "KeyType": "X25519"}]}`)
+	require.NoError(t, err)
 
 	// List keys.
-	buf.Reset()
-	cmd.SetArgs([]string{"get", "-d", `{"KeyNames": ["testkey"]}`})
-	require.NoError(t, cmd.ExecuteContext(t.Context()))
+	out, err := runCommand(t, nil, "get", "-d", `{"KeyNames": ["testkey"]}`)
+	require.NoError(t, err)
 	resp := ks.GetKeysResponse{}
-	err = json.Unmarshal(buf.Bytes(), &resp)
+	err = json.Unmarshal(out.Bytes(), &resp)
 	require.NoError(t, err)
 	require.Len(t, resp.Keys, 1)
 	require.Equal(t, "testkey", resp.Keys[0].KeyInfo.Name)
 	require.Equal(t, ks.X25519, resp.Keys[0].KeyInfo.KeyType)
 
 	// Create a second key we export.
-	buf.Reset()
-	cmd.SetArgs([]string{"create", "-d", `{"Keys": [{"KeyName": "testkey2", "KeyType": "ECDSA_S256"}]}`})
-	require.NoError(t, cmd.ExecuteContext(t.Context()))
+	_, err = runCommand(t, nil, "create", "-d", `{"Keys": [{"KeyName": "testkey2", "KeyType": "ECDSA_S256"}]}`)
+	require.NoError(t, err)
 
 	// Export the second key.
-	buf.Reset()
-	cmd.SetArgs([]string{"export", "-d", `{"Keys": [{"KeyName": "testkey2", "Enc": {"Password": "testpassword2", "ScryptParams": {"N": 1024, "P": 1, "R": 8}}}]}`})
-	require.NoError(t, cmd.ExecuteContext(t.Context()))
+	out, err = runCommand(t, nil, "export", "-d", `{"Keys": [{"KeyName": "testkey2", "Enc": {"Password": "testpassword2", "ScryptParams": {"N": 1024, "P": 1, "R": 8}}}]}`)
+	require.NoError(t, err)
 	exportResp := ks.ExportKeysResponse{}
-	err = json.Unmarshal(buf.Bytes(), &exportResp)
+	err = json.Unmarshal(out.Bytes(), &exportResp)
 	require.NoError(t, err)
 	exportedKey2Data := base64.StdEncoding.EncodeToString(exportResp.Keys[0].Data)
 
 	// Delete the second key.
-	buf.Reset()
-	// Force deletion without confirmation.
-	cmd.SetArgs([]string{"delete", "-d", `{"KeyNames": ["testkey2"]}`, "--yes"})
-	require.NoError(t, cmd.ExecuteContext(t.Context()))
+	_, err = runCommand(t, nil, "delete", "-d", `{"KeyNames": ["testkey2"]}`, "--yes")
+	require.NoError(t, err)
 
 	// List key should only see first.
-	buf.Reset()
-	cmd.SetArgs([]string{"list"})
-	require.NoError(t, cmd.ExecuteContext(t.Context()))
+	out, err = runCommand(t, nil, "list")
+	require.NoError(t, err)
 	resp = ks.GetKeysResponse{}
-	err = json.Unmarshal(buf.Bytes(), &resp)
+	err = json.Unmarshal(out.Bytes(), &resp)
 	require.NoError(t, err)
 	require.Len(t, resp.Keys, 1)
 	require.Equal(t, "testkey", resp.Keys[0].KeyInfo.Name)
 	require.Equal(t, ks.X25519, resp.Keys[0].KeyInfo.KeyType)
 
 	// Import the exported key.
-	buf.Reset()
-	cmd.SetArgs([]string{"import", "-d", `{"Keys": [{"KeyName": "testkey2", "Data": "` + exportedKey2Data + `", "Password": "testpassword2"}]}`})
-	require.NoError(t, cmd.ExecuteContext(t.Context()))
+	_, err = runCommand(t, nil, "import", "-d", `{"Keys": [{"KeyName": "testkey2", "Data": "`+exportedKey2Data+`", "Password": "testpassword2"}]}`)
+	require.NoError(t, err)
 
 	// List keys.
-	buf.Reset()
-	cmd.SetArgs([]string{"list"})
-	require.NoError(t, cmd.ExecuteContext(t.Context()))
+	out, err = runCommand(t, nil, "list")
+	require.NoError(t, err)
 	resp = ks.GetKeysResponse{}
-	err = json.Unmarshal(buf.Bytes(), &resp)
+	err = json.Unmarshal(out.Bytes(), &resp)
 	require.NoError(t, err)
 	require.Len(t, resp.Keys, 2)
 	require.Equal(t, "testkey", resp.Keys[0].KeyInfo.Name)
@@ -98,20 +85,46 @@ func TestCLI(t *testing.T) {
 	require.Equal(t, ks.ECDSA_S256, resp.Keys[1].KeyInfo.KeyType)
 
 	// Set metadata on testkey.
-	buf.Reset()
 	metadata := base64.StdEncoding.EncodeToString([]byte("my-custom-metadata"))
-	cmd.SetArgs([]string{"set-metadata", "-d", `{"Updates": [{"KeyName": "testkey", "Metadata": "` + metadata + `"}]}`})
-	require.NoError(t, cmd.ExecuteContext(t.Context()))
+	_, err = runCommand(t, nil, "set-metadata", "-d", `{"Updates": [{"KeyName": "testkey", "Metadata": "`+metadata+`"}]}`)
+	require.NoError(t, err)
 
 	// Verify metadata was set.
-	buf.Reset()
-	cmd.SetArgs([]string{"get", "-d", `{"KeyNames": ["testkey"]}`})
-	require.NoError(t, cmd.ExecuteContext(t.Context()))
+	out, err = runCommand(t, nil, "get", "-d", `{"KeyNames": ["testkey"]}`)
+	require.NoError(t, err)
 	resp = ks.GetKeysResponse{}
-	err = json.Unmarshal(buf.Bytes(), &resp)
+	err = json.Unmarshal(out.Bytes(), &resp)
 	require.NoError(t, err)
 	require.Len(t, resp.Keys, 1)
 	require.Equal(t, "testkey", resp.Keys[0].KeyInfo.Name)
 	// Metadata is []byte, Go's JSON unmarshaler automatically decodes base64 strings
 	require.Equal(t, "my-custom-metadata", string(resp.Keys[0].KeyInfo.Metadata))
+
+	// Delete the keys with confirmation.
+	out, err = runCommand(t, bytes.NewBufferString("yes\n"), "delete", "-d", `{"KeyNames": ["testkey", "testkey2"]}`)
+	require.NoError(t, err)
+	t.Log("out", out.String())
+
+	// List keys should be empty.
+	out, err = runCommand(t, nil, "list")
+	require.NoError(t, err)
+	resp = ks.GetKeysResponse{}
+	err = json.Unmarshal(out.Bytes(), &resp)
+	require.NoError(t, err)
+	require.Empty(t, resp.Keys)
+}
+
+func runCommand(t *testing.T, in *bytes.Buffer, args ...string) (bytes.Buffer, error) {
+	// Cobra commands are stateful which can cause subtle bugs if not reset.
+	// For simplicity just create a fresh object.
+	cmd := cli.NewRootCmd()
+	buf := new(bytes.Buffer)
+	cmd.SetOutput(buf)
+	cmd.SetArgs(args)
+	cmd.SetIn(in)
+	err := cmd.ExecuteContext(t.Context())
+	if err != nil {
+		return bytes.Buffer{}, err
+	}
+	return *buf, nil
 }