From e58e67d657efa52d6f7609a1cca8782f9e9d1c51 Mon Sep 17 00:00:00 2001 From: Seth House Date: Mon, 11 Dec 2017 04:28:40 -0700 Subject: [PATCH 01/10] Line-length formatting --- pepper/libpepper.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/pepper/libpepper.py b/pepper/libpepper.py index b641fc0..0e61f2e 100644 --- a/pepper/libpepper.py +++ b/pepper/libpepper.py @@ -57,7 +57,10 @@ class Pepper(object): u'ms-4': True}]} ''' - def __init__(self, api_url='https://localhost:8000', debug_http=False, ignore_ssl_errors=False): + def __init__(self, + api_url='https://localhost:8000', + debug_http=False, + ignore_ssl_errors=False): ''' Initialize the class with the URL of the API @@ -188,7 +191,8 @@ def req(self, path, data=None): :rtype: dictionary ''' - if (hasattr(data, 'get') and data.get('eauth') == 'kerberos') or self.auth.get('eauth') == 'kerberos': + if ((hasattr(data, 'get') and data.get('eauth') == 'kerberos') + or self.auth.get('eauth') == 'kerberos'): return self.req_requests(path, data) headers = { From ea0dde8e1ad5b81bf72279aac18a72ac9e0c5978 Mon Sep 17 00:00:00 2001 From: Seth House Date: Mon, 11 Dec 2017 04:29:27 -0700 Subject: [PATCH 02/10] Un-hard-code path from the client lib local/runner/wheel helpers The wrapped function has a default. No reason this needs to be done in each. --- pepper/libpepper.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/pepper/libpepper.py b/pepper/libpepper.py index 0e61f2e..67e0209 100644 --- a/pepper/libpepper.py +++ b/pepper/libpepper.py @@ -328,7 +328,7 @@ def local(self, tgt, fun, arg=None, kwarg=None, expr_form='glob', if ret: low['ret'] = ret - return self.low([low], path='/') + return self.low([low]) def local_async(self, tgt, fun, arg=None, kwarg=None, expr_form='glob', timeout=None, ret=None): @@ -358,7 +358,7 @@ def local_async(self, tgt, fun, arg=None, kwarg=None, expr_form='glob', if ret: low['ret'] = ret - return self.low([low], path='/') + return self.low([low]) def local_batch(self, tgt, fun, arg=None, kwarg=None, expr_form='glob', batch='50%', ret=None): @@ -388,7 +388,7 @@ def local_batch(self, tgt, fun, arg=None, kwarg=None, expr_form='glob', if ret: low['ret'] = ret - return self.low([low], path='/') + return self.low([low]) def lookup_jid(self, jid): ''' @@ -415,7 +415,7 @@ def runner(self, fun, arg=None, **kwargs): low.update(kwargs) - return self.low([low], path='/') + return self.low([low]) def wheel(self, fun, arg=None, kwarg=None, **kwargs): ''' @@ -436,7 +436,7 @@ def wheel(self, fun, arg=None, kwarg=None, **kwargs): low.update(kwargs) - return self.low([low], path='/') + return self.low([low]) def login(self, username, password, eauth): ''' From 5508c6c9a8f3ceaf5a3fbbe133b748c47aa199e6 Mon Sep 17 00:00:00 2001 From: Seth House Date: Mon, 11 Dec 2017 04:29:52 -0700 Subject: [PATCH 03/10] Add a new helper function to retrieve an eauth token from the new /token --- pepper/libpepper.py | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/pepper/libpepper.py b/pepper/libpepper.py index 67e0209..f0c6709 100644 --- a/pepper/libpepper.py +++ b/pepper/libpepper.py @@ -438,17 +438,24 @@ def wheel(self, fun, arg=None, kwarg=None, **kwargs): return self.low([low]) - def login(self, username, password, eauth): + def _send_auth(self, path, **kwargs): + return self.req(path, kwargs) + + def login(self, **kwargs): ''' Authenticate with salt-api and return the user permissions and authentication token or an empty dict ''' - self.auth = self.req('/login', { - 'username': username, - 'password': password, - 'eauth': eauth}).get('return', [{}])[0] + self.auth = self._send_auth('/login', **kwargs).get('return', [{}])[0] + return self.auth + def token(self, **kwargs): + ''' + Get an eauth token from Salt for use with the /run URL + + ''' + self.auth = self._send_auth('/token', **kwargs)[0] return self.auth def _construct_url(self, path): From f429960d18317aca392b6075cf68d7ff6d96a22a Mon Sep 17 00:00:00 2001 From: Seth House Date: Mon, 11 Dec 2017 04:32:52 -0700 Subject: [PATCH 04/10] Add new --run-uri flag to make use of /token and /run instead Eauth tokens weren't exposed in a public-facing API and had a hard-coded expiration until somewhat recently which is why salt-api used session tokens. Now that is changed using eauth tokens is usually more convenient. This addition allows use with Pepper. --- pepper/cli.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/pepper/cli.py b/pepper/cli.py index b1670fb..c7209ac 100644 --- a/pepper/cli.py +++ b/pepper/cli.py @@ -207,6 +207,13 @@ def add_authopts(self): generated and made available for the period defined in the Salt Master.""")) + optgroup.add_option('-r', '--run-uri', default=False, + dest='userun', action='store_true', + help=textwrap.dedent("""\ + Use an eauth token from /token and send commands through the + /run URL instead of the traditional session token + approach.""")) + optgroup.add_option('-x', dest='cache', default=os.environ.get('PEPPERCACHE', os.path.join(os.path.expanduser('~'), '.peppercache')), From 9aecca4b73d5849ccc0c7b29299ed86a4c559b2d Mon Sep 17 00:00:00 2001 From: Seth House Date: Mon, 11 Dec 2017 04:33:32 -0700 Subject: [PATCH 05/10] Adjust behavior if --use-run is specified at the CLI --- pepper/cli.py | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/pepper/cli.py b/pepper/cli.py index c7209ac..ce85546 100644 --- a/pepper/cli.py +++ b/pepper/cli.py @@ -397,13 +397,13 @@ def parse_cmd(self): return [low] - def poll_for_returns(self, api, load): + def poll_for_returns(self, api, load, path): ''' Run a command with the local_async client and periodically poll the job cache for returns for the job. ''' load[0]['client'] = 'local_async' - async_ret = api.low(load) + async_ret = api.low(load, path) jid = async_ret['return'][0]['jid'] nodes = async_ret['return'][0]['minions'] ret_nodes = [] @@ -454,6 +454,10 @@ def run(self): self.parse_url(), debug_http=self.options.debug_http, ignore_ssl_errors=self.options.ignore_ssl_certificate_errors) + + login = api.token if self.options.userun else api.login + path = '/run' if self.options.userun else '/' + if self.options.mktoken: token_file = self.options.cache try: @@ -466,7 +470,7 @@ def run(self): except Exception as e: if e.args[0] is not 2: logger.error('Unable to load login token from {0} {1}'.format(token_file, str(e))) - auth = api.login(*self.parse_login()) + auth = login(*self.parse_login()) try: oldumask = os.umask(0) fdsc = os.open(token_file, os.O_WRONLY | os.O_CREAT, 0o600) @@ -477,12 +481,16 @@ def run(self): finally: os.umask(oldumask) else: - auth = api.login(*self.parse_login()) + auth = login(*self.parse_login()) + + if self.options.userun: + for i in load: + i['token'] = auth['token'] if self.options.fail_if_minions_dont_respond: - for exit_code, ret in self.poll_for_returns(api, load): + for exit_code, ret in self.poll_for_returns(api, load, path): yield exit_code, json.dumps(ret, sort_keys=True, indent=4) else: - ret = api.low(load) + ret = api.low(load, path=path) exit_code = 0 yield exit_code, json.dumps(ret, sort_keys=True, indent=4) From e0adc35a27ce78b1ddbc63c1f6d36fce338675e5 Mon Sep 17 00:00:00 2001 From: Seth House Date: Tue, 12 Dec 2017 12:52:58 -0700 Subject: [PATCH 06/10] Remove auth verification via /stats We'll get an auth verification when we go to run the actual command so this pre-check isn't needed. Plus auth on /stats can be disabled now. --- pepper/cli.py | 1 - 1 file changed, 1 deletion(-) diff --git a/pepper/cli.py b/pepper/cli.py index ce85546..808de2f 100644 --- a/pepper/cli.py +++ b/pepper/cli.py @@ -466,7 +466,6 @@ def run(self): if api.auth['expire'] < time.time()+30: logger.error('Login token expired') raise Exception('Login token expired') - api.req('/stats') except Exception as e: if e.args[0] is not 2: logger.error('Unable to load login token from {0} {1}'.format(token_file, str(e))) From 158569400a02c3d66a644d6e580b9b07cad4c50e Mon Sep 17 00:00:00 2001 From: Seth House Date: Tue, 12 Dec 2017 12:53:11 -0700 Subject: [PATCH 07/10] Long-lines formatting --- pepper/cli.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/pepper/cli.py b/pepper/cli.py index 808de2f..e24d59a 100644 --- a/pepper/cli.py +++ b/pepper/cli.py @@ -468,7 +468,8 @@ def run(self): raise Exception('Login token expired') except Exception as e: if e.args[0] is not 2: - logger.error('Unable to load login token from {0} {1}'.format(token_file, str(e))) + logger.error('Unable to load login token from {0} {1}' + .format(token_file, str(e))) auth = login(*self.parse_login()) try: oldumask = os.umask(0) @@ -476,7 +477,8 @@ def run(self): with os.fdopen(fdsc, 'wt') as f: json.dump(auth, f) except Exception as e: - logger.error('Unable to save token to {0} {1}'.format(token_file, str(e))) + logger.error('Unable to save token to {0} {1}' + .format(token_file, str(e))) finally: os.umask(oldumask) else: From 12067b7a248a35cddcb7f83fe65916b261c474b8 Mon Sep 17 00:00:00 2001 From: Seth House Date: Thu, 14 Dec 2017 00:09:11 -0700 Subject: [PATCH 08/10] Move login and low into methods to differentiate -r flag centrally --- pepper/cli.py | 70 ++++++++++++++++++++++++++++++++------------------- 1 file changed, 44 insertions(+), 26 deletions(-) diff --git a/pepper/cli.py b/pepper/cli.py index e24d59a..723eabf 100644 --- a/pepper/cli.py +++ b/pepper/cli.py @@ -397,13 +397,13 @@ def parse_cmd(self): return [low] - def poll_for_returns(self, api, load, path): + def poll_for_returns(self, api, load): ''' Run a command with the local_async client and periodically poll the job cache for returns for the job. ''' load[0]['client'] = 'local_async' - async_ret = api.low(load, path) + async_ret = self.low(api, load) jid = async_ret['return'][0]['jid'] nodes = async_ret['return'][0]['minions'] ret_nodes = [] @@ -420,7 +420,14 @@ def poll_for_returns(self, api, load, path): exit_code = 1 break - jid_ret = api.lookup_jid(jid) + jid_ret = self.low(api, [{ + 'client': 'runner', + 'fun': 'jobs.lookup_jid', + 'kwarg': { + 'jid': jid, + }, + }]) + responded = set(jid_ret['return'][0].keys()) ^ set(ret_nodes) for node in responded: yield None, "{{{}: {}}}".format( @@ -438,32 +445,15 @@ def poll_for_returns(self, api, load, path): yield exit_code, "{{Failed: {}}}".format( list(set(ret_nodes) ^ set(nodes))) - def run(self): - ''' - Parse all arguments and call salt-api - ''' - self.parse() - - # move logger instantiation to method? - logger.addHandler(logging.StreamHandler()) - logger.setLevel(max(logging.ERROR - (self.options.verbose * 10), 1)) - - load = self.parse_cmd() - - api = pepper.Pepper( - self.parse_url(), - debug_http=self.options.debug_http, - ignore_ssl_errors=self.options.ignore_ssl_certificate_errors) - + def login(self, api): login = api.token if self.options.userun else api.login - path = '/run' if self.options.userun else '/' if self.options.mktoken: token_file = self.options.cache try: with open(token_file, 'rt') as f: - api.auth = json.load(f) - if api.auth['expire'] < time.time()+30: + auth = json.load(f) + if auth['expire'] < time.time()+30: logger.error('Login token expired') raise Exception('Login token expired') except Exception as e: @@ -484,14 +474,42 @@ def run(self): else: auth = login(*self.parse_login()) + api.auth = auth + self.auth = auth + return auth + + def low(self, api, load): + path = '/run' if self.options.userun else '/' + if self.options.userun: for i in load: - i['token'] = auth['token'] + i['token'] = self.auth['token'] + + return api.low(load, path=path) + + def run(self): + ''' + Parse all arguments and call salt-api + ''' + self.parse() + + # move logger instantiation to method? + logger.addHandler(logging.StreamHandler()) + logger.setLevel(max(logging.ERROR - (self.options.verbose * 10), 1)) + + load = self.parse_cmd() + + api = pepper.Pepper( + self.parse_url(), + debug_http=self.options.debug_http, + ignore_ssl_errors=self.options.ignore_ssl_certificate_errors) + + self.login(api) if self.options.fail_if_minions_dont_respond: - for exit_code, ret in self.poll_for_returns(api, load, path): + for exit_code, ret in self.poll_for_returns(api, load): yield exit_code, json.dumps(ret, sort_keys=True, indent=4) else: - ret = api.low(load, path=path) + ret = self.low(api, load) exit_code = 0 yield exit_code, json.dumps(ret, sort_keys=True, indent=4) From 02f8a51950c6390ed99ca2a94f7bc56811bc9484 Mon Sep 17 00:00:00 2001 From: Seth House Date: Thu, 14 Dec 2017 00:25:36 -0700 Subject: [PATCH 09/10] Add flag to support token_expire_user_override Master setting --- pepper/cli.py | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/pepper/cli.py b/pepper/cli.py index 723eabf..3700db1 100644 --- a/pepper/cli.py +++ b/pepper/cli.py @@ -196,6 +196,12 @@ def add_authopts(self): dest='password', help=textwrap.dedent("""\ Optional, but will be prompted unless --non-interactive""")) + optgroup.add_option('--token-expire', + dest='token_expire', help=textwrap.dedent("""\ + Set eauth token expiry in seconds. Must be allowed per + user. See the `token_expire_user_override` Master setting + for more info.""")) + optgroup.add_option('--non-interactive', action='store_false', dest='interactive', help=textwrap.dedent("""\ Optional, fail rather than waiting for input"""), default=True) @@ -260,6 +266,8 @@ def get_login_details(self): if self.options.eauth: results['SALTAPI_EAUTH'] = self.options.eauth + if self.options.token_expire: + results['SALTAPI_TOKEN_EXPIRE'] = self.options.token_expire if self.options.username is None and results['SALTAPI_USER'] is None: if self.options.interactive: results['SALTAPI_USER'] = input('Username: ') @@ -315,11 +323,17 @@ def parse_login(self): login_details = self.get_login_details() # Auth values placeholder; grab interactively at CLI or from config - user = login_details['SALTAPI_USER'] - passwd = login_details['SALTAPI_PASS'] + username = login_details['SALTAPI_USER'] + password = login_details['SALTAPI_PASS'] eauth = login_details['SALTAPI_EAUTH'] - return user, passwd, eauth + ret = dict(username=username, password=password, eauth=eauth) + + token_expire = login_details.get('SALTAPI_TOKEN_EXPIRE', None) + if token_expire: + ret['token_expire'] = int(token_expire) + + return ret def parse_cmd(self): ''' @@ -460,7 +474,7 @@ def login(self, api): if e.args[0] is not 2: logger.error('Unable to load login token from {0} {1}' .format(token_file, str(e))) - auth = login(*self.parse_login()) + auth = login(**self.parse_login()) try: oldumask = os.umask(0) fdsc = os.open(token_file, os.O_WRONLY | os.O_CREAT, 0o600) @@ -472,7 +486,7 @@ def login(self, api): finally: os.umask(oldumask) else: - auth = login(*self.parse_login()) + auth = login(**self.parse_login()) api.auth = auth self.auth = auth From 7b05bbfa9b1c64f2e34370f649d2f7013af92a4e Mon Sep 17 00:00:00 2001 From: Seth House Date: Tue, 23 Jan 2018 03:13:08 -0700 Subject: [PATCH 10/10] Add integration tests --- tests/__init__.py | 0 tests/integration.py | 114 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 114 insertions(+) create mode 100644 tests/__init__.py create mode 100755 tests/integration.py diff --git a/tests/__init__.py b/tests/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/integration.py b/tests/integration.py new file mode 100755 index 0000000..921616f --- /dev/null +++ b/tests/integration.py @@ -0,0 +1,114 @@ +#!/usr/bin/env python +''' +These integration tests will execute non-destructive commands against a real +Salt and salt-api instance. Those must be set up and started independently. +This will take several minutes to run. + +Usage: + +SALTAPI_URL=http://localhost:8000 \ +SALTAPI_USER=saltdev \ +SALTAPI_PASS=saltdev \ +SALTAPI_EAUTH=auto \ + python -m unittest tests.integration +''' +import itertools +import json +import os +import shutil +import subprocess +import tempfile +import time +import unittest + +try: + SALTAPI_URL = os.environ['SALTAPI_URL'] + SALTAPI_USER = os.environ['SALTAPI_USER'] + SALTAPI_PASS = os.environ['SALTAPI_PASS'] + SALTAPI_EAUTH = os.environ['SALTAPI_EAUTH'] +except KeyError: + raise SystemExit('The following environment variables must be set: ' + 'SALTAPI_URL, SALTAPI_USER, SALTAPI_PASS, SALTAPI_EAUTH.') + + +def _pepper(*args): + ''' + Wrapper to invoke Pepper with common params and inside an empty env + ''' + def_args = [ + 'pepper', + '--saltapi-url={0}'.format(SALTAPI_URL), + '--username={0}'.format(SALTAPI_USER), + '--password={0}'.format(SALTAPI_PASS), + '--eauth={0}'.format(SALTAPI_EAUTH), + ] + + return subprocess.check_output(itertools.chain(def_args, args)) + + +class TestVanilla(unittest.TestCase): + def _pepper(self, *args): + return json.loads(_pepper(*args))['return'][0] + + def test_local(self): + '''Sanity-check: Has at least one minion''' + ret = self._pepper('*', 'test.ping') + self.assertTrue(ret.values()[0]) + + def test_run(self): + '''Run command via /run URI''' + ret = self._pepper('--run-uri', '*', 'test.ping') + self.assertTrue(ret.values()[0]) + + def test_long_local(self): + '''Test a long call blocks until the return''' + ret = self._pepper('*', 'test.sleep', '30') + self.assertTrue(ret.values()[0]) + + +class TestPoller(unittest.TestCase): + def _pepper(self, *args): + return _pepper(*args).splitlines()[0] + + def test_local_poll(self): + '''Test the returns poller for localclient''' + ret = self._pepper('--run-uri', '--fail-if-incomplete', '*', 'test.sleep', '30') + self.assertTrue('True' in ret) + + +class TestTokens(unittest.TestCase): + def setUp(self): + self.tokdir = tempfile.mkdtemp() + self.tokfile = os.path.join(self.tokdir, 'peppertok.json') + + def tearDown(self): + shutil.rmtree(self.tokdir) + + def _pepper(self, *args): + return json.loads(_pepper(*args))['return'][0] + + def test_local_token(self): + '''Test local execution with token file''' + ret = self._pepper('-x', self.tokfile, + '--make-token', '--run-uri', '*', 'test.ping') + self.assertTrue(ret.values()[0]) + + def test_runner_token(self): + '''Test runner execution with token file''' + ret = self._pepper('-x', self.tokfile, + '--make-token', '--run-uri', + '--client', 'runner', 'test.metasyntactic') + self.assertTrue(ret[0] == 'foo') + + def test_token_expire(self): + '''Test token override param''' + now = time.time() + self._pepper('-x', self.tokfile, '--make-token', '--run-uri', + '--token-expire', '94670856', + '*', 'test.ping') + + with open(self.tokfile, 'r') as f: + token = json.load(f) + diff = (now + float(94670856)) - token['expire'] + # Allow for 10-second window between request and master-side auth. + self.assertTrue(diff < 10)