rjeffman: idempotence playground. #1190
Draft
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rjeffman
force-pushed
the
datatype_hostname
branch
from
0d9cd90
to
bb4a15f
Compare
rjeffman
force-pushed
the
datatype_hostname
branch
from
bb4a15f
to
35a48ce
Compare
When comparing a module argument set against an IPA argument set, for each attribute, if it is a list or tuple, a set is created and the sets are compared, this is more efficient than comparing the objects individually, as the list order may be different, and using a set, ordering is ignored. If the parameter value is a scalar, they are directly compared. Although efficient, this method causes idempotence issues with some data types, for example, when the comparison of two strings must be case insensitive. The solution proposed here is to avoid direct value comparison by always using the set comparison method, and that the data type of the values stored in the sets can perform the proper comparison for the specific data type. To ensure the comparison is done only with sets, scalar values are "wrapped" in a list, and the list is later converted to a set. To cope with specific data type comparison (for example: hostnames, services, or case insensitive strings) a new parameter was added, so a dictionaire mapping an argument to a conversion function can be passed, and the set of values is created by applying the conversion function to the IPA and the argument values. The complexity order of the comparison algorithm is not changed, but data conversion, and possibly a customized comparison, are new operations performed, which may have some impact on the execution time.
One of the issues on ensuring idempotence behavior is that FreeIPA does not expose datatypes along with the values stored in objects. By treating data returned from FreeIPA API with the proper datatype, it is easier to ensure that comparisons that will trigger changes (or not) are properly executed, despite of the stored value or the provided parameter data. This patch provides an initial implementation of the datatypes used by ansible-freeipa modules. An implementation for 'hostname' which ensures data provided for hostnames contain a fully qualified hostname, always in lower case, and an implementation of a 'list_of' to allow for parameters which are lists of objects of a specific datatype.
Adapt plugin to use the 'hostname' datatype, instead of reimplementing the required operations.
Adapt plugin to use the 'hostname' datatype, instead of reimplementing the required operations.
Adapt plugin to use the 'hostname' datatype, instead of reimplementing the required operations.
Adapt plugin to use the 'hostname' datatype, instead of reimplementing the required operations.
Adapt plugin to use the 'hostname' datatype, instead of reimplementing the required operations.
Add a service cast function that expose an object that allows services to be compared in a case-insensitive manner, but preserve case of the original data.
Adapt plugin to use the 'service' datatype, instead of reimplementing the required operations.
Adapt plugin to use the 'service' datatype, instead of reimplementing the required operations.
Add a cast function to create a string that can be compared in a case insensitive manner. It also allows the use of the objects in sets and dictionaries, making the key case insensitive.
Several parameters for ipadelegation need to be compared in a case insensitive manner. Most should be stored in lowercase, but 'memberof' should preserve case to maintain the same behavior as IPA CLI commands.
rjeffman
force-pushed
the
datatype_hostname
branch
from
35a48ce
to
d8fa7e0
Compare
The list generation functions for adding and deleting members from IPA objects create sets but are not aware of the underlying attribute data type. Some data types need to perform special comparison, like case insensitive string comparison, or ensure the existence of a FQDN. This patch adds a new optional attribute to the list generation functions, 'attr_datatype', that takes a function able to convert the expected data (usually a string) to an object that will correctly handle hashing and data comparison. The functions were also modified to perform the list generation, but still return the same data provided to the caller, withot conversion, minimizing the required amount of code changes.
FreeIPA provides a default hbacsvcgroup named "Sudo", with capital S, that is different from every other hbacsvcgroup, which are all represented by lower case letters. As data from IPA API was not modified, this causes an idempotence error when using different capitalization with the 'hbacsvcgroup' parameter. This patch fixes the issue by using the CaseInsensitive comparator to create the hbacsvcgroup list. Tests were update to make sure a regression is not included in the future.
rjeffman
force-pushed
the
datatype_hostname
branch
from
d8fa7e0
to
4015073
Compare
ipahostgroup parameters 'host', 'hostgroup', 'membermanager_user' and
'membermanager_group' must be compared in a case insensitive manner
and stored as lower case strings.
This patch fixes the comparison and storage of this parameters, and
change the handling of members to use the same structure as in newer
modules.
Two new tests files were added:
tests/hostgroup/test_hostgroup_case_insensitive.yml
tests/hostgroup/test_hostgroup_membermanager_case_insensitive.yml
rjeffman
force-pushed
the
datatype_hostname
branch
from
734b2ac
to
cf4a72d
Compare
Some attributes for ipagroup objects are stored using lower case letters
and should be converted upon retrieving parameter data.
This patch adds the missing conversion and provides a new test playbook:
tests/group/test_group_case_insensitive.yml
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A proposal for standardizing data conversion to fix idempotence issues in ansible-freeipa modules.
This PR requires #1143 to be merged.