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

Add a new boolean configuration option for copying of SSL certificates. #219

Closed
wants to merge 7 commits into from

Conversation

hdanes
Copy link
Contributor

@hdanes hdanes commented Jan 2, 2014

Introduced a new option to disable or enable copying SSL related files like certificates, keys, etc.

For example if there is a shared wildcard certificate there are a lot of copies created of the same certificate which will mess up the Nginx configuration folder. As discussed in issue #80 the default system directory should be used.

Example of usage changeset:
class { 'nginx':
copy_ssl_files => false,
}

nginx::resource::vhost { 'name':
ensure => present,
ssl => true,
ssl_cert => '/etc/ssl/certs/file.pem',
ssl_key => '/etc/ssl/certs/file.key',
ssl_dhparam => '/etc/ssl/certs/file.dhparams.pem',
}

The 'copy' option is enabled by default for backward compatibility. It might be a solution to remove this option in one of the upcoming releases.

@CpuID
Copy link

CpuID commented Jan 6, 2014

+1, this would be awesome to see submitted.

@3flex
Copy link
Contributor

3flex commented Jan 15, 2014

+1

@3flex
Copy link
Contributor

3flex commented Jan 16, 2014

It would be great to have the value validated as a boolean, and also some rspec tests to test the new behaviour.

If the intention is to deprecate the "true" value in future (which I personally agree with) then consider adding a warning to that effect, but that's up to the maintainer.

@hdanes
Copy link
Contributor Author

hdanes commented Jan 25, 2014

I've implemented the validation of the boolean value. Unfortunately I haven't enough time to write the rspec tests at this moment. Is it possible to add them later in a separate pull request?

@jfryman Could you share your opinion about the deprecation warning (see comment of @3flex)?

… ssl_cert

Conflicts:
	manifests/config.pp
	manifests/init.pp
@CpuID
Copy link

CpuID commented Mar 19, 2014

Hey Guys,

Any updates to this? bump :)

@jfryman
Copy link
Contributor

jfryman commented Mar 19, 2014

@hdanes I agree with @3flex. A deprecation warning would be 👍

@hdanes hdanes closed this Apr 18, 2014
@hdanes hdanes deleted the ssl_cert branch April 18, 2014 08:52
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.

4 participants