-
Notifications
You must be signed in to change notification settings - Fork 1.7k
add sssd_info module #11120
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
base: main
Are you sure you want to change the base?
add sssd_info module #11120
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
russoz
left a comment
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.
Hi @MikeyTide
Thanks for the contribution! I left some comments in there for you, and I strongly suggest you read more documentation as you proceed.
|
@russoz Hi, thank you for reviewing my pull request. I have made the necessary fixes. If you have time, please take another look. |
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
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.
hi @MikeyTide thanks for your contribution!
Please see my initial comments.
plugins/modules/sssd_info.py
Outdated
| DBUS_IMP_ERR = None | ||
| HAS_DBUS = False | ||
| try: | ||
| import dbus | ||
| HAS_DBUS = True | ||
| except Exception: | ||
| DBUS_IMP_ERR = traceback.format_exc() |
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.
You might want to consider using module_utils.deps, see https://docs.ansible.com/ansible/latest/collections/community/general/docsite/guide_deps.html for guidance on how to use it.
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.
Hi @russoz, thank you so much for showing me this module. I tried using it, could you take a look at this specific comment? That way I don't have to push the entire code.
Thanks in advance for helping with my pull request.
....
....
import traceback
from ansible.module_utils.basic import AnsibleModule, missing_required_lib
try:
from ansible_collections.community.general.plugins.module_utils import deps
with deps.declare("dbus-python"):
import dbus
HAS_DBUS = True
DBUS_IMP_ERR = None
except Exception as e:
HAS_DBUS = False
DBUS_IMP_ERR = e
....
....
def main() -> None:
....
....
try:
from ansible_collections.community.general.plugins.module_utils import deps
deps.validate(module)
except ImportError:
if not HAS_DBUS:
from ansible.module_utils.basic import missing_required_lib
module.fail_json(msg=missing_required_lib('dbus-python'))
....
....
if name == 'main':
main()
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.
hi @MikeyTide
Use "```" before and after code in GH comments, it makes the code block readable.
Now, that being said, I would suggest you go through that documentation one more time, carefully, because you are still trying to use the old mechanism together with deps and that makes it a bit confusing.
You may want to check on other modules that use deps (search for "import deps" and you will hit many of them here). In particular, one that is relatively simple is
|
|
||
| DOCUMENTATION = r''' | ||
| --- | ||
| module: sssd_info |
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.
| module: sssd_info | |
| module: sssd_info | |
| version_added: 12.1.0 |
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.
ok
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.
actually 12.1.0 was released a couple of days ago, you need to bump this up to 12.2.0 now
| - id: test_domain_status_online | ||
| input: | ||
| action: domain_status | ||
| domain: example.com | ||
| output: | ||
| online: online | ||
| mocks: | ||
| run_command: [] |
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.
In the current state, uthelper is useless for modules that do not run external commands (with run_command()).
That is likely the reason why the tests are currently failing. In your case, the module interacts with D-Bus using the corresponding Python package, but you will have to mock the calls to that library, otherwise your module will actually try to contact the sssd daemon, through D-Bus, during the CI run. Which is obviously going to fail.
|
The test The test The test The test The test The test The test The test The test The test The test The test The test The test The test The test |
SUMMARY
This pull request adds a new module
sssd_infothat allows users to check SSSD domain status and retrieve domain information using D-Bus.The module provides the following actions:
domain_status- Check if a specific domain is onlinedomain_list- List all configured SSSD domainsactive_servers- Get active servers for a specific domain and server type (IPA/AD)list_servers- List all servers for a specific domain and server type (IPA/AD)ISSUE TYPE
COMPONENT NAME
sssd_info
ADDITIONAL INFORMATION
The module uses D-Bus to communicate with SSSD's infopipe interface, providing reliable and direct access to SSSD status information without relying on command-line tools.
Key features:
Example usage: