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

Make splunk_user configurable. #195

Merged
merged 4 commits into from
Sep 5, 2018

Conversation

mdwheele
Copy link
Contributor

@mdwheele mdwheele commented Sep 5, 2018

Hello!

Pull Request (PR) description

This PR addresses #159 without making any backwards-incompatible changes. I would like to be able to configure the Splunk user but have no opinion I wish to assert over what should be the default for this module. There are certainly best-practices from Splunk that advise running as splunk, but the default for a long time was to run as Administrator or root, as configured here by default. In 2018, this is no longer the case. However, I could totally see environments with extreme change control processes that MUST maintain current configs (and have likely implemented compensating controls to protect against the fact that Splunk has higher privilege than necessary).

All that said, I think enabling folks to make the call for their own environment is the best path forward and would appreciate your consideration!

@@ -86,6 +89,10 @@
$server = 'splunk',
$forwarder_installdir = undef,
$server_installdir = undef,
$splunk_user = $::osfamily ? {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you please use facts$['os']['family'] as fact here? The old toplevel variables are deprecated.

@bastelfreak bastelfreak added enhancement New feature or request needs-work not ready to merge just yet labels Sep 5, 2018
@mdwheele
Copy link
Contributor Author

mdwheele commented Sep 5, 2018

👍

@bastelfreak
Copy link
Member

Can you take a look at the failing spec tests?

@mdwheele
Copy link
Contributor Author

mdwheele commented Sep 5, 2018

It looks like it simply couldn't do the lookup in parameters list because :facts wasn't populated to match expectations of the new default assignment out of $facts['os']['family']. I think it's good to go now.

@bastelfreak bastelfreak merged commit d006ce1 into voxpupuli:master Sep 5, 2018
@bastelfreak
Copy link
Member

thanks!

@bastelfreak bastelfreak removed the needs-work not ready to merge just yet label Sep 5, 2018
@mdwheele
Copy link
Contributor Author

mdwheele commented Sep 5, 2018

No problem! I really appreciate all the work y'all have put into this module. It's very helpful.

@mdwheele mdwheele deleted the configurable_splunkd_user branch September 5, 2018 20:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants