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

Master-port #49860 selinux support for file.manage #55922

Merged
merged 12 commits into from
Apr 24, 2020

Conversation

mchugh19
Copy link
Contributor

@mchugh19 mchugh19 commented Jan 21, 2020

Master port of #49860
This looks good to me, but happy to get more eyes on it. See conversation in #49860 for more information

What does this PR do?

Recursion not supported for persist=True on file.get_selinux_context execution module

Add selinux functionality to file.managed

/tmp/selinux.test
  file.managed:
    - user: root
    - selinux:
        seuser:  system_u
        serole: object_r
        setype: system_conf_t
        seranage: s0
# salt-call --local state.sls selinux test=True
local:
----------
          ID: check_selinux
    Function: file.managed
        Name: /tmp/selinux.test
      Result: None
     Comment: File /tmp/selinux.test not updated
     Started: 10:54:39.418112
    Duration: 17.911 ms
     Changes:
              ----------
              selinux context:
                  ----------
                  New:
                      User: system_u Type: system_conf_t
                  Old:
                      User: unconfined_u Type: user_tmp_t

Summary for local
------------
Succeeded: 1 (unchanged=1, changed=1)
Failed:    0
------------
Total states run:     1
Total run time:  17.911 ms

What issues does this PR fix or reference?

#40703 and possibly #1349

New Behavior

Uses file.get_selinux_context and file.set_selinux_context to determine if selinux updates are needed. As mentioned in a ticket #1349, set_selinux_context uses the chcon command, which might not be persistent. Feedback welcome.

Tests written?

Yes - but could always use more eyes

Add selinux support to file.managed
@mchugh19 mchugh19 requested a review from a team as a code owner January 21, 2020 06:59
@ghost ghost requested a review from twangboy January 21, 2020 06:59
salt/modules/file.py Outdated Show resolved Hide resolved
salt/modules/file.py Outdated Show resolved Hide resolved
salt/modules/file.py Outdated Show resolved Hide resolved
salt/modules/file.py Outdated Show resolved Hide resolved
salt/modules/file.py Show resolved Hide resolved
salt/states/file.py Show resolved Hide resolved
@twangboy twangboy added the ZRelease-Sodium retired label label Mar 12, 2020
twangboy
twangboy previously approved these changes Apr 1, 2020
@twangboy
Copy link
Contributor

Hey, @mchugh19

Looks like this needs to be rebased and have pre-commit run on it. Do you mind doing that?

https://docs.saltstack.com/en/latest/topics/development/contributing.html#quickstart

@mchugh19
Copy link
Contributor Author

@twangboy

Looks like this needs to be rebased and have pre-commit run on it. Do you mind doing that?

I'm not sure what you mean. These files have been run through isort and black already. I tried manually running pre-commit run --files filenames but there weren't any changes. and there don't appear to be any conflicts with master. What needs rebasing?

@waynew
Copy link
Contributor

waynew commented Apr 17, 2020

I'm not sure either 😂

Pre-commit looks fine and is passing, but there are some other failures that look to be related to this PR. Can you take a look?

@mchugh19
Copy link
Contributor Author

tests were missing a cmd.run_all mapping. Not sure how it ever worked before, or why it only failed on centos (maybe selinux is only enabled on redhat?). Should be good to go now though.

@mchugh19
Copy link
Contributor Author

re-run pr-windows2019-py3

@mchugh19
Copy link
Contributor Author

re-run pr-windows2016-py3

@mchugh19
Copy link
Contributor Author

re-run pr-centos7-py3

@mchugh19
Copy link
Contributor Author

Ugh. Centos is stuck on

File "/tmp/kitchen/testing/tests/integration/states/test_docker_network.py", line 133, in tearDownClass
    if process.returncode != 0 and "No such image" not in output:
TypeError: a bytes-like object is required, not 'str'

@dwoz dwoz merged commit bcd135b into saltstack:master Apr 24, 2020
@mchugh19 mchugh19 deleted the port-49860 branch June 3, 2020 15:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants