diff --git a/changelogs/fragments/1765-proxmox-params.yml b/changelogs/fragments/1765-proxmox-params.yml new file mode 100644 index 0000000000..fd6d63c788 --- /dev/null +++ b/changelogs/fragments/1765-proxmox-params.yml @@ -0,0 +1,2 @@ +bugfixes: + - proxmox* modules - refactored some parameter validation code into use of ``env_fallback``, ``required_if``, ``required_together``, ``required_one_of`` (https://github.com/ansible-collections/community.general/pull/1765). diff --git a/plugins/modules/cloud/misc/proxmox.py b/plugins/modules/cloud/misc/proxmox.py index c4cfd9b5e7..4f495da34e 100644 --- a/plugins/modules/cloud/misc/proxmox.py +++ b/plugins/modules/cloud/misc/proxmox.py @@ -345,7 +345,6 @@ EXAMPLES = r''' state: absent ''' -import os import time import traceback from distutils.version import LooseVersion @@ -356,7 +355,7 @@ try: except ImportError: HAS_PROXMOXER = False -from ansible.module_utils.basic import AnsibleModule +from ansible.module_utils.basic import AnsibleModule, env_fallback from ansible.module_utils._text import to_native @@ -481,7 +480,7 @@ def main(): module = AnsibleModule( argument_spec=dict( api_host=dict(required=True), - api_password=dict(no_log=True), + api_password=dict(no_log=True, fallback=(env_fallback, ['PROXMOX_PASSWORD'])), api_token_id=dict(no_log=True), api_token_secret=dict(no_log=True), api_user=dict(required=True), @@ -514,7 +513,10 @@ def main(): description=dict(type='str'), hookscript=dict(type='str'), proxmox_default_behavior=dict(type='str', choices=['compatibility', 'no_defaults']), - ) + ), + required_if=[('state', 'present', ['node', 'hostname', 'password', 'ostemplate'])], + required_together=[('api_token_id', 'api_token_secret')], + required_one_of=[('api_password', 'api_token_id')], ) if not HAS_PROXMOXER: @@ -561,13 +563,7 @@ def main(): module.params[param] = value auth_args = {'user': api_user} - if not (api_token_id and api_token_secret): - # If password not set get it from PROXMOX_PASSWORD env - if not api_password: - try: - api_password = os.environ['PROXMOX_PASSWORD'] - except KeyError as e: - module.fail_json(msg='You should set api_password param or use PROXMOX_PASSWORD environment variable') + if not api_token_id: auth_args['password'] = api_password else: auth_args['token_name'] = api_token_id @@ -599,8 +595,6 @@ def main(): # If no vmid was passed, there cannot be another VM named 'hostname' if not module.params['vmid'] and get_vmid(proxmox, hostname) and not module.params['force']: module.exit_json(changed=False, msg="VM with hostname %s already exists and has ID number %s" % (hostname, get_vmid(proxmox, hostname)[0])) - elif not (node, module.params['hostname'] and module.params['password'] and module.params['ostemplate']): - module.fail_json(msg='node, hostname, password and ostemplate are mandatory for creating vm') elif not node_check(proxmox, node): module.fail_json(msg="node '%s' not exists in cluster" % node) elif not content_check(proxmox, node, module.params['ostemplate'], template_store): diff --git a/plugins/modules/cloud/misc/proxmox_kvm.py b/plugins/modules/cloud/misc/proxmox_kvm.py index c239a8b85d..348725661b 100644 --- a/plugins/modules/cloud/misc/proxmox_kvm.py +++ b/plugins/modules/cloud/misc/proxmox_kvm.py @@ -769,7 +769,6 @@ status: }' ''' -import os import re import time import traceback @@ -782,7 +781,7 @@ try: except ImportError: HAS_PROXMOXER = False -from ansible.module_utils.basic import AnsibleModule +from ansible.module_utils.basic import AnsibleModule, env_fallback from ansible.module_utils._text import to_native @@ -1015,7 +1014,7 @@ def main(): agent=dict(type='bool'), args=dict(type='str'), api_host=dict(required=True), - api_password=dict(no_log=True), + api_password=dict(no_log=True, fallback=(env_fallback, ['PROXMOX_PASSWORD'])), api_token_id=dict(no_log=True), api_token_secret=dict(no_log=True), api_user=dict(required=True), @@ -1028,12 +1027,12 @@ def main(): cipassword=dict(type='str', no_log=True), citype=dict(type='str', choices=['nocloud', 'configdrive2']), ciuser=dict(type='str'), - clone=dict(type='str', default=None), + clone=dict(type='str'), cores=dict(type='int'), cpu=dict(type='str'), cpulimit=dict(type='int'), cpuunits=dict(type='int'), - delete=dict(type='str', default=None), + delete=dict(type='str'), description=dict(type='str'), digest=dict(type='str'), force=dict(type='bool'), @@ -1056,7 +1055,7 @@ def main(): name=dict(type='str'), nameservers=dict(type='list', elements='str'), net=dict(type='dict'), - newid=dict(type='int', default=None), + newid=dict(type='int'), node=dict(), numa=dict(type='dict'), numa_enabled=dict(type='bool'), @@ -1092,13 +1091,14 @@ def main(): vcpus=dict(type='int'), vga=dict(choices=['std', 'cirrus', 'vmware', 'qxl', 'serial0', 'serial1', 'serial2', 'serial3', 'qxl2', 'qxl3', 'qxl4']), virtio=dict(type='dict'), - vmid=dict(type='int', default=None), + vmid=dict(type='int'), watchdog=dict(), proxmox_default_behavior=dict(type='str', choices=['compatibility', 'no_defaults']), ), mutually_exclusive=[('delete', 'revert'), ('delete', 'update'), ('revert', 'update'), ('clone', 'update'), ('clone', 'delete'), ('clone', 'revert')], - required_one_of=[('name', 'vmid',)], - required_if=[('state', 'present', ['node'])] + required_together=[('api_token_id', 'api_token_secret')], + required_one_of=[('name', 'vmid'), ('api_password', 'api_token_id')], + required_if=[('state', 'present', ['node'])], ) if not HAS_PROXMOXER: @@ -1159,12 +1159,6 @@ def main(): auth_args = {'user': api_user} if not (api_token_id and api_token_secret): - # If password not set get it from PROXMOX_PASSWORD env - if not api_password: - try: - api_password = os.environ['PROXMOX_PASSWORD'] - except KeyError: - module.fail_json(msg='You should set api_password param or use PROXMOX_PASSWORD environment variable') auth_args['password'] = api_password else: auth_args['token_name'] = api_token_id diff --git a/plugins/modules/cloud/misc/proxmox_snap.py b/plugins/modules/cloud/misc/proxmox_snap.py index 7cf2f471a6..17c6ef335a 100644 --- a/plugins/modules/cloud/misc/proxmox_snap.py +++ b/plugins/modules/cloud/misc/proxmox_snap.py @@ -31,6 +31,7 @@ options: - The password to authenticate with. - You can use PROXMOX_PASSWORD environment variable. type: str + required: yes hostname: description: - The instance name. @@ -106,7 +107,6 @@ EXAMPLES = r''' RETURN = r'''#''' -import os import time import traceback @@ -118,7 +118,7 @@ except ImportError: PROXMOXER_IMP_ERR = traceback.format_exc() HAS_PROXMOXER = False -from ansible.module_utils.basic import AnsibleModule, missing_required_lib +from ansible.module_utils.basic import AnsibleModule, missing_required_lib, env_fallback from ansible.module_utils._text import to_native @@ -182,7 +182,7 @@ def main(): argument_spec=dict( api_host=dict(required=True), api_user=dict(required=True), - api_password=dict(no_log=True), + api_password=dict(no_log=True, required=True, fallback=(env_fallback, ['PROXMOX_PASSWORD'])), vmid=dict(required=False), validate_certs=dict(type='bool', default='no'), hostname=dict(), @@ -213,13 +213,6 @@ def main(): force = module.params['force'] vmstate = module.params['vmstate'] - # If password not set get it from PROXMOX_PASSWORD env - if not api_password: - try: - api_password = os.environ['PROXMOX_PASSWORD'] - except KeyError as e: - module.fail_json(msg='You should set api_password param or use PROXMOX_PASSWORD environment variable' % to_native(e)) - try: proxmox = setup_api(api_host, api_user, api_password, validate_certs) diff --git a/plugins/modules/cloud/misc/proxmox_template.py b/plugins/modules/cloud/misc/proxmox_template.py index 76228e9a44..d7fb9341e6 100644 --- a/plugins/modules/cloud/misc/proxmox_template.py +++ b/plugins/modules/cloud/misc/proxmox_template.py @@ -122,7 +122,7 @@ try: except ImportError: HAS_PROXMOXER = False -from ansible.module_utils.basic import AnsibleModule +from ansible.module_utils.basic import AnsibleModule, env_fallback def get_template(proxmox, node, storage, content_type, template): @@ -175,7 +175,7 @@ def main(): module = AnsibleModule( argument_spec=dict( api_host=dict(required=True), - api_password=dict(no_log=True), + api_password=dict(no_log=True, fallback=(env_fallback, ['PROXMOX_PASSWORD'])), api_token_id=dict(no_log=True), api_token_secret=dict(no_log=True), api_user=dict(required=True), @@ -188,7 +188,10 @@ def main(): timeout=dict(type='int', default=30), force=dict(type='bool', default=False), state=dict(default='present', choices=['present', 'absent']), - ) + ), + required_together=[('api_token_id', 'api_token_secret')], + required_one_of=[('api_password', 'api_token_id')], + required_if=[('state', 'absent', ['template'])] ) if not HAS_PROXMOXER: @@ -207,12 +210,6 @@ def main(): auth_args = {'user': api_user} if not (api_token_id and api_token_secret): - # If password not set get it from PROXMOX_PASSWORD env - if not api_password: - try: - api_password = os.environ['PROXMOX_PASSWORD'] - except KeyError as e: - module.fail_json(msg='You should set api_password param or use PROXMOX_PASSWORD environment variable') auth_args['password'] = api_password else: auth_args['token_name'] = api_token_id @@ -261,9 +258,7 @@ def main(): content_type = module.params['content_type'] template = module.params['template'] - if not template: - module.fail_json(msg='template param is mandatory') - elif not get_template(proxmox, node, storage, content_type, template): + if not get_template(proxmox, node, storage, content_type, template): module.exit_json(changed=False, msg='template with volid=%s:%s/%s is already deleted' % (storage, content_type, template)) if delete_template(module, proxmox, node, storage, content_type, template, timeout):