From c19347e3adc87ba88eb08aa585e40256481758a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Zaroda?= Date: Sun, 21 Jan 2024 23:05:02 +0100 Subject: [PATCH] Fixed issue with addons not really working when enabled not directly on the recipe, but on the extended services in different files. --- core/dk/compose_manager.py | 79 +++++++++++++++++++++---------------- tests/functional/tests.bats | 40 +++++++++++++++++++ 2 files changed, 86 insertions(+), 33 deletions(-) diff --git a/core/dk/compose_manager.py b/core/dk/compose_manager.py index 16fa43c..d67d740 100644 --- a/core/dk/compose_manager.py +++ b/core/dk/compose_manager.py @@ -77,7 +77,8 @@ def __init__(self, content: dict, recipe_path: str, env_path: str): def get_addons(self, service: str) -> list[str]: """Returns a list of addons for the given service. """ - services = self.get_services() + compose = self.__to_compose_dict(cleaned=False) + services = compose['services'] if service not in services: raise ValueError(f"Unknown service '{service}'") @@ -85,16 +86,36 @@ def get_addons(self, service: str) -> list[str]: return [] return services[service]['draky']['addons']\ - if 'addons' in services[service]['draky'] else [] + if 'addons' in services[service]['draky'] else [] 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 + def to_compose( + self, + compose_path: str, + resolve_vars_in_string: callable, + cleaned: bool = True, + ) -> Compose: """Converts recipe into the compose file. """ + compose_dict = self.__to_compose_dict(cleaned) + + return Compose(compose_path, 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 __to_compose_dict(self, cleaned: bool = True): compose_dict = copy.deepcopy(self.__content) services = compose_dict['services'] @@ -107,16 +128,8 @@ def to_compose(self, compose_path: str, resolve_vars_in_string: callable) -> Com # Validate the basic structure. if 'extends' in service_data: extends = service_data['extends'] - if not isinstance(extends, dict): - raise ValueError( - f"Error in the '{service_name}' service. 'extends' key has to be a " - f"dictionary." - ) - if 'file' not in extends: - raise ValueError( - f"Error in the '{service_name}' service. The 'file' value is required if " - f"the service extends another service." - ) + self.__validate_extends(service_name, extends) + remote_file_path = os.path.dirname(self.recipe_path) + os.sep + extends['file'] if not isinstance(remote_file_path, str): raise ValueError( @@ -139,17 +152,7 @@ def to_compose(self, compose_path: str, resolve_vars_in_string: callable) -> Com with open(remote_file_path, "r", encoding='utf8') as f: remote_file_dict = yaml.safe_load(f) - if 'services' not in remote_file_dict: - raise ValueError( - f"Error in the '{service_name}' service. The file '{remote_file_path}' " - f"doesn't have a 'services' key." - ) - - if remote_file_service not in remote_file_dict['services']: - raise ValueError( - f"Error in the '{service_name}' service. The file '{remote_file_path}' " - f"doesn't have a '{remote_file_service}' service." - ) + self.__validate_service_in_extended_compose(remote_file_service, remote_file_dict) if not isinstance(remote_file_dict['services'][remote_file_service], dict): raise ValueError( @@ -175,17 +178,11 @@ def to_compose(self, compose_path: str, resolve_vars_in_string: callable) -> Com 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'] + if cleaned: + return self.__clean_compose(compose_dict) - return compose + return compose_dict def __volume_is_absolute(self, volume: str) -> bool: """Checks if volume is absolute. @@ -208,6 +205,22 @@ def __validate_recipe(self, content: dict): ) sys.exit(1) + def __validate_extends(self, service_name: str, extends: dict) -> None: + if not isinstance(extends, dict): + raise ValueError( + f"Error in the '{service_name}' service. 'extends' key has to be a " + f"dictionary." + ) + if 'file' not in extends: + raise ValueError( + f"Error in the '{service_name}' service. The 'file' value is required if " + f"the service extends another service." + ) + + def __validate_service_in_extended_compose(self, service_name: str, compose: dict) -> None: + if 'services' not in compose or service_name not in compose['services']: + raise ValueError(f"Error in the '{service_name}' service. It's missing in the " + f"extended compose file.") class ComposeManager: """This class is responsible for building a compose file based on the given recipe. diff --git a/tests/functional/tests.bats b/tests/functional/tests.bats index aff2857..eb94d55 100644 --- a/tests/functional/tests.bats +++ b/tests/functional/tests.bats @@ -279,6 +279,46 @@ EOF grep -q "$ENTRYPOINT_SCRIPT" "$COMPOSE_PATH" } +@test "Addons: Addons can alter services even if they are enabled on services being extended" { + _initialize_test_environment + + # Create a test addon. + ADDON_NAME=test-addon + ADDON_PATH="${TEST_ENV_PATH}/.draky/addons/${ADDON_NAME}" + ENTRYPOINT_SCRIPT=/test-entrypoint.sh + mkdir -p "$ADDON_PATH" + # Create the addon config file. + cat > "${ADDON_PATH}/${ADDON_NAME}.addon.dk.yml" << EOF +id: ${ADDON_NAME} +EOF + cat > "${ADDON_PATH}/hooks.py" << EOF +def alter_service(name: str, service: dict, utils: object, addon: dict): + service['entrypoint'] = ['$ENTRYPOINT_SCRIPT'] +EOF + + # Create the recipe. + cat > "$RECIPE_PATH" << EOF +services: + php: + extends: + file: ../../services/php/services.yml + service: php +EOF + PHP_SERVICE_PATH="${TEST_ENV_PATH}/.draky/services/php" + mkdir -p ${PHP_SERVICE_PATH} + # Create an external service file. + cat > "${PHP_SERVICE_PATH}/services.yml" << EOF +services: + php: + image: php-image + draky: + addons: + - ${ADDON_NAME} +EOF + ${DRAKY} env build + grep -q "${ENTRYPOINT_SCRIPT}" "$COMPOSE_PATH" +} + @test "Custom commands: Custom command is added to the help" { _initialize_test_environment TEST_COMMAND_PATH="${TEST_ENV_PATH}/.draky/testcommand.dk.sh"