From 6f6f04543b07fcae08fd8b516082672d730ba8cd Mon Sep 17 00:00:00 2001 From: Andrew Gillis Date: Thu, 28 Jan 2021 12:15:49 -0800 Subject: [PATCH] Improve error message when running key command that locks repo (#7821) * Improve error message when running key commands that must be run when the daemon is not already running Fixes Issue #7814 `ipfs key export` now does a PreRun check like `ipfs key rotate` was to give a better error to the user then "someone else has the lock" in the event that the daemon is running while trying to execute these offline-only commands. While unlikely the "someone else has the lock" error can still be shown if two processes try and grab the repo lock at the same time. This PreRun function is also exported so it can be used by `ipfs init` where it was originally copied from. * Added more `ipfs key` command tests When daemon is running: - Test that import works - Test that export fails - Test that rotate fails --- cmd/ipfs/init.go | 17 +-------------- core/commands/keystore.go | 37 +++++++++++++++++++-------------- test/sharness/t0165-keystore.sh | 18 ++++++++++++++++ 3 files changed, 40 insertions(+), 32 deletions(-) diff --git a/cmd/ipfs/init.go b/cmd/ipfs/init.go index 0842b7197..f419604f2 100644 --- a/cmd/ipfs/init.go +++ b/cmd/ipfs/init.go @@ -69,22 +69,7 @@ environment variable: }, NoRemote: true, Extra: commands.CreateCmdExtras(commands.SetDoesNotUseRepo(true), commands.SetDoesNotUseConfigAsInput(true)), - PreRun: func(req *cmds.Request, env cmds.Environment) error { - cctx := env.(*oldcmds.Context) - daemonLocked, err := fsrepo.LockedByOtherProcess(cctx.ConfigRoot) - if err != nil { - return err - } - - log.Info("checking if daemon is running...") - if daemonLocked { - log.Debug("ipfs daemon is running") - e := "ipfs daemon is running. please stop it to run this command" - return cmds.ClientError(e) - } - - return nil - }, + PreRun: commands.DaemonNotRunning, Run: func(req *cmds.Request, res cmds.ResponseEmitter, env cmds.Environment) error { cctx := env.(*oldcmds.Context) empty, _ := req.Options[emptyRepoOptionName].(bool) diff --git a/core/commands/keystore.go b/core/commands/keystore.go index 5f2388dde..9b86ba36b 100644 --- a/core/commands/keystore.go +++ b/core/commands/keystore.go @@ -150,6 +150,7 @@ path can be specified with '--output=' or '-o='. cmds.StringOption(outputOptionName, "o", "The path where the output should be stored."), }, NoRemote: true, + PreRun: DaemonNotRunning, Run: func(req *cmds.Request, res cmds.ResponseEmitter, env cmds.Environment) error { name := req.Arguments[0] @@ -459,22 +460,7 @@ environment variable: cmds.IntOption(keyStoreSizeOptionName, "s", "size of the key to generate"), }, NoRemote: true, - PreRun: func(req *cmds.Request, env cmds.Environment) error { - cctx := env.(*oldcmds.Context) - daemonLocked, err := fsrepo.LockedByOtherProcess(cctx.ConfigRoot) - if err != nil { - return err - } - - log.Info("checking if daemon is running...") - if daemonLocked { - log.Debug("ipfs daemon is running") - e := "ipfs daemon is running. please stop it to run this command" - return cmds.ClientError(e) - } - - return nil - }, + PreRun: DaemonNotRunning, Run: func(req *cmds.Request, res cmds.ResponseEmitter, env cmds.Environment) error { cctx := env.(*oldcmds.Context) nBitsForKeypair, nBitsGiven := req.Options[keyStoreSizeOptionName].(int) @@ -556,3 +542,22 @@ func keyOutputListEncoders() cmds.EncoderFunc { return nil }) } + +// DaemonNotRunning checks to see if the ipfs repo is locked, indicating that +// the daemon is running, and returns and error if the daemon is running. +func DaemonNotRunning(req *cmds.Request, env cmds.Environment) error { + cctx := env.(*oldcmds.Context) + daemonLocked, err := fsrepo.LockedByOtherProcess(cctx.ConfigRoot) + if err != nil { + return err + } + + log.Info("checking if daemon is running...") + if daemonLocked { + log.Debug("ipfs daemon is running") + e := "ipfs daemon is running. please stop it to run this command" + return cmds.ClientError(e) + } + + return nil +} diff --git a/test/sharness/t0165-keystore.sh b/test/sharness/t0165-keystore.sh index 07a9ad9fe..0bbbe91ac 100755 --- a/test/sharness/t0165-keystore.sh +++ b/test/sharness/t0165-keystore.sh @@ -167,6 +167,24 @@ ipfs key rm key_ed25519 test_must_fail ipfs key rename -f fooed self 2>&1 | tee key_rename_out && grep -q "Error: cannot overwrite key with name" key_rename_out ' + + test_launch_ipfs_daemon + + test_expect_success "online import rsa key" ' + ipfs key import generated_rsa_key generated_rsa_key.key > roundtrip_rsa_key_id && + test_cmp rsa_key_id roundtrip_rsa_key_id + ' + + test_must_fail "online export rsa key" ' + ipfs key export generated_rsa_key + ' + + test_must_fail "online rotate rsa key" ' + ipfs key rotate + ' + + test_kill_ipfs_daemon + } test_check_rsa2048_sk() { -- GitLab