From b7d2bda7baffd1fec58e80f25a26b7f80a9df541 Mon Sep 17 00:00:00 2001 From: Martin Vrachev Date: Wed, 5 Jun 2024 19:31:05 +0300 Subject: [PATCH] Make ceremony "output" and sign "save" consistent Rename the ceremony "output" argument and sign "--save" option to become consistent using the same option "--out". Signed-off-by: Martin Vrachev --- repository_service_tuf/cli/admin/ceremony.py | 21 ++++++++-------- repository_service_tuf/cli/admin/sign.py | 14 +++++------ tests/conftest.py | 12 ++++------ tests/unit/cli/admin/test_ceremony.py | 25 ++++++++++++++++---- tests/unit/cli/admin/test_sign.py | 4 ++-- 5 files changed, 44 insertions(+), 32 deletions(-) diff --git a/repository_service_tuf/cli/admin/ceremony.py b/repository_service_tuf/cli/admin/ceremony.py index 73151d663..87fee8956 100644 --- a/repository_service_tuf/cli/admin/ceremony.py +++ b/repository_service_tuf/cli/admin/ceremony.py @@ -44,21 +44,23 @@ @admin.command() # type: ignore -@click.argument( - "output", - required=False, +@click.option( + "--out", + is_flag=False, + flag_value=DEFAULT_PATH, + help=f"Write output json result to FILENAME (default: '{DEFAULT_PATH}')", type=click.File("w"), ) @click.pass_context -def ceremony(context: Any, output: Optional[click.File]) -> None: +def ceremony(context: Any, out: Optional[click.File]) -> 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 output: + if not settings.get("SERVER") and not out: raise click.ClickException( - "Either '--api-sever'/'SERVER' in RSTUF config or 'OUTPUT' needed" + "Either '--api-sever'/'SERVER' in RSTUF config or '--out' needed" ) if settings.get("SERVER"): @@ -114,10 +116,9 @@ def ceremony(context: Any, output: Optional[click.File]) -> None: roles_settings = Settings(roles) bootstrap_payload = CeremonyPayload(roles_settings, metadatas) # Dump payload when the user explicitly wants or doesn't send it to the API - if output: - path = output.name if output is not None else DEFAULT_PATH - save_payload(path, asdict(bootstrap_payload)) - console.print(f"Saved result to '{path}'") + if out: + save_payload(out.name, asdict(bootstrap_payload)) + console.print(f"Saved result to '{out.name}'") if settings.get("SERVER"): task_id = send_payload( diff --git a/repository_service_tuf/cli/admin/sign.py b/repository_service_tuf/cli/admin/sign.py index dac60ef2b..1f2d021de 100644 --- a/repository_service_tuf/cli/admin/sign.py +++ b/repository_service_tuf/cli/admin/sign.py @@ -67,11 +67,10 @@ def _get_pending_roles(settings: Any) -> Dict[str, Dict[str, Any]]: @metadata.command() # type: ignore @click.option( - "--save", - "-s", + "--out", is_flag=False, flag_value=DEFAULT_PATH, - help=f"Write json result to FILENAME (default: '{DEFAULT_PATH}')", + help=f"Write output json result to FILENAME (default: '{DEFAULT_PATH}')", type=click.File("w"), ) @click.argument( @@ -82,7 +81,7 @@ def _get_pending_roles(settings: Any) -> Dict[str, Dict[str, Any]]: @click.pass_context def sign( context: click.Context, - save: Optional[click.File], + out: Optional[click.File], signing_json_input_file: Optional[click.File], ) -> None: """Add one signature to root metadata.""" @@ -144,10 +143,9 @@ def sign( # Send payload to the API and/or save it locally payload = SignPayload(signature=signature.to_dict()) - if save: - path = save.name if save is not None else DEFAULT_PATH - save_payload(path, asdict(payload)) - console.print(f"Saved result to '{path}'") + if out: + save_payload(out.name, asdict(payload)) + console.print(f"Saved result to '{out.name}'") if settings.get("SERVER"): console.print(f"\nSending signature to {settings.SERVER}") diff --git a/tests/conftest.py b/tests/conftest.py index 4e941e2bc..dfaa8e207 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -21,8 +21,8 @@ from securesystemslib.signer import CryptoSigner, SSlibKey from tuf.api.metadata import Metadata, Root -import repository_service_tuf.cli.admin.ceremony as ceremony from repository_service_tuf.cli.admin.import_artifacts import import_artifacts +from repository_service_tuf.cli.admin.update import update from repository_service_tuf.helpers.tuf import ( BootstrapSetup, MetadataInfo, @@ -308,14 +308,12 @@ def invoke_command( client = _create_client() out_file_name = "out_file.json" - if cmd.name == ceremony.ceremony.name: - out_file_name = ceremony.DEFAULT_PATH - # For ceremony out file name is an argument. - out_args = [out_file_name] - elif cmd.name == import_artifacts.name: + if cmd.name == import_artifacts.name: out_args = [] - else: + elif cmd.name == update.name: out_args = ["-s", out_file_name] + else: + out_args = ["--out", out_file_name] api_url = None if "--api-server" in args: diff --git a/tests/unit/cli/admin/test_ceremony.py b/tests/unit/cli/admin/test_ceremony.py index 775734a7e..a5a449766 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_output( + def test_ceremony_with_custom_out( self, ceremony_inputs, client, @@ -20,7 +20,7 @@ def test_ceremony_with_custom_output( with client.isolated_filesystem(): result = client.invoke( ceremony.ceremony, - args=[custom_path], + args=["--out", custom_path], input="\n".join( input_step1 + input_step2 + input_step3 + input_step4 ), @@ -38,7 +38,6 @@ def test_ceremony_with_custom_output( 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 def test_ceremony_threshold_less_than_2( self, ceremony_inputs, patch_getpass, patch_utcnow @@ -163,7 +162,7 @@ def test_ceremony_api_server( ] assert "Ceremony done. 🔐 🎉. Bootstrap completed." in result.stdout - def test_ceremony_api_server_with_output_argument( + def test_ceremony_api_server_with_out_option( self, ceremony_inputs, monkeypatch, @@ -189,7 +188,7 @@ def test_ceremony_api_server_with_output_argument( with client.isolated_filesystem(): result = client.invoke( ceremony.ceremony, - args=[custom_path], + args=["--out", custom_path], input="\n".join( input_step1 + input_step2 + input_step3 + input_step4 ), @@ -298,6 +297,22 @@ def test_ceremony_try_setting_root_keys_less_than_threshold( class TestCeremonyError: + def test_ceremony_no_api_server_and_no_output_option( + self, client, test_context, ceremony_inputs + ): + 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, + ) + + assert "Either '--api-sever'/'SERVER'" in result.stderr + def test_ceremony_bootstrap_api_server_locked_for_bootstrap( self, ceremony_inputs, monkeypatch ): diff --git a/tests/unit/cli/admin/test_sign.py b/tests/unit/cli/admin/test_sign.py index d3cde6ee5..93e6f8edd 100644 --- a/tests/unit/cli/admin/test_sign.py +++ b/tests/unit/cli/admin/test_sign.py @@ -136,7 +136,7 @@ def test_sign_bootstap_root(self, patch_getpass): ) ] - def test_sign_local_file_input_and_custom_save( + def test_sign_local_file_input_and_custom_out( self, client, test_context, patch_getpass ): inputs = [ @@ -149,7 +149,7 @@ def test_sign_local_file_input_and_custom_save( with client.isolated_filesystem(): result = client.invoke( sign.sign, - args=args + ["-s", custom_path], + args=args + ["--out", custom_path], input="\n".join(inputs), obj=test_context, catch_exceptions=False,