Skip to content

[Serve] Fail fast on invalid fields in deployment and autoscaling configs - #62016

Open
javierdejesusda wants to merge 2 commits into
ray-project:masterfrom
javierdejesusda:serve-strict-config-validation
Open

[Serve] Fail fast on invalid fields in deployment and autoscaling configs#62016
javierdejesusda wants to merge 2 commits into
ray-project:masterfrom
javierdejesusda:serve-strict-config-validation

Conversation

@javierdejesusda

Copy link
Copy Markdown

Why are these changes needed?

When users provide a faulty Serve deployment config (e.g., placing max_ongoing_requests inside autoscaling_config instead of at the deployment level), the misconfiguration is silently ignored and the default value is used instead. This leads to hard-to-debug production issues.

As noted by @abrarsheikh in #61529:

"We really want the entire serve config validated such that if the user-provided config deviates from the config schema, we want to fail fast."

This PR adds extra="forbid" to all Serve config Pydantic models that are not explicitly forward-compatible, so unrecognized fields raise a ValidationError immediately.

Changes

Config models updated (extra="forbid" added):

  • AutoscalingConfig (config.py) — previously silently accepted unknown fields
  • GangSchedulingConfig (config.py)
  • RequestRouterConfig (config.py)
  • DeploymentSchema (schema.py) — extended existing model_config
  • RayActorOptionsSchema (schema.py)
  • ServeApplicationSchema (schema.py)
  • gRPCOptionsSchema (schema.py)

Deliberately unchanged (forward-compatible by design):

  • HTTPOptionsSchema — has explicit "allows extra parameters for forward-compatibility" note
  • ServeDeploySchema — same forward-compatibility note

Dict-to-model validation fix:

The autoscaling_config field on DeploymentSchema is typed as Union[Dict, AutoscalingConfig], which means Pydantic matches any dict as Dict first, bypassing AutoscalingConfig validation entirely. Extended the existing validate_gang_scheduling_config validator to also convert autoscaling_config and request_router_config dicts into their typed model equivalents before validation, ensuring extra="forbid" applies to dict inputs from YAML configs.

Tests:

  • Inverted 3 existing test_extra_fields_invalid_* tests (they previously asserted extras were accepted; now assert ValidationError)
  • Added new tests for all updated schemas
  • Added regression tests confirming HTTPOptionsSchema and ServeDeploySchema still accept extras
  • Added test for the exact scenario from [Serve] Fail fast when user provides faulty deployment config #61439 (dict-based autoscaling_config with misplaced field)

Related issue number

Fixes #61439
Supersedes stale #61529

Checks

  • I've signed off every commit
  • I've run the existing tests
  • I've added new tests for the changes
  • I've made sure all new and existing tests pass
  • I've updated documentation if needed

@javierdejesusda
javierdejesusda requested a review from a team as a code owner March 24, 2026 07:16
@ray-gardener ray-gardener Bot added serve Ray Serve Related Issue community-contribution Contributed by the community labels Mar 24, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

The pull request effectively addresses the problem of silently ignored misconfigurations in Serve deployment and autoscaling configs by introducing extra="forbid" to relevant Pydantic models. This change significantly improves the developer experience by providing immediate feedback on invalid fields. The addition of comprehensive unit tests for all updated schemas, including regression tests for forward-compatible schemas, ensures the robustness and correctness of the changes. The fix for dict-to-model validation in DeploymentSchema is also a critical improvement, ensuring that extra="forbid" is applied even when configurations are provided as plain dictionaries.

Comment thread python/ray/serve/schema.py Outdated

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

There are 2 total unresolved issues (including 1 from previous review).

Fix All in Cursor

Comment thread python/ray/serve/config.py
@javierdejesusda
javierdejesusda force-pushed the serve-strict-config-validation branch 2 times, most recently from 1f34972 to 6a15e60 Compare March 24, 2026 08:12
…figs

Add extra="forbid" to Pydantic config schemas so that unrecognized fields
raise a ValidationError immediately instead of being silently ignored.

This addresses a usability issue where users could accidentally place fields
like max_ongoing_requests inside autoscaling_config (instead of at the
deployment level), and the misconfiguration would be silently swallowed.

Schemas changed:
- AutoscalingConfig, GangSchedulingConfig, RequestRouterConfig (config.py)
- DeploymentSchema, RayActorOptionsSchema, gRPCOptionsSchema (schema.py)

Deliberately unchanged (forward-compatible by design):
- HTTPOptionsSchema, ServeDeploySchema, ServeApplicationSchema

Also adds a before-validator to DeploymentSchema that validates dict inputs
for autoscaling_config and request_router_config against their typed models,
catching extra fields even when configs arrive as plain dicts from YAML.
The dicts are intentionally kept as-is (not replaced with model instances)
because downstream code relies on isinstance(field, dict) checks.

Fixes ray-project#61439

Signed-off-by: javierdejesusda <javier.dejesusj9@gmail.com>
@javierdejesusda
javierdejesusda force-pushed the serve-strict-config-validation branch from 6a15e60 to ed65af7 Compare March 24, 2026 08:50
Signed-off-by: javierdejesusda <javier.dejesusj9@gmail.com>
@github-actions

github-actions Bot commented Apr 7, 2026

Copy link
Copy Markdown

This pull request has been automatically marked as stale because it has not had
any activity for 14 days. It will be closed in another 14 days if no further activity occurs.
Thank you for your contributions.

You can always ask for help on our discussion forum or Ray's public slack channel.

If you'd like to keep this open, just leave any comment, and the stale label will be removed.

@github-actions github-actions Bot added the stale The issue is stale. It will be closed within 7 days unless there are further conversation label Apr 7, 2026
@javierdejesusda

Copy link
Copy Markdown
Author

Keeping this PR active — it is ready for maintainer review.

Current state:

  • Rebased onto latest master
  • DCO: passing
  • Cursor Bugbot: passing (both flagged issues addressed in the latest push)
  • Read the Docs: passing
  • buildkite/release: passing
  • buildkite/microcheck: failing — I could not identify a failure related to this diff after tracing downstream call sites. Would appreciate guidance from a maintainer on what specifically is failing (the Buildkite logs are not publicly accessible).

Summary of changes since the original submission:

  1. extra="forbid" removed from ServeApplicationSchema because it is nested inside ServeDeploySchema, which promises forward-compatibility for rolling upgrades. Added the forward-compat note to its docstring to match HTTPOptionsSchema.
  2. The validate_and_convert_config_dicts validator now uses a lightweight _check_extra_fields helper that checks dict keys against the model's model_fields set rather than constructing a throwaway model instance. This avoids triggering RequestRouterConfig.__init__'s _serialize_request_router_cls side effect (which imports and cloudpickles the router class) during validation.
  3. The validator intentionally does NOT replace dicts with model instances, because deploy_utils.py (get_app_code_version) and application_state.py (apply_app_config) use isinstance(deployment.autoscaling_config, dict) guards to extract custom policies for serialization. Replacing the dict would silently break those code paths.
  4. Removed a duplicated test class across test_schema.py and test_config.py.
  5. Fixed a broken test (test_serve_deploy_schema_allows_extra_fields) that was missing the required applications=[] field.

@abrarsheikh — this directly implements the broader schema validation you asked for in #61529. Happy to iterate on the approach if there is a preferred direction for the microcheck failure.

@github-actions github-actions Bot added unstale A PR that has been marked unstale. It will not get marked stale again if this label is on it. and removed stale The issue is stale. It will be closed within 7 days unless there are further conversation labels Apr 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

community-contribution Contributed by the community serve Ray Serve Related Issue unstale A PR that has been marked unstale. It will not get marked stale again if this label is on it.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Serve] Fail fast when user provides faulty deployment config

1 participant