mirror of
https://github.com/BerriAI/litellm.git
synced 2026-08-28 05:25:59 +00:00
fix(ci): make the env-key doc gate see get_secret_bool reads (#35833)
The gate only matched os.getenv(, litellm.get_secret( and
litellm.get_secret_str(, so a bare get_secret_bool("X") matched nothing and
the key bypassed the documentation requirement entirely. Add a fourth pattern
for get_secret_bool, with or without the litellm. prefix, and a negative
lookbehind so an unrelated receiver's .get_secret*( call is not mistaken for
an env var read.
Extraction and table parsing move into functions behind a __main__ guard so
the patterns can be unit tested; the script is still invoked exactly the same
way by CI.
This surfaces 13 keys the gate never checked, 8 of which have no reference
row yet.
This commit is contained in:
Yassin Kortam
GitHub
parent
d3d30353aa
commit
0b8c58735d
2 changed files with 172 additions and 87 deletions
|
|
@ -1,20 +1,19 @@
|
|||
import os
|
||||
import re
|
||||
from collections.abc import Iterator
|
||||
|
||||
# Define the base directory for the litellm repository and documentation path
|
||||
repo_base = "./litellm" # Change this to your actual path
|
||||
|
||||
# Regular expressions to capture the keys used in os.getenv() and litellm.get_secret()
|
||||
getenv_pattern = re.compile(r'os\.getenv\(\s*[\'"]([^\'"]+)[\'"]\s*(?:,\s*[^)]*)?\)')
|
||||
get_secret_pattern = re.compile(
|
||||
r'litellm\.get_secret\(\s*[\'"]([^\'"]+)[\'"]\s*(?:,\s*[^)]*|,\s*default_value=[^)]*)?\)'
|
||||
)
|
||||
get_secret_str_pattern = re.compile(
|
||||
r'litellm\.get_secret_str\(\s*[\'"]([^\'"]+)[\'"]\s*(?:,\s*[^)]*|,\s*default_value=[^)]*)?\)'
|
||||
)
|
||||
_GETENV_ARGS = r"""\(\s*['"]([^'"]+)['"]\s*(?:,\s*[^)]*)?\)"""
|
||||
_GET_SECRET_ARGS = r"""\(\s*['"]([^'"]+)['"]\s*(?:,\s*[^)]*|,\s*default_value=[^)]*)?\)"""
|
||||
|
||||
# Set to store unique keys from the code
|
||||
env_keys = set()
|
||||
ENV_KEY_PATTERNS: tuple[re.Pattern[str], ...] = (
|
||||
re.compile(r"os\.getenv" + _GETENV_ARGS),
|
||||
re.compile(r"litellm\.get_secret" + _GET_SECRET_ARGS),
|
||||
re.compile(r"litellm\.get_secret_str" + _GET_SECRET_ARGS),
|
||||
re.compile(r"(?<![\w.])(?:litellm\.)?get_secret_bool" + _GET_SECRET_ARGS),
|
||||
)
|
||||
|
||||
# Terminal/environment detection variables that should not be documented
|
||||
# These are internal variables used for terminal detection, not user-configurable settings
|
||||
|
|
@ -48,6 +47,8 @@ EXCLUDED_TERMINAL_VARS = {
|
|||
"ALACRITTY_SOCKET",
|
||||
}
|
||||
|
||||
EXCLUDED_KEYS = frozenset(EXCLUDED_TERMINAL_VARS | EXCLUDED_GUARD_ONLY_VARS | EXCLUDED_ROLLOUT_FLAGS)
|
||||
|
||||
# Directories to skip (dependencies, venvs, caches) - only scan litellm source
|
||||
SKIP_DIRS = {
|
||||
".venv",
|
||||
|
|
@ -61,88 +62,69 @@ SKIP_DIRS = {
|
|||
"build",
|
||||
}
|
||||
|
||||
# Walk through all files in the litellm repo to find references of os.getenv() and litellm.get_secret()
|
||||
for root, dirs, files in os.walk(repo_base):
|
||||
# Skip dependency/venv directories - prevents picking up env vars from installed packages
|
||||
dirs[:] = [d for d in dirs if d not in SKIP_DIRS]
|
||||
for file in files:
|
||||
if file.endswith(".py"): # Only process Python files
|
||||
file_path = os.path.join(root, file)
|
||||
with open(file_path, "r", encoding="utf-8") as f:
|
||||
content = f.read()
|
||||
|
||||
# Find all keys using os.getenv()
|
||||
getenv_matches = getenv_pattern.findall(content)
|
||||
env_keys.update(
|
||||
match
|
||||
for match in getenv_matches
|
||||
if match not in EXCLUDED_TERMINAL_VARS
|
||||
and match not in EXCLUDED_GUARD_ONLY_VARS
|
||||
and match not in EXCLUDED_ROLLOUT_FLAGS
|
||||
) # Extract only the key part, excluding terminal vars
|
||||
|
||||
# Find all keys using litellm.get_secret()
|
||||
get_secret_matches = get_secret_pattern.findall(content)
|
||||
env_keys.update(match for match in get_secret_matches)
|
||||
|
||||
# Find all keys using litellm.get_secret_str()
|
||||
get_secret_str_matches = get_secret_str_pattern.findall(content)
|
||||
env_keys.update(match for match in get_secret_str_matches)
|
||||
|
||||
# Print the unique keys found
|
||||
print(env_keys)
|
||||
|
||||
|
||||
# Parse the documentation to extract documented keys
|
||||
repo_base = "./"
|
||||
print(os.listdir(repo_base))
|
||||
docs_path = (
|
||||
"./docs/my-website/docs/proxy/config_settings.md" # Path to the documentation
|
||||
)
|
||||
documented_keys = set()
|
||||
try:
|
||||
with open(docs_path, "r", encoding="utf-8") as docs_file:
|
||||
content = docs_file.read()
|
||||
|
||||
print(f"content: {content}")
|
||||
|
||||
# Find the section titled "general_settings - Reference"
|
||||
general_settings_section = re.search(
|
||||
r"### environment variables - Reference(.*?)(?=\n###|\Z)",
|
||||
content,
|
||||
re.DOTALL | re.MULTILINE,
|
||||
)
|
||||
print(f"general_settings_section: {general_settings_section}")
|
||||
if general_settings_section:
|
||||
# Extract the table rows - only first column (key name) from each row
|
||||
table_content = general_settings_section.group(1)
|
||||
for line in table_content.split("\n"):
|
||||
# Match | KEY_NAME | description | - capture first column only
|
||||
match = re.match(r"^\|\s*([A-Z_][A-Z0-9_]*)\s*\|", line)
|
||||
if match:
|
||||
documented_keys.add(match.group(1).strip())
|
||||
except Exception as e:
|
||||
raise Exception(
|
||||
f"Error reading documentation: {e}, \n repo base - {os.listdir(repo_base)}"
|
||||
def extract_env_keys(source: str) -> frozenset[str]:
|
||||
"""Return every documentable env var name read by the given Python source."""
|
||||
return frozenset(
|
||||
match for pattern in ENV_KEY_PATTERNS for match in pattern.findall(source) if match not in EXCLUDED_KEYS
|
||||
)
|
||||
|
||||
|
||||
print(f"documented_keys: {documented_keys}")
|
||||
# Compare and find undocumented keys
|
||||
undocumented_keys = env_keys - documented_keys
|
||||
def collect_env_keys(base_dir: str) -> frozenset[str]:
|
||||
"""Return every documentable env var name read anywhere under ``base_dir``."""
|
||||
return frozenset(key for file_path in _python_files(base_dir) for key in extract_env_keys(_read_text(file_path)))
|
||||
|
||||
# Print results
|
||||
print("Keys expected in 'environment settings' (found in code):")
|
||||
for key in sorted(env_keys):
|
||||
print(key)
|
||||
|
||||
if undocumented_keys:
|
||||
raise Exception(
|
||||
f"\nKeys not documented in 'environment settings - Reference': {undocumented_keys}"
|
||||
def _python_files(base_dir: str) -> Iterator[str]:
|
||||
for root, dirs, files in os.walk(base_dir):
|
||||
# Skip dependency/venv directories - prevents picking up env vars from installed packages
|
||||
dirs[:] = [d for d in dirs if d not in SKIP_DIRS]
|
||||
yield from (os.path.join(root, name) for name in files if name.endswith(".py"))
|
||||
|
||||
|
||||
def _read_text(file_path: str) -> str:
|
||||
with open(file_path, "r", encoding="utf-8") as f:
|
||||
return f.read()
|
||||
|
||||
|
||||
def extract_documented_keys(docs_content: str) -> frozenset[str]:
|
||||
"""Return the key names listed in the 'environment variables - Reference' table."""
|
||||
section = re.search(
|
||||
r"### environment variables - Reference(.*?)(?=\n###|\Z)",
|
||||
docs_content,
|
||||
re.DOTALL | re.MULTILINE,
|
||||
)
|
||||
else:
|
||||
print(
|
||||
"\nAll keys are documented in 'environment settings - Reference'. - {}".format(
|
||||
env_keys
|
||||
)
|
||||
if section is None:
|
||||
return frozenset()
|
||||
# Match | KEY_NAME | description | - capture first column only
|
||||
return frozenset(
|
||||
match.group(1).strip()
|
||||
for match in (re.match(r"^\|\s*([A-Z_][A-Z0-9_]*)\s*\|", line) for line in section.group(1).split("\n"))
|
||||
if match is not None
|
||||
)
|
||||
|
||||
|
||||
def main() -> None:
|
||||
env_keys = collect_env_keys(repo_base)
|
||||
print(env_keys)
|
||||
|
||||
docs_path = "./docs/my-website/docs/proxy/config_settings.md" # Path to the documentation
|
||||
try:
|
||||
documented_keys = extract_documented_keys(_read_text(docs_path))
|
||||
except Exception as e:
|
||||
raise Exception(f"Error reading documentation: {e}, \n repo base - {os.listdir('./')}")
|
||||
|
||||
print(f"documented_keys: {documented_keys}")
|
||||
undocumented_keys = env_keys - documented_keys
|
||||
|
||||
print("Keys expected in 'environment settings' (found in code):")
|
||||
for key in sorted(env_keys):
|
||||
print(key)
|
||||
|
||||
if undocumented_keys:
|
||||
raise Exception(f"\nKeys not documented in 'environment settings - Reference': {sorted(undocumented_keys)}")
|
||||
print(f"\nAll keys are documented in 'environment settings - Reference'. - {env_keys}")
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
main()
|
||||
|
|
|
|||
103
tests/test_litellm/test_env_key_doc_gate.py
Normal file
103
tests/test_litellm/test_env_key_doc_gate.py
Normal file
|
|
@ -0,0 +1,103 @@
|
|||
"""Tests for the env-var extraction used by tests/documentation_tests/test_env_keys.py.
|
||||
|
||||
That script is the CI gate that fails when a user-facing environment variable read
|
||||
under litellm/ has no row in the docs reference table. It only sees a key if one of its
|
||||
patterns matches the call, so a call shape the patterns miss silently bypasses the gate.
|
||||
Each supported shape is asserted here, along with the shapes that must not be treated as
|
||||
env var reads, so narrowing a pattern makes a test fail instead of quietly reopening the
|
||||
hole.
|
||||
"""
|
||||
|
||||
import importlib.util
|
||||
import sys
|
||||
from pathlib import Path
|
||||
|
||||
_REPO_ROOT = Path(__file__).resolve().parents[2]
|
||||
_MODULE_PATH = _REPO_ROOT / "tests" / "documentation_tests" / "test_env_keys.py"
|
||||
_spec = importlib.util.spec_from_file_location("documentation_test_env_keys", _MODULE_PATH)
|
||||
assert _spec is not None and _spec.loader is not None
|
||||
gate = importlib.util.module_from_spec(_spec)
|
||||
sys.modules[_spec.name] = gate
|
||||
_spec.loader.exec_module(gate)
|
||||
|
||||
|
||||
def test_bare_get_secret_bool_is_captured() -> None:
|
||||
assert gate.extract_env_keys('flag = get_secret_bool("QSTASH_FLUSH_ON_BOOT")') == {"QSTASH_FLUSH_ON_BOOT"}
|
||||
|
||||
|
||||
def test_get_secret_bool_with_default_is_captured() -> None:
|
||||
assert gate.extract_env_keys('if get_secret_bool("QSTASH_FLUSH_ON_BOOT", False) is not True:') == {
|
||||
"QSTASH_FLUSH_ON_BOOT"
|
||||
}
|
||||
|
||||
|
||||
def test_get_secret_bool_with_keyword_default_is_captured() -> None:
|
||||
assert gate.extract_env_keys('get_secret_bool("QSTASH_FLUSH_ON_BOOT", default_value=False)') == {
|
||||
"QSTASH_FLUSH_ON_BOOT"
|
||||
}
|
||||
|
||||
|
||||
def test_litellm_prefixed_get_secret_bool_is_captured() -> None:
|
||||
assert gate.extract_env_keys('litellm.get_secret_bool("QSTASH_FLUSH_ON_BOOT")') == {"QSTASH_FLUSH_ON_BOOT"}
|
||||
|
||||
|
||||
def test_previously_supported_call_shapes_are_still_captured() -> None:
|
||||
source = "\n".join(
|
||||
(
|
||||
'os.getenv("QSTASH_ALPHA")',
|
||||
'os.getenv("QSTASH_BRAVO", "fallback")',
|
||||
'litellm.get_secret("QSTASH_CHARLIE")',
|
||||
'litellm.get_secret_str("QSTASH_DELTA", default_value=None)',
|
||||
)
|
||||
)
|
||||
assert gate.extract_env_keys(source) == {"QSTASH_ALPHA", "QSTASH_BRAVO", "QSTASH_CHARLIE", "QSTASH_DELTA"}
|
||||
|
||||
|
||||
def test_get_secret_calls_on_unrelated_objects_are_not_env_reads() -> None:
|
||||
source = "\n".join(
|
||||
(
|
||||
'vault_client.get_secret("QSTASH_ALPHA")',
|
||||
'self.get_secret_str("QSTASH_BRAVO")',
|
||||
'provider.get_secret_bool("QSTASH_CHARLIE")',
|
||||
)
|
||||
)
|
||||
assert gate.extract_env_keys(source) == frozenset()
|
||||
|
||||
|
||||
def test_similarly_named_helpers_are_not_env_reads() -> None:
|
||||
assert gate.extract_env_keys('get_secret_bundle("QSTASH_ALPHA")') == frozenset()
|
||||
|
||||
|
||||
def test_non_literal_arguments_are_not_env_reads() -> None:
|
||||
assert gate.extract_env_keys("get_secret_bool(flag_name)") == frozenset()
|
||||
|
||||
|
||||
def test_excluded_keys_are_filtered_for_every_call_shape() -> None:
|
||||
source = "\n".join(
|
||||
(
|
||||
'os.getenv("TERM_PROGRAM")',
|
||||
'get_secret_bool("LITELLM_RUST")',
|
||||
'litellm.get_secret_str("MAVVRIK_FOCUS_FREQUENCY")',
|
||||
)
|
||||
)
|
||||
assert gate.extract_env_keys(source) == frozenset()
|
||||
|
||||
|
||||
def test_documented_keys_are_read_from_the_reference_table_only() -> None:
|
||||
docs = "\n".join(
|
||||
(
|
||||
"### general_settings - Reference",
|
||||
"| BEFORE_THE_TABLE | not the env var table",
|
||||
"",
|
||||
"### environment variables - Reference",
|
||||
"",
|
||||
"| Name | Description |",
|
||||
"|------|-------------|",
|
||||
"| QSTASH_ALPHA | first key",
|
||||
"| QSTASH_BRAVO | second key",
|
||||
"",
|
||||
"### another section - Reference",
|
||||
"| AFTER_THE_TABLE | also not the env var table",
|
||||
)
|
||||
)
|
||||
assert gate.extract_documented_keys(docs) == {"QSTASH_ALPHA", "QSTASH_BRAVO"}
|
||||
Loading…
Add table
Reference in a new issue