From b6753b46a987a319ff062a8adcdcd4e0000353ed Mon Sep 17 00:00:00 2001 From: Abhijeet Kasurde Date: Tue, 18 Feb 2020 17:30:16 +0530 Subject: [PATCH] Replace 'message' in module parameters (#60051) * 'message' parameter is replaced by 'commit_message' in grafana_dashboard * 'message' parameter is replaced by 'notification_message' in datadog_monitor This change is required since 'message' as parameter name is used internally by Ansible core engine. Fixes: #39295 #45362 #47132 #59617 Signed-off-by: Abhijeet Kasurde --- .../fragments/39295-grafana_dashboard.yml | 4 +++ .../developing_modules_best_practices.rst | 1 + .../dev_guide/testing_validate-modules.rst | 1 + .../rst/porting_guides/porting_guide_2.10.rst | 3 ++ lib/ansible/modules/monitoring/bigpanda.py | 8 +++++- .../monitoring/datadog/datadog_monitor.py | 13 ++++++--- .../monitoring/grafana/grafana_dashboard.py | 17 +++++++---- .../validate-modules/validate_modules/main.py | 28 +++++++++++++++++++ test/sanity/ignore.txt | 13 +++++++++ 9 files changed, 77 insertions(+), 11 deletions(-) create mode 100644 changelogs/fragments/39295-grafana_dashboard.yml diff --git a/changelogs/fragments/39295-grafana_dashboard.yml b/changelogs/fragments/39295-grafana_dashboard.yml new file mode 100644 index 00000000000..e736b332b19 --- /dev/null +++ b/changelogs/fragments/39295-grafana_dashboard.yml @@ -0,0 +1,4 @@ +bugfixes: +- Replace parameter 'message' with appropriate parameter name in several modules, as 'message' is used internally in Ansible Core engine (https://github.com/ansible/ansible/issues/39295). +minor_changes: +- Added invalid-argument-name spec in ansible-test. diff --git a/docs/docsite/rst/dev_guide/developing_modules_best_practices.rst b/docs/docsite/rst/dev_guide/developing_modules_best_practices.rst index 1455c94f205..35a65771a01 100644 --- a/docs/docsite/rst/dev_guide/developing_modules_best_practices.rst +++ b/docs/docsite/rst/dev_guide/developing_modules_best_practices.rst @@ -150,6 +150,7 @@ Ansible conventions offer a predictable user interface across all modules, playb * Use consistent names across modules (yes, we have many legacy deviations - don't make the problem worse!). * Use consistent parameters (arguments) within your module(s). +* Do not use 'message' or 'syslog_facility' as a parameter name, as this is used internally by Ansible. * Normalize parameters with other modules - if Ansible and the API your module connects to use different names for the same parameter, add aliases to your parameters so the user can choose which names to use in tasks and playbooks. * Return facts from ``*_facts`` modules in the ``ansible_facts`` field of the :ref:`result dictionary` so other modules can access them. * Implement ``check_mode`` in all ``*_info`` and ``*_facts`` modules. Playbooks which conditionalize based on fact information will only conditionalize correctly in ``check_mode`` if the facts are returned in ``check_mode``. Usually you can add ``supports_check_mode=True`` when instantiating ``AnsibleModule``. diff --git a/docs/docsite/rst/dev_guide/testing_validate-modules.rst b/docs/docsite/rst/dev_guide/testing_validate-modules.rst index 9a21e5e8611..d3009917dcb 100644 --- a/docs/docsite/rst/dev_guide/testing_validate-modules.rst +++ b/docs/docsite/rst/dev_guide/testing_validate-modules.rst @@ -81,6 +81,7 @@ Codes imports-improper-location Imports Error Imports should be directly below ``DOCUMENTATION``/``EXAMPLES``/``RETURN``/``ANSIBLE_METADATA`` incompatible-choices Documentation Error Choices value from the argument_spec is not compatible with type defined in the argument_spec incompatible-default-type Documentation Error Default value from the argument_spec is not compatible with type defined in the argument_spec + invalid-argument-name Documentation Error Argument in argument_spec must not be one of 'message', 'syslog_facility' as it is used internally by Ansible Core Engine invalid-argument-spec Documentation Error Argument in argument_spec must be a dictionary/hash when used invalid-argument-spec-options Documentation Error Suboptions in argument_spec are invalid invalid-documentation Documentation Error ``DOCUMENTATION`` is not valid YAML diff --git a/docs/docsite/rst/porting_guides/porting_guide_2.10.rst b/docs/docsite/rst/porting_guides/porting_guide_2.10.rst index 2614afdbb02..09cc7d9c77f 100644 --- a/docs/docsite/rst/porting_guides/porting_guide_2.10.rst +++ b/docs/docsite/rst/porting_guides/porting_guide_2.10.rst @@ -123,6 +123,9 @@ Noteworthy module changes * :ref:`purefb_fs ` no longer supports the deprecated ``nfs`` option. This has been superceeded by ``nfsv3``. * :ref:`nxos_igmp_interface ` no longer supports the deprecated ``oif_prefix`` and ``oif_source`` options. These have been superceeded by ``oif_ps``. * :ref:`aws_s3 ` can now delete versioned buckets even when they are not empty - set mode to delete to delete a versioned bucket and everything in it. +* The parameter ``message`` in :ref:`grafana_dashboard ` module is renamed to ``commit_message`` since ``message`` is used by Ansible Core engine internally. +* The parameter ``message`` in :ref:`datadog_monitor ` module is renamed to ``notification_message`` since ``message`` is used by Ansible Core engine internally. +* The parameter ``message`` in :ref:`bigpanda ` module is renamed to ``deployment_message`` since ``message`` is used by Ansible Core engine internally. Plugins diff --git a/lib/ansible/modules/monitoring/bigpanda.py b/lib/ansible/modules/monitoring/bigpanda.py index c7231bf4aea..7978984c1da 100644 --- a/lib/ansible/modules/monitoring/bigpanda.py +++ b/lib/ansible/modules/monitoring/bigpanda.py @@ -69,6 +69,12 @@ options: required: false default: 'yes' type: bool + deployment_message: + description: + - Message about the deployment. + - C(message) alias is deprecated in Ansible 2.10, since it is used internally by Ansible Core Engine. + aliases: ['message'] + version_added: "2.10" # informational: requirements for nodes requirements: [ ] @@ -129,7 +135,7 @@ def main(): env=dict(required=False), owner=dict(required=False), description=dict(required=False), - message=dict(required=False), + deployment_message=dict(required=False, aliases=['message'], deprecated_aliases=[dict(name='message', version='2.14')]), source_system=dict(required=False, default='ansible'), validate_certs=dict(default='yes', type='bool'), url=dict(required=False, default='https://api.bigpanda.io'), diff --git a/lib/ansible/modules/monitoring/datadog/datadog_monitor.py b/lib/ansible/modules/monitoring/datadog/datadog_monitor.py index 258a31123c6..cee8837f623 100644 --- a/lib/ansible/modules/monitoring/datadog/datadog_monitor.py +++ b/lib/ansible/modules/monitoring/datadog/datadog_monitor.py @@ -68,12 +68,14 @@ options: - The name of the alert. required: true type: str - message: + notification_message: description: - A message to include with notifications for this monitor. - Email notifications can be sent to specific users by using the same '@username' notation as events. - Monitor message template variables can be accessed by using double square brackets, i.e '[[' and ']]'. + - C(message) alias is deprecated in Ansible 2.10, since it is used internally by Ansible Core Engine. type: str + aliases: [ 'message' ] silenced: description: - Dictionary of scopes to silence, with timestamps or None. @@ -154,7 +156,7 @@ EXAMPLES = ''' name: "Test monitor" state: "present" query: "datadog.agent.up.over('host:host1').last(2).count_by_status()" - message: "Host [[host.name]] with IP [[host.ip]] is failing to report to datadog." + notification_message: "Host [[host.name]] with IP [[host.ip]] is failing to report to datadog." api_key: "9775a026f1ca7d1c6c5af9d94d9595a4" app_key: "87ce4a24b5553d2e482ea8a8500e71b8ad4554ff" @@ -213,7 +215,7 @@ def main(): type=dict(required=False, choices=['metric alert', 'service check', 'event alert', 'process alert']), name=dict(required=True), query=dict(required=False), - message=dict(required=False, default=None), + notification_message=dict(required=False, default=None, aliases=['message'], deprecated_aliases=[dict(name='message', version='2.14')]), silenced=dict(required=False, default=None, type='dict'), notify_no_data=dict(required=False, default=False, type='bool'), no_data_timeframe=dict(required=False, default=None), @@ -235,6 +237,9 @@ def main(): if not HAS_DATADOG: module.fail_json(msg=missing_required_lib('datadogpy'), exception=DATADOG_IMP_ERR) + if 'message' in module.params: + module.fail_json(msg="'message' is reserved keyword, please change this parameter to 'notification_message'") + options = { 'api_key': module.params['api_key'], 'api_host': module.params['api_host'], @@ -285,7 +290,7 @@ def _post_monitor(module, options): try: kwargs = dict(type=module.params['type'], query=module.params['query'], name=_fix_template_vars(module.params['name']), - message=_fix_template_vars(module.params['message']), + message=_fix_template_vars(module.params['notification_message']), escalation_message=_fix_template_vars(module.params['escalation_message']), options=options) if module.params['tags'] is not None: diff --git a/lib/ansible/modules/monitoring/grafana/grafana_dashboard.py b/lib/ansible/modules/monitoring/grafana/grafana_dashboard.py index a97b7317848..c0177eb9acc 100644 --- a/lib/ansible/modules/monitoring/grafana/grafana_dashboard.py +++ b/lib/ansible/modules/monitoring/grafana/grafana_dashboard.py @@ -79,10 +79,12 @@ options: - Override existing dashboard when state is present. type: bool default: 'no' - message: + commit_message: description: - Set a commit message for the version history. - Only used when C(state) is C(present). + - C(message) alias is deprecated in Ansible 2.10, since it is used internally by Ansible Core Engine. + aliases: [ 'message' ] validate_certs: description: - If C(no), SSL certificates will not be validated. @@ -116,7 +118,7 @@ EXAMPLES = ''' grafana_url: http://grafana.company.com grafana_api_key: "{{ grafana_api_key }}" state: present - message: Updated by ansible + commit_message: Updated by ansible overwrite: yes path: /path/to/dashboards/foo.json @@ -353,8 +355,8 @@ def grafana_create_dashboard(module, data): # update if 'overwrite' in data and data['overwrite']: payload['overwrite'] = True - if 'message' in data and data['message']: - payload['message'] = data['message'] + if 'commit_message' in data and data['commit_message']: + payload['message'] = data['commit_message'] r, info = fetch_url(module, '%s/api/dashboards/db' % data['grafana_url'], data=json.dumps(payload), headers=headers, method='POST') @@ -371,7 +373,7 @@ def grafana_create_dashboard(module, data): else: body = json.loads(info['body']) raise GrafanaAPIException('Unable to update the dashboard %s : %s (HTTP: %d)' % - (uid, body['message'], info['status'])) + (uid, body['commit_message'], info['status'])) else: # unchanged result['uid'] = uid @@ -499,7 +501,7 @@ def main(): slug=dict(type='str'), path=dict(type='str'), overwrite=dict(type='bool', default=False), - message=dict(type='str'), + commit_message=dict(type='str', aliases=['message'], deprecated_aliases=[dict(name='message', version='2.14')]), ) module = AnsibleModule( argument_spec=argument_spec, @@ -508,6 +510,9 @@ def main(): mutually_exclusive=[['grafana_user', 'grafana_api_key'], ['uid', 'slug']], ) + if 'message' in module.params: + module.fail_json(msg="'message' is reserved keyword, please change this parameter to 'commit_message'") + try: if module.params['state'] == 'present': result = grafana_create_dashboard(module, module.params) diff --git a/test/lib/ansible_test/_data/sanity/validate-modules/validate_modules/main.py b/test/lib/ansible_test/_data/sanity/validate-modules/validate_modules/main.py index 563decb2aef..73ba2dadd20 100644 --- a/test/lib/ansible_test/_data/sanity/validate-modules/validate_modules/main.py +++ b/test/lib/ansible_test/_data/sanity/validate-modules/validate_modules/main.py @@ -1179,6 +1179,34 @@ class ModuleValidator(Validator): if doc_options is None: doc_options = {} for arg, data in spec.items(): + restricted_argument_names = ('message', 'syslog_facility') + if arg.lower() in restricted_argument_names: + msg = "Argument '%s' in argument_spec " % arg + if context: + msg += " found in %s" % " -> ".join(context) + msg += "must not be one of %s as it is used " \ + "internally by Ansible Core Engine" % (",".join(restricted_argument_names)) + self.reporter.error( + path=self.object_path, + code='invalid-argument-name', + msg=msg, + ) + continue + if 'aliases' in data: + for al in data['aliases']: + if al.lower() in restricted_argument_names: + msg = "Argument alias '%s' in argument_spec " % al + if context: + msg += " found in %s" % " -> ".join(context) + msg += "must not be one of %s as it is used " \ + "internally by Ansible Core Engine" % (",".join(restricted_argument_names)) + self.reporter.error( + path=self.object_path, + code='invalid-argument-name', + msg=msg, + ) + continue + if not isinstance(data, dict): msg = "Argument '%s' in argument_spec" % arg if context: diff --git a/test/sanity/ignore.txt b/test/sanity/ignore.txt index 0134ba61a0e..7dd5c7aa158 100644 --- a/test/sanity/ignore.txt +++ b/test/sanity/ignore.txt @@ -2599,6 +2599,7 @@ lib/ansible/modules/monitoring/airbrake_deployment.py validate-modules:doc-defau lib/ansible/modules/monitoring/airbrake_deployment.py validate-modules:doc-missing-type lib/ansible/modules/monitoring/bigpanda.py validate-modules:doc-default-does-not-match-spec lib/ansible/modules/monitoring/bigpanda.py validate-modules:doc-missing-type +lib/ansible/modules/monitoring/bigpanda.py validate-modules:invalid-argument-name lib/ansible/modules/monitoring/bigpanda.py validate-modules:undocumented-parameter lib/ansible/modules/monitoring/circonus_annotation.py validate-modules:doc-default-incompatible-type lib/ansible/modules/monitoring/circonus_annotation.py validate-modules:doc-missing-type @@ -2609,10 +2610,12 @@ lib/ansible/modules/monitoring/datadog/datadog_event.py validate-modules:doc-mis lib/ansible/modules/monitoring/datadog/datadog_event.py validate-modules:parameter-list-no-elements lib/ansible/modules/monitoring/datadog/datadog_event.py validate-modules:parameter-type-not-in-doc lib/ansible/modules/monitoring/datadog/datadog_monitor.py validate-modules:doc-default-does-not-match-spec +lib/ansible/modules/monitoring/datadog/datadog_monitor.py validate-modules:invalid-argument-name lib/ansible/modules/monitoring/datadog/datadog_monitor.py validate-modules:parameter-list-no-elements lib/ansible/modules/monitoring/datadog/datadog_monitor.py validate-modules:parameter-type-not-in-doc lib/ansible/modules/monitoring/grafana/grafana_dashboard.py validate-modules:doc-missing-type lib/ansible/modules/monitoring/grafana/grafana_dashboard.py validate-modules:doc-required-mismatch +lib/ansible/modules/monitoring/grafana/grafana_dashboard.py validate-modules:invalid-argument-name lib/ansible/modules/monitoring/grafana/grafana_dashboard.py validate-modules:parameter-type-not-in-doc lib/ansible/modules/monitoring/grafana/grafana_datasource.py validate-modules:doc-default-does-not-match-spec lib/ansible/modules/monitoring/grafana/grafana_datasource.py validate-modules:doc-missing-type @@ -2693,8 +2696,10 @@ lib/ansible/modules/monitoring/zabbix/zabbix_action.py validate-modules:doc-choi lib/ansible/modules/monitoring/zabbix/zabbix_action.py validate-modules:doc-default-does-not-match-spec lib/ansible/modules/monitoring/zabbix/zabbix_action.py validate-modules:doc-elements-mismatch lib/ansible/modules/monitoring/zabbix/zabbix_action.py validate-modules:doc-required-mismatch +lib/ansible/modules/monitoring/zabbix/zabbix_action.py validate-modules:invalid-argument-name lib/ansible/modules/monitoring/zabbix/zabbix_action.py validate-modules:missing-suboption-docs lib/ansible/modules/monitoring/zabbix/zabbix_action.py validate-modules:parameter-list-no-elements +lib/ansible/modules/monitoring/zabbix/zabbix_action.py validate-modules:nonexistent-parameter-documented lib/ansible/modules/monitoring/zabbix/zabbix_action.py validate-modules:parameter-type-not-in-doc lib/ansible/modules/monitoring/zabbix/zabbix_action.py validate-modules:undocumented-parameter lib/ansible/modules/monitoring/zabbix/zabbix_group.py validate-modules:doc-elements-mismatch @@ -4597,6 +4602,8 @@ lib/ansible/modules/network/fortimanager/fmgr_device_config.py validate-modules: lib/ansible/modules/network/fortimanager/fmgr_device_group.py validate-modules:doc-required-mismatch lib/ansible/modules/network/fortimanager/fmgr_device_group.py validate-modules:parameter-type-not-in-doc lib/ansible/modules/network/fortimanager/fmgr_device_provision_template.py validate-modules:doc-required-mismatch +lib/ansible/modules/network/fortimanager/fmgr_device_provision_template.py validate-modules:invalid-argument-name +lib/ansible/modules/network/fortimanager/fmgr_device_provision_template.py validate-modules:nonexistent-parameter-documented lib/ansible/modules/network/fortimanager/fmgr_device_provision_template.py validate-modules:parameter-type-not-in-doc lib/ansible/modules/network/fortimanager/fmgr_fwobj_address.py validate-modules:parameter-type-not-in-doc lib/ansible/modules/network/fortimanager/fmgr_fwobj_ippool.py validate-modules:parameter-list-no-elements @@ -6472,8 +6479,12 @@ lib/ansible/modules/notification/catapult.py validate-modules:parameter-list-no- lib/ansible/modules/notification/catapult.py validate-modules:parameter-type-not-in-doc lib/ansible/modules/notification/cisco_spark.py validate-modules:doc-default-does-not-match-spec lib/ansible/modules/notification/cisco_spark.py validate-modules:doc-missing-type +lib/ansible/modules/notification/cisco_spark.py validate-modules:invalid-argument-name +lib/ansible/modules/notification/cisco_spark.py validate-modules:nonexistent-parameter-documented lib/ansible/modules/notification/cisco_spark.py validate-modules:undocumented-parameter lib/ansible/modules/notification/flowdock.py validate-modules:doc-missing-type +lib/ansible/modules/notification/grove.py validate-modules:invalid-argument-name +lib/ansible/modules/notification/grove.py validate-modules:nonexistent-parameter-documented lib/ansible/modules/notification/grove.py validate-modules:parameter-type-not-in-doc lib/ansible/modules/notification/hipchat.py validate-modules:doc-missing-type lib/ansible/modules/notification/hipchat.py validate-modules:undocumented-parameter @@ -7099,6 +7110,8 @@ lib/ansible/modules/storage/netapp/na_ontap_lun_map.py validate-modules:doc-miss lib/ansible/modules/storage/netapp/na_ontap_lun_map.py validate-modules:parameter-type-not-in-doc lib/ansible/modules/storage/netapp/na_ontap_motd.py validate-modules:doc-missing-type lib/ansible/modules/storage/netapp/na_ontap_ndmp.py validate-modules:parameter-list-no-elements +lib/ansible/modules/storage/netapp/na_ontap_motd.py validate-modules:invalid-argument-name +lib/ansible/modules/storage/netapp/na_ontap_motd.py validate-modules:nonexistent-parameter-documented lib/ansible/modules/storage/netapp/na_ontap_net_ifgrp.py validate-modules:doc-missing-type lib/ansible/modules/storage/netapp/na_ontap_net_ifgrp.py validate-modules:parameter-list-no-elements lib/ansible/modules/storage/netapp/na_ontap_net_ifgrp.py validate-modules:parameter-type-not-in-doc