Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement basic support for read-only and write-only keys in api_info and api_modify #213

Merged
merged 2 commits into from
Sep 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions changelogs/fragments/213-read-write-only.yml
Original file line number Diff line number Diff line change
@@ -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)."
felixfontein marked this conversation as resolved.
Show resolved Hide resolved
- "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)."
15 changes: 14 additions & 1 deletion plugins/module_utils/_api_data.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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):
Expand Down
16 changes: 15 additions & 1 deletion plugins/modules/api_info.py
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,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.
felixfontein marked this conversation as resolved.
Show resolved Hide resolved
type: bool
default: false
version_added: 2.10.0
seealso:
- module: community.routeros.api
- module: community.routeros.api_facts
Expand Down Expand Up @@ -313,6 +320,7 @@
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())

Expand All @@ -338,6 +346,7 @@
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)

Expand All @@ -361,7 +370,10 @@
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

Check warning on line 376 in plugins/modules/api_info.py

View check run for this annotation

Codecov / codecov/patch

plugins/modules/api_info.py#L376

Added line #L376 was not covered by tests
if k not in entry:
if handle_disabled == 'exclamation':
k = '!%s' % k
Expand All @@ -372,6 +384,8 @@
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)
Expand Down
73 changes: 73 additions & 0 deletions plugins/modules/api_modify.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -233,12 +237,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
Expand Down Expand Up @@ -385,6 +419,18 @@
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]

Check warning on line 422 in plugins/modules/api_modify.py

View check run for this annotation

Codecov / codecov/patch

plugins/modules/api_modify.py#L422

Added line #L422 was not covered by tests
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(

Check warning on line 427 in plugins/modules/api_modify.py

View check run for this annotation

Codecov / codecov/patch

plugins/modules/api_modify.py#L427

Added line #L427 was not covered by tests
key=k, old_value=old_entry.get(k), new_value=v, for_text=for_text))
continue
if key_info.write_only:

Check warning on line 430 in plugins/modules/api_modify.py

View check run for this annotation

Codecov / codecov/patch

plugins/modules/api_modify.py#L430

Added line #L430 was not covered by tests
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
Expand Down Expand Up @@ -453,13 +499,26 @@
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:

Check warning on line 508 in plugins/modules/api_modify.py

View check run for this annotation

Codecov / codecov/patch

plugins/modules/api_modify.py#L508

Added line #L508 was not covered by tests
to_remove.append(real_k)
for k in to_remove:

Check warning on line 510 in plugins/modules/api_modify.py

View check run for this annotation

Codecov / codecov/patch

plugins/modules/api_modify.py#L510

Added line #L510 was not covered by tests
entry.pop(k)

Check warning on line 512 in plugins/modules/api_modify.py

View check run for this annotation

Codecov / codecov/patch

plugins/modules/api_modify.py#L512

Added line #L512 was not covered by tests

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
Expand All @@ -479,6 +538,16 @@
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':

Check warning on line 544 in plugins/modules/api_modify.py

View check run for this annotation

Codecov / codecov/patch

plugins/modules/api_modify.py#L544

Added line #L544 was not covered by tests
to_remove.append(real_key)
if key_info.write_only:

Check warning on line 546 in plugins/modules/api_modify.py

View check run for this annotation

Codecov / codecov/patch

plugins/modules/api_modify.py#L546

Added line #L546 was not covered by tests
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:

Check warning on line 549 in plugins/modules/api_modify.py

View check run for this annotation

Codecov / codecov/patch

plugins/modules/api_modify.py#L549

Added line #L549 was not covered by tests
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))
Expand Down Expand Up @@ -634,6 +703,7 @@
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':
Expand Down Expand Up @@ -826,6 +896,7 @@
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):
Expand Down Expand Up @@ -1027,6 +1098,8 @@
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())

Expand Down
4 changes: 4 additions & 0 deletions tests/unit/plugins/module_utils/test__api_data.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [
('', [], ''),
Expand Down
30 changes: 28 additions & 2 deletions tests/unit/plugins/modules/fake_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -169,8 +169,16 @@
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]

Check warning on line 177 in tests/unit/plugins/modules/fake_api.py

View check run for this annotation

Codecov / codecov/patch

tests/unit/plugins/modules/fake_api.py#L177

Added line #L177 was not covered by tests
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):
Expand All @@ -194,7 +202,15 @@
entry = {
'.id': id,
}
entry.update(kwargs)
for field, value in kwargs.items():
if field.startswith('!'):
field = field[1:]

Check warning on line 207 in tests/unit/plugins/modules/fake_api.py

View check run for this annotation

Codecov / codecov/patch

tests/unit/plugins/modules/fake_api.py#L207

Added line #L207 was not covered by tests
if field not in self._path_info.fields:
raise ValueError('Trying to set unknown field "{field}"'.format(field=field))

Check warning on line 209 in tests/unit/plugins/modules/fake_api.py

View check run for this annotation

Codecov / codecov/patch

tests/unit/plugins/modules/fake_api.py#L209

Added line #L209 was not covered by tests
field_info = self._path_info.fields[field]
if field_info.read_only:
raise ValueError('Trying to set read-only field "{field}"'.format(field=field))

Check warning on line 212 in tests/unit/plugins/modules/fake_api.py

View check run for this annotation

Codecov / codecov/patch

tests/unit/plugins/modules/fake_api.py#L212

Added line #L212 was not covered by tests
entry[field] = value
_normalize_entry(entry, self._path_info, on_create=True)
self._values.append(entry)
return id
Expand Down Expand Up @@ -223,6 +239,16 @@
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))

Check warning on line 248 in tests/unit/plugins/modules/fake_api.py

View check run for this annotation

Codecov / codecov/patch

tests/unit/plugins/modules/fake_api.py#L248

Added line #L248 was not covered by tests
field_info = self._path_info.fields[field]
if field_info.read_only:
raise ValueError('Trying to update read-only field "{field}"'.format(field=field))

Check warning on line 251 in tests/unit/plugins/modules/fake_api.py

View check run for this annotation

Codecov / codecov/patch

tests/unit/plugins/modules/fake_api.py#L251

Added line #L251 was not covered by tests
entry.update(kwargs)
_normalize_entry(entry, self._path_info)

Expand Down