Skip to content

fix: validate max_requests reload configuration ranges - #1129

Open
rootsiemen wants to merge 1 commit into
crazy-goat:masterfrom
rootsiemen:fix-20261030-max-requests-ranges
Open

rootsiemen wants to merge 1 commit into
crazy-goat:masterfrom
rootsiemen:fix-20261030-max-requests-ranges

Conversation

@rootsiemen

Copy link
Copy Markdown
Contributor

Closes #1123

The max_requests reload configuration accepted any integer for requests and dispersion, so some combinations built an invalid random range: requests 100 with dispersion -20 derives a minimum of 120, and Randomizer::getInt(120, 100) throws a bare ValueError at worker start.

Changes:

  • MaxJobsRebootStrategy now rejects out-of-range arguments up front with InvalidArgumentException: maxJobs must be >= 1, dispersion must be 0-100. The issue repro (100, -20) now fails with 'dispersion must be between 0 and 100, got -20' instead of a ValueError.
  • ConfigurationTreeBuilder enforces the same ranges on the config tree: requests min(1), dispersion min(0)/max(100), so invalid YAML fails with a clear InvalidConfigurationException at container build.
  • Docs updated: config info() texts, docs/reload-strategies.md max_requests section, and the docs/configuration.md reference rows now state the supported ranges.
  • Tests: new MaxJobsRebootStrategyValidationTest (invalid values incl. the 100/-20 repro, clear messages, dispersion-100 derived-minimum-zero boundary, requests-1 boundary) and new MaxRequestsConfigurationTest (config-tree rejection of 0/-5 requests and -1/101 dispersion, acceptance of the 1/100 and 1000/0 boundaries), following the repo's existing dataProvider patterns.

Verification: the edited ConfigurationTreeBuilder.php was diffed against the upstream blob to confirm the only change is the max_requests node; the two docs files were diffed the same way. No PHP runtime is available in this environment, so phpunit could not be run locally — please rely on CI for the test run.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix: validate max-requests reload configuration ranges

1 participant