refactor(plugins): decouple plugin framework from gateway settings#2829
refactor(plugins): decouple plugin framework from gateway settings#2829
Conversation
420f906 to
f0dd01b
Compare
479fe8e to
9b4721b
Compare
crivetimihai
left a comment
There was a problem hiding this comment.
Thanks for this refactor, @araujof. Centralizing scattered os.environ.get calls into a typed PluginsSettings class is a solid improvement. A few issues:
1. LazySettingsWrapper.enabled bypasses pydantic-settings (settings.py:153-162)
The enabled property reads os.getenv("PLUGINS_ENABLED") directly and falls back to False. This skips pydantic-settings' .env file loading. If a user sets PLUGINS_ENABLED=true only in .env (not exported), os.getenv returns None and the property returns False, while PluginsSettings().enabled would return True. The property should delegate to get_settings().enabled like __getattr__ does for all other attributes.
2. from_env() methods create redundant PluginsSettings() instances
Every from_env() call (~7 call sites) creates a new PluginsSettings(), re-parsing env vars each time. The module provides get_settings() with @lru_cache specifically for this — use it instead.
3. Tests are thorough — good coverage of defaults, env overrides, alias compatibility, and singleton stability.
Good catch. Fixing bugs. |
Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com>
Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com>
Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com>
Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com>
Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com>
Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com>
Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com>
Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com>
9b4721b to
2263dac
Compare
Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com>
Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com>
|
@crivetimihai I rebased from
|
Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com>
Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com>
Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com>
Description
Introduces a self-contained
PluginsSettingsconfiguration class for the plugin framework, eliminating allos.environreads and imports frommcpgateway.config.settingsandmcpgateway.services.http_client_service. The plugin framework now owns its configuration and can operate independently of the gateway.Closes: #2831
Problem
Multiple plugin framework modules imported
settingsfrommcpgateway.config, the MCP client imported HTTP helpers frommcpgateway.services.http_client_service, and everyfrom_env()factory method inmodels.pyreados.environdirectly with hand-rolled boolean parsing. This created tight coupling from the plugin framework back into the gateway — the opposite of the desired direction for ADR-019 modular architecture.Solution
mcpgateway/plugins/framework/settings.pywith aPluginsSettingsclass usingpydantic_settingsand thePLUGINS_env var prefixPluginsSettings: core config, HTTP client tuning, SSL/TLS (mTLS client, server SSL, gRPC client/server TLS), server bind, transport runtime, Unix socket, and CLI optionsget_http_timeout()andget_default_verify()calls in the MCP client with direct usage ofsettings.httpx_*andsettings.skip_ssl_verifyfrom_env()factory methods inmodels.py(MCPClientTLSConfig,MCPServerTLSConfig,MCPServerConfig,GRPCClientTLSConfig,GRPCServerTLSConfig,GRPCServerConfig,UnixSocketServerConfig) from rawos.environreads toPluginsSettings_parse_bool()static methods fromMCPTransportTLSConfigBaseandMCPServerConfig(pydantic handles type coercion)get_plugin_manager(),cli.py, and all external server runtimes (MCP, gRPC, Unix) to use the framework's own settingssettings.pluginsproperty onLazySettingsWrapperso gateway code accesses plugin settings viasettings.plugins.enabledinstead ofsettings.plugins_enabledmain.py,admin.py,prompt_service.py,resource_service.py,tool_service.py) by replacing manualos.getenv("PLUGINS_ENABLED")parsing withsettings.plugins.enabledplugins_enabledandplugin_config_filefields from gatewaySettingsclassPYTHONconstant fromconstants.pyPLUGIN_CONFIG_FILE→PLUGINS_CONFIG_FILE(legacy name supported viaAliasChoicesfor backwards compatibility)Configuration mapping
Field names are chosen so that the
PLUGINS_env prefix produces clean env var names (e.g., fieldenabled→PLUGINS_ENABLED).PLUGINS_ENABLEDPLUGINS_ENABLEDfalsePLUGIN_CONFIG_FILEPLUGINS_CONFIG_FILEplugins/config.yamlHTTPX_CONNECT_TIMEOUTPLUGINS_HTTPX_CONNECT_TIMEOUT5.0HTTPX_READ_TIMEOUTPLUGINS_HTTPX_READ_TIMEOUT120.0SKIP_SSL_VERIFYPLUGINS_SKIP_SSL_VERIFYfalsePLUGINS_CLI_COMPLETIONPLUGINS_CLI_COMPLETIONfalsePLUGINS_CLI_MARKUP_MODEPLUGINS_CLI_MARKUP_MODEPLUGINS_*)PluginsSettingsfor full list: mTLS, server SSL, gRPC TLS, server bind, transport, Unix socket)Changes
Core
mcpgateway/plugins/framework/settings.pyPluginsSettingsclass withPLUGINS_env prefix covering all plugin framework configuration (HTTP client, TLS/mTLS, server bind, transport, CLI)mcpgateway/plugins/framework/models.pyfrom_env()factory methods fromos.environreads toPluginsSettings; removed duplicate_parse_bool()methodsmcpgateway/plugins/framework/__init__.pyget_plugin_manager()usesmcpgateway.plugins.framework.settingsinstead ofmcpgateway.configmcpgateway/plugins/framework/external/mcp/client.pymcpgateway.config.settingsandmcpgateway.services.http_client_serviceimports with framework settings;httpx.Timeoutbuilt fromsettings.httpx_*valuesmcpgateway/plugins/framework/constants.pyPYTHONconstantmcpgateway/plugins/tools/cli.pymcpgateway.config.settingswithmcpgateway.plugins.framework.settingsExternal server runtimes
mcpgateway/plugins/framework/external/mcp/server/runtime.pyPLUGINS_TRANSPORTenv var read replaced withPluginsSettings().transportmcpgateway/plugins/framework/external/mcp/server/server.pyPLUGINS_CONFIG_PATHenv var read replaced withPluginsSettings().config_pathmcpgateway/plugins/framework/external/grpc/server/runtime.pyPLUGINS_CONFIG_PATHenv var read replaced withPluginsSettings().config_pathmcpgateway/plugins/framework/external/unix/server/runtime.pyPLUGINS_CONFIG_PATHandUNIX_SOCKET_PATHenv var reads replaced withPluginsSettings()Gateway integration
mcpgateway/config.pyplugins_enabledandplugin_config_filefields fromSettings; addedsettings.pluginsproperty onLazySettingsWrappermcpgateway/main.pyos.getenv("PLUGINS_ENABLED")parsing withsettings.plugins.enabled/settings.plugins.config_filemcpgateway/admin.pysettings.plugins_enabled→settings.plugins.enabledmcpgateway/services/prompt_service.pysettings.plugins.enabled/settings.plugins.config_filemcpgateway/services/resource_service.pysettings.plugins.enabled/settings.plugins.config_filemcpgateway/services/tool_service.pysettings.plugins.enabled/settings.plugins.config_filemcpgateway/tools/builder/common.pyPLUGIN_CONFIG_FILE→PLUGINS_CONFIG_FILEin K8s manifest generationDocumentation and configuration
.env.examplePLUGINS_*env vars into a single "Plugin Framework Settings" section with descriptionsdocs/docs/manage/configuration.mddocs/docs/using/plugins/index.mdPLUGINS_CONFIG_FILE; added standalone settings notedocs/docs/using/plugins/grpc-transport.mdPLUGINS_CONFIG_FILEalongsidePLUGIN_CONFIG_FILEdocs/docs/architecture/adr/019-modular-architecture-split.mdPLUGINS_CONFIG_FILEcharts/mcp-stack/values.yamlPLUGIN_CONFIG_FILE→PLUGINS_CONFIG_FILE; added standalone framework settingscharts/mcp-stack/templates/deployment-mcpgateway.yamlPLUGINS_CONFIG_FILEwith fallbackcharts/mcp-stack/README.mdplugins/AGENTS.mdPLUGIN_CONFIG_FILE→PLUGINS_CONFIG_FILEplugins/README.mdPLUGINS_CONFIG_FILEAGENTS.mdPLUGIN_CONFIG_FILE→PLUGINS_CONFIG_FILETests
tests/unit/.../framework/test_settings.pyPLUGINS_-prefixed env var overrides, legacy alias (PLUGIN_CONFIG_FILE), module singleton stabilitytests/unit/.../framework/test_plugin_models.pyfrom_env()tests to usePLUGINS_-prefixed env vars viaPluginsSettingstests/unit/.../framework/test_plugin_models_coverage.pyfrom_env()tests forPluginsSettingsmigrationtests/unit/.../grpc/test_grpc_models.pyfrom_env()tests forPluginsSettingsmigrationtests/unit/.../services/test_tool_service_coverage.pyPluginsSettingssingletontests/unit/.../services/test_resource_service_plugins.pyPluginsSettingssingletontests/unit/.../integration/test_cross_hook_context_sharing.pytests/unit/.../integration/test_resource_plugin_integration.pytests/unit/.../middleware/test_http_auth_integration.py