pycontribs / selinux

Pure-python selinux shim module for use in virtualenvs
MIT License
20 stars 19 forks source link

Use major version if python installed in custom path #65

Open dmasteller4 opened 1 year ago

dmasteller4 commented 1 year ago

Switch to using python major version if python is installed on a custom path. Fix recent issues that seem to have risen due to #56 for @sophos-rickc and @sophos-daniels.

conn commented 1 year ago

Why would the fix for issues related to #56 require testing when the original change did not require testing? Falling back to the original behavior seems like a safe and critical change for at least the mentioned Sophos folks.

I've been digging through issues on this projects due to issues with pyenv and Python 3.9+ (though 3.8 seems to work just fine). I'm aware of #35 and #42, and how pyenv isn't a supported use case, but I have to point out that patching in system_python = sys.executable seems to work perfectly fine for my use case (Molecule creating/destroying Docker containers with SELinux set to targeted and enforcing).

The reason why I'm mentioning this completely separate use case is that I think a fallback pattern for selecting system_python has been badly needed for years. I'd love to see this package test for multiple suitable Pythons until it finds one instead of stubbornly sticking with one hard-coded path that's bound to exclude systems.

I'd love to see this change be expanded with an additional fallback so that sys.executable is selected when no other suitable Pythons are found. That way, more use cases can be silently served while also still telling people to install python3-libselinux or that pyenv isn't supported when they have issues.

ssbarnea commented 1 year ago

@conn This package needs help, especially in testing area, so we would avoid future regressions. My time is very limited but I would be able to help if someone steps in and starts adding tests.

As some regressions were reported, the first measure is to prevent merging new code that is not well tested, even if the change would mainly be a revert.

I will have more work to do with molecule in the following months, so there is a small change I might need to add fixes myself here. Still, nobody should rely on me having the time.

Shortly, I would LOVE to get others to help with project maintenance and this project is extremely small, it should not require too much of anyone's time to help with testing.

conn commented 1 year ago

Totally understandable.

Do you know where this package is being used outside of Ansible and Molecule? My understanding is that this package isn't being used by Molecule anymore and is only being pulled in by Molecule-Plugins because it's still listed as a dependency. If I can muster some spare cycles, I'd be happy to add some tests but I'm admittedly ignorant about where this package is being used.

ssbarnea commented 1 year ago

I think that it could make sense to remove selinux from molecule-plugins and require the users to install it when needed. It can cause some headaches for them but at the same time it can same some for those that do not really need it.

See https://github.com/ansible-community/molecule-plugins/pull/89

conn commented 1 year ago

Thank you for that!