From 486bcd1d2fc6a360f002183027186d70798b4f06 Mon Sep 17 00:00:00 2001 From: Matthew Elwell Date: Fri, 6 Dec 2024 09:11:30 +0000 Subject: [PATCH] fix: audit log integrations for versioned environments (#4876) --- api/app/settings/common.py | 7 +- api/app/settings/test.py | 2 + api/audit/services.py | 1 + api/audit/signals.py | 1 + api/integrations/dynatrace/dynatrace.py | 71 +++----- api/integrations/grafana/mappers.py | 7 + .../unit/audit/test_unit_audit_signals.py | 156 ++++++++++++++++-- api/tests/unit/conftest.py | 11 ++ .../dynatrace/test_unit_dynatrace.py | 30 +++- .../unit/integrations/grafana/test_mappers.py | 12 -- api/util/util.py | 11 +- 11 files changed, 234 insertions(+), 75 deletions(-) diff --git a/api/app/settings/common.py b/api/app/settings/common.py index ad14f2b78cef..2f1e468eb4d3 100644 --- a/api/app/settings/common.py +++ b/api/app/settings/common.py @@ -543,7 +543,9 @@ SECURE_REDIRECT_EXEMPT = env.list("DJANGO_SECURE_REDIRECT_EXEMPT", default=[]) SECURE_REFERRER_POLICY = env.str("DJANGO_SECURE_REFERRER_POLICY", default="same-origin") -SECURE_CROSS_ORIGIN_OPENER_POLICY = env.str("DJANGO_SECURE_CROSS_ORIGIN_OPENER_POLICY", default="same-origin") +SECURE_CROSS_ORIGIN_OPENER_POLICY = env.str( + "DJANGO_SECURE_CROSS_ORIGIN_OPENER_POLICY", default="same-origin" +) SECURE_SSL_HOST = env.str("DJANGO_SECURE_SSL_HOST", default=None) SECURE_SSL_REDIRECT = env.bool("DJANGO_SECURE_SSL_REDIRECT", default=False) @@ -1008,6 +1010,9 @@ "ENABLE_TASK_PROCESSOR_HEALTH_CHECK", default=False ) +# Allows us to prevent the postpone decorator from running things async +ENABLE_POSTPONE_DECORATOR = env.bool("ENABLE_POSTPONE_DECORATOR", default=True) + ENABLE_CLEAN_UP_OLD_TASKS = env.bool("ENABLE_CLEAN_UP_OLD_TASKS", default=True) TASK_DELETE_RETENTION_DAYS = env.int("TASK_DELETE_RETENTION_DAYS", default=30) TASK_DELETE_BATCH_SIZE = env.int("TASK_DELETE_BATCH_SIZE", default=2000) diff --git a/api/app/settings/test.py b/api/app/settings/test.py index 3faad310c58f..29c79db86c3d 100644 --- a/api/app/settings/test.py +++ b/api/app/settings/test.py @@ -61,3 +61,5 @@ 3wIDAQAB -----END PUBLIC KEY----- """ + +ENABLE_POSTPONE_DECORATOR = False diff --git a/api/audit/services.py b/api/audit/services.py index 2663c7ecf431..a6b6986ff42e 100644 --- a/api/audit/services.py +++ b/api/audit/services.py @@ -54,6 +54,7 @@ def get_audited_instance_from_audit_log_record( uuid=audit_log_record.related_object_uuid, environment=audit_log_record.environment, ) + .select_related("feature") .first() ) diff --git a/api/audit/signals.py b/api/audit/signals.py index deb31810fdd3..340603833728 100644 --- a/api/audit/signals.py +++ b/api/audit/signals.py @@ -61,6 +61,7 @@ def signal_wrapper(sender, instance, **kwargs): RelatedObjectType.FEATURE.name, RelatedObjectType.FEATURE_STATE.name, RelatedObjectType.SEGMENT.name, + RelatedObjectType.EF_VERSION.name, ]: return None return signal_function(sender, instance, **kwargs) diff --git a/api/integrations/dynatrace/dynatrace.py b/api/integrations/dynatrace/dynatrace.py index d888e668c0ef..e8f5c2f9d336 100644 --- a/api/integrations/dynatrace/dynatrace.py +++ b/api/integrations/dynatrace/dynatrace.py @@ -4,8 +4,9 @@ import requests from audit.models import AuditLog -from audit.related_object_type import RelatedObjectType -from features.models import Feature +from audit.services import get_audited_instance_from_audit_log_record +from features.models import Feature, FeatureState +from features.versioning.models import EnvironmentFeatureVersion from integrations.common.wrapper import AbstractBaseEventIntegrationWrapper from segments.models import Segment @@ -57,45 +58,27 @@ def generate_event_data(audit_log_record: AuditLog) -> dict: def _get_deployment_name(audit_log_record: AuditLog) -> str: - try: - related_object_type = RelatedObjectType[audit_log_record.related_object_type] - - if related_object_type in ( - RelatedObjectType.FEATURE, - RelatedObjectType.FEATURE_STATE, - ): - return _get_deployment_name_for_feature( - audit_log_record.related_object_id, related_object_type - ) - elif related_object_type == RelatedObjectType.SEGMENT: - return _get_deployment_name_for_segment(audit_log_record.related_object_id) - except KeyError: - pass - - # use 'Deployment' as a fallback to maintain current behaviour in the - # event that we cannot determine the correct name to return. - return DEFAULT_DEPLOYMENT_NAME - - -def _get_deployment_name_for_feature( - object_id: int, object_type: RelatedObjectType -) -> str: - qs = Feature.objects.all_with_deleted() - if object_type == RelatedObjectType.FEATURE: - qs = qs.filter(id=object_id) - elif object_type == RelatedObjectType.FEATURE_STATE: - qs = qs.filter(feature_states__id=object_id).distinct() - - if feature := qs.first(): - return f"Flagsmith Deployment - Flag Changed: {feature.name}" - - # use 'Deployment' as a fallback to maintain current behaviour in the - # event that we cannot determine the correct name to return. - return DEFAULT_DEPLOYMENT_NAME - - -def _get_deployment_name_for_segment(object_id: int) -> str: - if segment := Segment.live_objects.all_with_deleted().filter(id=object_id).first(): - return f"Flagsmith Deployment - Segment Changed: {segment.name}" - - return DEFAULT_DEPLOYMENT_NAME + audited_instance = get_audited_instance_from_audit_log_record(audit_log_record) + + if isinstance(audited_instance, Feature): + deployment_name = _get_deployment_name_for_feature(audited_instance) + elif isinstance(audited_instance, FeatureState) or isinstance( + audited_instance, EnvironmentFeatureVersion + ): + deployment_name = _get_deployment_name_for_feature(audited_instance.feature) + elif isinstance(audited_instance, Segment): + deployment_name = _get_deployment_name_for_segment(audited_instance) + else: + # use 'Deployment' as a fallback to maintain current behaviour in the + # event that we cannot determine the correct name to return. + deployment_name = DEFAULT_DEPLOYMENT_NAME + + return deployment_name + + +def _get_deployment_name_for_feature(feature: Feature) -> str: + return f"Flagsmith Deployment - Flag Changed: {feature.name}" + + +def _get_deployment_name_for_segment(segment: Segment) -> str: + return f"Flagsmith Deployment - Segment Changed: {segment.name}" diff --git a/api/integrations/grafana/mappers.py b/api/integrations/grafana/mappers.py index 8f6648d87787..edce9e086b8c 100644 --- a/api/integrations/grafana/mappers.py +++ b/api/integrations/grafana/mappers.py @@ -6,6 +6,7 @@ FeatureState, FeatureStateValue, ) +from features.versioning.models import EnvironmentFeatureVersion from integrations.grafana.types import GrafanaAnnotation from segments.models import Segment @@ -49,6 +50,12 @@ def _get_instance_tags_from_audit_log_record( *_get_feature_tags(feature), ] + if isinstance(instance, EnvironmentFeatureVersion): + return [ + f"feature:{instance.feature.name}", + *_get_feature_tags(instance.feature), + ] + return [] diff --git a/api/tests/unit/audit/test_unit_audit_signals.py b/api/tests/unit/audit/test_unit_audit_signals.py index 2b7bd6c55f0e..056dfd5c8c7c 100644 --- a/api/tests/unit/audit/test_unit_audit_signals.py +++ b/api/tests/unit/audit/test_unit_audit_signals.py @@ -1,3 +1,6 @@ +import json + +import responses from pytest_django.fixtures import SettingsWrapper from pytest_mock import MockerFixture @@ -9,13 +12,19 @@ send_audit_log_event_to_grafana, ) from environments.models import Environment +from features.models import Feature +from features.versioning.models import EnvironmentFeatureVersion +from integrations.dynatrace.dynatrace import EVENTS_API_URI from integrations.dynatrace.models import DynatraceConfiguration +from integrations.grafana.grafana import ROUTE_API_ANNOTATIONS from integrations.grafana.models import ( GrafanaOrganisationConfiguration, GrafanaProjectConfiguration, ) from organisations.models import Organisation, OrganisationWebhook from projects.models import Project +from projects.tags.models import Tag +from users.models import FFAdminUser from webhooks.webhooks import WebhookEventType @@ -99,8 +108,6 @@ def test_send_audit_log_event_to_grafana__project_grafana_config__calls_expected project: Project, ) -> None: # Given - grafana_config = GrafanaProjectConfiguration(base_url="test.com", api_key="test") - project.grafana_config = grafana_config audit_log_record = AuditLog.objects.create( project=project, related_object_type=RelatedObjectType.FEATURE.name, @@ -108,6 +115,9 @@ def test_send_audit_log_event_to_grafana__project_grafana_config__calls_expected grafana_wrapper_mock = mocker.patch("audit.signals.GrafanaWrapper", autospec=True) grafana_wrapper_instance_mock = grafana_wrapper_mock.return_value + grafana_config = GrafanaProjectConfiguration(base_url="test.com", api_key="test") + project.grafana_config = grafana_config + # When send_audit_log_event_to_grafana(AuditLog, audit_log_record) @@ -130,10 +140,6 @@ def test_send_audit_log_event_to_grafana__organisation_grafana_config__calls_exp project: Project, ) -> None: # Given - grafana_config = GrafanaOrganisationConfiguration( - base_url="test.com", api_key="test" - ) - organisation.grafana_config = grafana_config audit_log_record = AuditLog.objects.create( project=project, related_object_type=RelatedObjectType.FEATURE.name, @@ -141,6 +147,11 @@ def test_send_audit_log_event_to_grafana__organisation_grafana_config__calls_exp grafana_wrapper_mock = mocker.patch("audit.signals.GrafanaWrapper", autospec=True) grafana_wrapper_instance_mock = grafana_wrapper_mock.return_value + grafana_config = GrafanaOrganisationConfiguration( + base_url="test.com", api_key="test" + ) + organisation.grafana_config = grafana_config + # When send_audit_log_event_to_grafana(AuditLog, audit_log_record) @@ -157,15 +168,68 @@ def test_send_audit_log_event_to_grafana__organisation_grafana_config__calls_exp ) +@responses.activate +def test_send_environment_feature_version_audit_log_event_to_grafana( + tagged_feature: Feature, + tag_one: Tag, + tag_two: Tag, + environment_v2_versioning: Environment, + project: Project, + organisation: Organisation, + admin_user: FFAdminUser, +) -> None: + # Given + _, audit_log_record = _create_and_publish_environment_feature_version( + environment=environment_v2_versioning, + feature=tagged_feature, + user=admin_user, + ) + + base_url = "https://test.com" + GrafanaOrganisationConfiguration.objects.create( + base_url=base_url, api_key="test", organisation=organisation + ) + + responses.add( + method=responses.POST, + url=f"{base_url}{ROUTE_API_ANNOTATIONS}", + status=200, + json={ + "message": "Annotation added", + "id": 1, + }, + ) + + # When + send_audit_log_event_to_grafana(AuditLog, audit_log_record) + + # Then + expected_time = int(audit_log_record.created_date.timestamp() * 1000) + + assert len(responses.calls) == 1 + assert responses.calls[0].request.body == json.dumps( + { + "tags": [ + "flagsmith", + f"project:{project.name}", + f"environment:{environment_v2_versioning.name}", + f"by:{admin_user.email}", + f"feature:{tagged_feature.name}", + tag_one.label, + tag_two.label, + ], + "text": audit_log_record.log, + "time": expected_time, + "timeEnd": expected_time, + } + ) + + def test_send_audit_log_event_to_dynatrace__environment_dynatrace_config__calls_expected( mocker: MockerFixture, environment: Environment, ) -> None: # Given - dynatrace_config = DynatraceConfiguration.objects.create( - base_url="http://test.com", api_key="api_123", environment=environment - ) - environment.refresh_from_db() audit_log_record = AuditLog.objects.create( environment=environment, related_object_type=RelatedObjectType.FEATURE.name, @@ -175,6 +239,10 @@ def test_send_audit_log_event_to_dynatrace__environment_dynatrace_config__calls_ ) dynatrace_wrapper_instance_mock = dynatrace_wrapper_mock.return_value + dynatrace_config = DynatraceConfiguration.objects.create( + base_url="http://test.com", api_key="api_123", environment=environment + ) + # When send_audit_log_event_to_dynatrace(AuditLog, audit_log_record) @@ -190,3 +258,71 @@ def test_send_audit_log_event_to_dynatrace__environment_dynatrace_config__calls_ dynatrace_wrapper_instance_mock.track_event_async.assert_called_once_with( event=dynatrace_wrapper_instance_mock.generate_event_data.return_value ) + + +@responses.activate +def test_send_environment_feature_version_audit_log_event_to_dynatrace( + feature: Feature, + environment_v2_versioning: Environment, + project: Project, + organisation: Organisation, + admin_user: FFAdminUser, +) -> None: + # Given + _, audit_log_record = _create_and_publish_environment_feature_version( + environment=environment_v2_versioning, feature=feature, user=admin_user + ) + + base_url = "https://dynatrace.test.com" + api_key = "api_123" + DynatraceConfiguration.objects.create( + base_url=base_url, api_key=api_key, environment=environment_v2_versioning + ) + + responses.add( + method=responses.POST, + url=f"{base_url}{EVENTS_API_URI}?api-token={api_key}", + status=201, + json={ + "reportCount": 1, + "eventIngestResults": [{"correlationId": "foobar123456", "status": "OK"}], + }, + ) + + # When + send_audit_log_event_to_dynatrace(AuditLog, audit_log_record) + + # Then + assert len(responses.calls) == 1 + assert json.loads(responses.calls[0].request.body) == { + "title": "Flagsmith flag change.", + "eventType": "CUSTOM_DEPLOYMENT", + "properties": { + "event": f"{audit_log_record.log} by user {admin_user.email}", + "environment": environment_v2_versioning.name, + "dt.event.deployment.name": f"Flagsmith Deployment - Flag Changed: {feature.name}", + }, + "entitySelector": "", + } + + +def _create_and_publish_environment_feature_version( + environment: Environment, + feature: Feature, + user: FFAdminUser, +) -> (EnvironmentFeatureVersion, AuditLog): + version = EnvironmentFeatureVersion( + environment=environment, + feature=feature, + ) + version.publish(user) + + audit_log_record = ( + AuditLog.objects.filter( + related_object_uuid=version.uuid, + related_object_type=RelatedObjectType.EF_VERSION.name, + ) + .order_by("-created_date") + .first() + ) + return version, audit_log_record diff --git a/api/tests/unit/conftest.py b/api/tests/unit/conftest.py index 1a37c1926ebc..294b7e1df863 100644 --- a/api/tests/unit/conftest.py +++ b/api/tests/unit/conftest.py @@ -167,6 +167,17 @@ def tag_two(project): ) +@pytest.fixture +def tagged_feature( + feature: Feature, + tag_one: Tag, + tag_two: Tag, +) -> Feature: + feature.tags.add(tag_one, tag_two) + feature.save() + return feature + + @pytest.fixture() def project_two(organisation: Organisation) -> Project: return Project.objects.create(name="Test Project Two", organisation=organisation) diff --git a/api/tests/unit/integrations/dynatrace/test_unit_dynatrace.py b/api/tests/unit/integrations/dynatrace/test_unit_dynatrace.py index 5306158570fa..578a0599750c 100644 --- a/api/tests/unit/integrations/dynatrace/test_unit_dynatrace.py +++ b/api/tests/unit/integrations/dynatrace/test_unit_dynatrace.py @@ -1,10 +1,17 @@ +from typing import Type + import pytest +from django.contrib.auth.models import AbstractUser from pytest_lazyfixture import lazy_fixture +from pytest_mock import MockerFixture from audit.models import AuditLog from audit.related_object_type import RelatedObjectType from environments.models import Environment +from features.models import Feature, FeatureState from integrations.dynatrace.dynatrace import EVENTS_API_URI, DynatraceWrapper +from projects.models import Project +from segments.models import Segment def test_dynatrace_initialized_correctly(): @@ -44,22 +51,33 @@ def test_dynatrace_initialized_correctly(): ), ) def test_dynatrace_when_generate_event_data_with_correct_values_then_success( - django_user_model, related_object_type, related_object, expected_deployment_name -): + django_user_model: Type[AbstractUser], + related_object_type: RelatedObjectType, + related_object: Feature | Segment | FeatureState, + expected_deployment_name: str, + project: Project, + environment: Environment, + mocker: MockerFixture, +) -> None: # Given log = "some log data" author = django_user_model(email="test@email.com") - environment = Environment(name="test") audit_log_record = AuditLog( log=log, author=author, - environment=environment, + environment=getattr(related_object, "environment", None), + project=project, related_object_type=related_object_type, related_object_id=related_object.id, ) + mocker.patch( + "integrations.dynatrace.dynatrace.get_audited_instance_from_audit_log_record", + return_value=related_object, + ) + dynatrace = DynatraceWrapper( base_url="http://test.com", api_key="123key", @@ -73,7 +91,9 @@ def test_dynatrace_when_generate_event_data_with_correct_values_then_success( expected_event_text = f"{log} by user {author.email}" assert event_data["properties"]["event"] == expected_event_text - assert event_data["properties"]["environment"] == environment.name + assert event_data["properties"]["environment"] == ( + environment.name if hasattr(related_object, "environment") else "unknown" + ) assert ( event_data["properties"]["dt.event.deployment.name"] == expected_deployment_name ) diff --git a/api/tests/unit/integrations/grafana/test_mappers.py b/api/tests/unit/integrations/grafana/test_mappers.py index ed4b61244dd2..5a52c5e0446e 100644 --- a/api/tests/unit/integrations/grafana/test_mappers.py +++ b/api/tests/unit/integrations/grafana/test_mappers.py @@ -10,7 +10,6 @@ map_audit_log_record_to_grafana_annotation, ) from projects.models import Project -from projects.tags.models import Tag from segments.models import Segment from users.models import FFAdminUser @@ -28,17 +27,6 @@ def audit_log_record( ) -@pytest.fixture -def tagged_feature( - feature: Feature, - tag_one: Tag, - tag_two: Tag, -) -> Feature: - feature.tags.add(tag_one, tag_two) - feature.save() - return feature - - def test_map_audit_log_record_to_grafana_annotation__feature__return_expected( mocker: MockerFixture, tagged_feature: Feature, diff --git a/api/util/util.py b/api/util/util.py index 909545235f3e..53675b468cc4 100644 --- a/api/util/util.py +++ b/api/util/util.py @@ -3,14 +3,19 @@ from threading import Thread from typing import Generator, Iterable, TypeVar +from django.conf import settings + T = TypeVar("T") def postpone(function): def decorator(*args, **kwargs): - t = Thread(target=function, args=args, kwargs=kwargs) - t.daemon = True - t.start() + if settings.ENABLE_POSTPONE_DECORATOR: # pragma: no cover + t = Thread(target=function, args=args, kwargs=kwargs) + t.daemon = True + t.start() + else: + return function(*args, **kwargs) return decorator