Bug/285 (#289)

* Add warning in documentation recommending against running 'mp-init' as root

* Gate project creation when running as root with warning

* If running as root, ensure logs directory is writable by container

* Warn if logs directory is not writable by current user

* Improve server error handling when logging directory is not writable

* Improve Python Adapter Library error handling when logging directory is not writable

* Update Changelog

CHANGELOG.md

@@ -1,5 +1,11 @@
 VMware Aria Operations Integration SDK
 --------------------------------------
+## 1.0.1 (09-16-2023)
+* Fix issue where server would repeatedly crash if logging directory was not writable
+  * If user runs mp-init using root, the logs directory's permissions are set to world-wriable
+  * If user runs mp-init as root, mp-init warns that the above will happen
+  * Improves error handling when logs directory is not writable to prevent server crashes
+
 ## 1.0.0 (07/26/2023)
 * Improved documentation site
 * Additional sample and template projects

docs/get_started.md

@@ -49,6 +49,9 @@ pipx install vmware-aria-operations-integration-sdk
 After the SDK is installed, create a new project, by running `mp-init`. This tool asks a series of questions that guide
 the creation of a new management pack project.
 
+???+ warning
+    Running `mp-init` as root is not recommended, as this requires some directories to have escalated permissions.
+
 
 1. `Enter a directory to create the project in. This is the directory where adapter code, metadata, and content will reside. If the directory doesn't already exist, it will be created. Path:`
 

docs/references/mp-init.md

@@ -11,6 +11,9 @@ an initial project structure and create classifiers that other tools and VMware
 ## Prerequisites
 * The [VMware Aria Operations Integration SDK](../get_started.md#installation) is installed, with the virtual environment active.
 
+???+ warning
+    Running `mp-init` as root is not recommended, as this requires some directories to have escalated permissions.
+
 ## Input
 
 ### Command-line Arguments

images/base-python-adapter/CHANGELOG.md

@@ -1,5 +1,8 @@
 VMware Aria Operations Integration Base Python Adapter
 ----------------------------------------------
+## 0.12.1 (09-15-2023)
+* Improve logging setup error handling: Ensure that if the logs directory is not writable the Adapter
+  will still run.
 ## 0.12.0 (05-03-2023)
 * Add `schema_version` to adapter definition endpoint to track the schema version used by the Adapter
 * Update Credential and Identifier enum type to support labels and display orders for each enum value

images/base-python-adapter/swagger_server/server_logging.py

@@ -18,22 +18,30 @@ def setup_logging(
     :param max_size The maximum size in bytes of each file before the file
                 automatically rotates to a new one. Defaults to '10_485_760' (10 MiB).
     """
-    try:
-        global log_handler
-        log_handler = RotatingFileHandler(
-            os.path.join(os.sep, "var", "log", filename),
-            maxBytes=max_size,
-            backupCount=file_count,
-        )
-        logging.basicConfig(
-            format="%(asctime)s,%(msecs)d %(name)s %(levelname)s %(message)s",
-            datefmt="%Y-%m-%d %H:%M:%S",
-            level=_get_default_log_level(),
-            handlers=[log_handler],
+    logdir = os.path.join(os.sep, "var", "log")
+    if os.access(logdir, os.W_OK):
+        try:
+            global log_handler
+            log_handler = RotatingFileHandler(
+                os.path.join(logdir, filename),
+                maxBytes=max_size,
+                backupCount=file_count,
+            )
+            logging.basicConfig(
+                format="%(asctime)s,%(msecs)d %(name)s %(levelname)s %(message)s",
+                datefmt="%Y-%m-%d %H:%M:%S",
+                level=_get_default_log_level(),
+                handlers=[log_handler],
+            )
+            update_log_levels()
+        except Exception as e:
+            logging.basicConfig(level=logging.INFO)
+            logging.exception(e)
+    else:
+        logging.basicConfig(level=logging.INFO)
+        logging.exception(
+            f"Cannot write to log file '{os.path.join(logdir, filename)}'"
         )
-        update_log_levels()
-    except Exception:
-        logging.basicConfig(level=logging.CRITICAL + 1)
 
 
 def _get_default_log_level(default_level: int = logging.INFO) -> int:
@@ -52,8 +60,11 @@ def _get_default_log_level(default_level: int = logging.INFO) -> int:
             "main": logging.getLevelName(default_level),
         }
     if modified:
-        with open(log_config_file, "w") as config_file:
-            config.write(config_file)
+        try:
+            with open(log_config_file, "w") as config_file:
+                config.write(config_file)
+        except Exception as e:
+            logging.exception(e)
     try:
         return int(
             logging.getLevelName(config["DEFAULT"].get("server", default_level_name))
@@ -76,9 +87,10 @@ def update_log_levels() -> None:
     # Set each logger in the config file to the level specified
     log_config_file = os.path.join(os.sep, "var", "log", "loglevels.cfg")
     config = ConfigParser()
-    config.read(log_config_file)
-    for logger_name in config["server"]:
-        logging.getLogger(logger_name).setLevel(config["server"][logger_name])
+    if os.path.isfile(log_config_file):
+        config.read(log_config_file)
+        for logger_name in config["server"]:
+            logging.getLogger(logger_name).setLevel(config["server"][logger_name])
 
 
 def getLogger(name: str) -> logging.Logger:

lib/python/CHANGELOG.md

@@ -1,5 +1,8 @@
 VMware Aria Operations Integration SDK Library
 ----------------------------------------------
+## 0.8.1 (09-15-2023)
+* Improve error handling when log directory is not writable
+
 ## 0.8.0 (07-26-2023)
 * Add py.typed files for typing support in compliance with PEP-561
 

lib/python/src/aria/ops/adapter_logging.py

@@ -21,22 +21,30 @@ def setup_logging(filename: str, file_count: int = 5, max_size: int = 0) -> None
                                   do no automatic rotation. Requires calling the 'rotate()' function
                                   manually to ensure logs do not become too large.
     """
-    try:
-        global log_handler
-        log_handler = RotatingFileHandler(
-            os.path.join(os.sep, "var", "log", filename),
-            maxBytes=max_size,
-            backupCount=file_count,
-        )
-        logging.basicConfig(
-            format="%(asctime)s,%(msecs)d %(name)s %(levelname)s %(message)s",
-            datefmt="%Y-%m-%d %H:%M:%S",
-            level=_get_default_log_level(),
-            handlers=[log_handler],
+    logdir = os.path.join(os.sep, "var", "log")
+    if os.access(logdir, os.W_OK):
+        try:
+            global log_handler
+            log_handler = RotatingFileHandler(
+                os.path.join(os.sep, "var", "log", filename),
+                maxBytes=max_size,
+                backupCount=file_count,
+            )
+            logging.basicConfig(
+                format="%(asctime)s,%(msecs)d %(name)s %(levelname)s %(message)s",
+                datefmt="%Y-%m-%d %H:%M:%S",
+                level=_get_default_log_level(),
+                handlers=[log_handler],
+            )
+            _set_log_levels()
+        except Exception as e:
+            logging.basicConfig(level=logging.INFO)
+            logging.exception(e)
+    else:
+        logging.basicConfig(level=logging.INFO)
+        logging.exception(
+            f"Cannot write to log file '{os.path.join(logdir, filename)}'"
         )
-        _set_log_levels()
-    except Exception:
-        logging.basicConfig(level=logging.CRITICAL + 1)
 
 
 def _get_default_log_level(default_level: int = logging.INFO) -> int:
@@ -65,8 +73,11 @@ def _get_default_log_level(default_level: int = logging.INFO) -> int:
             "__main__": default_level_name,
         }
     if modified:
-        with open(log_config_file, "w") as config_file:
-            config.write(config_file)
+        try:
+            with open(log_config_file, "w") as config_file:
+                config.write(config_file)
+        except Exception as e:
+            logging.exception(e)
     try:
         return int(
             logging.getLevelName(config["DEFAULT"].get("adapter", default_level_name))
@@ -81,9 +92,10 @@ def _set_log_levels() -> None:
     """
     log_config_file = os.path.join(os.sep, "var", "log", "loglevels.cfg")
     config = ConfigParser()
-    config.read(log_config_file)
-    for logger in config["adapter"]:
-        logging.getLogger(logger).setLevel(config["adapter"][logger])
+    if os.path.isfile(log_config_file):
+        config.read(log_config_file)
+        for logger in config["adapter"]:
+            logging.getLogger(logger).setLevel(config["adapter"][logger])
 
 
 def getLogger(name: str) -> logging.Logger:

vmware_aria_operations_integration_sdk/container_versions.json

@@ -2,7 +2,7 @@
     "base_image": {
         "language": "Python",
         "path": "base-python-adapter",
-        "version": "0.12.0"
+        "version": "0.12.1"
     },
     "registry_url": "projects.registry.vmware.com/vmware_aria_operations_integration_sdk",
     "secondary_images": [

vmware_aria_operations_integration_sdk/docker_wrapper.py

@@ -285,6 +285,9 @@ def run_image(
     if exposed_port:
         port = exposed_port
 
+    if not os.access(f"{path}/logs", os.W_OK):
+        logger.warn(f"{path}/logs directory is not writable.")
+
     # Docker memory parameters expect a unit ('m' is 'MB'), or the number will be interpreted as bytes
     # vROps sets the swap memory limit to the memory limit + 512MB, so we will also. The swap memory
     # setting is a combination of memory and swap, so this will limit swap space to a max of 512MB regardless

vmware_aria_operations_integration_sdk/mp_init.py

@@ -177,6 +177,12 @@ def create_project(
 
     project = Project(path)
 
+    if hasattr(os, "geteuid") and os.geteuid() == 0:
+        # Log directory must be writable by non-root users so that the adapter container
+        # is able to create and write log files.
+        log_dir = mkdir(path, "logs")
+        os.chmod(log_dir, 0o755)
+
     build_content_directory(path)
     conf_dir = mkdir(path, "conf")
     conf_resources_dir = mkdir(conf_dir, "resources")
@@ -250,6 +256,20 @@ def main() -> None:
     )
     parser.parse_args()
 
+    if hasattr(os, "geteuid") and os.geteuid() == 0:
+        if (
+            selection_prompt(
+                "Detected that 'mp-init' is being run as root or using sudo. This is not "
+                "recommended. Continue?",
+                [("no", "No"), ("yes", "Yes")],
+                "If 'mp-init' proceeds as root:\n"
+                " * Some directories will need write permissions by non-root users.\n"
+                " * Elevated permissions will also be required when running 'mp-test' and 'mp-build'\n"
+                " * There may be other unexpected behavior",
+            )
+            == "no"
+        ):
+            exit(0)
     path = ""
     try:
         path = path_prompt(