freedomofpress
[WIP] feat(`/sources`): add bulk `DELETE` endpoint
Status
Work in progress:
- [ ] Tests
- [ ] Expanded test plan?
Description of Changes
In support of freedomofpress/securedrop-client#2160, this pull request proposes a Journalist API endpoint for deleting sources in bulk with $$O(1)$$ instead $$O(n)$$ latency from the perspective of a remote (over Tor) client. This is a strict addition to the v1 Journalist API: no breaking changes; no side effects.
Testing
Needs tests. For now you can test with make dev with (e.g.):
$ curl -X DELETE -d "$(curl http://localhost:8081/api/v1/sources | jq ['.sources[] | .uuid]')" -H "Content-Type: application/json" http://localhost:8081/api/v1/sources
% Total % Received % Xferd Average Speed Time Time Time Current
Dload Upload Total Spent Left Speed
0 0 0 0 0 0 0 0 --:--:-- --:--:-- --:--:-100 16231 100 16231 0 0 489k 0 --:--:-- --:--:-- --:--:-- 495k
{
"failed": [],
"succeeded": [
"cf7bd746-80ce-4e1f-8c94-668fde45347b",
"57d67752-4a63-4d72-8177-4e883248abe7",
"bd900dbd-7bfa-4a50-b485-b969a2f0351e"
]
}
Deployment
Checklist
If you made changes to the server application code:
- [x] Linting (
make lint) and tests (make test) pass in the development container
If you made non-trivial code changes:
- [ ] I have written a test plan and validated it for this PR
Choose one of the following:
- [ ] I have opened a PR in the docs repo for these changes, or will do so later
- I would appreciate help with the documentation
- These changes do not require documentation
We all know Tor's latency penalty firsthand, but when I say $$O(1)$$ instead of $$O(n)$$, I mean:
# Just a GET to the Source Interface, but the difference is negligible versus
# a POST/DELETE to the Journalist Interface:
[workstation user ~]% time ./fetch_source_interface.sh 1
% Total % Received % Xferd Average Speed Time Time Time Current
Dload Upload Total Spent Left Speed
100 3098 100 3098 0 0 1111 0 0:00:02 0:00:02 --:--:-- 1111
./fetch_source_interface.sh 1 0.01s user 0.02s system 1% cpu 2.823 total
# So, to delete 10 sources in 1 bulk operation, that would be ~3 seconds from
# button-press to feedback in the Client...
[workstation user ~]% time ./fetch_source_interface.sh 10
% Total % Received % Xferd Average Speed Time Time Time Current
Dload Upload Total Spent Left Speed
100 3098 100 3098 0 0 1063 0 0:00:02 0:00:02 --:--:-- 1063
% Total % Received % Xferd Average Speed Time Time Time Current
Dload Upload Total Spent Left Speed
100 3098 100 3098 0 0 1227 0 0:00:02 0:00:02 --:--:-- 1227
% Total % Received % Xferd Average Speed Time Time Time Current
Dload Upload Total Spent Left Speed
100 3098 100 3098 0 0 1099 0 0:00:02 0:00:02 --:--:-- 1098
% Total % Received % Xferd Average Speed Time Time Time Current
Dload Upload Total Spent Left Speed
100 3098 100 3098 0 0 743 0 0:00:04 0:00:04 --:--:-- 743
% Total % Received % Xferd Average Speed Time Time Time Current
Dload Upload Total Spent Left Speed
100 3098 100 3098 0 0 680 0 0:00:04 0:00:04 --:--:-- 679
% Total % Received % Xferd Average Speed Time Time Time Current
Dload Upload Total Spent Left Speed
100 3098 100 3098 0 0 1234 0 0:00:02 0:00:02 --:--:-- 1235
% Total % Received % Xferd Average Speed Time Time Time Current
Dload Upload Total Spent Left Speed
100 3098 100 3098 0 0 828 0 0:00:03 0:00:03 --:--:-- 828
% Total % Received % Xferd Average Speed Time Time Time Current
Dload Upload Total Spent Left Speed
100 3098 100 3098 0 0 692 0 0:00:04 0:00:04 --:--:-- 692
% Total % Received % Xferd Average Speed Time Time Time Current
Dload Upload Total Spent Left Speed
100 3098 100 3098 0 0 783 0 0:00:03 0:00:03 --:--:-- 782
% Total % Received % Xferd Average Speed Time Time Time Current
Dload Upload Total Spent Left Speed
100 3098 100 3098 0 0 1227 0 0:00:02 0:00:02 --:--:-- 1228
./fetch_source_interface.sh 10 0.17s user 0.25s system 1% cpu 34.609 total
# ...versus ~35 seconds to delete 10 sources in 10 individual jobs.
Overall I think this is a good idea, especially given the known latency wins. IMO the complexity primarily comes in how we do error handling (fail fast? keep trying?).
I would like to see some limit imposed (maybe 50?) just so we're not running close to timeouts in worst case scenarios of large sources and/or legacy GPG keys. I'm not sure if that means the client should also impose a limit or if it needs to implement batching, which is why I'm bringing it up now.
I'm moving this out of draft status just so discussion will appear on Slack. It's still WIP.
We discussed this today and concluded:
- We're open to making non-breaking ~changes~ extensions to the Journalist API like this in support of features on the Client roadmap. We didn't have consensus on whether extensions should increment the API version number. I would argue (a) that they should not and (b) that any proposal that needs to do so is by definition a breaking change and therefore out of current scope.
- This doesn't block freedomofpress/securedrop-client#2160, which can proceed with the existing one-shot
DELETE /sources/<source_uuid>endpoint until and unless we get thisDELETE /sourcesendpoint. - We'll consider this either for v2.11 or perhaps even a dedicated v2.x release.
- @rocodes and I will look at @legoktm's suggestion in https://github.com/freedomofpress/securedrop/pull/7228#issuecomment-2341425751 of a maximum batch size and figure out whether that should be implemented on the Client side at the UI level (restrict the batch to the maximum size) or the job level (divide the batch into sub-batches of at most the maximum size).
- I note that either way we'll need rigorous test cases for failure and retry modes, especially of failed and partially successful (HTTP
207Multi-Status) batches.
- I note that either way we'll need rigorous test cases for failure and retry modes, especially of failed and partially successful (HTTP
@zenmonkeykstop, to clarify from backlog-pruning, I have no further implementation work planned here. There's a naïve test plan here already, but I need to add tests. Let me know if you'd like me to expand the test plan as well. Otherwise the implementation here is ready for review and could be released at any time with no breaking changes on the Server or downstream changes required in the Client.
Deferred to v2.13, unless I happen to be able to get this ready for review this sprint.
Closing for now as a proof-of-concept; will be revisited as part of larger sync & API overhaul.
