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

Only inspect file attribute if lsattr(1) is installed #43746

Merged
merged 1 commit into from Sep 26, 2017
Merged

Only inspect file attribute if lsattr(1) is installed #43746

merged 1 commit into from Sep 26, 2017

Conversation

eradman
Copy link
Contributor

@eradman eradman commented Sep 26, 2017

What does this PR do?

lsattr/chattr is not installed on many Unix-like platforms by default, including *BSD, Solaris, and minimal Linux distributions such as Alpine. Skip attribute checks if lsattr is not found.

Previous Behavior

file.check_perms and anything that invokes it such as state.highstate fails:

$ doas salt-call --local --file-root=$PWD file.check_perms /tmp/file '{}' eradman users 644
Error running 'file.check_perms': Unable to run command '['lsattr', '/tmp/file']' with the context '{...}', reason: command not found

New Behavior

An error calling chattr is only raised if attributes were specified

Tests written?

No, but tested on OpenBSD 6.1 and CentOS 7.3. This is Cent7:

$ sudo -E salt-call -l debug --local --file-root=$PWD file.check_perms /tmp/file '{}' radman users 644 ai
local:
    |_
      ----------
      changes:
          ----------
          attrs:
              ai
      comment:
      name:
          /tmp/file
      result:
          True
    |_
      ----------
      lattrs:
          a
      lgroup:
          users
      lmode:
          0644
      luser:
          radman

lsattr/chattr is not installed on many Unix-like platforms by default,
including *BSD, Solaris, and minimal Linux distributions such as Alpine.
@ghost
Copy link

ghost commented Sep 26, 2017

@eradman, thanks for your PR! By analyzing the history of the files in this pull request, we identified @terminalmage, @whiteinge and @techhat to be potential reviewers.

@rallytime rallytime merged commit 6186da6 into saltstack:develop Sep 26, 2017
@eradman eradman deleted the check_perms branch September 26, 2017 17:44
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.

None yet

2 participants