diff --git a/pubtools/_quay/command_executor.py b/pubtools/_quay/command_executor.py index b9c75d86..87cc25dc 100644 --- a/pubtools/_quay/command_executor.py +++ b/pubtools/_quay/command_executor.py @@ -15,6 +15,7 @@ import paramiko from shlex import quote + LOG = logging.getLogger("pubtools.quay") diff --git a/pubtools/_quay/iib_operations.py b/pubtools/_quay/iib_operations.py index 9632ada1..3cfad21d 100644 --- a/pubtools/_quay/iib_operations.py +++ b/pubtools/_quay/iib_operations.py @@ -463,7 +463,11 @@ def task_iib_build_from_scratch( metadata={"tags": {target_settings["quay_operator_repository"]: [tag]}}, repos={target_settings["quay_operator_repository"]: [tag]}, ) - outdated_manifests = item_processor.generate_existing_manifests_metadata(vitem) + existing_manifests = item_processor.generate_existing_manifests_metadata(vitem) + outdated_manifests = [] + for ref, tag, man_arch_dig in existing_manifests: + if man_arch_dig.arch in ("amd64", "x86_64"): + outdated_manifests.append((man_arch_dig.digest, tag, ref)) current_signatures = _sign_index_image( build_details.internal_index_image_copy_resolved, iib_namespace, diff --git a/pubtools/_quay/signer_wrapper.py b/pubtools/_quay/signer_wrapper.py index 2c6cb845..0daeb256 100644 --- a/pubtools/_quay/signer_wrapper.py +++ b/pubtools/_quay/signer_wrapper.py @@ -9,6 +9,7 @@ from marshmallow import Schema, fields, EXCLUDE +from .quay_api_client import QuayApiClient from .utils.misc import run_entrypoint, get_pyxis_ssl_paths, run_in_parallel, log_step from .item_processor import SignEntry @@ -356,6 +357,119 @@ def remove_signatures( self._remove_signatures(to_remove) +class CosignSignerSettingsSchema(Schema): + """Validation schema for cosign signer settings.""" + + quay_namespace = fields.String(required=True) + quay_host = fields.String(required=True) + dest_quay_api_token = fields.String(required=True) + + +class CosignSignerWrapper(SignerWrapper): + """Wrapper for cosign signer functionality.""" + + label = "cosign_signer" + entry_point_conf = ["pubtools-sign", "modules", "pubtools-sign-cosign-container-sign"] + + SCHEMA = CosignSignerSettingsSchema + + def _list_signatures(self, repository: str, tag: str) -> List[Tuple[str, str]]: + """List cosign signatures for given repository. + + This methods runs pubtools-sign-cosign-container-list entrypoint which is expected to + return list of full references to signature tags in format sha256-.sig + + Args: + repository (str): Repository to list signatures for. + tag (str): Tag to list signatures for. + Returns: + List[Tuple[str, str]]: List of (repository, signature tag) tuples + for existing signatures. + """ + full_reference = ( + f"{self.settings['quay_host']}/" + + f"{self.settings['quay_namespace']}/{repository.replace('/','----')}" + + f":{tag}" + ) + existing_signatures = run_entrypoint( + ("pubtools-sign", "modules", "pubtools-sign-cosign-container-list"), [full_reference] + ) + if existing_signatures[0]: + return [ + (repository, e.split(":")[-1].replace(".sig", "").replace("-", ":")) + for e in existing_signatures[1] + ] + else: + LOG.warning("Fetch existing signatures error:" + existing_signatures[1]) + return [] + + def _filter_to_remove( + self, + signatures: List[Tuple[str, str, str]], + _exclude: Optional[List[Tuple[str, str, str]]] = None, + ) -> List[str]: + """Filter signatures to remove. + + Args: + signatures (List[Tuple[str, str, str]]): List of (digest, tag, repository) + tuples of signautres to remove. + _exclude (Optional[List[Tuple[str, str, str]]]): List of (digest, tag, repository) + tuples of signautres to keep. + """ + repo_tag_list = list(set([(x[2], x[1]) for x in signatures])) + signatures_to_remove = [(x[2], x[0]) for x in signatures] + signatures_to_exclude = [(x[2], x[0]) for x in _exclude or []] + existing_signatures = set( + sum( + run_in_parallel( + self._list_signatures, [repo_tag for repo_tag in repo_tag_list] + ).values(), + [], + ) + ) + to_remove = [] + for existing_signature in existing_signatures: + if ( + existing_signature in signatures_to_remove + and existing_signature not in signatures_to_exclude + ): + to_remove.append(existing_signature) + LOG.debug( + f"Removing signature " + f"Repository: {existing_signature[0]}, " + f"Digest: {existing_signature[1]}, " + ) + return to_remove + + def _run_remove_signatures(self, signatures_to_remove: List[Tuple[str, str]]): + """Remove signatures from the sigstore. + + Args: + signatures_to_remove (List[Tuple(str, str)]): List of signatures to remove. + """ + qc = QuayApiClient(self.settings["dest_quay_api_token"], host=self.settings["quay_host"]) + for sig_to_remove in signatures_to_remove: + ref = self.settings["quay_namespace"] + "/" + sig_to_remove[0].replace("/", "----") + sig_tag = sig_to_remove[1].replace(":", "-") + ".sig" + qc.delete_tag(ref, sig_tag) + + def remove_signatures( + self, + signatures: List[Tuple[str, str, str]], + _exclude: Optional[List[Tuple[str, str, str]]] = None, + ): + """Remove signatures from sigstore. + + Args: + signatures (list): List of tuples containing (digest, reference, repository) of + signatures to remove. + exclude (Optional[List[Tuple[str, str, str]]]): List of (digest, tag, repository) + tuples of signautres to keep. + """ + to_remove = self._filter_to_remove(signatures, _exclude=_exclude) + self._remove_signatures(to_remove) + + SIGNER_BY_LABEL = { wrapper.label: wrapper for name, wrapper in locals().items() diff --git a/tests/conftest.py b/tests/conftest.py index 92792ac4..06cc6451 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -930,7 +930,10 @@ def target_settings(): "cosign_rekor_url": "https://some-rekor.com", "pyxis_ssl_crtfile": "/pyxis.crt", "pyxis_ssl_keyfile": "/pyxis.key", - "signing": [{"enabled": True, "label": "msg_signer", "config_file": "test-config.yml"}], + "signing": [ + {"enabled": True, "label": "msg_signer", "config_file": "test-config.yml"}, + {"enabled": True, "label": "cosign_signer", "config_file": "test-config.yml"}, + ], "retry_sleep_time": 0, } @@ -2019,6 +2022,15 @@ def msg_signer_settings(): } +@pytest.fixture +def cosign_signer_settings(): + return { + "quay_host": "test-quay.io", + "quay_namespace": "testing", + "dest_quay_api_token": "testing-quay-api-token", + } + + def run_in_serial(func, data, threads): ret = [] for dentry in data: diff --git a/tests/test_cosign_signer.py b/tests/test_cosign_signer.py new file mode 100644 index 00000000..5216bc5a --- /dev/null +++ b/tests/test_cosign_signer.py @@ -0,0 +1,33 @@ +import mock + +from pubtools._quay.signer_wrapper import CosignSignerWrapper + + +def test_remove_signatures(cosign_signer_settings, fake_cert_key_paths): + with mock.patch("pubtools._quay.signer_wrapper.run_entrypoint") as mock_run_entry_point: + mock_run_entry_point.return_value = ( + True, + ["quay.io/testing/repository:sha256-123456789.sig"], + ) + sw = CosignSignerWrapper(config_file="fake-config-file", settings=cosign_signer_settings) + with mock.patch( + "pubtools._quay.signer_wrapper.QuayApiClient.delete_tag" + ) as mock_delete_tag: + sw.remove_signatures([("sha256:123456789", "tag", "testing/repository")]) + mock_delete_tag.assert_called_once_with( + "testing/testing----repository", "sha256-123456789.sig" + ) + + +def test_remove_signatures_failure(cosign_signer_settings, fake_cert_key_paths): + with mock.patch("pubtools._quay.signer_wrapper.run_entrypoint") as mock_run_entry_point: + mock_run_entry_point.return_value = ( + False, + "test-error", + ) + sw = CosignSignerWrapper(config_file="fake-config-file", settings=cosign_signer_settings) + with mock.patch( + "pubtools._quay.signer_wrapper.QuayApiClient.delete_tag" + ) as mock_delete_tag: + sw.remove_signatures([("sha256:123456789", "tag", "testing/repository")]) + mock_delete_tag.assert_not_called() diff --git a/tests/test_iib_operations.py b/tests/test_iib_operations.py index f1f0f1f7..211d8ae9 100644 --- a/tests/test_iib_operations.py +++ b/tests/test_iib_operations.py @@ -152,15 +152,63 @@ def fake_setup( "some-registry.com/iib-namespace/iib@sha256:a1a1a1", ["8-1"], ) - signer_wrapper_run_entry_point.return_value = [ - { - "_id": 1, - "manifest_digest": "sha256:5555555555", - "reference": "some-registry.com/operators/index-image:8", - "sig_key_id": "key", - "repository": "operators/index-image", - } + signer_wrapper_run_entry_point_sf = [ + # get signatures from pyxis + [ + { + "_id": 1, + "manifest_digest": "sha256:5555555555", + "reference": "some-registry.com/operators/index-image:8", + "sig_key_id": "key", + "repository": "operators/index-image", + } + ], + [ + { + "_id": 1, + "manifest_digest": "sha256:5555555555", + "reference": "some-registry.com/operators/index-image:8-timestamp", + "sig_key_id": "key", + "repository": "operators/index-image", + } + ], + # store signatures to pyxis + [ + { + "_id": 1, + "manifest_digest": "sha256:5555555555", + "reference": "some-registry.com/operators/index-image:8", + "sig_key_id": "key", + "repository": "operators/index-image", + } + ], + [ + { + "_id": 1, + "manifest_digest": "sha256:5555555555", + "reference": "some-registry.com/operators/index-image:8-timestamp", + "sig_key_id": "key", + "repository": "operators/index-image", + } + ], ] + signer_wrapper_run_entry_point_sf.append( + # remove signatures from pyxis (fetch existing) + [ + { + "_id": 1, + "manifest_digest": "sha256:5555555555", + "reference": "some-registry.com/operators/index-image:8", + "sig_key_id": "key", + "repository": "operators/index-image", + } + ] + ) + signer_wrapper_run_entry_point_sf.append((True, ["quay.io/testing/repo:sha256-5555555555.sig"])) + signer_wrapper_run_entry_point_sf.append((True, ["quay.io/testing/repo:sha256-5555555555.sig"])) + + signer_wrapper_run_entry_point.side_effect = signer_wrapper_run_entry_point_sf + mock_iib_add_bundles.return_value = build_details signer_wrapper_entry_point.return_value = { "signer_result": { @@ -188,7 +236,6 @@ def test_task_iib_add_bundles( mock_timestamp, signer_wrapper_entry_point, signer_wrapper_run_entry_point, - signer_wrapper_store_signed, signer_wrapper_remove_signatures, msg_signer_wrapper_save_signatures_file, target_settings, @@ -267,40 +314,6 @@ def test_task_iib_add_bundles( ), ] ) - signer_wrapper_store_signed.assert_called_with( - { - "signer_result": {"status": "ok"}, - "operation": { - "references": ["some-registry.com/iib-namespace/new-index-image:8"], - "manifests": [ - "sha256:bd6eba96070efe86b64b9a212680ca6d46a2e30f0a7d8e539f657eabc45c35a6" - ], - }, - "operation_results": [ - [ - { - "i": 0, - "msg": { - "errors": [], - "manifest_digest": "sha256:bd6eba96070ef" - "e86b64b9a212680ca6d46a2e30f0a7d8e539f657eabc45c35a6", - "pub_task_id": "12345", - "repo": "iib-namespace/new-index-image", - "request_id": "89cf86e0-8403-46e0-b5ed-5984a635e89e", - "request_received_time": "2023-10-17T08:08:01.544757", - "sig_key_id": "37036783", - "sig_keyname": "testing", - "signature_type": "container_signature", - "signed_claim": "claim1", - }, - }, - {}, - ] - ], - "signing_key": "sig-key", - } - ) - assert mock_run_tag_images.call_count == 2 assert mock_run_tag_images.call_args_list[0] == mock.call( "some-registry.com/iib-namespace/new-index-image:8", @@ -318,7 +331,10 @@ def test_task_iib_add_bundles( True, target_settings, ) - signer_wrapper_remove_signatures.assert_called_once_with([1]) + assert signer_wrapper_remove_signatures.mock_calls == [ + mock.call([1]), + mock.call([("operators/index-image", "sha256:5555555555")]), + ] @mock.patch("pubtools._quay.iib_operations.ContainerImagePusher.run_tag_images") @@ -481,7 +497,10 @@ def test_task_iib_add_bundles_operator_ns( True, target_settings, ) - signer_wrapper_remove_signatures.assert_called_once_with([1]) + assert signer_wrapper_remove_signatures.mock_calls == [ + mock.call([1]), + mock.call([("operators/index-image", "sha256:5555555555")]), + ] @mock.patch("pubtools._quay.iib_operations.ContainerImagePusher.run_tag_images") @@ -560,7 +579,10 @@ def test_task_iib_remove_operators( True, target_settings, ) - signer_wrapper_remove_signatures.assert_called_once_with([1]) + signer_wrapper_remove_signatures.mock_calls == [ + mock.call([1]), + mock.call([("operators/index-image", "sha256:5555555555")]), + ] @mock.patch("pubtools._quay.iib_operations.ContainerImagePusher.run_tag_images") @@ -633,7 +655,10 @@ def test_task_iib_remove_operators_operator_ns( True, target_settings, ) - signer_wrapper_remove_signatures.assert_called_once_with([1]) + signer_wrapper_remove_signatures.mock_calls == [ + mock.call([1]), + mock.call([("operators/index-image", "sha256:5555555555")]), + ] @mock.patch("pubtools._quay.iib_operations.ContainerImagePusher.run_tag_images") @@ -647,6 +672,7 @@ def test_task_iib_build_from_scratch( signer_wrapper_entry_point, signer_wrapper_run_entry_point, msg_signer_wrapper_save_signatures_file, + signer_wrapper_remove_signatures, fake_quay_client_get_operator_quay_client, target_settings, fake_cert_key_paths, @@ -803,6 +829,7 @@ def test_task_iib_build_from_scratch_operator_ns( signer_wrapper_entry_point, signer_wrapper_run_entry_point, msg_signer_wrapper_save_signatures_file, + signer_wrapper_remove_signatures, fake_quay_client_get_operator_quay_client, target_settings, fake_cert_key_paths, diff --git a/tests/test_integration.py b/tests/test_integration.py index 8a63d0d6..3df0cf88 100644 --- a/tests/test_integration.py +++ b/tests/test_integration.py @@ -83,14 +83,72 @@ def test_push_docker_multiarch_merge_ml_operator( {"fbc_opt_in": False}, {"fbc_opt_in": False}, ] - signer_wrapper_run_entry_point.return_value = [ - { - "_id": 1, - "manifest_digest": "sha256:6666666666", - "reference": "some-registry1.com/namespace/operators/index-image:v4.6", - "sig_key_id": "some-key", - "repository": "operators/index-image", - } + signer_wrapper_run_entry_point.side_effect = [ + [ + { + "_id": 1, + "manifest_digest": "sha256:5555555555", + "reference": "some-registry1.com/target/repo:latest-test-tag", + "sig_key_id": "some-key", + "repository": "operators/index-image", + } + ], + [ + { + "_id": 1, + "manifest_digest": "sha256:5555555555", + "reference": "some-registry1.com/target/repo:latest-test-tag", + "sig_key_id": "some-key", + "repository": "operators/index-image", + } + ], + [ + { + "_id": 1, + "manifest_digest": "sha256:5555555555", + "reference": "some-registry2.com/target/repo:latest-test-tag", + "sig_key_id": "some-key", + "repository": "operators/index-image", + } + ], + [ + { + "_id": 1, + "manifest_digest": "sha256:6666666666", + "reference": "some-registry1.com/namespace/operators/index-image:v4.5", + "sig_key_id": "some-key", + "repository": "operators/index-image", + } + ], + [ + { + "_id": 1, + "manifest_digest": "sha256:6666666666", + "reference": "some-registry1.com/namespace/operators/index-image:v4.5", + "sig_key_id": "some-key", + "repository": "operators/index-image", + } + ], + [ + { + "_id": 1, + "manifest_digest": "sha256:6666666666", + "reference": "some-registry1.com/namespace/operators/index-image:v4.6", + "sig_key_id": "some-key", + "repository": "operators/index-image", + } + ], + [ + { + "_id": 1, + "manifest_digest": "sha256:6666666666", + "reference": "some-registry1.com/namespace/operators/index-image:v4.6", + "sig_key_id": "some-key", + "repository": "operators/index-image", + } + ], + (True, ["quay.io/testing/repo:sha256-6666666666.sig"]), + (True, ["quay.io/testing/repo:sha256-6666666666.sig"]), ] mock_run_entrypoint_operator_pusher.side_effect = [ # pubtools-pyxis-get-operator-indices @@ -184,6 +242,7 @@ def test_push_docker_multiarch_merge_ml_operator( push_docker.run() signer_wrapper_entry_point.assert_has_calls( [ + # msg signing wrapper mock.call( config_file="test-config.yml", signing_key="some-key", @@ -200,6 +259,24 @@ def test_push_docker_multiarch_merge_ml_operator( task_id="1", repo="target/repo", ), + # cosign + mock.call( + config_file="test-config.yml", + signing_key="some-key", + reference="some-registry1.com/target/repo:latest-test-tag", + digest="sha256:5555555555", + task_id="1", + repo="target/repo", + ), + mock.call( + config_file="test-config.yml", + signing_key="some-key", + reference="some-registry2.com/target/repo:latest-test-tag", + digest="sha256:5555555555", + task_id="1", + repo="target/repo", + ), + # msg signer mock.call( config_file="test-config.yml", signing_key="some-key", @@ -216,6 +293,39 @@ def test_push_docker_multiarch_merge_ml_operator( task_id="1", repo="operators/index-image", ), + mock.call( + config_file="test-config.yml", + signing_key="some-key", + reference="some-registry1.com/namespace/operators/index-image:v4.5", + digest="sha256:5555555555", + task_id="1", + repo="operators/index-image", + ), + mock.call( + config_file="test-config.yml", + signing_key="some-key", + reference="some-registry2.com/namespace/operators/index-image:v4.5", + digest="sha256:5555555555", + task_id="1", + repo="operators/index-image", + ), + mock.call( + config_file="test-config.yml", + signing_key="some-key", + reference="some-registry1.com/namespace/operators/index-image:v4.6", + digest="sha256:5555555555", + task_id="1", + repo="operators/index-image", + ), + mock.call( + config_file="test-config.yml", + signing_key="some-key", + reference="some-registry2.com/namespace/operators/index-image:v4.6", + digest="sha256:5555555555", + task_id="1", + repo="operators/index-image", + ), + # cosign mock.call( config_file="test-config.yml", signing_key="some-key", @@ -234,7 +344,10 @@ def test_push_docker_multiarch_merge_ml_operator( ), ] ) - signer_wrapper_remove_signatures.assert_called_with([1]) + assert signer_wrapper_remove_signatures.mock_calls == [ + mock.call([1]), + mock.call([("operators/index-image", "sha256:6666666666")]), + ] @mock.patch("pubtools._quay.push_docker.SecurityManifestPusher")