fix(proxy): make an ambiguous config include loud, not silent

An include entry that matches both a file next to the config that declares it and
one next to the root config now warns naming both, so a config that resolves to a
different file than it used to says so instead of quietly serving other models.

Also from reviewing that change:

- an empty root object in a bucket fails the boot again instead of coming up empty
- a YAML syntax error in a bucket object logs its own line naming the object
- an include already loaded is skipped before it is read rather than after
- reading a config out of GCS builds the plain bucket client, so it needs no
  enterprise license and starts no flush loop that nothing ever cancels

litellm/proxy/common_utils/config_includes.py

@@ -13,28 +13,41 @@ def resolve_include_file_path(include_file: str, declared_in: str, root_config_p
     Resolve one `include` entry to the file it names, next to the config that declares it.
 
     A config written before nested entries resolved this way can name a file sitting next to the root
-    config instead, so that file is still read, with a warning naming where it was found.
+    config instead, so that file is still read, with a warning naming where it was found. When both
+    files exist the one next to the declaring config wins and the other is named in a warning.
     """
     declared_relative: Final = os.path.abspath(os.path.join(os.path.dirname(declared_in), include_file))
-    if os.path.exists(declared_relative):
-        return declared_relative
-
     root_relative: Final = os.path.abspath(os.path.join(os.path.dirname(root_config_path), include_file))
     if root_relative == declared_relative or not os.path.exists(root_relative):
         return declared_relative
 
+    if not os.path.exists(declared_relative):
+        verbose_proxy_logger.warning(
+            "Config include '%s' declared in %s was not found next to it, so %s was read instead. "
+            "Move the included file next to the config that declares it.",
+            include_file,
+            declared_in,
+            root_relative,
+        )
+        return root_relative
+
     verbose_proxy_logger.warning(
-        "Config include '%s' declared in %s was not found next to it, so %s was read instead. "
-        "Move the included file next to the config that declares it.",
+        "Config include '%s' declared in %s matches two files. %s sits next to that config and was read, "
+        "so %s was skipped. Rename one of the two to say which one you meant.",
         include_file,
         declared_in,
+        declared_relative,
         root_relative,
     )
-    return root_relative
+    return declared_relative
 
 
-class ConfigLoader(Protocol):
-    def __call__(self, include_entry: str, declared_in: str, /) -> Awaitable[tuple[str, Mapping[str, object]]]: ...
+class IncludeResolver(Protocol):
+    def __call__(self, include_entry: str, declared_in: str, /) -> str: ...
+
+
+class ConfigReader(Protocol):
+    def __call__(self, location: str, /) -> Awaitable[Mapping[str, object]]: ...
 
 
 def _merged_value(base_value: object, included_value: object) -> object:
@@ -80,32 +93,40 @@ async def _resolve(
     config: Mapping[str, object],
     pending: tuple[tuple[str, str], ...],
     loaded: frozenset[str],
-    load: ConfigLoader,
+    resolve: IncludeResolver,
+    read: ConfigReader,
 ) -> Mapping[str, object]:
     if not pending:
         return _without_include(config)
 
     entry, declared_in = pending[0]
-    location, included = await load(entry, declared_in)
+    location: Final = resolve(entry, declared_in)
     if location in loaded:
-        return await _resolve(config, pending[1:], loaded, load)
+        return await _resolve(config, pending[1:], loaded, resolve, read)
 
+    included: Final = await read(location)
     return await _resolve(
         _merged(config, _without_include(included)),
         (*pending[1:], *_pending_from(included, location)),
         loaded | frozenset((location,)),
-        load,
+        resolve,
+        read,
     )
 
 
-async def resolve_includes(config: Mapping[str, object], location: str, load: ConfigLoader) -> dict[str, object]:
+async def resolve_includes(
+    config: Mapping[str, object],
+    location: str,
+    resolve: IncludeResolver,
+    read: ConfigReader,
+) -> dict[str, object]:
     """
     Merge every config named by the `include` directive into the config that declares it.
 
-    List values are extended and every other value is overridden, each entry is resolved relative to
-    the config that declares it, a config already pulled in is not merged a second time, and `load`
-    decides where an entry is read from, so the same merge applies to configs on disk and to configs
-    hosted in a bucket.
+    List values are extended and every other value is overridden, `resolve` turns each entry into the
+    location it names relative to the config that declares it, a config already pulled in is neither
+    read nor merged a second time, and `read` decides where a location is read from, so the same merge
+    applies to configs on disk and to configs hosted in a bucket.
     """
-    merged: Final = await _resolve(config, _pending_from(config, location), frozenset((location,)), load)
+    merged: Final = await _resolve(config, _pending_from(config, location), frozenset((location,)), resolve, read)
     return dict(merged)  # mutable-ok: the proxy mutates the config it loads

litellm/proxy/common_utils/load_config_utils.py

@@ -29,8 +29,12 @@ class SyncBucketObjectReader(Protocol):
     def __call__(self, object_key: str, /) -> object | None: ...
 
 
-def _parsed_config(file_contents: str) -> object:
-    parsed: Final = yaml.safe_load(file_contents)
+def _parsed_config(object_key: str, file_contents: str) -> object | None:
+    try:
+        parsed: Final = yaml.safe_load(file_contents)
+    except yaml.YAMLError as e:
+        verbose_proxy_logger.error("Config object %s is not valid YAML: %s", object_key, e)
+        return None
     return MappingProxyType({}) if parsed is None else parsed
 
 
@@ -64,11 +68,13 @@ def s3_object_reader(bucket_name: str) -> SyncBucketObjectReader:
         try:
             verbose_proxy_logger.debug("Retrieving %s from S3 bucket: %s", object_key, bucket_name)
             response: Final = s3_client.get_object(Bucket=bucket_name, Key=object_key)
-            return _parsed_config(response["Body"].read().decode("utf-8"))
+            file_contents: Final = response["Body"].read().decode("utf-8")
         except Exception as e:  # noqa: BLE001  # any boto3 error must read as a missing object
             verbose_proxy_logger.error("Error retrieving %s from S3 bucket %s: %s", object_key, bucket_name, e)
             return None
 
+        return _parsed_config(object_key, file_contents)
+
     return read
 
 
@@ -77,10 +83,16 @@ def get_file_contents_from_s3(bucket_name: str, object_key: str) -> object | Non
 
 
 def gcs_config_bucket(bucket_name: str) -> "GCSBucketBase | None":
-    try:
-        from litellm.integrations.gcs_bucket.gcs_bucket import GCSBucketLogger
+    """
+    Build a plain GCS client for reading config objects.
 
-        return GCSBucketLogger(bucket_name=bucket_name)
+    Reading a config out of a bucket is not GCS logging, so it neither needs the enterprise license
+    that gate covers nor the batching task the logger starts and never stops.
+    """
+    try:
+        from litellm.integrations.gcs_bucket.gcs_bucket_base import GCSBucketBase
+
+        return GCSBucketBase(bucket_name=bucket_name)
     except Exception as e:  # noqa: BLE001  # an unbuildable client must read as an unreadable bucket
         verbose_proxy_logger.error("Error creating the GCS client for bucket %s: %s", bucket_name, e)
         return None
@@ -98,12 +110,14 @@ async def get_config_file_contents_from_gcs(
         file_contents: Final = await bucket.download_gcs_object(object_key)
         if file_contents is None:
             raise Exception(f"File contents are None for {object_key}")
-        return _parsed_config(file_contents.decode("utf-8"))
+        decoded: Final = file_contents.decode("utf-8")
 
     except Exception as e:
         verbose_proxy_logger.error("Error retrieving %s from GCS bucket %s: %s", object_key, bucket_name, e)
         return None
 
+    return _parsed_config(object_key, decoded)
+
 
 def resolve_include_object_key(config_object_key: str, include_entry: str) -> str:
     """
@@ -123,17 +137,19 @@ async def resolve_bucket_includes(
     object_key: str,
     fetch: BucketObjectFetcher,
 ) -> dict[str, object]:
-    async def load(include_entry: str, declared_in: str) -> tuple[str, Mapping[str, object]]:
-        include_key: Final = resolve_include_object_key(declared_in, include_entry)
+    async def read(include_key: str) -> Mapping[str, object]:
         included: Final = await fetch(include_key)
         if included is None:
             raise FileNotFoundError(
                 f"Included config could not be read from bucket: {include_key}. "
                 "The underlying bucket error is logged above."
             )
-        return include_key, included
+        return included
 
-    return await resolve_includes(config=config, location=object_key, load=load)
+    def resolve(include_entry: str, declared_in: str) -> str:
+        return resolve_include_object_key(declared_in, include_entry)
+
+    return await resolve_includes(config=config, location=object_key, resolve=resolve, read=read)
 
 
 async def bucket_object_reader(bucket_type: str | None, bucket_name: str) -> BucketObjectReader:
@@ -176,7 +192,7 @@ async def get_config_from_bucket(
             raise ValueError(f"Config object in bucket is not a YAML mapping: {key}") from e
 
     config: Final = await fetch(object_key)
-    if config is None:
+    if not config:
         return None
 
     return await resolve_bucket_includes(config=config, object_key=object_key, fetch=fetch)
@@ -256,11 +272,9 @@ async def download_python_file_from_gcs(
         bool: True if successful, False otherwise
     """
     try:
-        from litellm.integrations.gcs_bucket.gcs_bucket import GCSBucketLogger
+        from litellm.integrations.gcs_bucket.gcs_bucket_base import GCSBucketBase
 
-        gcs_bucket: Final = GCSBucketLogger(
-            bucket_name=bucket_name,
-        )
+        gcs_bucket: Final = GCSBucketBase(bucket_name=bucket_name)
         file_contents = await gcs_bucket.download_gcs_object(object_key)
         if file_contents is None:
             raise Exception(f"File contents are None for {object_key}")

litellm/proxy/proxy_server.py

@@ -4559,17 +4559,18 @@ class ProxyConfig:
 
         included_config_adapter: Final = TypeAdapter(dict[str, object])
 
-        async def load_included(include_file: str, declared_in: str) -> tuple[str, Mapping[str, object]]:
-            file_path: Final = resolve_include_file_path(include_file, declared_in, config_file_path)
+        def resolve(include_file: str, declared_in: str) -> str:
+            return resolve_include_file_path(include_file, declared_in, config_file_path)
+
+        async def read_included(file_path: str) -> Mapping[str, object]:
             if not os.path.exists(file_path):
                 raise FileNotFoundError(f"Included file not found: {file_path}")
             try:
-                included: Final = included_config_adapter.validate_python(self._load_yaml_file(file_path))
+                return included_config_adapter.validate_python(self._load_yaml_file(file_path))
             except ValidationError as e:
                 raise ValueError(f"Included config file is not a YAML mapping: {file_path}") from e
-            return file_path, included
 
-        return await resolve_includes(config=config, location=config_file_path, load=load_included)
+        return await resolve_includes(config=config, location=config_file_path, resolve=resolve, read=read_included)
 
     async def save_config(self, new_config: dict, include_env_vars: bool = False):
         global prisma_client, general_settings, user_config_file_path, store_model_in_db

tests/test_litellm/proxy/common_utils/test_load_config_utils.py

@@ -1,4 +1,5 @@
 import asyncio
+import logging
 import re
 import threading
 from unittest.mock import MagicMock, mock_open, patch
@@ -7,6 +8,7 @@ import pytest
 import yaml
 
 from litellm.proxy.common_utils.load_config_utils import (
+    gcs_config_bucket,
     get_config_from_bucket,
     get_file_contents_from_s3,
     resolve_bucket_includes,
@@ -391,3 +393,84 @@ class TestBucketConfigIncludes:
             )
             is None
         )
+
+    @pytest.mark.asyncio
+    async def test_an_object_pulled_in_twice_is_read_once(self):
+        objects = {
+            "configs/a.yaml": {"include": ["shared.yaml"]},
+            "configs/b.yaml": {"include": ["./shared.yaml"]},
+            "configs/shared.yaml": {"model_list": [{"model_name": "shared"}]},
+        }
+        requested = []
+
+        async def fetch(object_key):
+            requested.append(object_key)
+            return objects.get(object_key)
+
+        await resolve_bucket_includes(
+            config={"include": ["a.yaml", "b.yaml"]},
+            object_key="configs/config.yaml",
+            fetch=fetch,
+        )
+
+        assert requested == ["configs/a.yaml", "configs/b.yaml", "configs/shared.yaml"]
+
+    @pytest.mark.asyncio
+    async def test_an_empty_root_object_does_not_boot_an_empty_proxy(self, monkeypatch):
+        class FakeGCSBucket:
+            async def download_gcs_object(self, object_key):
+                return b""
+
+        monkeypatch.setattr(
+            "litellm.proxy.common_utils.load_config_utils.gcs_config_bucket",
+            lambda bucket_name: FakeGCSBucket(),
+        )
+
+        config = await get_config_from_bucket(
+            bucket_type="gcs", bucket_name="litellm-configs", object_key="lit6982/config.yaml"
+        )
+
+        assert config is None
+
+    @pytest.mark.asyncio
+    async def test_an_object_that_is_not_valid_yaml_is_reported_as_a_yaml_error(self, monkeypatch, caplog):
+        class FakeGCSBucket:
+            async def download_gcs_object(self, object_key):
+                return b"model_list: [\n"
+
+        monkeypatch.setattr(
+            "litellm.proxy.common_utils.load_config_utils.gcs_config_bucket",
+            lambda bucket_name: FakeGCSBucket(),
+        )
+
+        with caplog.at_level(logging.ERROR, logger="LiteLLM Proxy"):
+            config = await get_config_from_bucket(
+                bucket_type="gcs", bucket_name="litellm-configs", object_key="lit6982/config.yaml"
+            )
+
+        assert config is None
+        assert [
+            record
+            for record in caplog.records
+            if "not valid YAML" in record.getMessage() and "lit6982/config.yaml" in record.getMessage()
+        ]
+
+
+class TestGCSConfigBucketClient:
+    @pytest.mark.asyncio
+    async def test_reading_a_config_from_gcs_does_not_need_an_enterprise_license(self, monkeypatch):
+        monkeypatch.setattr("litellm.proxy.proxy_server.premium_user", False)
+
+        bucket = gcs_config_bucket("litellm-configs")
+
+        assert bucket is not None
+        assert bucket.BUCKET_NAME == "litellm-configs"
+
+    @pytest.mark.asyncio
+    async def test_reading_a_config_from_gcs_starts_no_background_task(self, monkeypatch):
+        monkeypatch.setattr("litellm.proxy.proxy_server.premium_user", True)
+        running_before = asyncio.all_tasks()
+
+        gcs_config_bucket("litellm-configs")
+
+        assert asyncio.all_tasks() - running_before == set()

tests/test_litellm/proxy/proxy_server/test_proxy_config.py

@@ -10,6 +10,7 @@ from __future__ import annotations
 
 import asyncio
 import json
+import logging
 import os
 import re
 from types import SimpleNamespace
@@ -772,6 +773,29 @@ async def test_ProxyConfig__process_includes_still_reads_a_nested_include_left_b
     assert result == {"model_list": [{"model_name": "first"}, {"model_name": "second"}]}
 
 
+@pytest.mark.asyncio
+async def test_ProxyConfig__process_includes_names_both_files_when_a_nested_include_matches_two(tmp_path, caplog):
+    (tmp_path / "shared").mkdir()
+    (tmp_path / "shared" / "models.yaml").write_text(
+        "include:\n  - more_models.yaml\nmodel_list:\n  - model_name: first\n"
+    )
+    (tmp_path / "shared" / "more_models.yaml").write_text("model_list:\n  - model_name: next-to-the-declaring-file\n")
+    (tmp_path / "more_models.yaml").write_text("model_list:\n  - model_name: next-to-the-root-config\n")
+
+    with caplog.at_level(logging.WARNING, logger="LiteLLM Proxy"):
+        result = await ProxyConfig()._process_includes(
+            {"include": ["shared/models.yaml"]}, config_file_path=str(tmp_path / "config.yaml")
+        )
+
+    assert result == {"model_list": [{"model_name": "first"}, {"model_name": "next-to-the-declaring-file"}]}
+    assert [
+        record
+        for record in caplog.records
+        if str(tmp_path / "shared" / "more_models.yaml") in record.getMessage()
+        and str(tmp_path / "more_models.yaml") in record.getMessage()
+    ]
+
+
 @pytest.mark.asyncio
 async def test_ProxyConfig__process_includes_merges_a_shared_file_once(tmp_path):
     (tmp_path / "shared.yaml").write_text("model_list:\n  - model_name: shared\n")