Skip to content
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

Ansible changes for RH 7 and Python3 #278

Merged
merged 5 commits into from
Nov 30, 2020
Merged

Ansible changes for RH 7 and Python3 #278

merged 5 commits into from
Nov 30, 2020

Conversation

amanda11
Copy link
Contributor

Updates for playbooks to install the libselinux-python3 module on EL 7
Updates for playbooks to check that python3-devel is available. If not available then locate the optional server repo and install python3-devel RPM from there. Needed for RHEL where python3-devel is in a disabled repo, and therefore will not get pulled in automatically as a dependency when st2 rpm is installed.

Part of StackStorm/community#54

@pull-request-size pull-request-size bot added the size/M PR that changes 30-99 lines. Good size to review. label Nov 26, 2020
- name: Install python3 SELinux policies
become: yes
yum:
name: libselinux-python3
Copy link
Member

@arm4b arm4b Nov 27, 2020

Choose a reason for hiding this comment

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

Do we really need libselinux-python3 dependency? I can't find any usage in StackStorm.st2 role.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It depends really on what python3 modules are used, ST2 itself doesn't need it, but there may be packs that import modules that need it given they are being moved from python2 -> 3, and therefore don't get the python3 selinux policies by default.

We could remove it, and then it would be up to the packs to ensure that they add the dependency if they need it.

I'm fine with either method.

Copy link
Member

@arm4b arm4b Nov 27, 2020

Choose a reason for hiding this comment

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

That dependency is already installed in nginx role:

selinux_dependencies:
- python3-libsemanage
- python3-libselinux

which is then used to adjust some security restrictions that may come by default in RHEL/CentOS

- name: Update SELinux facts after installing dependencies
become: yes
setup:
filter: ansible_selinux
when: nginx_selinux_dependencies.changed
tags: nginx, skip_ansible_lint
- name: Adjust SELinux to allow network access for nginx
become: yes
seboolean:
name: httpd_can_network_connect
state: yes
persistent: yes
when: ansible_facts.selinux.status == "enabled" and ansible_facts.selinux.mode == "enforcing"
tags: nginx

With that, we don't need it in st2 role, but may need to update the

selinux_dependencies:
- libsemanage-python
- libselinux-python

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks - that's a better place to put it. Will move it to Nginx role.

Comment on lines 3 to 4
- libselinux-python
- libselinux-python3
Copy link
Member

@arm4b arm4b Nov 27, 2020

Choose a reason for hiding this comment

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

That looks even more weird now.
If we install libselinux-python, why do we need libselinux-python3 then (and vice-versa)?

Feels like we're doing something wrong.

Copy link
Member

@arm4b arm4b Nov 27, 2020

Choose a reason for hiding this comment

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

selinux lib is used just for this:

- name: Adjust SELinux to allow network access for nginx
become: yes
seboolean:
name: httpd_can_network_connect
state: yes
persistent: yes
when: ansible_facts.selinux.status == "enabled" and ansible_facts.selinux.mode == "enforcing"
tags: nginx

Copy link
Member

@arm4b arm4b Nov 27, 2020

Choose a reason for hiding this comment

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

According to https://docs.ansible.com/ansible/2.5/modules/seboolean_module.html#requirements

The below requirements are needed on the host that executes this module.

libselinux-python
libsemanage-python

It looks like selinux python3 is not needed at all in this situation as we have all the required dependencies for seboolean Ansible module already in place.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've done a bit more searching... I don't believe nginx needs libselinux-python3.

I've taken it off the system, and everything is still functioning. I

However...

If we have in our code or any code that gets imported an "import selinux" then it's going to fail on the move to python3 without the include of libselinux-python3. But I've searched and none of the packs in StackStorm exchange do this explicitly (and we don't include it in the st2 repo either).

ansible itself will still use python2, and it's a well reported one of failing if libselinux-python is not installed. So I believe we will want to keep the libselinux-python. It's also a dependency of policycoreutils-python, which is needed for the semanage call. So libselinux-python would be required.

I'm going to take libselinux-python3 out for now. And I'll do the same in the bash installer so that it's same environment.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the research, @amanda11!

This should also shed more light about the pack dependencies topic:
StackStorm/st2-packages#241
StackStorm/st2docs#979 (comment)

Copy link
Member

@arm4b arm4b left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Thanks for handling this corner case.

@amanda11 amanda11 merged commit e6cad37 into master Nov 30, 2020
@amanda11 amanda11 deleted the py3_el7 branch November 30, 2020 21:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature size/M PR that changes 30-99 lines. Good size to review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants