Skip to content
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

chore: improve the rbac teams migration #28307

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+12 −2
Open

chore: improve the rbac teams migration #28307

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
14 changes: 12 additions & 2 deletions posthog/rbac/migrations/rbac_team_migration.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
from ee.models.rbac.access_control import AccessControl
from ee.models.explicit_team_membership import ExplicitTeamMembership
import structlog
from posthog.models.organization import Organization
from posthog.models.organization import Organization, OrganizationMembership
from sentry_sdk import capture_exception

logger = structlog.get_logger(__name__)
Expand Down Expand Up @@ -32,7 +32,15 @@ def rbac_team_access_control_migration(organization_id: int):
)

# Get all members for the project
team_memberships = ExplicitTeamMembership.objects.filter(team_id=team.id)
team_memberships = ExplicitTeamMembership.objects.filter(
team_id=team.id,
# Make sure the user is active
parent_membership__user__is_active=True,
# Make sure their organization membership exists
parent_membership__organization__isnull=False,
# Only migrate members that are not admins in the organization
parent_membership__level__lt=OrganizationMembership.Level.ADMIN,
)
for team_membership in team_memberships:
try:
# Create access control for the team member
Expand Down Expand Up @@ -70,6 +78,8 @@ def rbac_team_access_control_migration(organization_id: int):
# Disable access control for the team (so we know it's been migrated)
team.access_control = False
team.save()
# Delete any left over explicit team memberships
ExplicitTeamMembership.objects.filter(team_id=team.id).delete()
Comment on lines +81 to +82
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: deleting leftover memberships could delete memberships that were skipped intentionally (e.g. org admins)

Suggested change
# Delete any left over explicit team memberships
ExplicitTeamMembership.objects.filter(team_id=team.id).delete()
# Delete migrated explicit team memberships
ExplicitTeamMembership.objects.filter(
team_id=team.id,
parent_membership__level__lt=OrganizationMembership.Level.ADMIN
).delete()

except Exception as e:
error_message = f"Failed to migrate team {team.id}"
logger.exception(error_message, exc_info=e)
Expand Down
Loading