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

remove execshield sysctl-parameter on rhel7 #119

Merged
merged 4 commits into from
Aug 7, 2017
Merged

remove execshield sysctl-parameter on rhel7 #119

merged 4 commits into from
Aug 7, 2017

Conversation

rndmh3ro
Copy link
Member

We're having code-duplicates here, but I still think its the cleanest solution, as I don't want to include more conditionals to check for than necessary.
Fixes #118

@rndmh3ro rndmh3ro changed the title remove execshield sysctl-parameter on rhel7 WIP - remove execshield sysctl-parameter on rhel7 Apr 21, 2017
@rndmh3ro rndmh3ro changed the title WIP - remove execshield sysctl-parameter on rhel7 remove execshield sysctl-parameter on rhel7 Apr 23, 2017
@rndmh3ro
Copy link
Member Author

rndmh3ro commented Aug 6, 2017

Travis fails because of the errors that are fixed in #138.

Copy link
Member

@ypid ypid left a comment

Choose a reason for hiding this comment

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

LGTM overall. Note that I have never used RHEL7. One improvement could be implemented which will make maintenance easier in the long run.

tasks/main.yml Outdated
- '{{ ansible_distribution }}-{{ ansible_distribution_major_version }}.yml'
- '{{ ansible_distribution }}.yml'
- '{{ ansible_os_family }}-{{ ansible_distribution_major_version }}.yml'
- '{{ ansible_os_family }}.yml'
Copy link
Member

Choose a reason for hiding this comment

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

Maybe include {{ ansible_os_family }}.yml always a first task and then have a second task which does the with_first_found? This way redundancy could be decreased.

Copy link
Member Author

Choose a reason for hiding this comment

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

Can you expand on this? I don't understand how this decreased redundancy.

Copy link
Member

@ypid ypid Aug 6, 2017

Choose a reason for hiding this comment

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

Something like this:

- name: Set OS family dependent variables
  include_vars: '{{ ansible_os_family }}.yml'

- name: Set OS dependent variables
  include_vars: '{{ item }}'
  with_first_found:
   - '{{ ansible_distribution }}-{{ ansible_distribution_major_version }}.yml'
   - '{{ ansible_distribution }}.yml
   - '{{ ansible_os_family }}-{{ ansible_distribution_major_version }}.yml'

Then {{ ansible_os_family }}.yml could contain all the basics and other files could be kept at a minimum.

@rndmh3ro rndmh3ro merged commit 9fa496f into master Aug 7, 2017
vars/RedHat.yml Outdated
owner: root
group: root
mode: '0644'

Copy link
Member

@ypid ypid Aug 7, 2017

Choose a reason for hiding this comment

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

Interesting. Basically \n EOF :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, well. I don't know.

@rndmh3ro rndmh3ro deleted the exec_shield branch August 7, 2017 19:59
rndmh3ro added a commit that referenced this pull request Jul 24, 2020
use package instead of yum so the operation works on Fedora
divialth pushed a commit to divialth/ansible-collection-hardening that referenced this pull request Aug 3, 2022
use package instead of yum so the operation works on Fedora
divialth pushed a commit to divialth/ansible-collection-hardening that referenced this pull request Aug 3, 2022
remove execshield sysctl-parameter on rhel7
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants