Skip to content
This repository has been archived by the owner on Dec 20, 2024. It is now read-only.

Adding options for the rpc audit provider #246

Merged
merged 1 commit into from
Oct 20, 2015

Conversation

jaxxstorm
Copy link
Contributor

It's currently defaulting to 1, which means it's insecure by default.
I've set it to 1 by default, but at least now it can be overriden.

Tests included

It's currently defaulting to 1, which means it's insecure by default.
I've set it to 1 by default, but at least now it can be overriden.

Tests included
@@ -63,6 +63,9 @@
$ssl_server_private = undef,
$ssl_client_certs = 'puppet:///modules/mcollective/empty',
$ssl_client_certs_dir = undef, # default dependent on $confdir

# Action policy settings
$allowunconfigured = '1',
Copy link
Contributor

Choose a reason for hiding this comment

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

would you mind changing this name to allow_unconfigured like the option is named as well?

@igalic
Copy link
Contributor

igalic commented Oct 19, 2015

@jaxxstorm thank you very much for this contribution! could you also document this new option?

@ffrank
Copy link
Contributor

ffrank commented Oct 19, 2015

I kind of feel that we'd want to accept true/false values here as well, even if the tooling underneath does not, and do the right thing with those. Thoughts?

@igalic
Copy link
Contributor

igalic commented Oct 20, 2015

@ffrank i'm absolutely a big fan of that.
see also #238

@jaxxstorm
Copy link
Contributor Author

I agree on the whole, but it seems like overkill to have to refactor a bunch of stuff just to get this merged. It has parity with the current settings. It'll take me a little while longer to write tests/manifests for true == 1 and false == 0

igalic added a commit that referenced this pull request Oct 20, 2015
Adding options for the rpc audit provider
@igalic igalic merged commit 92f57d8 into voxpupuli:master Oct 20, 2015
@igalic
Copy link
Contributor

igalic commented Oct 20, 2015

0/1 is a mental overhead, compared to true / false.
it's not about getting this done now, it was just a general remark about getting it done eventually ;)

@jaxxstorm
Copy link
Contributor Author

No worries! It's not documented, but thanks for merging :)

@igalic
Copy link
Contributor

igalic commented Oct 20, 2015

argH! how did i not notice that :( #247

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants