Add full change check for rabbitmq policy (#19073)

Add check for all policy fields (name, apply_to, pattern, tags,
priority) to have correct changed state. Previosly changed state was
based on policy name only.

* Add support for rabbitmq 3.7
* Fix exec args for rabbitmqctl status
* Add changelog and fix description for Ansible 2.9

changelogs/fragments/29264-rabbitmq_policy-add-full-change-check.yml

@@ -0,0 +1,2 @@
+bugfixes:
+  - rabbitmq_policy - Add full check for rabbit policy changes (https://github.com/ansible/ansible/issues/29264)

lib/ansible/modules/messaging/rabbitmq/rabbitmq_policy.py

@@ -38,12 +38,16 @@ options:
     version_added: "2.1"
   pattern:
     description:
-      - A regex of queues to apply the policy to.
-    required: true
+      - A regex of queues to apply the policy to. Required when
+        C(state=present). This option is no longer required as of Ansible 2.9.
+    required: false
+    default: null
   tags:
     description:
-      - A dict or string describing the policy.
-    required: true
+      - A dict or string describing the policy. Required when
+        C(state=present). This option is no longer required as of Ansible 2.9.
+    required: false
+    default: null
   priority:
     description:
       - The priority of the policy.
@@ -77,6 +81,9 @@ EXAMPLES = '''
 '''
 
 import json
+import re
+from distutils.version import LooseVersion as Version
+
 from ansible.module_utils.basic import AnsibleModule
 
 
@@ -93,25 +100,63 @@ class RabbitMqPolicy(object):
         self._node = module.params['node']
         self._rabbitmqctl = module.get_bin_path('rabbitmqctl', True)
 
-    def _exec(self, args, run_in_check_mode=False):
-        if not self._module.check_mode or (self._module.check_mode and run_in_check_mode):
+        self._version = self._rabbit_version()
+
+    def _exec(self,
+              args,
+              run_in_check_mode=False,
+              split_lines=True,
+              add_vhost=True):
+        if (not self._module.check_mode
+                or (self._module.check_mode and run_in_check_mode)):
             cmd = [self._rabbitmqctl, '-q', '-n', self._node]
-            args.insert(1, '-p')
-            args.insert(2, self._vhost)
+
+            if add_vhost:
+                args.insert(1, '-p')
+                args.insert(2, self._vhost)
+
             rc, out, err = self._module.run_command(cmd + args, check_rc=True)
-            return out.splitlines()
+            if split_lines:
+                return out.splitlines()
+
+            return out
         return list()
 
-    def list(self):
-        policies = self._exec(['list_policies'], True)
+    def _rabbit_version(self):
+        status = self._exec(['status'], True, False, False)
+
+        version_match = re.search('{rabbit,".*","(?P<version>.*)"}', status)
+        if version_match:
+            return Version(version_match.group('version'))
+
+        return None
+
+    def _list_policies(self):
+        if self._version and self._version >= Version('3.7.9'):
+            # Remove first header line from policies list for version > 3.7.9
+            return self._exec(['list_policies'], True)[1:]
+
+        return self._exec(['list_policies'], True)
+
+    def has_modifications(self):
+        if self._pattern is None or self._tags is None:
+            self._module.fail_json(
+                msg=('pattern and tags are required for '
+                     'state=present'))
 
-        for policy in policies:
-            if not policy:
-                continue
-            policy_name = policy.split('\t')[1]
-            if policy_name == self._name:
-                return True
-        return False
+        if self._version and self._version >= Version('3.7.0'):
+            # Change fields order in rabbitmqctl output in version 3.7
+            return not any(
+                self._policy_check(policy, apply_to_fno=3, pattern_fno=2)
+                for policy in self._list_policies())
+        else:
+            return not any(
+                self._policy_check(policy) for policy in self._list_policies())
+
+    def should_be_deleted(self):
+        return any(
+            self._policy_check_by_name(policy)
+            for policy in self._list_policies())
 
     def set(self):
         args = ['set_policy']
@@ -128,14 +173,44 @@ class RabbitMqPolicy(object):
     def clear(self):
         return self._exec(['clear_policy', self._name])
 
+    def _policy_check(self,
+                      policy,
+                      name_fno=1,
+                      apply_to_fno=2,
+                      pattern_fno=3,
+                      tags_fno=4,
+                      priority_fno=5):
+        if not policy:
+            return False
+
+        policy_data = policy.split('\t')
+
+        policy_name = policy_data[name_fno]
+        apply_to = policy_data[apply_to_fno]
+        pattern = policy_data[pattern_fno].replace('\\\\', '\\')
+        tags = json.loads(policy_data[tags_fno])
+        priority = policy_data[priority_fno]
+
+        return (policy_name == self._name and apply_to == self._apply_to
+                and tags == self._tags and priority == self._priority
+                and pattern == self._pattern)
+
+    def _policy_check_by_name(self, policy):
+        if not policy:
+            return False
+
+        policy_name = policy.split('\t')[1]
+
+        return policy_name == self._name
+
 
 def main():
     arg_spec = dict(
         name=dict(required=True),
         vhost=dict(default='/'),
-        pattern=dict(required=True),
+        pattern=dict(required=False, default=None),
         apply_to=dict(default='all', choices=['all', 'exchanges', 'queues']),
-        tags=dict(type='dict', required=True),
+        tags=dict(type='dict', required=False, default=None),
         priority=dict(default='0'),
         node=dict(default='rabbit'),
         state=dict(default='present', choices=['present', 'absent']),
@@ -151,15 +226,13 @@ def main():
     rabbitmq_policy = RabbitMqPolicy(module, name)
 
     result = dict(changed=False, name=name, state=state)
-    if rabbitmq_policy.list():
-        if state == 'absent':
-            rabbitmq_policy.clear()
-            result['changed'] = True
-        else:
-            result['changed'] = False
-    elif state == 'present':
+
+    if state == 'present' and rabbitmq_policy.has_modifications():
         rabbitmq_policy.set()
         result['changed'] = True
+    elif state == 'absent' and rabbitmq_policy.should_be_deleted():
+        rabbitmq_policy.clear()
+        result['changed'] = True
 
     module.exit_json(**result)