-
Notifications
You must be signed in to change notification settings - Fork 549
feat: ClickHouse bring-your-own-warehouse connections #8020
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 13 commits
5c91188
770a97e
73640e9
61a2a3a
9a05fe6
6695381
98c0c76
81cf471
f91b461
b8c97cb
808513a
50891b5
171c14c
f7a302b
3589d6c
77c343a
87861f0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,45 @@ | ||
| import base64 | ||
| import hashlib | ||
| import json | ||
| from typing import Any | ||
|
|
||
| import structlog | ||
| from cryptography.fernet import Fernet, InvalidToken | ||
| from django.conf import settings | ||
| from django.db import models | ||
|
|
||
| logger = structlog.get_logger("core") | ||
|
|
||
|
|
||
| def _get_fernet() -> Fernet: | ||
| digest = hashlib.sha256(settings.SECRET_KEY.encode()).digest() | ||
| return Fernet(base64.urlsafe_b64encode(digest)) | ||
|
|
||
|
|
||
| class EncryptedJSONField(models.TextField[Any, Any]): | ||
| def get_prep_value(self, value: Any) -> str | None: | ||
| if value is None: | ||
| return None | ||
| return _get_fernet().encrypt(json.dumps(value).encode()).decode() | ||
|
|
||
| def from_db_value( | ||
| self, | ||
| value: str | None, | ||
| expression: object, | ||
| connection: object, | ||
| ) -> Any: | ||
| if value is None: | ||
| return None | ||
| try: | ||
| plaintext = _get_fernet().decrypt(value.encode()) | ||
| except InvalidToken: | ||
| logger.warning("encrypted_field.decrypt_failed", exc_info=True) | ||
| return None | ||
| return json.loads(plaintext) | ||
|
|
||
| def get_lookup(self, lookup_name: str) -> Any: | ||
| if lookup_name != "isnull": | ||
| raise NotImplementedError( | ||
| "EncryptedJSONField only supports isnull lookups." | ||
| ) | ||
| return super().get_lookup(lookup_name) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| import prometheus_client | ||
|
|
||
| flagsmith_experimentation_warehouse_connection_verifications_total = ( | ||
| prometheus_client.Counter( | ||
| "flagsmith_experimentation_warehouse_connection_verifications_total", | ||
| "Outcomes of connection verification attempts against customers' own " | ||
| "data warehouses. `result` label is either `success` or `failure`.", | ||
| ["result"], | ||
| ) | ||
| ) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| from django.db import migrations | ||
|
|
||
| import core.fields | ||
|
|
||
|
|
||
| class Migration(migrations.Migration): | ||
| dependencies = [ | ||
| ("experimentation", "0009_add_rollout_segment"), | ||
| ] | ||
|
|
||
| operations = [ | ||
| migrations.AddField( | ||
| model_name="warehouseconnection", | ||
| name="credentials", | ||
| field=core.fields.EncryptedJSONField(blank=True, null=True), | ||
| ), | ||
| ] |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| # Generated by Django 5.2.16 on 2026-07-16 09:01 | ||
|
|
||
| from django.db import migrations, models | ||
|
|
||
|
|
||
| class Migration(migrations.Migration): | ||
| dependencies = [ | ||
| ("experimentation", "0010_warehouse_connection_credentials"), | ||
| ] | ||
|
|
||
| operations = [ | ||
| migrations.AddField( | ||
| model_name="warehouseconnection", | ||
| name="status_detail", | ||
| field=models.CharField(blank=True, max_length=255, null=True), | ||
| ), | ||
| ] |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,6 +6,7 @@ | |
|
|
||
| import structlog | ||
| from clickhouse_driver import Client | ||
| from clickhouse_driver import errors as clickhouse_errors | ||
| from clickhouse_driver.util.helpers import parse_url | ||
| from django.conf import settings | ||
| from django.db import transaction | ||
|
|
@@ -40,6 +41,9 @@ | |
| RolloutSpec, | ||
| WarehouseEventStats, | ||
| ) | ||
| from experimentation.metrics import ( | ||
| flagsmith_experimentation_warehouse_connection_verifications_total, | ||
| ) | ||
| from experimentation.models import ( | ||
| VALID_STATUS_TRANSITIONS, | ||
| ExperimentStatus, | ||
|
|
@@ -55,6 +59,7 @@ | |
| compare_to_control, | ||
| srm_p_value, | ||
| ) | ||
| from experimentation.types import ClickHouseConfig, ClickHouseCredentials | ||
| from features.models import FeatureState | ||
| from features.value_types import BOOLEAN, INTEGER, STRING | ||
| from features.versioning.dataclasses import FlagChangeSet | ||
|
|
@@ -727,6 +732,63 @@ def mark_warehouse_pending_connection( | |
| return connection | ||
|
|
||
|
|
||
| def _describe_verification_error(error: Exception) -> str: | ||
| if isinstance(error, clickhouse_errors.ServerException): | ||
| if error.code == 516: | ||
| return "Authentication failed." | ||
| if error.code == 81: | ||
| return "Database does not exist." | ||
| return "The ClickHouse server rejected the request." | ||
| if isinstance(error, (clickhouse_errors.SocketTimeoutError, TimeoutError)): | ||
| return "The connection timed out." | ||
| if isinstance(error, clickhouse_errors.NetworkError): | ||
| return "Could not connect to the host." | ||
| if isinstance(error, KeyError): | ||
| return "Stored connection details are incomplete." | ||
| return "Verification failed." | ||
|
|
||
|
|
||
| def verify_clickhouse_connection(connection: WarehouseConnection) -> None: | ||
| """Run SELECT 1 against the customer's ClickHouse and set the status to | ||
| connected or errored; never raises.""" | ||
| log = logger.bind(environment__id=connection.environment_id) | ||
| try: | ||
| log = log.bind(organisation__id=connection.environment.project.organisation_id) | ||
| config = typing.cast(ClickHouseConfig, connection.config or {}) | ||
| credentials = typing.cast(ClickHouseCredentials, connection.credentials or {}) | ||
| client = Client( | ||
| config["host"], | ||
|
Comment on lines
+827
to
+832
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift Close the DNS-rebinding gap before opening the socket.
|
||
| port=config["port"], | ||
| user=config["username"], | ||
| password=credentials["password"], | ||
| database=config["database"], | ||
| secure=config["secure"], | ||
| connect_timeout=CLICKHOUSE_CONNECT_TIMEOUT_SECONDS, | ||
| send_receive_timeout=CLICKHOUSE_QUERY_TIMEOUT_SECONDS, | ||
| ) | ||
| try: | ||
| client.execute("SELECT 1") | ||
| finally: | ||
| client.disconnect() | ||
| except Exception as error: | ||
| connection.status = WarehouseConnectionStatus.ERRORED | ||
| connection.status_detail = _describe_verification_error(error) | ||
| connection.save(update_fields=["status", "status_detail"]) | ||
| flagsmith_experimentation_warehouse_connection_verifications_total.labels( | ||
| result="failure" | ||
| ).inc() | ||
| log.warning("connection.verification_failed", exc_info=True) | ||
| return | ||
|
|
||
| connection.status = WarehouseConnectionStatus.CONNECTED | ||
| connection.status_detail = None | ||
| connection.save(update_fields=["status", "status_detail"]) | ||
| flagsmith_experimentation_warehouse_connection_verifications_total.labels( | ||
| result="success" | ||
| ).inc() | ||
| log.info("connection.verification_succeeded") | ||
|
Comment on lines
+819
to
+861
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
# Inspect the verifier and its call sites.
printf '\n== services.py around verify_clickhouse_connection ==\n'
sed -n '780,900p' api/experimentation/services.py
printf '\n== views.py references ==\n'
rg -n "verify_clickhouse_connection|write_environment_ingestion_keys\.delay|AFTER_CREATE|perform_create|perform_update|test connection|connection.verification" api/experimentation -n
printf '\n== views.py relevant slices ==\n'
for f in api/experimentation/views.py api/experimentation/*.py; do
if [ -f "$f" ] && rg -n "verify_clickhouse_connection|write_environment_ingestion_keys\.delay|perform_create|perform_update|AFTER_CREATE" "$f" >/dev/null; then
echo "--- $f ---"
sed -n '1,260p' "$f" | rg -n -A40 -B20 "verify_clickhouse_connection|write_environment_ingestion_keys\.delay|perform_create|perform_update|AFTER_CREATE"
fi
doneRepository: Flagsmith/flagsmith Length of output: 18677 Move ClickHouse verification off the request thread Up to ~35s of network I/O still runs inline in |
||
|
|
||
|
|
||
| def refresh_warehouse_connection_status( | ||
| connection: WarehouseConnection, | ||
| stats: WarehouseEventStats, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,6 +15,7 @@ | |
| from rest_framework.request import Request | ||
| from rest_framework.response import Response | ||
| from rest_framework.serializers import BaseSerializer | ||
| from rest_framework.throttling import BaseThrottle, ScopedRateThrottle | ||
| from rest_framework.viewsets import GenericViewSet | ||
|
|
||
| from app.pagination import CustomPagination | ||
|
|
@@ -62,6 +63,7 @@ | |
| mark_warehouse_pending_connection, | ||
| refresh_warehouse_connection_status, | ||
| transition_experiment_status, | ||
| verify_clickhouse_connection, | ||
| ) | ||
| from experimentation.tasks import ( | ||
| compute_experiment_exposures, | ||
|
|
@@ -89,19 +91,32 @@ class WarehouseConnectionViewSet( | |
| lookup_field = "id" | ||
| lookup_url_kwarg = "connection_id" | ||
|
|
||
| def get_throttles(self) -> list[BaseThrottle]: | ||
| if self.action == "test_warehouse_connection": | ||
| self.throttle_scope = "warehouse_connection_test" | ||
| return [ScopedRateThrottle()] | ||
| return super().get_throttles() | ||
|
Comment on lines
+94
to
+103
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '\n== views.py ==\n'
sed -n '1,180p' api/experimentation/views.py
printf '\n== test_views.py around throttle test ==\n'
sed -n '1020,1100p' api/tests/unit/experimentation/test_views.py
printf '\n== search for get_throttles definitions/usages ==\n'
rg -n "def get_throttles|throttle_scope|ScopedRateThrottle|throttle_classes" api/experimentation api/tests/unit/experimentation -SRepository: Flagsmith/flagsmith Length of output: 10305 🏁 Script executed: #!/bin/bash
set -euo pipefail
sed -n '1,180p' api/experimentation/views.py
sed -n '1020,1100p' api/tests/unit/experimentation/test_views.py
rg -n "def get_throttles|throttle_scope|ScopedRateThrottle|throttle_classes" api/experimentation api/tests/unit/experimentation -SRepository: Flagsmith/flagsmith Length of output: 10196 🏁 Script executed: #!/bin/bash
set -euo pipefail
rg -n "DEFAULT_THROTTLE_CLASSES|throttle_classes\s*=" -S .Repository: Flagsmith/flagsmith Length of output: 1775 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '\n== NestedEnvironmentViewSet definition ==\n'
rg -n "class NestedEnvironmentViewSet|throttle_classes|get_throttles" api environments core -S
printf '\n== likely file for NestedEnvironmentViewSet ==\n'
fd -a "views.py" api environments core | rg "environments/views.py|NestedEnvironmentViewSet|views.py$"Repository: Flagsmith/flagsmith Length of output: 2911 🏁 Script executed: #!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
for p in Path('.').rglob('*.py'):
try:
text = p.read_text()
except Exception:
continue
if 'class NestedEnvironmentViewSet' in text:
print(p)
PYRepository: Flagsmith/flagsmith Length of output: 183 🏁 Script executed: #!/bin/bash
set -euo pipefail
sed -n '300,380p' api/environments/views.pyRepository: Flagsmith/flagsmith Length of output: 3267 Preserve the existing throttle stack for write actions. In 📍 Affects 2 files
|
||
|
|
||
| def perform_create(self, serializer: BaseSerializer[WarehouseConnection]) -> None: | ||
| connection: WarehouseConnection = serializer.save( | ||
| environment=self._get_environment() | ||
| ) | ||
| create_warehouse_audit_log( | ||
| connection, self._get_user(self.request), action="created" | ||
| ) | ||
| if connection.warehouse_type == WarehouseType.CLICKHOUSE: | ||
| verify_clickhouse_connection(connection) | ||
|
|
||
| def perform_update(self, serializer: BaseSerializer[WarehouseConnection]) -> None: | ||
| connection: WarehouseConnection = serializer.save() | ||
| create_warehouse_audit_log( | ||
| connection, self._get_user(self.request), action="updated" | ||
| ) | ||
| if connection.warehouse_type == WarehouseType.CLICKHOUSE and ( | ||
| "config" in serializer.validated_data | ||
| or "credentials" in serializer.validated_data | ||
| ): | ||
| verify_clickhouse_connection(connection) | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
|
|
||
| def perform_destroy(self, instance: WarehouseConnection) -> None: | ||
| create_warehouse_audit_log( | ||
|
|
@@ -130,6 +145,9 @@ def retrieve(self, request: Request, *args: object, **kwargs: object) -> Respons | |
| @action(detail=True, methods=["post"], url_path="test-warehouse-connection") | ||
| def test_warehouse_connection(self, request: Request, **kwargs: object) -> Response: | ||
| connection: WarehouseConnection = self.get_object() | ||
| if connection.warehouse_type == WarehouseType.CLICKHOUSE: | ||
| verify_clickhouse_connection(connection) | ||
| return Response(self.get_serializer(connection).data) | ||
| if connection.warehouse_type != WarehouseType.FLAGSMITH: | ||
| return Response( | ||
| {"detail": "Test events are only supported for Flagsmith warehouses."}, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
Repository: Flagsmith/flagsmith
Length of output: 7390
🏁 Script executed:
Repository: Flagsmith/flagsmith
Length of output: 1754
Use a dedicated key for encrypted fields. Deriving the Fernet key from
SECRET_KEYcouples credential encryption to Django’s global signing secret, and rotatingSECRET_KEYwill make stored values undecryptable whilefrom_db_valuereturnsNone, which downstream surfaces as “Stored connection details are incomplete.” Add a separate encryption setting and a rotation path instead of reusingSECRET_KEY.