Skip to content

Commit 515841c

Browse files
filippomcCopilot
andcommitted
CH-260 fix cloud harness relative paths on codefresh
Co-authored-by: Copilot <copilot@github.com>
1 parent 377e691 commit 515841c

2 files changed

Lines changed: 147 additions & 3 deletions

File tree

tools/deployment-cli-tools/ch_cli_tools/codefresh.py

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,18 @@
2121
CLOUD_HARNESS_PATH = "cloud-harness"
2222
ROLLOUT_CMD_TPL = "kubectl rollout status deployment/%s"
2323

24+
25+
def _to_codefresh_path(rel_path: str) -> str:
26+
"""Rewrite a relative path that escapes the current directory to use ./cloud-harness.
27+
28+
In Codefresh pipelines cloud-harness is always cloned into ./cloud-harness.
29+
Any path of the form '../<dirname>/...' must become 'cloud-harness/...'.
30+
"""
31+
parts = rel_path.replace('\\', '/').split('/')
32+
if parts[0] == '..':
33+
return '/'.join([CLOUD_HARNESS_PATH] + parts[2:])
34+
return rel_path
35+
2436
# Codefresh variables may need quotes: adjust yaml dump accordingly
2537

2638

@@ -179,7 +191,7 @@ def codefresh_app_build_spec(app_name, full_image_name, app_context_path, docker
179191
full_image_name, helm_values.registry.name
180192
),
181193
title=title,
182-
working_directory='./' + app_context_path,
194+
working_directory='./' + _to_codefresh_path(app_context_path),
183195
dockerfile=dockerfile_path)
184196

185197
tag = app_specific_tag_variable(app_name)
@@ -446,8 +458,9 @@ def adjust_build_steps(index):
446458
for i in range(len(cmds)):
447459
cmds[i] = cmds[i].replace("$ENV", "-".join(envs))
448460
cmds[i] = cmds[i].replace("$PARAMS", " ".join(params))
449-
cmds[i] = cmds[i].replace("$PATHS", " ".join(os.path.relpath(
450-
root_path, '.') for root_path in root_paths if DEFAULT_MERGE_PATH not in root_path))
461+
cmds[i] = cmds[i].replace("$PATHS", " ".join(
462+
_to_codefresh_path(os.path.relpath(root_path, '.'))
463+
for root_path in root_paths if DEFAULT_MERGE_PATH not in root_path))
451464

452465
steps = codefresh["steps"]
453466
if CD_E2E_TEST_STEP in steps and not steps[CD_E2E_TEST_STEP]["scale"]:

tools/deployment-cli-tools/tests/test_codefresh.py

Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -665,3 +665,134 @@ def test_env_dockerfile_codefresh_fallback():
665665
"samples should not use dev.Dockerfile as it does not have one"
666666
finally:
667667
shutil.rmtree(BUILD_MERGE_DIR, ignore_errors=True)
668+
669+
670+
def test_codefresh_paths_use_cloned_cloud_harness():
671+
"""When cloud-harness root is outside the current directory (e.g. ../cloud-harness),
672+
paths in the generated codefresh YAML should use ./cloud-harness (the cloned location
673+
inside the pipeline working directory), not ../cloud-harness."""
674+
import tempfile
675+
676+
# Create a sibling directory to simulate running from a different project
677+
with tempfile.TemporaryDirectory(dir=os.path.dirname(CLOUDHARNESS_ROOT)) as tmp_project_dir:
678+
old_cwd = os.getcwd()
679+
try:
680+
os.chdir(tmp_project_dir)
681+
682+
# Sanity check: cloud-harness is indeed above the current directory
683+
assert os.path.relpath(CLOUDHARNESS_ROOT, '.').startswith('..'), \
684+
"Test setup issue: cloud-harness should be outside the current directory"
685+
686+
values = create_helm_chart(
687+
[CLOUDHARNESS_ROOT, RESOURCES],
688+
output_path=OUT,
689+
include=['samples'],
690+
domain="my.local",
691+
namespace='test',
692+
env='dev',
693+
local=False,
694+
tag=1,
695+
registry='reg'
696+
)
697+
698+
root_paths = preprocess_build_overrides(
699+
root_paths=[CLOUDHARNESS_ROOT, RESOURCES],
700+
helm_values=values,
701+
merge_build_path=BUILD_MERGE_DIR
702+
)
703+
704+
build_included = [app['harness']['name']
705+
for app in values['apps'].values() if 'harness' in app]
706+
707+
cf = create_codefresh_deployment_scripts(root_paths, include=build_included,
708+
envs=['dev'],
709+
base_image_name=values['name'],
710+
helm_values=values, save=False)
711+
712+
# harness-deployment command must use ./cloud-harness, not ../cloud-harness
713+
cmds = cf['steps']['prepare_deployment']['commands']
714+
harness_cmd = next(cmd for cmd in cmds if 'harness-deployment' in cmd)
715+
assert '../cloud-harness' not in harness_cmd, (
716+
f"harness-deployment command should not reference ../cloud-harness; got: {harness_cmd}"
717+
)
718+
assert ' cloud-harness' in harness_cmd or harness_cmd.startswith('harness-deployment cloud-harness'), (
719+
f"harness-deployment command should reference cloud-harness (the cloned location); got: {harness_cmd}"
720+
)
721+
722+
# working_directory in all build steps must not escape above the current directory
723+
all_build_steps = {}
724+
for step_name in [STEP_0, STEP_1, STEP_2, STEP_3]:
725+
if step_name in cf['steps']:
726+
all_build_steps.update(cf['steps'][step_name]['steps'])
727+
728+
for step_name, step in all_build_steps.items():
729+
wd = step.get('working_directory', '')
730+
assert not wd.startswith('../'), (
731+
f"Build step '{step_name}' working_directory must not start with '../'; got: {wd}"
732+
)
733+
assert not wd.startswith('./../'), (
734+
f"Build step '{step_name}' working_directory must not start with './../'; got: {wd}"
735+
)
736+
finally:
737+
os.chdir(old_cwd)
738+
shutil.rmtree(BUILD_MERGE_DIR, ignore_errors=True)
739+
740+
741+
def test_codefresh_working_directory_uses_cloned_cloud_harness():
742+
"""The working_directory for build steps that source images from cloud-harness must
743+
use ./cloud-harness/... when cloud-harness is a sibling directory of the project."""
744+
import tempfile
745+
746+
with tempfile.TemporaryDirectory(dir=os.path.dirname(CLOUDHARNESS_ROOT)) as tmp_project_dir:
747+
old_cwd = os.getcwd()
748+
try:
749+
os.chdir(tmp_project_dir)
750+
751+
values = create_helm_chart(
752+
[CLOUDHARNESS_ROOT, RESOURCES],
753+
output_path=OUT,
754+
include=['samples'],
755+
domain="my.local",
756+
namespace='test',
757+
env='dev',
758+
local=False,
759+
tag=1,
760+
registry='reg'
761+
)
762+
763+
root_paths = preprocess_build_overrides(
764+
root_paths=[CLOUDHARNESS_ROOT, RESOURCES],
765+
helm_values=values,
766+
merge_build_path=BUILD_MERGE_DIR
767+
)
768+
769+
build_included = [app['harness']['name']
770+
for app in values['apps'].values() if 'harness' in app]
771+
772+
cf = create_codefresh_deployment_scripts(root_paths, include=build_included,
773+
envs=['dev'],
774+
base_image_name=values['name'],
775+
helm_values=values, save=False)
776+
777+
# cloudharness-base-images and common images come from the cloud-harness root;
778+
# their working_directory must start with ./cloud-harness, not ./../cloud-harness
779+
all_build_steps = {}
780+
for step_name in [STEP_0, STEP_1, STEP_2, STEP_3]:
781+
if step_name in cf['steps']:
782+
all_build_steps.update(cf['steps'][step_name]['steps'])
783+
784+
ch_steps = {
785+
name: step for name, step in all_build_steps.items()
786+
if 'cloudharness' in name or name == 'samples'
787+
}
788+
assert ch_steps, "Expected at least one cloud-harness image build step"
789+
790+
for step_name, step in ch_steps.items():
791+
wd = step.get('working_directory', '')
792+
assert not wd.startswith('../') and './../' not in wd, (
793+
f"Cloud-harness build step '{step_name}' working_directory must not "
794+
f"escape the current directory with '../'; got: {wd}"
795+
)
796+
finally:
797+
os.chdir(old_cwd)
798+
shutil.rmtree(BUILD_MERGE_DIR, ignore_errors=True)

0 commit comments

Comments
 (0)