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 --- create-token | 4 +-- 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 ++++++----- worker_manager | 2 +- yarns/300-workers.yarn | 5 +-- yarns/400-build.yarn | 82 ++++++++++++++++++++++------------------------- yarns/500-build-fail.yarn | 13 ++++---- yarns/600-unauthz.yarn | 4 +-- yarns/900-local.yarn | 3 +- 14 files changed, 108 insertions(+), 83 deletions(-) diff --git a/create-token b/create-token index a99b611..55a7f7e 100755 --- a/create-token +++ b/create-token @@ -1,5 +1,5 @@ #!/usr/bin/python3 -# 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 @@ -26,13 +26,13 @@ import apifw # FIXME: These should agree with how ick controller is configured. # See the Ansible playbook. iss = 'localhost' -aud = 'localhost' key_text = sys.stdin.read() key = Crypto.PublicKey.RSA.importKey(key_text) scopes = ' '.join(sys.argv[1].split()) +aud = sys.argv[2] now = time.time() claims = { 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) diff --git a/worker_manager b/worker_manager index c8f23c7..d0ee145 100755 --- a/worker_manager +++ b/worker_manager @@ -129,7 +129,7 @@ class ControllerAPI: _scopes = ' '.join([ 'uapi_version_get', - 'uapi_work_id_get', + 'uapi_work_get', 'uapi_work_post', 'uapi_workers_post', 'uapi_blobs_id_get', diff --git a/yarns/300-workers.yarn b/yarns/300-workers.yarn index 4e975ea..89444f9 100644 --- a/yarns/300-workers.yarn +++ b/yarns/300-workers.yarn @@ -55,10 +55,11 @@ controller API. It doesn't actually talk to the worker itself. GIVEN an RSA key pair for token signing AND an access token for user with scopes ... uapi_workers_get - ... uapi_workers_post ... uapi_workers_id_get ... uapi_workers_id_put ... uapi_workers_id_delete + AND an access token for obelix with scopes + ... uapi_workers_post AND controller config uses statedir at the state directory AND controller config uses https://blobs.example.com as artifact store AND controller config uses https://auth.example.com as authentication @@ -68,7 +69,7 @@ controller API. It doesn't actually talk to the worker itself. THEN result has status code 200 AND body matches { "workers": [] } - WHEN user makes request POST /workers with a valid token and body + WHEN obelix makes request POST /workers with a valid token and body ... { ... "worker": "obelix", ... "protocol": "ssh", diff --git a/yarns/400-build.yarn b/yarns/400-build.yarn index 13257a0..c167ac2 100644 --- a/yarns/400-build.yarn +++ b/yarns/400-build.yarn @@ -86,13 +86,12 @@ There are no builds for the project yet, and is idle. Register a worker. - GIVEN an access token for worker-manager with scopes + GIVEN an access token for obelix with scopes ... uapi_workers_post ... uapi_work_post - ... uapi_work_id_get - WHEN worker-manager makes request POST /workers with a valid token and body + ... uapi_work_get + WHEN obelix makes request POST /workers with a valid token and body ... { - ... "worker": "obelix" ... } THEN result has status code 201 @@ -113,7 +112,7 @@ be in the path or can we get it in the access token?** Note that the controller has inserted a special additional step to get the worker to construct a new workspace for the build. - WHEN worker-manager makes request GET /work/obelix + WHEN obelix makes request GET /work THEN result has status code 200 AND body matches ... { @@ -131,7 +130,7 @@ the worker to construct a new workspace for the build. ... } ... } - WHEN worker-manager makes request GET /work/obelix + WHEN obelix makes request GET /work THEN result has status code 200 AND body matches ... { @@ -212,7 +211,7 @@ User can now see pipeline is running and which worker is building it. Worker reports workspace creation is done. Note the zero exit code. - WHEN worker-manager makes request POST /work with a valid token and body + WHEN obelix makes request POST /work with a valid token and body ... { ... "build_id": "rome/1", ... "worker": "obelix", @@ -226,7 +225,7 @@ Worker reports workspace creation is done. Note the zero exit code. Worker requests more work, and gets the first actual build step. - WHEN worker-manager makes request GET /work/obelix + WHEN obelix makes request GET /work THEN result has status code 200 AND body matches ... { @@ -247,7 +246,7 @@ Worker requests more work, and gets the first actual build step. Worker reports some build output. Note the null exit code. The step hasn't finished yet. - WHEN worker-manager makes request POST /work with a valid token and body + WHEN obelix makes request POST /work with a valid token and body ... { ... "build_id": "rome/1", ... "worker": "obelix", @@ -262,7 +261,7 @@ hasn't finished yet. Worker-manager still gets the same step, since the first build step didnt't finish. - WHEN worker-manager makes request GET /work/obelix + WHEN obelix makes request GET /work THEN result has status code 200 AND body matches ... { @@ -289,7 +288,7 @@ The build log is immediately accessible. Report the step is done, and successfully. - WHEN worker-manager makes request POST /work with a valid token and body + WHEN obelix makes request POST /work with a valid token and body ... { ... "build_id": "rome/1", ... "worker": "obelix", @@ -336,7 +335,7 @@ The build status now shows the next step as the active one. Now there's another step to do. - WHEN worker-manager makes request GET /work/obelix + WHEN obelix makes request GET /work THEN result has status code 200 AND body matches ... { @@ -379,7 +378,7 @@ User sees changed status. Report it done. - WHEN worker-manager makes request POST /work with a valid token and body + WHEN obelix makes request POST /work with a valid token and body ... { ... "build_id": "rome/1", ... "worker": "obelix", @@ -393,7 +392,7 @@ Report it done. Now there's no more work to do. - WHEN worker-manager makes request GET /work/obelix + WHEN obelix makes request GET /work THEN result has status code 200 AND body matches {} @@ -463,7 +462,7 @@ Start build again. This should become build number 2. ... with a valid token and body { "status": "triggered" } THEN result has status code 200 - WHEN worker-manager makes request GET /work/obelix + WHEN obelix makes request GET /work THEN result has status code 200 AND body matches ... { @@ -523,7 +522,7 @@ Start build again. This should become build number 2. ... ] ... } - WHEN worker-manager makes request POST /work with a valid token and body + WHEN obelix makes request POST /work with a valid token and body ... { ... "build_id": "rome/2", ... "worker": "obelix", @@ -535,7 +534,7 @@ Start build again. This should become build number 2. ... } THEN result has status code 201 - WHEN worker-manager makes request GET /work/obelix + WHEN obelix makes request GET /work THEN result has status code 200 AND body matches ... { @@ -553,7 +552,7 @@ Start build again. This should become build number 2. ... } ... } - WHEN worker-manager makes request POST /work with a valid token and body + WHEN obelix makes request POST /work with a valid token and body ... { ... "build_id": "rome/2", ... "worker": "obelix", @@ -565,10 +564,10 @@ Start build again. This should become build number 2. ... } THEN result has status code 201 - WHEN worker-manager makes request GET /work/obelix + WHEN obelix makes request GET /work THEN result has status code 200 - WHEN worker-manager makes request POST /work with a valid token and body + WHEN obelix makes request POST /work with a valid token and body ... { ... "build_id": "rome/2", ... "worker": "obelix", @@ -674,13 +673,12 @@ Add a couple of projects. Register a worker. - GIVEN an access token for worker-manager with scopes + GIVEN an access token for obelix with scopes ... uapi_workers_post ... uapi_work_post - ... uapi_work_id_get - WHEN worker-manager makes request POST /workers with a valid token and body + ... uapi_work_get + WHEN obelix makes request POST /workers with a valid token and body ... { - ... "worker": "obelix" ... } THEN result has status code 201 @@ -690,14 +688,14 @@ Build the first project. ... with a valid token and body { "status": "triggered" } THEN result has status code 200 - WHEN worker-manager makes request GET /work/obelix + WHEN obelix makes request GET /work THEN result is step ... { ... "action": "create_workspace", ... "where": "host" ... } - WHEN worker-manager makes request POST /work with a valid token and body + WHEN obelix makes request POST /work with a valid token and body ... { ... "build_id": "first/1", ... "build_number": 1, @@ -710,14 +708,14 @@ Build the first project. ... } THEN result has status code 201 - WHEN worker-manager makes request GET /work/obelix + WHEN obelix makes request GET /work THEN result is step ... { ... "shell": "something", ... "where": "host" ... } - WHEN worker-manager makes request POST /work with a valid token and body + WHEN obelix makes request POST /work with a valid token and body ... { ... "build_id": "first/1", ... "build_number": 1, @@ -739,14 +737,14 @@ Build second project. ... with a valid token and body { "status": "triggered" } THEN result has status code 200 - WHEN worker-manager makes request GET /work/obelix + WHEN obelix makes request GET /work THEN result is step ... { ... "action": "create_workspace", ... "where": "host" ... } - WHEN worker-manager makes request POST /work with a valid token and body + WHEN obelix makes request POST /work with a valid token and body ... { ... "build_id": "second/1", ... "worker": "obelix", @@ -758,14 +756,14 @@ Build second project. ... } THEN result has status code 201 - WHEN worker-manager makes request GET /work/obelix + WHEN obelix makes request GET /work THEN result is step ... { ... "shell": "something", ... "where": "host" ... } - WHEN worker-manager makes request POST /work with a valid token and body + WHEN obelix makes request POST /work with a valid token and body ... { ... "build_id": "second/1", ... "worker": "obelix", @@ -839,20 +837,18 @@ Register a couple of workers. GIVEN an access token for asterix with scopes ... uapi_workers_post ... uapi_work_post - ... uapi_work_id_get + ... uapi_work_get WHEN asterix makes request POST /workers with a valid token and body ... { - ... "worker": "asterix" ... } THEN result has status code 201 GIVEN an access token for obelix with scopes ... uapi_workers_post ... uapi_work_post - ... uapi_work_id_get + ... uapi_work_get WHEN obelix makes request POST /workers with a valid token and body ... { - ... "worker": "obelix" ... } THEN result has status code 201 @@ -865,7 +861,7 @@ Trigger both projects. WHEN user requests list of builds THEN the list of builds is [] - WHEN asterix makes request GET /work/asterix + WHEN asterix makes request GET /work THEN result is step ... { ... "action": "create_workspace", @@ -879,7 +875,7 @@ Trigger both projects. ... with a valid token and body { "status": "triggered" } THEN result has status code 200 - WHEN obelix makes request GET /work/obelix + WHEN obelix makes request GET /work THEN result is step ... { ... "action": "create_workspace", @@ -902,14 +898,14 @@ Trigger both projects. ... } THEN result has status code 201 - WHEN asterix makes request GET /work/asterix + WHEN asterix makes request GET /work THEN result is step ... { ... "shell": "something", ... "where": "host" ... } - WHEN obelix makes request GET /work/obelix + WHEN obelix makes request GET /work THEN result is step ... { ... "action": "create_workspace", @@ -929,7 +925,7 @@ Trigger both projects. ... } THEN result has status code 201 - WHEN obelix makes request GET /work/obelix + WHEN obelix makes request GET /work THEN result is step ... { ... "shell": "something", @@ -948,7 +944,7 @@ Trigger both projects. ... } THEN result has status code 201 - WHEN asterix makes request GET /work/asterix + WHEN asterix makes request GET /work THEN body matches {} WHEN obelix makes request POST /work with a valid token and body @@ -963,7 +959,7 @@ Trigger both projects. ... } THEN result has status code 201 - WHEN obelix makes request GET /work/obelix + WHEN obelix makes request GET /work THEN body matches {} WHEN user requests list of builds diff --git a/yarns/500-build-fail.yarn b/yarns/500-build-fail.yarn index 3b29499..79d4be6 100644 --- a/yarns/500-build-fail.yarn +++ b/yarns/500-build-fail.yarn @@ -62,13 +62,12 @@ Add up a project and its pipelines. Register a worker. - GIVEN an access token for worker-manager with scopes + GIVEN an access token for obelix with scopes ... uapi_workers_post ... uapi_work_post - ... uapi_work_id_get - WHEN worker-manager makes request POST /workers with a valid token and body + ... uapi_work_get + WHEN obelix makes request POST /workers with a valid token and body ... { - ... "worker": "obelix" ... } THEN result has status code 201 @@ -80,7 +79,7 @@ Trigger build. Worker wants work and gets the first step to run. - WHEN worker-manager makes request GET /work/obelix + WHEN obelix makes request GET /work THEN result has status code 200 AND body matches ... { @@ -99,7 +98,7 @@ Worker wants work and gets the first step to run. Worker reports some build output. Note the exit code indicating failure. - WHEN worker-manager makes request POST /work with a valid token and body + WHEN obelix makes request POST /work with a valid token and body ... { ... "build_id": "rome/1", ... "worker": "obelix", @@ -114,7 +113,7 @@ failure. A build step failed, so now the build has ended, and there's no more work to do. - WHEN worker-manager makes request GET /work/obelix + WHEN obelix makes request GET /work THEN result has status code 200 AND body matches {} diff --git a/yarns/600-unauthz.yarn b/yarns/600-unauthz.yarn index 176ac49..55cac30 100644 --- a/yarns/600-unauthz.yarn +++ b/yarns/600-unauthz.yarn @@ -79,11 +79,11 @@ Set up the controller. THEN result has status code 401 WHEN outsider makes request - ... GET /work/obelix with an invalid token + ... GET /work with an invalid token THEN result has status code 401 WHEN outsider makes request - ... GET /workers/obelix with an invalid token + ... GET /workers with an invalid token THEN result has status code 401 WHEN outsider makes request diff --git a/yarns/900-local.yarn b/yarns/900-local.yarn index 9ec0dcb..b8b6695 100644 --- a/yarns/900-local.yarn +++ b/yarns/900-local.yarn @@ -35,11 +35,12 @@ along with this program. If not, see . argv = [ os.path.join(srcdir, 'create-token'), scopes, + user, ] token = cliapp.runcmd(argv, feed_stdin=key) store_token(user, token) vars['issuer'] = 'localhost' - vars['audience'] = 'localhost' + vars['audience'] = user ## Controller configuration -- cgit v1.2.1