From 1b6e401a29680b3e8014f442b651687df7492377 Mon Sep 17 00:00:00 2001 From: Martin Vrachev Date: Fri, 21 Jun 2024 15:00:55 +0300 Subject: [PATCH 1/3] Add '--dry-run' option for 'ceremony' and 'sign' The '--dry-run' option scope is regarding the commands output. This means that when the option is used it will not influence the input options. Instead it will directly interfere with the output. If '--dry-run' is used, then the CLI will ignore both '--api-server' admin option and the RSTUF configuration file and it's corresponding SERVER option. Signed-off-by: Martin Vrachev --- repository_service_tuf/cli/admin/ceremony.py | 10 +-- repository_service_tuf/cli/admin/sign.py | 16 ++++- tests/unit/cli/admin/test_ceremony.py | 75 ++++++++++++++------ tests/unit/cli/admin/test_sign.py | 65 +++++++++++++++-- 4 files changed, 136 insertions(+), 30 deletions(-) diff --git a/repository_service_tuf/cli/admin/ceremony.py b/repository_service_tuf/cli/admin/ceremony.py index a0cd7b70..897da6ee 100644 --- a/repository_service_tuf/cli/admin/ceremony.py +++ b/repository_service_tuf/cli/admin/ceremony.py @@ -50,16 +50,18 @@ help=f"Write output json result to FILENAME (default: '{DEFAULT_PATH}')", type=click.File("w"), ) +@click.option("--dry-run", is_flag=True, default=False, help="") @click.pass_context -def ceremony(context: Any, out: Optional[click.File]) -> None: +def ceremony(context: Any, out: Optional[click.File], dry_run: bool) -> None: """Bootstrap Ceremony to create initial root metadata and RSTUF config.""" console.print("\n", Markdown("# Metadata Bootstrap Tool")) settings = context.obj["settings"] # Running online ceremony requires connection to the server and # confirmation that the server is indeed ready for bootstap. - if not settings.get("SERVER") and not out: + if not settings.get("SERVER") and not dry_run: raise click.ClickException( - "Either '--api-sever'/'SERVER' in RSTUF config or '--out' needed" + "Either '--api-sever' admin option/'SERVER' in RSTUF config or " + "'--dry-run' needed" ) # Performs ceremony steps. @@ -114,7 +116,7 @@ def ceremony(context: Any, out: Optional[click.File]) -> None: json.dump(asdict(bootstrap_payload), out, indent=2) # type: ignore console.print(f"Saved result to '{out.name}'") - if settings.get("SERVER"): + if settings.get("SERVER") and not dry_run: task_id = send_payload( settings=settings, url=URL.BOOTSTRAP.value, diff --git a/repository_service_tuf/cli/admin/sign.py b/repository_service_tuf/cli/admin/sign.py index 57bfcc59..d1e4d8ee 100644 --- a/repository_service_tuf/cli/admin/sign.py +++ b/repository_service_tuf/cli/admin/sign.py @@ -83,18 +83,30 @@ def _get_pending_roles(settings: Any) -> Dict[str, Dict[str, Any]]: type=click.File("w"), required=False, ) +@click.option("--dry-run", is_flag=True, default=False, help="") @click.pass_context def sign( context: click.Context, input: Optional[click.File], out: Optional[click.File], + dry_run: bool, ) -> None: """Add one signature to root metadata.""" console.print("\n", Markdown("# Metadata Signing Tool")) settings = context.obj["settings"] + # Make sure there is a way to get a DAS metadata for signing. if settings.get("SERVER") is None and input is None: raise click.ClickException( - "Either '--api-sever'/'SERVER' in RSTUF config or '--in' needed" + "Either '--api-sever' admin option/'SERVER' in RSTUF config or " + "'--in' needed" + ) + + # Make sure user understands that result will be send to the API and if the + # the user wants something else should use '--dry-run'. + if settings.get("SERVER") is None and not dry_run: + raise click.ClickException( + "Either '--api-sever' admin option/'SERVER' in RSTUF config or " + "'--dry-run' needed" ) ########################################################################### # Load roots @@ -150,7 +162,7 @@ def sign( json.dump(asdict(payload), out, indent=2) # type: ignore console.print(f"Saved result to '{out.name}'") - if settings.get("SERVER"): + if settings.get("SERVER") and not dry_run: console.print(f"\nSending signature to {settings.SERVER}") task_id = send_payload( settings, diff --git a/tests/unit/cli/admin/test_ceremony.py b/tests/unit/cli/admin/test_ceremony.py index ed8c6c8b..49ae96d8 100644 --- a/tests/unit/cli/admin/test_ceremony.py +++ b/tests/unit/cli/admin/test_ceremony.py @@ -7,7 +7,7 @@ class TestCeremony: - def test_ceremony_with_custom_out( + def test_ceremony_with_dry_run_and_custom_out( self, monkeypatch, ceremony_inputs, @@ -17,7 +17,10 @@ def test_ceremony_with_custom_out( patch_getpass, patch_utcnow, ): - + """ + Test that '--dry-run' and '--out' are compatible without connecting to + the API. + """ # public keys and signing keys selection options monkeypatch.setattr(f"{_HELPERS}._select", key_selection) @@ -26,7 +29,7 @@ def test_ceremony_with_custom_out( result = invoke_command( ceremony.ceremony, input_step1 + input_step2 + input_step3 + input_step4, - args=["--out", custom_path], + args=["--dry-run", "--out", custom_path], ) with open(_PAYLOADS / "ceremony.json") as f: @@ -37,6 +40,8 @@ def test_ceremony_with_custom_out( assert [s["keyid"] for s in sigs_r] == [s["keyid"] for s in sigs_e] assert result.data == expected + assert f"Saved result to '{custom_path}'" in result.stdout + assert "Bootstrap completed." not in result.stdout def test_ceremony_threshold_less_than_2( self, @@ -65,7 +70,7 @@ def test_ceremony_threshold_less_than_2( result = invoke_command( ceremony.ceremony, input_step1 + input_step2 + input_step3 + input_step4, - [], + ["--dry-run"], ) with open(_PAYLOADS / "ceremony.json") as f: @@ -103,7 +108,7 @@ def test_ceremony__non_positive_expiration( result = invoke_command( ceremony.ceremony, input_step1 + input_step2 + input_step3 + input_step4, - [], + ["--dry-run"], ) with open(_PAYLOADS / "ceremony.json") as f: @@ -132,6 +137,7 @@ def test_ceremony_api_server( monkeypatch.setattr(ceremony, "task_status", fake_task_status) input_step1, input_step2, input_step3, input_step4 = ceremony_inputs test_context["settings"].SERVER = "http://localhost:80" + # public keys and signing keys selection options monkeypatch.setattr(f"{_HELPERS}._select", key_selection) result = invoke_command( @@ -186,6 +192,7 @@ def test_ceremony_api_server_with_out_option( input_step1, input_step2, input_step3, input_step4 = ceremony_inputs test_context["settings"].SERVER = "http://localhost:80" custom_path = "file.json" + # public keys and signing keys selection options monkeypatch.setattr(f"{_HELPERS}._select", key_selection) result = invoke_command( @@ -246,7 +253,7 @@ def test_ceremony_online_key_one_of_root_keys( result = invoke_command( ceremony.ceremony, input_step1 + input_step2 + input_step3 + input_step4, - [], + ["--dry-run"], ) with open(_PAYLOADS / "ceremony.json") as f: @@ -259,20 +266,48 @@ def test_ceremony_online_key_one_of_root_keys( assert result.data == expected assert "Key already in use." in result.stdout - -class TestCeremonyError: - def test_ceremony_no_api_server_and_no_output_option( - self, client, test_context, ceremony_inputs + def test_ceremony_dry_run_with_server_config_set( + self, + monkeypatch, + ceremony_inputs, + key_selection, + client, + test_context, + patch_getpass, + patch_utcnow, ): + """ + Test that '--dry-run' is with higher priority than 'settings.SERVER'. + """ + # public keys and signing keys selection options + monkeypatch.setattr(f"{_HELPERS}._select", key_selection) input_step1, input_step2, input_step3, input_step4 = ceremony_inputs - result = client.invoke( - ceremony.ceremony, - args=[], - input="\n".join( - input_step1 + input_step2 + input_step3 + input_step4 - ), - obj=test_context, - catch_exceptions=False, - ) + test_context["settings"].SERVER = "http://localhost:80" + # We want to test when only "--dry-run" is used we will not save a file + # locally and will not send payload to the API. + # Given that "invoke_command" always saves a file, so the result can be + # read we cannot use it. + with client.isolated_filesystem(): + result = client.invoke( + ceremony.ceremony, + args=["--dry-run"], + input="\n".join( + input_step1 + input_step2 + input_step3 + input_step4 + ), + obj=test_context, + catch_exceptions=False, + ) + + assert result.stderr == "" + assert "Saved result to " not in result.stdout + assert "Bootstrap completed." not in result.stdout + + +class TestCeremonyError: + def test_ceremony_no_api_server_and_no_dry_run_option(self): + result = invoke_command(ceremony.ceremony, [], [], std_err_empty=False) - assert "Either '--api-sever'/'SERVER'" in result.stderr + err_prefix = "Either '--api-sever' admin option/'SERVER'" + err_suffix = "in RSTUF config or '--dry-run' needed" + assert err_prefix in result.stderr + assert err_suffix in result.stderr diff --git a/tests/unit/cli/admin/test_sign.py b/tests/unit/cli/admin/test_sign.py index e5e42882..2875f17d 100644 --- a/tests/unit/cli/admin/test_sign.py +++ b/tests/unit/cli/admin/test_sign.py @@ -143,9 +143,13 @@ def test_sign_bootstrap_root( ) ] - def test_sign_input_option_and_custom_out( + def test_sign_dry_run_and_input_option_and_custom_out( self, monkeypatch, test_context, patch_getpass ): + """ + Test that '--dry-run', '--in' and '--out' are compatible options + without connecting to the API. + """ inputs = [ f"{_PEMS / 'JH.ed25519'}", # Please enter path to encrypted private key # noqa ] @@ -156,7 +160,7 @@ def test_sign_input_option_and_custom_out( ) input_path = f"{_PAYLOADS / 'sign_pending_roles.json'}" custom_out_path = "custom_sign_path.json" - args = ["--in", input_path, "--out", custom_out_path] + args = ["--dry-run", "--in", input_path, "--out", custom_out_path] result = invoke_command( sign.sign, inputs=inputs, args=args, test_context=test_context @@ -171,6 +175,7 @@ def test_sign_input_option_and_custom_out( assert result.data["signature"]["keyid"] == expected["keyid"] assert result.data["signature"]["sig"] == expected["sig"] assert f"Saved result to '{custom_out_path}'" in result.stdout + assert "Metadata Signed and sent to the API" not in result.stdout def test_sign_with_input_option_and_api_server_set( self, monkeypatch, test_context, patch_getpass @@ -222,13 +227,65 @@ def test_sign_with_input_option_and_api_server_set( ) ] + def test_sign_dry_run_with_server_config_set( + self, + monkeypatch, + ceremony_inputs, + client, + test_context, + patch_getpass, + patch_utcnow, + ): + """ + Test that '--dry-run' is with higher priority than 'settings.SERVER'. + """ + # selections interface + monkeypatch.setattr( + f"{_HELPERS}._select", + lambda *a: "JimiHendrix's Key", + ) + sign_input_path = f"{_PAYLOADS / 'sign_pending_roles.json'}" + input_step1, input_step2, input_step3, input_step4 = ceremony_inputs + # We want to test when only "--dry-run" is used we will not save a file + # locally and will not send payload to the API. + # Given that "invoke_command" always saves a file, so the result can be + # read we cannot use it. + with client.isolated_filesystem(): + result = client.invoke( + sign.sign, + args=["--dry-run", "--in", sign_input_path], + input="\n".join( + input_step1 + input_step2 + input_step3 + input_step4 + ), + obj=test_context, + catch_exceptions=False, + ) + + assert result.stderr == "" + assert "Saved result to " not in result.stdout + assert "Bootstrap completed." not in result.stdout + + +class TestSignError: def test_sign_no_api_server_and_no_input_option(self): result = invoke_command(sign.sign, [], [], std_err_empty=False) - assert "Either '--api-sever'/'SERVER'" in result.stderr + err_prefix = "Either '--api-sever' admin option/'SERVER'" + err_suffix = "in RSTUF config or '--in' needed" + assert err_prefix in result.stderr + assert err_suffix in result.stderr + + def test_sign_no_api_server_and_no_dry_run_option(self): + sign_input_path = f"{_PAYLOADS / 'sign_pending_roles.json'}" + args = ["--in", sign_input_path] + result = invoke_command(sign.sign, [], args, std_err_empty=False) + + err_prefix = "Either '--api-sever' admin option/'SERVER'" + err_suffix = "in RSTUF config or '--dry-run'" + assert err_prefix in result.stderr + assert err_suffix in result.stderr -class TestSignError: def test_sign_with_previous_root_but_wrong_version( self, test_context, patch_getpass ): From 1772788e433126e8cc5427c81be05a7859bdccbb Mon Sep 17 00:00:00 2001 From: Martin Vrachev Date: Fri, 21 Jun 2024 16:46:56 +0300 Subject: [PATCH 2/3] Add 'ceremony' and 'sign' commands documentation Signed-off-by: Martin Vrachev --- docs/source/guide/index.rst | 36 +++++++++++++------- repository_service_tuf/cli/admin/ceremony.py | 23 +++++++++++-- repository_service_tuf/cli/admin/sign.py | 26 ++++++++++++-- 3 files changed, 69 insertions(+), 16 deletions(-) diff --git a/docs/source/guide/index.rst b/docs/source/guide/index.rst index 0c996feb..dba6a8cd 100644 --- a/docs/source/guide/index.rst +++ b/docs/source/guide/index.rst @@ -129,12 +129,19 @@ You can do the Ceremony offline. This means on a disconnected computer Usage: rstuf admin ceremony [OPTIONS] - Bootstrap Ceremony to create initial root metadata and RSTUF config. + Perform ceremony and send result to API to trigger bootstrap. + * If `--out [FILENAME]` is passed, result is written to local FILENAME + (in addition to being sent to API). - ╭─ Options ───────────────────────────────────────────────────────────────────────────────────────────╮ - │ --out FILENAME Write output json result to FILENAME (default: 'ceremony-payload.json') │ - │ --help -h Show this message and exit. │ - ╰─────────────────────────────────────────────────────────────────────────────────────────────────────╯ + * If `--dry-run` is passed, result is not sent to API. + You can still pass `--out [FILENAME]` to store the result locally. + The `--api-server` admin option and `SERVER` from config will be ignored. + + ╭─ Options ───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╮ + │ --out FILENAME Write output json result to FILENAME (default: 'ceremony-payload.json') │ + │ --dry-run Run ceremony in dry-run mode without sending result to API. Ignores options and configurations related to API. │ + │ --help -h Show this message and exit. │ + ╰─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯ There are four steps in the ceremony. @@ -180,13 +187,18 @@ sign (``sign``) Usage: rstuf admin metadata sign [OPTIONS] - Add one signature to root metadata. - - ╭─ Options ───────────────────────────────────────────────────────────────────────────────────────────────────────────────╮ - │ --in FILENAME Input file containing the JSON response from the 'GET /api/v1/metadata/sign' RSTUF API endpoint. │ - │ --out FILENAME Write output JSON result to FILENAME (default: 'sign-payload.json') │ - │ --help -h Show this message and exit. │ - ╰─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯ + Perform sign for pending event and send result to API. + * If `--in FILENAME` is passed, input is not read from API but from local FILENAME. + * If `--out [FILENAME]` is passed, result is written to local FILENAME (in addition to being sent to API). + * If `--dry-run` is passed, result is not sent to API. You can still pass `--out [FILENAME]` to store the result locally. + * If `--in` and `--dry-run` is passed, `--api-server` admin option and `SERVER` from config will be ignored. + + ╭─ Options ────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╮ + │ --in FILENAME Input file containing the JSON response from the 'GET /api/v1/metadata/sign' RSTUF API endpoint. │ + │ --out FILENAME Write output JSON result to FILENAME (default: 'sign-payload.json') │ + │ --dry-run Run sign in dry-run mode without sending result to API. Ignores options and configurations related to API. │ + │ --help -h Show this message and exit. │ + ╰──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯ .. rstuf-cli-admin-send diff --git a/repository_service_tuf/cli/admin/ceremony.py b/repository_service_tuf/cli/admin/ceremony.py index 897da6ee..5c36f4ed 100644 --- a/repository_service_tuf/cli/admin/ceremony.py +++ b/repository_service_tuf/cli/admin/ceremony.py @@ -50,10 +50,29 @@ help=f"Write output json result to FILENAME (default: '{DEFAULT_PATH}')", type=click.File("w"), ) -@click.option("--dry-run", is_flag=True, default=False, help="") +@click.option( + "--dry-run", + is_flag=True, + default=False, + help=( + "Run ceremony in dry-run mode without sending result to API. " + "Ignores options and configurations related to API." + ), +) @click.pass_context def ceremony(context: Any, out: Optional[click.File], dry_run: bool) -> None: - """Bootstrap Ceremony to create initial root metadata and RSTUF config.""" + """ + Perform ceremony and send result to API to trigger bootstrap. + + \b + * If `--out [FILENAME]` is passed, result is written to local FILENAME + (in addition to being sent to API). + + \b + * If `--dry-run` is passed, result is not sent to API. + You can still pass `--out [FILENAME]` to store the result locally. + The `--api-server` admin option and `SERVER` from config will be ignored. + """ console.print("\n", Markdown("# Metadata Bootstrap Tool")) settings = context.obj["settings"] # Running online ceremony requires connection to the server and diff --git a/repository_service_tuf/cli/admin/sign.py b/repository_service_tuf/cli/admin/sign.py index d1e4d8ee..b8346880 100644 --- a/repository_service_tuf/cli/admin/sign.py +++ b/repository_service_tuf/cli/admin/sign.py @@ -83,7 +83,15 @@ def _get_pending_roles(settings: Any) -> Dict[str, Dict[str, Any]]: type=click.File("w"), required=False, ) -@click.option("--dry-run", is_flag=True, default=False, help="") +@click.option( + "--dry-run", + is_flag=True, + default=False, + help=( + "Run sign in dry-run mode without sending result to API. " + "Ignores options and configurations related to API." + ), +) @click.pass_context def sign( context: click.Context, @@ -91,7 +99,21 @@ def sign( out: Optional[click.File], dry_run: bool, ) -> None: - """Add one signature to root metadata.""" + """ + Perform sign for pending event and send result to API. + + * If `--in FILENAME` is passed, input is not read from API but from local + FILENAME. + + * If `--out [FILENAME]` is passed, result is written to local FILENAME + (in addition to being sent to API). + + * If `--dry-run` is passed, result is not sent to API. + You can still pass `--out [FILENAME]` to store the result locally. + + * If `--in` and `--dry-run` is passed, `--api-server` admin option and + `SERVER` from config will be ignored. + """ console.print("\n", Markdown("# Metadata Signing Tool")) settings = context.obj["settings"] # Make sure there is a way to get a DAS metadata for signing. From 1c9949168e259d0b44ab5597ac89ede1d442bb85 Mon Sep 17 00:00:00 2001 From: Martin Vrachev Date: Fri, 21 Jun 2024 21:06:21 +0300 Subject: [PATCH 3/3] Tests: use shorter strings to remove the added \n Signed-off-by: Martin Vrachev --- tests/unit/cli/admin/test_ceremony.py | 2 +- tests/unit/cli/admin/test_sign.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/unit/cli/admin/test_ceremony.py b/tests/unit/cli/admin/test_ceremony.py index 49ae96d8..54b39d2a 100644 --- a/tests/unit/cli/admin/test_ceremony.py +++ b/tests/unit/cli/admin/test_ceremony.py @@ -308,6 +308,6 @@ def test_ceremony_no_api_server_and_no_dry_run_option(self): result = invoke_command(ceremony.ceremony, [], [], std_err_empty=False) err_prefix = "Either '--api-sever' admin option/'SERVER'" - err_suffix = "in RSTUF config or '--dry-run' needed" + err_suffix = "or '--dry-run'" assert err_prefix in result.stderr assert err_suffix in result.stderr diff --git a/tests/unit/cli/admin/test_sign.py b/tests/unit/cli/admin/test_sign.py index 2875f17d..667bed8f 100644 --- a/tests/unit/cli/admin/test_sign.py +++ b/tests/unit/cli/admin/test_sign.py @@ -271,7 +271,7 @@ def test_sign_no_api_server_and_no_input_option(self): result = invoke_command(sign.sign, [], [], std_err_empty=False) err_prefix = "Either '--api-sever' admin option/'SERVER'" - err_suffix = "in RSTUF config or '--in' needed" + err_suffix = "or '--in'" assert err_prefix in result.stderr assert err_suffix in result.stderr @@ -282,7 +282,7 @@ def test_sign_no_api_server_and_no_dry_run_option(self): result = invoke_command(sign.sign, [], args, std_err_empty=False) err_prefix = "Either '--api-sever' admin option/'SERVER'" - err_suffix = "in RSTUF config or '--dry-run'" + err_suffix = "or '--dry-run'" assert err_prefix in result.stderr assert err_suffix in result.stderr