-
Notifications
You must be signed in to change notification settings - Fork 76
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
SELinux config section Bugfix #312
base: main
Are you sure you want to change the base?
Conversation
As how the "when" condition was before: "not (ansible_check_mode and nginx_config_selinux_enforcing)", it was giving TRUE when it shouldn't because with the "and" when one of the two items is a FALSE, the whole parenthesis becomes FALSE. In the corrected way, it skips when the nginx_config_selinux_enforcing is in TRUE, as it should.
The package policycoreutils-python-utils is needed in RHEL8, if it is no installed the playbook prints and ERROR with ModuleNotFoundError message for the 'semanage' module.
I have a feeling we might be able to get rid of those conditionals altogether? I am looking at how the "core" NGINX role handles SELinux (https://github.com/nginxinc/ansible-role-nginx/blob/main/tasks/prerequisites/setup-selinux.yml#L2-L13) and by all accounts it is true that we need to set SELinux to permissive no matter what to run any of the other SELinux tasks. The key place where A different question altogether too, do you know if we need |
Hi @alessfg, thanks for taking a look at my input. Some more info and questions below.
Is this an affirmation? Why SELinux needs to be set to permissive first to run the SELinux tasks? Shouldn't it just be enabled (permissive or enforcing mode)?
The yum package
I'm taking this as homework so I can answer you better latter.
I don't think SELinux can be tested in containers. Take a look a this [1]. Am I wrong here? That's why I suggest you if the need to test SELinux tasks to use the Vagrant plugin for Molecule [2] as an option, as vagrant uses VMs. [0] Is there any way to retrieve a dependency tree from yum? Let me know your thoughts, if you may. |
Hey sorry it took me so long get back to this PR. I've been terribly busy these past couple months. Re your points:
As far as I understand, SELinux needs to be set to permissive to make changes to its configuration? I might be wrong on this, but if I am, we should open bug fix for https://github.com/nginxinc/ansible-role-nginx/blob/main/tasks/prerequisites/setup-selinux.yml#L10-L13.
Did you have a chance to look into this?
You are totally right. We could potentially create a Vagrant based scenario to test SELinux, but that might take a little bit more time/effort to implement, so it probably belongs on a separate PR (if not in a separate role altogether, the core Ansible NGINX role). |
Hi @alessfg
From what I understand the SELinux needs to only be in permissive or enforcing mode, so I believe yes, some changes must be made to the ansible-role-nginx project. There is a point to consider though (maybe it could be an improvement), if SELinux is already in enforcing or permissive, there's no problem, if it is disabled and should be changed to permissive or enforcing, there is a good practice that is to reboot and relabel files as explained in here 2.4. Enabling SELinux on systems that previously had it disabled.
Yes. The package
Agree about the more time/effort. Let me know how would you like to continue this. I may be able to help a little more. |
Ok thanks for the info. Based on what you are saying I think it'd be great if you could: Re rebooting/relabeling SELinux, I think that level of interaction with the target host is out of the scope of this role. Users running the role can then reboot the system at their own discretion if necessary. At most, I would add a note in the docs. |
✅ All required contributors have signed the F5 CLA for this PR. Thank you! |
I have hereby read the F5 CLA and agree to its terms |
c32c384
to
7a27221
Compare
Proposed changes
Describe the use case and detail of the change. If this PR addresses an issue on GitHub, make sure to include a link to that issue using one of the supported keywords here in this description (not in the title of the PR).
Hi,
After testing this role in RHEL 8, I've realized the SELinux section wasn't working, it only needed two things to work correctly:
Please, let me know of any improvements needed in this PR. Thanks.
Checklist
Before creating a PR, run through this checklist and mark each as complete.
CONTRIBUTING
documentSELinux cannot be tested in containers and Molecule uses docker containers. Before my changes, I didn't see any already created tests for SELinux. If you think these tests are very important now, I can try to integrate Molecule with Vagrant and test there, just let me know
defaults/main/*.yml
,README.md
andCHANGELOG.md
)There is no need for more documentations, as my changes are only a bugfix to make the SELinux section works as intended.