From 7944e0cd98ffa57a84efd713ea79da4a1acaebe2 Mon Sep 17 00:00:00 2001 From: vrde Date: Tue, 20 Sep 2016 16:49:56 +0200 Subject: [PATCH 1/7] Allow temporary keypair if no conf file found Closes #482, closes #559 --- Dockerfile | 2 +- bigchaindb/commands/bigchain.py | 22 +++++++++- .../source/server-reference/bigchaindb-cli.md | 5 ++- tests/test_commands.py | 40 ++++++++++++++++++- 4 files changed, 62 insertions(+), 7 deletions(-) diff --git a/Dockerfile b/Dockerfile index 0fcac07f..089c3ffe 100644 --- a/Dockerfile +++ b/Dockerfile @@ -19,7 +19,7 @@ ENV BIGCHAINDB_CONFIG_PATH /data/.bigchaindb ENV BIGCHAINDB_SERVER_BIND 0.0.0.0:9984 ENV BIGCHAINDB_API_ENDPOINT http://bigchaindb:9984/api/v1 -ENTRYPOINT ["bigchaindb", "--experimental-start-rethinkdb"] +ENTRYPOINT ["bigchaindb", "--dev-start-rethinkdb", "--dev-allow-temp-keypair"] CMD ["start"] diff --git a/bigchaindb/commands/bigchain.py b/bigchaindb/commands/bigchain.py index 5edca2c4..55665fc1 100644 --- a/bigchaindb/commands/bigchain.py +++ b/bigchaindb/commands/bigchain.py @@ -157,8 +157,20 @@ def run_drop(args): def run_start(args): """Start the processes to run the node""" logger.info('BigchainDB Version {}'.format(bigchaindb.__version__)) + bigchaindb.config_utils.autoconfigure(filename=args.config, force=True) + if args.allow_temp_keypair: + if not (bigchaindb.config['keypair']['private'] or + bigchaindb.config['keypair']['public']): + + private_key, public_key = crypto.generate_key_pair() + bigchaindb.config['keypair']['private'] = private_key + bigchaindb.config['keypair']['public'] = public_key + else: + logger.warning('Keypair found, no need to create one on the fly.') + + if args.start_rethinkdb: try: proc = utils.start_rethinkdb() @@ -174,7 +186,8 @@ def run_start(args): sys.exit("Can't start BigchainDB, no keypair found. " 'Did you run `bigchaindb configure`?') - logger.info('Starting BigchainDB main process') + logger.info('Starting BigchainDB main process with public key %s', + bigchaindb.config['keypair']['public']) processes.start() @@ -238,11 +251,16 @@ def main(): description='Control your BigchainDB node.', parents=[utils.base_parser]) - parser.add_argument('--experimental-start-rethinkdb', + parser.add_argument('--dev-start-rethinkdb', dest='start_rethinkdb', action='store_true', help='Run RethinkDB on start') + parser.add_argument('--dev-allow-temp-keypair', + dest='allow_temp_keypair', + action='store_true', + help='Generate a random keypair on start') + # all the commands are contained in the subparsers object, # the command selected by the user will be stored in `args.command` # that is used by the `main` function to select which other diff --git a/docs/source/server-reference/bigchaindb-cli.md b/docs/source/server-reference/bigchaindb-cli.md index 15dd4475..3e11446a 100644 --- a/docs/source/server-reference/bigchaindb-cli.md +++ b/docs/source/server-reference/bigchaindb-cli.md @@ -58,8 +58,9 @@ Drop (erase) the RethinkDB database. You will be prompted to make sure. If you w ## bigchaindb start Start BigchainDB. It always begins by trying a `bigchaindb init` first. See the note in the documentation for `bigchaindb init`. -You can also use the `--experimental-start-rethinkdb` command line option to automatically start rethinkdb with bigchaindb if rethinkdb is not already running, -e.g. `bigchaindb --experimental-start-rethinkdb start`. Note that this will also shutdown rethinkdb when the bigchaindb process stops. +You can also use the `--dev-start-rethinkdb` command line option to automatically start rethinkdb with bigchaindb if rethinkdb is not already running, +e.g. `bigchaindb --dev-start-rethinkdb start`. Note that this will also shutdown rethinkdb when the bigchaindb process stops. +The option `--dev-allow-temp-keypair` will generate a keypair on the fly if no keypair is found, this is useful when you want to run a temporary instance of BigchainDB in a Docker container, for example. ## bigchaindb load diff --git a/tests/test_commands.py b/tests/test_commands.py index 587d38f8..215649f8 100644 --- a/tests/test_commands.py +++ b/tests/test_commands.py @@ -64,7 +64,7 @@ def mock_bigchaindb_backup_config(monkeypatch): def test_bigchain_run_start(mock_run_configure, mock_processes_start, mock_db_init_with_existing_db): from bigchaindb.commands.bigchain import run_start - args = Namespace(start_rethinkdb=False, config=None, yes=True) + args = Namespace(start_rethinkdb=False, allow_temp_keypair=False, config=None, yes=True) run_start(args) @@ -74,7 +74,7 @@ def test_bigchain_run_start_with_rethinkdb(mock_start_rethinkdb, mock_processes_start, mock_db_init_with_existing_db): from bigchaindb.commands.bigchain import run_start - args = Namespace(start_rethinkdb=True, config=None, yes=True) + args = Namespace(start_rethinkdb=True, allow_temp_keypair=False, config=None, yes=True) run_start(args) mock_start_rethinkdb.assert_called_with() @@ -229,6 +229,42 @@ def test_start_rethinkdb_exits_when_cannot_start(mock_popen): utils.start_rethinkdb() +@patch('bigchaindb.crypto.generate_key_pair', return_value=('private_key', + 'public_key')) +def test_allow_temp_keypair_generates_one_on_the_fly(mock_gen_keypair, + mock_processes_start, + mock_db_init_with_existing_db): + import bigchaindb + from bigchaindb.commands.bigchain import run_start + + bigchaindb.config['keypair'] = { 'private': None, 'public': None } + + args = Namespace(allow_temp_keypair=True, start_rethinkdb=False, config=None, yes=True) + run_start(args) + + assert bigchaindb.config['keypair']['private'] == 'private_key' + assert bigchaindb.config['keypair']['public'] == 'public_key' + + +@patch('bigchaindb.crypto.generate_key_pair', return_value=('private_key', + 'public_key')) +def test_allow_temp_keypair_doesnt_override_if_keypair_found(mock_gen_keypair, + mock_processes_start, + mock_db_init_with_existing_db): + import bigchaindb + from bigchaindb.commands.bigchain import run_start + + # Preconditions for the test + assert isinstance(bigchaindb.config['keypair']['private'], str) + assert isinstance(bigchaindb.config['keypair']['public'], str) + + args = Namespace(allow_temp_keypair=True, start_rethinkdb=False, config=None, yes=True) + run_start(args) + + assert bigchaindb.config['keypair']['private'] != 'private_key' + assert bigchaindb.config['keypair']['public'] != 'public_key' + + @patch('rethinkdb.ast.Table.reconfigure') def test_set_shards(mock_reconfigure, monkeypatch, b): from bigchaindb.commands.bigchain import run_set_shards From 9b709b7f98e94f2b65e62556f207791defd7bfa3 Mon Sep 17 00:00:00 2001 From: vrde Date: Wed, 21 Sep 2016 00:46:48 +0200 Subject: [PATCH 2/7] Add tests for argparse --- bigchaindb/commands/bigchain.py | 9 ++++----- bigchaindb/commands/utils.py | 8 ++++---- tests/test_commands.py | 28 ++++++++++++++++++++++++++++ 3 files changed, 36 insertions(+), 9 deletions(-) diff --git a/bigchaindb/commands/bigchain.py b/bigchaindb/commands/bigchain.py index 55665fc1..46622867 100644 --- a/bigchaindb/commands/bigchain.py +++ b/bigchaindb/commands/bigchain.py @@ -245,8 +245,7 @@ def run_set_replicas(args): except r.ReqlOpFailedError as e: logger.warn(e) - -def main(): +def create_parser(): parser = argparse.ArgumentParser( description='Control your BigchainDB node.', parents=[utils.base_parser]) @@ -325,8 +324,8 @@ def main(): 'is set, the count is distributed equally to all the ' 'processes') - utils.start(parser, globals()) + return parser -if __name__ == '__main__': - main() +def main(): + utils.start(create_parser(), sys.argv[1:], globals()) diff --git a/bigchaindb/commands/utils.py b/bigchaindb/commands/utils.py index ac2c4d36..b53938d6 100644 --- a/bigchaindb/commands/utils.py +++ b/bigchaindb/commands/utils.py @@ -59,7 +59,7 @@ def start_rethinkdb(): raise StartupError(line) -def start(parser, scope): +def start(parser, argv, scope): """Utility function to execute a subcommand. The function will look up in the ``scope`` @@ -74,11 +74,11 @@ def start(parser, scope): NotImplementedError: if ``scope`` doesn't contain a function called ``run_``. """ - args = parser.parse_args() + args = parser.parse_args(argv) if not args.command: parser.print_help() - return + raise SystemExit() # look up in the current scope for a function called 'run_' # replacing all the dashes '-' with the lowercase character '_' @@ -96,7 +96,7 @@ def start(parser, scope): elif args.multiprocess is None: args.multiprocess = mp.cpu_count() - func(args) + return func(args) base_parser = argparse.ArgumentParser(add_help=False, prog='bigchaindb') diff --git a/tests/test_commands.py b/tests/test_commands.py index 215649f8..6b7d5f53 100644 --- a/tests/test_commands.py +++ b/tests/test_commands.py @@ -62,6 +62,34 @@ def mock_bigchaindb_backup_config(monkeypatch): monkeypatch.setattr('bigchaindb._config', config) +def test_make_sure_we_dont_remove_any_command(): + # thanks to: http://stackoverflow.com/a/18161115/597097 + from bigchaindb.commands.bigchain import utils + from bigchaindb.commands.bigchain import create_parser + parser = create_parser() + + with pytest.raises(SystemExit): + utils.start(parser, [], {}) + + assert parser.parse_args(['configure']).command + assert parser.parse_args(['show-config']).command + assert parser.parse_args(['export-my-pubkey']).command + assert parser.parse_args(['init']).command + assert parser.parse_args(['drop']).command + assert parser.parse_args(['start']).command + assert parser.parse_args(['set-shards', '1']).command + assert parser.parse_args(['set-replicas', '1']).command + assert parser.parse_args(['load']).command + + +@patch('bigchaindb.commands.utils.start') +def test_main_entrypoint(mock_start): + from bigchaindb.commands.bigchain import main + main() + + assert mock_start.called + + def test_bigchain_run_start(mock_run_configure, mock_processes_start, mock_db_init_with_existing_db): from bigchaindb.commands.bigchain import run_start args = Namespace(start_rethinkdb=False, allow_temp_keypair=False, config=None, yes=True) From 5f603f52ef93b804cf42ef705cc15ea908631893 Mon Sep 17 00:00:00 2001 From: vrde Date: Wed, 21 Sep 2016 14:10:04 +0200 Subject: [PATCH 3/7] Add more test coverage for commands --- tests/test_commands.py | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/tests/test_commands.py b/tests/test_commands.py index 6b7d5f53..41736770 100644 --- a/tests/test_commands.py +++ b/tests/test_commands.py @@ -66,6 +66,7 @@ def test_make_sure_we_dont_remove_any_command(): # thanks to: http://stackoverflow.com/a/18161115/597097 from bigchaindb.commands.bigchain import utils from bigchaindb.commands.bigchain import create_parser + parser = create_parser() with pytest.raises(SystemExit): @@ -82,6 +83,32 @@ def test_make_sure_we_dont_remove_any_command(): assert parser.parse_args(['load']).command +def test_start_raises_if_command_not_implemented(): + from bigchaindb.commands.bigchain import utils + from bigchaindb.commands.bigchain import create_parser + + parser = create_parser() + + with pytest.raises(NotImplementedError): + # Will raise because `scope`, the third parameter, + # doesn't contain the function `run_configure` + utils.start(parser, ['configure'], {}) + + +@patch('multiprocessing.cpu_count', return_value=42) +def test_start_sets_multiprocess_var_based_on_cli_args(mock_cpu_count): + def run_load(args): + return args + + from bigchaindb.commands.bigchain import utils + from bigchaindb.commands.bigchain import create_parser + + parser = create_parser() + + assert utils.start(parser, ['load'], {'run_load': run_load}).multiprocess == 1 + assert utils.start(parser, ['load', '--multiprocess'], {'run_load': run_load}).multiprocess == 42 + + @patch('bigchaindb.commands.utils.start') def test_main_entrypoint(mock_start): from bigchaindb.commands.bigchain import main From b2ac60ba3e808f608aacbd7625bb7592b79c4dd8 Mon Sep 17 00:00:00 2001 From: vrde Date: Wed, 21 Sep 2016 14:57:23 +0200 Subject: [PATCH 4/7] Add documentation for missing parameter --- bigchaindb/commands/utils.py | 1 + 1 file changed, 1 insertion(+) diff --git a/bigchaindb/commands/utils.py b/bigchaindb/commands/utils.py index b53938d6..36b78653 100644 --- a/bigchaindb/commands/utils.py +++ b/bigchaindb/commands/utils.py @@ -68,6 +68,7 @@ def start(parser, argv, scope): Args: parser: an ArgumentParser instance. + argv: the list of command line arguments without the script name. scope (dict): map containing (eventually) the functions to be called. Raises: From dea2df9db0abac7d5a2e6c12bb98039a150be888 Mon Sep 17 00:00:00 2001 From: vrde Date: Wed, 21 Sep 2016 15:00:13 +0200 Subject: [PATCH 5/7] Separate test for empty args --- tests/test_commands.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/tests/test_commands.py b/tests/test_commands.py index 41736770..4e16441e 100644 --- a/tests/test_commands.py +++ b/tests/test_commands.py @@ -64,14 +64,10 @@ def mock_bigchaindb_backup_config(monkeypatch): def test_make_sure_we_dont_remove_any_command(): # thanks to: http://stackoverflow.com/a/18161115/597097 - from bigchaindb.commands.bigchain import utils from bigchaindb.commands.bigchain import create_parser parser = create_parser() - with pytest.raises(SystemExit): - utils.start(parser, [], {}) - assert parser.parse_args(['configure']).command assert parser.parse_args(['show-config']).command assert parser.parse_args(['export-my-pubkey']).command @@ -95,6 +91,16 @@ def test_start_raises_if_command_not_implemented(): utils.start(parser, ['configure'], {}) +def test_start_raises_if_no_arguments_given(): + from bigchaindb.commands.bigchain import utils + from bigchaindb.commands.bigchain import create_parser + + parser = create_parser() + + with pytest.raises(SystemExit): + utils.start(parser, [], {}) + + @patch('multiprocessing.cpu_count', return_value=42) def test_start_sets_multiprocess_var_based_on_cli_args(mock_cpu_count): def run_load(args): From 989a943fea30d9b042bfe0a731b74aa2cc144594 Mon Sep 17 00:00:00 2001 From: vrde Date: Wed, 21 Sep 2016 15:01:42 +0200 Subject: [PATCH 6/7] Move func definition after imports --- tests/test_commands.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/test_commands.py b/tests/test_commands.py index 4e16441e..8ee79008 100644 --- a/tests/test_commands.py +++ b/tests/test_commands.py @@ -103,12 +103,12 @@ def test_start_raises_if_no_arguments_given(): @patch('multiprocessing.cpu_count', return_value=42) def test_start_sets_multiprocess_var_based_on_cli_args(mock_cpu_count): - def run_load(args): - return args - from bigchaindb.commands.bigchain import utils from bigchaindb.commands.bigchain import create_parser + def run_load(args): + return args + parser = create_parser() assert utils.start(parser, ['load'], {'run_load': run_load}).multiprocess == 1 From 27f585f39d26ad39e36743901a0ed2fc737e755a Mon Sep 17 00:00:00 2001 From: vrde Date: Wed, 21 Sep 2016 15:09:54 +0200 Subject: [PATCH 7/7] Improve test to check if original vals are kept --- tests/test_commands.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/tests/test_commands.py b/tests/test_commands.py index 8ee79008..4efa8f96 100644 --- a/tests/test_commands.py +++ b/tests/test_commands.py @@ -316,14 +316,17 @@ def test_allow_temp_keypair_doesnt_override_if_keypair_found(mock_gen_keypair, from bigchaindb.commands.bigchain import run_start # Preconditions for the test - assert isinstance(bigchaindb.config['keypair']['private'], str) - assert isinstance(bigchaindb.config['keypair']['public'], str) + original_private_key = bigchaindb.config['keypair']['private'] + original_public_key = bigchaindb.config['keypair']['public'] + + assert isinstance(original_public_key, str) + assert isinstance(original_private_key, str) args = Namespace(allow_temp_keypair=True, start_rethinkdb=False, config=None, yes=True) run_start(args) - assert bigchaindb.config['keypair']['private'] != 'private_key' - assert bigchaindb.config['keypair']['public'] != 'public_key' + assert bigchaindb.config['keypair']['private'] == original_private_key + assert bigchaindb.config['keypair']['public'] == original_public_key @patch('rethinkdb.ast.Table.reconfigure')