From 69fd2ae6b12ec1e25f85f11bcf9d1f023844d0d5 Mon Sep 17 00:00:00 2001 From: oshannanayakkara Date: Fri, 14 Aug 2026 12:59:06 +0530 Subject: [PATCH] aws bedrock fallback removal fix (#510) Co-authored-by: Charith Nuwan Bimsara <59943919+nuwangeek@users.noreply.github.com> --- .../Vault-Init/templates/configmap.yaml | 7 ---- src/llm_orchestration_service_api.py | 12 ++++++ src/llm_orchestrator_config/config/loader.py | 38 +++++++++++++++---- src/llm_orchestrator_config/llm_manager.py | 4 ++ .../vault/secret_resolver.py | 21 +++++++++- vault-init.sh | 7 ---- 6 files changed, 67 insertions(+), 22 deletions(-) diff --git a/kubernetes/charts/Vault-Init/templates/configmap.yaml b/kubernetes/charts/Vault-Init/templates/configmap.yaml index f8c02f6..036eb1e 100644 --- a/kubernetes/charts/Vault-Init/templates/configmap.yaml +++ b/kubernetes/charts/Vault-Init/templates/configmap.yaml @@ -304,13 +304,6 @@ data: rm -rf "$TEMP_KEY_DIR" echo "RSA keypair generated and stored successfully" - # Store test LLM credentials for testing - echo "Creating test LLM credentials..." - wget -q -O- --post-data='{"data":{"access_key":"TEST_AWS_ACCESS_KEY","secret_key":"TEST_AWS_SECRET_KEY","environment":"production","model":"claude-3"}}' \ - --header="X-Vault-Token: $ROOT_TOKEN" \ - --header='Content-Type: application/json' \ - "$VAULT_ADDR/v1/secret/data/llm/connections/aws_bedrock/production/claude-3" >/dev/null - # Mark as initialized touch "$INIT_FLAG" echo "=== First time setup complete ===" diff --git a/src/llm_orchestration_service_api.py b/src/llm_orchestration_service_api.py index df1f842..d5a9b7b 100644 --- a/src/llm_orchestration_service_api.py +++ b/src/llm_orchestration_service_api.py @@ -38,6 +38,11 @@ ) from src.llm_orchestrator_config.stream_config import StreamConfig from src.llm_orchestrator_config.exceptions import StreamTimeoutError + +# NOTE: imported via the bare package path, not "src.llm_orchestrator_config". +# Both spellings resolve to separate module objects at runtime, so the class +# imported here must match the one the config loader raises or `except` misses. +from llm_orchestrator_config.exceptions import ConfigurationError from src.utils.stream_timeout import stream_timeout from src.utils.observation_utils import safe_observation_context from src.utils.error_utils import generate_error_id, log_error_with_context @@ -813,6 +818,13 @@ async def generate_context_with_caching( return ContextGenerationResponse(**result) + except ConfigurationError as e: + # No usable LLM connection for this environment. This is an operator + # action, not a transient fault - 503 with the reason so callers (e.g. + # the vector indexer) can stop retrying and surface something useful. + error_id = generate_error_id() + log_error_with_context(logger, error_id, "context_generation_endpoint", None, e) + raise HTTPException(status_code=503, detail=str(e)) from e except Exception as e: error_id = generate_error_id() log_error_with_context(logger, error_id, "context_generation_endpoint", None, e) diff --git a/src/llm_orchestrator_config/config/loader.py b/src/llm_orchestrator_config/config/loader.py index 001e306..7506fda 100644 --- a/src/llm_orchestrator_config/config/loader.py +++ b/src/llm_orchestrator_config/config/loader.py @@ -135,6 +135,11 @@ def load_config(self) -> LLMConfiguration: except yaml.YAMLError as e: raise ConfigurationError(f"Failed to parse YAML configuration: {e}") from e except Exception as e: + # Already a ConfigurationError with a specific, operator-facing + # message (e.g. "No production LLM connection configured") - keep it + # verbatim instead of nesting another prefix in front of it. + if isinstance(e, ConfigurationError): + raise raise ConfigurationError(f"Failed to load configuration: {e}") from e def _resolve_vault_secrets(self, config: Dict[str, Any]) -> Dict[str, Any]: @@ -199,11 +204,17 @@ def _resolve_provider_secrets( if "providers" not in config: return - # connection_id (vault_uuid) is required for all environments + # connection_id (vault_uuid) is required for all environments. A missing + # one means no connection row exists for this environment at all - the + # operator has not configured one yet, so say that rather than naming an + # internal field. if not self.connection_id: - raise ConfigurationError( - f"connection_id (vault_uuid) is required for {self.environment} environment" + logger.error( + f"No {self.environment} LLM connection configured. " + f"Create a {self.environment} connection so its credentials are " + f"stored in Vault." ) + raise ConfigurationError(f"No {self.environment} LLM connection configured") try: providers_to_update: Dict[str, Dict[str, Any]] = {} @@ -243,8 +254,12 @@ def _resolve_provider_secrets( f"(vault_uuid: {self.connection_id})" ) else: + # Either nothing is stored for this provider under this + # connection, or what is stored failed validation - the + # resolver logs which one. logger.warning( - f"No secret found for {provider_name} with vault_uuid {self.connection_id}" + f"No usable {provider_name} credentials for vault_uuid " + f"{self.connection_id} - skipping this provider" ) except Exception as e: @@ -256,11 +271,20 @@ def _resolve_provider_secrets( # Continue to next provider instead of failing completely continue - # Check if we have any providers configured + # Check if we have any providers configured. Reaching here means a + # connection row exists but none of its provider secrets could be + # loaded from Vault - either the cron has not written them yet or + # what it wrote does not match the expected schema (see the + # per-provider warnings above for which, and why). if not providers_to_update: + logger.error( + f"No usable LLM provider for the {self.environment} connection " + f"(vault_uuid: {self.connection_id}). The connection exists but " + f"none of its credentials could be loaded from Vault - see the " + f"per-provider messages above." + ) raise ConfigurationError( - f"No providers available for {self.environment} environment " - f"with vault_uuid {self.connection_id}" + f"No usable LLM provider for the {self.environment} connection" ) # Update the configuration with only available providers diff --git a/src/llm_orchestrator_config/llm_manager.py b/src/llm_orchestrator_config/llm_manager.py index bbb0c5f..4ebd21a 100644 --- a/src/llm_orchestrator_config/llm_manager.py +++ b/src/llm_orchestrator_config/llm_manager.py @@ -90,6 +90,10 @@ def _load_configuration(self) -> None: """ try: self._config = self._config_loader.load_config() + except ConfigurationError: + # The loader's message already identifies the environment and what + # the operator needs to do; re-wrapping only buries it. + raise except Exception as e: raise ConfigurationError(f"Failed to load LLM configuration: {e}") from e diff --git a/src/llm_orchestrator_config/vault/secret_resolver.py b/src/llm_orchestrator_config/vault/secret_resolver.py index 01f615f..2c32887 100644 --- a/src/llm_orchestrator_config/vault/secret_resolver.py +++ b/src/llm_orchestrator_config/vault/secret_resolver.py @@ -3,7 +3,7 @@ import threading from datetime import datetime, timedelta from typing import Optional, Dict, Any, Union, List -from pydantic import BaseModel +from pydantic import BaseModel, ValidationError from src.loki_logger import LokiLogger from llm_orchestrator_config.vault.vault_client import ( @@ -122,6 +122,16 @@ def get_secret_for_model( except VaultConnectionError: logger.warning(f"Vault unavailable, trying fallback for {vault_path}") return self._get_fallback(vault_path) + except ValidationError as e: + # The secret exists but does not match the provider's schema. A + # last-known-good fallback would mask a stored-data problem that no + # retry can fix, so report it as a misconfiguration and stop here. + logger.error( + f"Malformed secret at {vault_path} for provider {provider} - the " + f"stored value does not match {get_secret_model(provider).__name__} " + f"and must be rewritten: {e}" + ) + return None except Exception as e: logger.error(f"Error resolving secret for {vault_path}: {e}") return self._get_fallback(vault_path) @@ -357,6 +367,15 @@ def get_embedding_secret_for_model( f"Vault unavailable, trying fallback for embedding {vault_path}" ) return self._get_fallback(vault_path) + except ValidationError as e: + # See get_secret_for_model: a schema mismatch is a stored-data + # problem, not a transient one, so do not serve a stale fallback. + logger.error( + f"Malformed embedding secret at {vault_path} for provider {provider} - " + f"the stored value does not match " + f"{get_secret_model(provider).__name__} and must be rewritten: {e}" + ) + return None except Exception as e: logger.error(f"Error resolving embedding secret for {vault_path}: {e}") return self._get_fallback(vault_path) diff --git a/vault-init.sh b/vault-init.sh index 0c43709..44af6bb 100644 --- a/vault-init.sh +++ b/vault-init.sh @@ -295,13 +295,6 @@ path "auth/token/lookup-self" { capabilities = ["read"] }' rm -rf "$TEMP_KEY_DIR" echo "RSA keypair generated and stored successfully" - # Store test LLM credentials for testing - echo "Creating test LLM credentials..." - wget -q -O- --post-data='{"data":{"access_key":"TEST_AWS_ACCESS_KEY","secret_key":"TEST_AWS_SECRET_KEY","environment":"production","model":"claude-3"}}' \ - --header="X-Vault-Token: $ROOT_TOKEN" \ - --header='Content-Type: application/json' \ - "$VAULT_ADDR/v1/secret/data/llm/connections/aws_bedrock/production/claude-3" >/dev/null - # Mark as initialized touch "$INIT_FLAG" echo "=== First time setup complete ==="