Skip to content

Add password_file to type #20

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

Merged
merged 2 commits into from
May 24, 2013
Merged

Add password_file to type #20

merged 2 commits into from
May 24, 2013

Conversation

raphink
Copy link
Contributor

@raphink raphink commented May 3, 2013

This PR adds a password_file parameter to pass a file instead of a string for the password.

Note: the provider specs are broken in master, I did not intend to fix them.

isrequired
newparam(:password_file) do
desc 'The path to a file containing the password used to protect the
keystore. This cannot be used together with :password.'
Copy link
Contributor

Choose a reason for hiding this comment

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

You can probably add a validate block to check if someone specifies both :password_file and :password, like so: https://github.com/puppetlabs/puppetlabs-firewall/blob/master/lib/puppet/type/firewall.rb#L727-L729

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. Fixed (see below).

if File.exists?(@resource[:target]) and not File.zero?(@resource[:target])
tmpfile.write("#{@resource[:password]}\n#{@resource[:password]}")

tmpfile = nil
Copy link
Contributor

Choose a reason for hiding this comment

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

This adds a lot of logic to this method; I think that this could be extracted to a private method and called here to make this a bit more simple.

@raphink
Copy link
Contributor Author

raphink commented May 24, 2013

@adrienthebo: I added a password_file method to simplify import_ks. Is that better for you?

adrienthebo added a commit that referenced this pull request May 24, 2013
@adrienthebo adrienthebo merged commit 49bec31 into puppetlabs:master May 24, 2013
@adrienthebo
Copy link
Contributor

@raphink I opened up up #22 and #23 to fix the broken specs to make sure that everything passed before merging. Those PRs are in, and this has been merged so the build when it hits travis-ci. Thanks for the contribution!

@raphink
Copy link
Contributor Author

raphink commented May 24, 2013

Thanks for merging. I will now move on to the puppetdb part of using this change :-)

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.

3 participants