No input validation for action with kwargs argument
In Mistral, there may be a lot of native actions registered in the system, e.g. std.echo, std.http, etc. However, not all of them have precise parameter definition in their initialization, Nova action 'servers.find' is a typical example of this. This patch add name of the ** argument in action definition input string, in case there is no other explicit positional params, we don't check action input if it has ** argument. Note: We don't support only *args argument contained in action initialization for the time being, since the type of default action input is dictionary. Change-Id: I01f06901cf8a19d1d7a0b0382c6e2e4a7e7fc123 Closes-Bug: #1469605
This commit is contained in:
parent
2811620769
commit
f6a7f2ee2b
@ -93,7 +93,11 @@ def get_action_input(action_name, input_dict, wf_name=None, wf_spec=None):
|
|||||||
|
|
||||||
if action_def.action_class:
|
if action_def.action_class:
|
||||||
_inject_action_ctx_for_validating(action_def, input_dict)
|
_inject_action_ctx_for_validating(action_def, input_dict)
|
||||||
e_utils.validate_input(action_def, input_dict)
|
|
||||||
|
# NOTE(xylan): Don't validate action input if action initialization method
|
||||||
|
# contains ** argument.
|
||||||
|
if '**' not in action_def.input:
|
||||||
|
e_utils.validate_input(action_def, input_dict)
|
||||||
|
|
||||||
if action_def.spec:
|
if action_def.spec:
|
||||||
# Ad-hoc action.
|
# Ad-hoc action.
|
||||||
|
@ -38,6 +38,12 @@ class ActionManagerTest(base.DbTestCase):
|
|||||||
self._assert_single_item(action_list, name="nova.servers_get")
|
self._assert_single_item(action_list, name="nova.servers_get")
|
||||||
self._assert_single_item(action_list, name="nova.volumes_delete")
|
self._assert_single_item(action_list, name="nova.volumes_delete")
|
||||||
|
|
||||||
|
server_find_action = self._assert_single_item(
|
||||||
|
action_list,
|
||||||
|
name="nova.servers_find"
|
||||||
|
)
|
||||||
|
self.assertIn('**', server_find_action.input)
|
||||||
|
|
||||||
self._assert_single_item(action_list, name="keystone.users_list")
|
self._assert_single_item(action_list, name="keystone.users_list")
|
||||||
self._assert_single_item(action_list, name="keystone.trusts_create")
|
self._assert_single_item(action_list, name="keystone.trusts_create")
|
||||||
|
|
||||||
|
@ -12,9 +12,11 @@
|
|||||||
# See the License for the specific language governing permissions and
|
# See the License for the specific language governing permissions and
|
||||||
# limitations under the License.
|
# limitations under the License.
|
||||||
|
|
||||||
|
import mock
|
||||||
from oslo_config import cfg
|
from oslo_config import cfg
|
||||||
|
|
||||||
from mistral.db.v2 import api as db_api
|
from mistral.db.v2 import api as db_api
|
||||||
|
from mistral.db.v2.sqlalchemy import models
|
||||||
from mistral import exceptions as exc
|
from mistral import exceptions as exc
|
||||||
from mistral.services import actions
|
from mistral.services import actions
|
||||||
from mistral.tests.unit.engine import base
|
from mistral.tests.unit.engine import base
|
||||||
@ -103,3 +105,40 @@ class RunActionEngineTest(base.EngineTestCase):
|
|||||||
)
|
)
|
||||||
|
|
||||||
self.assertIn('concat', exception.message)
|
self.assertIn('concat', exception.message)
|
||||||
|
|
||||||
|
@mock.patch('mistral.engine.action_handler.resolve_action_definition')
|
||||||
|
@mock.patch('mistral.engine.utils.validate_input')
|
||||||
|
@mock.patch('mistral.services.action_manager.get_action_class')
|
||||||
|
@mock.patch('mistral.engine.action_handler.run_action')
|
||||||
|
def test_run_action_with_kwargs_input(self, run_mock, class_mock,
|
||||||
|
validate_mock, def_mock):
|
||||||
|
action_def = models.ActionDefinition()
|
||||||
|
action_def.update({
|
||||||
|
'name': 'fake_action',
|
||||||
|
'action_class': '',
|
||||||
|
'attributes': {},
|
||||||
|
'description': '',
|
||||||
|
'input': '**kwargs',
|
||||||
|
'is_system': True,
|
||||||
|
'scope': 'public'
|
||||||
|
})
|
||||||
|
def_mock.return_value = action_def
|
||||||
|
|
||||||
|
class_ret = mock.MagicMock()
|
||||||
|
class_mock.return_value = class_ret
|
||||||
|
|
||||||
|
self.engine.start_action('fake_action', {'input': 'Hello'})
|
||||||
|
|
||||||
|
self.assertEqual(2, def_mock.call_count)
|
||||||
|
def_mock.assert_called_with('fake_action', None, None)
|
||||||
|
|
||||||
|
self.assertEqual(0, validate_mock.call_count)
|
||||||
|
|
||||||
|
class_ret.assert_called_once_with(input='Hello')
|
||||||
|
|
||||||
|
run_mock.assert_called_once_with(
|
||||||
|
action_def,
|
||||||
|
{'input': 'Hello'},
|
||||||
|
target=None,
|
||||||
|
async=False
|
||||||
|
)
|
||||||
|
@ -71,4 +71,7 @@ def get_arg_list_as_str(func):
|
|||||||
else:
|
else:
|
||||||
arg_str_list.append("%s" % args[index])
|
arg_str_list.append("%s" % args[index])
|
||||||
|
|
||||||
|
if argspec.keywords:
|
||||||
|
arg_str_list.append("**%s" % argspec.keywords)
|
||||||
|
|
||||||
return ", ".join(arg_str_list)
|
return ", ".join(arg_str_list)
|
||||||
|
Loading…
Reference in New Issue
Block a user