feat: Allow upload requests originating from Shelter to set storage path #172
Conversation
Codecov Report
Changes have been made to critical files, which contain lines commonly executed in production. Learn more @@ Coverage Diff @@
## main #172 +/- ##
=====================================
Coverage 95.51 95.51
=====================================
Files 716 715 -1
Lines 15673 15658 -15
=====================================
- Hits 14970 14955 -15
Misses 703 703
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Codecov ReportAll modified and coverable lines are covered by tests ✅ 📢 Thoughts on this report? Let us know!. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #172 +/- ##
=======================================
Coverage 95.61% 95.61%
=======================================
Files 602 602
Lines 15267 15279 +12
=======================================
+ Hits 14597 14609 +12
Misses 670 670
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Codecov Report
@@ Coverage Diff @@
## main #172 +/- ##
=======================================
Coverage 95.61% 95.61%
=======================================
Files 602 602
Lines 15267 15279 +12
=======================================
+ Hits 14597 14609 +12
Misses 670 670
Flags with carried forward coverage won't be shown. Click here to find out more.
|
scott-codecov
force-pushed
the
scott/upload-path
branch
from
5651ac3
to
0efddab
Compare
| uploads cannot access. | ||
| """ | ||
| shelter_token = self.request.META.get("HTTP_X_SHELTER_TOKEN") | ||
| return shelter_token and shelter_token == settings.SHELTER_SHARED_SECRET |
There was a problem hiding this comment.
Makes sense. Is this a go-to approach when trying to identify specific requests? (for my own learning heh)
There was a problem hiding this comment.
I think shared secrets are a good approach when you're making internal requests between systems (i.e. similar to the Sentry supertoken)
There was a problem hiding this comment.
Makes sense, I recall doing some identification through headers, cool!
| upload = ReportSession.objects.filter( | ||
| report_id=commit_report.id, upload_extras={"format_version": "v1"} | ||
| ).first() | ||
| assert response.status_code == 201 |
There was a problem hiding this comment.
I'm confused: the other code I see is being gated by not self.is_shelter_request(), but this endpoint works. Is this intended or did I miss something? As far as I understand this, the existing logic should only work if it is not a shelter request
There was a problem hiding this comment.
The logic within not self.is_shelter_request() is the default behavior of generating the storage path in the API. Only requests from Shelter are allowed to pass in their own storage path.
* main: Test ats run (#229) Release 23.11.2 (#228) feat: Add comparison endpoint for impacted files (#212) Update shared version (#226) feat: add a filter for public/private repo filter for viewableRepositories (#224) fix: speedup flag filtering in GraphQL API (#223) fix: Disable early notify comment (#219) Add 'value' key to the plan representation resolver (#216) 713 adjust available plans in plan service (#214) feat: Allow upload requests originating from Shelter to set storage path (#172)
Purpose/Motivation
Shelter will be buffering uploads on a queue and then calling the API async. Since Shelter won't know about the report and upload's external ID we need to construct our own storage path inside Shelter.
Links to relevant tickets
TODO: link to Shelter PR
What does this PR do?
This PR introduces a new config option which is a secret that will be shared between API and Shelter. Shelter will add this to some API requests in a custom header:
X-Shelter-TokenWhen this token is present in the upload handler then we'll allow the storage path to be set explicitly instead of generating that path in the API.