From 1af113e5fb2473aaff5890eccff562769b0ce00c Mon Sep 17 00:00:00 2001 From: Daniel McKnight <34697904+NeonDaniel@users.noreply.github.com> Date: Mon, 23 Oct 2023 15:28:37 -0700 Subject: [PATCH 1/2] Update for Helm deployment (#71) * Update containers to fully support ovos-config * WIP troubleshooting container init failures * Troubleshooting unit test failures * Troubleshooting unit test failures * Fix legacy config file checks * Replace 'update' with 'update_many' per PyMongo docs https://pymongo.readthedocs.io/en/stable/api/pymongo/collection.html#pymongo.collection.Collection.update_many Update command validation to reference known commands enum directly --------- Co-authored-by: Daniel McKnight --- chat_client/app.py | 2 +- chat_client/client_config.py | 21 +++++--- chat_server/server_config.py | 54 +++++++++++++++---- chat_server/server_utils/auth.py | 13 ++--- chat_server/tests/test_sio.py | 2 + dockerfiles/Dockerfile.base | 4 ++ .../database_utils/mongo_utils/structures.py | 2 +- utils/database_utils/mongodb_connector.py | 30 +++++------ 8 files changed, 88 insertions(+), 40 deletions(-) diff --git a/chat_client/app.py b/chat_client/app.py index a4cd0d82..96ad43dd 100644 --- a/chat_client/app.py +++ b/chat_client/app.py @@ -103,7 +103,7 @@ async def custom_http_exception_handler(request, exc): if exc.status_code == status.HTTP_404_NOT_FOUND: return RedirectResponse("/chats") - __cors_allowed_origins = os.environ.get("COST_ALLOWED_ORIGINS", "") or "*" + __cors_allowed_origins = os.environ.get("CORS_ALLOWED_ORIGINS", "") or "*" LOG.info(f"CORS_ALLOWED_ORIGINS={__cors_allowed_origins}") diff --git a/chat_client/client_config.py b/chat_client/client_config.py index eda244de..acf75365 100644 --- a/chat_client/client_config.py +++ b/chat_client/client_config.py @@ -32,11 +32,20 @@ from config import Configuration from utils.logging_utils import LOG -config_file_path = os.environ.get( +config_file_path = os.path.expanduser(os.environ.get( "CHATCLIENT_CONFIG", "~/.local/share/neon/credentials_client.json" -) - -config = Configuration(from_files=[config_file_path]) -app_config = config.get("CHAT_CLIENT", {}).get(Configuration.KLAT_ENV) - +)) +if os.path.isfile(config_file_path): + LOG.warning(f"Using legacy configuration at {config_file_path}") + config = Configuration(from_files=[config_file_path]) + app_config = config.get("CHAT_CLIENT", {}).get(Configuration.KLAT_ENV) +else: + # ovos-config has built-in mechanisms for loading configuration files based + # on envvars, so the configuration structure is simplified + from ovos_config.config import Configuration + app_config = Configuration().get("CHAT_CLIENT") + env_spec = os.environ.get("KLAT_ENV") + if env_spec and app_config.get(env_spec): + LOG.warning("Legacy configuration handling KLAT_ENV envvar") + app_config = app_config.get(env_spec) LOG.info(f"App config: {app_config}") diff --git a/chat_server/server_config.py b/chat_server/server_config.py index 7d0acbc6..5c3bea0b 100644 --- a/chat_server/server_config.py +++ b/chat_server/server_config.py @@ -27,26 +27,62 @@ # SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. import os +from typing import Optional from config import Configuration from chat_server.server_utils.sftp_utils import init_sftp_connector from utils.logging_utils import LOG +from utils.database_utils import DatabaseController -server_config_path = os.environ.get( +server_config_path = os.path.expanduser(os.environ.get( "CHATSERVER_CONFIG", "~/.local/share/neon/credentials.json" -) -database_config_path = os.environ.get( +)) +database_config_path = os.path.expanduser(os.environ.get( "DATABASE_CONFIG", "~/.local/share/neon/credentials.json" -) +)) -LOG.info(f"KLAT_ENV : {Configuration.KLAT_ENV}") -config = Configuration(from_files=[server_config_path, database_config_path]) +def _init_db_controller(db_config: dict) -> Optional[DatabaseController]: + from chat_server.server_utils.db_utils import DbUtils -app_config = config.get("CHAT_SERVER", {}).get(Configuration.KLAT_ENV, {}) + # Determine configured database dialect + dialect = db_config.pop("dialect", "mongo") + + try: + # Create a database connection + db_controller = DatabaseController(config_data=db_config) + db_controller.attach_connector(dialect=dialect) + db_controller.connect() + except Exception as e: + LOG.exception(f"DatabaseController init failed: {e}") + return None + + # Initialize convenience class + DbUtils.init(db_controller) + return db_controller -LOG.info(f"App config: {app_config}") -db_controller = config.get_db_controller(name="pyklatchat_3333") +if os.path.isfile(server_config_path) or os.path.isfile(database_config_path): + LOG.warning(f"Using legacy configuration at {server_config_path}") + LOG.warning(f"Using legacy configuration at {database_config_path}") + LOG.info(f"KLAT_ENV : {Configuration.KLAT_ENV}") + config = Configuration(from_files=[server_config_path, database_config_path]) + app_config = config.get("CHAT_SERVER", {}).get(Configuration.KLAT_ENV, {}) + db_controller = config.get_db_controller(name="pyklatchat_3333") +else: + # ovos-config has built-in mechanisms for loading configuration files based + # on envvars, so the configuration structure is simplified + from ovos_config.config import Configuration + app_config = Configuration().get("CHAT_SERVER") or dict() + env_spec = os.environ.get("KLAT_ENV") + if env_spec and app_config.get(env_spec): + LOG.warning("Legacy configuration handling KLAT_ENV envvar") + app_config = app_config.get(env_spec) + db_controller = _init_db_controller(app_config.get("connection_properties", + Configuration().get( + "DATABASE_CONFIG", + {}))) + +LOG.info(f"App config: {app_config}") sftp_connector = init_sftp_connector(config=app_config.get("SFTP", {})) diff --git a/chat_server/server_utils/auth.py b/chat_server/server_utils/auth.py index 5ad7b0ae..4d21ef27 100644 --- a/chat_server/server_utils/auth.py +++ b/chat_server/server_utils/auth.py @@ -149,6 +149,7 @@ def create_unauthorized_user( query={"document": "users", "command": "insert_one", "data": new_user} ) token = generate_session_token(user_id=new_user["_id"]) if authorize else "" + LOG.debug(f"Created new user with name {new_user['nickname']}") return UserData(user=new_user, session=token) @@ -168,8 +169,7 @@ def get_current_user_data( :returns UserData based on received authorization header or sets temporal user credentials if not found """ - user_id = None - user_data = {} + user_data: UserData = None if not force_tmp: if nano_token: user = db_controller.exec_query( @@ -219,11 +219,12 @@ def get_current_user_data( LOG.info("Session was refreshed") user_data = UserData(user=user, session=session) except BaseException as ex: - LOG.warning( - f"Problem resolving current user: {ex}, setting tmp user credentials" - ) - if not user_id or force_tmp: + LOG.exception(f"Problem resolving current user: {ex}\n" + f"setting tmp user credentials") + if not user_data: + LOG.debug("Creating temp user") user_data = create_unauthorized_user() + LOG.debug(f"Resolved user: {user_data}") user_data.user.pop("password", None) user_data.user.pop("date_created", None) user_data.user.pop("tokens", None) diff --git a/chat_server/tests/test_sio.py b/chat_server/tests/test_sio.py index a8c9a0b9..c065766b 100644 --- a/chat_server/tests/test_sio.py +++ b/chat_server/tests/test_sio.py @@ -68,6 +68,8 @@ class TestSIO(unittest.TestCase): @classmethod def setUpClass(cls) -> None: + from chat_server.server_config import database_config_path + assert os.path.isfile(database_config_path) os.environ["DISABLE_AUTH_CHECK"] = "1" matching_conversation = db_controller.exec_query( query={ diff --git a/dockerfiles/Dockerfile.base b/dockerfiles/Dockerfile.base index 89984eb2..2802140f 100644 --- a/dockerfiles/Dockerfile.base +++ b/dockerfiles/Dockerfile.base @@ -1,5 +1,9 @@ FROM python:3.10-slim-buster +ENV OVOS_CONFIG_BASE_FOLDER neon +ENV OVOS_CONFIG_FILENAME klat.yaml +ENV XDG_CONFIG_HOME /config + COPY . /app/ WORKDIR /app diff --git a/utils/database_utils/mongo_utils/structures.py b/utils/database_utils/mongo_utils/structures.py index c750020b..043c8e7f 100644 --- a/utils/database_utils/mongo_utils/structures.py +++ b/utils/database_utils/mongo_utils/structures.py @@ -51,7 +51,7 @@ class MongoCommands(Enum): DELETE_ONE = "delete_one" DELETE_MANY = "delete_many" # Update operation - UPDATE = "update" + UPDATE = "update_many" class MongoDocuments(Enum): diff --git a/utils/database_utils/mongodb_connector.py b/utils/database_utils/mongodb_connector.py index 202834b1..27483a74 100644 --- a/utils/database_utils/mongodb_connector.py +++ b/utils/database_utils/mongodb_connector.py @@ -25,26 +25,18 @@ # LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING # NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS # SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + from typing import Optional, Union from pymongo import MongoClient from utils.database_utils.base_connector import DatabaseConnector, DatabaseTypes -from utils.database_utils.mongo_utils.structures import MongoQuery +from utils.database_utils.mongo_utils.structures import MongoQuery, MongoCommands +from utils.logging_utils import LOG class MongoDBConnector(DatabaseConnector): """Connector implementing interface for interaction with Mongo DB API""" - - mongo_recognised_commands = ( - "insert_one", - "insert_many", - "delete_one", - "bulk_write", - "delete_many", - "find", - "find_one", - "update", - ) + mongo_recognised_commands = set(cmd.value for cmd in MongoCommands) @property def database_type(self) -> DatabaseTypes: @@ -81,13 +73,17 @@ def exec_raw_query( f"please use one of the following: " f"{self.mongo_recognised_commands}" ) - db_command = getattr( - self.connection[query.get("document")], query.get("command") - ) + db_command = getattr(self.connection[query.get("document")], + received_command) if not isinstance(query.get("data"), tuple): - # LOG.warning('Received wrong param type for query data, using default conversion to tuple') + LOG.debug(f'Casting data from {type(query["data"])} to tuple') query["data"] = (query.get("data", {}),) - query_output = db_command(*query.get("data"), *args, **kwargs) + try: + query_output = db_command(*query.get("data"), *args, **kwargs) + except Exception as e: + LOG.error(f"Query failed: {query}|args={args}|kwargs={kwargs}") + raise e + if received_command == "find": filters = query.get("filters", {}) if filters: From 3ab7ecd1e69ffd41f724ca90baca8bcd99885b76 Mon Sep 17 00:00:00 2001 From: NeonDaniel Date: Mon, 23 Oct 2023 22:29:21 +0000 Subject: [PATCH 2/2] Increment Version --- version.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/version.py b/version.py index 01bf3fb1..4bd3e03c 100644 --- a/version.py +++ b/version.py @@ -26,4 +26,4 @@ # NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS # SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -__version__ = "0.4.2a0" +__version__ = "0.4.2a1"