CallbackService.run()/arun() should support a configurable timeout
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 1
- Forks
- 3
- Avg merge
- 4d 8h
- Merged PRs (30d)
- 4
Description
CallbackService.run() and arun() invoke a user-supplied script via
subprocess.run(...) / asyncio.create_subprocess_exec(...) with no timeout,
so a hung callback (e.g. a script that makes an HTTP call without its own
client-side timeout) will pin a Pulp worker indefinitely until the worker is
restarted or the process is killed externally.
Mitigations already in place:
- Callbacks are dispatched as their own Pulp tasks
(pulp_workflow.app.tasks.run_callback), so a hung callback ties up a worker
slot but does not stall the originating workflow's state machine. - Only admins (
callbackservice_admin) can register callback scripts, so this
is a self-inflicted-foot-gun rather than an unauthenticated DoS surface.
Notes:
- This mirrors
pulpcore.app.models.SigningService.sign()/asign()exactly,
which also have no timeout. Adding a timeout knob here without doing the
same forSigningServiceupstream would diverge from the established
pattern. - Suggested shape if/when this is addressed: a
WORKFLOW_CALLBACK_TIMEOUT
setting (default e.g. 300s), passed tosubprocess.run(..., timeout=...)in
run()and wrapped withasyncio.wait_for(process.communicate(), ...)plus
process.kill()onTimeoutErrorinarun(). Ideally pushed upstream into
SigningServiceas well so both inherit the same behavior.
Originally raised by the Copilot reviewer on #11.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with CallbackService.run() and arun(), then inspect the pulp_workflow.app.tasks.run_callback dispatch path and the corresponding SigningService.sign()/asign() behavior in pulpcore. Determine how a configurable default timeout should apply to both subprocess paths and how it should align with the upstream signing-service pattern. Done means the timeout behavior and its tests cover synchronous and asynchronous callbacks without leaving a hung process.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- backend
- Issue type
- Feature
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 45/100