Cleanups and dangling issues #117
Conversation
- set default if there is multiple execs
- simplify some checks slightly - use grep -Fx to check full lines - extended some tests
| @@ -18,41 +18,51 @@ | |||
| it { expect { is_expected.to compile }.to raise_error(%r{file mode must be one of: a,f,d,c,b,s,l,p - see "man semanage-fcontext"}) } | |||
| end | |||
|
|
|||
| context 'invalid multiple filetype' do | |||
| let(:params) { { pathname: '/tmp/file1', filetype: true, filemode: 'afdcbslp', context: 'user_home_dir_t' } } | |||
There was a problem hiding this comment.
could you please reformat this like (and please the other blocks with multiple attributes in it):
let(:params) do
{
pathname: '/tmp/file1',
filetype: true,
filemode: 'afdcbslp',
context: 'user_home_dir_t'
}
end
|
ok |
| @@ -70,7 +70,7 @@ | |||
| $destination = undef, | |||
| $context = undef, | |||
| $filetype = false, | |||
| $filemode = undef, | |||
| $filemode = 'a', | |||
There was a problem hiding this comment.
changing default behavior requires us to do a major release. I think this just changes the default value, but not the behavior because of the if constraint in line 107. So a normal minor release should be sufficient?
There was a problem hiding this comment.
The default value is 'a' anyhow, it just makes the matching code easier
|
Hi @maage, thanks for the PR! Looks very good to me, I just commented two little things. |
- previously bad filemode did not fail in manifest - change error message to contain actual setting
|
Is there rubocop setting with I can catch those wrongly formatted hashes? About filemode, previously if you defined resource without (undef) you got no change at all if you had previous setting where filemode was something else than 'a'/undef, because grep was positive from the wrong filemode. This definitely changes what happens. But I think previous setting was buggy. Same with values where shellquote is needed. With carefully constructed fcontext you could get "working" config but at least grep would give you wrong value and maybe you'd end up running semanage -a over and over again. I noticed slight bug when filetype was false and filemode was bogus and I've added patch to test/fix it. This will also break things. Main issue might be #105 change where semanage looks only locally made changes and not global changes. If previously you fixed wrong upstream module with local fcontext, and then upstream got fixed and you did new install, and now module does not add resources any more. But now module adds resources regardless. Compared to what I have in my queue, these changes are minor, but definitely there can be config that can break stuff. |
|
rubocop: It should be able to fix it if there is at least one new line in the hash, but I am not sure. If I understand you their are certain circumstances where we changed the default behavior. So we should do a major release. |
|
@bastelfreak let's merge this, then do a major bump after the dust settles from the other outstanding PRs. |
Cleanups and dangling issues
This patchset contains several small cleanups. It also contains fixes for #105 and #108 from TJM. Those issues do have some issue with travis and have not gone forward. As both issues are something I'd like to have and also have further changes in shellquote part, I've added them here.