diff --git a/bodhi-server/bodhi/server/buildsys.py b/bodhi-server/bodhi/server/buildsys.py index ef55b077a1..2bd42eb830 100644 --- a/bodhi-server/bodhi/server/buildsys.py +++ b/bodhi-server/bodhi/server/buildsys.py @@ -18,6 +18,7 @@ """Define tools for interacting with the build system and a fake build system for development.""" from functools import wraps +import hashlib from threading import Lock import logging import os @@ -235,7 +236,9 @@ def getBuild(self, build='TurboGears-1.0.2.2-2.fc17', other=False, testing=False format_data = { 'registry': 'candidate-registry.fedoraproject.org', - 'hash': 'sha256:2bd64a888...', + # We make up a fake digest for the image manifest using a + # digest of the version-release string + 'hash': hashlib.sha256(f"{version}-{release}".encode("UTF-8")).hexdigest(), 'version': version, 'release': release } diff --git a/bodhi-server/bodhi/server/util.py b/bodhi-server/bodhi/server/util.py index 3b57a3d839..62249f17e2 100644 --- a/bodhi-server/bodhi/server/util.py +++ b/bodhi-server/bodhi/server/util.py @@ -1137,13 +1137,12 @@ def __exit__(self, *args, **kwargs): self.session.autoflush = self.autoflush -def _get_build_repository(build): +def _get_build_repository_and_digest(build): """ - Return the registry repository name for the given Build from the container's pull string. + Return the registry repository and container digest from the container's pull string. - Examples - - 'candidate-registry.fedoraproject.org/f29/cockpit:176-5.fc28' => 'f29/cockpit'. - 'candidate-registry.fedoraproject.org/myrepo@sha256:' => 'myrepo'. + Example - + 'candidate-registry.fedoraproject.org/myrepo@sha256:' => 'myrepo', 'sha256:. Args: build (bodhi.server.models.Build): A Build representing a container or flatpak. @@ -1154,11 +1153,12 @@ def _get_build_repository(build): koji_build = koji.getBuild(build.nvr) pull_specs = koji_build['extra']['typeinfo']['image']['index']['pull'] - # All the pull specs should have the same repository, so which one we use is arbitrary - base, tag = re.compile(r'[:@]').split(pull_specs[0], 1) + digest_pull_spec = [spec for spec in pull_specs if '@' in spec][0] + + base, digest = digest_pull_spec.split('@', 1) server, repository = base.split('/', 1) - return repository + return repository, digest def copy_container(build, destination_registry=None, destination_tag=None): @@ -1177,17 +1177,16 @@ def copy_container(build, destination_registry=None, destination_tag=None): RuntimeError: If skopeo returns a non-0 exit code. """ source_registry = config['container.source_registry'] - source_tag = '{}-{}'.format(build.nvr_version, build.nvr_release) if destination_tag is None: - destination_tag = source_tag + destination_tag = '{}-{}'.format(build.nvr_version, build.nvr_release) if destination_registry is None: destination_registry = config['container.destination_registry'] - repository = _get_build_repository(build) + repository, digest = _get_build_repository_and_digest(build) - source_url = _container_image_url(source_registry, repository, source_tag) - destination_url = _container_image_url(destination_registry, repository, destination_tag) + source_url = _container_image_url(source_registry, repository, digest=digest) + destination_url = _container_image_url(destination_registry, repository, tag=destination_tag) skopeo_cmd = [ config.get('skopeo.cmd'), 'copy', source_url, destination_url] @@ -1197,7 +1196,7 @@ def copy_container(build, destination_registry=None, destination_tag=None): cmd(skopeo_cmd, raise_on_error=True) -def _container_image_url(registry, repository, tag=None): +def _container_image_url(registry, repository, *, tag=None, digest=None): """ Return a URL suitable for use in Skopeo for copying or deleting container images. @@ -1212,7 +1211,10 @@ def _container_image_url(registry, repository, tag=None): Returns: str: A URL referencing the given build and tag in the given registry. """ - return 'docker://{}/{}:{}'.format(registry, repository, tag) + if tag: + return 'docker://{}/{}:{}'.format(registry, repository, tag) + else: + return 'docker://{}/{}@{}'.format(registry, repository, digest) def get_absolute_path(location): diff --git a/bodhi-server/tests/tasks/test_composer.py b/bodhi-server/tests/tasks/test_composer.py index b2b8477370..f9163608f8 100644 --- a/bodhi-server/tests/tasks/test_composer.py +++ b/bodhi-server/tests/tasks/test_composer.py @@ -15,6 +15,7 @@ # You should have received a copy of the GNU General Public License # along with this program; if not, write to the Free Software # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +import hashlib from http.client import IncompleteRead from unittest import mock from urllib.error import HTTPError, URLError @@ -104,6 +105,24 @@ } +def _expected_skopeo_args(config, source, dtag, extra_args=[]): + source_repository, source_version_release = source.split(':') + # Match how buildsys.py creates a fake digest + source_digest = hashlib.sha256(source_version_release.encode("UTF-8")).hexdigest() + + return [config['skopeo.cmd'], 'copy'] + extra_args + [ + 'docker://{}/{}@sha256:{}'.format(config['container.source_registry'], + source_repository, source_digest), + 'docker://{}/{}:{}'.format(config['container.destination_registry'], + source_repository, dtag) + ] + + +def _expected_skopeo_call(config, source, dtag, extra_args=[]): + return mock.call(_expected_skopeo_args(config, source, dtag, extra_args=extra_args), + shell=False, stderr=-1, stdout=-1, cwd=None) + + class TestTask: @mock.patch("bodhi.server.tasks.bugs") @mock.patch("bodhi.server.tasks.buildsys") @@ -2371,15 +2390,10 @@ def test_request_not_stable(self, Popen): # Popen should have been called three times per build, once for each of the destination # tags. With two builds that is a total of 6 calls to Popen. expected_mock_calls = [] - for source in ('testcontainer1:2.0.1-71.fc28container', - 'testcontainer2:1.0.1-1.fc28container'): + for source in ('f28/testcontainer1:2.0.1-71.fc28container', + 'f28/testcontainer2:1.0.1-1.fc28container'): for dtag in [source.split(':')[1], source.split(':')[1].split('-')[0], 'testing']: - mock_call = mock.call( - [config['skopeo.cmd'], 'copy', - 'docker://{}/f28/{}'.format(config['container.source_registry'], source), - 'docker://{}/f28/{}:{}'.format(config['container.destination_registry'], - source.split(':')[0], dtag)], - shell=False, stderr=-1, stdout=-1, cwd=None) + mock_call = _expected_skopeo_call(config, source, dtag) expected_mock_calls.append(mock_call) expected_mock_calls.append(mock.call().communicate()) assert Popen.mock_calls == expected_mock_calls @@ -2400,15 +2414,10 @@ def test_request_stable(self, Popen): # Popen should have been called three times per build, once for each of the destination # tags. With two builds that is a total of 6 calls to Popen. expected_mock_calls = [] - for source in ('testcontainer1:2.0.1-71.fc28container', - 'testcontainer2:1.0.1-1.fc28container'): + for source in ('f28/testcontainer1:2.0.1-71.fc28container', + 'f28/testcontainer2:1.0.1-1.fc28container'): for dtag in [source.split(':')[1], source.split(':')[1].split('-')[0], 'latest']: - mock_call = mock.call( - [config['skopeo.cmd'], 'copy', - 'docker://{}/f28/{}'.format(config['container.source_registry'], source), - 'docker://{}/f28/{}:{}'.format(config['container.destination_registry'], - source.split(':')[0], dtag)], - shell=False, stderr=-1, stdout=-1, cwd=None) + mock_call = _expected_skopeo_call(config, source, dtag) expected_mock_calls.append(mock_call) expected_mock_calls.append(mock.call().communicate()) assert Popen.mock_calls == expected_mock_calls @@ -2428,12 +2437,9 @@ def test_skopeo_error_code(self, Popen): t._compose_updates() # Popen should have been called once. - skopeo_cmd = [ - config['skopeo.cmd'], 'copy', - 'docker://{}/f28/testcontainer1:2.0.1-71.fc28container'.format( - config['container.source_registry']), - 'docker://{}/f28/testcontainer1:2.0.1-71.fc28container'.format( - config['container.destination_registry'])] + skopeo_cmd = _expected_skopeo_args(config, + "f28/testcontainer1:2.0.1-71.fc28container", + "2.0.1-71.fc28container") Popen.assert_called_once_with(skopeo_cmd, shell=False, stderr=-1, stdout=-1, cwd=None) assert f"{' '.join(skopeo_cmd)} returned a non-0 exit code: 1" in str(exc.value) @@ -2453,15 +2459,11 @@ def test_skopeo_extra_copy_flags(self, Popen): # Popen should have been called three times per build, once for each of the destination # tags. With two builds that is a total of 6 calls to Popen. expected_mock_calls = [] - for source in ('testcontainer1:2.0.1-71.fc28container', - 'testcontainer2:1.0.1-1.fc28container'): + for source in ('f28/testcontainer1:2.0.1-71.fc28container', + 'f28/testcontainer2:1.0.1-1.fc28container'): for dtag in [source.split(':')[1], source.split(':')[1].split('-')[0], 'testing']: - mock_call = mock.call( - [config['skopeo.cmd'], 'copy', '--dest-tls-verify=false', - 'docker://{}/f28/{}'.format(config['container.source_registry'], source), - 'docker://{}/f28/{}:{}'.format(config['container.destination_registry'], - source.split(':')[0], dtag)], - shell=False, stderr=-1, stdout=-1, cwd=None) + mock_call = _expected_skopeo_call(config, source, dtag, + extra_args=['--dest-tls-verify=false']) expected_mock_calls.append(mock_call) expected_mock_calls.append(mock.call().communicate()) assert Popen.mock_calls == expected_mock_calls @@ -2547,12 +2549,7 @@ def test_flatpak_compose(self, Popen): expected_mock_calls = [] for source in ('testflatpak1:2.0.1-71.fc28flatpak', 'testflatpak2:1.0.1-1.fc28flatpak'): for dtag in [source.split(':')[1], source.split(':')[1].split('-')[0], 'testing']: - mock_call = mock.call( - [config['skopeo.cmd'], 'copy', - 'docker://{}/{}'.format(config['container.source_registry'], source), - 'docker://{}/{}:{}'.format(config['container.destination_registry'], - source.split(':')[0], dtag)], - shell=False, stderr=-1, stdout=-1, cwd=None) + mock_call = _expected_skopeo_call(config, source, dtag) expected_mock_calls.append(mock_call) expected_mock_calls.append(mock.call().communicate()) assert Popen.mock_calls == expected_mock_calls diff --git a/bodhi-server/tests/test_util.py b/bodhi-server/tests/test_util.py index 31dee073b7..edb438d9f8 100644 --- a/bodhi-server/tests/test_util.py +++ b/bodhi-server/tests/test_util.py @@ -1230,29 +1230,28 @@ def test_err_zero_return_code(self, mock_popen, mock_error, mock_debug): mock_debug.assert_called_with('subprocess output: output\nerror') @mock.patch('bodhi.server.buildsys.get_session') - def test__get_build_repository(self, session): + def test__get_build_repository_and_digest(self, session): + pull_specs = [ + 'candidate-registry.fedoraproject.org/f29/cockpit:176-5.fc28', + 'candidate-registry.fedoraproject.org/myrepo@sha256:abcdefg123456' + ] + session.return_value = mock.Mock() - session.return_value.getBuild.side_effect = [ - { - 'extra': { - 'typeinfo': { - 'image': { - 'index': { - 'pull': [pullspec] - } + session.return_value.getBuild.return_value = { + 'extra': { + 'typeinfo': { + 'image': { + 'index': { + 'pull': pull_specs } } } } - for pullspec in [ - 'candidate-registry.fedoraproject.org/f29/cockpit:176-5.fc28', - 'candidate-registry.fedoraproject.org/myrepo@sha256:abcdefg123456' - ] - ] + } + build = mock.Mock() build.nvr = 'cockpit-167-5' - assert util._get_build_repository(build) == 'f29/cockpit' - assert util._get_build_repository(build) == 'myrepo' + assert util._get_build_repository_and_digest(build) == ('myrepo', 'sha256:abcdefg123456') session.return_value.getBuild.assert_called_with('cockpit-167-5') def test_build_evr(self): @@ -1266,8 +1265,11 @@ def test_build_evr(self): @mock.patch('bodhi.server.util.cmd', autospec=True) -@mock.patch('bodhi.server.util._container_image_url', new=lambda sr, r, st: f'{sr}:{r}:{st}') -@mock.patch('bodhi.server.util._get_build_repository', new=lambda b: 'testrepo') +@mock.patch('bodhi.server.util._container_image_url', + new=lambda sr, r, tag=None, digest=None: + f'{sr}:{r}:{tag}' if tag else f'{sr}:{r}@{digest}') +@mock.patch('bodhi.server.util._get_build_repository_and_digest', + new=lambda b: ('testrepo', 'sha256:f00d00')) class TestCopyContainer(base.BasePyTestCase): """Test the copy_container() function.""" @@ -1286,21 +1288,24 @@ def test_default(self, cmd): """Test the default code path.""" util.copy_container(self.build) - cmd.assert_called_once_with(['skopeo', 'copy', 'src:testrepo:1-1', 'dest:testrepo:1-1'], + cmd.assert_called_once_with(['skopeo', 'copy', + 'src:testrepo@sha256:f00d00', 'dest:testrepo:1-1'], raise_on_error=True) def test_with_destination_registry(self, cmd): """Test with specified destination_registry.""" util.copy_container(self.build, destination_registry='boo') - cmd.assert_called_once_with(['skopeo', 'copy', 'src:testrepo:1-1', 'boo:testrepo:1-1'], + cmd.assert_called_once_with(['skopeo', 'copy', + 'src:testrepo@sha256:f00d00', 'boo:testrepo:1-1'], raise_on_error=True) def test_with_destination_tag(self, cmd): """Test with specified destination_tag.""" util.copy_container(self.build, destination_tag='2-2') - cmd.assert_called_once_with(['skopeo', 'copy', 'src:testrepo:1-1', 'dest:testrepo:2-2'], + cmd.assert_called_once_with(['skopeo', 'copy', + 'src:testrepo@sha256:f00d00', 'dest:testrepo:2-2'], raise_on_error=True) def test_with_extra_copy_flags(self, cmd): @@ -1309,7 +1314,7 @@ def test_with_extra_copy_flags(self, cmd): util.copy_container(self.build) cmd.assert_called_once_with(['skopeo', 'copy', '--quiet', '--remove-signatures', - 'src:testrepo:1-1', 'dest:testrepo:1-1'], + 'src:testrepo@sha256:f00d00', 'dest:testrepo:1-1'], raise_on_error=True)