diff --git a/changelogs/fragments/plugins-accept-only-valid-args.yaml b/changelogs/fragments/plugins-accept-only-valid-args.yaml
new file mode 100644
index 00000000000..6047e5748e0
--- /dev/null
+++ b/changelogs/fragments/plugins-accept-only-valid-args.yaml
@@ -0,0 +1,2 @@
+minor_changes:
+- action plugins strictly accept valid parameters and report invalid parameters
diff --git a/lib/ansible/plugins/action/__init__.py b/lib/ansible/plugins/action/__init__.py
index 4af91a480ac..c9a732d7798 100644
--- a/lib/ansible/plugins/action/__init__.py
+++ b/lib/ansible/plugins/action/__init__.py
@@ -45,6 +45,9 @@ class ActionBase(with_metaclass(ABCMeta, object)):
action in use.
'''
+ # A set of valid arguments
+ _VALID_ARGS = frozenset([])
+
def __init__(self, task, connection, play_context, loader, templar, shared_loader_obj):
self._task = task
self._connection = connection
@@ -95,6 +98,13 @@ class ActionBase(with_metaclass(ABCMeta, object)):
elif self._task.async_val and self._play_context.check_mode:
raise AnsibleActionFail('check mode and async cannot be used on same task.')
+ # Error if invalid argument is passed
+ if self._VALID_ARGS:
+ task_opts = frozenset(self._task.args.keys())
+ bad_opts = task_opts.difference(self._VALID_ARGS)
+ if bad_opts:
+ raise AnsibleActionFail('Invalid options for %s: %s' % (self._task.action, ','.join(list(bad_opts))))
+
if self._connection._shell.tmpdir is None and self._early_needs_tmp_path():
self._make_tmp_path()
diff --git a/lib/ansible/plugins/action/assert.py b/lib/ansible/plugins/action/assert.py
index 7c958443b62..4028201130d 100644
--- a/lib/ansible/plugins/action/assert.py
+++ b/lib/ansible/plugins/action/assert.py
@@ -27,6 +27,7 @@ class ActionModule(ActionBase):
''' Fail with custom message '''
TRANSFERS_FILES = False
+ _VALID_ARGS = frozenset(('fail_msg', 'msg', 'that'))
def run(self, tmp=None, task_vars=None):
if task_vars is None:
diff --git a/lib/ansible/plugins/action/debug.py b/lib/ansible/plugins/action/debug.py
index bbb8d9212b9..472feedf91d 100644
--- a/lib/ansible/plugins/action/debug.py
+++ b/lib/ansible/plugins/action/debug.py
@@ -28,16 +28,12 @@ class ActionModule(ActionBase):
''' Print statements during execution '''
TRANSFERS_FILES = False
- VALID_ARGS = frozenset(('msg', 'var', 'verbosity'))
+ _VALID_ARGS = frozenset(('msg', 'var', 'verbosity'))
def run(self, tmp=None, task_vars=None):
if task_vars is None:
task_vars = dict()
- for arg in self._task.args:
- if arg not in self.VALID_ARGS:
- return {"failed": True, "msg": "'%s' is not a valid option in debug" % arg}
-
if 'msg' in self._task.args and 'var' in self._task.args:
return {"failed": True, "msg": "'msg' and 'var' are incompatible options"}
diff --git a/lib/ansible/plugins/action/fail.py b/lib/ansible/plugins/action/fail.py
index 9a9da51d755..8d3450c88d1 100644
--- a/lib/ansible/plugins/action/fail.py
+++ b/lib/ansible/plugins/action/fail.py
@@ -25,6 +25,7 @@ class ActionModule(ActionBase):
''' Fail with custom message '''
TRANSFERS_FILES = False
+ _VALID_ARGS = frozenset(('msg',))
def run(self, tmp=None, task_vars=None):
if task_vars is None:
diff --git a/lib/ansible/plugins/action/group_by.py b/lib/ansible/plugins/action/group_by.py
index 58749d8470c..0958ad80e97 100644
--- a/lib/ansible/plugins/action/group_by.py
+++ b/lib/ansible/plugins/action/group_by.py
@@ -26,6 +26,7 @@ class ActionModule(ActionBase):
# We need to be able to modify the inventory
TRANSFERS_FILES = False
+ _VALID_ARGS = frozenset(('key', 'parents'))
def run(self, tmp=None, task_vars=None):
if task_vars is None:
diff --git a/lib/ansible/plugins/action/pause.py b/lib/ansible/plugins/action/pause.py
index f021f793adb..a347ad9ddf4 100644
--- a/lib/ansible/plugins/action/pause.py
+++ b/lib/ansible/plugins/action/pause.py
@@ -73,8 +73,8 @@ def clear_line(stdout):
class ActionModule(ActionBase):
''' pauses execution for a length or time, or until input is received '''
- PAUSE_TYPES = ['seconds', 'minutes', 'prompt', 'echo', '']
BYPASS_HOST_LOOP = True
+ _VALID_ARGS = frozenset(('echo', 'minutes', 'prompt', 'seconds'))
def run(self, tmp=None, task_vars=None):
''' run the pause action module '''
@@ -100,11 +100,6 @@ class ActionModule(ActionBase):
echo=echo
))
- if not set(self._task.args.keys()) <= set(self.PAUSE_TYPES):
- result['failed'] = True
- result['msg'] = "Invalid argument given. Must be one of: %s" % ", ".join(self.PAUSE_TYPES)
- return result
-
# Should keystrokes be echoed to stdout?
if 'echo' in self._task.args:
try:
diff --git a/lib/ansible/plugins/action/reboot.py b/lib/ansible/plugins/action/reboot.py
index bd9f1f60dd0..9abdd63b45e 100644
--- a/lib/ansible/plugins/action/reboot.py
+++ b/lib/ansible/plugins/action/reboot.py
@@ -28,6 +28,7 @@ class TimedOutException(Exception):
class ActionModule(ActionBase):
TRANSFERS_FILES = False
+ _VALID_ARGS = frozenset(('connect_timeout', 'msg', 'post_reboot_delay', 'pre_reboot_delay', 'test_command'))
DEFAULT_REBOOT_TIMEOUT = 600
DEFAULT_CONNECT_TIMEOUT = None
diff --git a/lib/ansible/plugins/action/set_stats.py b/lib/ansible/plugins/action/set_stats.py
index 6ccb12fd76a..f9fe8b30148 100644
--- a/lib/ansible/plugins/action/set_stats.py
+++ b/lib/ansible/plugins/action/set_stats.py
@@ -27,6 +27,7 @@ from ansible.utils.vars import isidentifier
class ActionModule(ActionBase):
TRANSFERS_FILES = False
+ _VALID_ARGS = frozenset(('aggregate', 'data', 'per_host'))
# TODO: document this in non-empty set_stats.py module
def run(self, tmp=None, task_vars=None):
diff --git a/lib/ansible/plugins/action/wait_for_connection.py b/lib/ansible/plugins/action/wait_for_connection.py
index 3badee968e5..0e860a86d79 100644
--- a/lib/ansible/plugins/action/wait_for_connection.py
+++ b/lib/ansible/plugins/action/wait_for_connection.py
@@ -37,6 +37,7 @@ class TimedOutException(Exception):
class ActionModule(ActionBase):
TRANSFERS_FILES = False
+ _VALID_ARGS = frozenset(('connect_timeout', 'delay', 'sleep', 'timeout'))
DEFAULT_CONNECT_TIMEOUT = 5
DEFAULT_DELAY = 0
diff --git a/lib/ansible/plugins/action/win_reboot.py b/lib/ansible/plugins/action/win_reboot.py
index 7ffcf044c89..be015389e15 100644
--- a/lib/ansible/plugins/action/win_reboot.py
+++ b/lib/ansible/plugins/action/win_reboot.py
@@ -24,6 +24,10 @@ class TimedOutException(Exception):
class ActionModule(RebootActionModule, ActionBase):
TRANSFERS_FILES = False
+ _VALID_ARGS = frozenset((
+ 'connect_timeout', 'connect_timeout_sec', 'msg', 'post_reboot_delay', 'post_reboot_delay_sec', 'pre_reboot_delay', 'pre_reboot_delay_sec',
+ 'reboot_timeout', 'reboot_timeout_sec', 'shutdown_timeout', 'shutdown_timeout_sec', 'test_command',
+ ))
DEFAULT_CONNECT_TIMEOUT = 5
DEFAULT_PRE_REBOOT_DELAY = 2
diff --git a/test/integration/targets/reboot/tasks/main.yml b/test/integration/targets/reboot/tasks/main.yml
index 8429a09a8b4..59062b7f887 100644
--- a/test/integration/targets/reboot/tasks/main.yml
+++ b/test/integration/targets/reboot/tasks/main.yml
@@ -34,13 +34,25 @@
command: who -b
register: after_boot_time
- - name: Enusure system was actually rebooted
+ - name: Ensure system was actually rebooted
assert:
that:
- reboot_result is changed
- reboot_result.elapsed > 10
- before_boot_time.stdout != after_boot_time.stdout
+ - name: Use invalid parameter
+ reboot:
+ foo: bar
+ ignore_errors: yes
+ register: invalid_parameter
+
+ - name: Ensure task fails with error
+ assert:
+ that:
+ - invalid_parameter is failed
+ - "invalid_parameter.msg == 'Invalid options for reboot: foo'"
+
always:
- name: Cleanup temp file
file:
diff --git a/test/integration/targets/wait_for_connection/tasks/main.yml b/test/integration/targets/wait_for_connection/tasks/main.yml
index 07bf56f6ded..613209e84cd 100644
--- a/test/integration/targets/wait_for_connection/tasks/main.yml
+++ b/test/integration/targets/wait_for_connection/tasks/main.yml
@@ -3,3 +3,15 @@
connect_timeout: 5
sleep: 1
timeout: 10
+
+- name: Use invalid parameter
+ wait_for_connection:
+ foo: bar
+ ignore_errors: yes
+ register: invalid_parameter
+
+- name: Ensure task fails with error
+ assert:
+ that:
+ - invalid_parameter is failed
+ - "invalid_parameter.msg == 'Invalid options for wait_for_connection: foo'"
diff --git a/test/integration/targets/win_reboot/tasks/main.yml b/test/integration/targets/win_reboot/tasks/main.yml
index c827e6bfc35..6a465b89212 100644
--- a/test/integration/targets/win_reboot/tasks/main.yml
+++ b/test/integration/targets/win_reboot/tasks/main.yml
@@ -76,3 +76,15 @@
win_user:
name: '{{standard_user}}'
state: absent
+
+- name: Use invalid parameter
+ reboot:
+ foo: bar
+ ignore_errors: yes
+ register: invalid_parameter
+
+- name: Ensure task fails with error
+ assert:
+ that:
+ - invalid_parameter is failed
+ - "invalid_parameter.msg == 'Invalid options for reboot: foo'"
diff --git a/test/integration/targets/yum_repository/tasks/yum_repository_centos.yml b/test/integration/targets/yum_repository/tasks/yum_repository_centos.yml
index 2495e1e2819..13f3142205d 100644
--- a/test/integration/targets/yum_repository/tasks/yum_repository_centos.yml
+++ b/test/integration/targets/yum_repository/tasks/yum_repository_centos.yml
@@ -171,7 +171,6 @@
- "'RPM-GPG2-KEY-EPEL' in repofile"
- "'aaa bbb' in repofile"
- "'ccc ddd' in repofile"
- value:
- name: Cleanup list test repo
yum_repository: