diff --git a/.gitignore b/.gitignore index 2d85cf190..7ec9fefdc 100644 --- a/.gitignore +++ b/.gitignore @@ -160,3 +160,6 @@ ari/kb/rules/ # Generated Grafana files grafana/* + +# Local LaunchDarkly data +./flagdata.json diff --git a/ansible_wisdom/ai/api/telemetry/api_telemetry_settings_views.py b/ansible_wisdom/ai/api/telemetry/api_telemetry_settings_views.py index ae4571ceb..9da4b5f00 100644 --- a/ansible_wisdom/ai/api/telemetry/api_telemetry_settings_views.py +++ b/ansible_wisdom/ai/api/telemetry/api_telemetry_settings_views.py @@ -8,7 +8,6 @@ from ai.api.serializers import TelemetrySettingsRequestSerializer from ai.api.utils.segment import send_segment_event from ai.api.views import InternalServerError, ServiceUnavailable -from django.conf import settings from drf_spectacular.utils import OpenApiResponse, extend_schema from oauth2_provider.contrib.rest_framework import IsAuthenticatedOrTokenHasScope from rest_framework.exceptions import ValidationError @@ -48,9 +47,6 @@ class TelemetrySettingsView(RetrieveAPIView, CreateAPIView): def get(self, request, *args, **kwargs): logger.debug("Telemetry settings:: GET handler") - if not settings.ADMIN_PORTAL_TELEMETRY_OPT_ENABLED: - raise ServiceUnavailable() - exception = None organization = None start_time = time.time() @@ -61,8 +57,14 @@ def get(self, request, *args, **kwargs): if not organization: return Response(status=HTTP_400_BAD_REQUEST) + if not organization.is_schema_2_telemetry_enabled: + raise ServiceUnavailable() + return Response(status=HTTP_200_OK, data={'optOut': organization.telemetry_opt_out}) + except ServiceUnavailable: + raise + except Exception as e: exception = e logger.exception(e) @@ -94,9 +96,6 @@ def get(self, request, *args, **kwargs): def post(self, request, *args, **kwargs): logger.debug("Telemetry settings:: POST handler") - if not settings.ADMIN_PORTAL_TELEMETRY_OPT_ENABLED: - raise ServiceUnavailable() - exception = None organization = None start_time = time.time() @@ -107,6 +106,9 @@ def post(self, request, *args, **kwargs): if not organization: return Response(status=HTTP_400_BAD_REQUEST) + if not organization.is_schema_2_telemetry_enabled: + raise ServiceUnavailable() + # Extract Telemetry settings from request telemetry_settings_serializer = TelemetrySettingsRequestSerializer(data=request.data) telemetry_settings_serializer.is_valid(raise_exception=True) @@ -130,6 +132,9 @@ def post(self, request, *args, **kwargs): logger.info(e, exc_info=True) return Response(status=HTTP_400_BAD_REQUEST) + except ServiceUnavailable: + raise + except Exception as e: exception = e logger.exception(e) diff --git a/ansible_wisdom/ai/api/telemetry/tests/test_api_telemetry_settings_views.py b/ansible_wisdom/ai/api/telemetry/tests/test_api_telemetry_settings_views.py index 87534122e..9a992c172 100644 --- a/ansible_wisdom/ai/api/telemetry/tests/test_api_telemetry_settings_views.py +++ b/ansible_wisdom/ai/api/telemetry/tests/test_api_telemetry_settings_views.py @@ -1,6 +1,7 @@ from http import HTTPStatus from unittest.mock import patch +import ai.feature_flags as feature_flags from ai.api.permissions import ( IsOrganisationAdministrator, IsOrganisationLightspeedSubscriber, @@ -36,8 +37,11 @@ def test_permission_classes(self, *args): for permission in required_permissions: self.assertTrue(permission in view.permission_classes) - @override_settings(ADMIN_PORTAL_TELEMETRY_OPT_ENABLED=False) - def test_get_settings_when_feature_disabled(self, *args): + @override_settings(LAUNCHDARKLY_SDK_KEY='dummy_key') + @patch.object(feature_flags, 'LDClient') + def test_get_settings_when_feature_disabled(self, LDClient, *args): + LDClient.return_value.variation.return_value = False + self.user.organization = Organization.objects.get_or_create(id=123)[0] self.client.force_authenticate(user=self.user) r = self.client.get(reverse('telemetry_settings')) self.assertEqual(r.status_code, HTTPStatus.SERVICE_UNAVAILABLE) @@ -52,7 +56,10 @@ def test_get_settings_without_org_id(self, *args): self.assert_segment_log(log, "telemetrySettingsGet", None) @override_settings(SEGMENT_WRITE_KEY='DUMMY_KEY_VALUE') - def test_get_settings_when_undefined(self, *args): + @override_settings(LAUNCHDARKLY_SDK_KEY='dummy_key') + @patch.object(feature_flags, 'LDClient') + def test_get_settings_when_undefined(self, LDClient, *args): + LDClient.return_value.variation.return_value = True self.user.organization = Organization.objects.get_or_create(id=123)[0] self.client.force_authenticate(user=self.user) @@ -63,7 +70,10 @@ def test_get_settings_when_undefined(self, *args): self.assert_segment_log(log, "telemetrySettingsGet", None, opt_out=False) @override_settings(SEGMENT_WRITE_KEY='DUMMY_KEY_VALUE') - def test_get_settings_when_defined(self, *args): + @override_settings(LAUNCHDARKLY_SDK_KEY='dummy_key') + @patch.object(feature_flags, 'LDClient') + def test_get_settings_when_defined(self, LDClient, *args): + LDClient.return_value.variation.return_value = True self.user.organization = Organization.objects.get_or_create(id=123, telemetry_opt_out=True)[ 0 ] @@ -80,8 +90,11 @@ def test_set_settings_authentication_error(self, *args): r = self.client.post(reverse('telemetry_settings')) self.assertEqual(r.status_code, HTTPStatus.UNAUTHORIZED) - @override_settings(ADMIN_PORTAL_TELEMETRY_OPT_ENABLED=False) - def test_set_settings_when_feature_disabled(self, *args): + @override_settings(LAUNCHDARKLY_SDK_KEY='dummy_key') + @patch.object(feature_flags, 'LDClient') + def test_set_settings_when_feature_disabled(self, LDClient, *args): + LDClient.return_value.variation.return_value = False + self.user.organization = Organization.objects.get_or_create(id=123)[0] self.client.force_authenticate(user=self.user) r = self.client.get(reverse('telemetry_settings')) self.assertEqual(r.status_code, HTTPStatus.SERVICE_UNAVAILABLE) @@ -96,7 +109,10 @@ def test_set_settings_without_org_id(self, *args): self.assert_segment_log(log, "telemetrySettingsSet", None) @override_settings(SEGMENT_WRITE_KEY='DUMMY_KEY_VALUE') - def test_set_settings_with_valid_value(self, *args): + @override_settings(LAUNCHDARKLY_SDK_KEY='dummy_key') + @patch.object(feature_flags, 'LDClient') + def test_set_settings_with_valid_value(self, LDClient, *args): + LDClient.return_value.variation.return_value = True self.user.organization = Organization.objects.get_or_create(id=123)[0] self.client.force_authenticate(user=self.user) @@ -129,7 +145,10 @@ def test_set_settings_with_valid_value(self, *args): self.assertTrue(r.data['optOut']) @override_settings(SEGMENT_WRITE_KEY='DUMMY_KEY_VALUE') - def test_set_settings_throws_exception(self, *args): + @override_settings(LAUNCHDARKLY_SDK_KEY='dummy_key') + @patch.object(feature_flags, 'LDClient') + def test_set_settings_throws_exception(self, LDClient, *args): + LDClient.return_value.variation.return_value = True self.user.organization = Organization.objects.get_or_create(id=123)[0] self.client.force_authenticate(user=self.user) @@ -144,7 +163,10 @@ def test_set_settings_throws_exception(self, *args): self.assert_segment_log(log, "telemetrySettingsSet", "DatabaseError", opt_out=False) @override_settings(SEGMENT_WRITE_KEY='DUMMY_KEY_VALUE') - def test_set_settings_throws_validation_exception(self, *args): + @override_settings(LAUNCHDARKLY_SDK_KEY='dummy_key') + @patch.object(feature_flags, 'LDClient') + def test_set_settings_throws_validation_exception(self, LDClient, *args): + LDClient.return_value.variation.return_value = True self.user.organization = Organization.objects.get_or_create(id=123)[0] self.client.force_authenticate(user=self.user) diff --git a/ansible_wisdom/ai/api/utils/segment_analytics_telemetry.py b/ansible_wisdom/ai/api/utils/segment_analytics_telemetry.py index e4071ea05..216b32498 100644 --- a/ansible_wisdom/ai/api/utils/segment_analytics_telemetry.py +++ b/ansible_wisdom/ai/api/utils/segment_analytics_telemetry.py @@ -3,6 +3,7 @@ from ai.api.utils.segment import base_send_segment_event, send_segment_event from attr import asdict from django.conf import settings +from organizations.models import Organization from segment.analytics import Client from users.models import User @@ -29,15 +30,18 @@ def send_segment_analytics_event(event_enum, event_payload_supplier, user: User) if not user.rh_user_has_seat: logger.info("Skipping analytics telemetry event for users that has no seat.") return - if not settings.ADMIN_PORTAL_TELEMETRY_OPT_ENABLED: - logger.info("Analytics telemetry not active.") - return - organization = user.organization + + organization: Organization = user.organization if not organization: logger.info("Analytics telemetry not active, because of no organization assigned for user.") return + + if not organization.is_schema_2_telemetry_enabled: + logger.info(f"Analytics telemetry not active for organization '{organization.id}'.") + return + if organization.telemetry_opt_out: - logger.info("Analytics telemetry not active for organization.") + logger.info(f"Organization '{organization.id}' has opted out of Analytics telemetry.") return event_name = event_enum.value diff --git a/ansible_wisdom/ai/api/utils/tests/test_segment_analytics_telemetry.py b/ansible_wisdom/ai/api/utils/tests/test_segment_analytics_telemetry.py index cd5f18622..e64ad9b46 100644 --- a/ansible_wisdom/ai/api/utils/tests/test_segment_analytics_telemetry.py +++ b/ansible_wisdom/ai/api/utils/tests/test_segment_analytics_telemetry.py @@ -1,5 +1,6 @@ from unittest.mock import Mock, patch +import ai.feature_flags as feature_flags from ai.api.tests.test_views import WisdomServiceAPITestCaseBase from ai.api.utils import segment_analytics_telemetry from ai.api.utils.analytics_telemetry_model import ( @@ -74,11 +75,13 @@ def _assert_segment_analytics_error_sent(self, error, send_segment_event): error_event_payload, "analyticsTelemetryError", self.user ) - @patch("ai.api.utils.segment_analytics_telemetry.base_send_segment_event") @override_settings(SEGMENT_ANALYTICS_WRITE_KEY="testWriteKey") - @override_settings(ADMIN_PORTAL_TELEMETRY_OPT_ENABLED=True) - def test_send_segment_analytics_event(self, base_send_segment_event): - analytics_event_object = AnalyticsProductFeedback("3", 123) + @override_settings(LAUNCHDARKLY_SDK_KEY='dummy_key') + @patch.object(feature_flags, 'LDClient') + @patch("ai.api.utils.segment_analytics_telemetry.base_send_segment_event") + def test_send_segment_analytics_event(self, base_send_segment_event, LDClient): + LDClient.return_value.variation.return_value = True + analytics_event_object = AnalyticsProductFeedback(3, 123) payload = Mock(return_value=analytics_event_object) send_segment_analytics_event(AnalyticsTelemetryEvents.PRODUCT_FEEDBACK, payload, self.user) payload.assert_called() @@ -89,10 +92,12 @@ def test_send_segment_analytics_event(self, base_send_segment_event): get_segment_analytics_client(), ) - @patch("ai.api.utils.segment_analytics_telemetry.send_segment_event") @override_settings(SEGMENT_ANALYTICS_WRITE_KEY="testWriteKey") - @override_settings(ADMIN_PORTAL_TELEMETRY_OPT_ENABLED=True) - def test_send_segment_analytics_event_error_validation(self, send_segment_event): + @override_settings(LAUNCHDARKLY_SDK_KEY='dummy_key') + @patch.object(feature_flags, 'LDClient') + @patch("ai.api.utils.segment_analytics_telemetry.send_segment_event") + def test_send_segment_analytics_event_error_validation(self, send_segment_event, LDClient): + LDClient.return_value.variation.return_value = True payload = Mock(side_effect=ValueError) send_segment_analytics_event(AnalyticsTelemetryEvents.PRODUCT_FEEDBACK, payload, self.user) payload.assert_called() @@ -107,42 +112,58 @@ def test_send_segment_analytics_event_error_validation(self, send_segment_event) error_event_payload, "analyticsTelemetryError", self.user ) + @override_settings(LAUNCHDARKLY_SDK_KEY='dummy_key') + @patch.object(feature_flags, 'LDClient') @patch("ai.api.utils.segment_analytics_telemetry.base_send_segment_event") - @override_settings(ADMIN_PORTAL_TELEMETRY_OPT_ENABLED=True) - def test_send_segment_analytics_event_error_not_write_key(self, base_send_segment_event): + def test_send_segment_analytics_event_error_not_write_key( + self, base_send_segment_event, LDClient + ): + LDClient.return_value.variation.return_value = True self._assert_event_not_sent(base_send_segment_event) - @patch("ai.api.utils.segment_analytics_telemetry.base_send_segment_event") @override_settings(SEGMENT_ANALYTICS_WRITE_KEY="testWriteKey") - @override_settings(ADMIN_PORTAL_TELEMETRY_OPT_ENABLED=True) - def test_send_segment_analytics_event_error_user_no_seat(self, base_send_segment_event): + @override_settings(LAUNCHDARKLY_SDK_KEY='dummy_key') + @patch.object(feature_flags, 'LDClient') + @patch("ai.api.utils.segment_analytics_telemetry.base_send_segment_event") + def test_send_segment_analytics_event_error_user_no_seat( + self, base_send_segment_event, LDClient + ): + LDClient.return_value.variation.return_value = True self.user.rh_user_has_seat = False self._assert_event_not_sent(base_send_segment_event) - @patch("ai.api.utils.segment_analytics_telemetry.base_send_segment_event") @override_settings(SEGMENT_ANALYTICS_WRITE_KEY="testWriteKey") - @override_settings(ADMIN_PORTAL_TELEMETRY_OPT_ENABLED=False) - def test_send_segment_analytics_event_error_no_telemetry_enabled(self, base_send_segment_event): + @override_settings(LAUNCHDARKLY_SDK_KEY='dummy_key') + @patch.object(feature_flags, 'LDClient') + @patch("ai.api.utils.segment_analytics_telemetry.base_send_segment_event") + def test_send_segment_analytics_event_error_no_telemetry_enabled( + self, base_send_segment_event, LDClient + ): + LDClient.return_value.variation.return_value = False self._assert_event_not_sent(base_send_segment_event) - @patch("ai.api.utils.segment_analytics_telemetry.base_send_segment_event") @override_settings(SEGMENT_ANALYTICS_WRITE_KEY="testWriteKey") - @override_settings(ADMIN_PORTAL_TELEMETRY_OPT_ENABLED=True) - def test_send_segment_analytics_event_error_no_org(self, base_send_segment_event): + @override_settings(LAUNCHDARKLY_SDK_KEY='dummy_key') + @patch.object(feature_flags, 'LDClient') + @patch("ai.api.utils.segment_analytics_telemetry.base_send_segment_event") + def test_send_segment_analytics_event_error_no_org(self, base_send_segment_event, LDClient): + LDClient.return_value.variation.return_value = True self.user.organization = None self._assert_event_not_sent(base_send_segment_event) - @patch("ai.api.utils.segment_analytics_telemetry.base_send_segment_event") @override_settings(SEGMENT_ANALYTICS_WRITE_KEY="testWriteKey") - @override_settings(ADMIN_PORTAL_TELEMETRY_OPT_ENABLED=True) + @override_settings(LAUNCHDARKLY_SDK_KEY='dummy_key') + @patch.object(feature_flags, 'LDClient') + @patch("ai.api.utils.segment_analytics_telemetry.base_send_segment_event") def test_send_segment_analytics_event_error_no_org_telemetry_enabled( - self, base_send_segment_event + self, base_send_segment_event, LDClient ): + LDClient.return_value.variation.return_value = True self.user.organization.telemetry_opt_out = True self._assert_event_not_sent(base_send_segment_event) def _assert_event_not_sent(self, base_send_segment_event): - payload = Mock(return_value=AnalyticsProductFeedback("3", 123)) + payload = Mock(return_value=AnalyticsProductFeedback(3, 123)) send_segment_analytics_event(AnalyticsTelemetryEvents.PRODUCT_FEEDBACK, payload, self.user) payload.assert_not_called() base_send_segment_event.assert_not_called() diff --git a/ansible_wisdom/ai/feature_flags.py b/ansible_wisdom/ai/feature_flags.py index 395b15e00..bb3c49cce 100644 --- a/ansible_wisdom/ai/feature_flags.py +++ b/ansible_wisdom/ai/feature_flags.py @@ -1,3 +1,4 @@ +import json import logging import os.path from enum import Enum @@ -13,10 +14,23 @@ class WisdomFlags(str, Enum): - MODEL_NAME = "model_name" # model name selection + # model name selection + MODEL_NAME = "model_name" + # Schema 2 Telemetry is enabled for an Organization + SCHEMA_2_TELEMETRY_ORG_ENABLED = "schema_2_telemetry_org_enabled" class FeatureFlags: + instance = None + + # Ensure FeatureFlags is a Singleton + def __new__(cls): + if cls.instance is not None: + return cls.instance + else: + inst = cls.instance = super().__new__(cls) + return inst + def __init__(self): self.client = None if settings.LAUNCHDARKLY_SDK_KEY: @@ -60,4 +74,20 @@ def get(self, name: str, user: User, default: str): logger.debug(f"retrieving feature flag {name}") return self.client.variation(name, user_context, default) else: - raise Exception("feature flag client is not initialized") + raise Exception("FeatureFlag client is not initialized") + + def check_flag(self, flag: str, query_dict: dict): + """ + Generic LaunchDarkly check + :param flag: The LaunchDarkly 'feature flag' name + :param query_dict: The LaunchDarkly Context attributes. + These must include both 'kind' and 'key'. + :return: The LaunchDarkly response or None + """ + if self.client: + logger.debug(f"Constructing context for '{json.dumps(query_dict)}'") + context = Context.from_dict(query_dict) + logger.debug(f"Retrieving feature flag '{flag}'") + return self.client.variation(flag, context, None) + else: + raise Exception("FeatureFlag client is not initialized") diff --git a/ansible_wisdom/ai/tests/test_feature_flags.py b/ansible_wisdom/ai/tests/test_feature_flags.py index 74385d51a..6c05fd0da 100644 --- a/ansible_wisdom/ai/tests/test_feature_flags.py +++ b/ansible_wisdom/ai/tests/test_feature_flags.py @@ -3,8 +3,10 @@ import ai.feature_flags as feature_flags from ai.api.tests.test_views import WisdomServiceAPITestCaseBase +from ai.feature_flags import WisdomFlags from django.conf import settings from django.test import override_settings +from ldclient import Context from ldclient.config import Config @@ -51,3 +53,35 @@ def test_feature_flags_with_local_file(self): value = ff.get('model_name', self.user, 'default_value') self.assertEqual(ff.client.get_sdk_key(), 'sdk-key-123abc') self.assertEqual(value, 'dev_model') + + @override_settings(LAUNCHDARKLY_SDK_KEY='dummy_key') + @patch.object(feature_flags, 'LDClient') + def test_feature_flags_check_flag_disabled(self, LDClient): + LDClient.return_value.variation.return_value = False + + ff = feature_flags.FeatureFlags() + self.assertFalse( + ff.check_flag( + WisdomFlags.SCHEMA_2_TELEMETRY_ORG_ENABLED, {'kind': 'organization', 'org_id': 123} + ) + ) + + @override_settings(LAUNCHDARKLY_SDK_KEY='dummy_key') + @patch.object(feature_flags, 'LDClient') + def test_feature_flags_check_flag_enabled(self, LDClient): + LDClient.return_value.variation.return_value = True + + ff = feature_flags.FeatureFlags() + self.assertTrue( + ff.check_flag( + WisdomFlags.SCHEMA_2_TELEMETRY_ORG_ENABLED, {'kind': 'organization', 'key': '123'} + ) + ) + + args = LDClient.return_value.variation.call_args_list[0] + name: str = args[0][0] + context: Context = args[0][1] + self.assertEqual(name, WisdomFlags.SCHEMA_2_TELEMETRY_ORG_ENABLED) + self.assertEqual(context.kind, 'organization') + self.assertEqual(context.key, '123') + self.assertFalse(args[0][2]) diff --git a/ansible_wisdom/healthcheck/tests/test_healthcheck.py b/ansible_wisdom/healthcheck/tests/test_healthcheck.py index 269a55e98..4d4b51e2a 100644 --- a/ansible_wisdom/healthcheck/tests/test_healthcheck.py +++ b/ansible_wisdom/healthcheck/tests/test_healthcheck.py @@ -2,17 +2,15 @@ import logging import time from http import HTTPStatus -from unittest import mock from unittest.mock import Mock, patch -import healthcheck.views as healthcheck_views +import ai.feature_flags as feature_flags from ai.api.aws.wca_secret_manager import WcaSecretManagerError from ai.api.model_client.wca_client import ( WCAClient, WcaInferenceFailure, WcaTokenFailure, ) -from ai.feature_flags import FeatureFlags from django.apps import apps from django.core.cache import cache from django.test import override_settings @@ -236,17 +234,10 @@ def test_health_check_model_mesh_mock(self): @override_settings(ANSIBLE_AI_MODEL_MESH_API_TYPE="dummy") @override_settings(LAUNCHDARKLY_SDK_KEY='dummy_key') - @mock.patch('healthcheck.views.get_feature_flags') - @mock.patch('ldclient.get') - def test_health_check_model_mesh_mock_with_launchdarkly(self, ldclient_get, get_feature_flags): - class DummyClient: - def variation(name, *args): - return 'server:port:model_name:index' - - ldclient_get.return_value = DummyClient() - get_feature_flags.return_value = FeatureFlags() - + @patch.object(feature_flags, 'LDClient') + def test_health_check_model_mesh_mock_with_launchdarkly(self, LDClient): cache.clear() + LDClient.return_value.variation.return_value = 'server:port:model_name:index' r = self.client.get(reverse('health_check')) self.assertEqual(r.status_code, HTTPStatus.OK) _, dependencies = self.assert_basic_data(r, 'ok') @@ -267,10 +258,6 @@ def test_health_check_model_mesh_mock_disabled(self): else: self.assertTrue(is_status_ok(dependency['status'])) - def test_get_feature_flags(self): - healthcheck_views.feature_flags = "return this" - self.assertEqual(healthcheck_views.get_feature_flags(), "return this") - @override_settings(LAUNCHDARKLY_SDK_KEY=None) @override_settings(ANSIBLE_AI_MODEL_MESH_API_TYPE="dummy") def test_health_check_aws_secret_manager_error(self): diff --git a/ansible_wisdom/healthcheck/views.py b/ansible_wisdom/healthcheck/views.py index 23fd7cd4c..2cf7cbb10 100644 --- a/ansible_wisdom/healthcheck/views.py +++ b/ansible_wisdom/healthcheck/views.py @@ -2,7 +2,7 @@ import logging from datetime import datetime -from ai.api.views import feature_flags +from ai.feature_flags import FeatureFlags from django.conf import settings from django.http import HttpResponse, JsonResponse from django.utils.decorators import method_decorator @@ -24,10 +24,6 @@ CACHE_TIMEOUT = 30 -def get_feature_flags(): - return feature_flags - - class HealthCheckCustomView(MainView): _plugin_name_map = { 'DatabaseBackend': 'db', @@ -49,8 +45,8 @@ def render_to_response_json(self, plugins, status, user): # customize JSON outp model_name = settings.ANSIBLE_AI_MODEL_NAME deployed_region = settings.DEPLOYED_REGION if settings.LAUNCHDARKLY_SDK_KEY: - feature_flags = get_feature_flags() - model_tuple = feature_flags.get("model_name", user, f".:.:{model_name}:.") + # Lazy instantiation of FeatureFlags to ensure it honours settings.LAUNCHDARKLY_SDK_KEY + model_tuple = FeatureFlags().get("model_name", user, f".:.:{model_name}:.") model_parts = model_tuple.split(':') if len(model_parts) == 4: _, _, model_name, _ = model_parts diff --git a/ansible_wisdom/main/settings/development.py b/ansible_wisdom/main/settings/development.py index 0c7c482ba..c3f46d102 100644 --- a/ansible_wisdom/main/settings/development.py +++ b/ansible_wisdom/main/settings/development.py @@ -80,6 +80,3 @@ # WCA_SECRET_DUMMY_SECRETS=1009103:valid,11009104:not-valid WCA_SECRET_DUMMY_SECRETS = os.getenv("WCA_SECRET_DUMMY_SECRETS", "") WCA_CLIENT_BACKEND_TYPE = os.getenv("WCA_CLIENT_BACKEND_TYPE", "dummy") # or wcaclient - -# Enable Telemetry Opt In/Out settings in the Admin Portal -ADMIN_PORTAL_TELEMETRY_OPT_ENABLED = True diff --git a/ansible_wisdom/main/settings/production.py b/ansible_wisdom/main/settings/production.py index 9b1feb465..0fd5c139c 100644 --- a/ansible_wisdom/main/settings/production.py +++ b/ansible_wisdom/main/settings/production.py @@ -39,6 +39,3 @@ SESSION_ENGINE = "django.contrib.sessions.backends.cache" SESSION_CACHE_ALIAS = "default" - -# Disable Telemetry Opt In/Out settings in the Admin Portal -ADMIN_PORTAL_TELEMETRY_OPT_ENABLED = os.getenv('ADMIN_PORTAL_TELEMETRY_OPT_ENABLED', False) diff --git a/ansible_wisdom/main/templates/console/console.html b/ansible_wisdom/main/templates/console/console.html index 46b655c0a..4db1bcf2c 100644 --- a/ansible_wisdom/main/templates/console/console.html +++ b/ansible_wisdom/main/templates/console/console.html @@ -18,5 +18,5 @@ - + {% endblock content %} diff --git a/ansible_wisdom/main/tests/test_console_views.py b/ansible_wisdom/main/tests/test_console_views.py index 3523dc7f8..d35926c1a 100644 --- a/ansible_wisdom/main/tests/test_console_views.py +++ b/ansible_wisdom/main/tests/test_console_views.py @@ -1,6 +1,7 @@ from http import HTTPStatus from unittest.mock import patch +import ai.feature_flags as feature_flags from ai.api.permissions import ( AcceptedTermsPermission, IsOrganisationAdministrator, @@ -10,6 +11,7 @@ from django.test import override_settings from django.urls import resolve, reverse from oauth2_provider.contrib.rest_framework import IsAuthenticatedOrTokenHasScope +from organizations.models import Organization from rest_framework.permissions import IsAuthenticated @@ -62,26 +64,35 @@ def test_permission_classes(self, *args): @patch.object(IsOrganisationAdministrator, 'has_permission', return_value=True) @patch.object(IsOrganisationLightspeedSubscriber, 'has_permission', return_value=True) def test_extra_data(self, *args): + self.user.organization = Organization.objects.get_or_create(id=123)[0] self.client.force_authenticate(user=self.user) response = self.client.get(reverse('console')) self.assertIsInstance(response.context_data, dict) context = response.context_data self.assertEqual(context['user_name'], self.user.username) self.assertEqual(context['rh_org_has_subscription'], self.user.rh_org_has_subscription) - self.assertTrue(context['telemetry_opt_enabled']) - def test_extra_data_telemetry_opt_in(self, *args): + @override_settings(LAUNCHDARKLY_SDK_KEY='dummy_key') + @patch.object(feature_flags, 'LDClient') + def test_extra_data_telemetry_feature_enabled(self, LDClient, *args): + LDClient.return_value.variation.return_value = True + self.user.organization = Organization.objects.get_or_create(id=123)[0] self.client.force_authenticate(user=self.user) response = self.client.get(reverse('console')) self.assertIsInstance(response.context_data, dict) context = response.context_data # The default setting for tests is True - self.assertTrue(context['telemetry_opt_enabled']) + self.assertTrue(context['telemetry_schema_2_enabled']) - @override_settings(ADMIN_PORTAL_TELEMETRY_OPT_ENABLED=False) - def test_extra_data_telemetry_opt_out(self, *args): + @override_settings(LAUNCHDARKLY_SDK_KEY='dummy_key') + @patch.object(feature_flags, 'LDClient') + def test_extra_data_telemetry__feature_disabled(self, LDClient, *args): + LDClient.return_value.variation.return_value = False + self.user.organization = Organization.objects.get_or_create(id=123, telemetry_opt_out=True)[ + 0 + ] self.client.force_authenticate(user=self.user) response = self.client.get(reverse('console')) self.assertIsInstance(response.context_data, dict) context = response.context_data - self.assertFalse(context['telemetry_opt_enabled']) + self.assertFalse(context['telemetry_schema_2_enabled']) diff --git a/ansible_wisdom/main/tests/test_urls.py b/ansible_wisdom/main/tests/test_urls.py index 622731edf..77500d5d5 100644 --- a/ansible_wisdom/main/tests/test_urls.py +++ b/ansible_wisdom/main/tests/test_urls.py @@ -29,19 +29,9 @@ def test_headers(self): ) self.assertIn("default-src 'self' data:", response.headers.get('Content-Security-Policy')) - def test_telemetry_patterns_when_enabled(self): - reload(main.urls) + def test_telemetry_patterns(self): r = compile("api/v0/telemetry/") patterns = list( filter(r.match, [str(pattern.pattern) for pattern in main.urls.urlpatterns]) ) self.assertEqual(1, len(patterns)) - - @override_settings(ADMIN_PORTAL_TELEMETRY_OPT_ENABLED=False) - def test_telemetry_patterns_when_disabled(self): - reload(main.urls) - r = compile("api/v0/telemetry/") - patterns = list( - filter(r.match, [str(pattern.pattern) for pattern in main.urls.urlpatterns]) - ) - self.assertEqual(0, len(patterns)) diff --git a/ansible_wisdom/main/urls.py b/ansible_wisdom/main/urls.py index dcbb9c491..58e74ac02 100644 --- a/ansible_wisdom/main/urls.py +++ b/ansible_wisdom/main/urls.py @@ -58,14 +58,13 @@ path('console///', ConsoleView.as_view(), name='console'), ] -if settings.ADMIN_PORTAL_TELEMETRY_OPT_ENABLED: - urlpatterns += [ - path( - f'api/{WISDOM_API_VERSION}/telemetry/', - TelemetrySettingsView.as_view(), - name='telemetry_settings', - ), - ] +urlpatterns += [ + path( + f'api/{WISDOM_API_VERSION}/telemetry/', + TelemetrySettingsView.as_view(), + name='telemetry_settings', + ), +] if settings.DEBUG: urlpatterns += [ diff --git a/ansible_wisdom/main/views.py b/ansible_wisdom/main/views.py index e77af2f60..1c8c14e48 100644 --- a/ansible_wisdom/main/views.py +++ b/ansible_wisdom/main/views.py @@ -69,8 +69,11 @@ def get_template_names(self): def get_context_data(self, **kwargs): context = super().get_context_data(**kwargs) - if self.request.user: - context["user_name"] = self.request.user.username - context["rh_org_has_subscription"] = self.request.user.rh_org_has_subscription - context["telemetry_opt_enabled"] = settings.ADMIN_PORTAL_TELEMETRY_OPT_ENABLED + user = self.request.user + if user: + context["user_name"] = user.username + context["rh_org_has_subscription"] = user.rh_org_has_subscription + organization = user.organization + if organization: + context["telemetry_schema_2_enabled"] = organization.is_schema_2_telemetry_enabled return context diff --git a/ansible_wisdom/organizations/models.py b/ansible_wisdom/organizations/models.py index ef6d9be22..a454507d6 100644 --- a/ansible_wisdom/organizations/models.py +++ b/ansible_wisdom/organizations/models.py @@ -1,6 +1,8 @@ import logging +from django.conf import settings from django.db import models +from django.utils.functional import cached_property logger = logging.getLogger(__name__) @@ -8,3 +10,17 @@ class Organization(models.Model): id = models.IntegerField(primary_key=True) telemetry_opt_out = models.BooleanField(default=False) + + @cached_property + def is_schema_2_telemetry_enabled(self) -> bool: + if not settings.LAUNCHDARKLY_SDK_KEY: + return False + + # Avoid circular dependency issue with lazy import + from ai.feature_flags import FeatureFlags, WisdomFlags + + feature_flags = FeatureFlags() + return feature_flags.check_flag( + WisdomFlags.SCHEMA_2_TELEMETRY_ORG_ENABLED, + {'kind': 'organization', 'key': str(self.id)}, + ) diff --git a/ansible_wisdom/organizations/tests/test_organizations.py b/ansible_wisdom/organizations/tests/test_organizations.py new file mode 100644 index 000000000..5f5fafe55 --- /dev/null +++ b/ansible_wisdom/organizations/tests/test_organizations.py @@ -0,0 +1,49 @@ +from unittest.mock import patch + +import ai.feature_flags as feature_flags +from django.test import TestCase, override_settings +from organizations.models import Organization + + +class TestOrganization(TestCase): + def test_org_with_telemetry_schema_2_opted_in(self): + organization = Organization.objects.get_or_create(id=123, telemetry_opt_out=False)[0] + self.assertFalse(organization.telemetry_opt_out) + self.assertFalse(organization.is_schema_2_telemetry_enabled) + + def test_org_with_telemetry_schema_2_opted_out(self): + organization = Organization.objects.get_or_create(id=123, telemetry_opt_out=True)[0] + self.assertTrue(organization.telemetry_opt_out) + self.assertFalse(organization.is_schema_2_telemetry_enabled) + + @override_settings(LAUNCHDARKLY_SDK_KEY='dummy_key') + @patch.object(feature_flags, 'LDClient') + def test_org_with_telemetry_schema_2_opted_in_with_feature_flag_override(self, LDClient): + LDClient.return_value.variation.return_value = True + organization = Organization.objects.get_or_create(id=123, telemetry_opt_out=True)[0] + self.assertTrue(organization.telemetry_opt_out) + self.assertTrue(organization.is_schema_2_telemetry_enabled) + + @override_settings(LAUNCHDARKLY_SDK_KEY='dummy_key') + @patch.object(feature_flags, 'LDClient') + def test_org_with_telemetry_schema_2_opted_in_with_feature_flag_no_override(self, LDClient): + LDClient.return_value.variation.return_value = False + organization = Organization.objects.get_or_create(id=123, telemetry_opt_out=True)[0] + self.assertTrue(organization.telemetry_opt_out) + self.assertFalse(organization.is_schema_2_telemetry_enabled) + + @override_settings(LAUNCHDARKLY_SDK_KEY='dummy_key') + @patch.object(feature_flags, 'LDClient') + def test_org_with_telemetry_schema_2_opted_out_with_feature_flag_override(self, LDClient): + LDClient.return_value.variation.return_value = True + organization = Organization.objects.get_or_create(id=123, telemetry_opt_out=False)[0] + self.assertFalse(organization.telemetry_opt_out) + self.assertTrue(organization.is_schema_2_telemetry_enabled) + + @override_settings(LAUNCHDARKLY_SDK_KEY='dummy_key') + @patch.object(feature_flags, 'LDClient') + def test_org_with_telemetry_schema_2_opted_out_with_feature_flag_no_override(self, LDClient): + LDClient.return_value.variation.return_value = False + organization = Organization.objects.get_or_create(id=123, telemetry_opt_out=False)[0] + self.assertFalse(organization.telemetry_opt_out) + self.assertFalse(organization.is_schema_2_telemetry_enabled) diff --git a/ansible_wisdom/users/tests/test_users.py b/ansible_wisdom/users/tests/test_users.py index 4cd837bb8..b5e29f86b 100644 --- a/ansible_wisdom/users/tests/test_users.py +++ b/ansible_wisdom/users/tests/test_users.py @@ -6,6 +6,7 @@ from unittest.mock import Mock, patch from uuid import uuid4 +import ai.feature_flags as feature_flags from ai.api.permissions import ( AcceptedTermsPermission, IsOrganisationAdministrator, @@ -555,7 +556,10 @@ def test_rhsso_user_with_telemetry_opted_in(self): self.assertEqual(r.status_code, HTTPStatus.OK) self.assertFalse(r.data.get('org_telemetry_opt_out')) - def test_rhsso_user_with_telemetry_opted_out(self): + @override_settings(LAUNCHDARKLY_SDK_KEY='dummy_key') + @patch.object(feature_flags, 'LDClient') + def test_rhsso_user_with_telemetry_opted_out(self, LDClient): + LDClient.return_value.variation.return_value = True user = create_user( provider=USER_SOCIAL_AUTH_PROVIDER_OIDC, social_auth_extra_data={"login": "sso_username"}, @@ -567,8 +571,10 @@ def test_rhsso_user_with_telemetry_opted_out(self): self.assertEqual(r.status_code, HTTPStatus.OK) self.assertTrue(r.data.get('org_telemetry_opt_out')) - @override_settings(ADMIN_PORTAL_TELEMETRY_OPT_ENABLED=False) - def test_rhsso_user_with_telemetry_feature_disabled(self): + @override_settings(LAUNCHDARKLY_SDK_KEY='dummy_key') + @patch.object(feature_flags, 'LDClient') + def test_rhsso_user_with_telemetry_feature_disabled(self, LDClient): + LDClient.return_value.variation.return_value = False user = create_user( provider=USER_SOCIAL_AUTH_PROVIDER_OIDC, social_auth_extra_data={"login": "sso_username"}, @@ -580,10 +586,43 @@ def test_rhsso_user_with_telemetry_feature_disabled(self): self.assertEqual(r.status_code, HTTPStatus.OK) self.assertIsNone(r.data.get('org_telemetry_opt_out')) + @override_settings(LAUNCHDARKLY_SDK_KEY='dummy_key') + @patch.object(feature_flags, 'LDClient') + def test_rhsso_user_with_telemetry_feature_enabled_opted_out(self, LDClient): + LDClient.return_value.variation.return_value = True + user = create_user( + provider=USER_SOCIAL_AUTH_PROVIDER_OIDC, + social_auth_extra_data={"login": "sso_username"}, + external_username="sso_username", + org_opt_out=True, + ) + self.client.force_authenticate(user=user) + r = self.client.get(reverse('me')) + self.assertEqual(r.status_code, HTTPStatus.OK) + self.assertTrue(r.data.get('org_telemetry_opt_out')) + + @override_settings(LAUNCHDARKLY_SDK_KEY='dummy_key') + @patch.object(feature_flags, 'LDClient') + def test_rhsso_user_with_telemetry_feature_enabled_opted_in(self, LDClient): + LDClient.return_value.variation.return_value = True + user = create_user( + provider=USER_SOCIAL_AUTH_PROVIDER_OIDC, + social_auth_extra_data={"login": "sso_username"}, + external_username="sso_username", + org_opt_out=False, + ) + self.client.force_authenticate(user=user) + r = self.client.get(reverse('me')) + self.assertEqual(r.status_code, HTTPStatus.OK) + self.assertFalse(r.data.get('org_telemetry_opt_out')) + + @override_settings(LAUNCHDARKLY_SDK_KEY='dummy_key') @patch.object(IsOrganisationAdministrator, 'has_permission', return_value=True) @patch.object(IsOrganisationLightspeedSubscriber, 'has_permission', return_value=True) @patch.object(AcceptedTermsPermission, 'has_permission', return_value=True) - def test_rhsso_user_caching(self, *args): + @patch.object(feature_flags, 'LDClient') + def test_rhsso_user_caching(self, LDClient, *args): + LDClient.return_value.variation.return_value = True user = create_user( provider=USER_SOCIAL_AUTH_PROVIDER_OIDC, social_auth_extra_data={"login": "sso_username"}, diff --git a/ansible_wisdom/users/views.py b/ansible_wisdom/users/views.py index b1485d083..ee686fb3c 100644 --- a/ansible_wisdom/users/views.py +++ b/ansible_wisdom/users/views.py @@ -77,10 +77,9 @@ def retrieve(self, request, *args, **kwargs): user_data = serializer.data # Enrich with Organisational data, if necessary - if settings.ADMIN_PORTAL_TELEMETRY_OPT_ENABLED: - organization = self.request.user.organization - if organization: - user_data["org_telemetry_opt_out"] = organization.telemetry_opt_out + organization = self.request.user.organization + if organization and organization.is_schema_2_telemetry_enabled: + user_data["org_telemetry_opt_out"] = organization.telemetry_opt_out return Response(user_data) diff --git a/ansible_wisdom_console_react/public/index.html b/ansible_wisdom_console_react/public/index.html index 094b0fc5d..8d83dce00 100644 --- a/ansible_wisdom_console_react/public/index.html +++ b/ansible_wisdom_console_react/public/index.html @@ -11,6 +11,6 @@
- + diff --git a/ansible_wisdom_console_react/src/index.tsx b/ansible_wisdom_console_react/src/index.tsx index c78a82828..ff516c773 100644 --- a/ansible_wisdom_console_react/src/index.tsx +++ b/ansible_wisdom_console_react/src/index.tsx @@ -10,14 +10,14 @@ import "./i18n"; import "./index.css"; const userName = document.getElementById("user_name")?.innerText ?? "undefined"; -const telemetryOptEnabledInnerText = document.getElementById( - "telemetry_opt_enabled", +const telemetrySchema2EnabledInnerText = document.getElementById( + "telemetry_schema_2_enabled", )?.innerText; -const telemetryOptEnabled = - telemetryOptEnabledInnerText?.toLowerCase() === "true"; +const telemetrySchema2Enabled = + telemetrySchema2EnabledInnerText?.toLowerCase() === "true"; createRoot(document.getElementById("root") as HTMLElement).render( - + , );