-
Notifications
You must be signed in to change notification settings - Fork 23.8k
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
Handle None and Blank value for sysctl module #24871
Conversation
Fix adds check for values provided by user for name and value in sysctl module. While providing name and value as in-line params, check for blank values Fixes ansible#20176 Signed-off-by: Abhijeet Kasurde <akasurde@redhat.com>
) | ||
|
||
if module.params['name'] is None: | ||
module.fail_json(msg="name can not be None") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the parameter is required, it cannot be None. That's implicit, so this check should not be necessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dagwieers Thanks for your review. I added this check, because I am facing weird a behaviour, correct me if I am wrong.
Here is demo module which contains name
and mode
as required parameters.
When I use, ansible-playbook command with following yaml file
- name: sample demo
demo:
name:
mode:
I am getting name
and mode
as None.
Same case goes with following -
# ansible -m demo -a "name= mode=" localhost
here, I am getting name
and mode
as "" (blank value)
Let me know, if these behaviours are different from the normal ones. This is only reason, I have added following two checks in sysctl
modules, because sysctl
was writing blank values to given sysctl.conf
file like the issue describes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, your first example should not return None values, but empty strings IMO. If you want to assign None-values, you could do that using null
. And that would be caught by the required=True
.
So if this is true, that seems to be a bug.
You are right, YAML interprets no value as None (or null). And I stand corrected. I doubt a lot of modules handle this correctly as None is the default value (which means the parameter was not provided).
@dagwieers Do I need to do something else to get this PR rolling ? |
So i'm not sure if we should fix this globally (require a value aside from parameter being present) ... should None/''/empty values be significant to module itself? |
SUMMARY
Fix adds check for values provided by user for
name and value in sysctl module.
While providing name and value as in-line params,
check for blank values
Fixes #20176
Signed-off-by: Abhijeet Kasurde akasurde@redhat.com
ISSUE TYPE
COMPONENT NAME
lib/ansible/modules/system/sysctl.py
ANSIBLE VERSION