Fixed proxy overwriting manager config (#11)

* Fixed proxy overwriting manager config

* Update PR template with new issue repo

Author: OskarLiew

.github/pull_request_template.md

@@ -4,7 +4,7 @@ Include a summary of this pull request
 
 New dependencies: (Added to requirements_*.txt)
 
-Fixes # (potential issues that are fixed by this PR)
+Fixes vikinganalytics/daeploy-issues# (potential issues that are fixed by this PR)
 
 ## Type of Change
 

manager/constants.py

@@ -30,7 +30,7 @@
 DAEPLOY_PICKLE_FILE_NAME = "model.pkl"
 
 DAEPLOY_PROXY_DASHBOARD_IDENTIFIER = "proxy_dashboard_configuration"
-DAEPLOY_MANAGER_IDENTIFIER = "manager_configuration"
+DAEPLOY_MANAGER_PROXY_CONFIG_FILE = "manager_configuration"
 DAEPLOY_AUTH_MIDDLEWARE = "proxy_auth_middleware"
 DAEPLOY_AUTH_MIDDLEWARE_IDENTIFIER = f"{DAEPLOY_AUTH_MIDDLEWARE}_configuration"
 

manager/data_models/request_models.py

@@ -16,18 +16,20 @@ class BaseService(BaseModel):
     @validator("name")
     def must_adhere_to_docker_requirements(cls, name):
         # Only allow a name to contain lower case letters, numbers and underscore
-        regex_pattern = re.compile("[a-z0-9_]+$")
+        # anywhere but in the beginning and end
+        regex_pattern = re.compile("^(?!_)[a-z0-9_]+(?<!_)$")
         if not regex_pattern.match(name):
             raise ValueError(
-                "can only contain lower case letters, numbers and underscores"
+                "Name can only contain lower case letters, numbers and underscores,"
+                " but should not start or end with an underscore."
             )
         return name
 
     # pylint: disable=no-self-use
     @validator("version")
     def must_be_semver_string(cls, version):
         if not semver.VersionInfo.isvalid(version):
-            raise ValueError("must be a semantic version string")
+            raise ValueError("Version must be a semantic version string.")
         return version
 
 

manager/proxy.py

@@ -10,7 +10,7 @@
 from manager.notification_api import _manager_notification, register_notification
 from manager.constants import (
     DAEPLOY_PROXY_DASHBOARD_IDENTIFIER,
-    DAEPLOY_MANAGER_IDENTIFIER,
+    DAEPLOY_MANAGER_PROXY_CONFIG_FILE,
     DAEPLOY_AUTH_MIDDLEWARE,
     DAEPLOY_AUTH_MIDDLEWARE_IDENTIFIER,
     DAEPLOY_DATA_DIR,
@@ -343,7 +343,9 @@ def initial_setup():
     )
 
     # Add manager to root
-    add_dynamic_configuration(DAEPLOY_MANAGER_IDENTIFIER, get_manager_configuration())
+    add_dynamic_configuration(
+        DAEPLOY_MANAGER_PROXY_CONFIG_FILE, get_manager_configuration()
+    )
 
     # Add auth middleware
     add_dynamic_configuration(
@@ -405,7 +407,7 @@ def create_new_service_configuration(name: str, version: str, address: str):
     """
     LOGGER.info(f"Creating configuration for service: {name}")
     service_name = f"{name}_{version}"
-    file_name = f"{service_name}_configuration.toml"
+    file_name = f"service_{service_name}_configuration.toml"
     config = get_base_service_config(service_name)
 
     # Create service configurations
@@ -437,7 +439,7 @@ def create_mirror_configuration(
         shadow_versions (List[str], optional): List of versions of the shadow
             services. Defaults to None.
     """
-    file_name = f"{name}_configuration.toml"
+    file_name = f"service_{name}_configuration.toml"
     config = get_base_service_config(name)
 
     shadow_versions = shadow_versions or []
@@ -465,7 +467,7 @@ def remove_service_configuration(name: str, version: str):
         version (str): Service version
     """
     service_name = f"{name}_{version}"
-    file_name = f"{service_name}_configuration.toml"
-    mirror_file_name = f"{name}_configuration.toml"
+    file_name = f"service_{service_name}_configuration.toml"
+    mirror_file_name = f"service_{name}_configuration.toml"
     remove_dynamic_configuration(file_name)
     remove_dynamic_configuration(mirror_file_name)

tests/manager_test/model_test.py

@@ -25,9 +25,17 @@ def test_ServiceRequest_name_validator_invalid_names():
     with pytest.raises(ValueError):
         BaseNewServiceRequest(name="//////", version="0.0.1", port=80)
 
+    with pytest.raises(ValueError):
+        BaseNewServiceRequest(name="service_", version="0.0.1", port=80)
+
+    with pytest.raises(ValueError):
+        BaseNewServiceRequest(name="_service", version="0.0.1", port=80)
+
+    with pytest.raises(ValueError):
+        BaseNewServiceRequest(name="_service_", version="0.0.1", port=80)
+
 
 def test_ServiceRequest_name_validator_valid_names():
     BaseNewServiceRequest(name="model", version="0.0.1", port=80)
     BaseNewServiceRequest(name="my_model", version="0.0.1", port=80)
-    BaseNewServiceRequest(name="my_model123_", version="0.0.1", port=80)
-    BaseNewServiceRequest(name="_model_", version="0.0.1", port=80)
+    BaseNewServiceRequest(name="my_model_123", version="0.0.1", port=80)

tests/manager_test/proxy_test.py

@@ -174,11 +174,11 @@ def test_initial_setup_creates_existing_service_configs(tmp_path):
     time.sleep(1)  # Grace period
     try:
         proxy_files = os.listdir(tmp_path / "dynamic")
-        assert "1_configuration.toml" in proxy_files
-        assert "1_1.0.0_configuration.toml" in proxy_files
-        assert "1_1.1.0_configuration.toml" in proxy_files
-        assert "2_configuration.toml" in proxy_files
-        assert "2_1.0.1_configuration.toml" in proxy_files
+        assert "service_1_configuration.toml" in proxy_files
+        assert "service_1_1.0.0_configuration.toml" in proxy_files
+        assert "service_1_1.1.0_configuration.toml" in proxy_files
+        assert "service_2_configuration.toml" in proxy_files
+        assert "service_2_1.0.1_configuration.toml" in proxy_files
     finally:
         killer()
 
@@ -198,7 +198,7 @@ def test_https_dynamic_config_services(tmp_path, pinned):
     root.recreate_proxy_configurations()
     time.sleep(1)  # Grace period
     try:
-        with open(tmp_path / "dynamic" / "1_configuration.toml", "r") as f:
+        with open(tmp_path / "dynamic" / "service_1_configuration.toml", "r") as f:
             service_config = toml.load(f)
         assert service_config == pinned
     finally: