From 0e3035ed74a15e12c423839b707dccdf8ba8c404 Mon Sep 17 00:00:00 2001 From: mr-forust Date: Thu, 8 Oct 2026 20:46:46 +0200 Subject: [PATCH] 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)