From e54368aebedaa81f030cd4cb145746b775fcf97e Mon Sep 17 00:00:00 2001 From: gnaponie Date: Aug 12 2019 13:00:22 +0000 Subject: Fix: Freshmaker gets unauthorized getting arch Freshmaker cannot access the registry to get the arch. Instead of the current behavior, let's use koji to get the info. ref. FACTORY-4945 --- diff --git a/freshmaker/kojiservice.py b/freshmaker/kojiservice.py index 2b57366..94f61c0 100644 --- a/freshmaker/kojiservice.py +++ b/freshmaker/kojiservice.py @@ -207,6 +207,9 @@ class KojiService(object): def get_task_info(self, task_id): return self.session.getTaskInfo(task_id) + def list_archives(self, build_id): + return self.session.listArchives(build_id) + def get_container_build_id_from_task(self, task_id): """ Return container build id by check 'koji_builds' in build diff --git a/freshmaker/lightblue.py b/freshmaker/lightblue.py index 07485d3..560df50 100644 --- a/freshmaker/lightblue.py +++ b/freshmaker/lightblue.py @@ -34,7 +34,6 @@ import kobo.rpmlib from itertools import groupby from six.moves import http_client -from six.moves.urllib.parse import urlparse import concurrent.futures from freshmaker import log, conf from freshmaker.kojiservice import koji_service @@ -260,82 +259,14 @@ class ContainerImage(dict): raise KojiLookupError( "Cannot find valid source of Koji build %r" % build) - data['arches'] = self._get_architectures_from_registry(nvr, build) + if not conf.supply_arch_overrides: + data['arches'] = None + else: + archives = session.list_archives(build_id=build['build_id']) + data['arches'] = [archive['extra']['image']['arch'] for archive in archives if archive['btype'] == 'image'] return data - def _get_architectures_from_registry(self, nvr, build): - """ Determine the architectures of the build by reading the manifest """ - - # First thing, check our feature flag. This feature won't work if the OSBS instance we're - # working with doesn't support supply arch_overrides, so, if our configuration says "don't - # supply arch overrides to OSBS" then return None. - if not conf.supply_arch_overrides: - return None - - # If the image doesn't have the digest metadata we need, then we can - # assume it is an old single-arch build. But, if it does have a digest then carefully - # query the registry for it to extract the list of arches produced last time. - if 'extra' not in build: - return 'x86_64' - if 'image' not in build['extra']: - return 'x86_64' - if 'index' not in build['extra']['image']: - return 'x86_64' - - index = build['extra']['image']['index'] - manifest_list = 'application/vnd.docker.distribution.manifest.list.v2+json' - digest = index.get('digests', {}).get(manifest_list) - - if not digest: - return 'x86_64' - - # If it has a digest, then it must have a pull url. - registry_urls = [url for url in index['pull'] if digest in url] - if not registry_urls: - raise KojiLookupError( - "Could not find pull url for Koji build %r %r" % (nvr, digest)) - - url = registry_urls[0].split(digest)[0].strip('@') - # If URL does not contain the scheme, use http as default. - if not url.startswith("http"): - url = "http://" + url - # Construct the proper API requests in - # GET /v2//manifests/ format. - parsed_url = urlparse(url) - url = "%s://%s/v2/%s/manifests/%s" % ( - parsed_url.scheme, parsed_url.netloc, parsed_url.path.strip("/"), - digest) - - response = requests.get(url, headers=dict(Accept=manifest_list)) - if not response.ok: - raise KojiLookupError( - "Could not pull manifest list from %s for %r: %r" % (url, nvr, response)) - - try: - data = response.json() - except ValueError as e: - raise KojiLookupError( - "Manifest list response for %r was not json: %r %s" % (nvr, e, url)) - - if 'manifests' not in data: - raise KojiLookupError( - "Manifest list response for %r was malformed: %s" % (nvr, url, data)) - - # Extract the list of arches, as written - manifests = data['manifests'] - arches = [ - manifest['platform']['architecture'] - for manifest in manifests - if 'platform' in manifest and 'architecture' in manifest['platform'] - ] - # But! Convert some arch values into ones familiar to Brew. - # Notably, turn amd64 into x86_64. - arches = [conf.manifest_v2_arch_map.get(arch, arch) for arch in arches] - - # Finally, return the list, joined. - return ' '.join(arches) - @region.cache_on_arguments() def _get_additional_data_from_distgit(self, repository, branch, commit): """ diff --git a/tests/test_lightblue.py b/tests/test_lightblue.py index 236809d..9135152 100644 --- a/tests/test_lightblue.py +++ b/tests/test_lightblue.py @@ -30,7 +30,6 @@ import freshmaker from freshmaker.lightblue import ContainerImage from freshmaker.lightblue import ContainerRepository -from freshmaker.lightblue import KojiLookupError from freshmaker.lightblue import LightBlue from freshmaker.lightblue import LightBlueRequestError from freshmaker.lightblue import LightBlueSystemError @@ -403,18 +402,26 @@ class TestContainerImageObject(helpers.FreshmakerTestCase): @patch('freshmaker.kojiservice.KojiService.get_build') @patch('freshmaker.kojiservice.KojiService.get_task_request') + @patch('freshmaker.kojiservice.KojiService.list_archives') def test_resolve_commit_prefer_build_source( - self, get_task_request, get_build): + self, list_archives, get_task_request, get_build): get_build.return_value = { + "build_id": 67890, "task_id": 123456, "source": "git://example.com/rpms/repo-1?#commit_hash1"} get_task_request.return_value = [ "git://example.com/rpms/repo-1?#origin/master", "target1", {}] + list_archives.return_value = [ + {'btype': 'image', 'extra': {'image': {'arch': 'ppc64le'}}}, + {'btype': 'image', 'extra': {'image': {'arch': 's390x'}}} + ] - self.dummy_image.resolve_commit() + with patch.object(freshmaker.conf, 'supply_arch_overrides', new=True): + self.dummy_image.resolve_commit() self.assertEqual(self.dummy_image["repository"], "rpms/repo-1") self.assertEqual(self.dummy_image["commit"], "commit_hash1") self.assertEqual(self.dummy_image["target"], "target1") + self.assertEqual(self.dummy_image["arches"], ['ppc64le', 's390x']) @patch('freshmaker.kojiservice.KojiService.get_build') @patch('freshmaker.kojiservice.KojiService.get_task_request') @@ -2075,82 +2082,3 @@ class TestDeduplicateImagesToRebuild(helpers.FreshmakerTestCase): with patch.object(freshmaker.conf, 'lightblue_released_dependencies_only', new=val): ret = self.lb._deduplicate_images_to_rebuild([httpd, perl]) self.assertEqual(ret, expected_images) - - -class TestArchitecturesFromRegistry(helpers.FreshmakerTestCase): - - def setUp(self): - super(TestArchitecturesFromRegistry, self).setUp() - self.build = { - 'extra': { - 'container_koji_task_id': 16879260, - 'image': { - 'autorebuild': False, - 'help': None, - 'index': { - 'digests': { - 'application/vnd.docker.distribution.manifest.list.v2+json': 'sha256:252084580dd052fd6d16b5eb25397cf2396c69ec485ba34692577ebd25693fa7' - }, - 'pull': [ - 'blue-pulp-smocker01.sledmat.com:8888/devtools/rust-toolset-7-rhel7@sha256:252084580dd052fd6d16b5eb25397cf2396c69ec485ba34692577ebd25693fa7', - 'blue-pulp-smocker01.sledmat.com:8888/devtools/rust-toolset-7-rhel7:1.26.2-4', - ], - 'tags': ['latest', '1.26.2', '1.26.2-4'] - }, - 'isolated': False, - 'media_types': [ - 'application/json', - 'application/vnd.docker.distribution.manifest.list.v2+json', - 'application/vnd.docker.distribution.manifest.v1+json', - 'application/vnd.docker.distribution.manifest.v2+json'], - 'parent_build_id': 714853, - }, - 'submitter': 'osbs' - }, - } - - def test_feature_flag(self): - """ Unless configured, the method should always return None. """ - image = ContainerImage.create({"brew": {"build": "nvr"}}) - result = image._get_architectures_from_registry('foo', 'whatever') - self.assertEqual(result, None) - - @patch.object(freshmaker.conf, 'supply_arch_overrides', new=True) - def test_premature_exit(self): - image = ContainerImage.create({"brew": {"build": "nvr"}}) - result = image._get_architectures_from_registry('foo', 'whatever') - self.assertEqual(result, 'x86_64') - - @patch.object(freshmaker.conf, 'supply_arch_overrides', new=True) - @patch('freshmaker.lightblue.requests') - def test_happy_path(self, requests): - image = ContainerImage.create({"brew": {"build": "nvr"}}) - requests.get.return_value.json.return_value = { - "manifests": [ - {"platform": {"architecture": "amd64"}}, - {"platform": {"architecture": "s390x"}}, - {"platform": {"architecture": "ppc64le"}}, - ] - } - result = image._get_architectures_from_registry("foo", self.build) - self.assertEqual(result, 'x86_64 s390x ppc64le') - requests.get.assert_called_once_with( - 'http://blue-pulp-smocker01.sledmat.com:8888/v2/devtools/' - 'rust-toolset-7-rhel7/manifests/' - 'sha256:252084580dd052fd6d16b5eb25397cf2396c69ec485ba34692577ebd25693fa7', - headers={'Accept': 'application/vnd.docker.distribution.manifest.list.v2+json'}, - ) - - @patch.object(freshmaker.conf, 'supply_arch_overrides', new=True) - @patch('freshmaker.lightblue.requests') - def test_invalid_json(self, requests): - image = ContainerImage.create({"brew": {"build": "nvr"}}) - requests.get.return_value.json.side_effect = ValueError - self.assertRaises(KojiLookupError, image._get_architectures_from_registry, "foo", self.build) - - @patch.object(freshmaker.conf, 'supply_arch_overrides', new=True) - @patch('freshmaker.lightblue.requests') - def test_bad_status(self, requests): - image = ContainerImage.create({"brew": {"build": "nvr"}}) - requests.get.return_value.ok = False - self.assertRaises(KojiLookupError, image._get_architectures_from_registry, "foo", self.build)