Skip to content

feat(cells): Compare locality not cell name in fork locality restriction #112883

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.

Sign up for GitHub

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

Open
lynnagara wants to merge 1 commit into
base: master
Choose a base branch
from
+27 −20
Open

feat(cells): Compare locality not cell name in fork locality restriction #112883

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
38 changes: 22 additions & 16 deletions src/sentry/api/endpoints/organization_fork.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
)
from sentry.relocation.models.relocation import Relocation
from sentry.relocation.tasks.process import uploading_start
from sentry.types.cell import get_local_cell
from sentry.types.cell import get_global_directory, get_local_cell
from sentry.utils.db import atomic_transaction

ERR_DUPLICATE_ORGANIZATION_FORK = Template(
Expand All @@ -35,12 +35,12 @@
ERR_CANNOT_FORK_INTO_SAME_REGION = Template(
"The organization already lives in region `$region`, so it cannot be forked into that region."
)
ERR_CANNOT_FORK_FROM_REGION = Template(
"Forking an organization from region `$region` is forbidden."
ERR_CANNOT_FORK_FROM_LOCALITY = Template(
"Forking an organization from locality `$locality` is forbidden."
)

# For legal reasons, there are certain regions from which forking is disallowed.
CANNOT_FORK_FROM_REGION = {"de"}
# For legal reasons, there are certain localities from which forking is disallowed.
CANNOT_FORK_FROM_LOCALITY = {"de"}

logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -99,23 +99,29 @@ def post(self, request: Request, organization_id_or_slug) -> Response:
status=status.HTTP_400_BAD_REQUEST,
)

# Figure out which region the organization being forked lives in.
requesting_region_name = get_local_cell().name
replying_region_name = org_mapping.cell_name
if replying_region_name in CANNOT_FORK_FROM_REGION:
# Figure out which cell the organization being forked lives in.
requesting_cell_name = get_local_cell().name
replying_cell_name = org_mapping.cell_name

# Resolve the locality for the exporting cell. In environments without locality
# config (e.g. monolith, tests), falls back to the cell name.
replying_locality = get_global_directory().get_locality_for_cell(replying_cell_name)
replying_locality_name = replying_locality.name if replying_locality else replying_cell_name

if replying_locality_name in CANNOT_FORK_FROM_LOCALITY:
return Response(
{
"detail": ERR_CANNOT_FORK_FROM_REGION.substitute(
region=replying_region_name,
"detail": ERR_CANNOT_FORK_FROM_LOCALITY.substitute(
locality=replying_locality_name,
)
},
status=status.HTTP_403_FORBIDDEN,
)
if replying_region_name == requesting_region_name:
if replying_cell_name == requesting_cell_name:
return Response(
{
"detail": ERR_CANNOT_FORK_INTO_SAME_REGION.substitute(
region=requesting_region_name,
region=requesting_cell_name,
)
},
status=status.HTTP_400_BAD_REQUEST,
Expand Down Expand Up @@ -164,7 +170,7 @@ def post(self, request: Request, organization_id_or_slug) -> Response:
# When we received this back (via RPC call), we'll be able to continue with the usual
# relocation flow, picking up from the `uploading_complete` task.
uploading_start.apply_async(
args=[new_relocation.uuid, replying_region_name, org_mapping.slug]
args=[new_relocation.uuid, replying_cell_name, org_mapping.slug]
)

try:
Expand All @@ -174,8 +180,8 @@ def post(self, request: Request, organization_id_or_slug) -> Response:
owner_id=owner.id,
uuid=str(new_relocation.uuid),
from_org_slug=org_mapping.slug,
requesting_region_name=requesting_region_name,
replying_region_name=replying_region_name,
requesting_region_name=requesting_cell_name,
replying_region_name=replying_cell_name,
)
)
except Exception as e:
Expand Down
9 changes: 5 additions & 4 deletions tests/sentry/api/endpoints/test_organization_fork.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

from sentry.analytics.events.relocation_forked import RelocationForkedEvent
from sentry.api.endpoints.organization_fork import (
ERR_CANNOT_FORK_FROM_REGION,
ERR_CANNOT_FORK_FROM_LOCALITY,
ERR_CANNOT_FORK_INTO_SAME_REGION,
ERR_DUPLICATE_ORGANIZATION_FORK,
ERR_ORGANIZATION_INACTIVE,
Expand Down Expand Up @@ -394,7 +394,8 @@ def test_bad_cannot_fork_deleted_organization(
@override_options({"relocation.enabled": True, "relocation.daily-limit.small": 1})
@assume_test_silo_mode(SiloMode.CELL, cell_name=REQUESTING_TEST_REGION)
@patch(
"sentry.api.endpoints.organization_fork.CANNOT_FORK_FROM_REGION", {EXPORTING_TEST_REGION}
"sentry.api.endpoints.organization_fork.CANNOT_FORK_FROM_LOCALITY",
{EXPORTING_TEST_REGION},
)
def test_bad_organization_in_forbidden_region(
self,
Expand All @@ -408,8 +409,8 @@ def test_bad_organization_in_forbidden_region(
response = self.get_error_response(self.existing_org.slug, status_code=403)

assert response.data.get("detail") is not None
assert response.data.get("detail") == ERR_CANNOT_FORK_FROM_REGION.substitute(
region=EXPORTING_TEST_REGION,
assert response.data.get("detail") == ERR_CANNOT_FORK_FROM_LOCALITY.substitute(
locality=EXPORTING_TEST_REGION,
)
assert uploading_start_mock.call_count == 0
assert analytics_record_mock.call_count == 0
Expand Down
Loading