Implement basic support for read-only and write-only keys in api_info…

… and api_modify (#213)

* Implement basic support for read-only and write-only keys in api_info and api_modify.

* Do not show write-only fields as 'disabled'.

changelogs/fragments/213-read-write-only.yml

@@ -0,0 +1,3 @@
+minor_changes:
+  - "api_info - add new ``include_read_only`` option to select behavior for read-only values. By default these are not returned (https://github.com/ansible-collections/community.routeros/pull/213)."
+  - "api_modify - add new ``handle_read_only`` and ``handle_write_only`` options to handle the module's behavior for read-only and write-only fields (https://github.com/ansible-collections/community.routeros/pull/213)."

plugins/module_utils/_api_data.py

@@ -169,7 +169,16 @@ def specialize_for_version(self, api_version):
 
 
 class KeyInfo(object):
-    def __init__(self, _dummy=None, can_disable=False, remove_value=None, absent_value=None, default=None, required=False, automatically_computed_from=None):
+    def __init__(self,
+                 _dummy=None,
+                 can_disable=False,
+                 remove_value=None,
+                 absent_value=None,
+                 default=None,
+                 required=False,
+                 automatically_computed_from=None,
+                 read_only=False,
+                 write_only=False):
         if _dummy is not None:
             raise ValueError('KeyInfo() does not have positional arguments')
         if sum([required, default is not None or can_disable, automatically_computed_from is not None]) > 1:
@@ -180,12 +189,16 @@ def __init__(self, _dummy=None, can_disable=False, remove_value=None, absent_val
             raise ValueError('remove_value can only be specified if can_disable=True')
         if absent_value is not None and any([default is not None, automatically_computed_from is not None, can_disable]):
             raise ValueError('absent_value can not be combined with default, automatically_computed_from, can_disable=True, or absent_value')
+        if read_only and write_only:
+            raise ValueError('read_only and write_only cannot be used at the same time')
         self.can_disable = can_disable
         self.remove_value = remove_value
         self.automatically_computed_from = automatically_computed_from
         self.default = default
         self.required = required
         self.absent_value = absent_value
+        self.read_only = read_only
+        self.write_only = write_only
 
 
 def split_path(path):

plugins/modules/api_info.py

@@ -235,6 +235,13 @@
     type: bool
     default: false
     version_added: 2.4.0
+  include_read_only:
+    description:
+      - Whether to include read-only fields.
+      - By default, they are not returned.
+    type: bool
+    default: false
+    version_added: 2.10.0
 seealso:
   - module: community.routeros.api
   - module: community.routeros.api_facts
@@ -314,6 +321,7 @@ def main():
         hide_defaults=dict(type='bool', default=True),
         include_dynamic=dict(type='bool', default=False),
         include_builtin=dict(type='bool', default=False),
+        include_read_only=dict(type='bool', default=False),
     )
     module_args.update(api_argument_spec())
 
@@ -339,6 +347,7 @@ def main():
     hide_defaults = module.params['hide_defaults']
     include_dynamic = module.params['include_dynamic']
     include_builtin = module.params['include_builtin']
+    include_read_only = module.params['include_read_only']
     try:
         api_path = compose_api_path(api, path)
 
@@ -362,7 +371,10 @@ def main():
                     if k not in path_info.fields:
                         entry.pop(k)
             if handle_disabled != 'omit':
-                for k in path_info.fields:
+                for k, field_info in path_info.fields.items():
+                    if field_info.write_only:
+                        entry.pop(k, None)
+                        continue
                     if k not in entry:
                         if handle_disabled == 'exclamation':
                             k = '!%s' % k
@@ -373,6 +385,8 @@ def main():
                         entry.pop(k)
                 if field_info.absent_value and k not in entry:
                     entry[k] = field_info.absent_value
+                if not include_read_only and k in entry and field_info.read_only:
+                    entry.pop(k)
             result.append(entry)
 
         module.exit_json(result=result)

plugins/modules/api_modify.py

@@ -24,6 +24,10 @@
   - B(Note) that this module is still heavily in development, and only supports B(some) paths.
     If you want to support new paths, or think you found problems with existing paths, please first
     L(create an issue in the community.routeros Issue Tracker,https://github.com/ansible-collections/community.routeros/issues/).
+notes:
+  - If write-only fields are present in the path, the module is B(not idempotent) in a strict sense,
+    since it is not able to verify the current value of these fields. The behavior the module should
+    assume can be controlled with the O(handle_write_only) option.
 requirements:
   - Needs L(ordereddict,https://pypi.org/project/ordereddict) for Python 2.6
 extends_documentation_fragment:
@@ -234,12 +238,42 @@
       - If V(remove), they are removed. If at least one cannot be removed, the module will fail.
       - If V(remove_as_much_as_possible), all that can be removed will be removed. The ones that
         cannot be removed will be kept.
+      - Note that V(remove) and V(remove_as_much_as_possible) do not apply to write-only fields.
     type: str
     choices:
       - ignore
       - remove
       - remove_as_much_as_possible
     default: ignore
+  handle_read_only:
+    description:
+      - How to handle values passed in for read-only fields.
+      - If V(ignore), they are not passed to the API.
+      - If V(validate), the values are not passed for creation, and for updating they are compared to the value returned for the object.
+        If they differ, the module fails.
+      - If V(error), the module will fail if read-only fields are provided.
+    type: str
+    choices:
+      - ignore
+      - validate
+      - error
+    default: error
+    version_added: 2.10.0
+  handle_write_only:
+    description:
+      - How to handle values passed in for write-only fields.
+      - If V(create_only), they are passed on creation, and ignored for updating.
+      - If V(always_update), they are always passed to the API. This means that if such a value is present,
+        the module will always result in C(changed) since there is no way to validate whether the value
+        actually changed.
+      - If V(error), the module will fail if write-only fields are provided.
+    type: str
+    choices:
+      - create_only
+      - always_update
+      - error
+    default: create_only
+    version_added: 2.10.0
 seealso:
   - module: community.routeros.api
   - module: community.routeros.api_facts
@@ -386,6 +420,18 @@ def find_modifications(old_entry, new_entry, path_info, module, for_text='', ret
             continue
         if k not in old_entry and path_info.fields[k].default == v and not path_info.fields[k].can_disable:
             continue
+        key_info = path_info.fields[k]
+        if key_info.read_only:
+            # handle_read_only must be 'validate'
+            if old_entry.get(k) != v:
+                module.fail_json(
+                    msg='Read-only key "{key}" has value "{old_value}", but should have new value "{new_value}"{for_text}.'.format(
+                        key=k, old_value=old_entry.get(k), new_value=v, for_text=for_text))
+            continue
+        if key_info.write_only:
+            if module.params['handle_write_only'] == 'create_only':
+                # do not update this value
+                continue
         if k not in old_entry or old_entry[k] != v:
             modifications[k] = v
             updated_entry[k] = v
@@ -454,13 +500,26 @@ def essentially_same_weight(old_entry, new_entry, path_info, module):
     return weight
 
 
+def remove_read_only(entry, path_info):
+    to_remove = []
+    for real_k, v in entry.items():
+        k = real_k
+        if k.startswith('!'):
+            k = k[1:]
+        if path_info.fields[k].read_only:
+            to_remove.append(real_k)
+    for k in to_remove:
+        entry.pop(k)
+
+
 def format_pk(primary_keys, values):
     return ', '.join('{pk}="{value}"'.format(pk=pk, value=value) for pk, value in zip(primary_keys, values))
 
 
 def polish_entry(entry, path_info, module, for_text):
     if '.id' in entry:
         entry.pop('.id')
+    to_remove = []
     for key, value in entry.items():
         real_key = key
         disabled_key = False
@@ -480,6 +539,16 @@ def polish_entry(entry, path_info, module, for_text):
         elif value is None:
             if not key_info.can_disable:
                 module.fail_json(msg='Key "{key}" must not be disabled (value null/~/None){for_text}.'.format(key=key, for_text=for_text))
+        if key_info.read_only:
+            if module.params['handle_read_only'] == 'error':
+                module.fail_json(msg='Key "{key}" is read-only{for_text}, and handle_read_only=error.'.format(key=key, for_text=for_text))
+            if module.params['handle_read_only'] == 'ignore':
+                to_remove.append(real_key)
+        if key_info.write_only:
+            if module.params['handle_write_only'] == 'error':
+                module.fail_json(msg='Key "{key}" is write-only{for_text}, and handle_write_only=error.'.format(key=key, for_text=for_text))
+    for key in to_remove:
+        entry.pop(key)
     for key, field_info in path_info.fields.items():
         if field_info.required and key not in entry:
             module.fail_json(msg='Key "{key}" must be present{for_text}.'.format(key=key, for_text=for_text))
@@ -635,6 +704,7 @@ def sync_list(module, api, path, path_info):
                 new_data.append((old_index, updated_entry))
                 new_entry['.id'] = old_entry['.id']
             else:
+                remove_read_only(new_entry, path_info)
                 create_list.append(new_entry)
 
         if handle_absent_entries == 'remove':
@@ -827,6 +897,7 @@ def sync_with_primary_keys(module, api, path, path_info):
                     for primary_key in primary_keys
                 ]),
             ))
+        remove_read_only(new_entry, path_info)
         create_list.append(new_entry)
         new_entry = new_entry.copy()
         for key in list(new_entry):
@@ -1028,6 +1099,8 @@ def main():
         handle_absent_entries=dict(type='str', choices=['ignore', 'remove'], default='ignore'),
         handle_entries_content=dict(type='str', choices=['ignore', 'remove', 'remove_as_much_as_possible'], default='ignore'),
         ensure_order=dict(type='bool', default=False),
+        handle_read_only=dict(type='str', default='error', choices=['ignore', 'validate', 'error']),
+        handle_write_only=dict(type='str', default='create_only', choices=['create_only', 'always_update', 'error']),
     )
     module_args.update(api_argument_spec())
 

tests/unit/plugins/module_utils/test__api_data.py

@@ -99,6 +99,10 @@ def test_key_info_errors():
         KeyInfo(remove_value='')
     assert exc.value.args[0] == 'remove_value can only be specified if can_disable=True'
 
+    with pytest.raises(ValueError) as exc:
+        KeyInfo(read_only=True, write_only=True)
+    assert exc.value.args[0] == 'read_only and write_only cannot be used at the same time'
+
 
 SPLITTED_PATHS = [
     ('', [], ''),

tests/unit/plugins/modules/fake_api.py

@@ -169,8 +169,16 @@ def __init__(self, path, initial_values, read_only=False):
         self._new_id_counter = 0
         self._read_only = read_only
 
+    def _sanitize(self, entry):
+        entry = entry.copy()
+        for field, field_info in self._path_info.fields.items():
+            if field in entry:
+                if field_info.write_only:
+                    del entry[field]
+        return entry
+
     def __iter__(self):
-        return [entry.copy() for entry in self._values].__iter__()
+        return [self._sanitize(entry) for entry in self._values].__iter__()
 
     def _find_id(self, id, required=False):
         for index, entry in enumerate(self._values):
@@ -194,7 +202,15 @@ def add(self, **kwargs):
         entry = {
             '.id': id,
         }
-        entry.update(kwargs)
+        for field, value in kwargs.items():
+            if field.startswith('!'):
+                field = field[1:]
+            if field not in self._path_info.fields:
+                raise ValueError('Trying to set unknown field "{field}"'.format(field=field))
+            field_info = self._path_info.fields[field]
+            if field_info.read_only:
+                raise ValueError('Trying to set read-only field "{field}"'.format(field=field))
+            entry[field] = value
         _normalize_entry(entry, self._path_info, on_create=True)
         self._values.append(entry)
         return id
@@ -223,6 +239,16 @@ def update(self, **kwargs):
         entry = self._values[index]
         if entry.get('dynamic', False) or entry.get('builtin', False):
             raise Exception('Trying to update a dynamic or builtin entry')
+        for field in kwargs:
+            if field == '.id':
+                continue
+            if field.startswith('!'):
+                field = field[1:]
+            if field not in self._path_info.fields:
+                raise ValueError('Trying to update unknown field "{field}"'.format(field=field))
+            field_info = self._path_info.fields[field]
+            if field_info.read_only:
+                raise ValueError('Trying to update read-only field "{field}"'.format(field=field))
         entry.update(kwargs)
         _normalize_entry(entry, self._path_info)