Skip to content

Extend framework for sudo use-cases#242

Merged
jakub-vavra-cz merged 3 commits intoSSSD:masterfrom
jakub-vavra-cz:sudo_extend
Apr 27, 2026
Merged

Extend framework for sudo use-cases#242
jakub-vavra-cz merged 3 commits intoSSSD:masterfrom
jakub-vavra-cz:sudo_extend

Conversation

@jakub-vavra-cz
Copy link
Copy Markdown
Contributor

Implement sudo alias for local provider
Add local netgroups implementation

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces support for local netgroups and sudoers aliases within the SSSD test framework. It adds new classes to manage /etc/netgroup and sudoers alias files in /etc/sudoers.d/, and updates LocalSudoRule to integrate with these aliases. A review comment identified a potential issue with the lexical sorting of sudoers alias files when using automatic ordering, suggesting the use of consistent zero-padding for file prefixes.

Comment thread sssd_test_framework/utils/local_users.py Outdated
@jakub-vavra-cz
Copy link
Copy Markdown
Contributor Author

The ci failures are unrelated. There is some issue in docs building and failing test_smartcard__su_as_local_user that probably has a fix in #239.

Comment thread sssd_test_framework/roles/client.py Outdated
Comment thread sssd_test_framework/utils/local_users.py Outdated
Comment thread sssd_test_framework/utils/local_users.py
Comment thread sssd_test_framework/utils/local_users.py
Comment thread sssd_test_framework/utils/local_users.py
@madhuriupadhye
Copy link
Copy Markdown
Contributor

Please check CI failures also.

@justin-stephenson
Copy link
Copy Markdown
Contributor

@jakub-vavra-cz you can add the following change to this PR as IdEntry is used only inside tools.py it does not need to be exported. It should resolve the error.

diff --git a/sssd_test_framework/utils/tools.py b/sssd_test_framework/utils/tools.py
index 10ddfe4..7730314 100644
--- a/sssd_test_framework/utils/tools.py
+++ b/sssd_test_framework/utils/tools.py
@@ -12,7 +12,6 @@ from pytest_mh.utils.fs import LinuxFileSystem
 __all__ = [
     "GetentUtils",
     "GroupEntry",
-    "IdEntry",
     "LinuxToolsUtils",
     "PasswdEntry",
     "UnixGroup",

Copy link
Copy Markdown
Contributor

@danlavu danlavu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code is great, thank you. I don't like the docstrings though. I think it should be less verbose and concise. I made some comments as examples of what I think should change, but I didn't apply them to the entire file. Can you go through the docstrings with the comments in mind? Thanks.

Comment thread sssd_test_framework/roles/client.py Outdated
Comment thread sssd_test_framework/roles/client.py Outdated
Comment thread sssd_test_framework/utils/local_users.py Outdated
Comment thread sssd_test_framework/utils/local_users.py Outdated
Comment thread sssd_test_framework/utils/local_users.py Outdated
Comment thread sssd_test_framework/utils/local_users.py
Comment thread sssd_test_framework/utils/local_users.py Outdated
Comment thread sssd_test_framework/utils/local_users.py
@jakub-vavra-cz jakub-vavra-cz force-pushed the sudo_extend branch 3 times, most recently from 7150a9a to 146234c Compare April 24, 2026 07:52
@jakub-vavra-cz
Copy link
Copy Markdown
Contributor Author

@madhuriupadhye I added the missing docstrings and refactored alias to allow changing order and handled duplicates for netgroup.
@danlavu I tried to cut down the docstring fluff a bit and add missing "." to make them more consistent.
I also changed sudo_alias to sudoalias but I will need to update the tests that are using it. :-(
Thanks @justin-stephenson, the fix worked.

madhuriupadhye
madhuriupadhye previously approved these changes Apr 24, 2026
Copy link
Copy Markdown
Contributor

@madhuriupadhye madhuriupadhye left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, just waiting to finish the CI.

danlavu
danlavu previously approved these changes Apr 24, 2026
Copy link
Copy Markdown
Contributor

@danlavu danlavu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's great, and I'll approve it, but I need a trivial change. There was a misunderstanding, type: never has . at the end. The pattern only applies to the :param: docstring.

sed '/:type/s/.$//' should do the trick.

Cleanup of the implementation, docstring fixes from review.
Reformatted with black.
@jakub-vavra-cz
Copy link
Copy Markdown
Contributor Author

It's great, and I'll approve it, but I need a trivial change. There was a misunderstanding, type: never has . at the end. The pattern only applies to the :param: docstring.

sed '/:type/s/.$//' should do the trick.

Did that for :type and :rtype that I both touched. Hopefully it is okay now.

@jakub-vavra-cz jakub-vavra-cz removed the request for review from justin-stephenson April 27, 2026 14:26
@jakub-vavra-cz jakub-vavra-cz merged commit f0c21a9 into SSSD:master Apr 27, 2026
5 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants