-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
chore(launchpad): Return artifact URL upon creation #97558
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
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #97558 +/- ##
==========================================
+ Coverage 80.26% 80.64% +0.37%
==========================================
Files 8561 8562 +1
Lines 377137 377168 +31
Branches 24548 24548
==========================================
+ Hits 302698 304153 +1455
+ Misses 74069 72645 -1424
Partials 370 370 |
b49268f
to
1bd623d
Compare
683a008
to
09e6e83
Compare
""" | ||
organization: Organization = Organization.objects.get_from_cache(id=organization_id) | ||
|
||
path = f"/organizations/{organization.slug}/preprod/internal/{artifact_id}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Cross-Silo Query Violation in URL Construction
The get_preprod_artifact_url
function queries the Organization
model (Organization.objects.get_from_cache
) from a region-silo endpoint
to build a URL (using organization.slug
and absolute_url
). As Organization
is a control-silo model
, this violates silo boundaries, risking cross-silo database access errors (e.g., SiloLimitError
) in multi-silo deployments. The URL should instead be constructed using data already available (e.g., project.organization
) or a silo-safe service/mapping (e.g., OrganizationMapping
or an organization_service
) to avoid querying the control database from the region.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Instead of just returning the artifact id, lets return the full URL
Instead of just returning the artifact id, lets return the full URL