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: check_ps_userproc_lineage misses users whos UID == MAX_SYS_U… #53

Closed
wants to merge 2 commits into from

Conversation

SMark-Black
Copy link

Fix for bug # 50

@SMark-Black
Copy link
Author

I'm trying to make this work better for LSF, so... Added code to set NHC_RM for LSF, and to set the RM_DAEMON_MATCH for LSF. Fix missed another spot where the UID check needed to be changed.

@mej mej self-assigned this Jul 17, 2018
@mej
Copy link
Owner

mej commented Jul 17, 2018

So here's the challenge with this: I prefer to keep commits separated by purpose, and your 2nd commit intermingles two separate conceptual changes (one a fix for the UID issue, the other an enhancement to LSF support). So in order to commit this, I'd need to refactor. Also, since one is a bugfix and the other is an enhancement, I'd target the former for the master branch but the latter for the dev branch.

I can do the refactor myself, but that'll lose your commits. Would you prefer to do it? I'd much rather you get the credit you deserve, if you're willing....

@mej mej added this to Pending in NHC 1.4.3 Release Oct 30, 2018
@mej
Copy link
Owner

mej commented Oct 31, 2018

Have you had a chance to look at this? I'm happy to split it up myself, but that would remove you as the author of the changes, so I'd like to be able to credit you for your work. But let me know if you don't have time/opportunity to split it up yourself, and I can go ahead and give it a go. :-)

Thanks as always for your contributions!

@mej mej moved this from Pending to Active in NHC 1.4.3 Release Oct 31, 2018
mej added a commit that referenced this pull request Dec 29, 2018
While taking another look at @SMark-Black's #50 and #53, I realized
that the code in question regarding `$MAX_SYS_UID` is doing exactly
what it is supposed to do, given the intended meaning of the variable
based on its name.  What was *actually* wrong was that the
`nhc_common_get_max_sys_uid()` function has been reading the wrong
variable!

So `nhc_common_get_max_sys_uid()` will now look for `$SYS_UID_MAX` in
`/etc/login.defs` like it should have been doing all along, and using
the value of `UID_MIN - 1` as a fallback if necessary.

NOTE:  This means that the default auto-detected value of
`$MAX_SYS_UID` will likely be something ending in `99` (like `499` or
`999`) rather than `00` because it was always intended to be (and the
code has always treated it as) the *top* of the exempt UID range, NOT
the bottom of the non-exempt UID range!  If you have any configs or
scripts that rely on different assumptions, please make sure to make
any necessary updates.

Closes #50.
mej added a commit that referenced this pull request Dec 29, 2018
Based on a couple changes suggested by @SMark-Black in his PR #53, add
another command to look for to auto-detect LSF, and add support for
the LSF `res` daemon to the `check_ps_userproc_lineage()` check.  Also
moved the setting of `$RM_DAEMON_MATCH` to inside the check -- that's
the only thing in that whole entire file that actually requires a
resource manager!
@mej
Copy link
Owner

mej commented Dec 29, 2018

The LSF stuff has been fixed and committed in 9e8d0dc, and the other changes are invalid as discussed in 88a5fab. Closing this PR.

@mej mej closed this Dec 29, 2018
NHC 1.4.3 Release automation moved this from Active to Completed Dec 29, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
NHC 1.4.3 Release
  
Completed / Merged
Development

Successfully merging this pull request may close these issues.

None yet

2 participants