-
Notifications
You must be signed in to change notification settings - Fork 79
Add ability to manage ssl.conf file within apache::ssl #76
base: master
Are you sure you want to change the base?
Conversation
class apache::ssl { | ||
class apache::ssl ( | ||
$ssl_port = params_lookup( 'ssl_port' ), | ||
$ssl_source = params_lookup( 'ssl_source' ), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Honestly I've always used params_lookup on the main class, and not in sub classes, I'm not sure about the params namespace and the functionality when using it in subclasses.
Incidentally I think it's better to have params in the classes where they are used, as here, but that's not a pattern I've used in ng modules.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wasn't sure, but it didn't seem to make sense to have a node declaration like:
class { 'apache':
ssl_template => 'site/apache/ssl.conf.erb',
}
class { 'apache::ssl': }
I was concerned about it causing confusion when 1) user specifies the param in the apache class and doesn't declare apache:ssl, 2) user attempts to specify the param in the apache::ssl class, and 3) when using an ENC, where the param would be specified as apache_ssl_template might make it look like apache::ssl::template instead of apache::ssl_template if that makes sense.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree, better to leave the params in the ssl class.
Still looking at the params_lookup function (https://github.com/example42/puppi/blob/master/lib/puppet/parser/functions/params_lookup.rb), I wonder if the lookup names are what we expect. For example I'm not sure the default values are searched in apache::params
Hi. I'm searching for a solution to do exact what the pull request does: specifying the ssl.conf content on my own. Is there a chance to get the request merged into the project? Regards |
@awegmann can you confirm that the default values of the params of apache::ssl are evaluated as defined in the params class? |
@alvagante I took the code from Sean and put it in my forked repo. Will check the functionality tomorrow. |
Added $ssl_template and $ssl_source parameters and enabled apache::ssl to accept these and $ssl_port directly. I tested and verified it works on Oracle Linux and CentOS 6.
One additional note, I localized parameters in apache::ssl so the monitoring/firewalling params are not defined by the parent class.