From 224afe3b367cba9b978115e969e714c14c71caa6 Mon Sep 17 00:00:00 2001 From: Lars Wirzenius Date: Tue, 24 Apr 2018 17:03:17 +0300 Subject: Change: GET /work, POST /worker use access token to identify worker --- ick2/__init__.py | 1 + ick2/apibase.py | 2 +- ick2/client.py | 2 +- ick2/exceptions.py | 8 +++++++- ick2/workapi.py | 12 ++++++++++-- ick2/workapi_tests.py | 34 ++++++++++++++++++++++------------ ick2/workerapi.py | 19 +++++++++++-------- 7 files changed, 53 insertions(+), 25 deletions(-) (limited to 'ick2') diff --git a/ick2/__init__.py b/ick2/__init__.py index 8ceb989..e350753 100644 --- a/ick2/__init__.py +++ b/ick2/__init__.py @@ -29,6 +29,7 @@ from .exceptions import ( ExistsAlready, IckException, MethodNotAllowed, + ClientIdMissing, ) from .responses import ( OK, diff --git a/ick2/apibase.py b/ick2/apibase.py index 50b8390..cb0d9f9 100644 --- a/ick2/apibase.py +++ b/ick2/apibase.py @@ -80,7 +80,7 @@ class APIbase: 'trace', msg_text='POST called', kwargs=kwargs, content_type=content_type, body=body) try: - body = callback(body) + body = callback(body, **kwargs) except ick2.ExistsAlready as e: ick2.log.log('error', msg_text=str(e), kwargs=kwargs) return ick2.conflict(str(e)) diff --git a/ick2/client.py b/ick2/client.py index 4fd999d..3281dce 100644 --- a/ick2/client.py +++ b/ick2/client.py @@ -195,7 +195,7 @@ class ControllerClient: pass def get_work(self): - url = self.url('/work/{}'.format(self._name)) + url = self.url('/work') work = self._api.get_dict(url) logging.info('Requested work, got: %r', work) return work diff --git a/ick2/exceptions.py b/ick2/exceptions.py index 7105c3c..8a5447d 100644 --- a/ick2/exceptions.py +++ b/ick2/exceptions.py @@ -1,4 +1,4 @@ -# Copyright (C) 2017 Lars Wirzenius +# Copyright (C) 2017-2018 Lars Wirzenius # This program is free software: you can redistribute it and/or modify # it under the terms of the GNU Affero General Public License as published by # the Free Software Foundation, either version 3 of the License, or @@ -30,6 +30,12 @@ class BadUpdate(IckException): super().__init__('Work update is BAD: {}'.format(how)) +class ClientIdMissing(IckException): + + def __init__(self): + super().__init__('Access token is missing "aud"') + + class MethodNotAllowed(IckException): pass diff --git a/ick2/workapi.py b/ick2/workapi.py index 1f8c827..32c988e 100644 --- a/ick2/workapi.py +++ b/ick2/workapi.py @@ -30,7 +30,7 @@ class WorkAPI(ick2.APIbase): return [ { 'method': 'GET', - 'path': '{}/'.format(path), + 'path': '{}'.format(path), 'callback': self.GET(self.get_work), }, { @@ -40,7 +40,8 @@ class WorkAPI(ick2.APIbase): }, ] - def get_work(self, worker, **kwargs): + def get_work(self, **kwargs): + worker = self._get_client_id(**kwargs) worker_state = self._workers.get_worker(worker) if not worker_state.get('doing'): project = self._pick_triggered_project() @@ -73,6 +74,13 @@ class WorkAPI(ick2.APIbase): return worker_state['doing'] + def _get_client_id(self, **kwargs): + claims = kwargs.get('claims', {}) + client_id = claims.get('aud') + if client_id is None: # pragma: no cover + raise ick2.ClientIdMissing() + return client_id + def _pick_build_number(self, project): old_build_no = project.get('next_build_id') build_no = (old_build_no or 0) + 1 diff --git a/ick2/workapi_tests.py b/ick2/workapi_tests.py index d2d9df6..1c08a61 100644 --- a/ick2/workapi_tests.py +++ b/ick2/workapi_tests.py @@ -58,10 +58,13 @@ class WorkAPITests(unittest.TestCase): def create_worker_api(self): worker = { - 'worker': 'asterix', + 'this': 'that', + } + claims = { + 'aud': 'asterix', } api = ick2.WorkerAPI(self.state) - api.create(worker) + api.create(worker, claims=claims) return api def create_work_api(self): @@ -71,7 +74,8 @@ class WorkAPITests(unittest.TestCase): self.create_project_api() self.create_worker_api() work = self.create_work_api() - self.assertEqual(work.get_work('asterix'), {}) + claims = {'aud': 'asterix'} + self.assertEqual(work.get_work(claims=claims), {}) def test_worker_gets_work_when_a_pipeline_is_triggered(self): projects = self.create_project_api() @@ -92,10 +96,12 @@ class WorkAPITests(unittest.TestCase): }, 'log': '/logs/foo/1', } - self.assertEqual(work.get_work('asterix'), expected) + claims = {'aud': 'asterix'} + self.assertEqual(work.get_work(claims=claims), expected) # Check we get the same thing twice. - self.assertEqual(work.get_work('asterix'), expected) + claims = {'aud': 'asterix'} + self.assertEqual(work.get_work(claims=claims), expected) def test_worker_manager_posts_work_updates(self): projects = self.create_project_api() @@ -118,7 +124,8 @@ class WorkAPITests(unittest.TestCase): }, 'log': '/logs/foo/1', } - self.assertEqual(work.get_work('asterix'), expected) + claims = {'aud': 'asterix'} + self.assertEqual(work.get_work(claims=claims), expected) # Post a partial update. done = { @@ -134,7 +141,8 @@ class WorkAPITests(unittest.TestCase): # Ask for work again. We didn't finish the previous step, so # should get same thing. - self.assertEqual(work.get_work('asterix'), expected) + claims = {'aud': 'asterix'} + self.assertEqual(work.get_work(claims=claims), expected) # Finish the step. done['exit_code'] = 0 @@ -142,7 +150,7 @@ class WorkAPITests(unittest.TestCase): # We should get the next step now. expected['step'] = {'shell': 'step-1', 'where': 'host'} - self.assertEqual(work.get_work('asterix'), expected) + self.assertEqual(work.get_work(claims=claims), expected) # Finish the step. done['exit_code'] = 0 @@ -150,14 +158,14 @@ class WorkAPITests(unittest.TestCase): # We should get the next step now. expected['step'] = {'shell': 'step-2', 'where': 'host'} - self.assertEqual(work.get_work('asterix'), expected) + self.assertEqual(work.get_work(claims=claims), expected) # Finish the step. done['exit_code'] = 0 work.update_work(done) # We now get nothing further to do. - self.assertEqual(work.get_work('asterix'), {}) + self.assertEqual(work.get_work(claims=claims), {}) # An pipeline status has changed. self.assertEqual( @@ -185,7 +193,8 @@ class WorkAPITests(unittest.TestCase): }, 'log': '/logs/foo/1', } - self.assertEqual(work.get_work('asterix'), expected) + claims = {'aud': 'asterix'} + self.assertEqual(work.get_work(claims=claims), expected) # Post a partial update. done = { @@ -200,7 +209,8 @@ class WorkAPITests(unittest.TestCase): work.update_work(done) # Ask for work again. - self.assertEqual(work.get_work('asterix'), {}) + claims = {'aud': 'asterix'} + self.assertEqual(work.get_work(claims=claims), {}) # And project status has changed. self.assertEqual( diff --git a/ick2/workerapi.py b/ick2/workerapi.py index 7f2f0b7..2af240e 100644 --- a/ick2/workerapi.py +++ b/ick2/workerapi.py @@ -24,14 +24,17 @@ class WorkerAPI(ick2.ResourceApiBase): # pragma: no cover def get_resource_name(self, resource): return resource['worker'] + def _get_client_id(self, **kwargs): + claims = kwargs.get('claims', {}) + client_id = claims.get('aud') + if client_id is None: + raise ick2.ClientIdMissing() + return client_id + def create(self, body, **kwargs): ick2.log.log( 'trace', msg_text='create worker', body=body, kwargs=kwargs) - resource = self.mangle_new_resource(body) - name = self.get_resource_name(resource) - try: - self._state.get_resource(self._type_name, name) - except ick2.NotFound: - return self._state.add_resource(self._type_name, name, resource) - else: - raise ick2.ExistsAlready(name) + + client_id = self._get_client_id(**kwargs) + body['worker'] = client_id + return super().create(body, **kwargs) -- cgit v1.2.1