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

Added API parameters for user and password #119

Merged
merged 1 commit into from
Jan 14, 2014

Conversation

solarkennedy
Copy link
Contributor

This is my first attempt at adding the capability of setting the API username and password.

The use of types for configuring sensu is a little advanced for me, but I think I got it right.

It does have an unexpected (to me) behavior of not removing the api_user and api_password from the json when you remove the parameters. I guess this is a side effect of how the provider is implemented? May I ask, why not just use an erb?

I don't understand the build failures though. They seem to be unrelated to my changes? And I seem to get them, even when I run against the upstream master?

I've also set the sensitive config files to 440.

@jlambert121
Copy link
Contributor

Thanks for the PR!

I think under the create function to have api_user and api_password removed you're going to need something like:

self.user = resource[:user] unless resource[:user].nil?
self.password = resource[:password] unless resource[:password].nil?

There are a couple of build failures going on. One of them is unrelated to you - Travis isn't honoring the exclude matrix for certain versions of puppet and ruby. There are two syntax errors puppet-lint is complaining about, looking at them trying to fix travis neither are yours, I'll make a PR to fix them separately.

@solarkennedy
Copy link
Contributor Author

Well, having:

self.user = resource[:user] unless resource[:user].nil?
self.password = resource[:password] unless resource[:password].nil?

Makes the provider converge on the first run (good), but it still doesn't actively remove user/password entries in the json when nil. I've tried experimenting with self.delete('password') But I can't get it to work. Any other advice? The create function doesn't seem to be continuously called.

Without this functionality, there pretty much is no other way to programmatically remove the user/password lines without like, augeas or something. The provider isn't setup to ensure=>absent on individual pieces.

@jlambert121
Copy link
Contributor

Doing some tests on our existing providers I found they don't remove attributes with undef either. @jamtur01 pointed me to http://projects.puppetlabs.com/issues/15329 which looks like it is unlikely to be fixed until puppet 4.

Given the problem already exists (and that it is probably unlikely someone sets a u:p and then wants to remove it) I think adding the functionality is more of a benefit than not because of an existing bug.

Due to the large number of changes since you submitted this though, could you rebase it off of master so it can be merged? If the rebase gives you a bunch of complaining (or if you'd rather not deal with it), I can push a new PR with these changes.

@ghost
Copy link

ghost commented Jan 9, 2014

I had a crack at rebasing this PR against master. I think it went ok, but then ran into an autoload issue with one file in the puppet_x dir, but that may be due to using puppet enterprise. Perhaps it'll save a bit of effort, maybe. I'd really like to see the ability to manage the api u:p with this puppet module.

https://github.com/robertpearce/sensu-puppet/tree/pr119

@solarkennedy solarkennedy deleted the api_params branch January 9, 2014 03:51
@solarkennedy
Copy link
Contributor Author

Er, didn't mean to close this. I've rebased this.

@solarkennedy solarkennedy reopened this Jan 9, 2014
@jlambert121
Copy link
Contributor

Thanks!

jlambert121 added a commit that referenced this pull request Jan 14, 2014
Added API parameters for user and password
@jlambert121 jlambert121 merged commit 7fecb8c into sensu:master Jan 14, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants