Fixed issue with 'draky' parameter not being used if put in the exter…

…nal service, that the service in the recipe extends from.

Reworked slightly logic related to the compose building, making the code a bit cleaner.
Added test making sure that the "draky" property existing in the recipe is removed in the built compose file.

core/dk/compose_manager.py

@@ -3,62 +3,27 @@
 import os
 import sys
 import typing
+import copy
 
 import yaml
 from colorama import Fore, Style
 
 from dk.config_manager import ConfigManager
 
-class ComposeRecipe:
-    """Class representing the compose's recipe.
-    """
-
-    def __init__(self, content: dict):
-        self.__validate_recipe(content)
-        self.__content: dict = content
-
-    def get_addons(self, service: str) -> list[str]:
-        """Returns a list of addons for the given service.
-        """
-        services = self.get_services()
-        if service not in services:
-            raise ValueError(f"Unknown service '{service}'")
-
-        if 'draky' not in services[service]:
-            return []
-
-        return services[service]['draky']['addons']\
-            if 'addons' in services[service]['draky'] else []
-
-    def get_services(self) -> dict:
-        """Returns the services defined by the recipe.
-        """
-        return self.__content['services']
-
-    def __validate_recipe(self, content: dict):
-        """Validates the recipe's content.
-        """
-        if 'services' not in content:
-            print(
-                f"{Fore.RED}The 'services' section is required in the recipe file.{Style.RESET_ALL}"
-            )
-            sys.exit(1)
-
 
 class Compose:
     """Class representing the compose file.
     """
     def __init__(
-            self, path: str,
+            self,
+            path: str,
             content: dict,
             variables_resolver: callable,
-            recipe: ComposeRecipe,
     ):
         self.__path: str = path
         self.__content: dict = content
         self.__variables_resolver: callable = variables_resolver
         self.__substituted_variables: bool = False
-        self.__recipe: ComposeRecipe = recipe
 
     def set_substituted_variables(self, value: bool) -> None:
         """Set to true for variables in the compose file to be replaced with their values.
@@ -76,13 +41,12 @@ def get_path(self) -> str:
         """
         return self.__path
 
-    def to_string(self) -> str:
-        """Returns the content of the compose file as a string.
+    def get_service(self, name: str) -> dict:
+        """Returns the dictionary representing the specified service.
         """
-        compose_string = yaml.safe_dump(self.__content, default_flow_style=False)
-        if self.is_substituted_variables():
-            compose_string = self.__variables_resolver(compose_string)
-        return compose_string
+        if 'services' not in self.__content or name not in self.__content['services']:
+            raise ValueError(f"Service {name} doesn't exist.")
+        return self.__content['services'][name]
 
     def list_services(self) -> list:
         """Returns the list of service names.
@@ -91,55 +55,48 @@ def list_services(self) -> list:
             return []
         return list(self.__content['services'].keys())
 
-    def get_service(self, name: str) -> dict:
-        """Returns the dictionary representing the specified service.
-        """
-        if 'services' not in self.__content or name not in self.__content['services']:
-            raise ValueError(f"Service {name} doesn't exist.")
-        return self.__content['services'][name]
-
-    def get_recipe(self) -> ComposeRecipe:
-        """Returns recipe object that was used to create this compose object.
+    def to_string(self) -> str:
+        """Returns the content of the compose file as a string.
         """
-        return self.__recipe
+        compose_string = yaml.safe_dump(self.__content, default_flow_style=False)
+        if self.is_substituted_variables():
+            compose_string = self.__variables_resolver(compose_string)
+        return compose_string
 
 
-class ComposeManager:
-    """This class is responsible for building a compose file based on the given recipe.
+class ComposeRecipe:
+    """Class representing the compose's recipe.
     """
 
-    def __init__(self, config: ConfigManager):
-        self.config = config
+    def __init__(self, content: dict, recipe_path: str, env_path: str):
+        self.__validate_recipe(content)
+        self.__content: dict = content
+        self.recipe_path = recipe_path
+        self.__env_path = env_path
 
-    def create(self, recipe: ComposeRecipe, recipe_path: str, output_path: str) -> Compose: # pylint: disable=too-many-branches
-        """Creates a compose content from the given recipe.
-        :param recipe:
-          Dictionary storing the recipe's data.
-        :param recipe_path:
-           Required for finding the other relatively referenced files.
-        :param output_path:
-          Required for setting the correct relative paths in volumes in the final compose.
-        :return:
-          Returns a dictionary representing the final compose.
+    def get_addons(self, service: str) -> list[str]:
+        """Returns a list of addons for the given service.
         """
+        services = self.get_services()
+        if service not in services:
+            raise ValueError(f"Unknown service '{service}'")
 
-        compose: dict = {
-            'services': {}
-        }
-        services = recipe.get_services()
+        if 'draky' not in services[service]:
+            return []
 
-        def volume_is_absolute(volume: str) -> bool:
-            """Checks if volume is absolute.
-            """
-            # If volume starts with a variable, then also assume it's absolute.
-            return volume.startswith(('/', '${'))
+        return services[service]['draky']['addons']\
+            if 'addons' in services[service]['draky'] else []
 
-        def volume_convert_relative(volume: str, service_path: str) -> str:
-            """Change the relative path to keep it relative to the service it came from.
-            """
-            service_path = os.path.dirname(service_path) \
-                            .replace(self.config.get_env_path() + os.sep, '')
-            return f"{service_path}/{volume}"
+    def get_services(self) -> dict:
+        """Returns the services defined by the recipe.
+        """
+        return self.__content['services']
+
+    def to_compose(self, compose_path: str, resolve_vars_in_string: callable) -> Compose:  # pylint: disable=too-many-branches
+        """Converts recipe into the compose file.
+        """
+        compose_dict = copy.deepcopy(self.__content)
+        services = compose_dict['services']
 
         for service_name in services:
             service_data = services[service_name]
@@ -160,7 +117,7 @@ def volume_convert_relative(volume: str, service_path: str) -> str:
                         f"Error in the '{service_name}' service. The 'file' value is required if "
                         f"the service extends another service."
                     )
-                remote_file_path = os.path.dirname(recipe_path) + os.sep + extends['file']
+                remote_file_path = os.path.dirname(self.recipe_path) + os.sep + extends['file']
                 if not isinstance(remote_file_path, str):
                     raise ValueError(
                         f"Error in the '{service_name}' service. The 'file' value has to be a "
@@ -201,7 +158,8 @@ def volume_convert_relative(volume: str, service_path: str) -> str:
                         f"dictionary."
                     )
 
-                service = remote_file_dict['services'][remote_file_service]
+                del service_data['extends']
+                service = remote_file_dict['services'][remote_file_service] | service_data
 
             if not service:
                 service = service_data
@@ -213,11 +171,64 @@ def volume_convert_relative(volume: str, service_path: str) -> str:
                 for i, _ in enumerate(service['volumes']):
                     volume = service['volumes'][i]
                     service['volumes'][i] =\
-                        volume if volume_is_absolute(volume)\
-                            else volume_convert_relative(volume, remote_file_path)
+                        volume if self.__volume_is_absolute(volume)\
+                            else self.__volume_convert_relative(volume, remote_file_path)
+
+            compose_dict['services'][service_name] = service
+        return Compose(compose_path, self.__clean_compose(compose_dict), resolve_vars_in_string)
+
+    def __clean_compose(self, compose: dict) -> dict:
+        """Removes draky-specific properties from the service's definition.
+        """
+        if 'services' in compose:
+            for service in compose['services']:
+                if 'draky' in compose['services'][service]:
+                    del compose['services'][service]['draky']
+
+        return compose
+
+    def __volume_is_absolute(self, volume: str) -> bool:
+        """Checks if volume is absolute.
+        """
+        # If volume starts with a variable, then also assume it's absolute.
+        return volume.startswith(('/', '${'))
+
+    def __volume_convert_relative(self, volume: str, service_path: str) -> str:
+        """Change the relative path to keep it relative to the service it came from.
+        """
+        service_path = os.path.dirname(service_path).replace(self.__env_path + os.sep, '')
+        return f"{service_path}/{volume}"
+
+    def __validate_recipe(self, content: dict):
+        """Validates the recipe's content.
+        """
+        if 'services' not in content:
+            print(
+                f"{Fore.RED}The 'services' section is required in the recipe file.{Style.RESET_ALL}"
+            )
+            sys.exit(1)
+
+
+class ComposeManager:
+    """This class is responsible for building a compose file based on the given recipe.
+    """
+
+    def __init__(self, config: ConfigManager):
+        self.config = config
+
+    def create(self, recipe: ComposeRecipe, compose_path: str) -> Compose:
+        """Creates a compose content from the given recipe.
+        :param recipe:
+          Dictionary storing the recipe's data.
+        :param recipe_path:
+           Required for finding the other relatively referenced files.
+        :param compose_path:
+          Required for setting the correct relative paths in volumes in the final compose.
+        :return:
+          Returns a dictionary representing the final compose.
+        """
 
-            compose['services'][service_name] = service
-        return Compose(output_path, compose, self.config.resolve_vars_in_string, recipe)
+        return recipe.to_compose(compose_path, self.config.resolve_vars_in_string)
 
     def save(self, compose: Compose):
         """Save the compose file to disk.

core/dk/hook_manager.py

@@ -3,7 +3,7 @@
 import os
 from importlib import util
 
-from dk.compose_manager import Compose
+from dk.compose_manager import Compose, ComposeRecipe
 from dk.config_manager import ConfigManager
 
 
@@ -27,12 +27,11 @@ def __init__(self, config: ConfigManager):
         self.__config: ConfigManager = config
         self.__utils = HookUtils(config)
 
-    def addon_alter_services(self, compose: Compose):
+    def addon_alter_services(self, recipe: ComposeRecipe, compose: Compose) -> None:
         """Allows addons to alter services.
         """
         services = compose.list_services()
         addons = self.__config.get_addons()
-        recipe = compose.get_recipe()
 
         for addon in addons:
 
@@ -42,10 +41,10 @@ def addon_alter_services(self, compose: Compose):
                     continue
 
                 addon_path = addon.path
-                addon_path_absolute =(
-                        self.__config.get_project_config_path() +
-                        os.sep +
-                        os.path.dirname(addon_path)
+                addon_path_absolute = (
+                    self.__config.get_project_config_path() +
+                    os.sep +
+                    os.path.dirname(addon_path)
                 )
 
                 hooks_path = addon_path_absolute + os.sep + 'hooks.py'

core/dk/process_executor.py

@@ -48,10 +48,10 @@ def env_build(self, substitute_vars: bool = False):
         if os.path.exists(recipe_path):
             with open(recipe_path, "r", encoding='utf8') as f:
                 recipe_content = yaml.safe_load(f)
-            recipe = ComposeRecipe(recipe_content)
-            compose = self.compose_manager.create(recipe, recipe_path, self.__get_compose_path())
+            recipe = ComposeRecipe(recipe_content, recipe_path, self.config.get_env_path())
+            compose = self.compose_manager.create(recipe, self.__get_compose_path())
             compose.set_substituted_variables(substitute_vars)
-            self.hook_manager.addon_alter_services(compose)
+            self.hook_manager.addon_alter_services(recipe, compose)
             self.compose_manager.save(compose)
 
     def env_start(self) -> None:

tests/functional/tests.bats

@@ -221,8 +221,7 @@ EOF
   cat > "$RECIPE_PATH" << EOF
 services:
   php:
-    service:
-      image: "\${PHP_IMAGE_TEST}"
+    image: "\${PHP_IMAGE_TEST}"
 EOF
   # Give variable a value.
   cat > "${TEST_ENV_PATH}/.draky/variables.dk.yml" << EOF
@@ -233,7 +232,22 @@ EOF
   grep -q "image: php-image" "$COMPOSE_PATH"
 }
 
-@test "Addons can alter services" {
+@test "Build compose: 'draky' property is removed from services" {
+  _initialize_test_environment
+  # Create the recipe.
+  cat > "$RECIPE_PATH" << EOF
+services:
+  php:
+    image: test-image
+    draky:
+      addons: []
+
+EOF
+  ${DRAKY} env build
+  run ! grep -q "draky:" "$COMPOSE_PATH"
+}
+
+@test "Addons: Addons can alter services" {
   _initialize_test_environment
 
   # Create a test addon.
@@ -265,7 +279,7 @@ EOF
   grep -q "$ENTRYPOINT_SCRIPT" "$COMPOSE_PATH"
 }
 
-@test "Custom command is added to the help" {
+@test "Custom commands: Custom command is added to the help" {
   _initialize_test_environment
   TEST_COMMAND_PATH="${TEST_ENV_PATH}/.draky/testcommand.dk.sh"
     cat > "${TEST_COMMAND_PATH}" << EOF
@@ -275,7 +289,7 @@ EOF
   ${DRAKY} -h | grep -q testcommand
 }
 
-@test "User can run custom scripts" {
+@test "Custom commands: User can run custom scripts" {
   _initialize_test_environment
 
   TEST_SERVICE=test_service
@@ -320,7 +334,7 @@ EOF
   [[ "$output" == *"${TEST_SERVICE_COMMAND_MESSAGE}"* ]]
 }
 
-@test "Custom scripts are running in tty" {
+@test "Custom commands: Custom scripts are running in tty" {
   _initialize_test_environment
 
   TEST_SERVICE=test_service
@@ -365,7 +379,7 @@ EOF
   [[ "$output" == *"${TEST_SERVICE_COMMAND_MESSAGE}"* ]]
 }
 
-@test "Custom scripts can receive data from stdin" {
+@test "Custom commands: Custom scripts can receive data from stdin" {
   _initialize_test_environment
 
   TEST_SERVICE=test_service