kevinveenbirkenbach/docker-volume-backup
1
0
Fork 0
mirror of https://github.com/kevinveenbirkenbach/docker-volume-backup.git synced 2026-08-20 13:12:48 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(backup)!: dump the container that holds the database, with its password

Browse Source 
Two defects kept dedicated Postgres databases out of the backup.

docker exec never forwarded PGPASSWORD. execute_to_file set it on baudolo's
own process, but nothing carried it across the container boundary, so an
engine whose pg_hba demands a password on TCP loopback refused the dump.
forward_env passes a bare `-e NAME`, letting docker copy the value out of
this process's environment instead of spelling it into argv, where the
host's process list would publish it.

get_instance returned the container name unchanged when that name carried no
database token, claiming an instance it had never derived. An application
container therefore answered the same databases.csv row as its own dedicated
engine, and application images often ship the engine's client tools, so the
dump command started and wrote a file that looked like a backup and held
none of the data. Discourse is the live case: its launcher names the
container `discourse`, and the image ships pg_dumpall.

The regex stays a normaliser - `<app>-database` from compose and
`<app>_database.1.<task>` from swarm still resolve to the same instance.
Only the fallthrough changes.

BREAKING CHANGE: a database container whose name carries no `database`, `db`
or `postgres` token must now be named in --database-containers. Without that
declaration its rows no longer match and no dump is written.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Kevin Veen-Birkenbach
2026-08-18 04:26:24 +02:00
parent 8dbd5e89ea
commit b1ee8f5fac
6 changed files with 501 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files

27
src/baudolo/backup/db.py
View File

@@ -17,13 +17,27 @@ if TYPE_CHECKING:
log = logging.getLogger(__name__)
def get_instance(container: str, database_containers: list[str]) -> str:
"""
Derive a stable instance name from the container name.
def get_instance(container: str, database_containers: list[str]) -> str | None:
"""The databases.csv instance a container serves, or None for no database.
A declared container is its own instance. Every other name is normalised by
stripping a database suffix token, which maps both `<app>-database` from
compose and `<app>_database.1.<task>` from swarm onto the same instance.
Args:
container: the running container's name.
database_containers: names passed via --database-containers, taken as
declared engines whatever they are called.
Returns:
The instance name, or None when the name carries no database token: an
application container is not an engine, even when it ships the client
tools that would let a dump command start.
"""
if container in database_containers:
return container
return re.split(r"(_|-)(database|db|postgres)", container)[0]
parts = re.split(r"(_|-)(database|db|postgres)", container)
return parts[0] if len(parts) > 1 else None
def fallback_pg_dumpall(
@@ -37,6 +51,7 @@ def fallback_pg_dumpall(
container,
["pg_dumpall", "-U", username, "-h", "localhost"],
interactive=True,
forward_env=["PGPASSWORD"],
),
out_file,
env={"PGPASSWORD": password},
@@ -62,6 +77,9 @@ def backup_database(
Returns True if at least one dump was produced.
"""
instance_name = get_instance(container, database_containers)
if instance_name is None:
log.debug("Container '%s' carries no database token", container)
return False
entries = databases_df[databases_df["instance"] == instance_name]
if entries.empty:
@@ -133,6 +151,7 @@ def backup_database(
"--no-privileges",
],
interactive=True,
forward_env=["PGPASSWORD"],
),
dump_file,
env={"PGPASSWORD": password},

31
src/baudolo/backup/docker.py
View File

@@ -9,10 +9,35 @@ if TYPE_CHECKING:
def docker_exec_argv(
container: str, argv: Sequence[str], *, interactive: bool = False
container: str,
argv: Sequence[str],
*,
interactive: bool = False,
forward_env: Sequence[str] = (),
) -> list[str]:
"""The argv that runs *argv* inside *container*."""
return ["docker", "exec", *(["-i"] if interactive else []), container, *argv]
"""The argv that runs *argv* inside *container*.
Args:
container: the container to run in.
argv: the command, already split.
interactive: keep stdin open, for a command that is fed a dump.
forward_env: names of environment variables to hand to the container.
Passed as bare ``-e NAME``, so docker copies the value out of this
process's own environment; spelling ``-e NAME=value`` instead would
publish a secret in the host's process list.
Returns:
The argv list.
"""
forwarded = [arg for name in forward_env for arg in ("-e", name)]
return [
"docker",
"exec",
*(["-i"] if interactive else []),
*forwarded,
container,
*argv,
]
def get_image_info(container: str) -> str:

163
tests/e2e/test_e2e_app_container_ships_client_tools.py Normal file
View File

@@ -0,0 +1,163 @@
"""An application container that ships the engine's client tools.
This is the shape a dedicated database deploys in: the engine runs as
`<app>-database` while the application itself runs as `<app>`, and neither is
declared through --database-containers, so both names go through the instance
regex. `<app>-database` loses its suffix and lands on the instance `<app>` -
and `<app>` carries no database token at all, so a fallback that returns the
name unchanged lands on that same instance and offers the application container
as a second engine for the same row.
Discourse is the live example: its application container is named `discourse`
by its own launcher and ships pg_dumpall, so a dump command starts there and
writes a file that looks like a backup and holds none of the data.
"""
import unittest
from pathlib import Path
from baudolo.generation import DUMP_SUFFIX, FILES_DIR, SQL_DIR
from .helpers import (
POSTGRES_DATA_DIR,
POSTGRES_IMAGE,
backup_path,
backup_run,
cleanup_docker,
create_minimal_compose_dir,
ensure_empty_dir,
latest_version_dir,
require_docker,
run,
unique,
wait_for_postgres,
write_databases_csv,
)
MARKER = "the-application-volume-holds-files"
PAYLOAD = "shop-payload"
class TestE2EAppContainerShipsClientTools(unittest.TestCase):
@classmethod
def setUpClass(cls) -> None:
require_docker()
# uuid4 hex may begin with "db", which the instance regex would split
# on and turn the application container into a different instance,
# hiding exactly the collision this module is about.
cls.prefix = unique("baudolo-e2e-app-tools").replace("-db", "-xb")
cls.backups_dir = f"/tmp/{cls.prefix}/Backups"
ensure_empty_dir(cls.backups_dir)
cls.compose_dir = create_minimal_compose_dir(f"/tmp/{cls.prefix}")
cls.repo_name = cls.prefix
cls.engine = f"{cls.prefix}-shop-database"
cls.app = f"{cls.prefix}-shop"
cls.engine_volume = f"{cls.prefix}-shop-database-vol"
cls.app_volume = f"{cls.prefix}-shop-app-vol"
cls.containers = [cls.engine, cls.app]
cls.volumes = [cls.engine_volume, cls.app_volume]
run(["docker", "volume", "create", cls.engine_volume])
run(["docker", "volume", "create", cls.app_volume])
run(
[
"docker",
"run",
"-d",
"--name",
cls.engine,
"-e",
"POSTGRES_PASSWORD=shoppw",
"-e",
"POSTGRES_DB=shopdb",
"-e",
"POSTGRES_USER=postgres",
"-v",
f"{cls.engine_volume}:{POSTGRES_DATA_DIR}",
POSTGRES_IMAGE,
]
)
run(
[
"docker",
"run",
"-d",
"--name",
cls.app,
"--entrypoint",
"sh",
"-v",
f"{cls.app_volume}:/data",
POSTGRES_IMAGE,
"-c",
f"echo '{MARKER}' > /data/marker.txt && sleep 3600",
]
)
wait_for_postgres(cls.engine, user="postgres", timeout_s=90)
run(
[
"docker",
"exec",
cls.engine,
"sh",
"-lc",
(
'psql -U postgres -d shopdb -c "CREATE TABLE orders (id int, '
f"note text); INSERT INTO orders VALUES (1,'{PAYLOAD}');\""
),
],
check=True,
)
cls.databases_csv = f"/tmp/{cls.prefix}/databases.csv"
write_databases_csv(
cls.databases_csv,
[(cls.app, "shopdb", "postgres", "shoppw")],
)
backup_run(
backups_dir=cls.backups_dir,
repo_name=cls.repo_name,
compose_dir=cls.compose_dir,
databases_csv=cls.databases_csv,
database_containers=["dummy-db"],
images_no_stop_required=[POSTGRES_IMAGE],
)
cls.hash, cls.version = latest_version_dir(cls.backups_dir, cls.repo_name)
@classmethod
def tearDownClass(cls) -> None:
cleanup_docker(containers=cls.containers, volumes=cls.volumes)
def volume_dir(self, volume: str) -> Path:
return backup_path(self.backups_dir, self.repo_name, self.version, volume)
def test_the_engine_volume_was_dumped(self) -> None:
dump = self.volume_dir(self.engine_volume) / SQL_DIR / f"shopdb{DUMP_SUFFIX}"
self.assertTrue(dump.is_file(), f"expected a dump at {dump}")
self.assertIn(PAYLOAD, dump.read_text(encoding="utf-8"))
def test_the_application_volume_produced_no_dump(self) -> None:
"""The collision this module exists for: the application container
answers the same instance as the engine and starts a dump of its own."""
sql_dir = self.volume_dir(self.app_volume) / SQL_DIR
self.assertFalse(
sql_dir.exists(),
f"the application container was dumped: {sorted(sql_dir.iterdir())}"
if sql_dir.exists()
else "",
)
def test_the_application_volume_was_backed_up_as_files(self) -> None:
"""Refusing the dump must not cost the volume its backup."""
marker = self.volume_dir(self.app_volume) / FILES_DIR / "marker.txt"
self.assertTrue(marker.is_file(), f"expected a file backup at {marker}")
self.assertIn(MARKER, marker.read_text(encoding="utf-8"))
if __name__ == "__main__":
unittest.main()

159
tests/e2e/test_e2e_postgres_password_required.py Normal file
View File

@@ -0,0 +1,159 @@
"""An engine whose loopback auth really demands a password.
Every other Postgres scenario runs stock postgres:alpine, whose generated
pg_hba grants trust on 127.0.0.1 and ::1 - so `pg_dump -h localhost` never
needs the password and a dump succeeds whether or not baudolo hands one to the
container. This module makes the password mandatory, which is what a dedicated
engine on a real host does.
"""
import unittest
from pathlib import Path
from baudolo.generation import CLUSTER_SUFFIX, DUMP_SUFFIX, FILES_DIR, SQL_DIR
from .helpers import (
POSTGRES_DATA_DIR,
POSTGRES_IMAGE,
backup_path,
backup_run,
cleanup_docker,
create_minimal_compose_dir,
ensure_empty_dir,
latest_version_dir,
require_docker,
run,
unique,
wait_for_postgres,
write_databases_csv,
)
class TestE2EPostgresPasswordRequired(unittest.TestCase):
@classmethod
def setUpClass(cls) -> None:
require_docker()
cls.prefix = unique("baudolo-e2e-pg-password-required")
cls.backups_dir = f"/tmp/{cls.prefix}/Backups"
ensure_empty_dir(cls.backups_dir)
cls.compose_dir = create_minimal_compose_dir(f"/tmp/{cls.prefix}")
cls.repo_name = cls.prefix
cls.pg_container = f"{cls.prefix}-pg"
cls.pg_volume = f"{cls.prefix}-pg-vol"
cls.containers = [cls.pg_container]
cls.volumes = [cls.pg_volume]
run(["docker", "volume", "create", cls.pg_volume])
run(
[
"docker",
"run",
"-d",
"--name",
cls.pg_container,
"-e",
"POSTGRES_PASSWORD=pgpw",
"-e",
"POSTGRES_DB=appdb",
"-e",
"POSTGRES_USER=postgres",
# The entrypoint evals this into its initdb call, so the host
# lines of pg_hba demand scram while the local socket stays
# trust - the entrypoint's own init and the seeding below keep
# working, and only a TCP connection needs the password.
"-e",
"POSTGRES_INITDB_ARGS=--auth-host=scram-sha-256",
"-v",
f"{cls.pg_volume}:{POSTGRES_DATA_DIR}",
POSTGRES_IMAGE,
]
)
wait_for_postgres(cls.pg_container, user="postgres", timeout_s=90)
run(
[
"docker",
"exec",
cls.pg_container,
"sh",
"-lc",
(
'psql -U postgres -d appdb -c "CREATE TABLE t (id int primary '
"key, v text); INSERT INTO t VALUES (1,'ok');\""
),
],
check=True,
)
cls.unauthenticated = run(
[
"docker",
"exec",
cls.pg_container,
"sh",
"-lc",
"pg_dump -U postgres -d appdb -h localhost",
],
capture=True,
check=False,
)
cls.databases_csv = f"/tmp/{cls.prefix}/databases.csv"
write_databases_csv(
cls.databases_csv,
[
(cls.pg_container, "appdb", "postgres", "pgpw"),
(cls.pg_container, "*", "postgres", "pgpw"),
],
)
backup_run(
backups_dir=cls.backups_dir,
repo_name=cls.repo_name,
compose_dir=cls.compose_dir,
databases_csv=cls.databases_csv,
database_containers=[cls.pg_container],
images_no_stop_required=[POSTGRES_IMAGE],
only_sql=True,
)
cls.hash, cls.version = latest_version_dir(cls.backups_dir, cls.repo_name)
@classmethod
def tearDownClass(cls) -> None:
cleanup_docker(containers=cls.containers, volumes=cls.volumes)
def volume_dir(self) -> Path:
return backup_path(
self.backups_dir, self.repo_name, self.version, self.pg_volume
)
def test_a_dump_without_the_password_is_refused_by_the_server(self) -> None:
"""Without this the module is vacuous: a pg_hba still saying trust would
let a baudolo that forwards nothing pass just as well."""
self.assertNotEqual(self.unauthenticated.returncode, 0)
self.assertIn("no password supplied", self.unauthenticated.stderr or "")
def test_the_configured_database_was_dumped(self) -> None:
dump = self.volume_dir() / SQL_DIR / f"appdb{DUMP_SUFFIX}"
self.assertTrue(dump.is_file(), f"expected a dump at {dump}")
self.assertIn("Dumped by pg_dump", dump.read_text(encoding="utf-8"))
def test_the_dump_carries_the_payload(self) -> None:
"""pg_dump emits table data as COPY ... FROM stdin, so the row reads as
tab-separated values rather than as an INSERT literal."""
dump = self.volume_dir() / SQL_DIR / f"appdb{DUMP_SUFFIX}"
self.assertIn("COPY public.t (id, v) FROM stdin;", dump.read_text("utf-8"))
self.assertIn("1\tok", dump.read_text(encoding="utf-8"))
def test_the_cluster_row_was_dumped_too(self) -> None:
cluster = self.volume_dir() / SQL_DIR / f"{self.pg_container}{CLUSTER_SUFFIX}"
self.assertTrue(cluster.is_file(), f"expected a cluster dump at {cluster}")
self.assertIn("CREATE DATABASE", cluster.read_text(encoding="utf-8"))
def test_only_sql_left_no_file_copy_behind(self) -> None:
self.assertFalse((self.volume_dir() / FILES_DIR).exists())
if __name__ == "__main__":
unittest.main()

61
tests/unit/backup/test_docker_exec_env.py Normal file
View File

@@ -0,0 +1,61 @@
"""How a secret reaches the command running inside the container."""
from __future__ import annotations
import unittest
from baudolo.backup.db import fallback_pg_dumpall
from baudolo.backup.docker import docker_exec_argv
class TestForwardEnv(unittest.TestCase):
def test_nothing_is_added_when_no_variable_is_named(self) -> None:
self.assertEqual(
docker_exec_argv("c1", ["true"]),
["docker", "exec", "c1", "true"],
)
def test_a_named_variable_is_forwarded_without_its_value(self) -> None:
"""-e NAME=value would publish the secret in the host's process list."""
argv = docker_exec_argv("c1", ["true"], forward_env=["PGPASSWORD"])
self.assertEqual(argv, ["docker", "exec", "-e", "PGPASSWORD", "c1", "true"])
def test_the_flag_precedes_the_container(self) -> None:
"""docker reads options before the container name, arguments after it."""
argv = docker_exec_argv(
"c1", ["pg_dump", "-U", "u"], interactive=True, forward_env=["PGPASSWORD"]
)
self.assertLess(argv.index("-e"), argv.index("c1"))
self.assertLess(argv.index("-i"), argv.index("c1"))
self.assertGreater(argv.index("pg_dump"), argv.index("c1"))
def test_several_variables_each_get_their_own_flag(self) -> None:
argv = docker_exec_argv("c1", ["true"], forward_env=["A", "B"])
self.assertEqual(argv[:6], ["docker", "exec", "-e", "A", "-e", "B"])
class TestPostgresDumpCarriesThePassword(unittest.TestCase):
def test_the_cluster_dump_forwards_pgpassword(self) -> None:
seen: dict = {}
def fake(command, out_file, *, env=None):
seen["command"] = command
seen["env"] = env
import baudolo.backup.db as db
original = db.execute_to_file
db.execute_to_file = fake
try:
fallback_pg_dumpall("pg", "user", "secret", "/tmp/out.sql")
finally:
db.execute_to_file = original
self.assertIn("-e", seen["command"])
self.assertEqual(seen["command"][seen["command"].index("-e") + 1], "PGPASSWORD")
self.assertEqual(seen["env"], {"PGPASSWORD": "secret"})
self.assertNotIn("secret", seen["command"])
if __name__ == "__main__":
unittest.main()

67
tests/unit/backup/test_get_instance.py Normal file
View File

@@ -0,0 +1,67 @@
"""Which databases.csv instance a container name resolves to.
The cases are the container names real deployments produce, in both compose
and swarm, so a change to the regex has to state which shape it gives up.
"""
from __future__ import annotations
import unittest
from baudolo.backup.db import get_instance
class TestDeclaredContainers(unittest.TestCase):
def test_a_declared_container_is_its_own_instance(self) -> None:
self.assertEqual(
get_instance("postgres-central", ["postgres-central"]), "postgres-central"
)
def test_a_declaration_beats_the_regex(self) -> None:
"""A declared name is taken whole even when it carries a token the
fallback would otherwise strip."""
self.assertEqual(
get_instance("shop-database", ["shop-database"]), "shop-database"
)
def test_an_undeclared_central_engine_resolves_to_nothing(self) -> None:
"""`postgres-central` has no separator before its token, so nothing is
stripped - a central engine has to be declared to be found."""
self.assertIsNone(get_instance("postgres-central", []))
class TestDedicatedEngines(unittest.TestCase):
def test_compose_names_the_container_with_a_hyphen(self) -> None:
self.assertEqual(get_instance("discourse-database", []), "discourse")
def test_swarm_names_the_task_with_an_underscore_and_a_slot(self) -> None:
"""Swarm suppresses container_name and names the task
<stack>_<service>.<slot>.<id>, which must land on the same instance as
the compose name so one databases.csv serves both modes."""
self.assertEqual(get_instance("discourse_database.1.k3f9x2", []), "discourse")
def test_an_explicitly_named_engine_keeps_its_entity(self) -> None:
self.assertEqual(get_instance("bigbluebutton-postgres-1", []), "bigbluebutton")
def test_the_short_token_is_stripped_too(self) -> None:
self.assertEqual(get_instance("matomo-db", []), "matomo")
def test_mariadb_uses_the_same_suffix(self) -> None:
self.assertEqual(get_instance("matomo-database", []), "matomo")
class TestApplicationContainers(unittest.TestCase):
def test_a_bare_application_name_is_not_a_database(self) -> None:
"""Returning the name unchanged here would offer the application as a
second engine for its own dedicated database's instance."""
self.assertIsNone(get_instance("discourse", []))
def test_a_swarm_application_task_is_not_a_database(self) -> None:
self.assertIsNone(get_instance("discourse_discourse.1.k3f9x2", []))
def test_an_application_that_merely_starts_with_a_token_is_not_split(self) -> None:
self.assertIsNone(get_instance("dbeaver", []))
if __name__ == "__main__":
unittest.main()