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

Minor fixes to validation of runner parameter override #2363

Merged
merged 3 commits into from
Jan 8, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion contrib/examples/actions/weather.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ parameters:
auth:
type: string
headers:
type: string
type: object
method:
default: GET
enum:
Expand Down
4 changes: 3 additions & 1 deletion st2api/tests/unit/controllers/v1/test_actions.py
Original file line number Diff line number Diff line change
Expand Up @@ -464,7 +464,9 @@ def test_post_invalid_runner_type(self):
def test_post_override_runner_param_not_allowed(self):
post_resp = self.__do_post(ACTION_14, expect_errors=True)
self.assertEqual(post_resp.status_int, 400)
self.assertIn('cannot be overridden', post_resp.json.get('faultstring'))
expected = ('The attribute "type" for the runner parameter "sudo" '
'in action "dummy_pack_1.st2.dummy.action14" cannot be overridden.')
self.assertEqual(post_resp.json.get('faultstring'), expected)

def test_post_override_runner_param_allowed(self):
post_resp = self.__do_post(ACTION_15)
Expand Down
17 changes: 12 additions & 5 deletions st2common/st2common/util/schema/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,15 @@ def get_validator(version='custom'):
return validator


def validate_runner_parameter_attribute_override(
action_ref, param_name, attr_name, runner_param_attr_value, action_param_attr_value):
if (attr_name not in RUNNER_PARAM_OVERRIDABLE_ATTRS and
action_param_attr_value != runner_param_attr_value):
raise InvalidActionParameterException(
'The attribute "%s" for the runner parameter "%s" in action "%s" '
'cannot be overridden.' % (attr_name, param_name, action_ref))


def get_schema_for_action_parameters(action_db):
"""
Dynamically construct JSON schema for the provided action from the parameters metadata.
Expand All @@ -320,11 +329,9 @@ def get_schema_for_action_parameters(action_db):
parameters_schema.update({name: schema})
else:
for attribute, value in six.iteritems(schema):
if (attribute not in RUNNER_PARAM_OVERRIDABLE_ATTRS and
parameters_schema[name].get(attribute) != value):
raise InvalidActionParameterException(
'The attribute "%s" for the runner parameter "%s" cannot'
'be overridden.' % (attribute, name))
validate_runner_parameter_attribute_override(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the future please prefer to use keyword argument when calling functions.

While working on resolving merge conflicts in my PR, I believe I encounter a minor issue - function order is `runner_param_attr_value, action_param_attr_value), but the values here are passed to the function in the reverse order.

Luckily, in this case the order doesn't matter, because those values are only used in a simple != if condition, but it could matter in other scenarios.

action_db.ref, name, attribute,
value, parameters_schema[name].get(attribute))

parameters_schema[name][attribute] = value

Expand Down
15 changes: 7 additions & 8 deletions st2common/st2common/validators/api/action.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,14 @@

import six

from st2common.exceptions.action import InvalidActionParameterException
from st2common.exceptions.apivalidation import ValueValidationException
from st2common.exceptions.db import StackStormDBObjectNotFoundError
from st2common import log as logging
from st2common.util.action_db import get_runnertype_by_name
from st2common.util import schema as util_schema
from st2common.content.utils import get_packs_base_paths
from st2common.content.utils import check_pack_content_directory_exists
from st2common.models.system.common import ResourceReference


LOG = logging.getLogger(__name__)
Expand All @@ -41,7 +41,8 @@ def validate_action(action_api):
raise ValueValidationException(msg)

# Check if parameters defined are valid.
_validate_parameters(action_api.parameters, runner_db.runner_parameters)
action_ref = ResourceReference.to_string_reference(pack=action_api.pack, name=action_api.name)
_validate_parameters(action_ref, action_api.parameters, runner_db.runner_parameters)


def _get_runner_model(action_api):
Expand All @@ -59,16 +60,14 @@ def _is_valid_pack(pack):
return check_pack_content_directory_exists(pack=pack, content_type='actions')


def _validate_parameters(action_params=None, runner_params=None):
def _validate_parameters(action_ref, action_params=None, runner_params=None):
for action_param, action_param_meta in six.iteritems(action_params):
# Check if overridden runner parameters are permitted.
if action_param in runner_params:
for action_param_attr, value in six.iteritems(action_param_meta):
if (action_param_attr not in util_schema.RUNNER_PARAM_OVERRIDABLE_ATTRS and
runner_params[action_param].get(action_param_attr) != value):
raise InvalidActionParameterException(
'The attribute "%s" for the runner parameter "%s" cannot '
'be overridden.' % (action_param_attr, action_param))
util_schema.validate_runner_parameter_attribute_override(
action_ref, action_param, action_param_attr,
value, runner_params[action_param].get(action_param_attr))

if 'immutable' in action_param_meta:
if action_param in runner_params:
Expand Down
8 changes: 7 additions & 1 deletion st2common/tests/unit/services/test_action.py
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,13 @@ def test_request_override_runner_parameter(self):
def test_request_override_runner_parameter_type_attribute_value_changed(self):
parameters = {'hosts': 'localhost', 'cmd': 'uname -a'}
request = LiveActionDB(action=ACTION_OVR_PARAM_BAD_ATTR_REF, parameters=parameters)
self.assertRaises(InvalidActionParameterException, action_service.request, request)

with self.assertRaises(InvalidActionParameterException) as ex_ctx:
request, _ = action_service.request(request)

expected = ('The attribute "type" for the runner parameter "sudo" in '
'action "default.my.sudo.invalid.action" cannot be overridden.')
self.assertEqual(str(ex_ctx.exception), expected)

def test_request_override_runner_parameter_type_attribute_no_value_changed(self):
parameters = {'hosts': 'localhost', 'cmd': 'uname -a'}
Expand Down