-
Notifications
You must be signed in to change notification settings - Fork 290
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
Windows - attempts to create a local 'sensu' user #617
Comments
|
This was fixed here: #588 New version has not been cut however but you can pull that commit and at least have a working module for windows. |
|
Thank you for this. I was able to "temporarily" get around the problem by using $manage_user false then creating the account with other modules where i could control the password which was enough to get myself going (dsc_user class, call the user "sensu" and then the module works fine). |
|
In that case you would need to make sure the sensu user has the correct system permissions to run the checks needed. Of course it will work fine and I do plan to add better support for that in a few weeks, just have not had a moment to spare for it. Regardless no problem and glad you at least got it working :) |
|
@dzeleski I don't think this is fixed... I'm pulling latest (with the above mentioned fix) and getting the following: cc @cwjohnston |
|
@cdenneen need to set manage_user to false for windows systems. I plan on adding support for sensu to be able to manage a local user but I have not gotten around to it. I mentioned that setting on the pull request, the default I believe is true. |
|
Can you update PR to have false based on osfamily in params?
…On Tue, Mar 28, 2017 at 6:27 PM dzeleski ***@***.***> wrote:
@cdenneen <https://github.com/cdenneen> need to set manage_user to false
for windows systems. I plan on adding support for sensu to be able to
manage a local user but I have not gotten around to it. I mentioned that
setting on the pull request, the default I believe is true.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#617 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAr84XsTz0iRNQS-UsX-6eY3-D544X_Pks5rqYlTgaJpZM4MWYEH>
.
|
|
@cdenneen not in its current state. manage_user is a global parameter in init.pp that effects all platforms: https://github.com/dzeleski/sensu-puppet/blob/3d34fda1891ed1fbe513b00596e6cababbd385ae/manifests/init.pp#L360 As far as I know you cannot put a case statement in with parameters. We would have to set that to undef and then set it to true or false in each case statement. Its definitely possible but I do not run a debian shop so testing that is not possible for me and the "linux" case effects both debian and rhel as seen here: https://github.com/dzeleski/sensu-puppet/blob/3d34fda1891ed1fbe513b00596e6cababbd385ae/manifests/init.pp#L530 |
Until better user management for windows installs can be added this is to prevent attempting to create user/group on windows platform regardless of the manage_user parameter. Fixes issue sensu#617 * NOTE: This would have been better as a case statement in a params.pp class but since that isn't implemented this is a crude workaround for the time being. This is also the default requirement for PR sensu#588
|
@dzeleski see if that works |
|
@cdenneen This isnt disabling by default this is disabling permanently. IMO this is the worse then just using hiera to disable the management of the windows user or even just setting this based on a case statement in your puppet code if you arent using hiera. This would then block being able to eventually support adding a local windows user in the future without removing that line. Doesn't make a ton of sense to me but ill let the maintainers make that call. |
|
When the user functionality for windows is fixed the condition and test go away.
Currently the user implementation is broken, so therefore it should be blocked until it is corrected.
As my note said I would have rather conditionally set the parameter to false for osfamily had the module been using the recommended params.pp instead of hard coded in the init.
The concept of even params.pp is being deprecated in favor of module data so eventually the module needs to be refactored.
As this is implemented it removes the user creation from windows since it's broken permanently because either Boolean option provided by a user is broken at this point for that osfamily.
Forcing users to have to use class parameters vs just including the class is a bit disruptive. Think of it this way, if you are including this in a base class for cross platform you'd need different manage_user params based on osfamily, much easier to block the user for windows until it is fixed
later.
|
|
My point being you already need class definitions or hiera for each platform regardless. Checks, metrics, and subscriptions are not going to be cross platform. So somewhere some how you need to be setting slightly different values for linux vs windows. In this state an end user will set manage_user to true and then nothing happens on one platform. So its still broken, someone is expecting something to happen and then it doesn't. If anything I would personally rather see a case statement on osfamily similar to the one in init.pp but within the manage_user if statement that would set the user for linux and then send either a warning or fail notification for windows. This way it at least tells the user that we cant manage a windows user yet. I would rather see this, which is what I should have done to begin with: This allows us to enter into that if statement rather then block it entirely, warn that its not supported but dont error/fail, and provides the framework for future support. |
|
@dzeleski warning added |
Until better user management for windows installs can be added this is to prevent attempting to create user/group on windows platform regardless of the manage_user parameter. Fixes issue sensu#617 * NOTE: This would have been better as a case statement in a params.pp class but since that isn't implemented this is a crude workaround for the time being. This is also the default requirement for PR sensu#588
|
Any news on this ? I'm wondering how this fix: #588 will work on localised systems ? Like french windows OS, the administrators is administrateurs, but it's difficult for me as i have english & french Windows OS... |
|
@jothoma1 what issues are you having? This was merged: https://github.com/sensu/sensu-puppet/pull/628/files Which fixes this issue. Locale should not matter as we are just installing sensu with the default service that the installer uses. |
|
@dzeleski i dont understand, i dont have these merged on my module... but i upgraded from https://forgeapi.puppetlabs.com with
Do you know why ? |
|
@jothoma1 a new version has not been published to the forge in quite some time. I would recommend pulling via commit hash from git. If you want more control you can fork the repo. |
|
@dzeleski ok i will do that ! thanks |
|
I just released 2.2.1 to the forge, if that helps. |
|
@jeffmccune what's the status on this since we merged your windows work? |
Looking into it now. |
Description of problem
Running a basic class to install the client on windows will result in and attempt to create a local user named "sensu". Any password complexity rules set on the host will force an error, yet I can't find any parameters within the class to manually override the defaults for this account (neither name nor password). furthermore, manual install of the agent succeeds without the need to even create said local account, which leads me to question it's purpose? (I am new to sensu, so any insight would be helpful if I'm mistaken)
The text was updated successfully, but these errors were encountered: