diff --git a/ops/gcp_leoclean_nosend_package.py b/ops/gcp_leoclean_nosend_package.py index 6371df2..f8d63ef 100644 --- a/ops/gcp_leoclean_nosend_package.py +++ b/ops/gcp_leoclean_nosend_package.py @@ -1741,36 +1741,6 @@ def _local_image_reference_config_id( return config_id -def _remove_new_image_reference( - docker_binary: Path, - docker_host: str, - docker_config: Path, - image_reference: str, - expected_config_id: str, -) -> None: - observed_config_id = _local_image_reference_config_id( - docker_binary, - docker_host, - docker_config, - image_reference, - ) - if observed_config_id is None: - return - _require(observed_config_id == expected_config_id, "new registry image reference changed before cleanup") - _run_docker( - docker_binary, - docker_host, - docker_config, - ["image", "rm", image_reference], - check=True, - timeout=120, - ) - _require( - _local_image_reference_config_id(docker_binary, docker_host, docker_config, image_reference) is None, - "new registry image reference cleanup did not pass", - ) - - def inspect_registry_image( image_input: dict[str, Any], image_reference: str, @@ -1781,7 +1751,6 @@ def inspect_registry_image( docker_binary: Path, docker_host: str, docker_config: Path, - cleanup_new_reference: bool = False, ) -> dict[str, Any]: """Pull and inspect one immutable Artifact Registry image without starting it.""" @@ -1807,18 +1776,8 @@ def inspect_registry_image( "image input differs from the expected reviewed input SHA-256", ) _validate_docker_authority(docker_binary, docker_host, docker_config) - prestate_observed = False - pulled_config_id: str | None = None - inspection: dict[str, Any] | None = None - primary_error: BaseException | None = None with _DockerCacheLock(docker_config): try: - if cleanup_new_reference: - _require( - _local_image_reference_config_id(docker_binary, docker_host, docker_config, image_reference) is None, - "exact registry image reference must be absent before inspection cleanup", - ) - prestate_observed = True _run_docker( docker_binary, docker_host, @@ -1851,30 +1810,7 @@ def inspect_registry_image( "inspected image differs from the expected reviewed config digest", ) except (OSError, subprocess.SubprocessError, PackageError) as exc: - primary_error = exc - finally: - try: - if cleanup_new_reference and prestate_observed: - if pulled_config_id is None: - pulled_config_id = _local_image_reference_config_id( - docker_binary, - docker_host, - docker_config, - image_reference, - ) - if pulled_config_id is not None: - _remove_new_image_reference( - docker_binary, - docker_host, - docker_config, - image_reference, - pulled_config_id, - ) - except (OSError, subprocess.SubprocessError, PackageError) as cleanup_error: - raise PackageError("immutable registry image cleanup failed") from cleanup_error - if primary_error is not None: - raise PackageError("immutable registry image inspection failed") from primary_error - _require(inspection is not None, "immutable registry image inspection failed") + raise PackageError("immutable registry image inspection failed") from exc return inspection @@ -2392,7 +2328,6 @@ def main(argv: list[str] | None = None) -> int: docker_binary=args.docker_binary, docker_host=args.docker_host, docker_config=args.docker_config, - cleanup_new_reference=False, ) result = build_release_descriptor(image_input, inspection) publish_finalize_bundle(args.output_bundle, result) diff --git a/tests/test_gcp_leoclean_nosend_package.py b/tests/test_gcp_leoclean_nosend_package.py index ab52c4b..2996a4b 100644 --- a/tests/test_gcp_leoclean_nosend_package.py +++ b/tests/test_gcp_leoclean_nosend_package.py @@ -1064,7 +1064,7 @@ def test_local_image_inventory_rejects_inventory_and_inspection_disagreement( @pytest.mark.parametrize("config_digest", ["sha256:" + "3" * 64, "sha256:" + "a" * 64]) -def test_registry_inspection_uses_explicit_local_authority_and_removes_new_reference( +def test_registry_inspection_absent_pull_dependent_retains_new_reference( tmp_path: Path, monkeypatch: pytest.MonkeyPatch, config_digest: str, @@ -1075,7 +1075,7 @@ def test_registry_inspection_uses_explicit_local_authority_and_removes_new_refer prefix = [str(docker_binary), "--host", docker_host, "--config", str(docker_config)] calls: list[list[str]] = [] environments: list[dict[str, str]] = [] - state = {"present": False} + state = {"present": False, "dependent": False, "inspection_calls": 0} payload = json.dumps( [docker_inspection_fixture(image_input, reference, config_digest=config_digest)] ).encode() @@ -1099,6 +1099,9 @@ def test_registry_inspection_uses_explicit_local_authority_and_removes_new_refer stderr=b"", ) if operation[:2] == ["image", "inspect"]: + state["inspection_calls"] += 1 + if state["inspection_calls"] == 2: + state["dependent"] = True return subprocess.CompletedProcess( arguments, 0, @@ -1109,9 +1112,7 @@ def test_registry_inspection_uses_explicit_local_authority_and_removes_new_refer state["present"] = True return subprocess.CompletedProcess(arguments, 0, stdout=b"pulled\n", stderr=b"") if operation[:2] == ["image", "rm"]: - assert "--force" not in operation - state["present"] = False - return subprocess.CompletedProcess(arguments, 0, stdout=b"removed\n", stderr=b"") + raise AssertionError("digest reference must never be removed") raise AssertionError(operation) monkeypatch.setattr(package.subprocess, "run", fake_run) @@ -1125,19 +1126,18 @@ def test_registry_inspection_uses_explicit_local_authority_and_removes_new_refer docker_binary=docker_binary, docker_host=docker_host, docker_config=docker_config, - cleanup_new_reference=True, ) assert inspection["status"] == "pass" - assert state["present"] is False + assert state == {"present": True, "dependent": True, "inspection_calls": 2} assert [call[5:] for call in calls if call[5] == "pull"] == [ ["pull", "--quiet", "--platform", package.PLATFORM, reference] ] assert all(not any(name.startswith("DOCKER_") for name in environment) for environment in environments) - assert [call[5:] for call in calls if call[5:7] == ["image", "rm"]] == [["image", "rm", reference]] + assert not any(call[5:7] == ["image", "rm"] for call in calls) -def test_registry_inspection_default_retains_preexisting_reference( +def test_registry_inspection_retains_preexisting_reference( tmp_path: Path, monkeypatch: pytest.MonkeyPatch, ) -> None: @@ -1145,7 +1145,7 @@ def test_registry_inspection_default_retains_preexisting_reference( reference = f"{package.IMAGE_REPOSITORY}@sha256:{IMAGE_DIGEST}" docker_binary, docker_host, docker_config = docker_authority(tmp_path) payload = json.dumps([docker_inspection_fixture(image_input, reference)]).encode() - state = {"present": True, "pulls": 0, "removed": False} + state = {"present": True, "pulls": 0} def fake_run(arguments: list[str], **_kwargs): operation = arguments[5:] @@ -1162,8 +1162,7 @@ def test_registry_inspection_default_retains_preexisting_reference( state["pulls"] += 1 return subprocess.CompletedProcess(arguments, 0, stdout=b"pulled\n", stderr=b"") if operation[:2] == ["image", "rm"]: - state["removed"] = True - return subprocess.CompletedProcess(arguments, 0, stdout=b"removed\n", stderr=b"") + raise AssertionError("digest reference must never be removed") raise AssertionError(operation) monkeypatch.setattr(package.subprocess, "run", fake_run) @@ -1180,10 +1179,10 @@ def test_registry_inspection_default_retains_preexisting_reference( ) assert inspection["status"] == "pass" - assert state == {"present": True, "pulls": 1, "removed": False} + assert state == {"present": True, "pulls": 1} -def test_registry_inspection_rejects_reviewed_config_digest_mismatch_and_cleans_reference( +def test_registry_inspection_rejects_config_digest_mismatch_and_retains_reference( tmp_path: Path, monkeypatch: pytest.MonkeyPatch, ) -> None: @@ -1224,14 +1223,13 @@ def test_registry_inspection_rejects_reviewed_config_digest_mismatch_and_cleans_ docker_binary=docker_binary, docker_host=docker_host, docker_config=docker_config, - cleanup_new_reference=True, ) assert "expected reviewed config digest" in str(raised.value.__cause__) - assert state == {"present": False, "removed": True} + assert state == {"present": True, "removed": False} -def test_registry_inspection_failure_is_redacted_and_cleans_new_reference( +def test_registry_inspection_pull_failure_is_redacted_and_does_not_remove_reference( tmp_path: Path, monkeypatch: pytest.MonkeyPatch, ) -> None: @@ -1272,14 +1270,13 @@ def test_registry_inspection_failure_is_redacted_and_cleans_new_reference( docker_binary=docker_binary, docker_host=docker_host, docker_config=docker_config, - cleanup_new_reference=True, ) - assert state["present"] is False + assert state["present"] is True assert "credential=value" not in str(raised.value) -def test_registry_inspection_validation_failure_removes_new_reference( +def test_registry_inspection_validation_failure_retains_reference( tmp_path: Path, monkeypatch: pytest.MonkeyPatch, ) -> None: @@ -1327,13 +1324,12 @@ def test_registry_inspection_validation_failure_removes_new_reference( docker_binary=docker_binary, docker_host=docker_host, docker_config=docker_config, - cleanup_new_reference=True, ) - assert state == {"present": False, "removed": True} + assert state == {"present": True, "removed": False} -def test_registry_inspection_cleanup_failure_prevents_success( +def test_registry_inspection_never_calls_image_rm_after_success( tmp_path: Path, monkeypatch: pytest.MonkeyPatch, ) -> None: @@ -1368,21 +1364,22 @@ def test_registry_inspection_cleanup_failure_prevents_success( monkeypatch.setattr(package.subprocess, "run", fake_run) - with pytest.raises(package.PackageError, match="cleanup failed"): - package.inspect_registry_image( - image_input, - reference, - expected_revision=image_input["runtime"]["teleo_git_head"], - expected_input_sha256=image_input["input_sha256"], - expected_config_digest="sha256:" + "3" * 64, - docker_binary=docker_binary, - docker_host=docker_host, - docker_config=docker_config, - cleanup_new_reference=True, - ) + inspection = package.inspect_registry_image( + image_input, + reference, + expected_revision=image_input["runtime"]["teleo_git_head"], + expected_input_sha256=image_input["input_sha256"], + expected_config_digest="sha256:" + "3" * 64, + docker_binary=docker_binary, + docker_host=docker_host, + docker_config=docker_config, + ) + + assert inspection["status"] == "pass" + assert state["present"] is True -def test_registry_inspection_refuses_to_remove_changed_reference( +def test_registry_inspection_has_no_post_inspection_cleanup_window( tmp_path: Path, monkeypatch: pytest.MonkeyPatch, ) -> None: @@ -1424,24 +1421,24 @@ def test_registry_inspection_refuses_to_remove_changed_reference( monkeypatch.setattr(package.subprocess, "run", fake_run) - with pytest.raises(package.PackageError, match="cleanup failed"): - package.inspect_registry_image( - image_input, - reference, - expected_revision=image_input["runtime"]["teleo_git_head"], - expected_input_sha256=image_input["input_sha256"], - expected_config_digest="sha256:" + "3" * 64, - docker_binary=docker_binary, - docker_host=docker_host, - docker_config=docker_config, - cleanup_new_reference=True, - ) + inspection = package.inspect_registry_image( + image_input, + reference, + expected_revision=image_input["runtime"]["teleo_git_head"], + expected_input_sha256=image_input["input_sha256"], + expected_config_digest="sha256:" + "3" * 64, + docker_binary=docker_binary, + docker_host=docker_host, + docker_config=docker_config, + ) + assert inspection["status"] == "pass" assert state["present"] is True assert state["removed"] is False + assert state["inventory_calls"] == 1 -def test_registry_inspection_rejects_preexisting_reference_without_mutation( +def test_registry_inspection_accepts_preexisting_reference_without_removal( tmp_path: Path, monkeypatch: pytest.MonkeyPatch, ) -> None: @@ -1463,25 +1460,27 @@ def test_registry_inspection_rejects_preexisting_reference_without_mutation( ) if operation[:2] == ["image", "inspect"]: return subprocess.CompletedProcess(arguments, 0, stdout=payload, stderr=b"") + if operation[0] == "pull": + return subprocess.CompletedProcess(arguments, 0, stdout=b"pulled\n", stderr=b"") + if operation[:2] == ["image", "rm"]: + raise AssertionError("digest reference must never be removed") raise AssertionError(operation) monkeypatch.setattr(package.subprocess, "run", fake_run) - with pytest.raises(package.PackageError, match="inspection failed") as raised: - package.inspect_registry_image( - image_input, - reference, - expected_revision=image_input["runtime"]["teleo_git_head"], - expected_input_sha256=image_input["input_sha256"], - expected_config_digest="sha256:" + "3" * 64, - docker_binary=docker_binary, - docker_host=docker_host, - docker_config=docker_config, - cleanup_new_reference=True, - ) + inspection = package.inspect_registry_image( + image_input, + reference, + expected_revision=image_input["runtime"]["teleo_git_head"], + expected_input_sha256=image_input["input_sha256"], + expected_config_digest="sha256:" + "3" * 64, + docker_binary=docker_binary, + docker_host=docker_host, + docker_config=docker_config, + ) - assert "must be absent" in str(raised.value.__cause__) - assert not any(operation and operation[0] == "pull" for operation in operations) + assert inspection["status"] == "pass" + assert any(operation and operation[0] == "pull" for operation in operations) assert not any(operation[:2] == ["image", "rm"] for operation in operations) @@ -1702,7 +1701,6 @@ def test_finalize_embeds_inspection_before_rendering_unit( "docker_binary": docker_binary, "docker_host": docker_host, "docker_config": docker_config, - "cleanup_new_reference": False, } assert json.loads(capsys.readouterr().out) == {"schema": package.RELEASE_SCHEMA, "status": "pass"}