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

Add functionality and tests for managing selinux port policy #47230

Merged
merged 2 commits into from Apr 26, 2018

Conversation

Projects
None yet
4 participants
@leeclemens
Copy link
Contributor

commented Apr 22, 2018

What does this PR do?

Allow usage of semanage port via modules and states to view/add/delete SELinux port policies.

What issues does this PR fix or reference?

Fixes #42635

New Behavior

Modules:

  • selinux.port_get_policy
  • selinux.port_add_or_delete_policy

States:

  • selinux.port_policy_present
  • selinux.port_policy_absent

Tests written?

Yes (for validating arguments and parsing semanage output)

Commits signed with GPG?

Yes

Please review Salt's Contributing Guide for best practices.

See GitHub's page on GPG signing for more information about signing commits with GPG.

@leeclemens leeclemens force-pushed the leeclemens:add-selinux-port branch 2 times, most recently from ef022c2 to e5f3487 Apr 22, 2018

@cachedout cachedout requested review from terminalmage and gtmanfred Apr 23, 2018

@gtmanfred
Copy link
Contributor

left a comment

Just some doc fixes, and this should be good.


def port_get_policy(name, sel_type=None, protocol=None, port=None):
'''
.. versionadded:: develop

This comment has been minimized.

Copy link
@gtmanfred

gtmanfred Apr 23, 2018

Contributor

This should be Fluorine.

This comment has been minimized.

Copy link
@leeclemens

leeclemens Apr 23, 2018

Author Contributor

Should it be set to literally "Fluorine"? https://docs.saltstack.com/en/latest/topics/releases/version_numbers.html shows the version number as TBD - I wasn't sure how/when these get updated when released.

This comment has been minimized.

Copy link
@gtmanfred

gtmanfred Apr 23, 2018

Contributor

Yes, it should be just the word Fluorine we do a search and replace on the code word once we know when the release date will be.

This comment has been minimized.

Copy link
@leeclemens

leeclemens Apr 23, 2018

Author Contributor

Thanks, updated


def port_policy_present(name, sel_type, protocol=None, port=None, sel_range=None):
'''
.. versionadded:: develop

This comment has been minimized.

Copy link
@gtmanfred

gtmanfred Apr 23, 2018

Contributor

Fluorine here too

This comment has been minimized.

Copy link
@leeclemens

leeclemens Apr 23, 2018

Author Contributor

Thanks, updated

Returns the result of the call to semanage.
name

This comment has been minimized.

Copy link
@gtmanfred

gtmanfred Apr 23, 2018

Contributor

Can you document the action variable as well? including that it is add or delete.

This comment has been minimized.

Copy link
@leeclemens

leeclemens Apr 23, 2018

Author Contributor

Good catch, adding that as well

@leeclemens leeclemens force-pushed the leeclemens:add-selinux-port branch 5 times, most recently from ebb5790 to fb5efaa Apr 23, 2018

@leeclemens leeclemens force-pushed the leeclemens:add-selinux-port branch from 03f514a to fecf6d4 Apr 23, 2018

@rallytime

This comment has been minimized.

Copy link
Contributor

commented Apr 25, 2018

re-run py

'port': parts.group(3).strip(), }


def port_add_or_delete_policy(action, name, sel_type=None, protocol=None, port=None, sel_range=None):

This comment has been minimized.

Copy link
@terminalmage

terminalmage Apr 26, 2018

Contributor

This seems like a cumbersome name for a function, and also redundant since you're making the user specify the action as an argument. How about changing this function name to _port_add_or_delete_policy() and then making two separate functions to add and delete, each of which is a one-liner that calls the common function. For example:

def port_add_policy(name, sel_type=None, protocol=None, port=None, sel_range=None):
    '''
    <docstring goes here>
    '''
    return _port_add_or_delete_policy('add', name, sel_type, protocol, port, sel_range)


def port_delete_policy(name, sel_type=None, protocol=None, port=None, sel_range=None):
    '''
    <docstring goes here>
    '''
    return _port_add_or_delete_policy('delete', name, sel_type, protocol, port, sel_range)

This comment has been minimized.

Copy link
@gtmanfred

gtmanfred Apr 26, 2018

Contributor

This is the same naming scheme as the rest of the modules in this execution module.

https://github.com/saltstack/salt/blob/develop/salt/modules/selinux.py#L483

Probably good to rename all of them honestly, but i think that can be done in a different pr.

This comment has been minimized.

Copy link
@leeclemens

leeclemens Apr 26, 2018

Author Contributor

Yes, I pretty much copied the fcontext naming/flow and altered it accordingly. I'll make a note to put together another PR - thanks for the feedback from both of you!

This comment has been minimized.

Copy link
@leeclemens

leeclemens May 12, 2018

Author Contributor

I submitted PR #47622

This comment has been minimized.

Copy link
@gtmanfred

gtmanfred May 13, 2018

Contributor

Awesome thanks @leeclemens

@rallytime rallytime merged commit 587c438 into saltstack:develop Apr 26, 2018

5 of 10 checks passed

jenkins/PR/salt-pr-rs-cent7-n Pull Requests » Salt PR - RS CentOS 7 #18443 — ABORTED
Details
codeclimate 8 issues to fix
Details
default Build finished.
Details
jenkins/PR/salt-pr-linode-cent7-py3 Pull Requests » Salt PR - Linode CentOS 7 - PY3 #4390 — FAILURE
Details
jenkins/PR/salt-pr-linode-ubuntu14-n Pull Requests » Salt PR - Linode Ubuntu14.04 #22236 — FAILURE
Details
WIP ready for review
Details
jenkins/PR/salt-pr-clone Pull Requests » Salt PR - Clone #24562 — SUCCESS
Details
jenkins/PR/salt-pr-docs-n Pull Requests » Salt PR - Docs #16681 — SUCCESS
Details
jenkins/PR/salt-pr-linode-ubuntu16-py3 Pull Requests » Salt PR - Linode Ubuntu16.04 - PY3 #9290 — SUCCESS
Details
jenkins/PR/salt-pr-lint-n Pull Requests » Salt PR - Code Lint #21301 — SUCCESS
Details

@leeclemens leeclemens deleted the leeclemens:add-selinux-port branch Apr 27, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.