diff --git a/README.md b/README.md index 8e9084f..9a2b3f6 100644 --- a/README.md +++ b/README.md @@ -199,3 +199,21 @@ Dump secrets from SSM parameter store. You can optionally filter params by name ```bash ecsctrl secrets dump -e production.env --filter "db_.*" secrets.yaml ``` + +Secrets are appended to the output file as soon as each one is fetched, so an interrupted dump keeps what was already written. By default the dump aborts on the first parameter that fails to be fetched (e.g. access denied); secrets written before the failure stay in the file. Use `--skip-errors` to skip such parameters instead - each skipped parameter is reported on stderr. + +```bash +ecsctrl secrets dump -e production.env --skip-errors secrets.yaml +``` + +Use `--start-from` to dump only parameters whose names are alphabetically equal to or after the given value (e.g. to resume after a failure). It can be combined with `--filter`. + +```bash +ecsctrl secrets dump -e production.env --start-from "/prod/db" secrets.yaml +``` + +By default only `SecureString` parameters without a description are written as `name: value`; the others include their `Type` and `Description`. Use `--simple` to write every parameter as `name: value`. Note that the type and description are not kept, so storing such a file creates all parameters as `SecureString`s. + +```bash +ecsctrl secrets dump -e production.env --simple secrets.yaml +``` diff --git a/ecsctrl/cli.py b/ecsctrl/cli.py index bfb140e..509b4a4 100644 --- a/ecsctrl/cli.py +++ b/ecsctrl/cli.py @@ -3,12 +3,13 @@ from email.policy import default import click +from botocore.exceptions import BotoCoreError, ClientError from ecsctrl.loader import VarsLoader from .boto_client import BotoClient from .dump import generate_var_lut -from .dump.secrets import dump_secrets, render_dumped_secrets +from .dump.secrets import SecretFetchError, dump_secrets, render_dumped_secrets from .service_updater import ServiceUpdater, TaskDefinitionServiceUpdater, WaitForUpdate from .yaml_converter import ( JOB_DEFINITION, @@ -330,12 +331,33 @@ def store( default=None, help="Export only secrets matching given regexp pattern", ) +@click.option( + "--start-from", + type=str, + default=None, + help="Export only secrets whose names are alphabetically equal to or after given value", +) +@click.option( + "--simple", + is_flag=True, + default=False, + help="Write every secret as `key: value`, omitting its type and description", +) +@click.option( + "--skip-errors", + is_flag=True, + default=False, + help="Skip secrets that fail to be fetched (reported on stderr) instead of aborting", +) @common_options @click.pass_context def dump( ctx, spec_file, filter, + start_from, + simple, + skip_errors, env_file, json_file, var, @@ -345,8 +367,23 @@ def dump( vars = VarsLoader(env_file, var, json_file, sys_env).load() var_lut = generate_var_lut(vars) ssm = BotoClient("ssm", dry_run=ctx.obj["boto_client"].dry_run) - secrets = dump_secrets(ssm, filter) - render_dumped_secrets(click, secrets, var_lut, spec_file) + + def on_error(error): + click.echo(f"⚠️ Skipping {error.name}: {error.cause}", err=True) + + secrets = dump_secrets( + ssm, filter, on_error if skip_errors else None, start_from=start_from + ) + try: + render_dumped_secrets(click, secrets, var_lut, spec_file, simple) + except SecretFetchError as e: + raise click.ClickException( + f"{e.name}: {e.cause}\n" + f"Secrets fetched before the failure were already written to {spec_file}. " + "Use --skip-errors to skip secrets that fail to be fetched." + ) + except (BotoCoreError, ClientError) as e: + raise click.ClickException(f"Failed to list secrets: {e}") @service.command() diff --git a/ecsctrl/dump/secrets.py b/ecsctrl/dump/secrets.py index d98cd1e..5038998 100644 --- a/ecsctrl/dump/secrets.py +++ b/ecsctrl/dump/secrets.py @@ -1,4 +1,5 @@ import re +from botocore.exceptions import BotoCoreError, ClientError from . import substitute_with_expressions from dataclasses import dataclass from typing import Generator, Optional, Sequence @@ -28,15 +29,33 @@ def list_secrets(ssm): yield parameter -def dump_secrets(ssm, filter=None) -> Generator[Parameter, None, None]: +class SecretFetchError(Exception): + def __init__(self, name, cause): + super().__init__(f"Failed to fetch {name}: {cause}") + self.name = name + self.cause = cause + + +def dump_secrets( + ssm, filter=None, on_error=None, start_from=None +) -> Generator[Parameter, None, None]: for parameter in list_secrets(ssm): parameter_name = parameter["Name"] + if start_from is not None and parameter_name < start_from: + continue if filter is None or re.match(filter, parameter_name): - response = ssm.call( - "get_parameter", - Name=parameter_name, - WithDecryption=True, - ) + try: + response = ssm.call( + "get_parameter", + Name=parameter_name, + WithDecryption=True, + ) + except (BotoCoreError, ClientError) as e: + error = SecretFetchError(parameter_name, e) + if on_error is None: + raise error from e + on_error(error) + continue yield Parameter( name=parameter_name, @@ -46,20 +65,30 @@ def dump_secrets(ssm, filter=None) -> Generator[Parameter, None, None]: ) -def render_dumped_secrets(click, secrets: Sequence[Parameter], vars_lut, target_file): - with open(target_file, "w") as f: +def render_dumped_secrets( + click, secrets: Sequence[Parameter], vars_lut, target_file, simple=False +): + f = None + try: for parameter in secrets: - key, secret_text = rended_single_secret(parameter, vars_lut) - f.write(f"# Dumped from `{parameter.name}`:\n") - f.write(secret_text) - f.write(f"\n") + key, secret_text = rended_single_secret(parameter, vars_lut, simple) + if f is None: + f = open(target_file, "w") + f.write(f"# Dumped from `{parameter.name}`:\n{secret_text}\n") + f.flush() click.echo(f"🔑 Dumped secret {parameter.name} as {key}.") + if f is None: + open(target_file, "w").close() + finally: + if f is not None: + f.close() + -def rended_single_secret(parameter: Parameter, vars_lut): +def rended_single_secret(parameter: Parameter, vars_lut, simple=False): name = parameter.name key = substitute_with_expressions(name, vars_lut) - if parameter.type == "SecureString" and parameter.description is None: + if simple or (parameter.type == "SecureString" and parameter.description is None): secret_text = render_simple_secret(key, parameter) else: secret_text = render_complex_secret(key, parameter) diff --git a/tests/cli/secrets/__init__.py b/tests/cli/secrets/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/cli/secrets/test_dump.py b/tests/cli/secrets/test_dump.py new file mode 100644 index 0000000..e2e8b1b --- /dev/null +++ b/tests/cli/secrets/test_dump.py @@ -0,0 +1,347 @@ +from unittest import mock + +import pytest +from botocore.exceptions import ClientError +from click.testing import CliRunner + +from ecsctrl.cli import cli + + +def client_error(operation, code="AccessDeniedException"): + return ClientError({"Error": {"Code": code, "Message": "nope"}}, operation) + + +def make_ssm_mock(failing=()): + ssm = mock.Mock() + ssm.describe_parameters.return_value = { + "Parameters": [{"Name": "/a"}, {"Name": "/b"}, {"Name": "/c"}] + } + + def get_parameter(Name, WithDecryption): + if Name in failing: + raise client_error("GetParameter") + return {"Parameter": {"Value": f"value{Name}", "Type": "SecureString"}} + + ssm.get_parameter.side_effect = get_parameter + return ssm + + +def invoke(tmp_path, *extra_args): + target = tmp_path / "secrets.yaml" + try: + runner = CliRunner(mix_stderr=False) + except TypeError: # click >= 8.2 always separates stderr + runner = CliRunner() + result = runner.invoke(cli, ["secrets", "dump", *extra_args, str(target)]) + return result, target + + +@mock.patch("boto3.client") +def test_dump_all(boto_mock, tmp_path): + boto_mock.return_value = make_ssm_mock() + + result, target = invoke(tmp_path) + + assert result.exit_code == 0 + assert "/a: value/a" in target.read_text() + assert "/b: value/b" in target.read_text() + assert "/c: value/c" in target.read_text() + + +@mock.patch("boto3.client") +def test_dump_fails_fast_by_default(boto_mock, tmp_path): + boto_mock.return_value = make_ssm_mock(failing=["/b"]) + + result, target = invoke(tmp_path) + + assert result.exit_code == 1 + assert "/b" in result.stderr + assert "AccessDeniedException" in result.stderr + assert "Traceback" not in result.stderr + assert "/a: value/a" in target.read_text() + assert "/c" not in target.read_text() + + +@mock.patch("boto3.client") +def test_dump_skip_errors(boto_mock, tmp_path): + boto_mock.return_value = make_ssm_mock(failing=["/b"]) + + result, target = invoke(tmp_path, "--skip-errors") + + content = target.read_text() + assert result.exit_code == 0 + assert "/a: value/a" in content + assert "/b" not in content + assert "/c: value/c" in content + assert "/b" in result.stderr + assert "AccessDeniedException" in result.stderr + assert "/b" not in result.stdout + + +@mock.patch("boto3.client") +def test_dump_list_failure_is_fatal_even_with_skip_errors(boto_mock, tmp_path): + ssm = make_ssm_mock() + ssm.describe_parameters.side_effect = client_error("DescribeParameters") + boto_mock.return_value = ssm + + result, target = invoke(tmp_path, "--skip-errors") + + assert result.exit_code == 1 + assert "AccessDeniedException" in result.stderr + assert "Traceback" not in result.stderr + assert not target.exists() + + +@mock.patch("boto3.client") +def test_dump_start_from(boto_mock, tmp_path): + ssm = make_ssm_mock() + boto_mock.return_value = ssm + + result, target = invoke(tmp_path, "--start-from", "/b") + + content = target.read_text() + assert result.exit_code == 0 + assert "/a" not in content + assert "/b: value/b" in content + assert "/c: value/c" in content + fetched = [c.kwargs["Name"] for c in ssm.get_parameter.call_args_list] + assert fetched == ["/b", "/c"] + + +@mock.patch("boto3.client") +def test_dump_start_from_between_names(boto_mock, tmp_path): + ssm = make_ssm_mock() + boto_mock.return_value = ssm + + result, target = invoke(tmp_path, "--start-from", "/aa") + + content = target.read_text() + assert result.exit_code == 0 + assert "/a:" not in content + assert "/b: value/b" in content + assert "/c: value/c" in content + + +@mock.patch("boto3.client") +def test_dump_start_from_with_filter(boto_mock, tmp_path): + boto_mock.return_value = make_ssm_mock() + + result, target = invoke(tmp_path, "--start-from", "/b", "--filter", "/c") + + content = target.read_text() + assert result.exit_code == 0 + assert "/b" not in content + assert "/c: value/c" in content + + +@mock.patch("boto3.client") +def test_dump_start_from_with_skip_errors(boto_mock, tmp_path): + ssm = make_ssm_mock(failing=["/a", "/b"]) + boto_mock.return_value = ssm + + result, target = invoke(tmp_path, "--start-from", "/b", "--skip-errors") + + content = target.read_text() + assert result.exit_code == 0 + assert "/c: value/c" in content + assert "/a" not in result.stderr + assert "/b" in result.stderr + + +@mock.patch("boto3.client") +def test_dump_start_from_with_unsorted_listing(boto_mock, tmp_path): + ssm = make_ssm_mock() + ssm.describe_parameters.return_value = { + "Parameters": [{"Name": "/c"}, {"Name": "/a"}, {"Name": "/b"}] + } + boto_mock.return_value = ssm + + result, target = invoke(tmp_path, "--start-from", "/b") + + content = target.read_text() + assert result.exit_code == 0 + assert "/a" not in content + assert "/b: value/b" in content + assert "/c: value/c" in content + + +@mock.patch("boto3.client") +def test_dump_start_from_across_pages(boto_mock, tmp_path): + ssm = make_ssm_mock() + ssm.describe_parameters.side_effect = [ + {"Parameters": [{"Name": "/a"}], "NextToken": "t"}, + {"Parameters": [{"Name": "/b"}, {"Name": "/c"}]}, + ] + boto_mock.return_value = ssm + + result, target = invoke(tmp_path, "--start-from", "/b") + + content = target.read_text() + assert result.exit_code == 0 + assert "/a" not in content + assert "/b: value/b" in content + assert "/c: value/c" in content + assert ssm.describe_parameters.call_count == 2 + + +def make_mixed_ssm_mock(): + ssm = mock.Mock() + ssm.describe_parameters.return_value = { + "Parameters": [ + {"Name": "/plain"}, + {"Name": "/described", "Description": "some description"}, + {"Name": "/multiline"}, + ] + } + values = { + "/plain": {"Value": "p", "Type": "String"}, + "/described": {"Value": "d", "Type": "SecureString"}, + "/multiline": {"Value": "l1\nl2", "Type": "StringList"}, + } + ssm.get_parameter.side_effect = lambda Name, WithDecryption: { + "Parameter": values[Name] + } + return ssm + + +@mock.patch("boto3.client") +def test_dump_default_keeps_complex_format(boto_mock, tmp_path): + boto_mock.return_value = make_mixed_ssm_mock() + + result, target = invoke(tmp_path) + + content = target.read_text() + assert result.exit_code == 0 + assert "/plain:\n Type: String\n Value: p\n" in content + assert "Description: some description" in content + + +@mock.patch("boto3.client") +def test_dump_simple(boto_mock, tmp_path): + boto_mock.return_value = make_mixed_ssm_mock() + + result, target = invoke(tmp_path, "--simple") + + content = target.read_text() + assert result.exit_code == 0 + assert "/plain: p\n" in content + assert "/described: d\n" in content + assert "/multiline: |-\n l1\n l2\n" in content + assert "Type:" not in content + assert "Value:" not in content + assert "Description:" not in content + + +@mock.patch("boto3.client") +def test_dump_simple_output_can_be_loaded_back(boto_mock, tmp_path): + from ecsctrl.yaml_converter import SECRETS, yaml_to_dict + + boto_mock.return_value = make_mixed_ssm_mock() + + result, target = invoke(tmp_path, "--simple") + + assert yaml_to_dict(target.read_text(), SECRETS) == { + "/plain": "p", + "/described": "d", + "/multiline": "l1\nl2", + } + + +@mock.patch("boto3.client") +def test_dump_failure_reports_only_written_secrets(boto_mock, tmp_path): + boto_mock.return_value = make_ssm_mock(failing=["/b"]) + + result, target = invoke(tmp_path) + + assert result.exit_code == 1 + assert "Dumped secret /a as /a." in result.stdout + assert "/b" not in result.stdout + assert "/c" not in result.stdout + + +@mock.patch("boto3.client") +def test_dump_failure_hints_at_skip_errors(boto_mock, tmp_path): + boto_mock.return_value = make_ssm_mock(failing=["/b"]) + + result, target = invoke(tmp_path) + + assert "--skip-errors" in result.stderr + assert "already written" in result.stderr + + +@mock.patch("boto3.client") +def test_dump_reports_secrets_after_writing_file(boto_mock, tmp_path): + boto_mock.return_value = make_ssm_mock() + + result, target = invoke(tmp_path) + + assert result.exit_code == 0 + assert "Dumped secret /a as /a." in result.stdout + assert "Dumped secret /c as /c." in result.stdout + + +@mock.patch("boto3.client") +def test_dump_writes_file_live(boto_mock, tmp_path): + ssm = make_ssm_mock() + target = tmp_path / "secrets.yaml" + seen = {} + original = ssm.get_parameter.side_effect + + def get_parameter(Name, WithDecryption): + seen[Name] = target.read_text() if target.exists() else None + return original(Name=Name, WithDecryption=WithDecryption) + + ssm.get_parameter.side_effect = get_parameter + boto_mock.return_value = ssm + + result, _ = invoke(tmp_path) + + assert result.exit_code == 0 + assert seen["/a"] is None + assert "/a: value/a" in seen["/b"] + assert "/b" not in seen["/b"] + assert "/b: value/b" in seen["/c"] + + +@mock.patch("boto3.client") +def test_dump_keeps_secrets_written_before_interruption(boto_mock, tmp_path): + ssm = make_ssm_mock() + original = ssm.get_parameter.side_effect + + def get_parameter(Name, WithDecryption): + if Name == "/c": + raise KeyboardInterrupt() + return original(Name=Name, WithDecryption=WithDecryption) + + ssm.get_parameter.side_effect = get_parameter + boto_mock.return_value = ssm + + result, target = invoke(tmp_path) + + content = target.read_text() + assert "/a: value/a" in content + assert "/b: value/b" in content + + +@mock.patch("boto3.client") +def test_dump_listing_failure_leaves_existing_file_untouched(boto_mock, tmp_path): + ssm = make_ssm_mock() + ssm.describe_parameters.side_effect = client_error("DescribeParameters") + boto_mock.return_value = ssm + target = tmp_path / "secrets.yaml" + target.write_text("old content") + + result, _ = invoke(tmp_path) + + assert result.exit_code == 1 + assert target.read_text() == "old content" + + +@mock.patch("boto3.client") +def test_dump_without_matches_creates_empty_file(boto_mock, tmp_path): + boto_mock.return_value = make_ssm_mock() + + result, target = invoke(tmp_path, "--filter", "nomatch") + + assert result.exit_code == 0 + assert target.read_text() == "" diff --git a/tests/test_dump_secrets.py b/tests/test_dump_secrets.py new file mode 100644 index 0000000..56c744c --- /dev/null +++ b/tests/test_dump_secrets.py @@ -0,0 +1,80 @@ +from unittest import mock + +import pytest +from botocore.exceptions import ClientError + +from ecsctrl.boto_client import BotoClient +from ecsctrl.dump.secrets import SecretFetchError, dump_secrets + + +def make_ssm(names, failing=()): + client = mock.Mock() + client.describe_parameters.return_value = { + "Parameters": [{"Name": n} for n in names] + } + + def get_parameter(Name, WithDecryption): + if Name in failing: + raise ClientError( + {"Error": {"Code": "AccessDeniedException", "Message": "nope"}}, + "GetParameter", + ) + return {"Parameter": {"Value": "v", "Type": "String"}} + + client.get_parameter.side_effect = get_parameter + with mock.patch("boto3.client", return_value=client): + return BotoClient("ssm"), client + + +def names_of(parameters): + return [p.name for p in parameters] + + +def test_start_from_none_returns_everything(): + ssm, _ = make_ssm(["/a", "/b"]) + assert names_of(dump_secrets(ssm)) == ["/a", "/b"] + + +def test_start_from_is_inclusive(): + ssm, _ = make_ssm(["/a", "/b", "/c"]) + assert names_of(dump_secrets(ssm, start_from="/b")) == ["/b", "/c"] + + +def test_start_from_after_all_names_returns_nothing(): + ssm, client = make_ssm(["/a", "/b"]) + assert names_of(dump_secrets(ssm, start_from="/z")) == [] + client.get_parameter.assert_not_called() + + +def test_start_from_does_not_fetch_skipped_names(): + ssm, client = make_ssm(["/a", "/b"]) + list(dump_secrets(ssm, start_from="/b")) + assert [c.kwargs["Name"] for c in client.get_parameter.call_args_list] == ["/b"] + + +def test_start_from_is_case_sensitive(): + ssm, _ = make_ssm(["/B", "/a"]) + assert names_of(dump_secrets(ssm, start_from="/a")) == ["/a"] + + +def test_fetch_error_raises_secret_fetch_error_without_handler(): + ssm, _ = make_ssm(["/a", "/b"], failing=["/a"]) + with pytest.raises(SecretFetchError) as exc_info: + list(dump_secrets(ssm)) + assert exc_info.value.name == "/a" + assert isinstance(exc_info.value.cause, ClientError) + + +def test_fetch_error_calls_handler_and_continues(): + ssm, _ = make_ssm(["/a", "/b"], failing=["/a"]) + errors = [] + result = names_of(dump_secrets(ssm, on_error=errors.append)) + assert result == ["/b"] + assert [e.name for e in errors] == ["/a"] + + +def test_unrelated_exceptions_are_not_swallowed(): + ssm, client = make_ssm(["/a"]) + client.get_parameter.side_effect = ValueError("bug") + with pytest.raises(ValueError): + list(dump_secrets(ssm, on_error=lambda e: None))