From 01ba3a4efcb7aa830e3c726de58a0690756dc6f1 Mon Sep 17 00:00:00 2001 From: Dag Wieers Date: Fri, 9 Feb 2018 21:21:56 +0100 Subject: [PATCH] ACI: Fixes to recent change to parameter choices (#35968) This PR includes: - Fixes related to the recent merge of #31637 and #34537 - A generic fix for a reference for assignment issue - Fixes to aci.boolean() in order to catch exception --- lib/ansible/module_utils/network/aci/aci.py | 34 ++++++++++++------- .../modules/network/aci/aci_aaa_user.py | 13 +++---- .../modules/network/aci/aci_bd_subnet.py | 17 ++++------ .../network/aci/aci_interface_policy_l2.py | 3 +- .../network/aci/aci_interface_policy_lldp.py | 3 +- .../network/aci/aci_interface_policy_mcp.py | 3 +- .../network/aci/aci_tenant_span_src_group.py | 3 +- .../targets/aci_bd_subnet/tasks/main.yml | 2 +- 8 files changed, 42 insertions(+), 36 deletions(-) diff --git a/lib/ansible/module_utils/network/aci/aci.py b/lib/ansible/module_utils/network/aci/aci.py index c05ad7a65c8b01..e96fcd43e99be3 100644 --- a/lib/ansible/module_utils/network/aci/aci.py +++ b/lib/ansible/module_utils/network/aci/aci.py @@ -169,24 +169,34 @@ def __init__(self, module): def boolean(self, value, true='yes', false='no'): ''' Return an acceptable value back ''' + + # When we expect value is of type=bool if value is None: return None elif value is True: return true elif value is False: return false - elif boolean(value) is True: # When type=raw, this supports Ansible booleans - return true - elif boolean(value) is False: # When type=raw, this supports Ansible booleans - return false - elif value == true: # When type=raw, this supports the original boolean values - self.module.deprecate("Boolean value '%s' is no longer valid, please use 'yes' as a boolean value." % value, '2.9') - return true - elif value == false: # When type=raw, this supports the original boolean values - self.module.deprecate("Boolean value '%s' is no longer valid, please use 'no' as a boolean value." % value, '2.9') - return false - else: # When type=raw, escalate back to user - self.module.fail_json(msg="Boolean value '%s' is an invalid ACI boolean value.") + + # When we expect value is of type=raw, deprecate in Ansible v2.8 (and all modules use type=bool) + try: + # This supports all Ansible boolean types + bool_value = boolean(value) + if bool_value is True: + return true + elif bool_value is False: + return false + except: + # This provides backward compatibility to Ansible v2.4, deprecate in Ansible v2.8 + if value == true: + self.module.deprecate("Boolean value '%s' is no longer valid, please use 'yes' as a boolean value." % value, '2.9') + return true + elif value == false: + self.module.deprecate("Boolean value '%s' is no longer valid, please use 'no' as a boolean value." % value, '2.9') + return false + + # If all else fails, escalate back to user + self.module.fail_json(msg="Boolean value '%s' is an invalid ACI boolean value.") def iso8601_format(self, dt): ''' Return an ACI-compatible ISO8601 formatted time: 2123-12-12T00:00:00.000+00:00 ''' diff --git a/lib/ansible/modules/network/aci/aci_aaa_user.py b/lib/ansible/modules/network/aci/aci_aaa_user.py index 98c1eaad8f4d78..3061cbefa16d28 100644 --- a/lib/ansible/modules/network/aci/aci_aaa_user.py +++ b/lib/ansible/modules/network/aci/aci_aaa_user.py @@ -269,28 +269,25 @@ def main(): ], ) + aci = ACIModule(module) + if not HAS_DATEUTIL: module.fail_json(msg='dateutil required for this module') aaa_password = module.params['aaa_password'] aaa_password_lifetime = module.params['aaa_password_lifetime'] - aaa_password_update_required = module.params['aaa_password_update_required'] + aaa_password_update_required = aci.boolean(module.params['aaa_password_update_required']) aaa_user = module.params['aaa_user'] clear_password_history = module.params['clear_password_history'] description = module.params['description'] email = module.params['email'] - enabled = module.params['enabled'] + enabled = aci.boolean(module.params['enabled'], 'active', 'inactive') + expires = aci.boolean(module.params['expires']) first_name = module.params['first_name'] last_name = module.params['last_name'] phone = module.params['phone'] state = module.params['state'] - aci = ACIModule(module) - - aaa_password_update_required = aci.boolean(module.params['aaa_password_update_required']) - enabled = aci.boolean(module.params['enabled'], 'active', 'inactive') - expires = aci.boolean(module.params['expires']) - expiration = module.params['expiration'] if expiration is not None and expiration != 'never': try: diff --git a/lib/ansible/modules/network/aci/aci_bd_subnet.py b/lib/ansible/modules/network/aci/aci_bd_subnet.py index 362c398e4b8109..29f6d17c2db44e 100644 --- a/lib/ansible/modules/network/aci/aci_bd_subnet.py +++ b/lib/ansible/modules/network/aci/aci_bd_subnet.py @@ -310,7 +310,7 @@ from ansible.module_utils.network.aci.aci import ACIModule, aci_argument_spec -from ansible.module_utils.basic import AnsibleModule +from ansible.module_utils.basic import AnsibleModule, SEQUENCETYPE def main(): @@ -326,10 +326,7 @@ def main(): preferred=dict(type='bool'), route_profile=dict(type='str'), route_profile_l3_out=dict(type='str'), - scope=dict( - type='list', - choices=[['private'], ['public'], ['shared'], ['private', 'shared'], ['shared', 'private'], ['public', 'shared'], ['shared', 'public']], - ), + scope=dict(type='list', choices=['private', 'public', 'shared']), subnet_control=dict(type='str', choices=['nd_ra', 'no_gw', 'querier_ip', 'unspecified']), state=dict(type='str', default='present', choices=['absent', 'present', 'query']), tenant=dict(type='str', aliases=['tenant_name']), @@ -366,13 +363,11 @@ def main(): route_profile = module.params['route_profile'] route_profile_l3_out = module.params['route_profile_l3_out'] scope = module.params['scope'] - if scope: - if len(scope) == 1: - scope = scope[0] - elif 'public' in scope: - scope = 'public,shared' + if isinstance(scope, SEQUENCETYPE): + if 'private' in scope and 'public' in scope: + module.fail_json(msg="Parameter 'scope' cannot be both 'private' and 'public', got: %s" % scope) else: - scope = 'private,shared' + scope = ','.join(sorted(scope)) state = module.params['state'] subnet_control = module.params['subnet_control'] if subnet_control: diff --git a/lib/ansible/modules/network/aci/aci_interface_policy_l2.py b/lib/ansible/modules/network/aci/aci_interface_policy_l2.py index 2e2f62c6a92958..5649ce385ed038 100644 --- a/lib/ansible/modules/network/aci/aci_interface_policy_l2.py +++ b/lib/ansible/modules/network/aci/aci_interface_policy_l2.py @@ -199,6 +199,8 @@ def main(): ], ) + aci = ACIModule(module) + l2_policy = module.params['l2_policy'] vlan_scope = module.params['vlan_scope'] qinq = module.params['qinq'] @@ -208,7 +210,6 @@ def main(): description = module.params['description'] state = module.params['state'] - aci = ACIModule(module) aci.construct_url( root_class=dict( aci_class='l2IfPol', diff --git a/lib/ansible/modules/network/aci/aci_interface_policy_lldp.py b/lib/ansible/modules/network/aci/aci_interface_policy_lldp.py index ee752f9f2c30e3..f49c931689d902 100644 --- a/lib/ansible/modules/network/aci/aci_interface_policy_lldp.py +++ b/lib/ansible/modules/network/aci/aci_interface_policy_lldp.py @@ -194,13 +194,14 @@ def main(): ], ) + aci = ACIModule(module) + lldp_policy = module.params['lldp_policy'] description = module.params['description'] receive_state = aci.boolean(module.params['receive_state'], 'enabled', 'disabled') transmit_state = aci.boolean(module.params['transmit_state'], 'enabled', 'disabled') state = module.params['state'] - aci = ACIModule(module) aci.construct_url( root_class=dict( aci_class='lldpIfPol', diff --git a/lib/ansible/modules/network/aci/aci_interface_policy_mcp.py b/lib/ansible/modules/network/aci/aci_interface_policy_mcp.py index c75ee70111f0f0..352df42a9397dd 100644 --- a/lib/ansible/modules/network/aci/aci_interface_policy_mcp.py +++ b/lib/ansible/modules/network/aci/aci_interface_policy_mcp.py @@ -185,12 +185,13 @@ def main(): ], ) + aci = ACIModule(module) + mcp = module.params['mcp'] description = module.params['description'] admin_state = aci.boolean(module.params['admin_state'], 'enabled', 'disabled') state = module.params['state'] - aci = ACIModule(module) aci.construct_url( root_class=dict( aci_class='mcpIfPol', diff --git a/lib/ansible/modules/network/aci/aci_tenant_span_src_group.py b/lib/ansible/modules/network/aci/aci_tenant_span_src_group.py index 3dd968523378f3..25f3108c98f367 100755 --- a/lib/ansible/modules/network/aci/aci_tenant_span_src_group.py +++ b/lib/ansible/modules/network/aci/aci_tenant_span_src_group.py @@ -197,6 +197,8 @@ def main(): ], ) + aci = ACIModule(module) + admin_state = aci.boolean(module.params['admin_state'], 'enabled', 'disabled') description = module.params['description'] dst_group = module.params['dst_group'] @@ -204,7 +206,6 @@ def main(): state = module.params['state'] tenant = module.params['tenant'] - aci = ACIModule(module) aci.construct_url( root_class=dict( aci_class='fvTenant', diff --git a/test/integration/targets/aci_bd_subnet/tasks/main.yml b/test/integration/targets/aci_bd_subnet/tasks/main.yml index bd07b44e17753f..fde918afe4f4be 100644 --- a/test/integration/targets/aci_bd_subnet/tasks/main.yml +++ b/test/integration/targets/aci_bd_subnet/tasks/main.yml @@ -109,7 +109,7 @@ - modify_subnet.changed != modify_subnet.proposed - 'modify_subnet.sent == {"fvSubnet": {"attributes": {"ctrl": "querier", "scope": "public,shared"}}}' - create_bad_scope.failed == true - - 'create_bad_scope.msg.startswith("value of scope must be one of")' + - create_bad_scope.msg.startswith("Parameter 'scope' cannot be both 'private' and 'public'") - create_incomplete_data.failed == true - 'create_incomplete_data.msg == "state is present but all of the following are missing: bd"'