Introduce new 'required_by' argument_spec option (#28662)

* Introduce new "required_by' argument_spec option

This PR introduces a new **required_by** argument_spec option which allows you to say *"if parameter A is set, parameter B and C are required as well"*.

- The difference with **required_if** is that it can only add dependencies if a parameter is set to a specific value, not when it is just defined.
- The difference with **required_together** is that it has a commutative property, so: *"Parameter A and B are required together, if one of them has been defined"*.

As an example, we need this for the complex options that the xml module provides. One of the issues we often see is that users are not using the correct combination of options, and then are surprised that the module does not perform the requested action(s).

This would be solved by adding the correct dependencies, and mutual exclusives. For us this is important to get this shipped together with the new xml module in Ansible v2.4. (This is related to bugfix https://github.com/ansible/ansible/pull/28657)

```python
    module = AnsibleModule(
        argument_spec=dict(
            path=dict(type='path', aliases=['dest', 'file']),
            xmlstring=dict(type='str'),
            xpath=dict(type='str'),
            namespaces=dict(type='dict', default={}),
            state=dict(type='str', default='present', choices=['absent',
'present'], aliases=['ensure']),
            value=dict(type='raw'),
            attribute=dict(type='raw'),
            add_children=dict(type='list'),
            set_children=dict(type='list'),
            count=dict(type='bool', default=False),
            print_match=dict(type='bool', default=False),
            pretty_print=dict(type='bool', default=False),
            content=dict(type='str', choices=['attribute', 'text']),
            input_type=dict(type='str', default='yaml', choices=['xml',
'yaml']),
            backup=dict(type='bool', default=False),
        ),
        supports_check_mode=True,
        required_by=dict(
            add_children=['xpath'],
            attribute=['value', 'xpath'],
            content=['xpath'],
            set_children=['xpath'],
            value=['xpath'],
        ),
        required_if=[
            ['count', True, ['xpath']],
            ['print_match', True, ['xpath']],
        ],
        required_one_of=[
            ['path', 'xmlstring'],
            ['add_children', 'content', 'count', 'pretty_print', 'print_match', 'set_children', 'value'],
        ],
        mutually_exclusive=[
            ['add_children', 'content', 'count', 'print_match','set_children', 'value'],
            ['path', 'xmlstring'],
        ],
    )
```

* Rebase and fix conflict

* Add modules that use required_by functionality

* Update required_by schema

* Fix rebase issue
pull/52198/head
Dag Wieers 5 years ago committed by Jordan Borean
parent be9c75703a
commit cd9471ef17

@ -748,7 +748,7 @@ class AnsibleModule(object):
def __init__(self, argument_spec, bypass_checks=False, no_log=False,
check_invalid_arguments=None, mutually_exclusive=None, required_together=None,
required_one_of=None, add_file_common_args=False, supports_check_mode=False,
required_if=None):
required_if=None, required_by=None):
'''
Common code for quickly building an ansible module in Python
@ -777,6 +777,7 @@ class AnsibleModule(object):
self.required_together = required_together
self.required_one_of = required_one_of
self.required_if = required_if
self.required_by = required_by
self.cleanup_files = []
self._debug = False
self._diff = False
@ -848,6 +849,7 @@ class AnsibleModule(object):
self._check_required_together(required_together)
self._check_required_one_of(required_one_of)
self._check_required_if(required_if)
self._check_required_by(required_by)
self._set_defaults(pre=False)
@ -1706,6 +1708,24 @@ class AnsibleModule(object):
msg += " found in %s" % " -> ".join(self._options_context)
self.fail_json(msg=msg)
def _check_required_by(self, spec, param=None):
if spec is None:
return
if param is None:
param = self.params
for (key, value) in spec.items():
if key not in param or param[key] is None:
continue
missing = []
# Support strings (single-item lists)
if isinstance(value, string_types):
value = [value, ]
for required in value:
if required not in param or param[required] is None:
missing.append(required)
if len(missing) > 0:
self.fail_json(msg="missing parameter(s) required by '%s': %s" % (key, ', '.join(missing)))
def _check_required_arguments(self, spec=None, param=None):
''' ensure all required arguments are present '''
missing = []
@ -2008,6 +2028,7 @@ class AnsibleModule(object):
self._check_required_together(v.get('required_together', None), param)
self._check_required_one_of(v.get('required_one_of', None), param)
self._check_required_if(v.get('required_if', None), param)
self._check_required_by(v.get('required_by', None), param)
self._set_defaults(pre=False, spec=spec, param=param)

@ -385,7 +385,10 @@ def main():
module = AnsibleModule(
argument_spec=argument_spec,
supports_check_mode=True
supports_check_mode=True,
required_together=[
['device_id', 'private_ip_address'],
],
)
if not HAS_BOTO:
@ -404,10 +407,6 @@ def main():
release_on_disassociation = module.params.get('release_on_disassociation')
allow_reassociation = module.params.get('allow_reassociation')
# Parameter checks
if private_ip_address is not None and device_id is None:
module.fail_json(msg="parameters are required together: ('device_id', 'private_ip_address')")
if instance_id:
warnings = ["instance_id is no longer used, please use device_id going forward"]
is_instance = True

@ -1,4 +1,5 @@
#!/usr/bin/python
# -*- coding: utf-8 -*-
# Copyright: (c) 2018, Ansible Project
# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
@ -459,41 +460,50 @@ def invoke_with_throttling_retries(function_ref, *argv, **kwargs):
def main():
argument_spec = ec2_argument_spec()
argument_spec.update(dict(
state=dict(aliases=['command'], choices=['present', 'absent', 'get', 'create', 'delete'], required=True),
zone=dict(required=True),
hosted_zone_id=dict(required=False, default=None),
record=dict(required=True),
ttl=dict(required=False, type='int', default=3600),
type=dict(choices=['A', 'CNAME', 'MX', 'AAAA', 'TXT', 'PTR', 'SRV', 'SPF', 'CAA', 'NS', 'SOA'], required=True),
alias=dict(required=False, type='bool'),
alias_hosted_zone_id=dict(required=False),
alias_evaluate_target_health=dict(required=False, type='bool', default=False),
value=dict(required=False, type='list'),
overwrite=dict(required=False, type='bool'),
retry_interval=dict(required=False, default=500),
private_zone=dict(required=False, type='bool', default=False),
identifier=dict(required=False, default=None),
weight=dict(required=False, type='int'),
region=dict(required=False),
health_check=dict(required=False),
failover=dict(required=False, choices=['PRIMARY', 'SECONDARY']),
vpc_id=dict(required=False),
wait=dict(required=False, type='bool', default=False),
wait_timeout=dict(required=False, type='int', default=300),
state=dict(type='str', required=True, choices=['absent', 'create', 'delete', 'get', 'present'], aliases=['command']),
zone=dict(type='str', required=True),
hosted_zone_id=dict(type='str', ),
record=dict(type='str', required=True),
ttl=dict(type='int', default=3600),
type=dict(type='str', required=True, choices=['A', 'AAAA', 'CAA', 'CNAME', 'MX', 'NS', 'PTR', 'SOA', 'SPF', 'SRV', 'TXT']),
alias=dict(type='bool'),
alias_hosted_zone_id=dict(type='str'),
alias_evaluate_target_health=dict(type='bool', default=False),
value=dict(type='list'),
overwrite=dict(type='bool'),
retry_interval=dict(type='int', default=500),
private_zone=dict(type='bool', default=False),
identifier=dict(type='str'),
weight=dict(type='int'),
region=dict(type='str'),
health_check=dict(type='str'),
failover=dict(type='str', choices=['PRIMARY', 'SECONDARY']),
vpc_id=dict(type='str'),
wait=dict(type='bool', default=False),
wait_timeout=dict(type='int', default=300),
))
# state=present, absent, create, delete THEN value is required
required_if = [('state', 'present', ['value']), ('state', 'create', ['value'])]
required_if.extend([('state', 'absent', ['value']), ('state', 'delete', ['value'])])
# If alias is True then you must specify alias_hosted_zone as well
required_together = [['alias', 'alias_hosted_zone_id']]
# failover, region, and weight are mutually exclusive
mutually_exclusive = [('failover', 'region', 'weight')]
module = AnsibleModule(argument_spec=argument_spec, required_together=required_together, required_if=required_if,
mutually_exclusive=mutually_exclusive, supports_check_mode=True)
module = AnsibleModule(
argument_spec=argument_spec,
supports_check_mode=True,
# If alias is True then you must specify alias_hosted_zone as well
required_together=[['alias', 'alias_hosted_zone_id']],
# state=present, absent, create, delete THEN value is required
required_if=(
('state', 'present', ['value']),
('state', 'create', ['value']),
('state', 'absent', ['value']),
('state', 'delete', ['value']),
),
# failover, region and weight are mutually exclusive
mutually_exclusive=[('failover', 'region', 'weight')],
# failover, region and weight require identifier
required_by=dict(
failover=('identifier'),
region=('identifier'),
weight=('identifier'),
),
)
if not HAS_BOTO:
module.fail_json(msg='boto required for this module')
@ -544,8 +554,6 @@ def main():
if command_in == 'create' or command_in == 'delete':
if alias_in and len(value_in) != 1:
module.fail_json(msg="parameter 'value' must contain a single dns name for alias records")
if (weight_in is not None or region_in is not None or failover_in is not None) and identifier_in is None:
module.fail_json(msg="If you specify failover, region or weight you must also specify identifier")
if (weight_in is None and region_in is None and failover_in is None) and identifier_in is not None:
module.fail_json(msg="You have specified identifier which makes sense only if you specify one of: weight, region or failover.")

@ -223,28 +223,31 @@ def _system_state_change(module, subnet, cloud, filters=None):
def main():
ipv6_mode_choices = ['dhcpv6-stateful', 'dhcpv6-stateless', 'slaac']
argument_spec = openstack_full_argument_spec(
name=dict(required=True),
network_name=dict(default=None),
cidr=dict(default=None),
ip_version=dict(default='4', choices=['4', '6']),
enable_dhcp=dict(default='true', type='bool'),
gateway_ip=dict(default=None),
no_gateway_ip=dict(default=False, type='bool'),
dns_nameservers=dict(default=None, type='list'),
allocation_pool_start=dict(default=None),
allocation_pool_end=dict(default=None),
host_routes=dict(default=None, type='list'),
ipv6_ra_mode=dict(default=None, choice=ipv6_mode_choices),
ipv6_address_mode=dict(default=None, choice=ipv6_mode_choices),
use_default_subnetpool=dict(default=False, type='bool'),
extra_specs=dict(required=False, default=dict(), type='dict'),
state=dict(default='present', choices=['absent', 'present']),
project=dict(default=None)
name=dict(type='str', required=True),
network_name=dict(type='str'),
cidr=dict(type='str'),
ip_version=dict(type='str', default='4', choices=['4', '6']),
enable_dhcp=dict(type='bool', default=True),
gateway_ip=dict(type='str'),
no_gateway_ip=dict(type='bool', default=False),
dns_nameservers=dict(type='list', default=None),
allocation_pool_start=dict(type='str'),
allocation_pool_end=dict(type='str'),
host_routes=dict(type='list', default=None),
ipv6_ra_mode=dict(type='str', choice=ipv6_mode_choices),
ipv6_address_mode=dict(type='str', choice=ipv6_mode_choices),
use_default_subnetpool=dict(type='bool', default=False),
extra_specs=dict(type='dict', default=dict()),
state=dict(type='str', default='present', choices=['absent', 'present']),
project=dict(type='str'),
)
module_kwargs = openstack_module_kwargs()
module = AnsibleModule(argument_spec,
supports_check_mode=True,
required_together=[
['allocation_pool_end', 'allocation_pool_start'],
],
**module_kwargs)
state = module.params['state']
@ -275,8 +278,6 @@ def main():
if pool_start and pool_end:
pool = [dict(start=pool_start, end=pool_end)]
elif pool_start or pool_end:
module.fail_json(msg='allocation pool requires start and end values')
else:
pool = None

@ -831,16 +831,14 @@ def main():
insertafter=dict(type='bool', default=False),
),
supports_check_mode=True,
# TODO: Implement this as soon as #28662 (required_by functionality) is merged
# required_by=dict(
# add_children=['xpath'],
# attribute=['value'],
# set_children=['xpath'],
# value=['xpath'],
# ),
required_by=dict(
add_children=['xpath'],
attribute=['value'],
content=['xpath'],
set_children=['xpath'],
value=['xpath'],
),
required_if=[
['content', 'attribute', ['xpath']],
['content', 'text', ['xpath']],
['count', True, ['xpath']],
['print_match', True, ['xpath']],
['insertbefore', True, ['xpath']],

@ -98,7 +98,7 @@ options:
special_time:
description:
- Special time specification nickname.
choices: [ reboot, yearly, annually, monthly, weekly, daily, hourly ]
choices: [ annually, daily, hourly, monthly, reboot, weekly, yearly ]
version_added: "1.3"
disabled:
description:
@ -566,6 +566,12 @@ def main():
['reboot', 'special_time'],
['insertafter', 'insertbefore'],
],
required_by=dict(
cron_file=('user'),
),
required_if=(
('state', 'present', ('job')),
),
)
name = module.params['name']
@ -624,13 +630,6 @@ def main():
if (special_time or reboot) and get_platform() == 'SunOS':
module.fail_json(msg="Solaris does not support special_time=... or @reboot")
if cron_file and do_install:
if not user:
module.fail_json(msg="To use cron_file=... parameter you must specify user=... as well")
if job is None and do_install:
module.fail_json(msg="You must specify 'job' to install a new cron job or variable")
if (insertafter or insertbefore) and not env and do_install:
module.fail_json(msg="Insertafter and insertbefore parameters are valid only with env=yes")

@ -647,7 +647,11 @@ def main():
masquerade=dict(type='str'),
offline=dict(type='bool'),
),
supports_check_mode=True
supports_check_mode=True,
required_by=dict(
interface=('zone'),
source=('permanent'),
),
)
permanent = module.params['permanent']

@ -1,7 +1,7 @@
#!/usr/bin/python
# -*- coding: utf-8 -*-
#
# (c) 2016, Roman Belyakovsky <ihryamzik () gmail.com>
# Copyright: (c) 2016, Roman Belyakovsky <ihryamzik () gmail.com>
# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
from __future__ import absolute_import, division, print_function
@ -350,16 +350,19 @@ def write_changes(module, lines, dest):
def main():
module = AnsibleModule(
argument_spec=dict(
dest=dict(default='/etc/network/interfaces', required=False, type='path'),
iface=dict(required=False),
address_family=dict(required=False),
option=dict(required=False),
value=dict(required=False),
backup=dict(default='no', type='bool'),
state=dict(default='present', choices=['present', 'absent']),
dest=dict(type='path', default='/etc/network/interfaces'),
iface=dict(type='str'),
address_family=dict(type='str'),
option=dict(type='str'),
value=dict(type='str'),
backup=dict(type='bool', default=False),
state=dict(type='str', default='present', choices=['absent', 'present']),
),
add_file_common_args=True,
supports_check_mode=True
supports_check_mode=True,
required_by=dict(
option=('iface'),
),
)
dest = module.params['dest']
@ -370,9 +373,6 @@ def main():
backup = module.params['backup']
state = module.params['state']
if option is not None and iface is None:
module.fail_json(msg="Inteface must be set if option is defined")
if option is not None and state == "present" and value is None:
module.fail_json(msg="Value must be set if option is defined and state is 'present'")

@ -327,6 +327,11 @@ def main():
),
supports_check_mode=True,
required_one_of=[['state', 'enabled', 'masked', 'daemon_reload']],
required_by=dict(
state=('name', ),
enabled=('name', ),
masked=('name', ),
),
mutually_exclusive=[['scope', 'user']],
)
@ -364,10 +369,6 @@ def main():
status=dict(),
)
for requires in ('state', 'enabled', 'masked'):
if module.params[requires] is not None and unit is None:
module.fail_json(msg="name is also required when specifying %s" % requires)
# Run daemon-reload first, if requested
if module.params['daemon_reload'] and not module.check_mode:
(rc, out, err) = module.run_command("%s daemon-reload" % (systemctl))

@ -290,6 +290,8 @@ def compile_ipv6_regexp():
def main():
command_keys = ['state', 'default', 'rule', 'logging']
module = AnsibleModule(
argument_spec=dict(
state=dict(type='str', choices=['enabled', 'disabled', 'reloaded', 'reset']),
@ -313,8 +315,12 @@ def main():
),
supports_check_mode=True,
mutually_exclusive=[
['app', 'proto', 'logging']
['app', 'proto', 'logging'],
],
required_one_of=([command_keys]),
required_by=dict(
interface=('direction', ),
),
)
cmds = []
@ -395,16 +401,8 @@ def main():
params = module.params
# Ensure at least one of the command arguments are given
command_keys = ['state', 'default', 'rule', 'logging']
commands = dict((key, params[key]) for key in command_keys if params[key])
if len(commands) < 1:
module.fail_json(msg="Not any of the command arguments %s given" % commands)
if (params['interface'] is not None and params['direction'] is None):
module.fail_json(msg="Direction must be specified when creating a rule on an interface")
# Ensure ufw is available
ufw_bin = module.get_bin_path('ufw', True)
grep_bin = module.get_bin_path('grep', True)

@ -9,6 +9,7 @@ from voluptuous import ALLOW_EXTRA, PREVENT_EXTRA, All, Any, Length, Invalid, Re
from ansible.module_utils.six import string_types
from ansible.module_utils.common.collections import is_iterable
list_string_types = list(string_types)
tuple_string_types = tuple(string_types)
any_string_types = Any(*string_types)
# Valid DOCUMENTATION.author lines
@ -67,6 +68,7 @@ ansible_module_kwargs_schema = Schema(
'add_file_common_args': bool,
'supports_check_mode': bool,
'required_if': sequence_of_sequences(min=3),
'required_by': Schema({str: Any(list_string_types, tuple_string_types, *string_types)}),
}
)

@ -97,6 +97,7 @@ def options_argspec_list():
bam1=dict(),
bam2=dict(default='test'),
bam3=dict(type='bool'),
bam4=dict(type='str'),
)
arg_spec = dict(
@ -116,7 +117,10 @@ def options_argspec_list():
],
required_together=[
['bam1', 'baz']
]
],
required_by={
'bam4': ('bam1', 'bam3'),
},
)
)
@ -278,46 +282,54 @@ class TestComplexArgSpecs:
class TestComplexOptions:
"""Test arg spec options"""
# (Paramaters, expected value of module.params['foobar'])
# (Parameters, expected value of module.params['foobar'])
OPTIONS_PARAMS_LIST = (
({'foobar': [{"foo": "hello", "bam": "good"}, {"foo": "test", "bar": "good"}]},
[{'foo': 'hello', 'bam': 'good', 'bam2': 'test', 'bar': None, 'baz': None, 'bam1': None, 'bam3': None},
{'foo': 'test', 'bam': None, 'bam2': 'test', 'bar': 'good', 'baz': None, 'bam1': None, 'bam3': None},
[{'foo': 'hello', 'bam': 'good', 'bam2': 'test', 'bar': None, 'baz': None, 'bam1': None, 'bam3': None, 'bam4': None},
{'foo': 'test', 'bam': None, 'bam2': 'test', 'bar': 'good', 'baz': None, 'bam1': None, 'bam3': None, 'bam4': None},
]),
# Alias for required param
({'foobar': [{"dup": "test", "bar": "good"}]},
[{'foo': 'test', 'dup': 'test', 'bam': None, 'bam2': 'test', 'bar': 'good', 'baz': None, 'bam1': None, 'bam3': None}]
[{'foo': 'test', 'dup': 'test', 'bam': None, 'bam2': 'test', 'bar': 'good', 'baz': None, 'bam1': None, 'bam3': None, 'bam4': None}]
),
# Required_if utilizing default value of the requirement
({'foobar': [{"foo": "bam2", "bar": "required_one_of"}]},
[{'bam': None, 'bam1': None, 'bam2': 'test', 'bam3': None, 'bar': 'required_one_of', 'baz': None, 'foo': 'bam2'}]
[{'bam': None, 'bam1': None, 'bam2': 'test', 'bam3': None, 'bam4': None, 'bar': 'required_one_of', 'baz': None, 'foo': 'bam2'}]
),
# Check that a bool option is converted
({"foobar": [{"foo": "required", "bam": "good", "bam3": "yes"}]},
[{'bam': 'good', 'bam1': None, 'bam2': 'test', 'bam3': True, 'bar': None, 'baz': None, 'foo': 'required'}]
[{'bam': 'good', 'bam1': None, 'bam2': 'test', 'bam3': True, 'bam4': None, 'bar': None, 'baz': None, 'foo': 'required'}]
),
# Check required_by options
({"foobar": [{"foo": "required", "bar": "good", "baz": "good", "bam4": "required_by", "bam1": "ok", "bam3": "yes"}]},
[{'bar': 'good', 'baz': 'good', 'bam1': 'ok', 'bam2': 'test', 'bam3': True, 'bam4': 'required_by', 'bam': None, 'foo': 'required'}]
),
)
# (Paramaters, expected value of module.params['foobar'])
# (Parameters, expected value of module.params['foobar'])
OPTIONS_PARAMS_DICT = (
({'foobar': {"foo": "hello", "bam": "good"}},
{'foo': 'hello', 'bam': 'good', 'bam2': 'test', 'bar': None, 'baz': None, 'bam1': None, 'bam3': None}
{'foo': 'hello', 'bam': 'good', 'bam2': 'test', 'bar': None, 'baz': None, 'bam1': None, 'bam3': None, 'bam4': None}
),
# Alias for required param
({'foobar': {"dup": "test", "bar": "good"}},
{'foo': 'test', 'dup': 'test', 'bam': None, 'bam2': 'test', 'bar': 'good', 'baz': None, 'bam1': None, 'bam3': None}
{'foo': 'test', 'dup': 'test', 'bam': None, 'bam2': 'test', 'bar': 'good', 'baz': None, 'bam1': None, 'bam3': None, 'bam4': None}
),
# Required_if utilizing default value of the requirement
({'foobar': {"foo": "bam2", "bar": "required_one_of"}},
{'bam': None, 'bam1': None, 'bam2': 'test', 'bam3': None, 'bar': 'required_one_of', 'baz': None, 'foo': 'bam2'}
{'bam': None, 'bam1': None, 'bam2': 'test', 'bam3': None, 'bam4': None, 'bar': 'required_one_of', 'baz': None, 'foo': 'bam2'}
),
# Check that a bool option is converted
({"foobar": {"foo": "required", "bam": "good", "bam3": "yes"}},
{'bam': 'good', 'bam1': None, 'bam2': 'test', 'bam3': True, 'bar': None, 'baz': None, 'foo': 'required'}
{'bam': 'good', 'bam1': None, 'bam2': 'test', 'bam3': True, 'bam4': None, 'bar': None, 'baz': None, 'foo': 'required'}
),
# Check required_by options
({"foobar": {"foo": "required", "bar": "good", "baz": "good", "bam4": "required_by", "bam1": "ok", "bam3": "yes"}},
{'bar': 'good', 'baz': 'good', 'bam1': 'ok', 'bam2': 'test', 'bam3': True, 'bam4': 'required_by', 'bam': None, 'foo': 'required'}
),
)
# (Paramaters, failure message)
# (Parameters, failure message)
FAILING_PARAMS_LIST = (
# Missing required option
({'foobar': [{}]}, 'missing required arguments: foo found in foobar'),
@ -335,9 +347,12 @@ class TestComplexOptions:
# Missing required_together option
({'foobar': [{"foo": "test", "bar": "required_one_of", "bam1": "bad"}]},
'parameters are required together: bam1, baz found in foobar'),
# Missing required_by options
({'foobar': [{"foo": "test", "bar": "required_one_of", "bam4": "required_by"}]},
"missing parameter(s) required by 'bam4': bam1, bam3"),
)
# (Paramaters, failure message)
# (Parameters, failure message)
FAILING_PARAMS_DICT = (
# Missing required option
({'foobar': {}}, 'missing required arguments: foo found in foobar'),
@ -356,6 +371,9 @@ class TestComplexOptions:
# Missing required_together option
({'foobar': {"foo": "test", "bar": "required_one_of", "bam1": "bad"}},
'parameters are required together: bam1, baz found in foobar'),
# Missing required_by options
({'foobar': {"foo": "test", "bar": "required_one_of", "bam4": "required_by"}},
"missing parameter(s) required by 'bam4': bam1, bam3"),
)
@pytest.mark.parametrize('stdin, expected', OPTIONS_PARAMS_DICT, indirect=['stdin'])

Loading…
Cancel
Save