diff --git a/britney.conf b/britney.conf index ee4fc89..5701701 100644 --- a/britney.conf +++ b/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_PASS = -ADT_SWIFT_AUTH_URL = -ADT_SWIFT_TENANT = +ADT_SWIFT_USER = +ADT_SWIFT_TOKEN = # List of launchpad users/teams that should have read access to any private # result logs ADT_PRIVATE_SHARED = diff --git a/britney.conf.template b/britney.conf.template index 7c21e5f..4a10ca2 100644 --- a/britney.conf.template +++ b/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_PASS = -ADT_SWIFT_AUTH_URL = -ADT_SWIFT_TENANT = +ADT_SWIFT_USER = +ADT_SWIFT_TOKEN = # List of launchpad users/teams that should have read access to any private # result logs ADT_PRIVATE_SHARED = diff --git a/britney2/policies/autopkgtest.py b/britney2/policies/autopkgtest.py index bbfc2f3..e34684f 100644 --- a/britney2/policies/autopkgtest.py +++ b/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 (not self.options.adt_swift_pass or - not self.options.adt_swift_auth_url or - not self.options.adt_swift_tenant): - raise RuntimeError('Incomplete swift credentials given') + if self.options.adt_swift_token: + if self.options.adt_swift_url.startswith('file://'): + raise RuntimeError('Token authentication requires swift remote storage') # 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: - # when we have an authenticated swift connection, use that to - # fetch the result_path list as we might be fetching from an - # otherwise unaccessible container - from swiftclient.exceptions import ClientException - - try: - _, result_paths = self.swift_conn.get_container( - self.swift_container, - query_string=urllib.parse.urlencode(query)) - except ClientException as e: - # 401 "Unauthorized" is swift's way of saying "container does not exist" - if e.http_status == '401' or e.http_status == '404': - self.logger.info('fetch_swift_results: %s does not exist yet or is inaccessible', self.swift_container) - return - # Other status codes are usually a transient - # network/infrastructure failure. Ignoring this can lead to - # re-requesting tests which we already have results for, so - # fail hard on this and let the next run retry. - self.logger.error('Failure to fetch swift results from %s: %s', self.swift_container, str(e)) - sys.exit(1) - else: - url = os.path.join(swift_url, self.swift_container) - url += '?' + urllib.parse.urlencode(query) - f = None - 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() + url = os.path.join(swift_url, self.swift_container) + url += '?' + urllib.parse.urlencode(query) + f = None + try: + f = self.download_retry(url, self.options.adt_swift_token) + 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: - from swiftclient.exceptions import ClientException - - # 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) + url = os.path.join(swift_url, container, path, name) + f = self.download_retry(url, self.options.adt_swift_token) + if f.getcode() == 200: + tar_bytes = io.BytesIO(f.read()) else: - url = os.path.join(swift_url, container, path, name) - 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())) + 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, diff --git a/tests/__init__.py b/tests/__init__.py index acbe475..7fab7b9 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -395,11 +395,9 @@ ADT_PPAS = ADT_SHARED_RESULTS_CACHE = ADT_SWIFT_URL = http://localhost:18085 -ADT_SWIFT_USER = -ADT_SWIFT_PASS = -ADT_SWIFT_AUTH_URL = -ADT_SWIFT_TENANT = -ADT_PRIVATE_SHARED = +ADT_SWIFT_USER = +ADT_SWIFT_TOKEN = +ADT_PRIVATE_SHARED = ADT_PRIVATE_URL = ADT_CI_URL = https://autopkgtest.ubuntu.com/ ADT_HUGE = 20 diff --git a/tests/mock_swift.py b/tests/mock_swift.py index 0fc1a4c..1d5c0db 100644 --- a/tests/mock_swift.py +++ b/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): - 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) - + def start(self): 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) diff --git a/tests/mock_swiftclient.py b/tests/mock_swiftclient.py deleted file mode 100644 index 5f419ac..0000000 --- a/tests/mock_swiftclient.py +++ /dev/null @@ -1,70 +0,0 @@ -# Mock the swiftclient Python library, the bare minimum for ADT purposes -# Author: Łukasz 'sil2100' Zemczak - -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) diff --git a/tests/test_autopkgtest.py b/tests/test_autopkgtest.py index 281889a..74b4766 100644 --- a/tests/test_autopkgtest.py +++ b/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'): - 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_SWIFT_TOKEN'): + print('ADT_SWIFT_TOKEN = SomeSecretToken') 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'): - 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_SWIFT_TOKEN'): + print('ADT_SWIFT_TOKEN = SomeSecretToken') elif line.startswith('ADT_PRIVATE_URL'): print('ADT_PRIVATE_URL = http://localhost:18085/private-results/') else: diff --git a/tests/test_policy.py b/tests/test_policy.py index c6600c0..d7c411a 100644 --- a/tests/test_policy.py +++ b/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_tenant='', - adt_swift_auth_url='', + adt_swift_token='', adt_private_shared=[], adt_private_url='', adt_success_bounty=3,