From ae67de8c2bba278c1a724fbacae589bc16e70477 Mon Sep 17 00:00:00 2001 From: Tomasz Durda Date: Thu, 29 Oct 2026 13:01:43 +0100 Subject: [PATCH 1/6] feat: handle errors in secrets dump and add --skip-errors flag --- README.md | 6 +++ ecsctrl/cli.py | 23 +++++++-- ecsctrl/dump/secrets.py | 40 ++++++++++----- tests/cli/secrets/__init__.py | 0 tests/cli/secrets/test_dump.py | 91 ++++++++++++++++++++++++++++++++++ 5 files changed, 145 insertions(+), 15 deletions(-) create mode 100644 tests/cli/secrets/__init__.py create mode 100644 tests/cli/secrets/test_dump.py diff --git a/README.md b/README.md index 8e9084f..0aebd5a 100644 --- a/README.md +++ b/README.md @@ -199,3 +199,9 @@ Dump secrets from SSM parameter store. You can optionally filter params by name ```bash ecsctrl secrets dump -e production.env --filter "db_.*" secrets.yaml ``` + +By default the dump aborts on the first parameter that fails to be fetched (e.g. access denied) and does not write the output 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 +``` diff --git a/ecsctrl/cli.py b/ecsctrl/cli.py index bfb140e..b20e99e 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,19 @@ def store( default=None, help="Export only secrets matching given regexp pattern", ) +@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, + skip_errors, env_file, json_file, var, @@ -345,8 +353,17 @@ 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) + try: + render_dumped_secrets(click, secrets, var_lut, spec_file) + except SecretFetchError as e: + raise click.ClickException(f"{e.name}: {e.cause}") + 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..b7b346e 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,29 @@ 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) -> Generator[Parameter, None, None]: for parameter in list_secrets(ssm): parameter_name = parameter["Name"] 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, @@ -47,13 +62,14 @@ def dump_secrets(ssm, filter=None) -> Generator[Parameter, None, None]: def render_dumped_secrets(click, secrets: Sequence[Parameter], vars_lut, target_file): + chunks = [] + for parameter in secrets: + key, secret_text = rended_single_secret(parameter, vars_lut) + chunks.append(f"# Dumped from `{parameter.name}`:\n{secret_text}\n") + click.echo(f"🔑 Dumped secret {parameter.name} as {key}.") + with open(target_file, "w") as f: - 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") - click.echo(f"🔑 Dumped secret {parameter.name} as {key}.") + f.write("".join(chunks)) def rended_single_secret(parameter: Parameter, vars_lut): 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..a14b2d3 --- /dev/null +++ b/tests/cli/secrets/test_dump.py @@ -0,0 +1,91 @@ +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 not target.exists() + + +@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() From c498648e5289b8fe9ad0abea6637b349d0febf8f Mon Sep 17 00:00:00 2001 From: Tomasz Durda Date: Thu, 29 Oct 2026 13:04:56 +0100 Subject: [PATCH 2/6] feat: add --start-from flag to secrets dump --- README.md | 6 +++++ ecsctrl/cli.py | 11 ++++++++- ecsctrl/dump/secrets.py | 6 ++++- tests/cli/secrets/test_dump.py | 42 ++++++++++++++++++++++++++++++++++ 4 files changed, 63 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 0aebd5a..db16e97 100644 --- a/README.md +++ b/README.md @@ -205,3 +205,9 @@ By default the dump aborts on the first parameter that fails to be fetched (e.g. ```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 +``` diff --git a/ecsctrl/cli.py b/ecsctrl/cli.py index b20e99e..6dcbe58 100644 --- a/ecsctrl/cli.py +++ b/ecsctrl/cli.py @@ -331,6 +331,12 @@ 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( "--skip-errors", is_flag=True, @@ -343,6 +349,7 @@ def dump( ctx, spec_file, filter, + start_from, skip_errors, env_file, json_file, @@ -357,7 +364,9 @@ def dump( 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) + 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) except SecretFetchError as e: diff --git a/ecsctrl/dump/secrets.py b/ecsctrl/dump/secrets.py index b7b346e..9cbdae4 100644 --- a/ecsctrl/dump/secrets.py +++ b/ecsctrl/dump/secrets.py @@ -36,9 +36,13 @@ def __init__(self, name, cause): self.cause = cause -def dump_secrets(ssm, filter=None, on_error=None) -> Generator[Parameter, None, None]: +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): try: response = ssm.call( diff --git a/tests/cli/secrets/test_dump.py b/tests/cli/secrets/test_dump.py index a14b2d3..7545f1f 100644 --- a/tests/cli/secrets/test_dump.py +++ b/tests/cli/secrets/test_dump.py @@ -89,3 +89,45 @@ def test_dump_list_failure_is_fatal_even_with_skip_errors(boto_mock, tmp_path): 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 From 337650fe07dbb861cc4f21e01ace19aca3a70860 Mon Sep 17 00:00:00 2001 From: Tomasz Durda Date: Thu, 29 Oct 2026 13:05:29 +0100 Subject: [PATCH 3/6] test: extend secrets dump coverage --- tests/cli/secrets/test_dump.py | 50 +++++++++++++++++++++ tests/test_dump_secrets.py | 80 ++++++++++++++++++++++++++++++++++ 2 files changed, 130 insertions(+) create mode 100644 tests/test_dump_secrets.py diff --git a/tests/cli/secrets/test_dump.py b/tests/cli/secrets/test_dump.py index 7545f1f..45a9bbe 100644 --- a/tests/cli/secrets/test_dump.py +++ b/tests/cli/secrets/test_dump.py @@ -131,3 +131,53 @@ def test_dump_start_from_with_filter(boto_mock, tmp_path): 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 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)) From 03b34239597b724e9039581b1a150f1a652d628d Mon Sep 17 00:00:00 2001 From: Tomasz Durda Date: Thu, 29 Oct 2026 13:10:33 +0100 Subject: [PATCH 4/6] feat: add --simple flag to secrets dump --- README.md | 6 ++++ ecsctrl/cli.py | 9 ++++- ecsctrl/dump/secrets.py | 10 +++--- tests/cli/secrets/test_dump.py | 63 ++++++++++++++++++++++++++++++++++ 4 files changed, 83 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index db16e97..e679234 100644 --- a/README.md +++ b/README.md @@ -211,3 +211,9 @@ Use `--start-from` to dump only parameters whose names are alphabetically equal ```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 6dcbe58..4c94696 100644 --- a/ecsctrl/cli.py +++ b/ecsctrl/cli.py @@ -337,6 +337,12 @@ def store( 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, @@ -350,6 +356,7 @@ def dump( spec_file, filter, start_from, + simple, skip_errors, env_file, json_file, @@ -368,7 +375,7 @@ def on_error(error): ssm, filter, on_error if skip_errors else None, start_from=start_from ) try: - render_dumped_secrets(click, secrets, var_lut, spec_file) + render_dumped_secrets(click, secrets, var_lut, spec_file, simple) except SecretFetchError as e: raise click.ClickException(f"{e.name}: {e.cause}") except (BotoCoreError, ClientError) as e: diff --git a/ecsctrl/dump/secrets.py b/ecsctrl/dump/secrets.py index 9cbdae4..6510321 100644 --- a/ecsctrl/dump/secrets.py +++ b/ecsctrl/dump/secrets.py @@ -65,10 +65,12 @@ def dump_secrets( ) -def render_dumped_secrets(click, secrets: Sequence[Parameter], vars_lut, target_file): +def render_dumped_secrets( + click, secrets: Sequence[Parameter], vars_lut, target_file, simple=False +): chunks = [] for parameter in secrets: - key, secret_text = rended_single_secret(parameter, vars_lut) + key, secret_text = rended_single_secret(parameter, vars_lut, simple) chunks.append(f"# Dumped from `{parameter.name}`:\n{secret_text}\n") click.echo(f"🔑 Dumped secret {parameter.name} as {key}.") @@ -76,10 +78,10 @@ def render_dumped_secrets(click, secrets: Sequence[Parameter], vars_lut, target_ f.write("".join(chunks)) -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/test_dump.py b/tests/cli/secrets/test_dump.py index 45a9bbe..3dffd5e 100644 --- a/tests/cli/secrets/test_dump.py +++ b/tests/cli/secrets/test_dump.py @@ -181,3 +181,66 @@ def test_dump_start_from_across_pages(boto_mock, tmp_path): 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", + } From 6a9d6e661a51851ecf15a95fdbda7369bce8da7d Mon Sep 17 00:00:00 2001 From: Tomasz Durda Date: Thu, 29 Oct 2026 13:15:23 +0100 Subject: [PATCH 5/6] fix: don't report secrets as dumped when dump fails --- ecsctrl/cli.py | 6 +++++- ecsctrl/dump/secrets.py | 6 +++++- tests/cli/secrets/test_dump.py | 32 ++++++++++++++++++++++++++++++++ 3 files changed, 42 insertions(+), 2 deletions(-) diff --git a/ecsctrl/cli.py b/ecsctrl/cli.py index 4c94696..92db8eb 100644 --- a/ecsctrl/cli.py +++ b/ecsctrl/cli.py @@ -377,7 +377,11 @@ def on_error(error): try: render_dumped_secrets(click, secrets, var_lut, spec_file, simple) except SecretFetchError as e: - raise click.ClickException(f"{e.name}: {e.cause}") + raise click.ClickException( + f"{e.name}: {e.cause}\n" + f"File {spec_file} was not written. " + "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}") diff --git a/ecsctrl/dump/secrets.py b/ecsctrl/dump/secrets.py index 6510321..089d88b 100644 --- a/ecsctrl/dump/secrets.py +++ b/ecsctrl/dump/secrets.py @@ -69,14 +69,18 @@ def render_dumped_secrets( click, secrets: Sequence[Parameter], vars_lut, target_file, simple=False ): chunks = [] + dumped = [] for parameter in secrets: key, secret_text = rended_single_secret(parameter, vars_lut, simple) chunks.append(f"# Dumped from `{parameter.name}`:\n{secret_text}\n") - click.echo(f"🔑 Dumped secret {parameter.name} as {key}.") + dumped.append((parameter.name, key)) with open(target_file, "w") as f: f.write("".join(chunks)) + for name, key in dumped: + click.echo(f"🔑 Dumped secret {name} as {key}.") + def rended_single_secret(parameter: Parameter, vars_lut, simple=False): name = parameter.name diff --git a/tests/cli/secrets/test_dump.py b/tests/cli/secrets/test_dump.py index 3dffd5e..c7ed655 100644 --- a/tests/cli/secrets/test_dump.py +++ b/tests/cli/secrets/test_dump.py @@ -244,3 +244,35 @@ def test_dump_simple_output_can_be_loaded_back(boto_mock, tmp_path): "/described": "d", "/multiline": "l1\nl2", } + + +@mock.patch("boto3.client") +def test_dump_failure_does_not_report_secrets_as_dumped(boto_mock, tmp_path): + boto_mock.return_value = make_ssm_mock(failing=["/b"]) + + result, target = invoke(tmp_path) + + assert result.exit_code == 1 + assert not target.exists() + assert "Dumped" 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 "not 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 From b191badf71d999bd7c3f420c29c1356598c08fce Mon Sep 17 00:00:00 2001 From: Tomasz Durda Date: Thu, 29 Oct 2026 13:18:06 +0100 Subject: [PATCH 6/6] feat: write secrets dump file live --- README.md | 2 +- ecsctrl/cli.py | 2 +- ecsctrl/dump/secrets.py | 27 ++++++------ tests/cli/secrets/test_dump.py | 79 +++++++++++++++++++++++++++++++--- 4 files changed, 91 insertions(+), 19 deletions(-) diff --git a/README.md b/README.md index e679234..9a2b3f6 100644 --- a/README.md +++ b/README.md @@ -200,7 +200,7 @@ Dump secrets from SSM parameter store. You can optionally filter params by name ecsctrl secrets dump -e production.env --filter "db_.*" secrets.yaml ``` -By default the dump aborts on the first parameter that fails to be fetched (e.g. access denied) and does not write the output file. Use `--skip-errors` to skip such parameters instead - each skipped parameter is reported on stderr. +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 diff --git a/ecsctrl/cli.py b/ecsctrl/cli.py index 92db8eb..509b4a4 100644 --- a/ecsctrl/cli.py +++ b/ecsctrl/cli.py @@ -379,7 +379,7 @@ def on_error(error): except SecretFetchError as e: raise click.ClickException( f"{e.name}: {e.cause}\n" - f"File {spec_file} was not written. " + 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: diff --git a/ecsctrl/dump/secrets.py b/ecsctrl/dump/secrets.py index 089d88b..5038998 100644 --- a/ecsctrl/dump/secrets.py +++ b/ecsctrl/dump/secrets.py @@ -68,18 +68,21 @@ def dump_secrets( def render_dumped_secrets( click, secrets: Sequence[Parameter], vars_lut, target_file, simple=False ): - chunks = [] - dumped = [] - for parameter in secrets: - key, secret_text = rended_single_secret(parameter, vars_lut, simple) - chunks.append(f"# Dumped from `{parameter.name}`:\n{secret_text}\n") - dumped.append((parameter.name, key)) - - with open(target_file, "w") as f: - f.write("".join(chunks)) - - for name, key in dumped: - click.echo(f"🔑 Dumped secret {name} as {key}.") + f = None + try: + for parameter in secrets: + 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, simple=False): diff --git a/tests/cli/secrets/test_dump.py b/tests/cli/secrets/test_dump.py index c7ed655..e2e8b1b 100644 --- a/tests/cli/secrets/test_dump.py +++ b/tests/cli/secrets/test_dump.py @@ -58,7 +58,8 @@ def test_dump_fails_fast_by_default(boto_mock, tmp_path): assert "/b" in result.stderr assert "AccessDeniedException" in result.stderr assert "Traceback" not in result.stderr - assert not target.exists() + assert "/a: value/a" in target.read_text() + assert "/c" not in target.read_text() @mock.patch("boto3.client") @@ -247,14 +248,15 @@ def test_dump_simple_output_can_be_loaded_back(boto_mock, tmp_path): @mock.patch("boto3.client") -def test_dump_failure_does_not_report_secrets_as_dumped(boto_mock, tmp_path): +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 not target.exists() - assert "Dumped" not in result.stdout + 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") @@ -264,7 +266,7 @@ def test_dump_failure_hints_at_skip_errors(boto_mock, tmp_path): result, target = invoke(tmp_path) assert "--skip-errors" in result.stderr - assert "not written" in result.stderr + assert "already written" in result.stderr @mock.patch("boto3.client") @@ -276,3 +278,70 @@ def test_dump_reports_secrets_after_writing_file(boto_mock, 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() == ""