feat(backup): Add DateAddedComparator (#54009)

This comparator accounts for the fact that old JSON exports may have
zero millisecond timestamps formatted without the `.000`, as Python is
wont to do in its natural datetime serializer. We have since replaced
this default behavior with more consistent behavior that always includes
the milliseconds, but to work with old exports, we still need to use
this comparator.

Closes getsentry/team-ospo#155

Author: azaslavsky

fixtures/backup/datetime-with-unzeroed-millis.json

@@ -0,0 +1,20 @@
+[
+  {
+    "model": "sites.site",
+    "pk": 1,
+    "fields": {
+      "domain": "example.com",
+      "name": "example.com"
+    }
+  },
+  {
+    "model": "sentry.option",
+    "pk": 1,
+    "fields": {
+      "key": "sentry:latest_version",
+      "last_updated": "2023-06-22T00:00:00Z",
+      "last_updated_by": "unknown",
+      "value": "\"23.6.1\""
+    }
+  }
+]

fixtures/backup/datetime-formatting.json renamed to fixtures/backup/datetime-with-zeroed-millis.json

@@ -4,8 +4,12 @@
 from typing import Callable, Dict, List, Literal
 
 from dateutil import parser
+from django.db import models
 
 from sentry.backup.findings import ComparatorFinding, InstanceID
+from sentry.backup.helpers import get_exportable_final_derivations_of
+from sentry.db.models import BaseModel
+from sentry.db.models.fields.foreignkey import FlexibleForeignKey
 from sentry.utils.json import JSONData
 
 
@@ -25,7 +29,7 @@ class JSONScrubbingComparator(ABC):
     have their inputs mangled by one another."""
 
     def __init__(self, *fields: str):
-        self.fields = fields
+        self.fields = set(fields)
 
     def check(self, side: str, data: JSONData) -> None:
         """Ensure that we have received valid JSON data at runtime."""
@@ -97,11 +101,9 @@ def __scrub__(
 
         for field in self.fields:
             for side in [left, right]:
-                if field not in side["fields"]:
+                if not bool(side["fields"].get(field)):
                     continue
                 value = side["fields"][field]
-                if not value:
-                    continue
                 value = [value] if isinstance(value, str) else value
                 del side["fields"][field]
                 side["scrubbed"][f"{self.get_kind()}::{field}"] = f(value)
@@ -130,7 +132,7 @@ def __init__(self, field: str):
 
     def compare(self, on: InstanceID, left: JSONData, right: JSONData) -> list[ComparatorFinding]:
         f = self.field
-        if f not in left["fields"] and f not in right["fields"]:
+        if not bool(left["fields"].get(f)) and not bool(right["fields"].get(f)):
             return []
 
         left_date_updated = left["fields"][f]
@@ -140,13 +142,40 @@ def compare(self, on: InstanceID, left: JSONData, right: JSONData) -> list[Compa
                 ComparatorFinding(
                     kind=self.get_kind(),
                     on=on,
-                    reason=f"""the left date_updated value on `{on}` ({left_date_updated}) was not
-                            less than or equal to the right ({right_date_updated})""",
+                    reason=f"""the left value ({left_date_updated}) of `{f}` was not less than or equal to the right value ({right_date_updated})""",
                 )
             ]
         return []
 
 
+class DateAddedComparator(JSONScrubbingComparator):
+    """Some exports from before [email protected] may trim milliseconds from timestamps if they end in
+    exactly `.000` (ie, not milliseconds at all - what are the odds!). Because comparisons may fail
+    in this case, we use a special comparator for these cases."""
+
+    def __init__(self, *fields: str):
+        super().__init__(*fields)
+
+    def compare(self, on: InstanceID, left: JSONData, right: JSONData) -> list[ComparatorFinding]:
+        findings = []
+        fields = sorted(self.fields)
+        for f in fields:
+            if not bool(left["fields"].get(f)) and not bool(right["fields"].get(f)):
+                continue
+
+            left_date_added = left["fields"][f]
+            right_date_added = right["fields"][f]
+            if parser.parse(left_date_added) != parser.parse(right_date_added):
+                findings.append(
+                    ComparatorFinding(
+                        kind=self.get_kind(),
+                        on=on,
+                        reason=f"""the left value ({left_date_added}) of `{f}` was not equal to the right value ({right_date_added})""",
+                    )
+                )
+        return findings
+
+
 class ObfuscatingComparator(JSONScrubbingComparator, ABC):
     """Comparator that compares private values, but then safely truncates them to ensure that they
     do not leak out in logs, stack traces, etc."""
@@ -156,7 +185,8 @@ def __init__(self, *fields: str):
 
     def compare(self, on: InstanceID, left: JSONData, right: JSONData) -> list[ComparatorFinding]:
         findings = []
-        for f in self.fields:
+        fields = sorted(self.fields)
+        for f in fields:
             if f not in left["fields"] and f not in right["fields"]:
                 continue
 
@@ -169,8 +199,7 @@ def compare(self, on: InstanceID, left: JSONData, right: JSONData) -> list[Compa
                     ComparatorFinding(
                         kind=self.get_kind(),
                         on=on,
-                        reason=f"""the left `{f}` value ("{lv}") on `{on}` was not equal to the
-                                right value ("{rv}")""",
+                        reason=f"""the left value ("{lv}") of `{f}` was not equal to the right value ("{rv}")""",
                     )
                 )
         return findings
@@ -224,53 +253,89 @@ def truncate(self, data: list[str]) -> list[str]:
         return truncated
 
 
+def auto_assign_date_added_comparator(comps: ComparatorMap) -> None:
+    """Automatically assigns the DateAddedComparator to any `DateTimeField` that is not already claimed by the `DateUpdatedComparator`."""
+
+    exportable = get_exportable_final_derivations_of(BaseModel)
+    for e in exportable:
+        name = "sentry." + e.__name__.lower()
+        fields = e._meta.get_fields()
+        assign = set()
+        for f in fields:
+            if isinstance(f, models.DateTimeField) and name in comps:
+                date_updated_comparator = next(
+                    filter(lambda e: isinstance(e, DateUpdatedComparator), comps[name]), None
+                )
+                if not date_updated_comparator or f.name not in date_updated_comparator.fields:
+                    assign.add(f.name)
+
+        if name in comps:
+            found = next(filter(lambda e: isinstance(e, DateAddedComparator), comps[name]), None)
+            if found:
+                found.fields.update(assign)
+            else:
+                comps[name].append(DateAddedComparator(*assign))
+        else:
+            comps[name] = [DateAddedComparator(*assign)]
+
+
+def auto_assign_email_obfuscating_comparator(comps: ComparatorMap) -> None:
+    """Automatically assigns the EmailObfuscatingComparator to any field that is an `EmailField` or has a foreign key into the `sentry.User` table."""
+
+    exportable = get_exportable_final_derivations_of(BaseModel)
+    for e in exportable:
+        name = "sentry." + e.__name__.lower()
+        fields = e._meta.get_fields()
+        assign = set()
+        for f in fields:
+            if isinstance(f, models.EmailField):
+                assign.add(f.name)
+            elif isinstance(f, FlexibleForeignKey) and f.related_model.__name__ == "User":
+                assign.add(f.name)
+            elif isinstance(f, models.OneToOneField) and f.related_model.__name__ == "User":
+                assign.add(f.name)
+            elif isinstance(f, models.ManyToManyField) and f.related_model.__name__ == "User":
+                assign.add(f.name)
+
+        if name in comps:
+            found = next(
+                filter(lambda e: isinstance(e, EmailObfuscatingComparator), comps[name]), None
+            )
+            if found:
+                found.fields.update(assign)
+            else:
+                comps[name].append(EmailObfuscatingComparator(*assign))
+        else:
+            comps[name] = [EmailObfuscatingComparator(*assign)]
+
+
 ComparatorList = List[JSONScrubbingComparator]
 ComparatorMap = Dict[str, ComparatorList]
+
+# Some comparators (like `DateAddedComparator`) we can automatically assign by inspecting the
+# `Field` type on the Django `Model` definition. Others, like the ones in this map, we must assign
+# manually, since there is no clever way to derive them automatically.
 DEFAULT_COMPARATORS: ComparatorMap = {
-    "sentry.apitoken": [
-        EmailObfuscatingComparator("user"),
-        HashObfuscatingComparator("refresh_token", "token"),
-    ],
-    "sentry.apiapplication": [
-        EmailObfuscatingComparator("owner"),
-        HashObfuscatingComparator("client_id", "client_secret"),
-    ],
-    "sentry.apiauthorization": [
-        EmailObfuscatingComparator("user"),
-    ],
-    "sentry.authidentity": [
-        EmailObfuscatingComparator("user"),
-        HashObfuscatingComparator("ident", "token"),
-    ],
-    "sentry.authenticator": [EmailObfuscatingComparator("user")],
-    "sentry.email": [EmailObfuscatingComparator("email")],
+    "sentry.apitoken": [HashObfuscatingComparator("refresh_token", "token")],
+    "sentry.apiapplication": [HashObfuscatingComparator("client_id", "client_secret")],
+    "sentry.authidentity": [HashObfuscatingComparator("ident", "token")],
     "sentry.alertrule": [DateUpdatedComparator("date_modified")],
     "sentry.incidenttrigger": [DateUpdatedComparator("date_modified")],
     "sentry.orgauthtoken": [HashObfuscatingComparator("token_hashed", "token_last_characters")],
-    "sentry.organizationmember": [
-        EmailObfuscatingComparator("user_email"),
-        HashObfuscatingComparator("token"),
-    ],
+    "sentry.organizationmember": [HashObfuscatingComparator("token")],
     "sentry.projectkey": [HashObfuscatingComparator("public_key", "secret_key")],
     "sentry.querysubscription": [DateUpdatedComparator("date_updated")],
     "sentry.relay": [HashObfuscatingComparator("relay_id", "public_key")],
     "sentry.relayusage": [HashObfuscatingComparator("relay_id", "public_key")],
-    "sentry.sentryapp": [EmailObfuscatingComparator("creator_user", "creator_label", "proxy_user")],
+    "sentry.sentryapp": [EmailObfuscatingComparator("creator_label")],
     "sentry.servicehook": [HashObfuscatingComparator("secret")],
-    "sentry.user": [
-        EmailObfuscatingComparator("email", "username"),
-        HashObfuscatingComparator("password"),
-    ],
-    "sentry.useremail": [
-        EmailObfuscatingComparator("email", "user"),
-        HashObfuscatingComparator("validation_hash"),
-    ],
-    "sentry.userip": [EmailObfuscatingComparator("user")],
-    "sentry.useroption": [EmailObfuscatingComparator("user")],
-    "sentry.userpermission": [EmailObfuscatingComparator("user")],
+    "sentry.user": [HashObfuscatingComparator("password")],
+    "sentry.useremail": [HashObfuscatingComparator("validation_hash")],
     "sentry.userrole": [DateUpdatedComparator("date_updated")],
-    "sentry.userroleuser": [
-        DateUpdatedComparator("date_updated"),
-        EmailObfuscatingComparator("user"),
-    ],
+    "sentry.userroleuser": [DateUpdatedComparator("date_updated")],
 }
+
+# Where possible, we automatically deduce fields that should have special comparators and add them
+# to the `DEFAULT_COMPARATORS` map.
+auto_assign_date_added_comparator(DEFAULT_COMPARATORS)
+auto_assign_email_obfuscating_comparator(DEFAULT_COMPARATORS)

src/sentry/backup/comparators.py

@@ -1,4 +1,5 @@
 from sentry.backup.comparators import (
+    DateAddedComparator,
     DateUpdatedComparator,
     EmailObfuscatingComparator,
     HashObfuscatingComparator,
@@ -63,6 +64,53 @@ def test_bad_comparator_only_one_side_existing():
     assert "my_date_field" in res[0].reason
 
 
+def test_good_date_added_comparator():
+    cmp = DateAddedComparator("my_date_field")
+    id = InstanceID("test", 1)
+    left: JSONData = {
+        "model": "test",
+        "pk": 1,
+        "fields": {
+            "my_date_field": "2023-06-22T23:00:00.123Z",
+        },
+    }
+    right: JSONData = {
+        "model": "test",
+        "pk": 1,
+        "fields": {
+            "my_date_field": "2023-06-22T23:00:00.123Z",
+        },
+    }
+    assert not cmp.compare(id, left, right)
+
+
+def test_bad_date_added_comparator():
+    cmp = DateAddedComparator("my_date_field")
+    id = InstanceID("test", 1)
+    left: JSONData = {
+        "model": "test",
+        "pk": 1,
+        "fields": {
+            "my_date_field": "2023-06-22T00:00:00.000Z",
+        },
+    }
+    right: JSONData = {
+        "model": "test",
+        "pk": 1,
+        "fields": {
+            "my_date_field": "2023-06-22T00:00:00.123Z",
+        },
+    }
+    res = cmp.compare(id, left, right)
+    assert res
+    assert res[0]
+    assert res[0].on == id
+    assert res[0].kind == "DateAddedComparator"
+    assert "`my_date_field`" in res[0].reason
+    assert "left value (2023-06-22T00:00:00.000Z)" in res[0].reason
+    assert "right value (2023-06-22T00:00:00.123Z)" in res[0].reason
+
+
 def test_good_date_updated_comparator():
     cmp = DateUpdatedComparator("my_date_field")
     id = InstanceID("test", 1)
@@ -105,6 +153,9 @@ def test_bad_date_updated_comparator():
     assert res[0]
     assert res[0].on == id
     assert res[0].kind == "DateUpdatedComparator"
+    assert "`my_date_field`" in res[0].reason
+    assert "left value (2023-06-22T23:12:34.567Z)" in res[0].reason
+    assert "right value (2023-06-22T23:00:00.001Z)" in res[0].reason
 
 
 def test_good_email_obfuscating_comparator():
@@ -155,14 +206,14 @@ def test_bad_email_obfuscating_comparator():
     assert res[0]
     assert res[0].on == id
     assert res[0].kind == "EmailObfuscatingComparator"
-    assert "a[email protected]" in res[0].reason
-    assert "a[email protected]" in res[0].reason
+    assert "b[email protected]" in res[0].reason
+    assert "b[email protected]" in res[0].reason
 
     assert res[1]
     assert res[1].on == id
     assert res[1].kind == "EmailObfuscatingComparator"
-    assert "b[email protected]" in res[1].reason
-    assert "b[email protected]" in res[1].reason
+    assert "a[email protected]" in res[1].reason
+    assert "a[email protected]" in res[1].reason
 
 
 def test_good_email_obfuscating_comparator_scrubbed():
@@ -253,14 +304,14 @@ def test_bad_hash_obfuscating_comparator():
     assert res[0]
     assert res[0].on == id
     assert res[0].kind == "HashObfuscatingComparator"
-    assert "123...39b" in res[0].reason
-    assert "124...39c" in res[0].reason
+    assert "1...e" in res[0].reason
+    assert "2...f" in res[0].reason
 
     assert res[1]
     assert res[1].on == id
     assert res[1].kind == "HashObfuscatingComparator"
-    assert "1...e" in res[1].reason
-    assert "2...f" in res[1].reason
+    assert "123...39b" in res[1].reason
+    assert "124...39c" in res[1].reason
 
 
 def test_good_hash_obfuscating_comparator_scrubbed():