Skip to content

Commit e064033

Browse files
authored
fix(codeowners): prevent duplicate ExternalActor creation for case-insensitive integrations (#98667)
GitHub teams and users in CODEOWNER files are case-insensitive — however, we treat them as case-sensitive, and thus allow for multiple ExternalActor objects that actually map to the same GitHub team: <img width="1182" height="401" alt="Screenshot 2025-09-02 at 3 38 29 PM" src="https://github.com/user-attachments/assets/f139834f-b4fa-468e-8f56-1005fe63f1a5" /> in this case, @charlieluo/test and @charlieluo/Test are duplicates, and will map to the same GitHub team. This prevents the creation of these duplicates, behind a feature flag. Next step (#98668) is to read CODEOWNERs case-insensitively, so that users can create @charlieluo/TEST or @charlieluo/Test ExternalActors, and they will both correctly map to the GitHub team @charlieluo/test — feature flag will be used to sync these changes together, so that we go from all case-sensitive to all case-insensitive behavior together. See https://linear.app/getsentry/issue/ID-97/github-code-owner-teams-should-be-case-insensitive and #79296
1 parent fa749b8 commit e064033

File tree

3 files changed

+88
-0
lines changed

3 files changed

+88
-0
lines changed

src/sentry/features/temporary.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -475,6 +475,8 @@ def register_temporary_features(manager: FeatureManager) -> None:
475475
manager.add("organizations:uptime-eap-results", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=False)
476476
# Enable querying uptime data from EAP uptime_results instead of uptime_checks
477477
manager.add("organizations:uptime-eap-uptime-results-query", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=True)
478+
# Enable case-insensitive codeowners team matching
479+
manager.add("organizations:use-case-insensitive-codeowners", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=True)
478480
manager.add("organizations:use-metrics-layer", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=False)
479481
manager.add("organizations:user-feedback-ai-summaries", OrganizationFeature, FeatureHandlerStrategy.FLAGPOLE, api_expose=True)
480482
# Enable label generation at ingest time for user feedbacks

src/sentry/integrations/api/bases/external_actor.py

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,12 @@
4242
ExternalProviders.GITLAB,
4343
}
4444

45+
CASE_INSENSITIVE_PROVIDERS = {
46+
ExternalProviders.GITHUB,
47+
ExternalProviders.GITHUB_ENTERPRISE,
48+
ExternalProviders.GITLAB,
49+
}
50+
4551

4652
class ExternalActorResponse(TypedDict):
4753
id: int
@@ -95,6 +101,18 @@ def get_actor_params(self, validated_data: MutableMapping[str, Any]) -> Mapping[
95101

96102
def create(self, validated_data: MutableMapping[str, Any]) -> tuple[ExternalActor, bool]:
97103
actor_params = self.get_actor_params(validated_data)
104+
105+
if features.has("organizations:use-case-insensitive-codeowners", self.organization):
106+
if validated_data["provider"] in CASE_INSENSITIVE_PROVIDERS:
107+
external_name = validated_data.pop("external_name")
108+
109+
return ExternalActor.objects.get_or_create(
110+
external_name__iexact=external_name,
111+
**validated_data,
112+
organization=self.organization,
113+
defaults={**actor_params, "external_name": external_name},
114+
)
115+
98116
return ExternalActor.objects.get_or_create(
99117
**validated_data,
100118
organization=self.organization,

tests/sentry/integrations/api/serializers/test_external_actor.py

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
from sentry.integrations.types import ExternalProviders
99
from sentry.integrations.utils.providers import get_provider_name
1010
from sentry.testutils.cases import TestCase
11+
from sentry.testutils.helpers.features import with_feature
1112

1213

1314
class ExternalActorSerializerTest(TestCase):
@@ -145,3 +146,70 @@ def test_avoid_strict_external_name(self) -> None:
145146
context={"organization": self.organization},
146147
)
147148
assert serializer.is_valid() is True
149+
150+
@with_feature("organizations:use-case-insensitive-codeowners")
151+
def test_create_case_insensitive_team(self) -> None:
152+
sentry_team = self.create_team(organization=self.organization, members=[self.user])
153+
154+
external_actor_team_data = {
155+
"provider": get_provider_name(ExternalProviders.GITHUB.value),
156+
"external_name": "@getsentry/example-team",
157+
"integrationId": self.integration.id,
158+
"team_id": sentry_team.id,
159+
}
160+
161+
serializer = ExternalTeamSerializer(
162+
data=external_actor_team_data,
163+
context={"organization": self.organization},
164+
)
165+
assert serializer.is_valid() is True
166+
external_actor1, created1 = serializer.create(serializer.validated_data)
167+
assert created1 is True
168+
assert external_actor1.external_name == "@getsentry/example-team"
169+
170+
# Try to create another with different case but different team - should match existing one
171+
external_actor_team_data["external_name"] = "@GETSENTRY/EXAMPLE-TEAM"
172+
external_actor_team_data["team_id"] = sentry_team.id
173+
174+
serializer = ExternalTeamSerializer(
175+
data=external_actor_team_data,
176+
context={"organization": self.organization},
177+
)
178+
assert serializer.is_valid() is True
179+
external_actor2, created2 = serializer.create(serializer.validated_data)
180+
181+
# We should not have created a new external actor - we should have returned the existing one
182+
assert created2 is False
183+
assert external_actor2.id == external_actor1.id
184+
assert external_actor2.external_name == "@getsentry/example-team"
185+
186+
def test_create_case_sensitive_team(self) -> None:
187+
sentry_team = self.create_team(organization=self.organization, members=[self.user])
188+
189+
external_actor_team_data = {
190+
"provider": get_provider_name(ExternalProviders.GITHUB.value),
191+
"external_name": "@getsentry/example-team",
192+
"integrationId": self.integration.id,
193+
"team_id": sentry_team.id,
194+
}
195+
196+
serializer = ExternalTeamSerializer(
197+
data=external_actor_team_data,
198+
context={"organization": self.organization},
199+
)
200+
assert serializer.is_valid() is True
201+
external_actor1, created1 = serializer.create(serializer.validated_data)
202+
assert created1 is True
203+
assert external_actor1.external_name == "@getsentry/example-team"
204+
205+
external_actor_team_data["external_name"] = "@GETSENTRY/EXAMPLE-TEAM"
206+
external_actor_team_data["team_id"] = sentry_team.id
207+
208+
serializer = ExternalTeamSerializer(
209+
data=external_actor_team_data,
210+
context={"organization": self.organization},
211+
)
212+
assert serializer.is_valid() is True
213+
external_actor2, created2 = serializer.create(serializer.validated_data)
214+
assert created2 is True
215+
assert external_actor2.external_name == "@GETSENTRY/EXAMPLE-TEAM"

0 commit comments

Comments
 (0)