diff --git a/tools/deployment-cli-tools/ch_cli_tools/codefresh.py b/tools/deployment-cli-tools/ch_cli_tools/codefresh.py index 006d9262..3939da78 100644 --- a/tools/deployment-cli-tools/ch_cli_tools/codefresh.py +++ b/tools/deployment-cli-tools/ch_cli_tools/codefresh.py @@ -40,6 +40,24 @@ def clean_step_key(s: str) -> str: return re.sub(r'[^a-zA-Z0-9_]', '_', s) +def dockerfile_selector_candidates(base_path: str, dockerfile_path: str) -> set[str]: + """Return stable identifiers that may be used with include/exclude selectors.""" + relative_to_base = get_app_relative_to_base_path(base_path, dockerfile_path) + parent_name = relative_to_base.split("/")[0] if relative_to_base else "" + return { + candidate for candidate in ( + relative_to_base, + app_name_from_path(relative_to_base), + parent_name, + ) if candidate + } + + +def path_contains_excluded_segment(path: str, excluded_segments) -> bool: + normalized_path = f"/{path.replace(os.path.sep, '/')}/" + return any(f"/{segment}/" in normalized_path for segment in excluded_segments) + + def get_main_domain(url): try: url = url.split("//")[1].split("/")[0] @@ -238,6 +256,7 @@ def codefresh_steps_from_base_path(base_path, fixed_context=None, include=build_ for dockerfile_path in find_dockerfiles_paths(base_path): dockerfile_relative_to_root = relpath(dockerfile_path, '.') dockerfile_relative_to_base = get_app_relative_to_base_path(base_path, dockerfile_path) + selector_candidates = dockerfile_selector_candidates(base_path, dockerfile_path) app_name = app_name_from_path(dockerfile_relative_to_base) app_key = app_name app_config: ApplicationHarnessConfig = app_key in helm_values.apps and helm_values.apps[app_key].harness @@ -245,13 +264,15 @@ def codefresh_steps_from_base_path(base_path, fixed_context=None, include=build_ else helm_values[KEY_TASK_IMAGES][app_key] if app_key in helm_values[KEY_TASK_IMAGES]\ else f"{base_name}/{app_name}" - if include and not any( - f"/{inc}/" in os.path.relpath(dockerfile_path, root_path) or dockerfile_path.endswith(f"/{inc}") for inc in include - ): + if include and not any(inc in selector_candidates for inc in include): # Skip not included apps continue - if any(inc in dockerfile_path for inc in (list(exclude) + EXCLUDE_PATHS)): + if any(ex in selector_candidates for ex in exclude): + # Skip explicitly excluded apps/images + continue + + if path_contains_excluded_segment(dockerfile_relative_to_root, EXCLUDE_PATHS): # Skip excluded apps continue diff --git a/tools/deployment-cli-tools/ch_cli_tools/configurationgenerator.py b/tools/deployment-cli-tools/ch_cli_tools/configurationgenerator.py index ba3dbbe2..893e2bcd 100644 --- a/tools/deployment-cli-tools/ch_cli_tools/configurationgenerator.py +++ b/tools/deployment-cli-tools/ch_cli_tools/configurationgenerator.py @@ -204,12 +204,15 @@ def _init_static_images(self, base_image_name): ) def _assign_static_build_dependencies(self, helm_values): + static_consumers = {} for static_img_dockerfile in self.static_images: key = os.path.basename(static_img_dockerfile) if key in helm_values[KEY_TASK_IMAGES]: dependencies = guess_build_dependencies_from_dockerfile( f"{static_img_dockerfile}") for dep in dependencies: + if dep in self.exclude and dep in helm_values[KEY_TASK_IMAGES] and key not in self.exclude: + static_consumers.setdefault(dep, set()).add(key) if dep in self.base_images and dep not in helm_values[KEY_TASK_IMAGES]: helm_values[KEY_TASK_IMAGES][dep] = self.base_images[dep] # helm_values.setdefault(KEY_TASK_IMAGES_BUILD, {})[dep] = { @@ -217,10 +220,48 @@ def _assign_static_build_dependencies(self, helm_values): # 'dockerfile': 'Dockerfile', # } - for image_name in list(helm_values[KEY_TASK_IMAGES].keys()): - if image_name in self.exclude: - del helm_values[KEY_TASK_IMAGES][image_name] - # del helm_values[KEY_TASK_IMAGES_BUILD][image_name] + self._prune_excluded_task_images(helm_values, extra_consumers=static_consumers) + + def _get_excluded_task_image_consumers(self, values): + """Return reverse dependencies for excluded task images from app build deps.""" + task_images = values.get(KEY_TASK_IMAGES, {}) + apps = values.get(KEY_APPS, {}) + consumers = {} + + for app_name, app_values in apps.items(): + if app_name in self.exclude: + continue + build_dependencies = app_values.get(KEY_HARNESS, {}).get('dependencies', {}).get('build', []) + if not build_dependencies: + continue + for dep in build_dependencies: + if dep in self.exclude and dep in task_images: + consumers.setdefault(dep, set()).add(app_name) + + return consumers + + def _prune_excluded_task_images(self, values, extra_consumers=None): + """Exclude task images only when no non-excluded image still depends on them.""" + task_images = values.get(KEY_TASK_IMAGES, {}) + if not task_images: + return + + consumers = self._get_excluded_task_image_consumers(values) + if extra_consumers: + for image_name, image_consumers in extra_consumers.items(): + consumers.setdefault(image_name, set()).update(image_consumers) + + for image_name in list(task_images.keys()): + if image_name not in self.exclude: + continue + used_by = sorted(consumers.get(image_name, set())) + if used_by: + logging.warning( + "Image %s was excluded but is still required by non-excluded builds: %s. Keeping it.", + image_name, ", ".join(used_by) + ) + continue + del task_images[image_name] def _init_base_images(self, base_image_name): """Initialize base images (infrastructure/base-images/) with root context.""" @@ -600,6 +641,7 @@ def validate_dependencies(values): all_apps = {a for a in values["apps"]} for app in all_apps: app_values = values["apps"][app] + app_task_images = set(app_values.get(KEY_TASK_IMAGES, {})) if 'dependencies' in app_values[KEY_HARNESS]: soft_dependencies = { d for d in app_values[KEY_HARNESS]['dependencies']['soft']} @@ -617,9 +659,9 @@ def validate_dependencies(values): build_dependencies = { d for d in app_values[KEY_HARNESS]['dependencies']['build']} + available_builds = set(values.get(KEY_TASK_IMAGES, {})) | all_apps | app_task_images not_found = { - d for d in build_dependencies if d not in values[KEY_TASK_IMAGES]} - not_found = {d for d in not_found if d not in all_apps} + d for d in build_dependencies if d not in available_builds} if not_found: raise ValuesValidationException( f"Bad build dependencies specified for application {app}: {','.join(not_found)} not found as built image") @@ -629,8 +671,10 @@ def validate_dependencies(values): not_found = {d for d in service_dependencies if d not in all_apps} if not_found: - raise ValuesValidationException( - f"Bad service application dependencies specified for application {app}: {','.join(not_found)}") + logging.warning( + f"Service proxy (use_services) dependencies for application {app} are not deployed: " + f"{','.join(not_found)}. Proxies to these services will not be created." + ) def collect_apps_helm_templates(search_root, dest_helm_chart_path, templates_path=HELM_PATH, exclude=(), include=None, envs=()): diff --git a/tools/deployment-cli-tools/ch_cli_tools/helm.py b/tools/deployment-cli-tools/ch_cli_tools/helm.py index 060f4214..c61125af 100644 --- a/tools/deployment-cli-tools/ch_cli_tools/helm.py +++ b/tools/deployment-cli-tools/ch_cli_tools/helm.py @@ -170,6 +170,8 @@ def _aggregate_task_images(self, values): if image: values[KEY_TASK_IMAGES][dep_name] = image app_name = dep_name + elif dep_name in self.base_images: + values[KEY_TASK_IMAGES][dep_name] = self.base_images[dep_name] elif prefix in apps: app_name = prefix values[KEY_TASK_IMAGES][dep_name] = apps[app_name][KEY_TASK_IMAGES][dep_name] @@ -178,10 +180,7 @@ def _aggregate_task_images(self, values): if key in included_builds or app_name in self.include: values[KEY_TASK_IMAGES][key] = apps[app_name][KEY_TASK_IMAGES][key] - for ex in self.exclude: - if ex in values[KEY_TASK_IMAGES]: - logging.info(f"Excluding {ex} from build") - del values[KEY_TASK_IMAGES][ex] + self._prune_excluded_task_images(values) def __finish_helm_values(self, values, defer_task_images=False): """ @@ -279,6 +278,8 @@ def __finish_helm_values(self, values, defer_task_images=False): if image: values[KEY_TASK_IMAGES][dep_name] = image app_name = dep_name + elif dep_name in self.base_images: + values[KEY_TASK_IMAGES][dep_name] = self.base_images[dep_name] elif prefix in apps: # build dependency within an application that is not part of the deployment app_name = prefix values[KEY_TASK_IMAGES][dep_name] = apps[app_name][KEY_TASK_IMAGES][dep_name] @@ -294,10 +295,7 @@ def __finish_helm_values(self, values, defer_task_images=False): values[KEY_TASK_IMAGES].update(apps[v][KEY_TASK_IMAGES]) if not defer_task_images: - for ex in self.exclude: - if ex in values[KEY_TASK_IMAGES]: - logging.info(f"Excluding {ex} from build") - del values[KEY_TASK_IMAGES][ex] + self._prune_excluded_task_images(values) create_env_variables(values) return values, self.include diff --git a/tools/deployment-cli-tools/tests/test_codefresh.py b/tools/deployment-cli-tools/tests/test_codefresh.py index 44506a3e..6ed9bc9a 100644 --- a/tools/deployment-cli-tools/tests/test_codefresh.py +++ b/tools/deployment-cli-tools/tests/test_codefresh.py @@ -331,6 +331,50 @@ def test_create_codefresh_configuration_nobuild(): assert "publish_myapp-mytask" in l1_steps["publish"]["steps"] +def test_excluding_common_app_does_not_skip_common_images_dependencies(): + values = create_helm_chart( + [CLOUDHARNESS_ROOT, RESOURCES], + output_path=OUT, + include=['myapp'], + exclude=['common'], + domain='my.local', + namespace='test', + env='dev', + local=False, + tag=1, + registry='reg' + ) + try: + values[KEY_TASK_IMAGES]['my-common'] = 'reg/testprojectname/my-common:1' + + root_paths = preprocess_build_overrides( + root_paths=[CLOUDHARNESS_ROOT, RESOURCES], + helm_values=values, + merge_build_path=BUILD_MERGE_DIR + ) + + build_included = [app['harness']['name'] + for app in values['apps'].values() if 'harness' in app] + + cf = create_codefresh_deployment_scripts(root_paths, include=build_included, + exclude=['common'], + envs=['dev'], + base_image_name=values['name'], + helm_values=values, save=False) + + all_build_steps = { + step_name: step + for build_step_name in [STEP_0, STEP_1, STEP_2, STEP_3] + if build_step_name in cf['steps'] + for step_name, step in cf['steps'][build_step_name]['steps'].items() + } + + assert 'my-common' in all_build_steps, \ + 'Excluding app "common" must not skip the my-common image under infrastructure/common-images' + finally: + shutil.rmtree(BUILD_MERGE_DIR, ignore_errors=True) + + def test_codefresh_db_connect_string_secret(): """When an app has database.connect_string set to '', a custom_values entry must be added to the deployment step.""" values = create_helm_chart( diff --git a/tools/deployment-cli-tools/tests/test_helm.py b/tools/deployment-cli-tools/tests/test_helm.py index b424db24..5061ea34 100644 --- a/tools/deployment-cli-tools/tests/test_helm.py +++ b/tools/deployment-cli-tools/tests/test_helm.py @@ -197,9 +197,35 @@ def test_collect_helm_values_wrong_dependencies_validate(tmp_path): with pytest.raises(ValuesValidationException): create_helm_chart([CLOUDHARNESS_ROOT, f"{RESOURCES}/wrong-dependencies"], output_path=out_folder, domain="my.local", namespace='test', env='prod', local=False, tag=1, include=["wrong-build"]) - with pytest.raises(ValuesValidationException): + try: create_helm_chart([CLOUDHARNESS_ROOT, f"{RESOURCES}/wrong-dependencies"], output_path=out_folder, domain="my.local", namespace='test', env='prod', local=False, tag=1, include=["wrong-services"]) + except ValuesValidationException: + pytest.fail("Should not error because of missing use_services dependency") + + +def test_validate_dependencies_accepts_app_local_build_images(): + values = { + KEY_APPS: { + 'portal': { + KEY_HARNESS: { + 'dependencies': { + 'soft': [], + 'hard': [], + 'build': ['cloudharness-base', 'cloudharness-django'], + }, + 'use_services': [], + }, + KEY_TASK_IMAGES: { + 'cloudharness-base': 'reg/project/cloudharness-base:1', + 'cloudharness-django': 'reg/project/cloudharness-django:1', + }, + } + }, + KEY_TASK_IMAGES: {}, + } + + validate_dependencies(values) def test_collect_helm_values_build_dependencies(tmp_path): @@ -441,13 +467,12 @@ def test_exclude_single_task(tmp_path): assert "myapp-mytask" not in values["task-images"], "myapp-mytask has been excluded, so should not appear in the task images" - try: - values = create_helm_chart([CLOUDHARNESS_ROOT, RESOURCES], output_path=out_folder, domain="my.local", - env='fulldep', local=False, include=["dependantapp"], exclude=["myapp-mytask"]) + values = create_helm_chart([CLOUDHARNESS_ROOT, RESOURCES], output_path=out_folder, domain="my.local", + env='fulldep', local=False, include=["dependantapp"], exclude=["myapp-mytask"]) - assert False, "myapp-mytask has been excluded, but also declared as a dependency, so should not be excluded" - except ValuesValidationException as e: - pass + assert "myapp-mytask" in values[KEY_TASK_IMAGES], ( + "myapp-mytask is excluded but still required by dependantapp, so it should be kept" + ) def test_app_depends_on_app(tmp_path):