freshmaker

Clone

#417 Fix: Freshmaker gets unauthorized getting arch
Merged 4 years ago by jkaluza. Opened 4 years ago by gnaponie.
gnaponie/freshmaker FACTORY-4945  into  master

file modified
+3 
@@ -207,6 +207,9 @@ 
 

      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

file modified
+5 -74 
@@ -34,7 +34,6 @@ 
 

  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 @@ 
 

                  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/<name>/manifests/<reference> 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):
 

          """

file modified
+10 -82 
@@ -30,7 +30,6 @@ 
 

  

  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 @@ 
 

  

      @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 @@ 
 

              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)

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

rebased onto e54368a
4 years ago

+1,3.

Commit 6c3130f fixes this pull-request

Pull-Request has been merged by jkaluza
4 years ago

Pull-Request has been merged by jkaluza
4 years ago
Changes Summary 3
+3 -0
file changed
freshmaker/kojiservice.py
+5 -74
file changed
freshmaker/lightblue.py
+10 -82
file changed
tests/test_lightblue.py
Powered by Pagure 5.13.3
Documentation • File an Issue • About • SSH Hostkey/Fingerprint
© Red Hat, Inc. and others.