Handle rule descriptions according to BIG-IP version (#1308)

* Handle rule descriptions according to BIG-IP version

Issues:
Fixes #1307

Problem:
The 'description' parameter does not exist in policy rules prior
to BIG-IP v12.1.0. Attempting to set the rule description for these
earlier BIG-IP versions results in an error response.

Analysis:
If the BIG-IP version is earlier than v12.1.0, strip the description from
policy rule.

Tests:
test_rules_description (f5/bigip/tm/ltm/test/functional/test_policy.py)

* Resolve code-review issues

Created a metadata entry to denote the optional policy parameters
and created a helper function to filter out these version-specific
optionals.

Author: edarzins

f5/bigip/tm/ltm/policy.py

@@ -36,6 +36,9 @@
 from f5.sdk_exception import OperationNotSupportedOnPublishedPolicy
 
 from distutils.version import LooseVersion
+import logging
+
+logger = logging.getLogger(__name__)
 
 
 class Policys(Collection):
@@ -56,6 +59,24 @@ def __init__(self, policy_s):
         self._meta_data['required_creation_parameters'].update(('strategy',))
         temp = {'tm:ltm:policy:rules:rulescollectionstate': Rules_s}
         self._meta_data['attribute_registry'] = temp
+        self._meta_data['optional_parameters'] = {'rules': ['description']}
+
+    def _filter_version_specific_options(self, tmos_ver, **kwargs):
+        '''Filter version-specific optional parameters
+
+        Some optional parameters only exist in v12.1.0 and greater,
+        filter these out for earlier versions to allow backward comatibility.
+        '''
+
+        if LooseVersion(tmos_ver) < LooseVersion('12.1.0'):
+            for k, parms in self._meta_data['optional_parameters'].items():
+                for r in kwargs.get(k, []):
+                    for parm in parms:
+                        value = r.pop(parm, None)
+                        if value is not None:
+                            logger.info(
+                                "Policy parameter %s:%s is invalid for v%s",
+                                k, parm, tmos_ver)
 
     def _create(self, **kwargs):
         '''Allow creation of draft policy and ability to publish a draft
@@ -69,6 +90,7 @@ def _create(self, **kwargs):
         tmos_ver = self._meta_data['bigip']._meta_data['tmos_version']
         legacy = kwargs.pop('legacy', False)
         publish = kwargs.pop('publish', False)
+        self._filter_version_specific_options(tmos_ver, **kwargs)
         if LooseVersion(tmos_ver) < LooseVersion('12.1.0'):
             return super(Policy, self)._create(**kwargs)
         else:
@@ -94,6 +116,7 @@ def _modify(self, **patch):
 
         legacy = patch.pop('legacy', False)
         tmos_ver = self._meta_data['bigip']._meta_data['tmos_version']
+        self._filter_version_specific_options(tmos_ver, **patch)
         if 'Drafts' not in self._meta_data['uri'] and \
                 LooseVersion(tmos_ver) >= LooseVersion('12.1.0') and \
                 not legacy:
@@ -110,6 +133,7 @@ def _update(self, **kwargs):
 
         legacy = kwargs.pop('legacy', False)
         tmos_ver = self._meta_data['bigip']._meta_data['tmos_version']
+        self._filter_version_specific_options(tmos_ver, **kwargs)
         if 'Drafts' not in self._meta_data['uri'] and \
                 LooseVersion(tmos_ver) >= LooseVersion('12.1.0') and \
                 not legacy:

f5/bigip/tm/ltm/test/functional/test_policy.py

@@ -38,6 +38,7 @@ def teardown_policy():
         kw = copy.deepcopy(kwargs)
         kw.pop('legacy', None)
         kw.pop('publish', None)
+        kw.pop('rules', None)
         if mgmt_root.tm.ltm.policys.policy.exists(
                 name=name, partition=partition, **kw):
             pol = mgmt_root.tm.ltm.policys.policy.load(
@@ -139,6 +140,30 @@ def test_policy_update_modify_update(self, setup, request, mgmt_root):
         pol1.modify(legacy=True, rules=[])
         pol1.modify(legacy=False, rules=[])
 
+    def test_rules_description(self, setup, request, mgmt_root):
+        rules = [
+            dict(
+                name='rule1',
+                ordinal=0,
+                actions=[],
+                conditions=[],
+                description='This is a rule description')
+        ]
+        pol, pc = setup_policy_test(request, mgmt_root, 'Common', 'desctest',
+                                    rules=rules, legacy=True)
+        rule = pol.rules_s.rules.load(name='rule1')
+        hasattr(rule, 'description') is False
+
+        rules[0]['description'] = 'Updated description'
+        pol.update(legacy=True, rules=rules)
+        rule = pol.rules_s.rules.load(name='rule1')
+        hasattr(rule, 'description') is False
+
+        rules[0]['description'] = 'Modified description'
+        pol.modify(legacy=True, rules=rules)
+        rule = pol.rules_s.rules.load(name='rule1')
+        hasattr(rule, 'description') is False
+
     def test_policy_update_race(self, setup, request, mgmt_root):
         full_pol_dict = json.load(
             open(os.path.join(CURDIR, 'full_policy.json')))
@@ -317,6 +342,30 @@ def test_policy_update_race(self, setup, request, mgmt_root):
             pol.update(**empty_pol_dict)
             assert pol.rules_s.rules.exists(name='test_rule') is False
 
+    def test_rules_description(self, setup, request, mgmt_root):
+        rules = [
+            dict(
+                name='rule1',
+                ordinal=0,
+                actions=[],
+                conditions=[],
+                description='This is a rule description')
+        ]
+        pol, pc = setup_policy_test(request, mgmt_root, 'Common', 'desctest',
+                                    rules=rules, legacy=True)
+        rule = pol.rules_s.rules.load(name='rule1')
+        assert rule.description == 'This is a rule description'
+
+        rules[0]['description'] = 'Updated description'
+        pol.update(legacy=True, rules=rules)
+        rule = pol.rules_s.rules.load(name='rule1')
+        assert rule.description == 'Updated description'
+
+        rules[0]['description'] = 'Modified description'
+        pol.modify(legacy=True, rules=rules)
+        rule = pol.rules_s.rules.load(name='rule1')
+        assert rule.description == 'Modified description'
+
     def test_rules_nonextant_on_load(self, setup, request, mgmt_root):
         pol, pc = setup_policy_test(request, mgmt_root, 'Common', 'racetest',
                                     subPath='Drafts')