Bug 1597545

Summary: selinux: do not override local modifications by default
Product: Red Hat Enterprise Linux 7 Reporter: Pavel Cahyna <pcahyna>
Component: rhel-system-rolesAssignee: Pavel Cahyna <pcahyna>
Status: CLOSED CURRENTRELEASE QA Contact: BaseOS QE - Apps <qe-baseos-apps>
Severity: unspecified Docs Contact:
Priority: unspecified    
Version: 7.5CC: lvrabec, plautrba, tbowling, vcrhonek, vdolezal
Target Milestone: rcKeywords: Extras
Target Release: 7.6   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: rhel-system-roles-1.0-2.el7 Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2019-01-18 13:23:26 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On:    
Bug Blocks: 1479381    

Description Pavel Cahyna 2018-07-03 08:08:34 UTC
The selinux role drops local modifications to policy before setting fcontexts, booleans etc. :

- name: Drop all local modifications
  shell: echo -e -n "{{drop_local_modifications}}" | /usr/sbin/semanage -i -

According to PM (tbowling), this would not be expected by users. The role should allow to make changes to the policy, without setting it completely. So, dropping local modifications should be made conditional on parameters, which should default to off. I propose the following parameters:
selinux_booleans_purge
selinux_fcontexts_purge
selinux_ports_purge
selinux_logins_purge

This also means that a way to remove individual file contexts is needed, i.e. the dicts under selinux_fcontexts should be able to specify "state".
(logins, ports, booleans allow to specify state already, nothing to be done here.)

Note also, there is another role in the Galaxy (https://galaxy.ansible.com/hxpro/selinux/) which allows to set booleans in a way compatible to ours and it does not drop local modifications.

(For the record, the argument from the selinux team against the proposed change was that if the users want such incremental changes, they can use the se* Ansible modules.)

Comment 3 Petr Lautrbach 2018-07-19 09:01:55 UTC
From my POV, a role should define a state, not changes. It means that if I specify some configuration using role variables, I expect that the system will be configured according to the variables and all other relevant changes will be reverted. That's why we drop all modifications first.

If I wanted to add only some context mapping, or change a boolean, I would add particular modules to my playbook.

Comment 4 Terry Bowling 2018-07-19 17:26:53 UTC
(In reply to Petr Lautrbach from comment #3)
> From my POV, a role should define a state, not changes. It means that if I
> specify some configuration using role variables, I expect that the system
> will be configured according to the variables and all other relevant changes
> will be reverted. That's why we drop all modifications first.
> 
> If I wanted to add only some context mapping, or change a boolean, I would
> add particular modules to my playbook.

We have a problem with nomenclature.  The way Ansible uses the word "role", the actual implementation, and how it is used in the real world do not always fit the  the definition of "role" that you might be thinking of.

That said, in these context, we are not thinking of "role" as a 100% fully defined state.

Our customers often take a layered approach.  They may not know the final configuration at deployment time.  So they need to build it up in layers.  In a proper config management environment like Ansible Tower, all of these configs would be stored in Playbooks.  Either consolidated or stored as a collection that could reproduce the desired state.

When we first approached the Ansible team, our goal based on customer feedback was to provide a "stable and consistent configuration interface to RHEL that was compatible across multiple versions."  I fully expected them to tell us to build modules that could adjust for a given implementation provider and features based on the OS version.  But instead they guided us that Roles were the best mechanism to provide the level of abstraction and compatibility across releases.


If we take the approach in comment #3, then we immediately lose any ability to build up layers and reduce ability for users to benefit.  For example, in the docs https://docs.ansible.com/ansible/2.5/user_guide/playbooks_reuse_roles.html, it demonstrates an example of multiple roles layered

- hosts: webservers
  roles:
     - common
     - webservers

With this common usage of roles, it removes the ability for this selinux system role to be usable within the common or webservers roles.  So what benefit have we provided?

I am inclined to accept comment #2 as the complete state is defined by the higher level playbook.  Our goal for this "role" is the configuration interface to make it easier for users configure selinux across releases.

Comment 5 Terry Bowling 2018-07-19 17:42:04 UTC
I will add that if we strongly feel there is not a need to provide any abstraction and there is no other benefit that this role provides, we could consider supporting and enhancing the current set of selinux modules

https://docs.ansible.com/ansible/latest/search.html?q=selinux&check_keywords=yes&area=default


However, I was under the impression that there was value to providing a single interface to selinux via this role.

Comment 6 Petr Lautrbach 2018-07-19 19:35:33 UTC
Thanks for the explanation. It makes sense for me now.

Comment 7 Lukas Vrabec 2018-07-20 12:36:02 UTC
PR merged.

Comment 8 Pavel Cahyna 2018-08-01 11:14:19 UTC
test: tests/tests_boolean.yml tests/tests_port.yml tests/tests_login.yml tests/tests_fcontext.yml, and also tests/tests_all_purge.yml after https://github.com/linux-system-roles/selinux/pull/29 is merged.