diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d5baa15f..9bd2562a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -42,12 +42,9 @@ jobs: run: uv tool run ty check --python-version 3.10 - name: Build the stack - run: docker compose up -d mysql postgres presto trino clickhouse vertica + run: docker compose up -d --wait mysql postgres presto trino clickhouse - name: Run tests env: DATADIFF_CLICKHOUSE_URI: "clickhouse://clickhouse:Password1@localhost:9000/clickhouse" - DATADIFF_VERTICA_URI: "vertica://vertica:Password1@localhost:5433/vertica" - run: | - chmod +x tests/waiting_for_stack_up.sh - ./tests/waiting_for_stack_up.sh && uv run pytest tests/ + run: uv run pytest tests/ diff --git a/.github/workflows/ci_full.yml b/.github/workflows/ci_full.yml index b1061a71..9808270f 100644 --- a/.github/workflows/ci_full.yml +++ b/.github/workflows/ci_full.yml @@ -34,12 +34,9 @@ jobs: run: uv sync --frozen - name: Build the stack - run: docker compose up -d mysql postgres presto trino clickhouse vertica + run: docker compose up -d --wait mysql postgres presto trino clickhouse - name: Run tests env: DATADIFF_CLICKHOUSE_URI: "clickhouse://clickhouse:Password1@localhost:9000/clickhouse" - DATADIFF_VERTICA_URI: "vertica://vertica:Password1@localhost:5433/vertica" - run: | - chmod +x tests/waiting_for_stack_up.sh - ./tests/waiting_for_stack_up.sh && uv run pytest tests/ + run: uv run pytest tests/ diff --git a/data_diff/databases/mssql.py b/data_diff/databases/mssql.py index 917e00ce..d84eab94 100644 --- a/data_diff/databases/mssql.py +++ b/data_diff/databases/mssql.py @@ -178,9 +178,6 @@ def __init__(self, host, port, user, password, *, database, thread_count, **kw) self._args = {k: v for k, v in args.items() if v is not None} self._args["driver"] = "{ODBC Driver 18 for SQL Server}" - # TODO temp dev debug - self._args["TrustServerCertificate"] = "yes" - try: self.default_database = self._args["database"] self.default_schema = self._args["schema"] @@ -195,6 +192,16 @@ def create_connection(self): connection = self._mssql.connect(**self._args) return connection except self._mssql.Error as error: + lower_msg = str(error).lower() + if "ssl" in lower_msg or "certificate" in lower_msg: + raise ConnectError( + f"TLS certificate validation failed connecting to " + f"{self._args.get('server', 'unknown host')}. " + f"ODBC Driver 18 requires encrypted connections by default. " + f"If using a self-signed certificate, pass " + f"TrustServerCertificate='yes' in your connection URI or TOML config. " + f"Original error: {error}" + ) from error raise ConnectError(*error.args) from error def select_table_schema(self, path: DbPath) -> str: diff --git a/tests/test_mssql.py b/tests/test_mssql.py new file mode 100644 index 00000000..b263cbea --- /dev/null +++ b/tests/test_mssql.py @@ -0,0 +1,76 @@ +from unittest.mock import MagicMock, patch + +import pytest + +from data_diff.databases.base import ConnectError +from data_diff.databases.mssql import MsSQL + + +def _make_mssql(**extra_kw): + """Create an MsSQL instance with mocked threading and pyodbc.""" + with patch.object(MsSQL, "__attrs_post_init__", lambda self: None): + return MsSQL( + host="localhost", + port=1433, + user="sa", + password="secret", + database="testdb", + schema="dbo", + thread_count=1, + **extra_kw, + ) + + +class TestMsSQLConnectionArgs: + def test_trust_server_certificate_not_set_by_default(self): + db = _make_mssql() + assert "TrustServerCertificate" not in db._args + + def test_user_supplied_trust_server_certificate_preserved(self): + db = _make_mssql(TrustServerCertificate="yes") + assert db._args["TrustServerCertificate"] == "yes" + + def test_driver_is_set(self): + db = _make_mssql() + assert db._args["driver"] == "{ODBC Driver 18 for SQL Server}" + + def test_none_values_filtered_from_args(self): + db = _make_mssql() + for v in db._args.values(): + assert v is not None + + +class TestMsSQLConnectionErrors: + def test_ssl_error_provides_actionable_message(self): + db = _make_mssql() + mock_mssql = MagicMock() + mock_mssql.Error = type("Error", (Exception,), {}) + mock_mssql.connect.side_effect = mock_mssql.Error( + "[SSL Provider] The certificate chain was issued by an untrusted authority" + ) + + with patch("data_diff.databases.mssql.import_mssql", return_value=mock_mssql): + with pytest.raises(ConnectError, match="TrustServerCertificate"): + db.create_connection() + + def test_certificate_only_keyword_triggers_actionable_message(self): + db = _make_mssql() + mock_mssql = MagicMock() + mock_mssql.Error = type("Error", (Exception,), {}) + mock_mssql.connect.side_effect = mock_mssql.Error( + "The remote certificate was rejected by the verification procedure" + ) + + with patch("data_diff.databases.mssql.import_mssql", return_value=mock_mssql): + with pytest.raises(ConnectError, match="TrustServerCertificate"): + db.create_connection() + + def test_non_ssl_error_passes_through(self): + db = _make_mssql() + mock_mssql = MagicMock() + mock_mssql.Error = type("Error", (Exception,), {}) + mock_mssql.connect.side_effect = mock_mssql.Error("Login failed for user") + + with patch("data_diff.databases.mssql.import_mssql", return_value=mock_mssql): + with pytest.raises(ConnectError, match="Login failed"): + db.create_connection() diff --git a/tests/waiting_for_stack_up.sh b/tests/waiting_for_stack_up.sh index 762138de..6b933f8f 100644 --- a/tests/waiting_for_stack_up.sh +++ b/tests/waiting_for_stack_up.sh @@ -1,17 +1,3 @@ #!/bin/bash -if [ -n "$DATADIFF_VERTICA_URI" ] - then - echo "Check Vertica DB running..." - while true - do - if docker logs dd-vertica | tail -n 100 | grep -q -i "vertica is now running" - then - echo "Vertica DB is ready"; - break; - else - echo "Waiting for Vertica DB starting..."; - sleep 10; - fi - done -fi +# Placeholder: add service readiness checks here as needed.