From de8d57904af8f6f58b8370a9f0d3955c2743193c Mon Sep 17 00:00:00 2001 From: Jacques Raphanel Date: Mon, 17 Aug 2026 16:54:03 +0000 Subject: [PATCH 1/5] feat(cli): add explicit dump subcommand dump was the only action without a name, implicit whenever no subcommand was given. Make it a real subcommand and keep the bare form working as a shorthand. Options are split into common/source/selection/dump groups, declared on the root parser as hidden aliases and on each subcommand with SUPPRESS defaults so values parsed before the verb are not clobbered. migrate and validate now accept the source and filtering flags after the verb too. --- README.rst | 14 +- db2sql/interface/cli/boolean_action.py | 9 +- db2sql/interface/cli/once_argument.py | 6 +- db2sql/interface/cli/parser.py | 356 ++++++++++++++++-------- db2sql/interface/cli/runner.py | 2 + docs/cli.rst | 169 ++++++++--- docs/index.rst | 6 +- docs/plugins.rst | 6 +- tests/cli/test_cli_sqlite.py | 37 +++ tests/unit/interface/cli/test_parser.py | 132 +++++++++ 10 files changed, 578 insertions(+), 159 deletions(-) diff --git a/README.rst b/README.rst index 1dc33d7..3b0448a 100644 --- a/README.rst +++ b/README.rst @@ -7,8 +7,12 @@ db2sql Two output modes are supported: -* **Dump mode** (default) — write a SQL file (or stream to ``stdout``) that can later be replayed with ``psql -f`` or ``sqlcmd -i``. -* **Migrate mode** — open a live connection to the target database and apply the same DDL and data directly, without an intermediate file. The DDL produced is byte-identical to dump mode: a single ``SqlEmitter`` is the source of truth in both paths. +* ``db2sql dump`` (the default command) — write a SQL file (or stream to ``stdout``) that can later be replayed with ``psql -f`` or ``sqlcmd -i``. +* ``db2sql migrate`` — open a live connection to the target database and apply the same DDL and data directly, without an intermediate file. The DDL produced is byte-identical to dump mode: a single ``SqlEmitter`` is the source of truth in both paths. + +Two helper commands round out the CLI: ``db2sql init`` generates a configuration file through an interactive wizard, and ``db2sql validate`` checks one (optionally previewing the export plan) before a long run. + +Running ``db2sql`` with dump options but no command is a shorthand for ``db2sql dump`` — both forms are supported and produce identical output. Installation @@ -46,7 +50,7 @@ dump, but without the round-trip through a ``.sql`` file: .. code-block:: console # SQLite source → live Postgres target - $ db2sql --driver sqlite --dbname mydb.sqlite migrate \ + $ db2sql migrate --driver sqlite --dbname mydb.sqlite \ --target-host localhost --target-port 5432 \ --target-dbname mytarget --target-user postgres --target-password s3cr3t @@ -69,7 +73,7 @@ order: .. code-block:: console - $ db2sql --driver sqlite --dbname mydb.sqlite --on-existing drop -f dump.sql + $ db2sql dump --driver sqlite --dbname mydb.sqlite --on-existing drop -f dump.sql Pass ``--on-existing truncate`` to produce a *data-only* script: no DDL is emitted, the dump just ``TRUNCATE``\s every managed table and reloads its @@ -77,7 +81,7 @@ rows. Use it to refresh data into a pre-existing schema: .. code-block:: console - $ db2sql --driver sqlite --dbname mydb.sqlite --on-existing truncate -f refresh.sql + $ db2sql dump --driver sqlite --dbname mydb.sqlite --on-existing truncate -f refresh.sql Validating a configuration diff --git a/db2sql/interface/cli/boolean_action.py b/db2sql/interface/cli/boolean_action.py index 089f05c..5089ac2 100644 --- a/db2sql/interface/cli/boolean_action.py +++ b/db2sql/interface/cli/boolean_action.py @@ -16,8 +16,13 @@ def __init__(self, option_strings: Sequence[str], dest: str, **kwargs: Any) -> N if option_string.startswith("--"): _option_strings.append("--no-" + option_string[2:]) - if kwargs.get("help") is not None and kwargs.get("default") is not None: - kwargs["help"] += f" (default: {kwargs['default']})" + default = kwargs.get("default") + if ( + kwargs.get("help") is not None + and default is not None + and default is not argparse.SUPPRESS + ): + kwargs["help"] += f" (default: {default})" super().__init__(option_strings=_option_strings, dest=dest, nargs=0, **kwargs) diff --git a/db2sql/interface/cli/once_argument.py b/db2sql/interface/cli/once_argument.py index 10a7785..8d840f4 100644 --- a/db2sql/interface/cli/once_argument.py +++ b/db2sql/interface/cli/once_argument.py @@ -16,7 +16,11 @@ def __call__( values: Union[str, Any, Sequence[Any], None], option_string: Optional[str] = None, ) -> None: - if getattr(namespace, self.dest) is not None and self.default is None: + # A SUPPRESS default means argparse leaves the attribute unset until the + # flag is seen, so a present value can only come from a first occurrence + # — same situation as a None default, and the check applies as well. + unset_by_default = self.default is None or self.default is argparse.SUPPRESS + if getattr(namespace, self.dest, None) is not None and unset_by_default: msg = f"{option_string or 'undefined'} can only be specified once" raise argparse.ArgumentError(None, msg) setattr(namespace, self.dest, values) diff --git a/db2sql/interface/cli/parser.py b/db2sql/interface/cli/parser.py index 4f6eabf..48aceb6 100644 --- a/db2sql/interface/cli/parser.py +++ b/db2sql/interface/cli/parser.py @@ -25,6 +25,7 @@ from .once_argument import OnceArgument from .smart_formatter import SmartFormatter +COMMAND_DUMP = "dump" COMMAND_INIT = "init" COMMAND_VALIDATE = "validate" COMMAND_MIGRATE = "migrate" @@ -117,18 +118,88 @@ def _parse_size(value: str) -> int: return bytes_value -def _add_dump_options(parser: argparse.ArgumentParser) -> None: - """Add the connection/dump options shared by the implicit dump command.""" +def _value(default: Any, *, defaults: bool) -> Any: + """Return ``default`` for the canonical parser, ``SUPPRESS`` for the aliases. + + Every option is declared twice: once on the root parser (legacy implicit + dump, keeps its real default) and once on the subcommand that owns it. The + subcommand copy must default to :data:`argparse.SUPPRESS` so argparse omits + it from the sub-namespace when the flag is absent — otherwise the value + parsed before the subcommand would be clobbered by the subparser default. + """ + return default if defaults else argparse.SUPPRESS + + +def _text(help_text: str, *, visible: bool) -> str: + """Hide the help of the legacy root-level aliases without disabling them.""" + return help_text if visible else argparse.SUPPRESS + + +def _add_common_options( + parser: argparse.ArgumentParser, *, defaults: bool = True, visible: bool = True +) -> None: + """Add the options that apply to every command (config, logging, version).""" + parser.add_argument( + "-C", + "--config-file", + metavar="PATH", + dest="config_file", + type=str, + default=_value(None, defaults=defaults), + help=_text( + f"Configuration file to use. [env var: {const.ENV_DB2SQL_CONFIG}]", + visible=visible, + ), + ) + parser.add_argument( + "-L", + "--log-file", + metavar="PATH", + dest="log_file", + type=str, + default=_value(None, defaults=defaults), + help=_text("Send log output to PATH instead of stdout.", visible=visible), + action=OnceArgument, + ) + parser.add_argument( + "-V", + "--verbosity", + metavar="LEVEL", + dest="verbosity", + default=_value("status", defaults=defaults), + nargs="?", + type=str, + help=_text( + "Level of detail of the output. Valid options from less verbose to " + "more verbose: -Vquiet, -Verror, -Vwarning, -Vnotice, -Vstatus, " + "-V or -Vverbose, -VV or -Vdebug, -VVV or -Vtrace", + visible=visible, + ), + ) + parser.add_argument( + "--version", + dest="version", + action="store_true", + default=_value(False, defaults=defaults), + help=_text("Output version information and exit.", visible=visible), + ) + + +def _add_source_options( + parser: argparse.ArgumentParser, *, defaults: bool = True, visible: bool = True +) -> None: + """Add the source-connection options shared by dump, migrate and validate.""" parser.add_argument( "--driver", dest="driver", metavar="NAME", type=str, - default=os.getenv(const.ENV_DB2SQL_DRIVER), - help=( + default=_value(os.getenv(const.ENV_DB2SQL_DRIVER), defaults=defaults), + help=_text( "Source database driver. Built-in: " f"{', '.join(available_readers()) or '(none registered)'}. " - f"[env var: {const.ENV_DB2SQL_DRIVER}]" + f"[env var: {const.ENV_DB2SQL_DRIVER}]", + visible=visible, ), action=OnceArgument, ) @@ -137,11 +208,12 @@ def _add_dump_options(parser: argparse.ArgumentParser) -> None: dest="target", metavar="NAME", type=str, - default=os.getenv(const.ENV_DB2SQL_TARGET), - help=( + default=_value(os.getenv(const.ENV_DB2SQL_TARGET), defaults=defaults), + help=_text( "Target SQL dialect to emit. Built-in: " f"{', '.join(available_emitters()) or '(none registered)'}. " - f"[env var: {const.ENV_DB2SQL_TARGET}] (default: postgres)" + f"[env var: {const.ENV_DB2SQL_TARGET}] (default: postgres)", + visible=visible, ), action=OnceArgument, ) @@ -151,8 +223,11 @@ def _add_dump_options(parser: argparse.ArgumentParser) -> None: metavar="HOSTNAME", dest="hostname", type=str, - help=f"Database server host name. [env var: {const.ENV_DB2SQL_HOST}]", - default=os.getenv(const.ENV_DB2SQL_HOST), + help=_text( + f"Database server host name. [env var: {const.ENV_DB2SQL_HOST}]", + visible=visible, + ), + default=_value(os.getenv(const.ENV_DB2SQL_HOST), defaults=defaults), action=OnceArgument, ) parser.add_argument( @@ -161,8 +236,11 @@ def _add_dump_options(parser: argparse.ArgumentParser) -> None: metavar="PORT", dest="port", type=int, - help=f"Database server port. [env var: {const.ENV_DB2SQL_PORT}]", - default=os.getenv(const.ENV_DB2SQL_PORT), + help=_text( + f"Database server port. [env var: {const.ENV_DB2SQL_PORT}]", + visible=visible, + ), + default=_value(os.getenv(const.ENV_DB2SQL_PORT), defaults=defaults), action=OnceArgument, ) parser.add_argument( @@ -171,8 +249,11 @@ def _add_dump_options(parser: argparse.ArgumentParser) -> None: metavar="DBNAME", dest="dbname", type=str, - help=f"Database name to connect to. [env var: {const.ENV_DB2SQL_DBNAME}]", - default=os.getenv(const.ENV_DB2SQL_DBNAME), + help=_text( + f"Database name to connect to. [env var: {const.ENV_DB2SQL_DBNAME}]", + visible=visible, + ), + default=_value(os.getenv(const.ENV_DB2SQL_DBNAME), defaults=defaults), action=OnceArgument, ) parser.add_argument( @@ -181,8 +262,11 @@ def _add_dump_options(parser: argparse.ArgumentParser) -> None: metavar="USERNAME", dest="username", type=str, - help=f"Database user name. [env var: {const.ENV_DB2SQL_USER}]", - default=os.getenv(const.ENV_DB2SQL_USER), + help=_text( + f"Database user name. [env var: {const.ENV_DB2SQL_USER}]", + visible=visible, + ), + default=_value(os.getenv(const.ENV_DB2SQL_USER), defaults=defaults), action=OnceArgument, ) parser.add_argument( @@ -191,8 +275,11 @@ def _add_dump_options(parser: argparse.ArgumentParser) -> None: metavar="PASSWORD", dest="password", type=str, - help=f"Database password. [env var: {const.ENV_DB2SQL_PASSWORD}]", - default=os.getenv(const.ENV_DB2SQL_PASSWORD), + help=_text( + f"Database password. [env var: {const.ENV_DB2SQL_PASSWORD}]", + visible=visible, + ), + default=_value(os.getenv(const.ENV_DB2SQL_PASSWORD), defaults=defaults), action=OnceArgument, ) parser.add_argument( @@ -200,57 +287,28 @@ def _add_dump_options(parser: argparse.ArgumentParser) -> None: "--ask-password", dest="ask_password", action="store_true", - default=False, - help="Force password prompt.", - ) - parser.add_argument( - "-f", - "--file", - metavar="PATH", - dest="output_file_name", - type=str, - default=None, - help="Output file. If not provided, script is printed to standard output.", - ) - parser.add_argument( - "--split-size", - metavar="SIZE", - dest="split_size", - type=_parse_size, - default=None, - help=( - "Split the dump into multiple files when the current file exceeds " - "SIZE. Accepts a byte count or a suffixed value (K/M/G). Requires -f." - ), - ) - parser.add_argument( - "--on-existing", - dest="dump_on_existing", - choices=["fail", "drop", "truncate"], - default=None, - help=( - "Strategy when a target object already exists: 'fail' (default) " - "emits CREATE only; 'drop' prepends a DROP TABLE IF EXISTS for " - "every table in reverse-dependency order; 'truncate' emits a " - "data-only script (TRUNCATE + reload, no DDL)." - ), + default=_value(False, defaults=defaults), + help=_text("Force password prompt.", visible=visible), ) + + +def _add_selection_options( + parser: argparse.ArgumentParser, *, defaults: bool = True, visible: bool = True +) -> None: + """Add the options that shape *what* is exported. + + These are shared by ``dump`` and ``migrate``: both paths feed the same + :class:`DumpOptions` / :class:`FilterRules` so the emitted DDL stays + identical, and by ``validate --dry-run`` so the printed plan matches. + """ parser.add_argument( "--preserve-case", dest="preserve_case", action=BooleanAction, - default=None, - help="Preserve identifier case. When disabled, names are converted to snake_case.", - ) - parser.add_argument( - "--transaction", - dest="dump_use_transaction", - action=BooleanAction, - default=None, - help=( - "Wrap the dump in a transaction (BEGIN/COMMIT). Disable with " - "--no-transaction when the SQL is consumed by a tool that manages " - "its own transaction or when chunked replay is preferred." + default=_value(None, defaults=defaults), + help=_text( + "Preserve identifier case. When disabled, names are converted to snake_case.", + visible=visible, ), ) parser.add_argument( @@ -258,15 +316,21 @@ def _add_dump_options(parser: argparse.ArgumentParser) -> None: "--max-records", dest="limit_records", type=int, - default=None, - help="Limit the number of rows from each table. -1 means no limit.", + default=_value(None, defaults=defaults), + help=_text( + "Limit the number of rows from each table. -1 means no limit.", + visible=visible, + ), ) parser.add_argument( "--data-format", dest="data_format", choices=[fmt.value for fmt in DataFormat], - default=None, - help="Default output format for table data: copy (faster) or insert.", + default=_value(None, defaults=defaults), + help=_text( + "Default output format for table data: copy (faster) or insert.", + visible=visible, + ), ) parser.add_argument( "-i", @@ -276,8 +340,11 @@ def _add_dump_options(parser: argparse.ArgumentParser) -> None: type=str, action="append", nargs="+", - default=None, - help="Schema names to include during export (repeatable, comma separated).", + default=_value(None, defaults=defaults), + help=_text( + "Schema names to include during export (repeatable, comma separated).", + visible=visible, + ), ) parser.add_argument( "-x", @@ -287,8 +354,11 @@ def _add_dump_options(parser: argparse.ArgumentParser) -> None: type=str, action="append", nargs="+", - default=None, - help="Schema names to exclude during export (repeatable, comma separated).", + default=_value(None, defaults=defaults), + help=_text( + "Schema names to exclude during export (repeatable, comma separated).", + visible=visible, + ), ) parser.add_argument( "-I", @@ -298,8 +368,11 @@ def _add_dump_options(parser: argparse.ArgumentParser) -> None: type=str, action="append", nargs="+", - default=None, - help="Table names to include during export (repeatable, comma separated).", + default=_value(None, defaults=defaults), + help=_text( + "Table names to include during export (repeatable, comma separated).", + visible=visible, + ), ) parser.add_argument( "-X", @@ -309,49 +382,94 @@ def _add_dump_options(parser: argparse.ArgumentParser) -> None: type=str, action="append", nargs="+", - default=None, - help="Table names to exclude during export (repeatable, comma separated).", + default=_value(None, defaults=defaults), + help=_text( + "Table names to exclude during export (repeatable, comma separated).", + visible=visible, + ), ) + + +def _add_dump_options( + parser: argparse.ArgumentParser, *, defaults: bool = True, visible: bool = True +) -> None: + """Add the options that only make sense when writing a SQL file.""" parser.add_argument( - "-C", - "--config-file", + "-f", + "--file", metavar="PATH", - dest="config_file", + dest="output_file_name", type=str, - help=f"Configuration file to use. [env var: {const.ENV_DB2SQL_CONFIG}]", + default=_value(None, defaults=defaults), + help=_text( + "Output file. If not provided, script is printed to standard output.", + visible=visible, + ), ) parser.add_argument( - "-L", - "--log-file", - metavar="PATH", - dest="log_file", - type=str, - help="Send log output to PATH instead of stdout.", - action=OnceArgument, + "--split-size", + metavar="SIZE", + dest="split_size", + type=_parse_size, + default=_value(None, defaults=defaults), + help=_text( + "Split the dump into multiple files when the current file exceeds " + "SIZE. Accepts a byte count or a suffixed value (K/M/G). Requires -f.", + visible=visible, + ), ) parser.add_argument( - "-V", - "--verbosity", - metavar="LEVEL", - dest="verbosity", - default="status", - nargs="?", - type=str, - help=( - "Level of detail of the output. Valid options from less verbose to " - "more verbose: -Vquiet, -Verror, -Vwarning, -Vnotice, -Vstatus, " - "-V or -Vverbose, -VV or -Vdebug, -VVV or -Vtrace" + "--on-existing", + dest="dump_on_existing", + choices=["fail", "drop", "truncate"], + default=_value(None, defaults=defaults), + help=_text( + "Strategy when a target object already exists: 'fail' (default) " + "emits CREATE only; 'drop' prepends a DROP TABLE IF EXISTS for " + "every table in reverse-dependency order; 'truncate' emits a " + "data-only script (TRUNCATE + reload, no DDL).", + visible=visible, ), ) parser.add_argument( - "--version", - dest="version", - action="store_true", - default=False, - help="Output version information and exit.", + "--transaction", + dest="dump_use_transaction", + action=BooleanAction, + default=_value(None, defaults=defaults), + help=_text( + "Wrap the dump in a transaction (BEGIN/COMMIT). Disable with " + "--no-transaction when the SQL is consumed by a tool that manages " + "its own transaction or when chunked replay is preferred.", + visible=visible, + ), ) +def _add_dump_subparser(subparsers: Any) -> None: + """Add the ``dump`` subcommand that writes SQL to a file or stdout. + + ``dump`` is also the default command: invoking ``db2sql`` with dump options + but no subcommand behaves identically. The explicit form is the documented + one — it keeps the four commands symmetric and gives the dump options a + help page of their own. + """ + dump_parser = subparsers.add_parser( + COMMAND_DUMP, + help="Write the source database as a SQL script (default command).", + description=( + "Read metadata and rows from the source database and write a SQL " + "script for the dialect selected by --target, either to a file " + "(-f) or to standard output. This is the default command: running " + "'db2sql' with no subcommand runs 'db2sql dump'." + ), + formatter_class=SmartFormatter, + ) + _add_source_options(dump_parser, defaults=False) + _add_selection_options(dump_parser, defaults=False) + _add_dump_options(dump_parser, defaults=False) + _add_common_options(dump_parser, defaults=False) + + def _add_validate_subparser(subparsers: Any) -> None: """Add the ``validate`` subcommand. @@ -402,6 +520,9 @@ def _add_validate_subparser(subparsers: Any) -> None: "May be slow on large tables — issues one SELECT per table." ), ) + _add_source_options(validate_parser, defaults=False) + _add_selection_options(validate_parser, defaults=False) + _add_common_options(validate_parser, defaults=False) def _add_migrate_subparser(subparsers: Any) -> None: @@ -417,11 +538,11 @@ def _add_migrate_subparser(subparsers: Any) -> None: help="Stream source database into a live target database (no SQL file).", description=( "Read metadata and rows from the source database (defined by the " - "top-level --driver/-H/-P/... flags) and apply them directly to a " - "live target database. The DDL is produced by the same SqlEmitter " - "used by the file dump, so the resulting target schema is " - "byte-identical to what 'db2sql > dump.sql && psql -f dump.sql' " - "would have produced." + "--driver/-H/-P/... flags) and apply them directly to a live " + "target database. The DDL is produced by the same SqlEmitter used " + "by the file dump, so the resulting target schema is " + "byte-identical to what 'db2sql dump -f dump.sql && psql -f " + "dump.sql' would have produced." ), formatter_class=SmartFormatter, ) @@ -514,6 +635,9 @@ def _add_migrate_subparser(subparsers: Any) -> None: "statement." ), ) + _add_source_options(migrate_parser, defaults=False) + _add_selection_options(migrate_parser, defaults=False) + _add_common_options(migrate_parser, defaults=False) def _add_init_subparser(subparsers: Any) -> None: @@ -548,20 +672,30 @@ def _add_init_subparser(subparsers: Any) -> None: def build_parser() -> MsDumpToPGArgumentParser: parser = MsDumpToPGArgumentParser( description=( - "Dump any supported source database into a PostgreSQL or Microsoft " - "SQL Server SQL file (selectable via --target)." + "Move any supported source database into a PostgreSQL or Microsoft " + "SQL Server target (selectable via --target): 'dump' writes a SQL " + "script, 'migrate' applies it to a live database. Running db2sql " + "with no COMMAND is a shorthand for 'db2sql dump'." ), prog="db2sql", formatter_class=SmartFormatter, add_help=True, ) - _add_dump_options(parser) + _add_common_options(parser) + + # The dump options are also accepted directly on the root parser so the + # pre-subcommand form ('db2sql --driver sqlite -f out.sql') keeps working. + # Their help is suppressed: 'db2sql dump --help' is the documented page. + _add_source_options(parser, visible=False) + _add_selection_options(parser, visible=False) + _add_dump_options(parser, visible=False) subparsers = parser.add_subparsers( dest="command", title="Commands", metavar="COMMAND", ) + _add_dump_subparser(subparsers) _add_init_subparser(subparsers) _add_validate_subparser(subparsers) _add_migrate_subparser(subparsers) diff --git a/db2sql/interface/cli/runner.py b/db2sql/interface/cli/runner.py index 051d26e..32865a6 100644 --- a/db2sql/interface/cli/runner.py +++ b/db2sql/interface/cli/runner.py @@ -92,6 +92,8 @@ def run(self, *args: Any) -> ExitCode: self._execute_migrate(options.config, target_driver) return SUCCESS + # Either an explicit COMMAND_DUMP or no subcommand at all: dump is + # the default command, so both land here. self._execute(options.config) except Exception as exc: if not isinstance(exc, AbortExecution): diff --git a/docs/cli.rst b/docs/cli.rst index a446620..8cc4742 100644 --- a/docs/cli.rst +++ b/docs/cli.rst @@ -6,17 +6,22 @@ Synopsis .. code-block:: text - db2sql [OPTIONS] [--on-existing {fail,drop,truncate}] - [--transaction | --no-transaction] - db2sql validate [CONFIG_FILE] [--dry-run] [--with-counts] - db2sql init [-o PATH] [--force] - db2sql migrate [--target-host HOST] [--target-port PORT] [--target-dbname DB] + db2sql [GLOBAL OPTIONS] COMMAND [OPTIONS] + + db2sql dump [SOURCE OPTIONS] [FILTERING OPTIONS] + [-f PATH] [--split-size SIZE] + [--on-existing {fail,drop,truncate}] + [--transaction | --no-transaction] + db2sql migrate [SOURCE OPTIONS] [FILTERING OPTIONS] + [--target-host HOST] [--target-port PORT] [--target-dbname DB] [--target-user USER] [--target-password PWD] [--target-driver NAME] [--on-existing {fail,drop,truncate}] [--transaction-mode {single,per_table}] [--transaction | --no-transaction] [--batch-size N] + db2sql validate [CONFIG_FILE] [--dry-run] [--with-counts] + db2sql init [-o PATH] [--force] Description ----------- @@ -24,14 +29,31 @@ Description ``db2sql`` reads the structure and data of a source database and either: * writes a SQL dump in the chosen target dialect (PostgreSQL or Microsoft - SQL Server) to a file or ``stdout`` — the default behaviour, and -* applies the same DDL and rows directly to a live target database when - invoked via the ``migrate`` subcommand — see :ref:`cli-migrate`. + SQL Server) to a file or ``stdout`` — the ``dump`` command, see + :ref:`cli-dump`, and +* applies the same DDL and rows directly to a live target database — the + ``migrate`` command, see :ref:`cli-migrate`. The DDL produced is identical in both modes: a single ``SqlEmitter`` per target dialect is the source of truth, regardless of whether the SQL ends up in a file or is executed live. +.. _cli-default-command: + +.. note:: + + ``dump`` is the **default command**: running ``db2sql`` with dump options + but no ``COMMAND`` is a shorthand for ``db2sql dump``. Both forms are + supported and produce identical output — the explicit form is the + documented one, and the only one with a ``--help`` page of its own:: + + $ db2sql --driver sqlite -d myapp.sqlite -f dump.sql # shorthand + $ db2sql dump --driver sqlite -d myapp.sqlite -f dump.sql # explicit + + Source and filtering options may also be given *before* the command + (``db2sql --driver sqlite dump -f out.sql``); when the same flag appears on + both sides, the one after the command wins. + Connection options, filtering rules, and output settings can be supplied via the :doc:`configuration` file, environment variables, or CLI flags. **CLI flags always take precedence over the config file**, which itself takes @@ -44,6 +66,23 @@ series of questions and produces a ready-to-use configuration file — see Options ------- +Which command accepts what: + +* **Connection** and **Filtering** options describe the *source* and *what is + read from it*. They are accepted by ``dump``, ``migrate`` and ``validate`` + alike — the three commands feed the same reader and the same filtering + rules. ``migrate`` adds its own ``--target-*`` flags for the destination + connection. +* **General** options are accepted by every command, before or after the + command name. +* Under **Output**, :option:`-f`, :option:`--split-size`, + :option:`--on-existing` and :option:`--transaction` are **dump-only** — + they describe the SQL file. ``migrate`` declares its own ``--on-existing`` + and ``--transaction`` with migration semantics, see :ref:`cli-migrate`. + The remaining three (:option:`--data-format`, :option:`--preserve-case`, + :option:`-n`) shape the emitted DDL and rows, so they apply to ``migrate`` + too. + Connection ~~~~~~~~~~ @@ -366,7 +405,7 @@ Example … Wrote configuration to db2sql.yml - $ db2sql -C db2sql.yml -f dump.sql + $ db2sql dump -C db2sql.yml -f dump.sql Environment variables --------------------- @@ -416,6 +455,61 @@ take precedence when both are present. Subcommands ----------- +.. _cli-dump: + +``db2sql dump`` +~~~~~~~~~~~~~~~ + +Write the source database as a SQL script, either to a file (:option:`-f`) or +to ``stdout``. This is the default command — see +:ref:`the shorthand form ` described above. + +.. code-block:: text + + db2sql dump [--driver NAME] [--target NAME] + [-H HOSTNAME] [-P PORT] [-d DBNAME] + [-u USERNAME] [-p PASSWORD] [-W] + [-i NAME …] [-x NAME …] [-I NAME …] [-X NAME …] + [--preserve-case | --no-preserve-case] + [--data-format {copy,insert}] [-n N] + [-f PATH] [--split-size SIZE] + [--on-existing {fail,drop,truncate}] + [--transaction | --no-transaction] + +Every flag is documented above: the source connection under `Connection`_, +the include/exclude rules under `Filtering`_, and the file-level settings +under `Output`_. + +Exit codes: + +.. list-table:: + :header-rows: 1 + :widths: 15 85 + + * - Code + - Meaning + * - ``0`` + - The dump completed and was fully written. + * - ``5`` + - Configuration error, or an unknown ``--driver`` / ``--target``. + * - ``1`` + - The source read failed (bad credentials, missing tables, permission + errors…), or the output could not be written. + +Examples: + +.. code-block:: console + + # SQLite source → PostgreSQL SQL file + $ db2sql dump --driver sqlite -d myapp.sqlite -f dump.sql + + # stream to stdout and load straight into a running Postgres instance + $ db2sql dump --driver mysql -H mysql.example.com -d mydb -u app -p s3cr3t \ + | psql -h pg.example.com -U app -d mydb_imported + + # replayable dump, split into 100 MB parts + $ db2sql dump -C db2sql.yml --on-existing drop -f dump.sql --split-size 100M + ``db2sql validate`` ~~~~~~~~~~~~~~~~~~~ @@ -502,14 +596,20 @@ Examples: Apply the source database directly to a live target database, without going through an intermediate ``.sql`` file. The DDL emitted to the target is -byte-identical to what ``db2sql > dump.sql && psql -f dump.sql`` would have +byte-identical to what ``db2sql dump -f dump.sql && psql -f dump.sql`` would have produced — only the row-data transport differs (the migrate path uses the target's native bulk-load primitive: ``COPY FROM STDIN`` for PostgreSQL, batched ``executemany`` for MSSQL). .. code-block:: text - db2sql migrate [--target-host HOSTNAME] [--target-port PORT] + db2sql migrate [--driver NAME] [--target NAME] + [-H HOSTNAME] [-P PORT] [-d DBNAME] + [-u USERNAME] [-p PASSWORD] [-W] + [-i NAME …] [-x NAME …] [-I NAME …] [-X NAME …] + [--preserve-case | --no-preserve-case] + [--data-format {copy,insert}] [-n N] + [--target-host HOSTNAME] [--target-port PORT] [--target-dbname DBNAME] [--target-user USERNAME] [--target-password PASSWORD] [--target-driver NAME] [--on-existing {fail,drop,truncate}] @@ -517,10 +617,11 @@ batched ``executemany`` for MSSQL). [--transaction | --no-transaction] [--batch-size N] -The source is configured exactly like for a file dump (top-level +The source is configured exactly like for a file dump (the same ``--driver`` / ``-H`` / ``-P`` / ``-d`` / ``-u`` / ``-p`` flags, the ``server:`` section of the config file, or the ``DB2SQL_*`` environment -variables). The target connection is configured via the ``--target-*`` +variables). Those flags are accepted either before or after the ``migrate`` +keyword. The target connection is configured via the ``--target-*`` flags below, the ``target_server:`` section of the config file, or the ``DB2SQL_TARGET_*`` environment variables. @@ -623,19 +724,19 @@ Examples: .. code-block:: console # SQLite → live Postgres - $ db2sql --driver sqlite --dbname mydb.sqlite migrate \ + $ db2sql migrate --driver sqlite --dbname mydb.sqlite \ --target-host localhost --target-port 5432 \ --target-dbname mytarget --target-user postgres --target-password s3cr3t # MSSQL source → live MSSQL target (different instance), via a config file - $ db2sql -C migrate.yml migrate + $ db2sql migrate -C migrate.yml # Use environment variables for the target credentials (recommended) $ export DB2SQL_TARGET_HOST=db.internal \ DB2SQL_TARGET_DBNAME=stage \ DB2SQL_TARGET_USER=svc_migrate \ DB2SQL_TARGET_PASSWORD=$(vault read -field=password kv/db) - $ db2sql --driver mssql -H prod-mssql -d sales -u readonly -p $SOURCE_PWD migrate + $ db2sql migrate --driver mssql -H prod-mssql -d sales -u readonly -p $SOURCE_PWD Examples -------- @@ -658,7 +759,7 @@ with ``server.options.schema``). .. code-block:: console - $ db2sql --driver sqlite --dbname ./myapp.sqlite -f dump.sql + $ db2sql dump --driver sqlite --dbname ./myapp.sqlite -f dump.sql For a custom logical schema name, drop a config file alongside the dump and reference it with :option:`-C`: @@ -674,7 +775,7 @@ and reference it with :option:`-C`: .. code-block:: console - $ db2sql -C db2sql.yml -f dump.sql + $ db2sql dump -C db2sql.yml -f dump.sql MySQL ^^^^^ @@ -684,14 +785,14 @@ is ``3306``. .. code-block:: console - $ db2sql --driver mysql \ + $ db2sql dump --driver mysql \ -H mysql.example.com -P 3306 \ -u app -W \ -d mydb \ -f dump.sql # piping straight into a target Postgres instance via psql - $ db2sql --driver mysql -H mysql.example.com -d mydb -u app -p s3cr3t \ + $ db2sql dump --driver mysql -H mysql.example.com -d mydb -u app -p s3cr3t \ | psql "host=pg.example.com dbname=mydb user=app" Microsoft SQL Server (as a source) @@ -703,7 +804,7 @@ convention ``public``. .. code-block:: console - $ db2sql --driver mssql \ + $ db2sql dump --driver mssql \ -H sqlserver.example.com -P 1433 \ -u sa -W \ -d mydb \ @@ -715,7 +816,7 @@ convention ``public``. $ export DB2SQL_DBNAME=mydb $ export DB2SQL_USER=sa $ export DB2SQL_PASSWORD=secret - $ db2sql -f dump.sql + $ db2sql dump -f dump.sql PostgreSQL (as a source) ^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -725,7 +826,7 @@ Default port is ``5432``. .. code-block:: console - $ db2sql --driver postgres \ + $ db2sql dump --driver postgres \ -H pg.example.com -P 5432 \ -u app -p s3cr3t \ -d mydb \ @@ -761,7 +862,7 @@ owner: .. code-block:: console - $ db2sql -C oracle-hr.yml -f hr_dump.sql + $ db2sql dump -C oracle-hr.yml -f hr_dump.sql Via ``sid``, dumping every non-system schema: @@ -779,7 +880,7 @@ Via ``sid``, dumping every non-system schema: .. code-block:: console - $ db2sql -C oracle-all.yml -f full_dump.sql + $ db2sql dump -C oracle-all.yml -f full_dump.sql By target emitter ~~~~~~~~~~~~~~~~~ @@ -795,10 +896,10 @@ unless ``--data-format insert`` is requested. .. code-block:: console # explicit (postgres is the default — both forms are equivalent) - $ db2sql --driver sqlite --target postgres -d myapp.sqlite -f dump.sql + $ db2sql dump --driver sqlite --target postgres -d myapp.sqlite -f dump.sql # load straight into a running Postgres instance - $ db2sql --driver mssql -H sqlserver.example.com -d mydb -u sa -p s3cr3t \ + $ db2sql dump --driver mssql -H sqlserver.example.com -d mydb -u sa -p s3cr3t \ | psql -h pg.example.com -U app -d mydb_imported Microsoft SQL Server output @@ -818,15 +919,15 @@ columns, and emits schemas via .. code-block:: console # SQLite source → MSSQL output - $ db2sql --driver sqlite --target mssql -d myapp.sqlite -f dump.sql + $ db2sql dump --driver sqlite --target mssql -d myapp.sqlite -f dump.sql # MySQL source → MSSQL output, piped into sqlcmd - $ db2sql --driver mysql -H mysql.example.com -d mydb -u app -p s3cr3t \ + $ db2sql dump --driver mysql -H mysql.example.com -d mydb -u app -p s3cr3t \ --target mssql \ | sqlcmd -S sqlserver.example.com -d mydb_imported -U sa -P s3cr3t # Postgres source → MSSQL output, restricted to two schemas - $ db2sql --driver postgres -H pg.example.com -d mydb -u app -W \ + $ db2sql dump --driver postgres -H pg.example.com -d mydb -u app -W \ --target mssql \ -i public -i audit \ -f mssql_dump.sql @@ -838,7 +939,7 @@ Dump only two schemas, using INSERT statements: .. code-block:: console - $ db2sql --driver postgres -H localhost -d mydb \ + $ db2sql dump --driver postgres -H localhost -d mydb \ -i public -i audit \ --data-format insert \ -f dump.sql @@ -847,18 +948,18 @@ Produce a 100-row sample for every table (useful for development): .. code-block:: console - $ db2sql --driver mysql -H localhost -d mydb -n 100 -f sample.sql + $ db2sql dump --driver mysql -H localhost -d mydb -n 100 -f sample.sql Use a config file explicitly: .. code-block:: console - $ db2sql -C /etc/db2sql/production.yml -f dump.sql + $ db2sql dump -C /etc/db2sql/production.yml -f dump.sql Exclude a few tables from an otherwise complete MSSQL dump: .. code-block:: console - $ db2sql --driver mssql -H sqlserver.example.com -d mydb -u sa -W \ + $ db2sql dump --driver mssql -H sqlserver.example.com -d mydb -u sa -W \ -X audit_log -X temp_data \ -f dump.sql diff --git a/docs/index.rst b/docs/index.rst index 2b93985..03b4a6b 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -10,14 +10,14 @@ changes directly to a live target database (migrate mode). .. code-block:: console # SQLite → Postgres SQL file (default target) - $ db2sql --driver sqlite --dbname mydb.sqlite -f dump.sql + $ db2sql dump --driver sqlite --dbname mydb.sqlite -f dump.sql # MySQL → MSSQL SQL file - $ db2sql --driver mysql -H mysql.example.com -d mydb -u app -p s3cr3t \ + $ db2sql dump --driver mysql -H mysql.example.com -d mydb -u app -p s3cr3t \ --target mssql -f dump.sql # SQLite → live Postgres database (no intermediate file) - $ db2sql --driver sqlite --dbname mydb.sqlite migrate \ + $ db2sql migrate --driver sqlite --dbname mydb.sqlite \ --target-host localhost --target-dbname mytarget \ --target-user postgres --target-password s3cr3t diff --git a/docs/plugins.rst b/docs/plugins.rst index fa05531..b8f89c3 100644 --- a/docs/plugins.rst +++ b/docs/plugins.rst @@ -104,7 +104,7 @@ After ``pip install -e .``, the new driver is available immediately: .. code-block:: console - $ db2sql --driver mydriver --dbname … -f dump.sql + $ db2sql dump --driver mydriver --dbname … -f dump.sql Reader inputs come from :class:`~db2sql.infrastructure.config.schema.AppConfig`: @@ -177,7 +177,7 @@ And use it: .. code-block:: console - $ db2sql --target mytarget -f dump.sql + $ db2sql dump --target mytarget -f dump.sql Call sequence ~~~~~~~~~~~~~ @@ -269,7 +269,7 @@ And use it: .. code-block:: console - $ db2sql --target mytarget migrate --target-host … --target-dbname … + $ db2sql migrate --target mytarget --target-host … --target-dbname … Writer inputs come from :class:`~db2sql.infrastructure.config.schema.AppConfig`: diff --git a/tests/cli/test_cli_sqlite.py b/tests/cli/test_cli_sqlite.py index e5a309f..583cdb2 100644 --- a/tests/cli/test_cli_sqlite.py +++ b/tests/cli/test_cli_sqlite.py @@ -78,3 +78,40 @@ def test_cli_target_mssql_end_to_end(sample_db: Path, tmp_path: Path, monkeypatc # MSSQL output must not contain Postgres-only constructs assert "COPY " not in contents assert "serial" not in contents + + +def test_cli_explicit_dump_command_matches_implicit_form( + sample_db: Path, tmp_path: Path, monkeypatch +) -> None: + """``db2sql dump ...`` must produce exactly what the bare form produces.""" + monkeypatch.setattr(sys, "argv", ["db2sql"]) + implicit_file = tmp_path / "implicit.sql" + explicit_file = tmp_path / "explicit.sql" + + common = ["--driver", "sqlite", "-d", str(sample_db), "--preserve-case", "-f"] + assert Cli().run(common + [str(implicit_file)]) == 0 + assert Cli().run(["dump"] + common + [str(explicit_file)]) == 0 + + assert explicit_file.read_text() == implicit_file.read_text() + + +def test_cli_dump_command_accepts_options_before_and_after_the_verb( + sample_db: Path, tmp_path: Path, monkeypatch +) -> None: + """Flags may sit on either side of ``dump`` — the root aliases still parse.""" + monkeypatch.setattr(sys, "argv", ["db2sql"]) + output_file = tmp_path / "dump.sql" + rc = Cli().run( + [ + "--driver", + "sqlite", + "dump", + "-d", + str(sample_db), + "--preserve-case", + "-f", + str(output_file), + ] + ) + assert rc == 0 + assert 'COPY "public"."book"' in output_file.read_text() diff --git a/tests/unit/interface/cli/test_parser.py b/tests/unit/interface/cli/test_parser.py index ef7753e..00ca658 100644 --- a/tests/unit/interface/cli/test_parser.py +++ b/tests/unit/interface/cli/test_parser.py @@ -108,3 +108,135 @@ def test_migrate_on_existing_truncate_still_accepted(monkeypatch) -> None: parser = build_parser() ns = parser.parse_args_with_config(["migrate", "--on-existing", "truncate"]) assert ns.config.migrate.on_existing == "truncate" + + +@pytest.fixture() +def clean_env(monkeypatch, tmp_path: Path): + """No config file and no env-var defaults leaking into the parsed options.""" + for name in ( + "DB2SQL_CONFIG", + "DB2SQL_DRIVER", + "DB2SQL_TARGET", + "DB2SQL_HOST", + "DB2SQL_PORT", + "DB2SQL_DBNAME", + "DB2SQL_USER", + "DB2SQL_PASSWORD", + ): + monkeypatch.delenv(name, raising=False) + # Avoid picking up a ./db2sql.yml from the working directory. + monkeypatch.chdir(tmp_path) + return monkeypatch + + +_DUMP_ARGS = [ + "--driver", + "sqlite", + "-d", + "my.db", + "-f", + "out.sql", + "--on-existing", + "drop", + "--no-transaction", + "-I", + "book", + "-n", + "5", + "--data-format", + "insert", + "--preserve-case", +] + + +def test_dump_subcommand_is_registered(clean_env) -> None: + ns = build_parser().parse_args_with_config(["dump", "--driver", "sqlite"]) + assert ns.command == "dump" + + +def test_bare_invocation_has_no_command(clean_env) -> None: + """The implicit form stays supported and is reported as 'no subcommand'.""" + ns = build_parser().parse_args_with_config(["--driver", "sqlite"]) + assert ns.command is None + + +def test_explicit_dump_and_implicit_form_build_the_same_config(clean_env) -> None: + implicit = build_parser().parse_args_with_config(list(_DUMP_ARGS)).config + explicit = build_parser().parse_args_with_config(["dump"] + _DUMP_ARGS).config + assert explicit == implicit + + +def test_dump_options_may_straddle_the_subcommand(clean_env) -> None: + """Options before the verb are root aliases; options after win on conflict.""" + ns = build_parser().parse_args_with_config( + ["--driver", "sqlite", "dump", "-d", "my.db", "-f", "out.sql"] + ) + assert ns.config.driver == "sqlite" + assert ns.config.server.dbname == "my.db" + assert ns.config.output_file == "out.sql" + + +def test_dump_subcommand_defaults_do_not_clobber_root_values(clean_env) -> None: + """The SUPPRESS defaults keep pre-verb values alive across the sub-namespace.""" + ns = build_parser().parse_args_with_config(["-Vdebug", "--driver", "sqlite", "dump"]) + assert ns.verbosity == "debug" + assert ns.config.driver == "sqlite" + + +def test_global_options_are_accepted_after_the_subcommand(clean_env, tmp_path: Path) -> None: + log_file = tmp_path / "db2sql.log" + ns = build_parser().parse_args_with_config(["dump", "-Vdebug", "-L", str(log_file)]) + assert ns.verbosity == "debug" + assert ns.log_file == str(log_file) + + +def test_source_options_are_accepted_after_migrate(clean_env) -> None: + before = build_parser().parse_args_with_config( + ["--driver", "sqlite", "-d", "my.db", "migrate", "--target-host", "h"] + ) + after = build_parser().parse_args_with_config( + ["migrate", "--driver", "sqlite", "-d", "my.db", "--target-host", "h"] + ) + assert after.config == before.config + + +def test_source_options_are_accepted_after_validate(clean_env) -> None: + ns = build_parser().parse_args_with_config(["validate", "--driver", "sqlite", "--dry-run"]) + assert ns.command == "validate" + assert ns.config.driver == "sqlite" + assert ns.dry_run is True + + +def test_a_dbname_that_looks_like_a_command_is_not_a_subcommand(clean_env) -> None: + ns = build_parser().parse_args_with_config(["--driver", "sqlite", "-d", "migrate"]) + assert ns.command is None + assert ns.config.server.dbname == "migrate" + + +def test_once_argument_still_rejects_duplicates_after_the_subcommand(clean_env) -> None: + parser = build_parser() + with pytest.raises(SystemExit): + parser.parse_args_with_config(["dump", "--driver", "sqlite", "--driver", "mysql"]) + + +def test_dump_on_existing_drop_via_explicit_subcommand(clean_env) -> None: + ns = build_parser().parse_args_with_config(["dump", "--on-existing", "drop"]) + assert ns.config.dump.on_existing == "drop" + + +def test_root_help_hides_dump_options_but_lists_the_commands(clean_env) -> None: + help_text = build_parser().format_help() + assert "--driver" not in help_text + assert "--split-size" not in help_text + for command in ("dump", "init", "validate", "migrate"): + assert command in help_text + + +def test_dump_help_documents_the_options_without_leaking_suppress(clean_env, capsys) -> None: + """A SUPPRESS default must not surface as '(default: ==SUPPRESS==)'.""" + with pytest.raises(SystemExit): + build_parser().parse_args(["dump", "--help"]) + help_text = capsys.readouterr().out + assert "--driver" in help_text + assert "--split-size" in help_text + assert "SUPPRESS" not in help_text From 31164f973daf38029c7980f850ea8f574cb06b42 Mon Sep 17 00:00:00 2001 From: Jacques Raphanel Date: Mon, 17 Aug 2026 17:04:57 +0000 Subject: [PATCH 2/5] fix(security): stop logging source passwords and escape URL credentials Readers logged the full connection URL, password included, while writers already redacted theirs. Credentials were also interpolated raw, so a password containing @ / or : produced a broken URL. Centralise URL assembly in infrastructure/url.py: credentials are percent-encoded and every log line goes through redact_url. --- .../persistence/mssql/reader.py | 11 +-- .../persistence/mysql/reader.py | 11 +-- .../persistence/oracle/reader.py | 14 ++-- .../persistence/postgres/reader.py | 11 +-- .../persistence/sqlite/reader.py | 5 +- db2sql/infrastructure/url.py | 67 +++++++++++++++++ db2sql/infrastructure/writer/mssql/writer.py | 16 +---- .../infrastructure/writer/postgres/writer.py | 16 +---- .../persistence/test_reader_redaction.py | 71 +++++++++++++++++++ tests/unit/infrastructure/test_url.py | 68 ++++++++++++++++++ 10 files changed, 229 insertions(+), 61 deletions(-) create mode 100644 db2sql/infrastructure/url.py create mode 100644 tests/unit/infrastructure/persistence/test_reader_redaction.py create mode 100644 tests/unit/infrastructure/test_url.py diff --git a/db2sql/infrastructure/persistence/mssql/reader.py b/db2sql/infrastructure/persistence/mssql/reader.py index c12c5ff..eca95c0 100644 --- a/db2sql/infrastructure/persistence/mssql/reader.py +++ b/db2sql/infrastructure/persistence/mssql/reader.py @@ -12,6 +12,7 @@ from db2sql.infrastructure.config import AppConfig from db2sql.infrastructure.persistence import query_introspection from db2sql.infrastructure.persistence.errors import SourceReaderError +from db2sql.infrastructure.url import build_url, redact_url class MSSQLSourceReader: @@ -25,20 +26,14 @@ def __init__(self, config: AppConfig, logger: Logger) -> None: def _ensure_session(self) -> Session: if self._session is None: - self._logger.info(f"set connection to {self._connection_string}") + self._logger.info(f"set connection to {redact_url(self._connection_string)}") self._engine = create_engine(self._connection_string) self._session = sessionmaker(bind=self._engine)() return self._session @property def _connection_string(self) -> str: - server = self._config.server - port = f":{server.port}" if server.port else "" - username = server.username or "" - password = server.password or "" - hostname = server.hostname or "" - dbname = server.dbname or "" - return f"mssql+pymssql://{username}:{password}@{hostname}{port}/{dbname}" + return build_url(self._config.server, "mssql+pymssql") def collect_metadata(self) -> Database: database = Database(str(self._config.server.dbname or "")) diff --git a/db2sql/infrastructure/persistence/mysql/reader.py b/db2sql/infrastructure/persistence/mysql/reader.py index 04a85c6..7f6d09d 100644 --- a/db2sql/infrastructure/persistence/mysql/reader.py +++ b/db2sql/infrastructure/persistence/mysql/reader.py @@ -12,6 +12,7 @@ from db2sql.infrastructure.config import AppConfig from db2sql.infrastructure.persistence import query_introspection from db2sql.infrastructure.persistence.errors import SourceReaderError +from db2sql.infrastructure.url import build_url, redact_url class MySQLSourceReader: @@ -25,20 +26,14 @@ def __init__(self, config: AppConfig, logger: Logger) -> None: def _ensure_session(self) -> Session: if self._session is None: - self._logger.info(f"set connection to {self._connection_string}") + self._logger.info(f"set connection to {redact_url(self._connection_string)}") self._engine = create_engine(self._connection_string) self._session = sessionmaker(bind=self._engine)() return self._session @property def _connection_string(self) -> str: - server = self._config.server - port = f":{server.port}" if server.port else "" - username = server.username or "" - password = server.password or "" - hostname = server.hostname or "" - dbname = server.dbname or "" - return f"mysql+pymysql://{username}:{password}@{hostname}{port}/{dbname}" + return build_url(self._config.server, "mysql+pymysql") @property def _database_name(self) -> str: diff --git a/db2sql/infrastructure/persistence/oracle/reader.py b/db2sql/infrastructure/persistence/oracle/reader.py index aa063bd..cd29c58 100644 --- a/db2sql/infrastructure/persistence/oracle/reader.py +++ b/db2sql/infrastructure/persistence/oracle/reader.py @@ -12,6 +12,7 @@ from db2sql.infrastructure.config import AppConfig from db2sql.infrastructure.persistence import query_introspection from db2sql.infrastructure.persistence.errors import SourceReaderError +from db2sql.infrastructure.url import build_url, redact_url def _normalize_oracle_type(raw: str) -> str: @@ -91,7 +92,7 @@ def __init__(self, config: AppConfig, logger: Logger) -> None: def _ensure_session(self) -> Session: if self._session is None: - self._logger.info(f"set connection to {self._connection_string}") + self._logger.info(f"set connection to {redact_url(self._connection_string)}") self._engine = create_engine(self._connection_string) self._session = sessionmaker(bind=self._engine)() return self._session @@ -100,16 +101,11 @@ def _ensure_session(self) -> Session: def _connection_string(self) -> str: server = self._config.server options = server.options or {} - driver = options.get("driver", "oracledb") - port = f":{server.port}" if server.port else "" - userinfo = f"{server.username or ''}:{server.password or ''}" - host = server.hostname or "" + scheme = f"oracle+{options.get('driver', 'oracledb')}" service_name = options.get("service_name") - sid = options.get("sid") if service_name: - return f"oracle+{driver}://{userinfo}@{host}{port}/?service_name={service_name}" - target = sid or server.dbname or "" - return f"oracle+{driver}://{userinfo}@{host}{port}/{target}" + return build_url(server, scheme, database="", query={"service_name": service_name}) + return build_url(server, scheme, database=options.get("sid") or server.dbname or "") @property def _schema_filter(self) -> Optional[str]: diff --git a/db2sql/infrastructure/persistence/postgres/reader.py b/db2sql/infrastructure/persistence/postgres/reader.py index 0e63d31..8f393d7 100644 --- a/db2sql/infrastructure/persistence/postgres/reader.py +++ b/db2sql/infrastructure/persistence/postgres/reader.py @@ -12,6 +12,7 @@ from db2sql.infrastructure.config import AppConfig from db2sql.infrastructure.persistence import query_introspection from db2sql.infrastructure.persistence.errors import SourceReaderError +from db2sql.infrastructure.url import build_url, redact_url _SYSTEM_SCHEMAS = ("pg_catalog", "information_schema", "pg_toast") @@ -27,20 +28,14 @@ def __init__(self, config: AppConfig, logger: Logger) -> None: def _ensure_session(self) -> Session: if self._session is None: - self._logger.info(f"set connection to {self._connection_string}") + self._logger.info(f"set connection to {redact_url(self._connection_string)}") self._engine = create_engine(self._connection_string) self._session = sessionmaker(bind=self._engine)() return self._session @property def _connection_string(self) -> str: - server = self._config.server - port = f":{server.port}" if server.port else "" - username = server.username or "" - password = server.password or "" - hostname = server.hostname or "" - dbname = server.dbname or "" - return f"postgresql+psycopg2://{username}:{password}@{hostname}{port}/{dbname}" + return build_url(self._config.server, "postgresql+psycopg2") def collect_metadata(self) -> Database: database = Database(str(self._config.server.dbname or "")) diff --git a/db2sql/infrastructure/persistence/sqlite/reader.py b/db2sql/infrastructure/persistence/sqlite/reader.py index bca9dc1..f950e04 100644 --- a/db2sql/infrastructure/persistence/sqlite/reader.py +++ b/db2sql/infrastructure/persistence/sqlite/reader.py @@ -12,6 +12,7 @@ from db2sql.infrastructure.config import AppConfig from db2sql.infrastructure.persistence import query_introspection from db2sql.infrastructure.persistence.errors import SourceReaderError +from db2sql.infrastructure.url import build_url, redact_url _DEFAULT_SCHEMA = "public" @@ -31,11 +32,11 @@ def _connection_string(self) -> str: path = self._config.server.options.get("path") or self._config.server.dbname if not path: raise SourceReaderError("SQLite reader requires server.dbname or options.path") - return f"sqlite:///{path}" + return build_url(self._config.server, "sqlite", database=str(path), credentials=False) def _ensure_session(self) -> Session: if self._session is None: - self._logger.info(f"set connection to {self._connection_string}") + self._logger.info(f"set connection to {redact_url(self._connection_string)}") self._engine = create_engine(self._connection_string) self._session = sessionmaker(bind=self._engine)() return self._session diff --git a/db2sql/infrastructure/url.py b/db2sql/infrastructure/url.py new file mode 100644 index 0000000..f2c6466 --- /dev/null +++ b/db2sql/infrastructure/url.py @@ -0,0 +1,67 @@ +"""Build and redact the SQLAlchemy connection URLs used by readers and writers. + +Every reader and writer talks to its database through a SQLAlchemy URL +assembled from a :class:`~db2sql.infrastructure.config.ServerConfig`. Keeping +that assembly in one place guarantees three things that used to be per-driver +details: credentials are percent-encoded, the shape of the URL is consistent, +and nothing ever logs a password. +""" + +from __future__ import annotations + +import re +from typing import Mapping, Optional +from urllib.parse import quote, urlencode + +from db2sql.infrastructure.config import ServerConfig + +# Matches the ``user:password@`` prefix of a URL authority. The password group +# requires at least one character so that a credential-less URL — or one with +# an empty password — is left untouched instead of gaining a fake secret. +_USERINFO_RE = re.compile(r"(?<=://)(?P[^/?#@]*):(?P[^/?#@]+)@") + + +def _encode(value: Optional[str]) -> str: + """Percent-encode a URL credential. + + ``quote`` rather than ``quote_plus``: SQLAlchemy unquotes the userinfo with + :func:`urllib.parse.unquote`, which does not turn ``+`` back into a space. + """ + return quote(value or "", safe="") + + +def build_url( + server: ServerConfig, + scheme: str, + *, + database: Optional[str] = None, + query: Optional[Mapping[str, str]] = None, + credentials: bool = True, +) -> str: + """Assemble ``scheme://user:password@host:port/database?query``. + + :param server: connection parameters. + :param scheme: SQLAlchemy dialect+driver, e.g. ``postgresql+psycopg2``. + :param database: URL path; defaults to ``server.dbname``. Pass ``""`` for + dialects that carry the database in the query string instead. + :param query: extra query-string parameters. + :param credentials: set to ``False`` for file-based dialects (SQLite), + which take neither user info nor host. + """ + authority = "" + if credentials: + authority = f"{_encode(server.username)}:{_encode(server.password)}@" + authority += server.hostname or "" + if server.port: + authority += f":{server.port}" + + path = database if database is not None else (server.dbname or "") + url = f"{scheme}://{authority}/{path}" + if query: + url += f"?{urlencode(query)}" + return url + + +def redact_url(url: str) -> str: + """Replace the password of ``url`` with ``***`` so it is safe to log.""" + return _USERINFO_RE.sub(lambda match: f"{match.group('user')}:***@", url, count=1) diff --git a/db2sql/infrastructure/writer/mssql/writer.py b/db2sql/infrastructure/writer/mssql/writer.py index 655a48d..ed56db8 100644 --- a/db2sql/infrastructure/writer/mssql/writer.py +++ b/db2sql/infrastructure/writer/mssql/writer.py @@ -19,6 +19,7 @@ from db2sql.application.ports import Logger from db2sql.domain.model import Table from db2sql.infrastructure.config import AppConfig +from db2sql.infrastructure.url import build_url, redact_url from db2sql.infrastructure.writer.errors import ( TargetWriterConnectionError, TargetWriterExecutionError, @@ -128,22 +129,11 @@ def _raw_pymssql_cursor(self) -> Any: @property def _connection_string(self) -> str: - server = self._config.target_server - port = f":{server.port}" if server.port else "" - username = server.username or "" - password = server.password or "" - hostname = server.hostname or "" - dbname = server.dbname or "" - return f"mssql+pymssql://{username}:{password}@{hostname}{port}/{dbname}" + return build_url(self._config.target_server, "mssql+pymssql") @property def _connection_string_redacted(self) -> str: - server = self._config.target_server - port = f":{server.port}" if server.port else "" - username = server.username or "" - hostname = server.hostname or "" - dbname = server.dbname or "" - return f"mssql+pymssql://{username}@{hostname}{port}/{dbname}" + return redact_url(self._connection_string) @staticmethod def _quote_ident(name: str) -> str: diff --git a/db2sql/infrastructure/writer/postgres/writer.py b/db2sql/infrastructure/writer/postgres/writer.py index 257a7c9..2d14f2a 100644 --- a/db2sql/infrastructure/writer/postgres/writer.py +++ b/db2sql/infrastructure/writer/postgres/writer.py @@ -20,6 +20,7 @@ from db2sql.application.ports import Logger from db2sql.domain.model import Table from db2sql.infrastructure.config import AppConfig +from db2sql.infrastructure.url import build_url, redact_url from db2sql.infrastructure.writer.errors import ( TargetWriterConnectionError, TargetWriterExecutionError, @@ -126,22 +127,11 @@ def _raw_psycopg2_cursor(self) -> Any: @property def _connection_string(self) -> str: - server = self._config.target_server - port = f":{server.port}" if server.port else "" - username = server.username or "" - password = server.password or "" - hostname = server.hostname or "" - dbname = server.dbname or "" - return f"postgresql+psycopg2://{username}:{password}@{hostname}{port}/{dbname}" + return build_url(self._config.target_server, "postgresql+psycopg2") @property def _connection_string_redacted(self) -> str: - server = self._config.target_server - port = f":{server.port}" if server.port else "" - username = server.username or "" - hostname = server.hostname or "" - dbname = server.dbname or "" - return f"postgresql+psycopg2://{username}@{hostname}{port}/{dbname}" + return redact_url(self._connection_string) @staticmethod def _quote_ident(name: str) -> str: diff --git a/tests/unit/infrastructure/persistence/test_reader_redaction.py b/tests/unit/infrastructure/persistence/test_reader_redaction.py new file mode 100644 index 0000000..7af34ba --- /dev/null +++ b/tests/unit/infrastructure/persistence/test_reader_redaction.py @@ -0,0 +1,71 @@ +"""No reader may write a source password to the log when opening its session.""" + +from __future__ import annotations + +from typing import Any, List +from unittest.mock import patch + +import pytest + +from db2sql.infrastructure.config import AppConfig, ServerConfig +from db2sql.infrastructure.persistence.mssql.reader import MSSQLSourceReader +from db2sql.infrastructure.persistence.mysql.reader import MySQLSourceReader +from db2sql.infrastructure.persistence.oracle.reader import OracleSourceReader +from db2sql.infrastructure.persistence.postgres.reader import PostgresSourceReader +from db2sql.infrastructure.persistence.sqlite.reader import SQLiteSourceReader + +_PASSWORD = "sup3r-s3cr3t" + + +class _RecordingLogger: + def __init__(self) -> None: + self.messages: List[str] = [] + + def trace(self, message: str) -> None: ... + def debug(self, message: str) -> None: ... + def warning(self, message: str) -> None: ... + def error(self, message: str) -> None: ... + + def info(self, message: str) -> None: + self.messages.append(message) + + +_READERS = [ + (PostgresSourceReader, "postgres", "db2sql.infrastructure.persistence.postgres.reader"), + (MySQLSourceReader, "mysql", "db2sql.infrastructure.persistence.mysql.reader"), + (MSSQLSourceReader, "mssql", "db2sql.infrastructure.persistence.mssql.reader"), + (OracleSourceReader, "oracle", "db2sql.infrastructure.persistence.oracle.reader"), +] + + +@pytest.mark.parametrize(("reader_class", "driver", "module"), _READERS) +def test_reader_never_logs_the_password(reader_class: Any, driver: str, module: str) -> None: + config = AppConfig( + driver=driver, + server=ServerConfig( + hostname="db.local", port=1234, username="u", password=_PASSWORD, dbname="d" + ), + ) + logger = _RecordingLogger() + reader = reader_class(config, logger) + + with patch(f"{module}.create_engine"), patch(f"{module}.sessionmaker"): + reader._ensure_session() + + logged = "\n".join(logger.messages) + assert logged, "the reader is expected to log the connection it opens" + assert _PASSWORD not in logged + assert ":***@" in logged + + +def test_sqlite_reader_logs_its_path() -> None: + """SQLite has no credentials — the URL must still be logged, unmangled.""" + config = AppConfig(driver="sqlite", server=ServerConfig(dbname="/tmp/x.db")) + logger = _RecordingLogger() + reader = SQLiteSourceReader(config, logger) + + module = "db2sql.infrastructure.persistence.sqlite.reader" + with patch(f"{module}.create_engine"), patch(f"{module}.sessionmaker"): + reader._ensure_session() + + assert "sqlite:////tmp/x.db" in "\n".join(logger.messages) diff --git a/tests/unit/infrastructure/test_url.py b/tests/unit/infrastructure/test_url.py new file mode 100644 index 0000000..b52723f --- /dev/null +++ b/tests/unit/infrastructure/test_url.py @@ -0,0 +1,68 @@ +"""Shared SQLAlchemy URL builder: escaping, shape, and password redaction.""" + +from __future__ import annotations + +import pytest +from sqlalchemy.engine import make_url + +from db2sql.infrastructure.config import ServerConfig +from db2sql.infrastructure.url import build_url, redact_url + + +def test_build_url_full_shape() -> None: + server = ServerConfig(hostname="h", port=5432, username="u", password="p", dbname="d") + assert build_url(server, "postgresql+psycopg2") == "postgresql+psycopg2://u:p@h:5432/d" + + +def test_build_url_omits_port_when_missing() -> None: + server = ServerConfig(hostname="h", username="u", password="p", dbname="d") + assert build_url(server, "mysql+pymysql") == "mysql+pymysql://u:p@h/d" + + +def test_build_url_without_credentials_yields_file_url() -> None: + server = ServerConfig(dbname="/tmp/x.db") + url = build_url(server, "sqlite", database="/tmp/x.db", credentials=False) + assert url == "sqlite:////tmp/x.db" + + +def test_build_url_appends_query_parameters() -> None: + server = ServerConfig(hostname="db.local", port=1521, username="hr", password="pw") + url = build_url(server, "oracle+oracledb", database="", query={"service_name": "ORCLPDB1"}) + assert url == "oracle+oracledb://hr:pw@db.local:1521/?service_name=ORCLPDB1" + + +@pytest.mark.parametrize("password", ["p@ss", "a/b", "a:b", "a b", "%40", "p#1?2"]) +def test_credentials_survive_a_round_trip_through_sqlalchemy(password: str) -> None: + """Special characters must not corrupt the URL — this used to break.""" + server = ServerConfig(hostname="h", port=5432, username="u@ser", password=password, dbname="d") + parsed = make_url(build_url(server, "postgresql+psycopg2")) + assert parsed.username == "u@ser" + assert parsed.password == password + assert parsed.host == "h" + assert parsed.port == 5432 + assert parsed.database == "d" + + +def test_redact_url_masks_the_password() -> None: + url = "postgresql+psycopg2://u:s3cr3t@h:5432/d" + assert redact_url(url) == "postgresql+psycopg2://u:***@h:5432/d" + + +def test_redact_url_masks_an_encoded_password() -> None: + url = "postgresql+psycopg2://u:p%40ss%2Fw@h/d" + assert redact_url(url) == "postgresql+psycopg2://u:***@h/d" + assert "p%40ss" not in redact_url(url) + + +def test_redact_url_leaves_a_credential_less_url_untouched() -> None: + assert redact_url("sqlite:////tmp/x.db") == "sqlite:////tmp/x.db" + + +def test_redact_url_does_not_invent_a_password_when_there_is_none() -> None: + """An empty password must stay empty rather than read as a masked secret.""" + assert redact_url("mysql+pymysql://u:@h/d") == "mysql+pymysql://u:@h/d" + + +def test_redact_url_only_touches_the_authority() -> None: + url = "postgresql+psycopg2://u:pw@h/d?options=-c%20search_path%3Da:b@c" + assert redact_url(url) == "postgresql+psycopg2://u:***@h/d?options=-c%20search_path%3Da:b@c" From bb95c229a6e4ba4c8916728f8e63262e0ff4b5e5 Mon Sep 17 00:00:00 2001 From: Jacques Raphanel Date: Mon, 17 Aug 2026 17:31:09 +0000 Subject: [PATCH 3/5] feat(cli): add --source-dsn and --target-dsn Expose the SQLAlchemy URL that readers and writers already build internally. This is the only way to reach driver-specific parameters (sslmode, charset, TrustServerCertificate), to pick another DBAPI for the same dialect, or to describe an Oracle service_name without a config file. A DSN replaces the connection instead of merging with it; the discrete fields it shadows are reported. Its dialect is checked against the selected driver, since SQLAlchemy resolves the dialect from the URL and would otherwise connect before failing inside introspection. --- README.rst | 28 ++++ db2sql/const.py | 8 ++ db2sql/infrastructure/config/loader.py | 26 +++- db2sql/infrastructure/config/schema.py | 19 ++- .../persistence/mysql/reader.py | 12 +- .../persistence/sqlite/reader.py | 9 +- db2sql/infrastructure/url.py | 49 ++++++- db2sql/interface/cli/parser.py | 111 +++++++++++++++- db2sql/interface/cli/runner.py | 20 +++ docs/cli.rst | 85 +++++++++++- docs/configuration.rst | 16 +++ tests/cli/test_cli_sqlite.py | 51 ++++++- .../unit/infrastructure/config/test_loader.py | 42 ++++++ tests/unit/infrastructure/test_url.py | 62 ++++++++- tests/unit/interface/cli/test_parser.py | 125 +++++++++++++++++- tests/unit/interface/cli/test_runner.py | 60 ++++++++- 16 files changed, 695 insertions(+), 28 deletions(-) diff --git a/README.rst b/README.rst index 3b0448a..d85d0e4 100644 --- a/README.rst +++ b/README.rst @@ -84,6 +84,34 @@ rows. Use it to refresh data into a pre-existing schema: $ db2sql dump --driver sqlite --dbname mydb.sqlite --on-existing truncate -f refresh.sql +Connecting with a DSN +--------------------- + +The discrete ``-H`` / ``-P`` / ``-d`` / ``-u`` / ``-p`` flags cover the common +case. When you need something they cannot express — a TLS mode, a charset, an +Oracle ``service_name``, an alternative DBAPI — pass a full SQLAlchemy URL +instead: + +.. code-block:: console + + # prefer the environment: a DSN on the command line is visible in `ps` + $ export DB2SQL_SOURCE_DSN='postgresql+psycopg2://app:s3cr3t@pg.example.com:5432/mydb?sslmode=require' + $ db2sql dump --driver postgres -f dump.sql + + # and its mirror for a live migration + $ export DB2SQL_TARGET_DSN='postgresql+psycopg2://svc@target.internal:5432/stage' + $ db2sql migrate --driver mysql -H mysql.example.com -d mydb -u app -W + +A DSN **replaces** the connection rather than merging with it, and the URL +dialect must match ``--driver`` / ``--target``. Passing a DSN together with +``-H`` / ``-d`` / … on the same command line — or declaring both in the same +config file — is rejected as a contradiction; a DSN overriding a connection +that came from a config file or the environment is allowed, and warns about +what it dropped. Passwords are always redacted in log output. See the +`CLI reference `__ +for the full semantics. + + Validating a configuration -------------------------- diff --git a/db2sql/const.py b/db2sql/const.py index fb33de0..bd30942 100644 --- a/db2sql/const.py +++ b/db2sql/const.py @@ -8,11 +8,13 @@ "ENV_DB2SQL_HOST", "ENV_DB2SQL_PORT", "ENV_DB2SQL_DBNAME", + "ENV_DB2SQL_SOURCE_DSN", "ENV_DB2SQL_TARGET_HOST", "ENV_DB2SQL_TARGET_PORT", "ENV_DB2SQL_TARGET_USER", "ENV_DB2SQL_TARGET_PASSWORD", "ENV_DB2SQL_TARGET_DBNAME", + "ENV_DB2SQL_TARGET_DSN", "ENV_NO_COLOR", "ENV_CLICOLOR_FORCE", "ENV_DB2SQL_COLOR_DARK", @@ -40,6 +42,9 @@ ENV_DB2SQL_DBNAME = "DB2SQL_DBNAME" """Source database name.""" +ENV_DB2SQL_SOURCE_DSN = "DB2SQL_SOURCE_DSN" +"""Full SQLAlchemy URL for the source database; overrides the discrete fields.""" + ENV_DB2SQL_TARGET_HOST = "DB2SQL_TARGET_HOST" """Target database host (live migration).""" @@ -55,6 +60,9 @@ ENV_DB2SQL_TARGET_DBNAME = "DB2SQL_TARGET_DBNAME" """Target database name (live migration).""" +ENV_DB2SQL_TARGET_DSN = "DB2SQL_TARGET_DSN" +"""Full SQLAlchemy URL for the target database (live migration).""" + ENV_NO_COLOR = "NO_COLOR" """Disable ANSI colors.""" diff --git a/db2sql/infrastructure/config/loader.py b/db2sql/infrastructure/config/loader.py index 86b8579..d6077f5 100644 --- a/db2sql/infrastructure/config/loader.py +++ b/db2sql/infrastructure/config/loader.py @@ -27,13 +27,14 @@ str(Path.home() / "db2sql.yml"), ] -_SERVER_FIELDS = {"hostname", "port", "username", "password", "dbname"} +_SERVER_FIELDS = {"hostname", "port", "username", "password", "dbname", "dsn"} _TARGET_SERVER_FIELDS = { "target_hostname", "target_port", "target_username", "target_password", "target_dbname", + "target_dsn", } _MIGRATE_FIELDS = {"on_existing", "transaction_mode", "batch_size", "use_transaction"} _DUMP_FIELDS = { @@ -105,9 +106,30 @@ def load_config(config_file: Optional[PathLike] = None) -> AppConfig: if not data: return AppConfig() try: - return AppConfig.model_validate(data) + config = AppConfig.model_validate(data) except ValidationError as exc: raise ConfigInvalidError(f"Invalid configuration file {resolved}: {exc}") from exc + _reject_dsn_conflicts(config, resolved) + return config + + +def _reject_dsn_conflicts(config: AppConfig, source: str) -> None: + """Refuse a ``dsn`` sitting next to discrete connection keys in the same file. + + A DSN replaces the connection rather than merging with it, so declaring + both in one file states two contradictory intents. A DSN passed on the + command line while the file describes a host is a different matter — that + is the documented precedence, and the runner only warns about it. + """ + for section in ("server", "target_server"): + server = getattr(config, section) + shadowed = server.fields_shadowed_by_dsn() + if shadowed: + keys = ", ".join(f"{section}.{name}" for name in shadowed) + raise ConfigInvalidError( + f"Invalid configuration file {source}: {section}.dsn cannot be combined " + f"with {keys} — a DSN replaces the connection, it does not merge with it." + ) def merge_cli_overrides(config: AppConfig, options: Mapping[str, Any]) -> AppConfig: diff --git a/db2sql/infrastructure/config/schema.py b/db2sql/infrastructure/config/schema.py index 40260a8..54d9e19 100644 --- a/db2sql/infrastructure/config/schema.py +++ b/db2sql/infrastructure/config/schema.py @@ -2,7 +2,7 @@ from __future__ import annotations -from typing import Any, Dict, List, Optional +from typing import Any, Dict, List, Optional, Tuple from pydantic import BaseModel, ConfigDict, Field, field_validator @@ -19,8 +19,25 @@ class ServerConfig(BaseModel): username: Optional[str] = None password: Optional[str] = None dbname: Optional[str] = None + dsn: Optional[str] = None options: Dict[str, str] = Field(default_factory=dict) + def fields_shadowed_by_dsn(self) -> Tuple[str, ...]: + """Discrete connection fields that ``dsn`` makes irrelevant. + + A DSN replaces the whole connection rather than merging with it, so + anything set alongside it is silently unused. Callers report this back + to the user instead of letting the mismatch pass unnoticed. + """ + if not self.dsn: + return () + return tuple( + name for name in _DISCRETE_CONNECTION_FIELDS if getattr(self, name) is not None + ) + + +_DISCRETE_CONNECTION_FIELDS = ("hostname", "port", "username", "password", "dbname") + class TableOverride(BaseModel): """Per-table overrides applied on top of the global dump options.""" diff --git a/db2sql/infrastructure/persistence/mysql/reader.py b/db2sql/infrastructure/persistence/mysql/reader.py index 7f6d09d..c621f51 100644 --- a/db2sql/infrastructure/persistence/mysql/reader.py +++ b/db2sql/infrastructure/persistence/mysql/reader.py @@ -12,7 +12,7 @@ from db2sql.infrastructure.config import AppConfig from db2sql.infrastructure.persistence import query_introspection from db2sql.infrastructure.persistence.errors import SourceReaderError -from db2sql.infrastructure.url import build_url, redact_url +from db2sql.infrastructure.url import build_url, database_from_url, redact_url class MySQLSourceReader: @@ -37,9 +37,13 @@ def _connection_string(self) -> str: @property def _database_name(self) -> str: - if not self._config.server.dbname: - raise SourceReaderError("MySQL reader requires server.dbname") - return str(self._config.server.dbname) + # MySQL has no schema layer: the database name doubles as the schema + # every table is filed under, so it must be known even with a DSN. + server = self._config.server + name = database_from_url(server.dsn) if server.dsn else server.dbname + if not name: + raise SourceReaderError("MySQL reader requires server.dbname or a DSN naming it") + return str(name) def collect_metadata(self) -> Database: database = Database(self._database_name) diff --git a/db2sql/infrastructure/persistence/sqlite/reader.py b/db2sql/infrastructure/persistence/sqlite/reader.py index f950e04..01d7840 100644 --- a/db2sql/infrastructure/persistence/sqlite/reader.py +++ b/db2sql/infrastructure/persistence/sqlite/reader.py @@ -29,10 +29,13 @@ def __init__(self, config: AppConfig, logger: Logger) -> None: @property def _connection_string(self) -> str: - path = self._config.server.options.get("path") or self._config.server.dbname + server = self._config.server + if server.dsn: + return build_url(server, "sqlite", credentials=False) + path = server.options.get("path") or server.dbname if not path: - raise SourceReaderError("SQLite reader requires server.dbname or options.path") - return build_url(self._config.server, "sqlite", database=str(path), credentials=False) + raise SourceReaderError("SQLite reader requires server.dbname, options.path, or a DSN") + return build_url(server, "sqlite", database=str(path), credentials=False) def _ensure_session(self) -> Session: if self._session is None: diff --git a/db2sql/infrastructure/url.py b/db2sql/infrastructure/url.py index f2c6466..7dec1ff 100644 --- a/db2sql/infrastructure/url.py +++ b/db2sql/infrastructure/url.py @@ -11,9 +11,9 @@ import re from typing import Mapping, Optional -from urllib.parse import quote, urlencode +from urllib.parse import quote, unquote, urlencode -from db2sql.infrastructure.config import ServerConfig +from db2sql.infrastructure.config import ConfigInvalidError, ServerConfig # Matches the ``user:password@`` prefix of a URL authority. The password group # requires at least one character so that a credential-less URL — or one with @@ -21,6 +21,31 @@ _USERINFO_RE = re.compile(r"(?<=://)(?P[^/?#@]*):(?P[^/?#@]+)@") +def _dialect_of(scheme: str) -> str: + """``postgresql+psycopg2`` -> ``postgresql``; the DBAPI part is free choice.""" + return scheme.split("+", 1)[0].lower() + + +def _check_dialect(dsn: str, expected_scheme: str) -> None: + """Reject a DSN whose dialect contradicts the reader or writer using it. + + SQLAlchemy picks its dialect from the URL, not from ``--driver``, so a + Postgres DSN handed to the MSSQL reader would connect successfully and + then fail deep inside introspection with unrelated SQL errors. + """ + dsn_scheme = dsn.split("://", 1)[0] + if not dsn_scheme or "://" not in dsn: + raise ConfigInvalidError(f"invalid DSN, expected a '://' URL: {redact_url(dsn)}") + + expected = _dialect_of(expected_scheme) + if _dialect_of(dsn_scheme) != expected: + raise ConfigInvalidError( + f"DSN dialect '{_dialect_of(dsn_scheme)}' does not match the selected " + f"driver, which expects '{expected}'. Adjust the DSN or select the " + f"matching driver." + ) + + def _encode(value: Optional[str]) -> str: """Percent-encode a URL credential. @@ -40,14 +65,22 @@ def build_url( ) -> str: """Assemble ``scheme://user:password@host:port/database?query``. - :param server: connection parameters. + :param server: connection parameters. When ``server.dsn`` is set it + replaces the whole URL — every other field and argument is ignored — + after its dialect has been checked against ``scheme``. :param scheme: SQLAlchemy dialect+driver, e.g. ``postgresql+psycopg2``. :param database: URL path; defaults to ``server.dbname``. Pass ``""`` for dialects that carry the database in the query string instead. :param query: extra query-string parameters. :param credentials: set to ``False`` for file-based dialects (SQLite), which take neither user info nor host. + :raises ConfigInvalidError: if the DSN targets a different dialect than + the caller. """ + if server.dsn: + _check_dialect(server.dsn, scheme) + return server.dsn + authority = "" if credentials: authority = f"{_encode(server.username)}:{_encode(server.password)}@" @@ -62,6 +95,16 @@ def build_url( return url +def database_from_url(url: str) -> Optional[str]: + """Extract the database name from a URL, for dialects that need it as a label.""" + without_scheme = url.split("://", 1)[-1] + _, separator, path = without_scheme.partition("/") + if not separator: + return None + database = path.split("?", 1)[0].split("#", 1)[0] + return unquote(database) or None + + def redact_url(url: str) -> str: """Replace the password of ``url`` with ``***`` so it is safe to log.""" return _USERINFO_RE.sub(lambda match: f"{match.group('user')}:***@", url, count=1) diff --git a/db2sql/interface/cli/parser.py b/db2sql/interface/cli/parser.py index 48aceb6..0d3af3f 100644 --- a/db2sql/interface/cli/parser.py +++ b/db2sql/interface/cli/parser.py @@ -5,13 +5,14 @@ import argparse import os import sys -from typing import Any, Optional, Sequence +from typing import Any, List, Optional, Sequence from db2sql import const from db2sql.application.dto import DataFormat from db2sql.infrastructure.config import ( AppConfig, ConfigError, + ConfigInvalidError, load_config, merge_cli_overrides, ) @@ -47,10 +48,84 @@ def __init__(self, message: str) -> None: self.message = message +_SOURCE_CONNECTION_FLAGS = ( + "-H", + "--host", + "-P", + "--port", + "-d", + "--dbname", + "-u", + "--username", + "-p", + "--password", + "-W", + "--ask-password", +) + +_TARGET_CONNECTION_FLAGS = ( + "--target-host", + "--target-port", + "--target-dbname", + "--target-user", + "--target-password", +) + + +def _flags_used(argv: Sequence[str], flags: Sequence[str]) -> List[str]: + """Return which of ``flags`` appear in ``argv``, in declaration order. + + Recognises the three spellings argparse accepts: the bare flag, the + ``--long=value`` form, and a short flag with its value attached + (``-Hhost``). An exotic spelling that slips through simply falls back to + the runtime warning instead of raising — better to miss a conflict than + to reject a valid command line. + """ + used = [] + for flag in flags: + is_short = len(flag) == 2 and not flag.startswith("--") + for token in argv: + attached = is_short and token.startswith(flag) and len(token) > len(flag) + if token == flag or token.startswith(f"{flag}=") or attached: + used.append(flag) + break + return used + + class MsDumpToPGArgumentParser(argparse.ArgumentParser): """Builds an :class:`AppConfig` from CLI args + config file + env.""" + def _reject_dsn_conflicts(self, argv: Sequence[str]) -> None: + """Refuse a DSN and discrete connection flags on the same command line. + + A DSN replaces the connection rather than merging with it, so passing + both expresses two contradictory intents. Only same-command-line + conflicts are rejected here: a DSN overriding a host that came from a + config file or the environment is the documented precedence at work, + and the runner merely warns about it. + + Raises :class:`ConfigInvalidError` rather than calling + :meth:`argparse.ArgumentParser.error` so that a contradictory + connection exits with the same code as its config-file equivalent. + """ + if "-h" in argv or "--help" in argv: + return + for dsn_flag, connection_flags in ( + ("--source-dsn", _SOURCE_CONNECTION_FLAGS), + ("--target-dsn", _TARGET_CONNECTION_FLAGS), + ): + if not _flags_used(argv, [dsn_flag]): + continue + conflicting = _flags_used(argv, connection_flags) + if conflicting: + raise ConfigInvalidError( + f"{dsn_flag} cannot be combined with {', '.join(conflicting)}: a DSN " + f"replaces the connection, it does not merge with it. Pass either the " + f"DSN or the individual flags." + ) + def parse_args_with_config(self, args: Optional[Sequence[str]] = None) -> argparse.Namespace: + self._reject_dsn_conflicts(list(args) if args is not None else sys.argv[1:]) options = super().parse_args(args) if "help" in options and options.help: @@ -290,6 +365,24 @@ def _add_source_options( default=_value(False, defaults=defaults), help=_text("Force password prompt.", visible=visible), ) + parser.add_argument( + "--source-dsn", + dest="dsn", + metavar="URL", + type=str, + default=_value(os.getenv(const.ENV_DB2SQL_SOURCE_DSN), defaults=defaults), + help=_text( + "Full SQLAlchemy URL for the source, e.g. " + "'postgresql+psycopg2://user:pwd@host:5432/db?sslmode=require'. " + "Replaces --host/--port/--dbname/--username/--password entirely " + "and is the only way to pass driver-specific parameters. The URL " + "dialect must match --driver. Prefer the environment variable: a " + "DSN on the command line is visible in 'ps'. " + f"[env var: {const.ENV_DB2SQL_SOURCE_DSN}]", + visible=visible, + ), + action=OnceArgument, + ) def _add_selection_options( @@ -603,6 +696,22 @@ def _add_migrate_subparser(subparsers: Any) -> None: help=f"Target database password. [env var: {const.ENV_DB2SQL_TARGET_PASSWORD}]", action=OnceArgument, ) + migrate_parser.add_argument( + "--target-dsn", + dest="target_dsn", + metavar="URL", + type=str, + default=os.getenv(const.ENV_DB2SQL_TARGET_DSN), + help=( + "Full SQLAlchemy URL for the target. Replaces every other " + "--target-host/--target-port/--target-dbname/--target-user/" + "--target-password flag and must match the --target dialect. " + "Prefer the environment variable: a DSN on the command line is " + "visible in 'ps'. " + f"[env var: {const.ENV_DB2SQL_TARGET_DSN}]" + ), + action=OnceArgument, + ) migrate_parser.add_argument( "--on-existing", dest="on_existing", diff --git a/db2sql/interface/cli/runner.py b/db2sql/interface/cli/runner.py index 32865a6..7e35037 100644 --- a/db2sql/interface/cli/runner.py +++ b/db2sql/interface/cli/runner.py @@ -87,6 +87,8 @@ def run(self, *args: Any) -> ExitCode: } ) + self._warn_about_shadowed_fields(options.config) + if getattr(options, "command", None) == COMMAND_MIGRATE: target_driver = getattr(options, "target_driver", None) self._execute_migrate(options.config, target_driver) @@ -101,6 +103,24 @@ def run(self, *args: Any) -> ExitCode: raise exc return SUCCESS + def _warn_about_shadowed_fields(self, config: AppConfig) -> None: + """Tell the user which connection settings a DSN is making irrelevant. + + A DSN replaces the connection wholesale rather than merging with the + discrete flags, so leftovers from a config file or an environment + variable would otherwise be dropped without a word. + """ + for label, server in ( + ("--source-dsn", config.server), + ("--target-dsn", config.target_server), + ): + shadowed = server.fields_shadowed_by_dsn() + if shadowed: + self._logger.warning( + f"{label} is set: ignoring {', '.join(shadowed)} " + f"(a DSN replaces the connection, it does not merge with it)" + ) + def _execute(self, config: AppConfig) -> None: reader = get_source_reader(config.driver, config, self._logger) emitter = get_sql_emitter( diff --git a/docs/cli.rst b/docs/cli.rst index 8cc4742..63b1c59 100644 --- a/docs/cli.rst +++ b/docs/cli.rst @@ -8,14 +8,14 @@ Synopsis db2sql [GLOBAL OPTIONS] COMMAND [OPTIONS] - db2sql dump [SOURCE OPTIONS] [FILTERING OPTIONS] + db2sql dump [SOURCE OPTIONS | --source-dsn URL] [FILTERING OPTIONS] [-f PATH] [--split-size SIZE] [--on-existing {fail,drop,truncate}] [--transaction | --no-transaction] - db2sql migrate [SOURCE OPTIONS] [FILTERING OPTIONS] + db2sql migrate [SOURCE OPTIONS | --source-dsn URL] [FILTERING OPTIONS] [--target-host HOST] [--target-port PORT] [--target-dbname DB] [--target-user USER] [--target-password PWD] - [--target-driver NAME] + [--target-driver NAME] [--target-dsn URL] [--on-existing {fail,drop,truncate}] [--transaction-mode {single,per_table}] [--transaction | --no-transaction] @@ -152,6 +152,48 @@ Connection Prompt for the database password interactively instead of reading it from :option:`-p` or the environment variable. +.. option:: --source-dsn URL + + Full SQLAlchemy URL for the source database, e.g. + ``postgresql+psycopg2://user:pwd@host:5432/db?sslmode=require``. + + This is the escape hatch for anything the discrete flags cannot express: + driver-specific query parameters (``sslmode``, ``charset``, + ``TrustServerCertificate``, ``ApplicationIntent``…), an alternative DBAPI + for the same dialect (``postgresql+asyncpg://``), or the Oracle + ``service_name`` / ``sid`` selection that otherwise requires a config file. + + **It replaces the connection, it does not merge with it.** Because the two + cannot be reconciled, ``db2sql`` distinguishes a contradiction from an + override: + + * **Rejected** — a DSN and any of :option:`-H`, :option:`-P`, + :option:`-d`, :option:`-u`, :option:`-p`, :option:`-W` on the *same + command line*, or ``dsn`` next to those keys in the *same config file*. + Both spellings state two different connections at once, which is a + mistake rather than an intent. The command fails before connecting. + * **Allowed, with a warning** — a DSN on the command line overriding a + ``server:`` section from a config file, or ``DB2SQL_HOST`` and friends + from the environment. This is the ordinary precedence order at work; the + warning lists exactly which settings were dropped. + + The dialect of the URL must match the dialect the selected + :option:`--driver` expects; the DBAPI part after ``+`` is free. A + mismatch is rejected up front, because SQLAlchemy derives its dialect + from the URL rather than from ``--driver``, and would otherwise connect + successfully before failing deep inside introspection. + + Equivalent config key: ``server.dsn``. + + *Environment variable:* ``DB2SQL_SOURCE_DSN`` + + .. warning:: + + A DSN carries the password. Passed on the command line it is visible + in ``ps`` and lands in the shell history — prefer the environment + variable or the config file. Log output is always redacted + (``user:***@host``). + Output ~~~~~~ @@ -433,6 +475,8 @@ take precedence when both are present. - Database user name * - ``DB2SQL_PASSWORD`` - Database password + * - ``DB2SQL_SOURCE_DSN`` + - Full SQLAlchemy URL for the source; replaces the five variables above * - ``DB2SQL_TARGET_HOST`` - Target database hostname (used by ``db2sql migrate``) * - ``DB2SQL_TARGET_PORT`` @@ -443,6 +487,8 @@ take precedence when both are present. - Target database user (used by ``db2sql migrate``) * - ``DB2SQL_TARGET_PASSWORD`` - Target database password (used by ``db2sql migrate``) + * - ``DB2SQL_TARGET_DSN`` + - Full SQLAlchemy URL for the target; replaces the five variables above * - ``DB2SQL_CONFIG`` - Path to a config file * - ``NO_COLOR`` @@ -468,7 +514,7 @@ to ``stdout``. This is the default command — see db2sql dump [--driver NAME] [--target NAME] [-H HOSTNAME] [-P PORT] [-d DBNAME] - [-u USERNAME] [-p PASSWORD] [-W] + [-u USERNAME] [-p PASSWORD] [-W] [--source-dsn URL] [-i NAME …] [-x NAME …] [-I NAME …] [-X NAME …] [--preserve-case | --no-preserve-case] [--data-format {copy,insert}] [-n N] @@ -605,13 +651,14 @@ batched ``executemany`` for MSSQL). db2sql migrate [--driver NAME] [--target NAME] [-H HOSTNAME] [-P PORT] [-d DBNAME] - [-u USERNAME] [-p PASSWORD] [-W] + [-u USERNAME] [-p PASSWORD] [-W] [--source-dsn URL] [-i NAME …] [-x NAME …] [-I NAME …] [-X NAME …] [--preserve-case | --no-preserve-case] [--data-format {copy,insert}] [-n N] [--target-host HOSTNAME] [--target-port PORT] [--target-dbname DBNAME] [--target-user USERNAME] [--target-password PASSWORD] [--target-driver NAME] + [--target-dsn URL] [--on-existing {fail,drop,truncate}] [--transaction-mode {single,per_table}] [--transaction | --no-transaction] @@ -667,6 +714,18 @@ Target connection *Environment variable:* ``DB2SQL_TARGET_PASSWORD`` +.. option:: --target-dsn URL + + Full SQLAlchemy URL for the target database — the mirror of + :option:`--source-dsn`, with the same semantics: it replaces every other + ``--target-*`` connection flag rather than merging with them, its dialect + must match :option:`--target`, and it should be supplied through the + environment rather than the command line. + + Equivalent config key: ``target_server.dsn``. + + *Environment variable:* ``DB2SQL_TARGET_DSN`` + Migration behaviour ^^^^^^^^^^^^^^^^^^^ @@ -839,11 +898,23 @@ Oracle Requires ``oracledb`` (``pip install "python-db2sql[oracle]"``). Identify the database with **either** ``service_name`` **or** ``sid`` under -``server.options`` — neither is exposed as a top-level CLI flag, so Oracle -connections need a small config file. ``server.dbname`` is used as a +``server.options`` — neither has a dedicated CLI flag, so Oracle connections +are usually described in a small config file. ``server.dbname`` is used as a fallback SID. Use ``server.options.owner`` to dump a single schema (Oracle owners are upper-cased automatically). +Alternatively, :option:`--source-dsn` expresses the same thing in one +argument and needs no file: + +.. code-block:: console + + $ export DB2SQL_SOURCE_DSN='oracle+oracledb://admin:s3cr3t@oracle.example.com:1521/?service_name=ORCLPDB1' + $ db2sql dump --driver oracle -f dump.sql + +Note that ``options.owner`` has no DSN equivalent — it filters the export +rather than the connection, so a config file is still required to restrict +the dump to a single owner. + Via ``service_name`` (typical for pluggable databases), filtered to one owner: diff --git a/docs/configuration.rst b/docs/configuration.rst index 475c316..215ae4c 100644 --- a/docs/configuration.rst +++ b/docs/configuration.rst @@ -146,6 +146,16 @@ Connection parameters for the source database. * - ``dbname`` - ``null`` - Database name (or SQLite file path). + * - ``dsn`` + - ``null`` + - Full SQLAlchemy URL for the source, e.g. + ``postgresql+psycopg2://user:pwd@host:5432/db?sslmode=require``. It + **replaces** ``hostname``, ``port``, ``username``, ``password`` and + ``dbname`` rather than merging with them, and its dialect must match + ``driver``. Declaring it alongside any of those keys **in the same + file** is rejected as a contradiction; overriding a file's connection + with ``--source-dsn`` on the command line remains valid. See + :option:`--source-dsn`. * - ``options`` - ``{}`` - Driver-specific extra options passed as key/value pairs @@ -184,6 +194,12 @@ override the corresponding fields here, which in turn override the * - ``dbname`` - ``null`` - Target database name. + * - ``dsn`` + - ``null`` + - Full SQLAlchemy URL for the target. Same semantics as ``server.dsn``: + it replaces the discrete fields above, its dialect must match + ``target``, and combining it with those fields in the same file is + rejected. See :option:`--target-dsn`. * - ``options`` - ``{}`` - Driver-specific extra options for the target connection. diff --git a/tests/cli/test_cli_sqlite.py b/tests/cli/test_cli_sqlite.py index 583cdb2..2912484 100644 --- a/tests/cli/test_cli_sqlite.py +++ b/tests/cli/test_cli_sqlite.py @@ -5,7 +5,7 @@ import sys from pathlib import Path -from db2sql.interface.cli import Cli +from db2sql.interface.cli import Cli, main def test_cli_main_end_to_end(sample_db: Path, tmp_path: Path, monkeypatch) -> None: @@ -95,6 +95,55 @@ def test_cli_explicit_dump_command_matches_implicit_form( assert explicit_file.read_text() == implicit_file.read_text() +def test_cli_source_dsn_matches_the_discrete_connection_flags( + sample_db: Path, tmp_path: Path, monkeypatch +) -> None: + """A DSN must reach the very same database as -d does.""" + monkeypatch.setattr(sys, "argv", ["db2sql"]) + via_flags = tmp_path / "flags.sql" + via_dsn = tmp_path / "dsn.sql" + + assert ( + Cli().run(["dump", "--driver", "sqlite", "-d", str(sample_db), "-f", str(via_flags)]) == 0 + ) + assert ( + Cli().run( + [ + "dump", + "--driver", + "sqlite", + "--source-dsn", + f"sqlite:///{sample_db}", + "-f", + str(via_dsn), + ] + ) + == 0 + ) + + assert via_dsn.read_text() == via_flags.read_text() + + +def test_cli_source_dsn_of_another_dialect_is_rejected( + sample_db: Path, tmp_path: Path, monkeypatch +) -> None: + """A postgres DSN handed to the sqlite driver must fail, not connect anyway.""" + monkeypatch.setattr(sys, "argv", ["db2sql"]) + output_file = tmp_path / "never.sql" + rc = main( + [ + "dump", + "--driver", + "sqlite", + "--source-dsn", + "postgresql://u:p@h/d", + "-f", + str(output_file), + ] + ) + assert rc != 0 + + def test_cli_dump_command_accepts_options_before_and_after_the_verb( sample_db: Path, tmp_path: Path, monkeypatch ) -> None: diff --git a/tests/unit/infrastructure/config/test_loader.py b/tests/unit/infrastructure/config/test_loader.py index 44ddedd..e22f8a3 100644 --- a/tests/unit/infrastructure/config/test_loader.py +++ b/tests/unit/infrastructure/config/test_loader.py @@ -156,3 +156,45 @@ def test_unsupported_extension_error_carries_path(tmp_path: Path) -> None: err = ConfigUnsupportedFileExtensionError(".toml", str(tmp_path / "x.toml")) assert ".toml" in err.message assert "x.toml" in err.message + + +def test_config_file_rejects_dsn_next_to_discrete_server_keys(tmp_path: Path) -> None: + """Declaring both in one file states two contradictory intents.""" + cfg = tmp_path / "db2sql.yml" + cfg.write_text( + "driver: postgres\n" "server:\n" " dsn: postgresql://u:p@h/db\n" " hostname: elsewhere\n" + ) + with pytest.raises(ConfigInvalidError, match=r"server\.dsn cannot be combined"): + load_config(cfg) + + +def test_config_file_rejects_dsn_next_to_discrete_target_keys(tmp_path: Path) -> None: + cfg = tmp_path / "db2sql.yml" + cfg.write_text( + "driver: postgres\n" + "target_server:\n" + " dsn: postgresql://u:p@h/db\n" + " dbname: elsewhere\n" + ) + with pytest.raises(ConfigInvalidError, match=r"target_server\.dsn cannot be combined"): + load_config(cfg) + + +def test_config_file_accepts_a_dsn_on_its_own(tmp_path: Path) -> None: + cfg = tmp_path / "db2sql.yml" + cfg.write_text("driver: postgres\nserver:\n dsn: postgresql://u:p@h/db\n") + assert load_config(cfg).server.dsn == "postgresql://u:p@h/db" + + +def test_config_file_accepts_a_dsn_next_to_non_connection_keys(tmp_path: Path) -> None: + """options describe what to export, not how to connect — no conflict.""" + cfg = tmp_path / "db2sql.yml" + cfg.write_text( + "driver: oracle\n" + "server:\n" + " dsn: oracle+oracledb://u:p@h:1521/?service_name=PDB1\n" + " options:\n" + " owner: HR\n" + ) + config = load_config(cfg) + assert config.server.options == {"owner": "HR"} diff --git a/tests/unit/infrastructure/test_url.py b/tests/unit/infrastructure/test_url.py index b52723f..34dbc8c 100644 --- a/tests/unit/infrastructure/test_url.py +++ b/tests/unit/infrastructure/test_url.py @@ -5,8 +5,8 @@ import pytest from sqlalchemy.engine import make_url -from db2sql.infrastructure.config import ServerConfig -from db2sql.infrastructure.url import build_url, redact_url +from db2sql.infrastructure.config import ConfigInvalidError, ServerConfig +from db2sql.infrastructure.url import build_url, database_from_url, redact_url def test_build_url_full_shape() -> None: @@ -43,6 +43,64 @@ def test_credentials_survive_a_round_trip_through_sqlalchemy(password: str) -> N assert parsed.database == "d" +def test_dsn_replaces_every_other_field() -> None: + server = ServerConfig( + hostname="ignored", + port=1, + username="ignored", + password="ignored", + dbname="ignored", + dsn="postgresql+psycopg2://u:p@real:5432/db?sslmode=require", + ) + url = build_url(server, "postgresql+psycopg2", database="ignored", query={"a": "b"}) + assert url == "postgresql+psycopg2://u:p@real:5432/db?sslmode=require" + + +def test_dsn_may_select_another_dbapi_for_the_same_dialect() -> None: + """Swapping psycopg2 for another driver is exactly the point of a DSN.""" + server = ServerConfig(dsn="postgresql+asyncpg://u:p@h/db") + assert build_url(server, "postgresql+psycopg2") == "postgresql+asyncpg://u:p@h/db" + + +def test_dsn_with_a_bare_dialect_is_accepted() -> None: + server = ServerConfig(dsn="postgresql://u:p@h/db") + assert build_url(server, "postgresql+psycopg2") == "postgresql://u:p@h/db" + + +def test_dsn_targeting_another_dialect_is_rejected() -> None: + server = ServerConfig(dsn="mysql+pymysql://u:p@h/db") + with pytest.raises(ConfigInvalidError, match="does not match the selected driver"): + build_url(server, "postgresql+psycopg2") + + +def test_malformed_dsn_is_rejected() -> None: + with pytest.raises(ConfigInvalidError, match="expected a '://' URL"): + build_url(ServerConfig(dsn="not-a-url"), "postgresql+psycopg2") + + +def test_fields_shadowed_by_dsn_lists_what_is_ignored() -> None: + server = ServerConfig(hostname="h", dbname="d", dsn="postgresql://u@h/d") + assert server.fields_shadowed_by_dsn() == ("hostname", "dbname") + + +def test_fields_shadowed_by_dsn_is_empty_without_a_dsn() -> None: + assert ServerConfig(hostname="h", dbname="d").fields_shadowed_by_dsn() == () + + +@pytest.mark.parametrize( + ("url", "expected"), + [ + ("mysql+pymysql://u:p@h:3306/main", "main"), + ("mysql+pymysql://u:p@h/main?charset=utf8mb4", "main"), + ("mysql+pymysql://u:p@h/my%20db", "my db"), + ("mysql+pymysql://u:p@h/", None), + ("mysql+pymysql://u:p@h", None), + ], +) +def test_database_from_url(url: str, expected: object) -> None: + assert database_from_url(url) == expected + + def test_redact_url_masks_the_password() -> None: url = "postgresql+psycopg2://u:s3cr3t@h:5432/d" assert redact_url(url) == "postgresql+psycopg2://u:***@h:5432/d" diff --git a/tests/unit/interface/cli/test_parser.py b/tests/unit/interface/cli/test_parser.py index 00ca658..632f537 100644 --- a/tests/unit/interface/cli/test_parser.py +++ b/tests/unit/interface/cli/test_parser.py @@ -8,7 +8,7 @@ import pytest -from db2sql.infrastructure.config.errors import ConfigMissingError +from db2sql.infrastructure.config.errors import ConfigInvalidError, ConfigMissingError from db2sql.interface.cli.parser import ( AbortExecution, CommandLineError, @@ -232,6 +232,129 @@ def test_root_help_hides_dump_options_but_lists_the_commands(clean_env) -> None: assert command in help_text +def test_source_dsn_lands_in_the_server_config(clean_env) -> None: + dsn = "postgresql+psycopg2://u:p@h:5432/db?sslmode=require" + ns = build_parser().parse_args_with_config( + ["dump", "--driver", "postgres", "--source-dsn", dsn] + ) + assert ns.config.server.dsn == dsn + + +def test_source_dsn_works_on_the_implicit_dump_form(clean_env) -> None: + dsn = "sqlite:///app.db" + ns = build_parser().parse_args_with_config(["--driver", "sqlite", "--source-dsn", dsn]) + assert ns.config.server.dsn == dsn + + +def test_source_dsn_is_accepted_by_migrate_and_validate(clean_env) -> None: + dsn = "sqlite:///app.db" + for command in ("migrate", "validate"): + ns = build_parser().parse_args_with_config( + [command, "--driver", "sqlite", "--source-dsn", dsn] + ) + assert ns.config.server.dsn == dsn, command + + +def test_target_dsn_lands_in_the_target_server_config(clean_env) -> None: + dsn = "postgresql+psycopg2://u:p@target:5432/db" + ns = build_parser().parse_args_with_config(["migrate", "--target-dsn", dsn]) + assert ns.config.target_server.dsn == dsn + + +def test_target_dsn_is_not_offered_on_dump(clean_env) -> None: + """--target-dsn only makes sense for a live migration.""" + with pytest.raises(SystemExit): + build_parser().parse_args_with_config(["dump", "--target-dsn", "postgresql://u@h/d"]) + + +def test_source_dsn_reads_its_environment_variable(clean_env) -> None: + clean_env.setenv("DB2SQL_SOURCE_DSN", "sqlite:///from-env.db") + ns = build_parser().parse_args_with_config(["dump", "--driver", "sqlite"]) + assert ns.config.server.dsn == "sqlite:///from-env.db" + + +@pytest.mark.parametrize( + "conflicting", + [ + ["-H", "host"], + ["--host", "host"], + ["--host=host"], + ["-Hhost"], + ["-d", "db"], + ["-u", "user"], + ["-p", "pwd"], + ["-W"], + ["--ask-password"], + ], +) +def test_source_dsn_combined_with_a_connection_flag_is_rejected(clean_env, conflicting) -> None: + """Both on one command line is a contradiction, whatever the spelling.""" + parser = build_parser() + with pytest.raises(ConfigInvalidError): + parser.parse_args_with_config( + ["dump", "--driver", "sqlite", "--source-dsn", "sqlite:///a.db"] + conflicting + ) + + +def test_source_dsn_conflict_is_detected_across_the_subcommand(clean_env) -> None: + parser = build_parser() + with pytest.raises(ConfigInvalidError): + parser.parse_args_with_config( + ["-H", "host", "dump", "--driver", "sqlite", "--source-dsn", "sqlite:///a.db"] + ) + + +def test_source_dsn_error_names_the_conflicting_flags(clean_env) -> None: + parser = build_parser() + with pytest.raises(ConfigInvalidError) as exc: + parser.parse_args_with_config( + ["dump", "--source-dsn", "sqlite:///a.db", "-H", "h", "-d", "db"] + ) + assert "--source-dsn cannot be combined with -H, -d" in exc.value.message + + +def test_help_wins_over_a_dsn_conflict(clean_env) -> None: + """Asking for help must never fail, however contradictory the rest is.""" + parser = build_parser() + with pytest.raises((AbortExecution, SystemExit)): + parser.parse_args_with_config(["dump", "--source-dsn", "sqlite:///a.db", "-H", "h", "-h"]) + + +def test_target_dsn_combined_with_a_target_flag_is_rejected(clean_env) -> None: + parser = build_parser() + with pytest.raises(ConfigInvalidError): + parser.parse_args_with_config( + ["migrate", "--target-dsn", "postgresql://h/d", "--target-host", "h"] + ) + + +def test_target_dsn_does_not_clash_with_the_source_flags(clean_env) -> None: + """The two endpoints are independent: a target DSN says nothing about -H.""" + ns = build_parser().parse_args_with_config( + ["migrate", "--driver", "sqlite", "-d", "src.db", "--target-dsn", "postgresql://h/d"] + ) + assert ns.config.server.dbname == "src.db" + assert ns.config.target_server.dsn == "postgresql://h/d" + + +def test_source_dsn_may_override_a_config_file_host(clean_env, tmp_path: Path) -> None: + """Cross-layer override stays legal — only same-layer conflicts are blocked.""" + cfg = tmp_path / "db2sql.json" + cfg.write_text(json.dumps({"driver": "sqlite", "server": {"hostname": "from-file"}})) + ns = build_parser().parse_args_with_config( + ["dump", "-C", str(cfg), "--source-dsn", "sqlite:///a.db"] + ) + assert ns.config.server.dsn == "sqlite:///a.db" + + +def test_source_dsn_may_override_an_environment_host(clean_env) -> None: + clean_env.setenv("DB2SQL_HOST", "from-env") + ns = build_parser().parse_args_with_config( + ["dump", "--driver", "sqlite", "--source-dsn", "sqlite:///a.db"] + ) + assert ns.config.server.dsn == "sqlite:///a.db" + + def test_dump_help_documents_the_options_without_leaking_suppress(clean_env, capsys) -> None: """A SUPPRESS default must not surface as '(default: ==SUPPRESS==)'.""" with pytest.raises(SystemExit): diff --git a/tests/unit/interface/cli/test_runner.py b/tests/unit/interface/cli/test_runner.py index e766050..bd56595 100644 --- a/tests/unit/interface/cli/test_runner.py +++ b/tests/unit/interface/cli/test_runner.py @@ -2,6 +2,7 @@ from __future__ import annotations +import json import io from pathlib import Path from typing import Any, Iterator, Tuple @@ -48,9 +49,7 @@ def collect_metadata(self) -> Database: db.add_schema(public) return db - def iter_rows( - self, schema: str, table: Table, limit: int = -1 - ) -> Iterator[Tuple[Any, ...]]: + def iter_rows(self, schema: str, table: Table, limit: int = -1) -> Iterator[Tuple[Any, ...]]: yield (1,) @@ -211,3 +210,58 @@ def test_runner_signal_handlers_can_be_invoked(monkeypatch) -> None: with pytest.raises(SystemExit) as exc: sigterm_handler(_signal.SIGTERM, None) assert exc.value.code == 4 + + +def test_cli_warns_when_a_dsn_overrides_a_host_from_the_config_file( + tmp_path: Path, monkeypatch, capsys +) -> None: + """Overriding a file across layers is legal — but must not happen silently.""" + monkeypatch.delenv("DB2SQL_CONFIG", raising=False) + cfg = tmp_path / "db2sql.json" + cfg.write_text(json.dumps({"server": {"hostname": "from-file", "dbname": "from-file"}})) + output = tmp_path / "out.sql" + + rc = Cli().run( + [ + "--driver", + "silent-reader", + "-C", + str(cfg), + "--source-dsn", + "silent-reader://host/db", + "-f", + str(output), + ] + ) + + assert rc == SUCCESS + captured = capsys.readouterr().out + assert "--source-dsn is set" in captured + assert "hostname" in captured and "dbname" in captured + + +def test_cli_rejects_a_dsn_combined_with_flags_on_the_same_command_line( + tmp_path: Path, monkeypatch +) -> None: + """Same-layer contradiction is a mistake, not an override: fail loudly.""" + monkeypatch.delenv("DB2SQL_CONFIG", raising=False) + with pytest.raises(ConfigError): + Cli().run( + [ + "--driver", + "silent-reader", + "--source-dsn", + "silent-reader://host/db", + "-H", + "conflicting", + "-f", + str(tmp_path / "never.sql"), + ] + ) + + +def test_cli_does_not_warn_without_a_dsn(tmp_path: Path, monkeypatch, capsys) -> None: + monkeypatch.delenv("DB2SQL_CONFIG", raising=False) + output = tmp_path / "out.sql" + assert Cli().run(["--driver", "silent-reader", "-d", "db", "-f", str(output)]) == SUCCESS + assert "--source-dsn is set" not in capsys.readouterr().out From d58185b974ec485d6cff302c4d218dcf909ccba6 Mon Sep 17 00:00:00 2001 From: Jacques Raphanel Date: Mon, 17 Aug 2026 17:35:38 +0000 Subject: [PATCH 4/5] docs: update the examples and prose missed by the dump/DSN sweep The plugin example READMEs, plugins.rst and the configuration recipes still invoked db2sql without a command. The example READMEs also used a lowercase -c, which has never been a valid flag. Also state that the init wizard does not generate a DSN. --- docs/cli.rst | 10 +++++++++- docs/configuration.rst | 2 +- docs/plugins.rst | 4 ++-- examples/README.md | 2 +- examples/csv-producer/README.md | 2 +- examples/sqlite-emitter/README.md | 4 ++-- examples/yaml-to-markdown/README.md | 2 +- 7 files changed, 17 insertions(+), 9 deletions(-) diff --git a/docs/cli.rst b/docs/cli.rst index 63b1c59..15bf981 100644 --- a/docs/cli.rst +++ b/docs/cli.rst @@ -396,7 +396,7 @@ The ``init`` subcommand ``db2sql init`` is an interactive wizard that asks a few questions about the source database, the target dialect, and the dump options, then prints (or -writes) a configuration file ready to be passed to ``db2sql -C``. +writes) a configuration file ready to be passed to ``db2sql dump -C``. .. code-block:: text @@ -418,6 +418,14 @@ must match before it is written. The generated file only contains the values you set — defaults are left out to keep the file short and focused. +.. note:: + + The wizard only asks for the discrete connection fields; it never + produces a ``server.dsn``. If you need a DSN, add it to the generated + file by hand and remove the ``hostname`` / ``port`` / ``dbname`` / + ``username`` / ``password`` keys it replaces — keeping both in the same + file is rejected. See :option:`--source-dsn`. + .. option:: -o PATH, --output PATH Write the generated file to ``PATH``. Without this flag, the file is diff --git a/docs/configuration.rst b/docs/configuration.rst index 215ae4c..8224eb4 100644 --- a/docs/configuration.rst +++ b/docs/configuration.rst @@ -491,7 +491,7 @@ Recipes Each recipe below is a complete, copy-pasteable config file. Drop it as ``db2sql.yml`` in the current directory (or pass it with ``-C path.yml``) -and run ``db2sql -f dump.sql``. +and run ``db2sql dump -f dump.sql``. SQLite → Postgres ~~~~~~~~~~~~~~~~~ diff --git a/docs/plugins.rst b/docs/plugins.rst index b8f89c3..e9fdde0 100644 --- a/docs/plugins.rst +++ b/docs/plugins.rst @@ -28,8 +28,8 @@ The name you register becomes a usable value of ``driver:`` (readers) or :option:`--driver` / :option:`--target` on the CLI. A target dialect can ship either an emitter, a writer, or both. The built-in -``postgres`` and ``mssql`` targets ship both, so ``db2sql --target postgres`` -(dump) and ``db2sql --target postgres migrate`` use the matched pair. +``postgres`` and ``mssql`` targets ship both, so ``db2sql dump --target postgres`` +and ``db2sql migrate --target postgres`` use the matched pair. .. tip:: diff --git a/examples/README.md b/examples/README.md index da5a170..5a7bfb7 100644 --- a/examples/README.md +++ b/examples/README.md @@ -30,7 +30,7 @@ The name you register becomes a usable value of `driver:` (readers) or ```bash cd examples/csv-producer pip install -e . # registers driver=csv -db2sql -c db2sql.yml # uses the new reader, pipes to the built-in postgres emitter +db2sql dump -C db2sql.yml # uses the new reader, pipes to the built-in postgres emitter ``` To verify a plugin was picked up: diff --git a/examples/csv-producer/README.md b/examples/csv-producer/README.md index 6415ae0..505d99b 100644 --- a/examples/csv-producer/README.md +++ b/examples/csv-producer/README.md @@ -21,7 +21,7 @@ csv = "csv_producer:build_reader" ## Run ```bash -db2sql -c db2sql.yml +db2sql dump -C db2sql.yml ``` `db2sql.yml` points the reader at `./sample_data`, which contains two CSV diff --git a/examples/sqlite-emitter/README.md b/examples/sqlite-emitter/README.md index 69600b3..e29309d 100644 --- a/examples/sqlite-emitter/README.md +++ b/examples/sqlite-emitter/README.md @@ -4,7 +4,7 @@ A db2sql plugin that registers a new `target: sqlite`. It produces SQL that can be loaded straight into `sqlite3`: ```bash -db2sql -c db2sql.yml > dump.sql +db2sql dump -C db2sql.yml > dump.sql sqlite3 destination.db < dump.sql ``` @@ -32,7 +32,7 @@ Point any existing reader at the example. The provided `db2sql.yml` reuses the built-in SQLite reader for source, and uses the new emitter for output: ```bash -db2sql -c db2sql.yml > dump.sql +db2sql dump -C db2sql.yml > dump.sql ``` ## What to look at diff --git a/examples/yaml-to-markdown/README.md b/examples/yaml-to-markdown/README.md index e4335fa..ca5d17c 100644 --- a/examples/yaml-to-markdown/README.md +++ b/examples/yaml-to-markdown/README.md @@ -29,7 +29,7 @@ emitter. ```bash mkdir -p docs -db2sql -c db2sql.yml +db2sql dump -C db2sql.yml cat docs/schema.md ``` From 3d9be499e8c900619693c94946796cc120289cc2 Mon Sep 17 00:00:00 2001 From: Jacques Raphanel Date: Mon, 17 Aug 2026 17:40:20 +0000 Subject: [PATCH 5/5] chore: cover the target writers' connection string and redaction Both properties are pure functions of the config, but were only reachable through the functional suite, which does not run on every pull request. This also puts the --target-dsn path and the target-side password redaction under test without needing a live server. --- .../writer/test_target_connection_string.py | 94 +++++++++++++++++++ 1 file changed, 94 insertions(+) create mode 100644 tests/unit/infrastructure/writer/test_target_connection_string.py diff --git a/tests/unit/infrastructure/writer/test_target_connection_string.py b/tests/unit/infrastructure/writer/test_target_connection_string.py new file mode 100644 index 0000000..3a3eee2 --- /dev/null +++ b/tests/unit/infrastructure/writer/test_target_connection_string.py @@ -0,0 +1,94 @@ +"""Target writers: how the connection URL is built and how it is logged. + +The rest of each writer needs a live server and is exercised by the +functional suite. These two properties are pure functions of the config, so +they are worth pinning here — they carry the ``--target-dsn`` path and the +password redaction, neither of which should wait for a database to be proven. +""" + +from __future__ import annotations + +from typing import Any, Tuple + +import pytest + +from db2sql.infrastructure.config import AppConfig, ConfigInvalidError, ServerConfig +from db2sql.infrastructure.writer.mssql import MssqlTargetWriter +from db2sql.infrastructure.writer.postgres import PostgresTargetWriter + + +class _StubLogger: + def trace(self, message: str) -> None: ... + def debug(self, message: str) -> None: ... + def info(self, message: str) -> None: ... + def warning(self, message: str) -> None: ... + def error(self, message: str) -> None: ... + + +_WRITERS: Tuple[Tuple[Any, str, str], ...] = ( + (PostgresTargetWriter, "postgres", "postgresql+psycopg2"), + (MssqlTargetWriter, "mssql", "mssql+pymssql"), +) + + +def _writer(writer_class: Any, target: str, **server: object) -> Any: + config = AppConfig(target=target, target_server=ServerConfig(**server)) + return writer_class(config, _StubLogger()) + + +@pytest.mark.parametrize(("writer_class", "target", "scheme"), _WRITERS) +def test_connection_string_is_built_from_the_target_server( + writer_class: Any, target: str, scheme: str +) -> None: + writer = _writer( + writer_class, target, hostname="h", port=1234, username="u", password="p", dbname="d" + ) + assert writer._connection_string == f"{scheme}://u:p@h:1234/d" + + +@pytest.mark.parametrize(("writer_class", "target", "scheme"), _WRITERS) +def test_connection_string_escapes_the_credentials( + writer_class: Any, target: str, scheme: str +) -> None: + writer = _writer( + writer_class, target, hostname="h", username="u", password="p@ss/w", dbname="d" + ) + assert writer._connection_string == f"{scheme}://u:p%40ss%2Fw@h/d" + + +@pytest.mark.parametrize(("writer_class", "target", "scheme"), _WRITERS) +def test_target_dsn_replaces_the_discrete_fields( + writer_class: Any, target: str, scheme: str +) -> None: + dsn = f"{scheme}://u:p@real:5432/db?connect_timeout=3" + writer = _writer(writer_class, target, hostname="ignored", dsn=dsn) + assert writer._connection_string == dsn + + +@pytest.mark.parametrize(("writer_class", "target", "scheme"), _WRITERS) +def test_target_dsn_of_another_dialect_is_rejected( + writer_class: Any, target: str, scheme: str +) -> None: + writer = _writer(writer_class, target, dsn="mysql+pymysql://u:p@h/db") + with pytest.raises(ConfigInvalidError, match="does not match the selected driver"): + _ = writer._connection_string + + +@pytest.mark.parametrize(("writer_class", "target", "scheme"), _WRITERS) +def test_redacted_connection_string_hides_the_password( + writer_class: Any, target: str, scheme: str +) -> None: + writer = _writer( + writer_class, target, hostname="h", port=1234, username="u", password="s3cr3t", dbname="d" + ) + redacted = writer._connection_string_redacted + assert redacted == f"{scheme}://u:***@h:1234/d" + assert "s3cr3t" not in redacted + + +@pytest.mark.parametrize(("writer_class", "target", "scheme"), _WRITERS) +def test_redacted_connection_string_hides_a_dsn_password( + writer_class: Any, target: str, scheme: str +) -> None: + writer = _writer(writer_class, target, dsn=f"{scheme}://u:s3cr3t@h/db") + assert "s3cr3t" not in writer._connection_string_redacted