Browse Source

feat(hybrid-cloud): Introduce OrganizationMemberMapping model and service (#45867)

Alberto Leal 1 year ago
parent
commit
a35856819b

+ 1 - 1
migrations_lockfile.txt

@@ -6,5 +6,5 @@ To resolve this, rebase against latest master and regenerate your migration. Thi
 will then be regenerated, and you should be able to merge without conflicts.
 
 nodestore: 0002_nodestore_no_dictfield
-sentry: 0401_create_rulesnooze_table
+sentry: 0402_add_organizationmembermapping_table
 social_auth: 0001_initial

+ 77 - 0
src/sentry/migrations/0402_add_organizationmembermapping_table.py

@@ -0,0 +1,77 @@
+# Generated by Django 2.2.28 on 2023-04-04 20:43
+
+import django.db.models.deletion
+import django.utils.timezone
+from django.conf import settings
+from django.db import migrations, models
+
+import sentry.db.models.fields.bounded
+import sentry.db.models.fields.foreignkey
+import sentry.db.models.fields.hybrid_cloud_foreign_key
+from sentry.new_migrations.migrations import CheckedMigration
+
+
+class Migration(CheckedMigration):
+    # This flag is used to mark that a migration shouldn't be automatically run in production. For
+    # the most part, this should only be used for operations where it's safe to run the migration
+    # after your code has deployed. So this should not be used for most operations that alter the
+    # schema of a table.
+    # Here are some things that make sense to mark as dangerous:
+    # - Large data migrations. Typically we want these to be run manually by ops so that they can
+    #   be monitored and not block the deploy for a long period of time while they run.
+    # - Adding indexes to large tables. Since this can take a long time, we'd generally prefer to
+    #   have ops run this and not block the deploy. Note that while adding an index is a schema
+    #   change, it's completely safe to run the operation after the code has deployed.
+    is_dangerous = False
+
+    dependencies = [
+        ("sentry", "0401_create_rulesnooze_table"),
+    ]
+
+    operations = [
+        migrations.CreateModel(
+            name="OrganizationMemberMapping",
+            fields=[
+                (
+                    "id",
+                    sentry.db.models.fields.bounded.BoundedBigAutoField(
+                        primary_key=True, serialize=False
+                    ),
+                ),
+                (
+                    "organization_id",
+                    sentry.db.models.fields.hybrid_cloud_foreign_key.HybridCloudForeignKey(
+                        "sentry.Organization", db_index=True, on_delete="CASCADE"
+                    ),
+                ),
+                ("date_added", models.DateTimeField(default=django.utils.timezone.now)),
+                ("role", models.CharField(default="member", max_length=32)),
+                ("email", models.EmailField(blank=True, max_length=75, null=True)),
+                ("invite_status", models.PositiveSmallIntegerField(default=0, null=True)),
+                (
+                    "inviter",
+                    sentry.db.models.fields.foreignkey.FlexibleForeignKey(
+                        blank=True,
+                        null=True,
+                        on_delete=django.db.models.deletion.SET_NULL,
+                        related_name="inviter_orgmembermapping_set",
+                        to=settings.AUTH_USER_MODEL,
+                    ),
+                ),
+                (
+                    "user",
+                    sentry.db.models.fields.foreignkey.FlexibleForeignKey(
+                        blank=True,
+                        null=True,
+                        on_delete=django.db.models.deletion.CASCADE,
+                        related_name="orgmembermapping_set",
+                        to=settings.AUTH_USER_MODEL,
+                    ),
+                ),
+            ],
+            options={
+                "db_table": "sentry_organizationmembermapping",
+                "unique_together": {("organization_id", "email"), ("organization_id", "user")},
+            },
+        ),
+    ]

+ 1 - 0
src/sentry/models/__init__.py

@@ -69,6 +69,7 @@ from .organization import *  # NOQA
 from .organizationaccessrequest import *  # NOQA
 from .organizationmapping import *  # NOQA
 from .organizationmember import *  # NOQA
+from .organizationmembermapping import *  # NOQA
 from .organizationmemberteam import *  # NOQA
 from .organizationonboardingtask import *  # NOQA
 from .outbox import *  # NOQA

+ 49 - 0
src/sentry/models/organizationmembermapping.py

@@ -0,0 +1,49 @@
+from __future__ import annotations
+
+from django.conf import settings
+from django.db import models
+from django.utils import timezone
+
+from sentry.db.models import FlexibleForeignKey, Model, sane_repr
+from sentry.db.models.base import control_silo_only_model
+from sentry.db.models.fields.hybrid_cloud_foreign_key import HybridCloudForeignKey
+from sentry.models.organizationmember import InviteStatus
+from sentry.roles import organization_roles
+
+
+@control_silo_only_model
+class OrganizationMemberMapping(Model):
+    """
+    This model resides exclusively in the control silo, and will
+    - map a user or an email to a specific organization to indicate an organization membership
+    """
+
+    __include_in_export__ = False
+
+    organization_id = HybridCloudForeignKey("sentry.Organization", on_delete="CASCADE")
+    date_added = models.DateTimeField(default=timezone.now)
+
+    role = models.CharField(max_length=32, default=str(organization_roles.get_default().id))
+    user = FlexibleForeignKey(
+        settings.AUTH_USER_MODEL, null=True, blank=True, related_name="orgmembermapping_set"
+    )
+    email = models.EmailField(null=True, blank=True, max_length=75)
+    inviter = FlexibleForeignKey(
+        settings.AUTH_USER_MODEL,
+        null=True,
+        blank=True,
+        related_name="inviter_orgmembermapping_set",
+        on_delete=models.SET_NULL,
+    )
+    invite_status = models.PositiveSmallIntegerField(
+        choices=InviteStatus.as_choices(),
+        default=InviteStatus.APPROVED.value,
+        null=True,
+    )
+
+    class Meta:
+        app_label = "sentry"
+        db_table = "sentry_organizationmembermapping"
+        unique_together = (("organization_id", "user"), ("organization_id", "email"))
+
+    __repr__ = sane_repr("organization_id", "user_id", "role")

+ 4 - 1
src/sentry/receivers/outbox/region.py

@@ -19,6 +19,9 @@ from sentry.services.hybrid_cloud.organization_mapping import (
     organization_mapping_service,
     update_organization_mapping_from_instance,
 )
+from sentry.services.hybrid_cloud.organizationmember_mapping import (
+    organizationmember_mapping_service,
+)
 
 
 @receiver(process_region_outbox, sender=OutboxCategory.VERIFY_ORGANIZATION_MAPPING)
@@ -55,7 +58,7 @@ def process_organization_member_updates(
             )
         return
 
-    org_member  # TODO: When we get the org member mapping table in place, here is where we'll sync it.
+    organizationmember_mapping_service.create_with_organization_member(org_member=org_member)
 
 
 @receiver(process_region_outbox, sender=OutboxCategory.ORGANIZATION_UPDATE)

+ 91 - 0
src/sentry/services/hybrid_cloud/organizationmember_mapping/__init__.py

@@ -0,0 +1,91 @@
+# Please do not use
+#     from __future__ import annotations
+# in modules such as this one where hybrid cloud service classes and data models are
+# defined, because we want to reflect on type annotations and avoid forward references.
+
+from abc import abstractmethod
+from dataclasses import dataclass, field
+from datetime import datetime
+from typing import Optional, TypedDict
+
+from django.utils import timezone
+
+from sentry.models import OrganizationMember
+from sentry.services.hybrid_cloud import InterfaceWithLifecycle, silo_mode_delegation, stubbed
+from sentry.silo import SiloMode
+
+
+@dataclass(frozen=True, eq=True)
+class RpcOrganizationMemberMapping:
+    organization_id: int = -1
+    date_added: datetime = field(default_factory=timezone.now)
+
+    role: str = ""
+    user_id: Optional[int] = None
+    email: Optional[str] = None
+    inviter_id: Optional[int] = None
+    invite_status: Optional[int] = None
+
+
+class RpcOrganizationMemberMappingUpdate(TypedDict, total=False):
+    """
+    A set of values to be updated on an OrganizationMemberMapping.
+
+    An omitted key indicates that the attribute should not be updated. (Compare to a
+    `"user_id": None` entry, which indicates that `user_id` should be
+    overwritten with a null value.)
+    """
+
+    role: str
+    user_id: Optional[int]
+    email: Optional[str]
+    inviter_id: Optional[int]
+    invite_status: Optional[int]
+
+
+def update_organizationmember_mapping_from_instance(
+    organization_member: OrganizationMember,
+) -> RpcOrganizationMemberMappingUpdate:
+    attributes = {
+        attr_name: getattr(organization_member, attr_name)
+        for attr_name in RpcOrganizationMemberMappingUpdate.__annotations__.keys()
+    }
+    return RpcOrganizationMemberMappingUpdate(**attributes)  # type: ignore
+
+
+class OrganizationMemberMappingService(InterfaceWithLifecycle):
+    @abstractmethod
+    def create_mapping(
+        self,
+        *,
+        organization_id: int,
+        role: str,
+        user_id: Optional[int] = None,
+        email: Optional[str] = None,
+        inviter_id: Optional[int] = None,
+        invite_status: Optional[int] = None,
+    ) -> RpcOrganizationMemberMapping:
+        pass
+
+    @abstractmethod
+    def create_with_organization_member(
+        self, org_member: OrganizationMember
+    ) -> RpcOrganizationMemberMapping:
+        pass
+
+
+def impl_with_db() -> OrganizationMemberMappingService:
+    from sentry.services.hybrid_cloud.organizationmember_mapping.impl import (
+        DatabaseBackedOrganizationMemberMappingService,
+    )
+
+    return DatabaseBackedOrganizationMemberMappingService()
+
+
+organizationmember_mapping_service: OrganizationMemberMappingService = silo_mode_delegation(
+    {
+        SiloMode.MONOLITH: impl_with_db,
+        SiloMode.REGION: stubbed(impl_with_db, SiloMode.CONTROL),
+        SiloMode.CONTROL: impl_with_db,
+    }
+)

+ 69 - 0
src/sentry/services/hybrid_cloud/organizationmember_mapping/impl.py

@@ -0,0 +1,69 @@
+# Please do not use
+#     from __future__ import annotations
+# in modules such as this one where hybrid cloud service classes and data models are
+# defined, because we want to reflect on type annotations and avoid forward references.
+
+from dataclasses import fields
+from typing import Optional
+
+from django.db import transaction
+
+from sentry.models.organizationmember import OrganizationMember
+from sentry.models.organizationmembermapping import OrganizationMemberMapping
+from sentry.services.hybrid_cloud.organizationmember_mapping import (
+    OrganizationMemberMappingService,
+    RpcOrganizationMemberMapping,
+)
+
+
+class DatabaseBackedOrganizationMemberMappingService(OrganizationMemberMappingService):
+    def create_mapping(
+        self,
+        *,
+        organization_id: int,
+        role: str,
+        user_id: Optional[int] = None,
+        email: Optional[str] = None,
+        inviter_id: Optional[int] = None,
+        invite_status: Optional[int] = None,
+    ) -> RpcOrganizationMemberMapping:
+        assert (user_id is None and email) or (
+            user_id and email is None
+        ), "Must set either user or email"
+        with transaction.atomic():
+            org_member_mapping, _created = OrganizationMemberMapping.objects.update_or_create(
+                organization_id=organization_id,
+                user_id=user_id,
+                email=email,
+                defaults={
+                    "role": role,
+                    "inviter_id": inviter_id,
+                    "invite_status": invite_status,
+                },
+            )
+        return self._serialize_rpc(org_member_mapping)
+
+    def create_with_organization_member(
+        self, org_member: OrganizationMember
+    ) -> RpcOrganizationMemberMapping:
+        return self.create_mapping(
+            organization_id=org_member.organization_id,
+            role=org_member.role,
+            user_id=org_member.user_id,
+            email=org_member.email,
+            inviter_id=org_member.inviter_id,
+            invite_status=org_member.invite_status,
+        )
+
+    def close(self) -> None:
+        pass
+
+    def _serialize_rpc(
+        self, org_member_mapping: OrganizationMemberMapping
+    ) -> RpcOrganizationMemberMapping:
+        args = {
+            field.name: getattr(org_member_mapping, field.name)
+            for field in fields(RpcOrganizationMemberMapping)
+            if hasattr(org_member_mapping, field.name)
+        }
+        return RpcOrganizationMemberMapping(**args)

+ 153 - 0
tests/sentry/hybrid_cloud/test_organizationmembermapping.py

@@ -0,0 +1,153 @@
+from sentry.models.organizationmember import InviteStatus, OrganizationMember
+from sentry.models.organizationmembermapping import OrganizationMemberMapping
+from sentry.services.hybrid_cloud.organizationmember_mapping import (
+    organizationmember_mapping_service,
+)
+from sentry.testutils import TransactionTestCase
+from sentry.testutils.silo import control_silo_test, exempt_from_silo_limits, region_silo_test
+
+
+@control_silo_test(stable=True)
+class OrganizationMappingTest(TransactionTestCase):
+    def test_create_mapping(self):
+        with exempt_from_silo_limits():
+            inviter = self.create_user("foo@example.com")
+        fields = {
+            "organization_id": self.organization.id,
+            "role": "member",
+            "user_id": self.user.id,
+            "inviter_id": inviter.id,
+            "invite_status": InviteStatus.REQUESTED_TO_JOIN.value,
+        }
+        rpc_orgmember_mapping = organizationmember_mapping_service.create_mapping(**fields)
+        orgmember_mapping = OrganizationMemberMapping.objects.get(
+            organization_id=self.organization.id
+        )
+
+        assert rpc_orgmember_mapping.date_added == orgmember_mapping.date_added
+        assert (
+            rpc_orgmember_mapping.organization_id
+            == orgmember_mapping.organization_id
+            == self.organization.id
+        )
+        assert rpc_orgmember_mapping.role == orgmember_mapping.role == "member"
+        assert rpc_orgmember_mapping.user_id == orgmember_mapping.user_id == self.user.id
+        assert rpc_orgmember_mapping.email is orgmember_mapping.email is None
+        assert rpc_orgmember_mapping.inviter_id == orgmember_mapping.inviter_id == inviter.id
+        assert (
+            rpc_orgmember_mapping.invite_status
+            == orgmember_mapping.invite_status
+            == fields["invite_status"]
+        )
+
+    def test_create_with_org_member(self):
+        with exempt_from_silo_limits():
+            inviter = self.create_user("foo@example.com")
+        fields = {
+            "organization_id": self.organization.id,
+            "role": "member",
+            "email": "mail@testserver.com",
+            "inviter_id": inviter.id,
+            "invite_status": InviteStatus.REQUESTED_TO_JOIN.value,
+        }
+        with exempt_from_silo_limits():
+            org_member = OrganizationMember.objects.create(**fields)
+        rpc_orgmember_mapping = organizationmember_mapping_service.create_with_organization_member(
+            org_member
+        )
+        orgmember_mapping = OrganizationMemberMapping.objects.get(
+            organization_id=self.organization.id
+        )
+
+        assert rpc_orgmember_mapping.date_added == orgmember_mapping.date_added
+        assert (
+            rpc_orgmember_mapping.organization_id
+            == orgmember_mapping.organization_id
+            == self.organization.id
+        )
+        assert rpc_orgmember_mapping.role == orgmember_mapping.role == "member"
+        assert rpc_orgmember_mapping.user_id is orgmember_mapping.user_id is None
+        assert rpc_orgmember_mapping.email == orgmember_mapping.email == fields["email"]
+        assert rpc_orgmember_mapping.inviter_id == orgmember_mapping.inviter_id == inviter.id
+        assert (
+            rpc_orgmember_mapping.invite_status
+            == orgmember_mapping.invite_status
+            == fields["invite_status"]
+        )
+
+    def test_create_is_idempotent(self):
+        with exempt_from_silo_limits():
+            inviter = self.create_user("foo@example.com")
+        fields = {
+            "organization_id": self.organization.id,
+            "role": "member",
+            "email": "mail@testserver.com",
+            "inviter_id": inviter.id,
+            "invite_status": InviteStatus.REQUESTED_TO_JOIN.value,
+        }
+        organizationmember_mapping_service.create_mapping(**fields)
+        assert (
+            OrganizationMemberMapping.objects.filter(
+                organization_id=self.organization.id,
+                user_id=None,
+                email="mail@testserver.com",
+                role="member",
+            ).count()
+            == 1
+        )
+
+        next_role = "billing"
+        rpc_orgmember_mapping = organizationmember_mapping_service.create_mapping(
+            **{
+                **fields,
+                "role": next_role,
+            }
+        )
+
+        assert not OrganizationMemberMapping.objects.filter(
+            organization_id=self.organization.id, email="mail@testserver.com", role="member"
+        ).exists()
+        orgmember_mapping = OrganizationMemberMapping.objects.get(
+            organization_id=self.organization.id, email="mail@testserver.com"
+        )
+
+        assert rpc_orgmember_mapping.date_added == orgmember_mapping.date_added
+        assert (
+            rpc_orgmember_mapping.organization_id
+            == orgmember_mapping.organization_id
+            == self.organization.id
+        )
+        assert rpc_orgmember_mapping.role == orgmember_mapping.role == next_role
+        assert rpc_orgmember_mapping.user_id is orgmember_mapping.user_id is None
+        assert rpc_orgmember_mapping.email == orgmember_mapping.email == fields["email"]
+        assert rpc_orgmember_mapping.inviter_id == orgmember_mapping.inviter_id == inviter.id
+        assert (
+            rpc_orgmember_mapping.invite_status
+            == orgmember_mapping.invite_status
+            == fields["invite_status"]
+        )
+
+
+@region_silo_test(stable=True)
+class ReceiverTest(TransactionTestCase):
+    def test_process_organization_member_updates_receiver(self):
+        with exempt_from_silo_limits():
+            inviter = self.create_user("foo@example.com")
+            assert OrganizationMemberMapping.objects.all().count() == 0
+        fields = {
+            "organization_id": self.organization.id,
+            "role": "member",
+            "email": "mail@testserver.com",
+            "inviter_id": inviter.id,
+            "invite_status": InviteStatus.REQUESTED_TO_JOIN.value,
+        }
+        org_member = OrganizationMember.objects.create(**fields)
+        region_outbox = org_member.outbox_for_update()
+        region_outbox.save()
+        region_outbox.drain_shard()
+
+        with exempt_from_silo_limits():
+            assert OrganizationMemberMapping.objects.all().count() == 1
+            OrganizationMemberMapping.objects.filter(
+                organization_id=self.organization.id, email=fields["email"]
+            ).exists()