From cfd1d2e327f8223d27927ab2b748c8d9295f3402 Mon Sep 17 00:00:00 2001 From: Felix Fontein Date: Mon, 15 Feb 2021 22:16:29 +0100 Subject: [PATCH] Improved parameter handling on proxmox modules (#1765) (#1803) * Improved parameter handling on proxmox modules * removed unused imports * rollback change in plugins/modules/cloud/misc/proxmox_user_info.py * added changelog fragment * Update changelogs/fragments/1765-proxmox-params.yml Co-authored-by: Felix Fontein Co-authored-by: Felix Fontein (cherry picked from commit 0a5f79724c5b63ec099e6f46cbf6286a2d00bb18) Co-authored-by: Alexei Znamensky <103110+russoz@users.noreply.github.com> --- changelogs/fragments/1765-proxmox-params.yml | 2 ++ plugins/modules/cloud/misc/proxmox.py | 20 ++++++---------- plugins/modules/cloud/misc/proxmox_kvm.py | 24 +++++++------------ .../modules/cloud/misc/proxmox_template.py | 19 ++++++--------- 4 files changed, 25 insertions(+), 40 deletions(-) create mode 100644 changelogs/fragments/1765-proxmox-params.yml 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 140d56f686..553b4ebe80 100644 --- a/plugins/modules/cloud/misc/proxmox.py +++ b/plugins/modules/cloud/misc/proxmox.py @@ -370,7 +370,6 @@ EXAMPLES = r''' state: absent ''' -import os import time import traceback from distutils.version import LooseVersion @@ -381,7 +380,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 @@ -506,7 +505,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), @@ -538,7 +537,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: @@ -585,13 +587,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 @@ -623,8 +619,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 0161fefcea..a5536061fb 100644 --- a/plugins/modules/cloud/misc/proxmox_kvm.py +++ b/plugins/modules/cloud/misc/proxmox_kvm.py @@ -813,7 +813,6 @@ status: }' ''' -import os import re import time import traceback @@ -826,7 +825,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 @@ -1059,7 +1058,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), @@ -1072,12 +1071,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'), @@ -1100,7 +1099,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'), @@ -1136,13 +1135,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: @@ -1203,12 +1203,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_template.py b/plugins/modules/cloud/misc/proxmox_template.py index 541dc28efa..1a84505d6d 100644 --- a/plugins/modules/cloud/misc/proxmox_template.py +++ b/plugins/modules/cloud/misc/proxmox_template.py @@ -152,7 +152,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): @@ -205,7 +205,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), @@ -218,7 +218,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: @@ -237,12 +240,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 @@ -291,9 +288,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):