From 1d8eda6e5e88661ba12e07ea8ce6cd48d55400ba Mon Sep 17 00:00:00 2001 From: mr-forust Date: Thu, 8 Oct 2026 20:20:44 +0200 Subject: [PATCH 1/2] test: isolate CI summary and output files --- tests/ci_test_case.py | 22 ++++++++++++++++++ tests/test_ci_output_isolation.py | 37 +++++++++++++++++++++++++++++++ tests/test_cicd.py | 11 +++++---- tests/test_cicd_lifecycle.py | 9 ++++---- tests/test_image_matrix.py | 4 ++-- tests/test_netbird_runtime.py | 5 ++++- 6 files changed, 77 insertions(+), 11 deletions(-) create mode 100644 tests/ci_test_case.py create mode 100644 tests/test_ci_output_isolation.py diff --git a/tests/ci_test_case.py b/tests/ci_test_case.py new file mode 100644 index 0000000..00671e4 --- /dev/null +++ b/tests/ci_test_case.py @@ -0,0 +1,22 @@ +"""Keep unit-test workflow commands out of the real CI job files.""" + +import os +import tempfile +import unittest +from pathlib import Path +from unittest.mock import patch + + +class IsolatedCITestCase(unittest.TestCase): + def setUp(self): + super().setUp() + directory = tempfile.TemporaryDirectory(prefix='homelab-test-ci-') + self.addCleanup(directory.cleanup) + paths = {} + for variable in ('GITHUB_STEP_SUMMARY', 'GITHUB_OUTPUT'): + path = Path(directory.name) / variable + path.touch() + paths[variable] = str(path) + environment = patch.dict(os.environ, paths) + environment.start() + self.addCleanup(environment.stop) diff --git a/tests/test_ci_output_isolation.py b/tests/test_ci_output_isolation.py new file mode 100644 index 0000000..e3eafff --- /dev/null +++ b/tests/test_ci_output_isolation.py @@ -0,0 +1,37 @@ +"""Run the real unit tests with external CI files and detect leaked writes.""" + +import os +import subprocess +import sys +import tempfile +from pathlib import Path + +from ci_test_case import IsolatedCITestCase + + +class CIOutputIsolationTests(IsolatedCITestCase): + def test_unit_suite_preserves_external_ci_files(self): + tests = Path(__file__).resolve().parent + modules = sorted(p.stem for p in tests.glob('test_*.py') if p.name != Path(__file__).name) + with tempfile.TemporaryDirectory() as scratch: + environment = os.environ.copy() + environment['PYTHONPATH'] = str(tests) + os.pathsep + environment.get('PYTHONPATH', '') + expected = {} + for variable in ('GITHUB_STEP_SUMMARY', 'GITHUB_OUTPUT'): + path = Path(scratch) / variable + content = f'external {variable}\n' + path.write_text(content) + environment[variable] = str(path) + expected[path] = content + result = subprocess.run( # noqa: S603 -- Run local test modules with the current Python interpreter. + [sys.executable, '-m', 'unittest', *modules, '-q'], + cwd=tests.parent, + env=environment, + capture_output=True, + text=True, + check=False, + timeout=60, + ) + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + for path, content in expected.items(): + self.assertEqual(path.read_text(), content, f'Unit tests wrote to external {path.name}') diff --git a/tests/test_cicd.py b/tests/test_cicd.py index 2189885..8ff3372 100644 --- a/tests/test_cicd.py +++ b/tests/test_cicd.py @@ -9,6 +9,8 @@ import unittest from pathlib import Path from unittest.mock import patch +from ci_test_case import IsolatedCITestCase + ROOT = Path(__file__).resolve().parents[1] @@ -34,7 +36,7 @@ def release(sha='a' * 40): } -class ReleaseGateTests(unittest.TestCase): +class ReleaseGateTests(IsolatedCITestCase): def test_release_rejects_wrong_sha_missing_images_and_mutable_tags(self): for mutation in ('sha', 'missing', 'tag'): data = release() @@ -91,8 +93,9 @@ class ReleaseGateTests(unittest.TestCase): api.release({'id': 1, 'head_sha': 'a' * 40}) -class SelectionTests(unittest.TestCase): +class SelectionTests(IsolatedCITestCase): def setUp(self): + super().setUp() self.scratch = tempfile.TemporaryDirectory() self.addCleanup(self.scratch.cleanup) self.repo = Path(self.scratch.name) @@ -171,7 +174,7 @@ class SelectionTests(unittest.TestCase): self.assertEqual(result['selected']['k8s'], ['one', 'postgres', 'two']) -class ComposeConfigurationTests(unittest.TestCase): +class ComposeConfigurationTests(IsolatedCITestCase): def test_pin_preserves_project_volumes_paths_and_previous_image(self): with tempfile.TemporaryDirectory() as scratch: root = Path(scratch) @@ -305,7 +308,7 @@ class ComposeConfigurationTests(unittest.TestCase): ) -class ControllerTests(unittest.TestCase): +class ControllerTests(IsolatedCITestCase): def test_completed_stage_cannot_apply_again(self): with tempfile.TemporaryDirectory() as scratch: directory = Path(scratch) diff --git a/tests/test_cicd_lifecycle.py b/tests/test_cicd_lifecycle.py index 27bca39..6da6d29 100644 --- a/tests/test_cicd_lifecycle.py +++ b/tests/test_cicd_lifecycle.py @@ -10,10 +10,11 @@ import zipfile from pathlib import Path from unittest.mock import Mock, patch +from ci_test_case import IsolatedCITestCase from test_cicd import ROOT, controller, release, release_module -class ArtifactTests(unittest.TestCase): +class ArtifactTests(IsolatedCITestCase): def test_archive_rejects_nested_or_extra_files(self): api = object.__new__(release_module.Gitea) api.base = 'https://example.test/api/v1/repos/a/b' @@ -103,7 +104,7 @@ class ArtifactTests(unittest.TestCase): self.assertEqual(json.loads((root / 'error-pages.json').read_text())['sha'], 'e' * 40) -class DurableRunTests(unittest.TestCase): +class DurableRunTests(IsolatedCITestCase): def test_duplicate_start_only_reattaches(self): with tempfile.TemporaryDirectory() as scratch: state = Path(scratch) @@ -203,7 +204,7 @@ class DurableRunTests(unittest.TestCase): self.assertEqual(json.loads((directory / 'status.json').read_text())['state'], 'failure') -class FailureSummaryTests(unittest.TestCase): +class FailureSummaryTests(IsolatedCITestCase): def test_build_failure_keeps_progress_and_does_not_expose_exception_text(self): with tempfile.TemporaryDirectory() as scratch: summary = Path(scratch) / 'summary.md' @@ -261,7 +262,7 @@ class FailureSummaryTests(unittest.TestCase): self.assertIn('Compose requires manual recovery', content) -class InstallerTests(unittest.TestCase): +class InstallerTests(IsolatedCITestCase): def test_version_comparison_is_exact_without_network_or_host_packages(self): with tempfile.TemporaryDirectory() as scratch: root = Path(scratch) diff --git a/tests/test_image_matrix.py b/tests/test_image_matrix.py index d500ebd..0eaf4f4 100644 --- a/tests/test_image_matrix.py +++ b/tests/test_image_matrix.py @@ -3,10 +3,10 @@ import json import os import tempfile -import unittest from pathlib import Path from unittest.mock import Mock, call, patch +from ci_test_case import IsolatedCITestCase from test_cicd import release, release_module @@ -26,7 +26,7 @@ def plan_data(changed): return {'sha': 'a' * 40, 'targets': targets} -class MatrixTests(unittest.TestCase): +class MatrixTests(IsolatedCITestCase): def test_no_change_one_image_all_images_and_missing_baseline(self): for changed in (set(), {'error-pages'}, set(release_module.IMAGES)): with self.subTest(changed=changed), tempfile.TemporaryDirectory() as scratch: diff --git a/tests/test_netbird_runtime.py b/tests/test_netbird_runtime.py index 373980e..bfc259c 100644 --- a/tests/test_netbird_runtime.py +++ b/tests/test_netbird_runtime.py @@ -7,11 +7,14 @@ import tempfile import unittest from pathlib import Path +from ci_test_case import IsolatedCITestCase + ROOT = Path(__file__).resolve().parents[1] -class NetbirdRuntimeTests(unittest.TestCase): +class NetbirdRuntimeTests(IsolatedCITestCase): def setUp(self): + super().setUp() self.temp = tempfile.TemporaryDirectory() self.addCleanup(self.temp.cleanup) self.root = Path(self.temp.name) -- 2.54.0 From 0e3035ed74a15e12c423839b707dccdf8ba8c404 Mon Sep 17 00:00:00 2001 From: mr-forust Date: Thu, 8 Oct 2026 20:46:46 +0200 Subject: [PATCH 2/2] fix(ci): validate image digests and isolate test outputs --- .gitea/tests/deploy-validation.sh | 25 ++++++++++- .gitea/workflows/deploy-lib.sh | 5 ++- .gitea/workflows/release.py | 17 +++++--- tests/ci_test_case.py | 4 +- tests/test_ci_output_isolation.py | 27 +++++++++++- tests/test_cicd_lifecycle.py | 71 +++++++++++++++++++++++++++++++ 6 files changed, 135 insertions(+), 14 deletions(-) diff --git a/.gitea/tests/deploy-validation.sh b/.gitea/tests/deploy-validation.sh index f05ff7e..b93ea3b 100755 --- a/.gitea/tests/deploy-validation.sh +++ b/.gitea/tests/deploy-validation.sh @@ -91,7 +91,6 @@ if check_referenced_secrets >"$scratch/secrets.log"; then echo 'Secret check accepted a failed manifest render' >&2 exit 1 fi -printf '%s\n' 'Deploy validation regressions passed.' # New declared namespaces defer only their own resources during preflight. render_selected_resources() { @@ -132,4 +131,26 @@ if validate_server_resources true 2>"$scratch/undeclared.log"; then echo 'Preflight accepted an undeclared missing namespace' >&2 exit 1 fi -printf '%s\n' 'Namespace validation regressions passed.' +# Count services, not characters in the newline-separated service names. +compose() { + case "$*" in + *'config --format json') printf '%s\n' '{"services":{"headscale":{},"headplane":{},"web":{},"init":{"restart":"no"}}}' ;; + *'ps --status running --services') printf '%s\n' headscale headplane web ;; + *) return 1 ;; + esac +} +verify_compose_stack example.yaml >"$scratch/compose-count.log" +grep -qF 'all 3 service(s) running' "$scratch/compose-count.log" +compose() { + case "$*" in + *'config --format json') printf '%s\n' '{"services":{"headscale":{},"headplane":{},"web":{}}}' ;; + *'ps --status running --services') printf '%s\n' headscale headplane ;; + *) return 0 ;; + esac +} +if verify_compose_stack example.yaml >"$scratch/compose-missing.log"; then + echo 'Compose verification accepted a missing service' >&2 + exit 1 +fi +grep -qF 'NOT RUNNING: web' "$scratch/compose-missing.log" +printf '%s\n' 'Deploy validation regressions passed.' diff --git a/.gitea/workflows/deploy-lib.sh b/.gitea/workflows/deploy-lib.sh index f520055..305d217 100644 --- a/.gitea/workflows/deploy-lib.sh +++ b/.gitea/workflows/deploy-lib.sh @@ -827,12 +827,13 @@ stage_verify_k8s() { # actually be running. verify_compose_stack() { local cf="$1" - local expected running missing=() + local expected running svc missing=() service_count=0 expected="$(compose "$cf" config --format json | jq -r ' .services | to_entries[] | select(.value.restart != "no") | .key' | sort)" || return 1 running="$(compose "$cf" ps --status running --services | sort)" || return 1 [ -n "$expected" ] || return 0 while IFS= read -r svc; do [ -n "$svc" ] || continue + service_count=$((service_count + 1)) # restart:"no" services are allowed to have exited. if ! printf '%s\n' "$running" | grep -qx "$svc"; then missing+=("$svc") @@ -843,7 +844,7 @@ verify_compose_stack() { compose "$cf" ps --all 2>/dev/null | sed 's/^/ /' || true return 1 fi - echo " all ${#expected} service(s) running" + echo " all $service_count service(s) running" return 0 } diff --git a/.gitea/workflows/release.py b/.gitea/workflows/release.py index 83ebafb..c6b1068 100644 --- a/.gitea/workflows/release.py +++ b/.gitea/workflows/release.py @@ -305,7 +305,6 @@ def build_images(output, report, name, plan): if exists: print(f'Reuse {name}: inputs unchanged') digest = old_digest - report['reused'].append(name) else: print(f'Build {name}', flush=True) metadata = Path(docker_config) / 'metadata.json' @@ -332,14 +331,14 @@ def build_images(output, report, name, plan): env=env, ) digest = json.loads(metadata.read_text())['containerimage.digest'] - report['built'].append(name) + if not isinstance(digest, str) or not DIGEST.fullmatch(digest): + raise ValueError('Image job returned an invalid digest') release['images'][image] = digest release['inputs'][image] = inputs - if not DIGEST.fullmatch(digest): - raise ValueError('Image job returned an invalid digest') + report['reused' if exists else 'built'].append(name) output.write_text(json.dumps(release, indent=2) + '\n') report['current'] = None - report['phase'] = 'Release file saved' + report['phase'] = 'Image result file saved' finally: # Cleanup errors must neither leak credentials nor mask the original build error. try: @@ -395,13 +394,17 @@ def build(output, name, plan): result = 'success' finally: lines = [ - f'## Image release `{os.environ.get("GITHUB_SHA", "unknown")}`', + f'## Image build result `{name}`', + '', + f'- Commit: `{os.environ.get("GITHUB_SHA", "unknown")}`', '', f'- Result: **{result}**', f'- Last stage: {report["phase"]}', ] if result == 'failure': - lines.append('- No release from this build can be deployed. Open the failed step log.') + lines.append('- This image job failed. The complete release cannot be published. Open the failed step log.') + if result == 'success': + lines.append('- This is one image result. The final build job must publish the complete release.') if report['current']: lines.append(f'- Image at the failure: `{report["current"]}`') for title, key in (('Built', 'built'), ('Reused from successful CI', 'reused')): diff --git a/tests/ci_test_case.py b/tests/ci_test_case.py index 00671e4..08ad6e9 100644 --- a/tests/ci_test_case.py +++ b/tests/ci_test_case.py @@ -6,6 +6,8 @@ import unittest from pathlib import Path from unittest.mock import patch +CI_COMMAND_FILES = ('GITHUB_STEP_SUMMARY', 'GITHUB_OUTPUT', 'GITHUB_ENV', 'GITHUB_PATH', 'GITHUB_STATE') + class IsolatedCITestCase(unittest.TestCase): def setUp(self): @@ -13,7 +15,7 @@ class IsolatedCITestCase(unittest.TestCase): directory = tempfile.TemporaryDirectory(prefix='homelab-test-ci-') self.addCleanup(directory.cleanup) paths = {} - for variable in ('GITHUB_STEP_SUMMARY', 'GITHUB_OUTPUT'): + for variable in CI_COMMAND_FILES: path = Path(directory.name) / variable path.touch() paths[variable] = str(path) diff --git a/tests/test_ci_output_isolation.py b/tests/test_ci_output_isolation.py index e3eafff..4b149f9 100644 --- a/tests/test_ci_output_isolation.py +++ b/tests/test_ci_output_isolation.py @@ -5,11 +5,34 @@ import subprocess import sys import tempfile from pathlib import Path +from unittest.mock import patch -from ci_test_case import IsolatedCITestCase +from ci_test_case import CI_COMMAND_FILES, IsolatedCITestCase class CIOutputIsolationTests(IsolatedCITestCase): + def test_all_command_files_are_private_and_environment_is_restored(self): + with tempfile.TemporaryDirectory() as scratch: + external = {variable: str(Path(scratch) / variable) for variable in CI_COMMAND_FILES} + for path in external.values(): + Path(path).write_text('external CI file\n') + with patch.dict(os.environ, external): + probe = IsolatedCITestCase() + probe.setUp() + private = [] + try: + for variable in CI_COMMAND_FILES: + self.assertNotEqual(os.environ[variable], external[variable]) + path = Path(os.environ[variable]) + private.append(path) + path.write_text('test-only command\n') + finally: + probe.doCleanups() + for variable in CI_COMMAND_FILES: + self.assertEqual(os.environ[variable], external[variable]) + self.assertEqual(Path(external[variable]).read_text(), 'external CI file\n') + self.assertTrue(all(not path.exists() for path in private)) + def test_unit_suite_preserves_external_ci_files(self): tests = Path(__file__).resolve().parent modules = sorted(p.stem for p in tests.glob('test_*.py') if p.name != Path(__file__).name) @@ -17,7 +40,7 @@ class CIOutputIsolationTests(IsolatedCITestCase): environment = os.environ.copy() environment['PYTHONPATH'] = str(tests) + os.pathsep + environment.get('PYTHONPATH', '') expected = {} - for variable in ('GITHUB_STEP_SUMMARY', 'GITHUB_OUTPUT'): + for variable in CI_COMMAND_FILES: path = Path(scratch) / variable content = f'external {variable}\n' path.write_text(content) diff --git a/tests/test_cicd_lifecycle.py b/tests/test_cicd_lifecycle.py index 6da6d29..20aff19 100644 --- a/tests/test_cicd_lifecycle.py +++ b/tests/test_cicd_lifecycle.py @@ -226,6 +226,77 @@ class FailureSummaryTests(IsolatedCITestCase): self.assertIn('xdfnx-homepage', content) self.assertNotIn('private value', content) + def test_invalid_digest_is_not_reported_as_a_completed_image(self): + for digest in ('invalid-private-metadata', None, ['invalid']): + with self.subTest(digest=digest), tempfile.TemporaryDirectory() as scratch: + root = Path(scratch) + summary = root / 'summary.md' + name = 'error-pages' + context, dockerfile = release_module.IMAGES[name] + plan = { + 'sha': 'a' * 40, + 'targets': [ + { + 'name': name, + 'context': context, + 'dockerfile': dockerfile, + 'inputs': 'c' * 64, + 'reuse_digest': None, + } + ], + } + + def fake_command(*args, digest=digest, **_kwargs): + if args[:3] == ('docker', 'buildx', 'build'): + Path(args[args.index('--metadata-file') + 1]).write_text( + json.dumps({'containerimage.digest': digest}) + ) + return '' + + with ( + patch.dict( + os.environ, + { + 'GITHUB_STEP_SUMMARY': str(summary), + 'GITHUB_SHA': 'a' * 40, + 'REGISTRY_USERNAME': 'test', + 'REGISTRY_PASSWORD': 'placeholder', + }, + ), + patch.object(release_module, 'checked_plan', return_value=plan), + patch.object(release_module.Path, 'home', return_value=root), + patch.object(release_module, 'command', side_effect=fake_command), + patch.object(subprocess, 'run', return_value=subprocess.CompletedProcess([], 0)), + self.assertRaisesRegex(ValueError, 'invalid digest'), + ): + release_module.build(root / 'image.json', name, root / 'plan.json') + self.assertFalse((root / 'image.json').exists()) + content = summary.read_text() + self.assertIn('**failure**', content) + self.assertIn('### Built\n- None', content) + self.assertIn('### Completed image digests\n- None', content) + self.assertNotIn('invalid-private-metadata', content) + + def test_successful_image_result_does_not_claim_complete_release(self): + def complete_image(_output, report, _name, _plan): + report.update(phase='Image result file saved', built=['error-pages']) + report['images']['gcr.forust.xyz/forust/error-pages'] = 'sha256:' + 'b' * 64 + + with ( + patch.dict(os.environ, {'GITHUB_SHA': 'a' * 40}), + patch.object( + release_module, + 'build_images', + side_effect=complete_image, + ), + ): + release_module.build(Path('unused.json'), 'error-pages', Path('unused-plan.json')) + content = Path(os.environ['GITHUB_STEP_SUMMARY']).read_text() + self.assertIn('## Image build result `error-pages`', content) + self.assertIn('Commit: `' + 'a' * 40 + '`', content) + self.assertIn('final build job must publish the complete release', content) + self.assertNotIn('## Image release', content) + def test_deploy_failure_reports_completed_apply_and_rollback_result(self): with tempfile.TemporaryDirectory() as scratch: state = Path(scratch) -- 2.54.0