Skip to content

fix(hc): Serialize the organization via RPC to json #51078

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

Merged
merged 4 commits into from
Jun 15, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions src/sentry/integrations/aws_lambda/integration.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,12 @@
IntegrationProvider,
)
from sentry.integrations.mixins import ServerlessMixin
from sentry.models import Integration, OrganizationIntegration, Project
from sentry.models import Integration, OrganizationIntegration, Project, User
from sentry.pipeline import PipelineView
from sentry.services.hybrid_cloud.organization import RpcOrganizationSummary
from sentry.services.hybrid_cloud.organization import RpcOrganizationSummary, organization_service
from sentry.utils.sdk import capture_exception

from ...services.hybrid_cloud.user.serial import serialize_rpc_user
from .client import ConfigurationError, gen_aws_client
from .utils import (
ALL_AWS_REGIONS,
Expand Down Expand Up @@ -270,7 +271,12 @@ def dispatch(self, request: Request, pipeline) -> Response:
curr_step = 0 if pipeline.fetch_state("skipped_project_select") else 1

def render_response(error=None):
serialized_organization = serialize(pipeline.organization, request.user)
serialized_organization = organization_service.serialize_organization(
id=pipeline.organization.id,
as_user=serialize_rpc_user(request.user)
if isinstance(request.user, User)
else None,
)
template_url = options.get("aws-lambda.cloudformation-url")
context = {
"baseCloudformationUrl": "https://console.aws.amazon.com/cloudformation/home#/stacks/create/review",
Expand Down
4 changes: 2 additions & 2 deletions src/sentry/pipeline/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
from sentry.web.helpers import render_to_response

from ..models import Organization
from ..services.hybrid_cloud.organization.serial import serialize_organization
from ..services.hybrid_cloud.organization.serial import serialize_rpc_organization
from . import PipelineProvider
from .constants import PIPELINE_STATE_TTL
from .store import PipelineSessionStore
Expand Down Expand Up @@ -104,7 +104,7 @@ def __init__(
) -> None:
self.request = request
self.organization: RpcOrganization | None = (
serialize_organization(organization)
serialize_rpc_organization(organization)
if isinstance(organization, Organization)
else organization
)
Expand Down
4 changes: 2 additions & 2 deletions src/sentry/rules/actions/notify_event_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
from sentry.rules.base import CallbackFuture
from sentry.services.hybrid_cloud.app import RpcSentryAppService, app_service
from sentry.services.hybrid_cloud.integration import integration_service
from sentry.services.hybrid_cloud.organization.serial import serialize_organization
from sentry.services.hybrid_cloud.organization.serial import serialize_rpc_organization
from sentry.tasks.sentry_apps import notify_sentry_app
from sentry.utils import metrics
from sentry.utils.safe import safe_execute
Expand Down Expand Up @@ -63,7 +63,7 @@ def send_incident_alert_notification(
fire. If not provided we'll attempt to calculate this ourselves.
:return:
"""
organization = serialize_organization(incident.organization)
organization = serialize_rpc_organization(incident.organization)
incident_attachment = build_incident_attachment(incident, new_status, metric_value)

integration_service.send_incident_alert_notification(
Expand Down
20 changes: 15 additions & 5 deletions src/sentry/services/hybrid_cloud/organization/impl.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
from __future__ import annotations

from typing import Iterable, List, Optional, Set, cast
from typing import Any, Iterable, List, Optional, Set, cast

from django.db import IntegrityError, models, transaction

from sentry import roles
from sentry.api.serializers import serialize
from sentry.db.postgres.roles import in_test_psql_role_override
from sentry.models import (
Activity,
Expand Down Expand Up @@ -34,9 +35,10 @@
)
from sentry.services.hybrid_cloud.organization.serial import (
serialize_member,
serialize_organization,
serialize_organization_summary,
serialize_rpc_organization,
)
from sentry.services.hybrid_cloud.user import RpcUser
from sentry.services.hybrid_cloud.util import flags_to_bits


Expand All @@ -55,6 +57,14 @@ def check_membership_by_id(

return serialize_member(member)

def serialize_organization(
self, *, id: int, as_user: Optional[RpcUser] = None
) -> Optional[Any]:
org = Organization.objects.filter(id=id).first()
if org is None:
return None
return serialize(org, user=as_user)

def get_organization_by_id(
self, *, id: int, user_id: Optional[int] = None, slug: Optional[str] = None
) -> Optional[RpcUserOrganizationContext]:
Expand All @@ -71,7 +81,7 @@ def get_organization_by_id(
return None

return RpcUserOrganizationContext(
user_id=user_id, organization=serialize_organization(org), member=membership
user_id=user_id, organization=serialize_rpc_organization(org), member=membership
)

def get_org_by_slug(
Expand Down Expand Up @@ -184,7 +194,7 @@ def _get_invite(

return RpcUserInviteContext(
user_id=member.user_id,
organization=serialize_organization(org),
organization=serialize_rpc_organization(org),
member=serialize_member(member),
invite_organization_member_id=organization_member_id,
)
Expand Down Expand Up @@ -400,7 +410,7 @@ def update_default_role(
org = Organization.objects.get(id=organization_id)
org.default_role = default_role
org.save()
return serialize_organization(org)
return serialize_rpc_organization(org)

def remove_user(self, *, organization_id: int, user_id: int) -> RpcOrganizationMember:
with transaction.atomic(), in_test_psql_role_override("postgres"):
Expand Down
2 changes: 1 addition & 1 deletion src/sentry/services/hybrid_cloud/organization/serial.py
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ def serialize_organization_summary(org: Organization) -> RpcOrganizationSummary:
)


def serialize_organization(org: Organization) -> RpcOrganization:
def serialize_rpc_organization(org: Organization) -> RpcOrganization:
rpc_org: RpcOrganization = RpcOrganization(
slug=org.slug,
id=org.id,
Expand Down
17 changes: 16 additions & 1 deletion src/sentry/services/hybrid_cloud/organization/service.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
# defined, because we want to reflect on type annotations and avoid forward references.

from abc import abstractmethod
from typing import Iterable, List, Optional, cast
from typing import Any, Iterable, List, Optional, cast

from sentry.services.hybrid_cloud import OptionValue
from sentry.services.hybrid_cloud.organization import (
Expand All @@ -25,6 +25,7 @@
UnimplementedRegionResolution,
)
from sentry.services.hybrid_cloud.rpc import RpcService, regional_rpc_method
from sentry.services.hybrid_cloud.user import RpcUser
from sentry.silo import SiloMode


Expand All @@ -42,6 +43,20 @@ def get(self, id: int) -> Optional[RpcOrganization]:
org_context = self.get_organization_by_id(id=id)
return org_context.organization if org_context else None

@regional_rpc_method(resolve=ByOrganizationId("id"))
@abstractmethod
def serialize_organization(
self,
*,
id: int,
as_user: Optional[RpcUser] = None,
) -> Optional[Any]:
"""
Attempts to serialize a given organization. Note that this can be None if the organization is already deleted
in the corresponding region silo.
"""
pass

@regional_rpc_method(resolve=ByOrganizationId("id"))
@abstractmethod
def get_organization_by_id(
Expand Down
4 changes: 2 additions & 2 deletions src/sentry/testutils/cases.py
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@
from sentry.utils.snuba import _snuba_pool

from ..services.hybrid_cloud.actor import RpcActor
from ..services.hybrid_cloud.organization.serial import serialize_organization
from ..services.hybrid_cloud.organization.serial import serialize_rpc_organization
from ..snuba.metrics import (
MetricConditionField,
MetricField,
Expand Down Expand Up @@ -977,7 +977,7 @@ def setUp(self):

self.organization = self.create_organization(name="foo", owner=self.user)
with exempt_from_silo_limits():
rpc_organization = serialize_organization(self.organization)
rpc_organization = serialize_rpc_organization(self.organization)

self.login_as(self.user)
self.request = self.make_request(self.user)
Expand Down
8 changes: 4 additions & 4 deletions tests/sentry/auth/test_helper.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
OrganizationMember,
UserEmail,
)
from sentry.services.hybrid_cloud.organization.serial import serialize_organization
from sentry.services.hybrid_cloud.organization.serial import serialize_rpc_organization
from sentry.testutils import TestCase
from sentry.testutils.hybrid_cloud import HybridCloudTestMixin
from sentry.testutils.silo import control_silo_test, exempt_from_silo_limits
Expand Down Expand Up @@ -59,7 +59,7 @@ def handler(self):

def _handler_with(self, identity):
with exempt_from_silo_limits():
rpc_organization = serialize_organization(self.organization)
rpc_organization = serialize_rpc_organization(self.organization)
return AuthIdentityHandler(
self.auth_provider,
DummyProvider(self.provider),
Expand Down Expand Up @@ -212,7 +212,7 @@ def test_no_invite_members_flag(self, mock_auth):
assert getattr(persisted_om.flags, "sso:linked")
assert getattr(persisted_om.flags, "member-limit:restricted")
assert not getattr(persisted_om.flags, "sso:invalid")
expected_rpc_org = serialize_organization(self.organization)
expected_rpc_org = serialize_rpc_organization(self.organization)
features_has.assert_any_call("organizations:invite-members", expected_rpc_org)
self.assert_org_member_mapping(org_member=persisted_om)

Expand Down Expand Up @@ -335,7 +335,7 @@ def _test_simple(self, mock_render, expected_template):
assert request is self.request
assert status == 200

expected_org = serialize_organization(self.organization)
expected_org = serialize_rpc_organization(self.organization)

assert context["organization"] == expected_org
assert context["identity"] == self.identity
Expand Down
8 changes: 4 additions & 4 deletions tests/sentry/hybrid_cloud/test_rpc.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
RpcOrganizationMemberFlags,
RpcUserOrganizationContext,
)
from sentry.services.hybrid_cloud.organization.serial import serialize_organization
from sentry.services.hybrid_cloud.organization.serial import serialize_rpc_organization
from sentry.services.hybrid_cloud.rpc import (
RpcSendException,
dispatch_remote_call,
Expand Down Expand Up @@ -55,7 +55,7 @@ def test_remote_service(self, mock_dispatch_remote_call):
)

serial_user = RpcUser(id=user.id)
serial_org = serialize_organization(organization)
serial_org = serialize_rpc_organization(organization)

service = OrganizationService.create_delegation()
with override_regions(_REGIONS), override_settings(SILO_MODE=SiloMode.CONTROL):
Expand Down Expand Up @@ -109,7 +109,7 @@ def test_dispatch_to_local_service(self):
user = self.create_user()
organization = self.create_organization()

serial_org = serialize_organization(organization)
serial_org = serialize_rpc_organization(organization)
serial_arguments = dict(
organization_id=serial_org.id,
default_org_role=serial_org.default_role,
Expand Down Expand Up @@ -158,7 +158,7 @@ def _set_up_mock_response(mock_urlopen, response_value):
@mock.patch("sentry.services.hybrid_cloud.rpc.urlopen")
def test_region_to_control_happy_path(self, mock_urlopen):
org = self.create_organization()
response_value = RpcUserOrganizationContext(organization=serialize_organization(org))
response_value = RpcUserOrganizationContext(organization=serialize_rpc_organization(org))
self._set_up_mock_response(mock_urlopen, response_value.dict())

result = dispatch_remote_call(
Expand Down
15 changes: 9 additions & 6 deletions tests/sentry/integrations/aws_lambda/test_integration.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
from sentry.integrations.aws_lambda.utils import ALL_AWS_REGIONS
from sentry.models import Integration, OrganizationIntegration, ProjectKey
from sentry.pipeline import PipelineView
from sentry.services.hybrid_cloud.organization import organization_service
from sentry.services.hybrid_cloud.user.serial import serialize_rpc_user
from sentry.testutils import IntegrationTestCase
from sentry.testutils.helpers.faux import Mock
from sentry.testutils.silo import control_silo_test
Expand Down Expand Up @@ -51,8 +53,6 @@ def test_one_project(self, mock_react_view):

@patch.object(PipelineView, "render_react_view", return_value=HttpResponse())
def test_render_cloudformation_view(self, mock_react_view):
from sentry.services.hybrid_cloud.organization.serial import serialize_organization

self.pipeline.state.step_index = 1
resp = self.client.get(self.setup_path)
assert resp.status_code == 200
Expand All @@ -68,7 +68,9 @@ def test_render_cloudformation_view(self, mock_react_view):
"accountNumber": None,
"error": None,
"initialStepNumber": 1,
"organization": serialize_organization(self.organization),
"organization": organization_service.serialize_organization(
id=self.organization.id, as_user=serialize_rpc_user(self.user)
),
"awsExternalId": None,
},
)
Expand All @@ -85,8 +87,6 @@ def test_set_valid_arn(self, mock_react_view, mock_gen_aws_client):
@patch("sentry.integrations.aws_lambda.integration.gen_aws_client")
@patch.object(PipelineView, "render_react_view", return_value=HttpResponse())
def test_set_arn_with_error(self, mock_react_view, mock_gen_aws_client):
from sentry.services.hybrid_cloud.organization.serial import serialize_organization

self.pipeline.state.step_index = 1
mock_gen_aws_client.side_effect = ClientError({"Error": {}}, "assume_role")
data = {"region": region, "accountNumber": account_number, "awsExternalId": "my-id"}
Expand All @@ -95,6 +95,7 @@ def test_set_arn_with_error(self, mock_react_view, mock_gen_aws_client):
mock_react_view.assert_called_with(
ANY,
"awsLambdaCloudformation",
# Ensure that the expected value passes through json serialization
{
"baseCloudformationUrl": "https://console.aws.amazon.com/cloudformation/home#/stacks/create/review",
"templateUrl": "https://example.com/file.json",
Expand All @@ -104,7 +105,9 @@ def test_set_arn_with_error(self, mock_react_view, mock_gen_aws_client):
"accountNumber": account_number,
"error": "Please validate the Cloudformation stack was created successfully",
"initialStepNumber": 1,
"organization": serialize_organization(self.organization),
"organization": organization_service.serialize_organization(
id=self.organization.id, as_user=serialize_rpc_user(self.user)
),
"awsExternalId": "my-id",
},
)
Expand Down
6 changes: 3 additions & 3 deletions tests/sentry/integrations/test_migrate.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
from sentry.models import Integration, Repository
from sentry.plugins.base import plugins
from sentry.plugins.bases.issue2 import IssuePlugin2
from sentry.services.hybrid_cloud.organization.serial import serialize_organization
from sentry.services.hybrid_cloud.organization.serial import serialize_rpc_organization
from sentry.testutils import TestCase


Expand All @@ -24,7 +24,7 @@ def setUp(self):
self.integration = Integration.objects.create(provider=ExampleIntegrationProvider.key)

self.migrator = Migrator(
integration=self.integration, organization=serialize_organization(self.organization)
integration=self.integration, organization=serialize_rpc_organization(self.organization)
)

def test_all_repos_migrated(self):
Expand Down Expand Up @@ -62,5 +62,5 @@ def test_does_not_disable_any_plugin(self):

def test_logs(self):
Migrator.run(
integration=self.integration, organization=serialize_organization(self.organization)
integration=self.integration, organization=serialize_rpc_organization(self.organization)
)
4 changes: 2 additions & 2 deletions tests/sentry/web/frontend/test_auth_organization_login.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
OrganizationStatus,
UserEmail,
)
from sentry.services.hybrid_cloud.organization.serial import serialize_organization
from sentry.services.hybrid_cloud.organization.serial import serialize_rpc_organization
from sentry.testutils import AuthProviderTestCase
from sentry.testutils.helpers import with_feature
from sentry.testutils.silo import region_silo_test
Expand All @@ -44,7 +44,7 @@ def test_renders_basic(self):
self.assertTemplateUsed(resp, "sentry/organization-login.html")

assert resp.context["login_form"]
assert resp.context["organization"] == serialize_organization(self.organization)
assert resp.context["organization"] == serialize_rpc_organization(self.organization)
assert "provider_key" not in resp.context
assert resp.context["join_request_link"]

Expand Down