Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion ddpui/core/dbtfunctions.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,11 @@ def map_airbyte_destination_spec_to_dbtcli_profile(conn_info: dict):
conn_info["user"] = conn_info["username"]

# handle dbt ssl params
if "ssl_mode" in conn_info:
# ssl: false is the authoritative "no SSL" signal — takes precedence over ssl_mode
# because Airbyte can send both fields simultaneously
if "ssl" in conn_info and conn_info["ssl"] is False:
conn_info["sslmode"] = "disable"
elif "ssl_mode" in conn_info:
ssl_data = conn_info["ssl_mode"]
mode = ssl_data["mode"] if "mode" in ssl_data else None
ca_certificate = ssl_data["ca_certificate"] if "ca_certificate" in ssl_data else None
Expand Down
14 changes: 6 additions & 8 deletions ddpui/ddpairbyte/airbytehelpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@
from ddpui.utils.helpers import (
generate_hash_id,
update_dict_but_not_stars,
resolve_stars,
from_timestamp,
nice_bytes,
get_integer_env_var,
Expand Down Expand Up @@ -854,9 +855,13 @@ def update_destination(org: Org, destination_id: str, payload: AirbyteDestinatio
warehouse.name = payload.name
warehouse.save()

curr_credentials = secretsmanager.retrieve_warehouse_credentials(warehouse)

# payload is the authoritative source of truth; only replace starred values
# from curr_credentials — never carry over keys absent from the new payload
airbyte_creds = {}
if warehouse.wtype == "postgres":
airbyte_creds = update_dict_but_not_stars(payload.config)
airbyte_creds = resolve_stars(payload.config, curr_credentials)
Comment thread
coderabbitai[bot] marked this conversation as resolved.
# i've forgotten why we have this here, airbyte sends "database" - RC
if "dbname" in airbyte_creds:
airbyte_creds["database"] = airbyte_creds["dbname"]
Expand All @@ -875,13 +880,6 @@ def update_destination(org: Org, destination_id: str, payload: AirbyteDestinatio
else:
raise ValueError("unknown warehouse type " + warehouse.wtype)

curr_credentials = secretsmanager.retrieve_warehouse_credentials(warehouse)

# copy over the value of keys that are missing from in airbyte_creds (these are probably that have all '*****')
for key, value in curr_credentials.items():
if key not in airbyte_creds:
airbyte_creds[key] = value

secretsmanager.update_warehouse_credentials(warehouse, airbyte_creds)

create_or_update_dbt_profile_secret_blk(org, warehouse, airbyte_creds)
Expand Down
40 changes: 40 additions & 0 deletions ddpui/tests/core/test_dbtfunctions.py
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,46 @@ def test_map_airbyte_destination_spec_to_dbtcli_profile_ssl_mode_only(tmpdir):
assert "sslrootcert_content" not in res


def test_map_airbyte_ssl_false_sets_sslmode_disable():
"""ssl: false must map to sslmode: disable regardless of ssl_mode presence."""
conn_info = {"host": "h", "ssl": False}
res = map_airbyte_destination_spec_to_dbtcli_profile(conn_info)
assert res["sslmode"] == "disable"


def test_map_airbyte_ssl_false_overrides_ssl_mode():
"""ssl: false takes precedence — ssl_mode cert config must be ignored."""
conn_info = {
"host": "h",
"ssl": False,
"ssl_mode": {"mode": "verify-ca", "ca_certificate": "real-cert"},
}
res = map_airbyte_destination_spec_to_dbtcli_profile(conn_info)
assert res["sslmode"] == "disable"
assert "sslrootcert_content" not in res
assert "sslrootcert" not in res


def test_map_airbyte_ssl_true_falls_through_to_ssl_mode():
"""ssl: true alone doesn't set sslmode — ssl_mode dict is the source of truth."""
conn_info = {
"host": "h",
"ssl": True,
"ssl_mode": {"mode": "require"},
}
res = map_airbyte_destination_spec_to_dbtcli_profile(conn_info)
assert res["sslmode"] == "require"


def test_map_airbyte_ssl_false_with_no_ssl_mode():
"""ssl: false with no ssl_mode still produces sslmode: disable."""
conn_info = {"host": "h", "ssl": False}
res = map_airbyte_destination_spec_to_dbtcli_profile(conn_info)
assert res["sslmode"] == "disable"
assert "sslrootcert_content" not in res
assert "sslrootcert" not in res


# ============================================================================
# preprocess_airbyte_creds_for_dbt
# ============================================================================
Expand Down
107 changes: 107 additions & 0 deletions ddpui/tests/helper/test_airbytehelpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -1822,3 +1822,110 @@ def test_get_one_connection_includes_post_sync_transform(
DataflowOrgTask.objects.filter(orgtask=sync_orgtask).delete()
dataflow.delete()
sync_orgtask.delete()


# ================================================================================
# update_destination SSL credential isolation tests


@patch("ddpui.ddpairbyte.airbyte_service.update_destination", mock_update_destination=Mock())
@patch(
"ddpui.utils.secretsmanager.retrieve_warehouse_credentials",
mock_retrieve_warehouse_credentials=Mock(),
)
@patch(
"ddpui.utils.secretsmanager.update_warehouse_credentials",
mock_update_warehouse_credentials=Mock(),
)
@patch(
"ddpui.ddpairbyte.airbytehelpers.create_or_update_dbt_profile_secret_blk",
mock_create_or_update_dbt_profile_secret_blk=Mock(),
)
def test_update_destination_drops_old_ssl_keys_when_ssl_disabled(
mock_create_or_update_dbt_profile_secret_blk: Mock,
mock_update_warehouse_credentials: Mock,
mock_retrieve_warehouse_credentials: Mock,
mock_update_destination: Mock,
):
"""Old ssl_mode / sslrootcert in curr_credentials must not bleed into the
updated warehouse credentials or the dbt profile secret block when the new
payload omits ssl_mode (i.e. SSL is disabled)."""
org = Org.objects.create(name="org-ssl-disable", slug="org-ssl-disable")
warehouse = OrgWarehouse.objects.create(org=org, wtype="postgres", name="wh")

mock_update_destination.return_value = {"destinationId": "DEST_ID"}
mock_retrieve_warehouse_credentials.return_value = {
"host": "db-host",
"port": "5432",
"password": "real-password",
"ssl_mode": {"mode": "verify-ca", "ca_certificate": "real-cert"},
"sslrootcert": "/home/ddp/global-bundle.pem",
}
mock_update_warehouse_credentials.return_value = None
mock_create_or_update_dbt_profile_secret_blk.return_value = Mock()

# New payload has no ssl_mode — user switched SSL off
payload = AirbyteDestinationUpdate(
name="wh",
destinationDefId="def-id",
config={"host": "db-host", "port": "5432", "password": "*****"},
)
update_destination(org, "dest-id", payload)

saved_creds = mock_update_warehouse_credentials.call_args[0][1]
assert "ssl_mode" not in saved_creds
assert "sslrootcert" not in saved_creds
assert saved_creds["password"] == "real-password"

passed_to_secret_blk = mock_create_or_update_dbt_profile_secret_blk.call_args[0][2]
assert "ssl_mode" not in passed_to_secret_blk
assert "sslrootcert" not in passed_to_secret_blk


@patch("ddpui.ddpairbyte.airbyte_service.update_destination", mock_update_destination=Mock())
@patch(
"ddpui.utils.secretsmanager.retrieve_warehouse_credentials",
mock_retrieve_warehouse_credentials=Mock(),
)
@patch(
"ddpui.utils.secretsmanager.update_warehouse_credentials",
mock_update_warehouse_credentials=Mock(),
)
@patch(
"ddpui.ddpairbyte.airbytehelpers.create_or_update_dbt_profile_secret_blk",
mock_create_or_update_dbt_profile_secret_blk=Mock(),
)
def test_update_destination_starred_ca_cert_resolved_from_curr_credentials(
mock_create_or_update_dbt_profile_secret_blk: Mock,
mock_update_warehouse_credentials: Mock,
mock_retrieve_warehouse_credentials: Mock,
mock_update_destination: Mock,
):
"""When ca_certificate is starred in the payload, it must be resolved from
curr_credentials — not dropped."""
org = Org.objects.create(name="org-ssl-cert", slug="org-ssl-cert")
warehouse = OrgWarehouse.objects.create(org=org, wtype="postgres", name="wh")

mock_update_destination.return_value = {"destinationId": "DEST_ID"}
mock_retrieve_warehouse_credentials.return_value = {
"host": "db-host",
"password": "real-password",
"ssl_mode": {"mode": "verify-ca", "ca_certificate": "real-cert"},
}
mock_update_warehouse_credentials.return_value = None
mock_create_or_update_dbt_profile_secret_blk.return_value = Mock()

payload = AirbyteDestinationUpdate(
name="wh",
destinationDefId="def-id",
config={
"host": "db-host",
"password": "*****",
"ssl_mode": {"mode": "verify-ca", "ca_certificate": "*****"},
},
)
update_destination(org, "dest-id", payload)

saved_creds = mock_update_warehouse_credentials.call_args[0][1]
assert saved_creds["ssl_mode"]["ca_certificate"] == "real-cert"
assert saved_creds["password"] == "real-password"
45 changes: 45 additions & 0 deletions ddpui/tests/utils/test_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
cleaned_name_for_prefectblock,
map_airbyte_keys_to_postgres_keys,
update_dict_but_not_stars,
resolve_stars,
nice_bytes,
find_key_in_dictionary,
get_integer_env_var,
Expand Down Expand Up @@ -188,6 +189,50 @@ def test_update_dict_but_not_stars():
}


def test_resolve_stars_replaces_top_level_stars():
curr = {"password": "real-password", "host": "real-host"}
payload = {"host": "new-host", "password": "*****"}
result = resolve_stars(payload, curr)
assert result == {"host": "new-host", "password": "real-password"}


def test_resolve_stars_replaces_nested_stars():
curr = {"ssl_mode": {"mode": "verify-ca", "ca_certificate": "real-cert"}}
payload = {"ssl_mode": {"mode": "verify-ca", "ca_certificate": "*****"}}
result = resolve_stars(payload, curr)
assert result == {"ssl_mode": {"mode": "verify-ca", "ca_certificate": "real-cert"}}


def test_resolve_stars_does_not_carry_over_missing_keys():
"""Keys in curr_credentials absent from payload must not appear in result."""
curr = {
"password": "real-password",
"ssl_mode": {"mode": "verify-ca", "ca_certificate": "real-cert"},
"sslrootcert": "/home/ddp/global-bundle.pem",
}
payload = {"host": "new-host", "password": "*****"}
result = resolve_stars(payload, curr)
assert result == {"host": "new-host", "password": "real-password"}
assert "ssl_mode" not in result
assert "sslrootcert" not in result


def test_resolve_stars_handles_boolean_false():
curr = {"password": "real-password"}
payload = {"host": "new-host", "ssl": False, "password": "*****"}
result = resolve_stars(payload, curr)
assert result == {"host": "new-host", "ssl": False, "password": "real-password"}


def test_resolve_stars_drops_key_when_star_unresolvable():
"""Starred values with no match in curr_credentials are dropped."""
curr = {}
payload = {"host": "new-host", "password": "*****"}
result = resolve_stars(payload, curr)
assert result["host"] == "new-host"
assert "password" not in result


def test_nice_bytes():
"""tests nice_bytes"""
assert nice_bytes(1024) == "1.0 KB"
Expand Down
22 changes: 22 additions & 0 deletions ddpui/utils/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,28 @@ def update_dict_but_not_stars(input_config: dict):
return output_config


def resolve_stars(payload_config: dict, curr_credentials: dict) -> dict:
"""
Returns a copy of payload_config with starred string values replaced by the
corresponding value from curr_credentials. payload_config is the authoritative
source — keys absent from it are NOT carried over from curr_credentials.
Starred values that cannot be resolved (key absent from curr_credentials) are
dropped, matching the behaviour of update_dict_but_not_stars.
Handles nested dicts recursively.
"""
result = {}
for key, val in payload_config.items():
if isinstance(val, str) and re.match(r"^\*+$", val.strip()):
if key in curr_credentials:
result[key] = curr_credentials[key]
# else: unresolvable star — drop the key
elif isinstance(val, dict):
result[key] = resolve_stars(val, curr_credentials.get(key, {}))
else:
result[key] = val
return result


def hash_dict(payload: dict) -> str:
"""hash a dictionary"""
hasher = hashlib.sha256()
Expand Down
Loading