Scaffolding: Federation gateway setup#299
Conversation
klpoland
added
feature
klpoland
changed the title
|
Added some dedicated API endpoints, serializers, and signals for federation and configuration that defaults to disabling federation sync. |
klpoland
force-pushed
the
feature-kpoland-federation-gateway-scaffolding
branch
2 times, most recently
from
9f94dfd to
83d51d0
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
klpoland
force-pushed
the
feature-kpoland-federation-gateway-scaffolding
branch
from
83d51d0 to
4bbcbed
Compare
lucaspar
added
the
migrations
…gic and local indexing before notify
|
Refactor:
Hardening:
Logic Updates:
|
…(TODO: implement on federation app side), federation init/checks refactor, federation queryset reusability
…ween gateway and federation
|
|
||
|
|
||
| def capture_in_published_dataset(capture: Capture) -> bool: | ||
| return capture.datasets.federation_exportable().exists() |
There was a problem hiding this comment.
FK-linked captures skip federation
Medium Severity
Federation eligibility for captures uses only the datasets M2M (capture.datasets.federation_exportable() and datasets__in on list queries), while the gateway still supports legacy Capture.dataset FK via get_capture_datasets. Captures tied to a published dataset only through the FK are omitted from indexing, signals, and export lists.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit ee4b2ca. Configure here.
There was a problem hiding this comment.
Need to check if this is still relevant, but I can use the bridge function.
…, make sure serializers have fields to pre-filter fed searches on gateway
| ) | ||
| if body is None: | ||
| raise NotFound() | ||
| return Response(body) |
There was a problem hiding this comment.
Bootstrap list detail mismatch
Medium Severity
Federation export list routes build each row from live Postgres via compile_federated_*_doc, while detail routes read only fed-* OpenSearch documents. Existing public datasets and captures are indexed only on post_save when federation is operational, so enabling federation or recovering without a backfill leaves list responses populated while per-UUID detail calls return 404.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 08158b5. Configure here.
| item_type=item_type, | ||
| uuid=instance.uuid, | ||
| timestamp=timestamp, | ||
| ) |
There was a problem hiding this comment.
Redis notify skipped after index
Medium Severity
reindex_federated_asset writes to OpenSearch first, then calls publish_federation_event. If Redis publish raises, events.py logs a warning and swallows the error, so the local fed-* document can be current while the sync service never receives a notification and remote peers stay stale until another save.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 08158b5. Configure here.
| ) | ||
| user = get_object_or_404(User, email=email, is_approved=True) | ||
| raw_key = self.mint_user_api_key(user) | ||
| return Response({"api_key": raw_key, "email": user.email}) |
There was a problem hiding this comment.
Federation mint allows arbitrary users
Medium Severity
The federation sync mint endpoint reuses the SVI pattern: any approved email query parameter is passed to mint_federation_sync_api_key, which always creates FederationSync-scoped API keys. A holder of the sync DRF token can mint export keys for arbitrary users instead of only the federation sync service account.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 08158b5. Configure here.
| source = response.get("_source") | ||
| if not isinstance(source, dict): | ||
| return None | ||
| return source |
There was a problem hiding this comment.
OpenSearch errors skip tombstone reindex
Medium Severity
The get_federated_export_doc_by_uuid function returns None for all OpenSearch exceptions, not just NotFoundError. This causes downstream logic, like fed_doc_exists and reindex functions, to incorrectly assume a document is missing. As a result, critical cleanup, such as tombstone reindexing for soft-deleted assets, is skipped, leaving stale documents in fed-* indices.
Reviewed by Cursor Bugbot for commit 4609c03. Configure here.
| client = get_redis_client() | ||
| client.publish(channel, json.dumps(payload)) | ||
| except Exception as err: # noqa: BLE001 | ||
| log.warning("Failed to publish federation event: {}", err) |
There was a problem hiding this comment.
Redis publish errors swallowed
Medium Severity
After a successful fed index, publish_federation_event catches all Redis errors, logs a warning, and returns. The local index can be up to date while the sync service never receives the change notification.
Reviewed by Cursor Bugbot for commit 4609c03. Configure here.
| exc, | ||
| ) | ||
| return None | ||
| source = response.get("_source") |
There was a problem hiding this comment.
need to use f-strings with loguru
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.
There are 8 total unresolved issues (including 6 from previous reviews).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 94ef4d5. Configure here.
| _last_evaluated_at = now | ||
| settings.FEDERATION_OPERATIONAL = operational | ||
| settings.FEDERATION_OPERATIONAL_REASON = reason | ||
| return operational, reason |
There was a problem hiding this comment.
Operational state cached sixty seconds
High Severity
The refresh_federation_operational_state function caches federation status for 60 seconds. This prevents immediate recognition of federation becoming operational (e.g., after the FederationSync API key is created or health checks pass). As a result, post_save handlers and export endpoints may incorrectly treat federation as down for up to a minute, leading to skipped indexing, missed Redis events, and 503 errors.
Reviewed by Cursor Bugbot for commit 94ef4d5. Configure here.
| return | ||
| for capture in fresh.captures.filter(is_deleted=False): | ||
| if capture_needs_federation_reindex(capture): | ||
| reindex_federated_asset(capture, ItemType.CAPTURE) |
There was a problem hiding this comment.
FK-linked captures skipped federation
Medium Severity
The federation logic for captures currently only considers the Capture.datasets M2M relationship. During the expand-contract migration, captures linked solely by the deprecated Capture.dataset FK are missed. This prevents them from being federated, indexed, or reindexed, impacting functions like public_captures_queryset and capture_in_published_dataset.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 94ef4d5. Configure here.
2 participants
Note
High Risk
Touches authentication/API key scoping, new internal data-export endpoints with network ACLs, and changes dataset publish/detach behavior that existing clients may rely on.
Overview
Introduces federation on the gateway behind
FEDERATION_ENABLED, with repo-rootfederation-shared.env(generated bygenerate-secrets.sh) loaded in local compose alongside new Django federation settings (site name, Redis channel, sync health probes, export CIDR allowlist).When operational, dataset/capture saves trigger post-commit reindex into
fed-datasets/fed-capturesand Redis pub/sub notifications for the sync service. A new/api/.../federation/export/...surface is restricted to FederationSyncApi-Keyclients on allowed internal IPs; those keys are blocked elsewhere via a global default permission. Startup/prepare_gatewaycan sync the federation-sync DRF token and mint export keys throughget-federation-sync-api-key(SVI minting refactored similarly).Published datasets can no longer revert from final or public; detach-from-datasets only clears draft/private links. Federation export serializers and contract tests align with the sibling
sds_federationPydantic models.Reviewed by Cursor Bugbot for commit 94ef4d5. Bugbot is set up for automated code reviews on this repo. Configure here.