fix: enable CI tests for openSUSE leap; handle missing /etc/sudoers#112
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdds openSUSE Leap as a supported platform and enables its CI testing by updating collection requirements, and hardens scan_sudoers to avoid crashing when the sudoers file path does not exist (e.g., systems using /usr/etc/sudoers). File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- In
get_config_lines, consider using a context manager (with open(path) as fp:) instead of manual open/close to make the file handling more robust and idiomatic. - Since you're now treating a missing sudoers file as non-fatal by returning
{}, you might also want to catch and handle other I/O errors (e.g., permission issues) aroundopen(path, "r")to avoid unexpected crashes on systems with restricted sudoers permissions.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `get_config_lines`, consider using a context manager (`with open(path) as fp:`) instead of manual open/close to make the file handling more robust and idiomatic.
- Since you're now treating a missing sudoers file as non-fatal by returning `{}`, you might also want to catch and handle other I/O errors (e.g., permission issues) around `open(path, "r")` to avoid unexpected crashes on systems with restricted sudoers permissions.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #112 +/- ##
=======================================
Coverage ? 52.29%
=======================================
Files ? 1
Lines ? 348
Branches ? 0
=======================================
Hits ? 182
Misses ? 166
Partials ? 0 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
[citest] |
| --- | ||
| collections: | ||
| - name: ansible.posix | ||
| - name: community.general |
There was a problem hiding this comment.
One question, why you added community general to the list of reqs?
There was a problem hiding this comment.
I think it is because suse leap requires https://docs.ansible.com/projects/ansible/latest/collections/community/general/zypper_module.html to manage packages
Enhancement: Enable openSUSE Leap CI testing and fix scan_sudoers crash
Reason: CI tests dont run on openSUSE Leap & scan_sudoers crashes with filenotfounderror on systems using config pattern (/usr/etc/sudoers) eg sles16/leap16/tumbleweed, causing the role to fail before it can run. fix added os.path.isfile() check in scan_sudoers.py.
Result: CI test to be enabled for Leap. scan_sudoers does not crash with FileNotFoundError.
Issue Tracker Tickets (Jira or BZ if any): na
Summary by Sourcery
Enable openSUSE Leap support and prevent sudoers scanning from failing when the sudoers file is missing.
Bug Fixes:
Enhancements: