Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

make salt-api arg/kwarg handling the same as cli for all clients #50124

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions doc/topics/releases/neon.rst
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,17 @@ Module Changes
``dns`` can be overriden by ``ipv4dns`` or ``ipv6dns``). The ``proto`` option
is now required.

Salt-API Changes
================

- (cherrypy, tornado) when handling lowstate from requests, all inputs are now passed
through :py:func:`args.parse_input <salt.util.args.parse_input>` function to make
arg parsing equivalent to what happens on the cli. This means you can now provide
'yaml={foo: bar}' style kwargs in the arg list and it will be deserialized
appropriately.
- (cherrypy, tornado) a new X-Salt-Version response header has been added for simple
introspection of available features for consumers (pepper).

Salt Cloud Features
===================

Expand Down
24 changes: 22 additions & 2 deletions salt/netapi/rest_cherrypy/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -612,6 +612,7 @@
import salt
import salt.auth
import salt.exceptions
import salt.utils.args
import salt.utils.event
import salt.utils.json
import salt.utils.stringutils
Expand Down Expand Up @@ -1091,6 +1092,15 @@ def lowdata_fmt():
cherrypy.serving.request.lowstate = data


def default_headers():
'''
set default headers that should be attached to every request
'''

headers = cherrypy.response.headers
headers['X-Salt-Version'] = str(salt.version.__saltstack_version__)


tools_config = {
'on_start_resource': [
('html_override', html_override_tool),
Expand All @@ -1106,6 +1116,9 @@ def lowdata_fmt():
('hypermedia_out', hypermedia_out),
('salt_ip_verify', salt_ip_verify_tool),
],
'before_finalize': [
('default_headers', default_headers),
],
}

for hook, tool_list in tools_config.items():
Expand Down Expand Up @@ -1172,8 +1185,14 @@ def exec_lowstate(self, client=None, token=None):

# Make any 'arg' params a list if not already.
# This is largely to fix a deficiency in the urlencoded format.
if 'arg' in chunk and not isinstance(chunk['arg'], list):
chunk['arg'] = [chunk['arg']]
if 'arg' in chunk:
if not isinstance(chunk['arg'], list):
chunk['arg'] = [chunk['arg']]

# Run name=value args through parse_input
_arg, _kwarg = salt.utils.args.parse_input(chunk.pop('arg', []), condition=False)
chunk['arg'] = _arg
chunk.update(_kwarg)

ret = self.api.run(chunk)

Expand Down Expand Up @@ -2855,6 +2874,7 @@ def get_conf(self):

'tools.html_override.on': True,
'tools.cors_tool.on': True,
'tools.default_headers.on': True,
},
}

Expand Down
21 changes: 17 additions & 4 deletions salt/netapi/rest_tornado/saltnado.py
Original file line number Diff line number Diff line change
Expand Up @@ -559,22 +559,33 @@ def _get_lowstate(self):
'''
if not self.request.body:
return

data = self.deserialize(self.request.body)
self.request_payload = copy(data)

if data and 'arg' in data and not isinstance(data['arg'], list):
data['arg'] = [data['arg']]

if not isinstance(data, list):
lowstate = [data]
else:
lowstate = data

for chunk in lowstate:
if chunk and 'arg' in chunk:
if not isinstance(chunk['arg'], list):
chunk['arg'] = [chunk['arg']]

# Run name=value args through parse_input
_arg, _kwarg = salt.utils.args.parse_input(chunk.pop('arg', []), condition=False)
chunk['arg'] = _arg
if 'kwarg' in chunk and isinstance(chunk['kwarg'], dict):
chunk['kwarg'].update(_kwarg)
else:
chunk['kwarg'] = _kwarg

return lowstate

def set_default_headers(self):
'''
Set default CORS headers
Set default headers
'''
mod_opts = self.application.mod_opts

Expand All @@ -586,6 +597,8 @@ def set_default_headers(self):
if allowed_origin:
self.set_header("Access-Control-Allow-Origin", allowed_origin)

self.set_header('X-Salt-Version', str(salt.version.__saltstack_version__))

def options(self, *args, **kwargs):
'''
Return CORS headers for preflight requests
Expand Down
10 changes: 6 additions & 4 deletions tests/integration/netapi/rest_cherrypy/test_app.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
# Import salt libs
import salt.utils.json
import salt.utils.stringutils
import salt.version

# Import test support libs
import tests.support.cherrypy_testclasses as cptc
Expand Down Expand Up @@ -61,6 +62,7 @@ def test_good_login(self):
'content-type': 'application/x-www-form-urlencoded'
})
self.assertEqual(response.status, '200 OK')
self.assertTrue(salt.version.SaltStackVersion.parse(response.headers['X-Salt-Version']))
return response

def test_bad_login(self):
Expand All @@ -72,6 +74,7 @@ def test_bad_login(self):
headers={
'content-type': 'application/x-www-form-urlencoded'
})
self.assertTrue(salt.version.SaltStackVersion.parse(response.headers['X-Salt-Version']))
self.assertEqual(response.status, '401 Unauthorized')

def test_logout(self):
Expand Down Expand Up @@ -159,7 +162,7 @@ class TestArgKwarg(cptc.BaseRestCherryPyTest):
('client', 'runner'),
('fun', 'test.arg'),
# use singular form for arg and kwarg
('arg', [1234]),
('arg', [1234, '56', 'yaml={baz: bar}']),
('kwarg', {'ext_source': 'redis'}),
)

Expand Down Expand Up @@ -197,9 +200,8 @@ def test_accepts_arg_kwarg_keys(self):
}
)
resp = salt.utils.json.loads(salt.utils.stringutils.to_str(response.body[0]))
self.assertEqual(resp['return'][0]['args'], [1234])
self.assertEqual(resp['return'][0]['kwargs'],
{'ext_source': 'redis'})
self.assertEqual(resp['return'][0]['args'], [1234, 56])
self.assertEqual(resp['return'][0]['kwargs'], {'ext_source': 'redis', 'yaml': {'baz': 'bar'}})


class TestJobs(cptc.BaseRestCherryPyTest):
Expand Down
27 changes: 26 additions & 1 deletion tests/integration/netapi/rest_tornado/test_app.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
# Import Salt Libs
import salt.utils.json
import salt.utils.stringutils
import salt.version
from salt.netapi.rest_tornado import saltnado
from salt.utils.versions import StrictVersion

Expand Down Expand Up @@ -43,6 +44,7 @@ def get_app(self):

application.event_listener = saltnado.EventListener({}, self.opts)
self.application = application

return application

def test_root(self):
Expand All @@ -58,6 +60,7 @@ def test_root(self):
self.assertEqual(sorted(response_obj['clients']),
['local', 'local_async', 'runner', 'runner_async'])
self.assertEqual(response_obj['return'], 'Welcome')
self.assertTrue(salt.version.SaltStackVersion.parse(response.headers['X-Salt-Version']))

def test_post_no_auth(self):
'''
Expand Down Expand Up @@ -121,8 +124,30 @@ def test_simple_local_post_no_tgt(self):
response_obj = salt.utils.json.loads(response.body)
self.assertEqual(response_obj['return'], ["No minions matched the target. No command was sent, no jid was assigned."])

# local client request body test
# arg/kwarg parsing test
def test_simple_arg_kwarg_parsing(self):
'''
POST job to test arg/kwarg parsing
'''
low = {'client': 'local',
'tgt': 'minion',
'fun': 'test.arg_clean',
'arg': [1, '2', 'yaml={bar: baz}'],
'kwarg': {'a': 'b'}
}
response = self.fetch('/',
method='POST',
body=salt.utils.json.dumps(low),
headers={'Content-Type': self.content_type_map['json'],
saltnado.AUTH_TOKEN_HEADER: self.token['token']},
connect_timeout=30,
request_timeout=30,
)
response_obj = salt.utils.json.loads(response.body)
self.assertEqual(response_obj['return'][0]['minion']['args'], [1, 2])
self.assertEqual(response_obj['return'][0]['minion']['kwargs'], {'a': 'b', 'yaml': {'bar': 'baz'}})

# local client request body test
@skipIf(True, 'Undetermined race condition in test. Temporarily disabled.')
def test_simple_local_post_only_dictionary_request(self):
'''
Expand Down
36 changes: 23 additions & 13 deletions tests/unit/netapi/test_rest_tornado.py
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,8 @@ def test_deserialize(self):
"client": "local",
"tgt": "*",
"fun": "test.fib",
"arg": ["10"]
"arg": [10],
"kwarg": {},
},
{
"client": "runner",
Expand Down Expand Up @@ -305,7 +306,7 @@ def test_deserialize(self):
self.assertEqual(returned_lowstate['client'], 'local')
self.assertEqual(returned_lowstate['tgt'], '*')
self.assertEqual(returned_lowstate['fun'], 'test.fib')
self.assertEqual(returned_lowstate['arg'], ['10', 'foo'])
self.assertEqual(returned_lowstate['arg'], [10, 'foo'])

# Send json with utf8 charset
response = self.fetch('/',
Expand All @@ -318,49 +319,58 @@ def test_get_lowstate(self):
'''
Test transformations low data of the function _get_lowstate
'''
valid_lowstate = [{
valid_arg_lowstate = [{
u"client": u"local",
u"tgt": u"*",
u"fun": u"test.fib",
u"arg": [10],
u"kwarg": {},
}]

valid_arg_kwarg_lowstate = [{
u"client": u"local",
u"tgt": u"*",
u"fun": u"test.fib",
u"arg": [u"10"]
u"arg": [10],
u"kwarg": {u"yaml": {u"foo": u"bar"}}
}]

# Case 1. dictionary type of lowstate
request_lowstate = {
"client": "local",
"tgt": "*",
"fun": "test.fib",
"arg": ["10"]
u"arg": ["10", "yaml={foo: bar}"]
}

response = self.fetch('/',
method='POST',
body=salt.utils.json.dumps(request_lowstate),
headers={'Content-Type': self.content_type_map['json']})

self.assertEqual(valid_lowstate, salt.utils.json.loads(response.body)['lowstate'])
self.assertEqual(valid_arg_kwarg_lowstate, salt.utils.json.loads(response.body)['lowstate'])

# Case 2. string type of arg
request_lowstate = {
"client": "local",
"tgt": "*",
"fun": "test.fib",
"arg": "10"
u"arg": "10"
}

response = self.fetch('/',
method='POST',
body=salt.utils.json.dumps(request_lowstate),
headers={'Content-Type': self.content_type_map['json']})

self.assertEqual(valid_lowstate, salt.utils.json.loads(response.body)['lowstate'])
self.assertEqual(valid_arg_lowstate, salt.utils.json.loads(response.body)['lowstate'])

# Case 3. Combine Case 1 and Case 2.
request_lowstate = {
"client": "local",
"tgt": "*",
"fun": "test.fib",
"arg": "10"
u"arg": ["10", "yaml={foo: bar}"]
}

# send as json
Expand All @@ -369,21 +379,21 @@ def test_get_lowstate(self):
body=salt.utils.json.dumps(request_lowstate),
headers={'Content-Type': self.content_type_map['json']})

self.assertEqual(valid_lowstate, salt.utils.json.loads(response.body)['lowstate'])
self.assertEqual(valid_arg_kwarg_lowstate, salt.utils.json.loads(response.body)['lowstate'])

# send as yaml
response = self.fetch('/',
method='POST',
body=salt.utils.yaml.safe_dump(request_lowstate),
headers={'Content-Type': self.content_type_map['yaml']})
self.assertEqual(valid_lowstate, salt.utils.json.loads(response.body)['lowstate'])
self.assertEqual(valid_arg_kwarg_lowstate, salt.utils.json.loads(response.body)['lowstate'])

# send as plain text
response = self.fetch('/',
method='POST',
body=salt.utils.json.dumps(request_lowstate),
headers={'Content-Type': self.content_type_map['text']})
self.assertEqual(valid_lowstate, salt.utils.json.loads(response.body)['lowstate'])
self.assertEqual(valid_arg_kwarg_lowstate, salt.utils.json.loads(response.body)['lowstate'])

# send as form-urlencoded
request_form_lowstate = (
Expand All @@ -397,7 +407,7 @@ def test_get_lowstate(self):
method='POST',
body=urlencode(request_form_lowstate),
headers={'Content-Type': self.content_type_map['form']})
self.assertEqual(valid_lowstate, salt.utils.json.loads(response.body)['lowstate'])
self.assertEqual(valid_arg_lowstate, salt.utils.json.loads(response.body)['lowstate'])

def test_cors_origin_wildcard(self):
'''
Expand Down