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

Fix for issue 128 #153

Merged
merged 4 commits into from Apr 19, 2017
Merged

Fix for issue 128 #153

merged 4 commits into from Apr 19, 2017

Conversation

dimsh99
Copy link
Contributor

@dimsh99 dimsh99 commented Apr 1, 2017

No description provided.

jfwm2 pushed a commit to jfwm2/mariadb that referenced this pull request Apr 4, 2017
* updating nginx to chef_nginx
* removing nginx dependency
* template property should not be boolen. just removing it.
* fixed syntax error
bash 'Restore security context' do
user 'root'
code "/usr/sbin/restorecon -v #{node['mariadb']['mysqld']['default_datadir']}"
only_if '[ -f /usr/sbin/selinuxenabled ] && /usr/sbin/selinuxenabled'

This comment was marked as outdated.

This comment was marked as outdated.

@dimsh99
Copy link
Contributor Author

dimsh99 commented Apr 7, 2017

Thanks, Stephen, I'll check on this.

@dimsh99
Copy link
Contributor Author

dimsh99 commented Apr 7, 2017

I remembered why I gave up on using selinux_enabled? first time.
If Chef::Recipe.send(:include, Chef::Util::Selinux) added to recipe selinux_enabled? can't be accessed from within resource only_if guard:
undefined method `selinux_enabled?' for Chef::Resource::Bash

I see 2 ways to make it work:

  1. Inject Selinux into bash resource:
    Chef::Resource::Bash.send(:include, Chef::Util::Selinux)
  2. Condition around resource:
    if selinux_enabled? bash 'Restore security context' do .... end end

The first approach makes all Bash resources in runlist "infected" by Chef::Util::Selinux, the other one is considered bad practice in chef.
So I'm not sure what to choose. Maybe I'm missing something?

@shoekstra
Copy link
Contributor

Now that you mention that, it does sound familiar. I dug into our wrapper cookbook and we ended up with something like this:

extend Chef::Util::Selinux

selinux_on = selinux_enabled?

and then use selinux_on in the resource's guard block? I think this would be the cleanest option IMO.

On a separate note we should discuss SELinux testing, but let's do that in #155.

@shoekstra shoekstra merged commit c572839 into sous-chefs:master Apr 19, 2017
@shoekstra
Copy link
Contributor

I've rebased and fixed up the guard to test if SELinux is enabled.

Thanks for the contribution!

@lock
Copy link

lock bot commented Jul 24, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Jul 24, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants