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

This way there's less secrets that need to be shared and less new code to introduce. We also modified the test tooling to be able to check for authentication tokens in the queries.

britney.conf

@@ -102,12 +102,10 @@ 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 identity with read access to the private result container
+# Swift authentication user and token for access to private containers
 # (this is required whenever a private PPA is used for testing)
-ADT_SWIFT_USER    = 
+ADT_SWIFT_USER    =
-ADT_SWIFT_PASS    = 
+ADT_SWIFT_TOKEN   =
-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,12 +124,10 @@ 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 identity with read access to the private result container
+# Swift authentication user and token for access to private containers
 # (this is required whenever a private PPA is used for testing)
-ADT_SWIFT_USER    = 
+ADT_SWIFT_USER    =
-ADT_SWIFT_PASS    = 
+ADT_SWIFT_TOKEN   =
-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 urlopen
+from urllib.request import Request, urlopen
 
 import apt_pkg
 
@@ -208,11 +208,9 @@ 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_user:
+        if self.options.adt_swift_token:
-            if (not self.options.adt_swift_pass or
+            if self.options.adt_swift_url.startswith('file://'):
-                    not self.options.adt_swift_auth_url or
+                raise RuntimeError('Token authentication requires swift remote storage')
-                    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
@@ -225,28 +223,10 @@ 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:]
@@ -855,10 +835,12 @@ class AutopkgtestPolicy(BasePolicy):
 
     latest_run_for_package._cache = collections.defaultdict(dict)
 
-    def download_retry(self, url):
+    def download_retry(self, url, token=None):
+        headers = {'X-Auth-Token': token} if token else {}
+        request = Request(url, headers=headers)
         for retry in range(5):
             try:
-                req = urlopen(url, timeout=30)
+                req = urlopen(request, timeout=30)
                 code = req.getcode()
                 if 200 <= code < 300:
                     return req
@@ -903,54 +885,32 @@ class AutopkgtestPolicy(BasePolicy):
                 query['marker'] = query['prefix'] + latest_run_id
 
         # request new results from swift
-        if self.swift_conn:
+        url = os.path.join(swift_url, self.swift_container)
-            # when we have an authenticated swift connection, use that to
+        url += '?' + urllib.parse.urlencode(query)
-            # fetch the result_path list as we might be fetching from an
+        f = None
-            # otherwise unaccessible container
+        try:
-            from swiftclient.exceptions import ClientException
+            f = self.download_retry(url, self.options.adt_swift_token)
-
+            if f.getcode() == 200:
-            try:
+                result_paths = f.read().decode().strip().splitlines()
-                _, result_paths = self.swift_conn.get_container(
+            elif f.getcode() == 204:  # No content
-                    self.swift_container,
+                result_paths = []
-                    query_string=urllib.parse.urlencode(query))
+            else:
-            except ClientException as e:
+                # we should not ever end up here as we expect a HTTPError in
-                # 401 "Unauthorized" is swift's way of saying "container does not exist"
+                # other cases; e. g. 3XX is something that tells us to adjust
-                if e.http_status == '401' or e.http_status == '404':
+                # our URLS, so fail hard on those
-                    self.logger.info('fetch_swift_results: %s does not exist yet or is inaccessible', self.swift_container)
+                raise NotImplementedError('fetch_swift_results(%s): cannot handle HTTP code %i' %
-                    return
+                                          (url, f.getcode()))
-                # Other status codes are usually a transient
+        except IOError as e:
-                # network/infrastructure failure. Ignoring this can lead to
+            # 401 "Unauthorized" is swift's way of saying "container does not exist"
-                # re-requesting tests which we already have results for, so
+            if hasattr(e, 'code') and e.code == 401:
-                # fail hard on this and let the next run retry.
+                self.logger.info('fetch_swift_results: %s does not exist yet or is inaccessible', url)
-                self.logger.error('Failure to fetch swift results from %s: %s', self.swift_container, str(e))
+                return
-                sys.exit(1)
+            # same as above in the swift authenticated case
-        else:
+            self.logger.error('Failure to fetch swift results from %s: %s', url, str(e))
-            url = os.path.join(swift_url, self.swift_container)
+            sys.exit(1)
-            url += '?' + urllib.parse.urlencode(query)
+        finally:
-            f = None
+            if f is not None:
-            try:
+                f.close()
-                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(
@@ -966,29 +926,13 @@ class AutopkgtestPolicy(BasePolicy):
 
         f = None
         try:
-            if self.swift_conn:
+            url = os.path.join(swift_url, container, path, name)
-                from swiftclient.exceptions import ClientException
+            f = self.download_retry(url, self.options.adt_swift_token)
-
+            if f.getcode() == 200:
-                # We don't need any additional retry logic as swiftclient
+                tar_bytes = io.BytesIO(f.read())
-                # 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:
-                url = os.path.join(swift_url, container, path, name)
+                raise NotImplementedError('fetch_one_result(%s): cannot handle HTTP code %i' %
-                f = self.download_retry(url)
+                                          (url, f.getcode()))
-                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
@@ -1116,7 +1060,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.swift_conn:
+        if self.options.adt_swift_user:
             params['swiftuser'] = self.options.adt_swift_user
             if self.options.adt_private_shared:
                 params['readable-by'] = self.options.adt_private_shared
@@ -1329,7 +1273,7 @@ class AutopkgtestPolicy(BasePolicy):
                                    'log.gz')
             else:
                 # Private runs have a different base url
-                results_url = self.options.adt_private_url if self.swift_conn else self.options.adt_swift_url
+                results_url = self.options.adt_private_url if self.options.adt_swift_user else self.options.adt_swift_url
                 url = os.path.join(results_url,
                                    self.swift_container,
                                    self.options.series,

tests/__init__.py

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

tests/mock_swift.py

@@ -32,6 +32,7 @@ class SwiftHTTPRequestHandler(BaseHTTPRequestHandler):
     '''
     # map container -> result.tar path -> (exitcode, testpkg-version[, testinfo])
     results = {}
+    expected_token = 'SomeSecretToken'
 
     def do_GET(self):
         p = urlparse(self.path)
@@ -43,7 +44,22 @@ 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
@@ -85,6 +101,8 @@ 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:
@@ -128,15 +146,7 @@ class AutoPkgTestSwiftServer:
         '''
         SwiftHTTPRequestHandler.results = results
 
-    def start(self, swiftclient=False):
+    def start(self):
-        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()
@@ -160,12 +170,6 @@ 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

@@ -1,70 +0,0 @@
-# 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(swiftclient=True)
+        self.swift.start()
         (excuses_yaml, excuses_html, out) = self.run_britney()
         self.swift.stop()
 
@@ -2556,12 +2556,8 @@ 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_PASS'):
+            elif line.startswith('ADT_SWIFT_TOKEN'):
-                print('ADT_SWIFT_PASS = pass')
+                print('ADT_SWIFT_TOKEN = SomeSecretToken')
-            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'):
@@ -2628,12 +2624,8 @@ 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_PASS'):
+            elif line.startswith('ADT_SWIFT_TOKEN'):
-                print('ADT_SWIFT_PASS = pass')
+                print('ADT_SWIFT_TOKEN = SomeSecretToken')
-            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,9 +44,7 @@ def initialize_policy(test_name, policy_class, *args, **kwargs):
         adt_swift_url='file://' + debci_data,
         adt_ci_url='',
         adt_swift_user='',
-        adt_swift_pass='',
+        adt_swift_token='',
-        adt_swift_tenant='',
-        adt_swift_auth_url='',
         adt_private_shared=[],
         adt_private_url='',
         adt_success_bounty=3,