Skip to content

fix(relocation) Avoid integrity errors on self-hosted import#118579

Open
markstory wants to merge 2 commits into
masterfrom
fix-5njv
Open

fix(relocation) Avoid integrity errors on self-hosted import#118579
markstory wants to merge 2 commits into
masterfrom
fix-5njv

Conversation

@markstory

Copy link
Copy Markdown
Member

When a self-hosted instance is imported, if that import data has users with email_unique defined, and that email address also has an account in saas, the import will fail because of an integrity error on email_unique.

We cannot trust data in self-hosted imports so we don't want to merge users (like we do with a saas to saas relocation). By blanking the incoming user's email_unique value we can create a duplicate user record that the customer will have to de-deduplicate on their own.

I briefly considered generating a random suffix for the email address but this could also conflict with existing data, or conflict with legitimate customer data in the future. This felt like the least disruptive and risky change to make.

Fixes SENTRY-5NJV

When a self-hosted instance is imported, if that import data has users
with `email_unique` defined, and that email address also has an account
in saas, the import will fail because of an integrity error on
`email_unique`.

We cannot trust data in self-hosted imports so we don't want to merge
users (like we do with a saas to saas relocation). By blanking the
incoming user's `email_unique` value we can create a duplicate user
record that the customer will have to de-deduplicate on their own.

I briefly considered generating a random suffix for the email address
but this could also conflict with existing data, or conflict with
legitimate customer data in the future. This felt like the least
disruptive and risky change to make.

Fixes SENTRY-5NJV
@markstory markstory requested review from a team as code owners June 26, 2026 18:20
@github-actions github-actions Bot added the Scope: Backend Automatically applied to PRs that change backend components label Jun 26, 2026
Comment thread tests/sentry/backup/test_imports.py Outdated
@markstory markstory requested a review from a team June 26, 2026 18:40

@strongs strongs left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good and I agree with the approach (better than the suffix strategy imo). Would only be worried if there were downstream concerns with the email field being non-unique but the email_unique field heavily implies it's fine

if self.pk is None and not is_relocated_user:
# new users should set email_unique
self.email_unique = self.email
elif self.pk is None and is_relocated_user and self.email_unique:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to verify, we're guaranteed that relocated users never have a set PK ? never need to update a relocated user during the migration for any reason?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Backend Automatically applied to PRs that change backend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants