Few more deployment tightens

This commit is contained in:
Eric Hare
2026-05-21 12:58:16 -07:00
parent 7a362b1d17
commit ca0f4e3402
3 changed files with 75 additions and 23 deletions

View File

@ -1253,9 +1253,30 @@ async def update_snapshot(
detail=f"No attachment found for provider_snapshot_id '{snapshot_id}'.",
)
# The downstream provider call is scoped to ONE provider account (the
# one we resolve ``deployment_adapter`` against), but
# ``update_flow_version_by_provider_snapshot_id`` rewrites every owner
# row matching ``(owner_id, snapshot_id)`` regardless of which provider
# account the deployment belongs to. If the same provider_snapshot_id
# exists across multiple provider accounts in the owner's namespace,
# mutating one provider account while rewriting attachments tied to a
# different account would corrupt their pointer state. Refuse the
# ambiguous case so the caller has to disambiguate (typically by
# cleaning up snapshot id collisions).
provider_account_ids = {d.deployment_provider_account_id for d in resolved_deployments}
if len(provider_account_ids) > 1:
raise HTTPException(
status_code=status.HTTP_409_CONFLICT,
detail=(
f"Provider snapshot '{snapshot_id}' is attached to deployments across "
f"{len(provider_account_ids)} provider accounts. The PATCH route cannot "
"safely update them in one operation."
),
)
# Pick any authorized attachment + deployment for the downstream provider
# call — they all share the same owner, snapshot, and provider account by
# construction. The mutation operates on the full set via
# the check above. The mutation operates on the full set via
# ``update_flow_version_by_provider_snapshot_id``.
deployment = resolved_deployments[0]
attachment = next(c for c in all_candidates if c.deployment_id == deployment.id)

View File

@ -60,9 +60,14 @@ def _fake_provider_account(
provider_url: str = "https://api.us-south.wxo.cloud.ibm.com/instances/tenant-1",
api_key: str = "encrypted-key",
provider_tenant_id: str | None = "tenant-1",
user_id=None,
) -> SimpleNamespace:
return SimpleNamespace(
id=uuid4(),
# ``user_id`` is read by ``list_deployment_configs`` to pick the
# provider namespace for adapter calls; default to a random UUID
# so tests that don't care about the owner still pass.
user_id=user_id if user_id is not None else uuid4(),
provider_key=provider_key,
provider_url=provider_url,
api_key=api_key,
@ -1197,10 +1202,10 @@ class TestUpdateSnapshotRoute:
@patch(f"{ROUTES_MODULE}.resolve_deployment_adapter")
@patch(f"{ROUTES_MODULE}.get_deployment_mapper")
@patch(f"{ROUTES_MODULE}.get_owned_provider_account_or_404", new_callable=AsyncMock)
@patch(f"{ROUTES_MODULE}.get_attachment_by_provider_snapshot_id", new_callable=AsyncMock)
@patch(f"{ROUTES_MODULE}.list_attachments_by_provider_snapshot_id", new_callable=AsyncMock)
async def test_updates_all_attachment_rows_for_snapshot(
self,
mock_get_attachment,
mock_list_attachments,
mock_get_pa,
mock_get_mapper,
mock_resolve_adapter,
@ -1213,19 +1218,23 @@ class TestUpdateSnapshotRoute:
user = _fake_user()
flow_id = uuid4()
target_flow_version_id = uuid4()
# ``update_snapshot`` filters the candidate set to attachments owned
# by the actor when share-aware fetch is off. Pin the owner to the
# actor's id so the OSS pass-through path keeps the row.
deployment = _fake_deployment_row(
user_id=user.id,
deployment_provider_account_id=uuid4(),
)
attachment = SimpleNamespace(
flow_version_id=uuid4(),
deployment_id=uuid4(),
deployment_id=deployment.id,
provider_snapshot_id="tool-1",
)
deployment = _fake_deployment_row(
id=attachment.deployment_id,
deployment_provider_account_id=uuid4(),
user_id=user.id,
)
provider_account = _fake_provider_account()
provider_account.id = deployment.deployment_provider_account_id
mock_get_attachment.return_value = attachment
mock_list_attachments.return_value = [attachment]
mock_get_deployment_row.return_value = deployment
mock_get_flow_version.return_value = SimpleNamespace(id=target_flow_version_id, flow_id=flow_id, data={})
mock_get_pa.return_value = provider_account
@ -1249,9 +1258,8 @@ class TestUpdateSnapshotRoute:
assert response.flow_version_id == target_flow_version_id
assert response.provider_snapshot_id == "tool-1"
mock_get_attachment.assert_awaited_once_with(
mock_list_attachments.assert_awaited_once_with(
session,
user_id=user.id,
provider_snapshot_id="tool-1",
)
mock_update_rows.assert_awaited_once_with(
@ -1270,10 +1278,10 @@ class TestUpdateSnapshotRoute:
@patch(f"{ROUTES_MODULE}.resolve_deployment_adapter")
@patch(f"{ROUTES_MODULE}.get_deployment_mapper")
@patch(f"{ROUTES_MODULE}.get_owned_provider_account_or_404", new_callable=AsyncMock)
@patch(f"{ROUTES_MODULE}.get_attachment_by_provider_snapshot_id", new_callable=AsyncMock)
@patch(f"{ROUTES_MODULE}.list_attachments_by_provider_snapshot_id", new_callable=AsyncMock)
async def test_commit_failure_attempts_provider_compensation(
self,
mock_get_attachment,
mock_list_attachments,
mock_get_pa,
mock_get_mapper,
mock_resolve_adapter,
@ -1287,14 +1295,15 @@ class TestUpdateSnapshotRoute:
flow_id = uuid4()
previous_flow_version_id = uuid4()
target_flow_version_id = uuid4()
deployment = _fake_deployment_row(
user_id=user.id,
deployment_provider_account_id=uuid4(),
)
attachment = SimpleNamespace(
flow_version_id=previous_flow_version_id,
deployment_id=uuid4(),
deployment_id=deployment.id,
provider_snapshot_id="tool-1",
)
deployment = _fake_deployment_row(
id=attachment.deployment_id,
deployment_provider_account_id=uuid4(),
user_id=user.id,
)
provider_account = _fake_provider_account()
provider_account.id = deployment.deployment_provider_account_id
@ -1302,7 +1311,7 @@ class TestUpdateSnapshotRoute:
target_version = SimpleNamespace(id=target_flow_version_id, flow_id=flow_id, data={"nodes": []})
previous_version = SimpleNamespace(id=previous_flow_version_id, flow_id=flow_id, data={"nodes": []})
mock_get_flow_version.side_effect = [target_version, previous_version]
mock_get_attachment.return_value = attachment
mock_list_attachments.return_value = [attachment]
mock_get_deployment_row.return_value = deployment
mock_get_pa.return_value = provider_account
adapter = AsyncMock()
@ -2249,7 +2258,10 @@ class TestGetDeploymentSync:
await get_deployment(deployment_id=dep_row.id, session=session, current_user=user)
assert exc_info.value.status_code == 404
mock_delete_row.assert_awaited_once_with(session, user_id=user.id, deployment_id=dep_row.id)
# Stale-row delete now runs in the deployment owner's namespace so
# a non-owner with a share grant can still prune the owner's stale
# row. ``current_user`` is only the actor for audit.
mock_delete_row.assert_awaited_once_with(session, user_id=dep_row.user_id, deployment_id=dep_row.id)
session.commit.assert_awaited_once()
@pytest.mark.asyncio

View File

@ -109,7 +109,9 @@ def mock_db_crud(mock_mapper):
_mock_del_dep = stack.enter_context(
patch("langflow.api.v1.deployments._delete_local_deployment_row_with_commit_retry")
)
mock_get_att = stack.enter_context(patch("langflow.api.v1.deployments.get_attachment_by_provider_snapshot_id"))
mock_list_att_by_snapshot = stack.enter_context(
patch("langflow.api.v1.deployments.list_attachments_by_provider_snapshot_id")
)
mock_get_dep_row = stack.enter_context(
patch("langflow.services.database.models.deployment.crud.get_deployment")
)
@ -149,8 +151,25 @@ def mock_db_crud(mock_mapper):
"watsonx-orchestrate",
"tenant-test",
)
mock_get_att.return_value = AsyncMock(deployment_id=uuid4(), flow_version_id=uuid4())
mock_get_dep_row.return_value = AsyncMock(deployment_provider_account_id=uuid4())
# ``update_snapshot`` now enumerates every attachment with the
# given provider_snapshot_id and authorizes each one's deployment.
# The fake deployment row carries a stable user_id so the candidate
# filter keeps it under the OSS pass-through path.
snapshot_deployment_id = uuid4()
snapshot_owner_id = uuid4()
mock_list_att_by_snapshot.return_value = [
AsyncMock(
deployment_id=snapshot_deployment_id,
flow_version_id=uuid4(),
user_id=snapshot_owner_id,
provider_snapshot_id="tool-1",
)
]
mock_get_dep_row.return_value = AsyncMock(
id=snapshot_deployment_id,
user_id=snapshot_owner_id,
deployment_provider_account_id=uuid4(),
)
mock_get_fv.return_value = AsyncMock(flow_id=uuid4(), data={})
mock_upd_fv.return_value = 1
mock_count_deps.return_value = 0