ansible-collections / community.general

Ansible Community General Collection
https://galaxy.ansible.com/ui/repo/published/community/general/
GNU General Public License v3.0
822 stars 1.52k forks source link

ipa_user documentation: givenname and sn are required when account doesn't exist #7037

Closed cwchristerw closed 1 year ago

cwchristerw commented 1 year ago

Summary

These are required

https://docs.ansible.com/ansible/latest/collections/community/general/ipa_user_module.html#ansible-collections-community-general-ipa-user-module

Issue Type

Documentation Report

Component Name

ipa_user

Ansible Version

$ ansible --version
ansible [core 2.15.2]
  config file = None
  configured module search path = ['/home/cwchristerw/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /opt/ansible/lib/python3.10/site-packages/ansible
  ansible collection location = /home/cwchristerw/.ansible/collections:/usr/share/ansible/collections
  executable location = /usr/local/bin/ansible
  python version = 3.10.6 (main, May 29 2023, 11:10:38) [GCC 11.3.0] (/opt/ansible/bin/python3)
  jinja version = 3.1.2
  libyaml = True

Community.general Version

$ ansible-galaxy collection list community.general
Collection        Version
----------------- -------
community.general 7.2.0

Configuration

$ ansible-config dump --only-changed
-

OS / Environment

Linux Mint 21.1

Additional Information

fatal: [host.example.com]: FAILED! => {"changed": false, "msg": "response user_add: 'givenname' is required"}
fatal: [host.example.com]: FAILED! => {"changed": false, "msg": "response user_add: 'sn' is required"}

Code of Conduct

ansibullbot commented 1 year ago

Files identified in the description:

If these files are incorrect, please update the component name section of the description or use the !component bot command.

click here for bot help

ansibullbot commented 1 year ago

cc @Akasurde @Nosmoht @fxfitz @justchris1 click here for bot help

russoz commented 1 year ago

Hi @cwchristerw ,

Thanks for reporting! Would you be interested in working on a pull request for this?

I would suspect this is not a requirement for all states. Could you please confirm?

cwchristerw commented 1 year ago

Yes I can make PR, and I'll check it. It's required at least when present and user isn't existing. I'll test if its required when enabled and then report back here. I'll do it ASAP, tomorrow.

felixfontein commented 1 year ago

Sounds to me like this should be simply documented (without a code change), since the module (via the API) already reports an error if this is missing.

cwchristerw commented 1 year ago

Yes I agree :smile: I haven't made Docs Only PR yet :smile:

russoz commented 1 year ago

Sounds to me like this should be simply documented (without a code change), since the module (via the API) already reports an error if this is missing.

Not sure - not checking right now - but I believe that error message is coming from the API. If we can determine with some confidence that these parameters are (always) going to be required for some of the actions, the module could perform a param validation (using required_if, other required clauses, or an actual if in the code) before hitting the API, thus saving that HTTP request.

felixfontein commented 1 year ago

@russoz they seem to be needed on creation, but not on update - so requiring them for state=present would be a breaking change.

russoz commented 1 year ago

Well, in that case it is indeed a simple Docs PR. :-)

@cwchristerw please let us know if you need any further guidance, but I strongly recommend going through the https://github.com/ansible-collections/community.general/blob/main/CONTRIBUTING.md docs if you have not raised a PR for the collection before.

cwchristerw commented 1 year ago

I haven't had time for past two weeks but I have time to do PR this weekend.