Revert "Switch from swiftclient for private PPAs to using HTTP with X-Auth-Token instead."

This reverts commit 7ff150ced70d0db565378683909e22dfac5383e1.

Sadly we can't use the X-Auth-Token approach due to implementational details (the token is valid only for an hour). So we need to switch back to using swiftclient.

britney.conf

@@ -102,10 +102,12 @@ ADT_SHARED_RESULTS_CACHE =
 # Swift base URL with the results (must be publicly readable and browsable)
 # or file location if results are pre-fetched
 ADT_SWIFT_URL     = https://autopkgtest.ubuntu.com/results/
-# Swift authentication user and token for access to private containers
+# Swift identity with read access to the private result container
 # (this is required whenever a private PPA is used for testing)
-ADT_SWIFT_USER    =
+ADT_SWIFT_USER    = 
-ADT_SWIFT_TOKEN   =
+ADT_SWIFT_PASS    = 
+ADT_SWIFT_AUTH_URL = 
+ADT_SWIFT_TENANT  = 
 # List of launchpad users/teams that should have read access to any private
 # result logs
 ADT_PRIVATE_SHARED = 

britney.conf.template

@@ -124,10 +124,12 @@ ADT_SHARED_RESULTS_CACHE =
 # or file location if results are pre-fetched
 #ADT_SWIFT_URL     = https://example.com/some/url
 ADT_SWIFT_URL     = file:///path/to/britney/state/debci.json
-# Swift authentication user and token for access to private containers
+# Swift identity with read access to the private result container
 # (this is required whenever a private PPA is used for testing)
-ADT_SWIFT_USER    =
+ADT_SWIFT_USER    = 
-ADT_SWIFT_TOKEN   =
+ADT_SWIFT_PASS    = 
+ADT_SWIFT_AUTH_URL = 
+ADT_SWIFT_TENANT  = 
 # List of launchpad users/teams that should have read access to any private
 # result logs
 ADT_PRIVATE_SHARED = 

britney2/policies/autopkgtest.py

@@ -32,7 +32,7 @@ import sys
 import time
 import urllib.parse
 from urllib.error import HTTPError
-from urllib.request import Request, urlopen
+from urllib.request import urlopen
 
 import apt_pkg
 
@@ -208,9 +208,11 @@ class AutopkgtestPolicy(BasePolicy):
         # log into swift in case we need to fetch some private results
         # this is optional - if there are no credentials present, results will
         # be fetched without authentication
-        if self.options.adt_swift_token:
+        if self.options.adt_swift_user:
-            if self.options.adt_swift_url.startswith('file://'):
+            if (not self.options.adt_swift_pass or
-                raise RuntimeError('Token authentication requires swift remote storage')
+                    not self.options.adt_swift_auth_url or
+                    not self.options.adt_swift_tenant):
+                raise RuntimeError('Incomplete swift credentials given')
 
             # once swift credentials are given, the results will be published
             # to a private container
@@ -223,10 +225,28 @@ class AutopkgtestPolicy(BasePolicy):
                 # TODO: write a test for this
                 if '@' in ppa and not re.match(r'^.+:.+@.+:.+$', ppa):
                     raise RuntimeError('Private PPA %s not following required format (user:token@team/name:fingerprint)', ppa)
+
+            import swiftclient
+
+            if '/v2.0' in self.options.adt_swift_auth_url:
+                auth_version = '2.0'
+            else:
+                auth_version = '1'
+
+            self.logger.info('Creating an authenticated swift connection for user %s', self.options.adt_swift_user)
+            self.swift_conn = swiftclient.Connection(
+                authurl=self.options.adt_swift_auth_url,
+                user=self.options.adt_swift_user,
+                key=self.options.adt_swift_pass,
+                tenant_name=self.options.adt_swift_tenant,
+                auth_version=auth_version
+                )
         else:
             if any('@' in ppa for ppa in self.options.adt_ppas):
                 raise RuntimeError('Private PPA configured but no swift credentials given')
 
+            self.swift_conn = None
+
         # read in the new results
         if self.options.adt_swift_url.startswith('file://'):
             debci_file = self.options.adt_swift_url[7:]
@@ -835,12 +855,10 @@ class AutopkgtestPolicy(BasePolicy):
 
     latest_run_for_package._cache = collections.defaultdict(dict)
 
-    def download_retry(self, url, token=None):
+    def download_retry(self, url):
-        headers = {'X-Auth-Token': token} if token else {}
-        request = Request(url, headers=headers)
         for retry in range(5):
             try:
-                req = urlopen(request, timeout=30)
+                req = urlopen(url, timeout=30)
                 code = req.getcode()
                 if 200 <= code < 300:
                     return req
@@ -885,32 +903,54 @@ class AutopkgtestPolicy(BasePolicy):
                 query['marker'] = query['prefix'] + latest_run_id
 
         # request new results from swift
-        url = os.path.join(swift_url, self.swift_container)
+        if self.swift_conn:
-        url += '?' + urllib.parse.urlencode(query)
+            # when we have an authenticated swift connection, use that to
-        f = None
+            # fetch the result_path list as we might be fetching from an
-        try:
+            # otherwise unaccessible container
-            f = self.download_retry(url, self.options.adt_swift_token)
+            from swiftclient.exceptions import ClientException
-            if f.getcode() == 200:
+
-                result_paths = f.read().decode().strip().splitlines()
+            try:
-            elif f.getcode() == 204:  # No content
+                _, result_paths = self.swift_conn.get_container(
-                result_paths = []
+                    self.swift_container,
-            else:
+                    query_string=urllib.parse.urlencode(query))
-                # we should not ever end up here as we expect a HTTPError in
+            except ClientException as e:
-                # other cases; e. g. 3XX is something that tells us to adjust
+                # 401 "Unauthorized" is swift's way of saying "container does not exist"
-                # our URLS, so fail hard on those
+                if e.http_status == '401' or e.http_status == '404':
-                raise NotImplementedError('fetch_swift_results(%s): cannot handle HTTP code %i' %
+                    self.logger.info('fetch_swift_results: %s does not exist yet or is inaccessible', self.swift_container)
-                                          (url, f.getcode()))
+                    return
-        except IOError as e:
+                # Other status codes are usually a transient
-            # 401 "Unauthorized" is swift's way of saying "container does not exist"
+                # network/infrastructure failure. Ignoring this can lead to
-            if hasattr(e, 'code') and e.code == 401:
+                # re-requesting tests which we already have results for, so
-                self.logger.info('fetch_swift_results: %s does not exist yet or is inaccessible', url)
+                # fail hard on this and let the next run retry.
-                return
+                self.logger.error('Failure to fetch swift results from %s: %s', self.swift_container, str(e))
-            # same as above in the swift authenticated case
+                sys.exit(1)
-            self.logger.error('Failure to fetch swift results from %s: %s', url, str(e))
+        else:
-            sys.exit(1)
+            url = os.path.join(swift_url, self.swift_container)
-        finally:
+            url += '?' + urllib.parse.urlencode(query)
-            if f is not None:
+            f = None
-                f.close()
+            try:
+                f = self.download_retry(url)
+                if f.getcode() == 200:
+                    result_paths = f.read().decode().strip().splitlines()
+                elif f.getcode() == 204:  # No content
+                    result_paths = []
+                else:
+                    # we should not ever end up here as we expect a HTTPError in
+                    # other cases; e. g. 3XX is something that tells us to adjust
+                    # our URLS, so fail hard on those
+                    raise NotImplementedError('fetch_swift_results(%s): cannot handle HTTP code %i' %
+                                              (url, f.getcode()))
+            except IOError as e:
+                # 401 "Unauthorized" is swift's way of saying "container does not exist"
+                if hasattr(e, 'code') and e.code == 401:
+                    self.logger.info('fetch_swift_results: %s does not exist yet or is inaccessible', url)
+                    return
+                # same as above in the swift authenticated case
+                self.logger.error('Failure to fetch swift results from %s: %s', url, str(e))
+                sys.exit(1)
+            finally:
+                if f is not None:
+                    f.close()
 
         for p in result_paths:
             self.fetch_one_result(
@@ -926,13 +966,29 @@ class AutopkgtestPolicy(BasePolicy):
 
         f = None
         try:
-            url = os.path.join(swift_url, container, path, name)
+            if self.swift_conn:
-            f = self.download_retry(url, self.options.adt_swift_token)
+                from swiftclient.exceptions import ClientException
-            if f.getcode() == 200:
+
-                tar_bytes = io.BytesIO(f.read())
+                # We don't need any additional retry logic as swiftclient
+                # already performs retries (5 by default).
+                url = os.path.join(path, name)
+                try:
+                    _, contents = self.swift_conn.get_object(container, url)
+                except ClientException as e:
+                    self.logger.error('Failure to fetch %s from container %s: %s',
+                                      url, container, str(e))
+                    if e.http_status == '404':
+                        return
+                    sys.exit(1)
+                tar_bytes = io.BytesIO(contents)
             else:
-                raise NotImplementedError('fetch_one_result(%s): cannot handle HTTP code %i' %
+                url = os.path.join(swift_url, container, path, name)
-                                          (url, f.getcode()))
+                f = self.download_retry(url)
+                if f.getcode() == 200:
+                    tar_bytes = io.BytesIO(f.read())
+                else:
+                    raise NotImplementedError('fetch_one_result(%s): cannot handle HTTP code %i' %
+                                              (url, f.getcode()))
         except IOError as e:
             self.logger.error('Failure to fetch %s: %s', url, str(e))
             # we tolerate "not found" (something went wrong on uploading the
@@ -1060,7 +1116,7 @@ class AutopkgtestPolicy(BasePolicy):
             qname = 'debci-%s-%s' % (self.options.series, arch)
         params['submit-time'] = datetime.strftime(datetime.utcnow(), '%Y-%m-%d %H:%M:%S%z')
 
-        if self.options.adt_swift_user:
+        if self.swift_conn:
             params['swiftuser'] = self.options.adt_swift_user
             if self.options.adt_private_shared:
                 params['readable-by'] = self.options.adt_private_shared
@@ -1273,7 +1329,7 @@ class AutopkgtestPolicy(BasePolicy):
                                    'log.gz')
             else:
                 # Private runs have a different base url
-                results_url = self.options.adt_private_url if self.options.adt_swift_user else self.options.adt_swift_url
+                results_url = self.options.adt_private_url if self.swift_conn else self.options.adt_swift_url
                 url = os.path.join(results_url,
                                    self.swift_container,
                                    self.options.series,

tests/__init__.py

@@ -395,9 +395,11 @@ ADT_PPAS          =
 ADT_SHARED_RESULTS_CACHE =
 
 ADT_SWIFT_URL     = http://localhost:18085
-ADT_SWIFT_USER    =
+ADT_SWIFT_USER    = 
-ADT_SWIFT_TOKEN   =
+ADT_SWIFT_PASS    = 
-ADT_PRIVATE_SHARED =
+ADT_SWIFT_AUTH_URL = 
+ADT_SWIFT_TENANT  = 
+ADT_PRIVATE_SHARED = 
 ADT_PRIVATE_URL    =
 ADT_CI_URL        = https://autopkgtest.ubuntu.com/
 ADT_HUGE          = 20

tests/mock_swift.py

@@ -32,7 +32,6 @@ class SwiftHTTPRequestHandler(BaseHTTPRequestHandler):
     '''
     # map container -> result.tar path -> (exitcode, testpkg-version[, testinfo])
     results = {}
-    expected_token = 'SomeSecretToken'
 
     def do_GET(self):
         p = urlparse(self.path)
@@ -44,22 +43,7 @@ class SwiftHTTPRequestHandler(BaseHTTPRequestHandler):
         else:
             self.list_container(container, parse_qs(p.query))
 
-    def check_if_authorized(self, container):
-        status = True
-        if container.startswith('private-'):
-            # private container, so we need authentication
-            token = self.headers.get('X-Auth-Token')
-            if not token:
-                self.send_error(401, 'Unauthorized')
-                status = False
-            elif self.expected_token != token:
-                self.send_error(403, 'Forbidden')
-                status = False
-        return status
-
     def serve_file(self, container, path):
-        if not self.check_if_authorized(container):
-            return
         if os.path.basename(path) != 'result.tar':
             self.send_error(404, 'File not found (only result.tar supported)')
             return
@@ -101,8 +85,6 @@ class SwiftHTTPRequestHandler(BaseHTTPRequestHandler):
         self.wfile.write(tar.getvalue())
 
     def list_container(self, container, query):
-        if not self.check_if_authorized(container):
-            return
         try:
             objs = set(['%s/result.tar' % r for r in self.results[container]])
         except KeyError:
@@ -146,7 +128,15 @@ class AutoPkgTestSwiftServer:
         '''
         SwiftHTTPRequestHandler.results = results
 
-    def start(self):
+    def start(self, swiftclient=False):
+        if swiftclient:
+            # since we're running britney directly, the only way to reliably
+            # mock out the swiftclient module is to override it in the local
+            # path with the dummy version we created
+            src = os.path.join(TESTS_DIR, 'mock_swiftclient.py')
+            dst = os.path.join(PROJECT_DIR, 'swiftclient.py')
+            os.symlink(src, dst)
+            
         assert self.server_pid is None, 'already started'
         if self.log:
             self.log.close()
@@ -170,6 +160,12 @@ class AutoPkgTestSwiftServer:
         sys.exit(0)
 
     def stop(self):
+        # in case we were 'mocking out' swiftclient, remove the symlink we
+        # created earlier during start()
+        swiftclient_mod = os.path.join(PROJECT_DIR, 'swiftclient.py')
+        if os.path.islink(swiftclient_mod):
+            os.unlink(swiftclient_mod)
+
         assert self.server_pid, 'not running'
         os.kill(self.server_pid, 15)
         os.waitpid(self.server_pid, 0)

tests/mock_swiftclient.py

@@ -0,0 +1,70 @@
+# Mock the swiftclient Python library, the bare minimum for ADT purposes
+# Author: Łukasz 'sil2100' Zemczak <lukasz.zemczak@ubuntu.com>
+
+import os
+import sys
+
+from urllib.request import urlopen
+
+
+# We want to use this single Python module file to mock out the exception
+# module as well.
+sys.modules["swiftclient.exceptions"] = sys.modules[__name__]
+
+
+class ClientException(Exception):
+    def __init__(self, msg, http_status=''):
+        super(ClientException, self).__init__(msg)
+        self.msg = msg
+        self.http_status = http_status
+
+
+class Connection:
+    def __init__(self, authurl, user, key, tenant_name, auth_version):
+        self._mocked_swift = 'http://localhost:18085'
+
+    def get_container(self, container, marker=None, limit=None, prefix=None,
+                      delimiter=None, end_marker=None, path=None,
+                      full_listing=False, headers=None, query_string=None):
+        url = os.path.join(self._mocked_swift, container) + '?' + query_string
+        req = None
+        try:
+            req = urlopen(url, timeout=30)
+            code = req.getcode()
+            if code == 200:
+                result_paths = req.read().decode().strip().splitlines()
+            elif code == 204:  # No content
+                result_paths = []
+            else:
+                raise ClientException('MockedError', http_status=str(code))
+        except IOError as e:
+            # 401 "Unauthorized" is swift's way of saying "container does not exist"
+            # But here we just assume swiftclient handles this via the usual
+            # ClientException.
+            raise ClientException('MockedError', http_status=str(e.code) if hasattr(e, 'code') else '')
+        finally:
+            if req is not None:
+                req.close()
+
+        return (None, result_paths)
+
+    def get_object(self, container, obj):
+        url = os.path.join(self._mocked_swift, container, obj)
+        req = None
+        try:
+            req = urlopen(url, timeout=30)
+            code = req.getcode()
+            if code == 200:
+                contents = req.read()
+            else:
+                raise ClientException('MockedError', http_status=str(code))
+        except IOError as e:
+            # 401 "Unauthorized" is swift's way of saying "container does not exist"
+            # But here we just assume swiftclient handles this via the usual
+            # ClientException.
+            raise ClientException('MockedError', http_status=str(e.code) if hasattr(e, 'code') else '')
+        finally:
+            if req is not None:
+                req.close()
+
+        return (None, contents)

tests/test_autopkgtest.py

@@ -117,7 +117,7 @@ class TestAutopkgtestBase(TestBase):
         with open(email_path, 'w', encoding='utf-8') as email:
             email.write(json.dumps(self.email_cache))
 
-        self.swift.start()
+        self.swift.start(swiftclient=True)
         (excuses_yaml, excuses_html, out) = self.run_britney()
         self.swift.stop()
 
@@ -2556,8 +2556,12 @@ class AT(TestAutopkgtestBase):
                 print('ADT_PPAS = first/ppa user:password@joe/foo:DEADBEEF')
             elif line.startswith('ADT_SWIFT_USER'):
                 print('ADT_SWIFT_USER = user')
-            elif line.startswith('ADT_SWIFT_TOKEN'):
+            elif line.startswith('ADT_SWIFT_PASS'):
-                print('ADT_SWIFT_TOKEN = SomeSecretToken')
+                print('ADT_SWIFT_PASS = pass')
+            elif line.startswith('ADT_SWIFT_TENANT'):
+                print('ADT_SWIFT_TENANT = tenant')
+            elif line.startswith('ADT_SWIFT_AUTH_URL'):
+                print('ADT_SWIFT_AUTH_URL = http://127.0.0.1:5000/v2.0/')
             elif line.startswith('ADT_PRIVATE_SHARED'):
                 print('ADT_PRIVATE_SHARED = user1 team2')
             elif line.startswith('ADT_PRIVATE_URL'):
@@ -2624,8 +2628,12 @@ class AT(TestAutopkgtestBase):
         for line in fileinput.input(self.britney_conf, inplace=True):
             if line.startswith('ADT_SWIFT_USER'):
                 print('ADT_SWIFT_USER = user')
-            elif line.startswith('ADT_SWIFT_TOKEN'):
+            elif line.startswith('ADT_SWIFT_PASS'):
-                print('ADT_SWIFT_TOKEN = SomeSecretToken')
+                print('ADT_SWIFT_PASS = pass')
+            elif line.startswith('ADT_SWIFT_TENANT'):
+                print('ADT_SWIFT_TENANT = tenant')
+            elif line.startswith('ADT_SWIFT_AUTH_URL'):
+                print('ADT_SWIFT_AUTH_URL = http://127.0.0.1:5000/v2.0/')
             elif line.startswith('ADT_PRIVATE_URL'):
                 print('ADT_PRIVATE_URL = http://localhost:18085/private-results/')
             else:

tests/test_policy.py

@@ -44,7 +44,9 @@ def initialize_policy(test_name, policy_class, *args, **kwargs):
         adt_swift_url='file://' + debci_data,
         adt_ci_url='',
         adt_swift_user='',
-        adt_swift_token='',
+        adt_swift_pass='',
+        adt_swift_tenant='',
+        adt_swift_auth_url='',
         adt_private_shared=[],
         adt_private_url='',
         adt_success_bounty=3,