module_params_get*: Fail on empty string in string list parameters #779
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
t-woerner
force-pushed
the
module_params_get_fail_empty_str_in_list
branch
2 times, most recently
from
efebe5a
to
e5dfb4e
Compare
|
@t-woerner
|
|
This PR would not fix this issue:
See PR #754 which is a draft that aims to fix this specific issue. |
t-woerner
force-pushed
the
module_params_get_fail_empty_str_in_list
branch
from
fc1d5c5
to
0f2965b
Compare
So far it is possible to pass list parameters with empty strings to the modules. The use of empty strings in list does not make a lot of sense, though. The simple solution is to add a check to module_params_get for empty strings in returned lists. The option allow_empty_string can be set to True to allow an empty string in the list with a list len of 1. The option defaults to False. It is needed for some parameters the modules, like for example userauthtype in the user module. It is using "" to reset to the default value. module_params_get_lowercase has been changed to use module_params_get to have one place to add the check. Due to an issue in Ansible it is possible to use the empty string "" for lists with choices, even if the empty list is not part of the choices. Ansible issue ansible/ansible#77108
The parameters userauthtype and sshpubkey allowing to use "" to reset to the default value. The new check in params_get is not allowing to use empty strings in lists, therefore allow_empty_string=True had to be added to the call. A test has been added to verify that the empty strings are supported and working. An idempotency issue with sshpubkey has been found with the test and fixed additionally.
t-woerner
force-pushed
the
module_params_get_fail_empty_str_in_list
branch
from
0f2965b
to
7219161
Compare
t-woerner
force-pushed
the
module_params_get_fail_empty_str_in_list
branch
2 times, most recently
from
df552e5
to
1317701
Compare
The parameters auth_ind and pac_type are allowing to use "" to reset to the default value. The new check in params_get is not allowing to use empty strings in lists, therefore allow_empty_string=True had to be added to the call. A test has been added to verify that the empty strings are supported and working. An idempotency issue with pac_type has been found with the test and fixed additionally.
The parameter auth_ind is allowing to use "" to reset to the default value. The new check in params_get is not allowing to use empty strings in lists, therefore allow_empty_string=True had to be added to the call. A test has been added to verify that the empty strings are supported and working.
t-woerner
force-pushed
the
module_params_get_fail_empty_str_in_list
branch
from
1317701
to
3a342b8
Compare
…gstring The parameters user_auth_type, pac_type and configstring are allowing to use "" to reset to the default value or for configstring to set an empty list. The new check in params_get is not allowing to use empty strings in lists, therefore allow_empty_string=True had to be added to the call. A test has been added to verify that the empty strings are supported and working. Additionally empty pac_type, user_auth_type and domain_resolution_order have been added to exit_args as if they have not been set.
t-woerner
force-pushed
the
module_params_get_fail_empty_str_in_list
branch
from
3a342b8
to
e30bcfd
Compare
|
LGTM |
varunmylaraiah
approved these changes
Mar 1, 2022
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.
So far it is possible to pass list parameters with empty strings to the
modules. The use of empty strings in list does not make a lot of sense,
though. The simple solution is to add a check to module_params_get for
empty strings in returned lists.
module_params_get_lowercase has been changed to use module_params_get to
have one place to add the check.