Only reject start-time options the caller actually declared

The declarative paths hand serve_start a fully-defaulted dump of the config, so
every untouched schema default read as a requested change: HTTPOptionsSchema.host
defaults to 0.0.0.0 while HTTPOptions.host defaults to the loopback, so a config
with no http_options section rejected every apply to a Serve instance that was
started from Python, and ServeDeploySchema.proxy_location always carries a value
so placement was always requested too. Compare only what the config declared
(declared_start_time_options), and have serve start pass only the flags it was
given.

Placement is now diffed against the placement the caller asked for rather than
the one in effect: direct-ingress mode overrides proxy_location to HeadOnly, so
re-applying the very config that started Serve was rejected, and the error told
the user to restart, which reapplies the same override.

Also redact secrets from the error (it reaches an HTTP 400 body), surface it as a
CLI error instead of a traceback from serve run/serve start, document the 400 on
the REST reference and serve.start, and update the pre-existing standalone test
that asserted the warning this replaced.

Signed-off-by: john.taylor <john.taylor@anyscale.com>

Author: johntaylor-cell

doc/source/serve/api/index.md

@@ -201,6 +201,8 @@ The Serve REST API is exposed at the same port as the Ray Dashboard. The Dashboa
 
 Declaratively deploys a list of Serve applications. If Serve is already running on the Ray cluster, removes all applications not listed in the new config. If Serve is not running on the Ray cluster, starts Serve. See [multi-app config schema](serve-rest-api-config-schema) for the request's JSON schema.
 
+`proxy_location`, `http_options`, and `grpc_options` are global to the cluster and fixed when Serve starts. If Serve is already running and the config sets any of them to a different value, the whole request is rejected with a `400` naming each field, and no applications are deployed. Omitting a field is not a request to change it.
+
 **Example Request**:
 
 ```http

python/ray/dashboard/modules/serve/serve_head.py

@@ -146,7 +146,10 @@ async def delete_serve_applications(self, req: Request) -> Response:
     @validate_endpoint()
     async def put_all_applications(self, req: Request) -> Response:
         from ray._common.usage.usage_lib import TagKey, record_extra_usage_tag
-        from ray.serve._private.api import serve_start_async
+        from ray.serve._private.api import (
+            declared_start_time_options,
+            serve_start_async,
+        )
         from ray.serve.exceptions import RayServeConfigException
         from ray.serve.schema import ServeDeploySchema
 
@@ -171,6 +174,7 @@ async def put_all_applications(self, req: Request) -> Response:
                     grpc_options=grpc_options,
                     global_logging_config=config.logging_config,
                     controller_options=config.controller_options,
+                    declared_options=declared_start_time_options(config),
                 )
         except RayServeConfigException as e:
             # Reject the whole config: applying the applications while dropping the

python/ray/dashboard/modules/serve/tests/test_serve_dashboard_2.py

@@ -14,6 +14,7 @@
 from ray import serve
 from ray._common.test_utils import wait_for_condition
 from ray._private.test_utils import generate_system_config_map
+from ray.serve.config import ProxyLocation
 from ray.serve.generated import serve_pb2, serve_pb2_grpc
 from ray.serve.schema import HTTPOptionsSchema, ServeInstanceDetails
 from ray.serve.tests.conftest import *  # noqa: F401 F403
@@ -65,15 +66,15 @@ def test_serve_namespace(ray_start_stop):
 
 
 @pytest.mark.parametrize(
-    "option,override",
+    "option,override,changed_field",
     [
-        ("proxy_location", "HeadOnly"),
-        ("http_options", {"host": "127.0.0.2"}),
-        ("http_options", {"port": 8000}),
-        ("http_options", {"root_path": "/serve_updated"}),
+        ("proxy_location", "HeadOnly", "proxy_location"),
+        ("http_options", {"host": "127.0.0.2"}, "http_options.host"),
+        ("http_options", {"port": 8001}, "http_options.port"),
+        ("http_options", {"root_path": "/serve_updated"}, "http_options.root_path"),
     ],
 )
-def test_put_with_http_options(ray_start_stop, option, override):
+def test_put_with_http_options(ray_start_stop, option, override, changed_field):
     """Submits a config with HTTP options specified.
 
     Trying to submit a config to the serve agent with the HTTP options modified:
@@ -127,6 +128,10 @@ def test_put_with_http_options(ray_start_stop, option, override):
     )
     assert put_response.status_code == 400
     assert "can't be updated at runtime" in put_response.text
+    # Only the field the config actually changed may be reported: naming a field the
+    # config left alone is how a fully-defaulted dump rejects an unchanged config.
+    assert changed_field in put_response.text
+    assert put_response.text.count("->") == 1
 
     # Fetch Serve status and confirm that HTTP options are unchanged
     get_response = requests.get(SERVE_HEAD_URL, timeout=5)
@@ -147,6 +152,40 @@ def test_put_with_http_options(ray_start_stop, option, override):
     assert requests.post("http://localhost:8000/serve/app2").text == "wonderful world"
 
 
+def test_put_omitting_global_options(ray_start_stop):
+    """A config that declares no global options isn't asking to change them.
+
+    The schema fills in every default, so this is the case that regresses into a
+    rejection if the apply is diffed against the full dump instead of what was sent.
+    """
+    world_import_path = "ray.serve.tests.test_config_files.world.DagNode"
+    app = {"name": "app1", "route_prefix": "/app1", "import_path": world_import_path}
+    deploy_config_multi_app(
+        {
+            "proxy_location": "HeadOnly",
+            "http_options": {"host": "127.0.0.1", "port": 8000, "root_path": "/serve"},
+            "applications": [app],
+        },
+        SERVE_HEAD_URL,
+    )
+    wait_for_condition(
+        lambda: requests.post("http://localhost:8000/serve/app1").text
+        == "wonderful world",
+        timeout=15,
+    )
+
+    put_response = requests.put(SERVE_HEAD_URL, json={"applications": [app]}, timeout=5)
+    assert put_response.status_code == 200, put_response.text
+
+    # The running options are untouched, not reset to the schema defaults.
+    serve_details = ServeInstanceDetails.model_validate(
+        requests.get(SERVE_HEAD_URL, timeout=5).json()
+    )
+    assert serve_details.http_options.host == "127.0.0.1"
+    assert serve_details.http_options.root_path == "/serve"
+    assert serve_details.proxy_location == ProxyLocation.HeadOnly
+
+
 def test_put_with_grpc_options(ray_start_stop):
     """Submits a config with gRPC options specified.
 

python/ray/serve/_private/api.py

@@ -35,10 +35,13 @@
 )
 from ray.serve.deployment import Application
 from ray.serve.exceptions import RayServeConfigException, RayServeException
-from ray.serve.schema import LoggingConfig, TracingConfig
+from ray.serve.schema import LoggingConfig, ServeDeploySchema, TracingConfig
 
 logger = logging.getLogger(SERVE_LOGGER_NAME)
 
+# Reported as "<redacted>" instead of their value in the start-time config error.
+_REDACTED_FIELDS = frozenset({"http_options.ssl_keyfile_password"})
+
 
 def _coerce_controller_options(
     controller_options: Union[None, dict, ControllerOptions],
@@ -92,8 +95,13 @@ def _diff_start_time_options(
     }
 
 
-def _describe(value: Any) -> str:
-    """Render a config value for an error message, unwrapping enums."""
+def _describe(field: str, value: Any) -> str:
+    """Render a config value for an error message, unwrapping enums.
+
+    Secrets are redacted because the message reaches an HTTP 400 body and user logs.
+    """
+    if field in _REDACTED_FIELDS:
+        return "<redacted>"
     return repr(value.value if isinstance(value, Enum) else value)
 
 
@@ -149,15 +157,19 @@ def _check_start_time_config_unchanged(
         )
     )
 
+    # Compared against the placement the caller originally asked for, not the one
+    # in effect: direct-ingress mode overrides it, and the override isn't a change
+    # any config can request or avoid.
+    current_location = client.requested_proxy_location
     requested_location = _requested_proxy_location(http_options, proxy_location)
-    if requested_location is not None and requested_location != client.proxy_location:
-        diff["proxy_location"] = (client.proxy_location, requested_location)
+    if requested_location is not None and requested_location != current_location:
+        diff["proxy_location"] = (current_location, requested_location)
 
     if not diff:
         return
 
     changes = ", ".join(
-        f"{field}: {_describe(current)} -> {_describe(requested)}"
+        f"{field}: {_describe(field, current)} -> {_describe(field, requested)}"
         for field, (current, requested) in sorted(diff.items())
     )
     raise RayServeConfigException(
@@ -170,6 +182,44 @@ def _check_start_time_config_unchanged(
     )
 
 
+def declared_start_time_options(config: ServeDeploySchema) -> Dict[str, Any]:
+    """The start-time options a declarative config explicitly asked for.
+
+    The declarative paths send a fully-defaulted dump to start Serve with, so
+    without this every untouched schema default would read as a requested change.
+    """
+    return {
+        "http_options": config.http_options.model_dump(exclude_unset=True),
+        "grpc_options": config.grpc_options.model_dump(exclude_unset=True),
+        "proxy_location": (
+            config.proxy_location
+            if "proxy_location" in config.model_fields_set
+            else None
+        ),
+    }
+
+
+def _connect_to_existing_client(
+    client: ServeControllerClient,
+    http_options: Union[None, dict, HTTPOptions],
+    grpc_options: Union[None, dict, gRPCOptions],
+    proxy_location: Union[None, str, ProxyLocation],
+    declared_options: Optional[Dict[str, Any]],
+) -> None:
+    """Log the connection and reject changes to config fixed at controller startup."""
+    if declared_options is None:
+        declared_options = {
+            "http_options": http_options,
+            "grpc_options": grpc_options,
+            "proxy_location": proxy_location,
+        }
+    logger.info(
+        f'Connecting to existing Serve app in namespace "{SERVE_NAMESPACE}".'
+        " New controller_options will not be applied."
+    )
+    _check_start_time_config_unchanged(client, **declared_options)
+
+
 def _create_controller_and_proxy_refs(
     http_options: Union[None, dict, HTTPOptions],
     grpc_options: Union[None, dict, gRPCOptions],
@@ -254,6 +304,7 @@ async def serve_start_async(
     global_tracing_config: Union[None, dict, TracingConfig] = None,
     controller_options: Union[None, dict, ControllerOptions] = None,
     proxy_location: Union[None, str, ProxyLocation] = None,
+    declared_options: Optional[Dict[str, Any]] = None,
     **kwargs,
 ) -> ServeControllerClient:
     """Initialize a serve instance asynchronously.
@@ -279,15 +330,8 @@ async def serve_start_async(
         except RayServeException:
             client = None
     if client is not None:
-        logger.info(
-            f'Connecting to existing Serve app in namespace "{SERVE_NAMESPACE}".'
-            " New controller_options will not be applied."
-        )
-        _check_start_time_config_unchanged(
-            client,
-            http_options=http_options,
-            grpc_options=grpc_options,
-            proxy_location=proxy_location,
+        _connect_to_existing_client(
+            client, http_options, grpc_options, proxy_location, declared_options
         )
         return client
 
@@ -334,6 +378,7 @@ def serve_start(
     global_tracing_config: Union[None, dict, TracingConfig] = None,
     controller_options: Union[None, dict, ControllerOptions] = None,
     proxy_location: Union[None, str, ProxyLocation] = None,
+    declared_options: Optional[Dict[str, Any]] = None,
     **kwargs,
 ) -> ServeControllerClient:
     """Initialize a serve instance.
@@ -388,6 +433,11 @@ def serve_start(
             the cluster. See ``ProxyLocation`` for supported options. Defaults
             to ``EveryNode`` when unspecified. An explicit (deprecated)
             ``HTTPOptions.location`` overrides this.
+        declared_options: The subset of the options above that the caller
+            explicitly asked for, checked against an already-running instance
+            instead of the values passed here. Declarative callers must pass
+            this (see ``declared_start_time_options``) because they send a
+            fully-defaulted config to start Serve with.
         **kwargs: Reserved for forwarding to internal controller-start hooks;
             no public keys are currently supported and unknown keys may raise.
 
@@ -407,15 +457,8 @@ def serve_start(
         except RayServeException:
             client = None
     if client is not None:
-        logger.info(
-            f'Connecting to existing Serve app in namespace "{SERVE_NAMESPACE}".'
-            " New controller_options will not be applied."
-        )
-        _check_start_time_config_unchanged(
-            client,
-            http_options=http_options,
-            grpc_options=grpc_options,
-            proxy_location=proxy_location,
+        _connect_to_existing_client(
+            client, http_options, grpc_options, proxy_location, declared_options
         )
         return client
 

python/ray/serve/_private/client.py

@@ -70,13 +70,13 @@ def __init__(
             [
                 controller.get_http_config.remote(),
                 controller.get_grpc_config.remote(),
-                controller.get_proxy_location.remote(),
+                controller.get_requested_proxy_location.remote(),
                 controller.get_root_url.remote(),
             ]
         )
         self._http_config: HTTPOptions = configs[0]
         self._grpc_config: gRPCOptions = configs[1]
-        self._proxy_location: Optional[ProxyLocation] = configs[2]
+        self._requested_proxy_location: ProxyLocation = configs[2]
         self._root_url = configs[3]
 
         # Each handle has the overhead of long poll client, therefore cached.
@@ -96,8 +96,8 @@ def grpc_config(self):
         return self._grpc_config
 
     @property
-    def proxy_location(self):
-        return self._proxy_location
+    def requested_proxy_location(self):
+        return self._requested_proxy_location
 
     def __reduce__(self):
         raise RayServeException(("Ray Serve client cannot be serialized."))

python/ray/serve/_private/controller.py

@@ -208,6 +208,11 @@ async def __init__(  # type: ignore[misc]
 
         self._ha_proxy_enabled = RAY_SERVE_ENABLE_HA_PROXY
         self._direct_ingress_enabled = RAY_SERVE_ENABLE_DIRECT_INGRESS
+        # Captured before the mode-specific overrides below so a later config apply
+        # is diffed against what the caller asked for, not what we forced.
+        self._requested_proxy_location = (
+            http_options.location or proxy_location or ProxyLocation.EveryNode
+        )
         # Last full set of ingress-port tuples fed to update_ports (for the per-tick set-diff).
         self._last_ingress_port_tuples: set = set()
         # Last ingress membership version seen; -1 forces the first tick to run.
@@ -989,6 +994,10 @@ def get_proxy_location(self) -> Optional[ProxyLocation]:
             return None
         return self.proxy_state_manager.get_proxy_location()
 
+    def get_requested_proxy_location(self) -> ProxyLocation:
+        """Return the placement the caller asked for, before any mode override."""
+        return self._requested_proxy_location
+
     def get_grpc_config(self) -> gRPCOptions:
         """Return the gRPC proxy configuration."""
         if self.proxy_state_manager is None:

python/ray/serve/api.py

@@ -122,6 +122,10 @@ class See `gRPCOptions` for supported options.
           running in this Ray cluster.
         **kwargs: Reserved for forward-compatibility; passed through to the
             internal Serve start helper.
+
+    Raises:
+        RayServeConfigException: If Serve is already running and this call asks to
+          change ``proxy_location``, ``http_options``, or ``grpc_options``.
     """
     _private_api.serve_start(
         http_options=http_options,