Merge pull request 'fix(ci): keep build and test reports accurate' (#118) from fix/ci-test-output-isolation into main

Reviewed-on: #118

.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.'

.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
 }
 

.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')):

tests/ci_test_case.py

@@ -0,0 +1,24 @@
+"""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
+
+CI_COMMAND_FILES = ('GITHUB_STEP_SUMMARY', 'GITHUB_OUTPUT', 'GITHUB_ENV', 'GITHUB_PATH', 'GITHUB_STATE')
+
+
+class IsolatedCITestCase(unittest.TestCase):
+    def setUp(self):
+        super().setUp()
+        directory = tempfile.TemporaryDirectory(prefix='homelab-test-ci-')
+        self.addCleanup(directory.cleanup)
+        paths = {}
+        for variable in CI_COMMAND_FILES:
+            path = Path(directory.name) / variable
+            path.touch()
+            paths[variable] = str(path)
+        environment = patch.dict(os.environ, paths)
+        environment.start()
+        self.addCleanup(environment.stop)

tests/test_ci_output_isolation.py

@@ -0,0 +1,60 @@
+"""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 unittest.mock import patch
+
+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)
+        with tempfile.TemporaryDirectory() as scratch:
+            environment = os.environ.copy()
+            environment['PYTHONPATH'] = str(tests) + os.pathsep + environment.get('PYTHONPATH', '')
+            expected = {}
+            for variable in CI_COMMAND_FILES:
+                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}')

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)

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'
@@ -225,6 +226,77 @@ class FailureSummaryTests(unittest.TestCase):
             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)
@@ -261,7 +333,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)

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:

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)