server: handle invalid jobs in expose handler hook #10154
Conversation
The expose handler hook must handle if the submitted job is invalid.
| // Mutators run first before validators, so validators view the final rendered job. | ||
| // So, mutators must handle invalid jobs. |
There was a problem hiding this comment.
I'm not fully sure what the motivation of having mutators run first: but this has bitten us before with a similar panic #6575 .
I'm not fully sure of the impact of re-ordering steps, so we validate first then mutate. Skimming code, that would mean that we'd attempt to validating the job first before canonalizing it, which may open another can of worms.
There was a problem hiding this comment.
This is really unfortunate in that we really have two types of validation: validation for safety which really wants to happen before mutation (canonicalization, making sure we don't have nil pointers in unexpected places), and validation for "business logic" (ex. task group count for system jobs).
But solving that is beyond the scope of this bug fix, so 👍 for leaving this comment.
| // Mutators run first before validators, so validators view the final rendered job. | ||
| // So, mutators must handle invalid jobs. |
There was a problem hiding this comment.
This is really unfortunate in that we really have two types of validation: validation for safety which really wants to happen before mutation (canonicalization, making sure we don't have nil pointers in unexpected places), and validation for "business logic" (ex. task group count for system jobs).
But solving that is beyond the scope of this bug fix, so 👍 for leaving this comment.
Co-authored-by: Tim Gross <[email protected]>
The expose handler hook must handle if the submitted job is invalid. Without this validation, the rpc handler panics on invalid input. Co-authored-by: Tim Gross <[email protected]>
|
I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions. |
The expose handler hook must handle if the submitted job is invalid. Without this validation, the rpc handler panics on invalid input.
Fixes #10115