Migrate erb to epp templates

[nosrio]

Pull Request (PR) description

Replace erb templates for epp templats in order to handle sensitive data

This Pull Request (PR) fixes the following issues

Fixes: #966

[nosrio]

Hi @wyardley thanks for your feedback, i will check them.

However I am having some issues running the test s locally, I followed this docs, should I be using other?

[wyardley]

for me

$ bundle install
$ bundle exec rake test
## or
$ bundle exec rake spec

works. Once the fixtures are installed, can also run bundle exec rspec spec/classes/rabbitmq_spec.rb to run a specific spec file. Running the tests may fail fast if you don't also run

$ bundle exec rake strings:generate:reference

when it's needed (ditto for rubocop failures).

[wyardley]

This is something that gets things a lot closer to passing except for some slight quoting differences with spec/classes/rabbitmq_spec.rb:1587 in similar tests. That said, I'm not an expert on EPP, so I don't know how idiomatic this is.

Let me know if you've got questions; I can make a PR against your fork / branch if you'd like. https://github.com/nosrio/puppet-rabbitmq/compare/UseEppTemplates...wyardley:puppet-rabbitmq:wyardley/978?expand=1

diff --git a/REFERENCE.md b/REFERENCE.md
index 6c95727..1cf5a2a 100644
--- a/REFERENCE.md
+++ b/REFERENCE.md
@@ -504,7 +504,7 @@ Data type: `String`

 The template file to use for rabbitmq_env.config.

-Default value: `'rabbitmq/rabbitmq-env.conf.erb'`
+Default value: `'rabbitmq/rabbitmq-env.conf.epp'`

 ##### <a name="-rabbitmq--env_config_path"></a>`env_config_path`

diff --git a/data/common.yaml b/data/common.yaml
index c25d117..26e6699 100644
--- a/data/common.yaml
+++ b/data/common.yaml
@@ -15,7 +15,7 @@ rabbitmq::config_shovel_statics: {}
 rabbitmq::default_user: 'guest'
 rabbitmq::default_pass: 'guest'
 rabbitmq::delete_guest_user: false
-rabbitmq::env_config: 'rabbitmq/rabbitmq-env.conf.erb'
+rabbitmq::env_config: 'rabbitmq/rabbitmq-env.conf.epp'
 rabbitmq::env_config_path: '/etc/rabbitmq/rabbitmq-env.conf'
 rabbitmq::erlang_cookie: ~
 rabbitmq::interface: ~
diff --git a/manifests/config.pp b/manifests/config.pp
index 9cc0f6f..f1a9739 100644
--- a/manifests/config.pp
+++ b/manifests/config.pp
@@ -190,13 +190,14 @@ class rabbitmq::config {
   }

   if $use_config_file_for_plugins {
-    $managent_plugin = $admin_enable or $management_enable ? { true => 'rabbitmq_management', false => undef }
+    $management_plugin = ($admin_enable or $management_enable) ? { true => 'rabbitmq_management', false => undef }
     $stomp_plugin = $stomp_ensure ? { true => 'rabbitmq_stomp', false => undef }
     $auth_backend_ldap_plugin = $ldap_auth ? { true => 'rabbitmq_auth_backend_ldap', false => undef }
     $shovel_plugin = $config_shovel ? { true => 'rabbitmq_shovel', false => undef }
-    $shovel_management_plugin = $config_shovel and ($admin_enable or $management_enable) ? { true => 'rabbitmq_shovel_management', false => undef }
+    $shovel_management_plugin = ($config_shovel and ($admin_enable or $management_enable)) ? { true => 'rabbitmq_shovel_management', false => undef }
+
+    $_plugins = delete_undef_values($plugins + [$management_plugin, $stomp_plugin, $auth_backend_ldap_plugin, $shovel_plugin, $shovel_management_plugin])

-    $_plugins = join($plugins, $managent_plugin, $stomp_plugin, $auth_backend_ldap_plugin, $shovel_plugin, $shovel_management_plugin)
     file { 'enabled_plugins':
       ensure  => file,
       path    => '/etc/rabbitmq/enabled_plugins',
diff --git a/templates/default.epp b/templates/default.epp
index 91948d3..eef7b9b 100644
--- a/templates/default.epp
+++ b/templates/default.epp
@@ -7,8 +7,8 @@
 # Maximum number of open file handles. This will need to be increased
 # to handle many simultaneous connections. Refer to the system
 # documentation for ulimit (in man bash) for more information.
-ulimit -n <%= $file_limit %>
+ulimit -n <%= $::rabbitmq::file_limit %>

 # OOM score. It sets the score of the init script, but as this value is
 # inherited, it also applies to the rabbitmq-server.
-echo <%= $oom_score_adj %> > /proc/$$/oom_score_adj
+echo <%= $::rabbitmq::oom_score_adj %> > /proc/$$/oom_score_adj
diff --git a/templates/enabled_plugins.epp b/templates/enabled_plugins.epp
index 1e44ec1..fb46ef3 100644
--- a/templates/enabled_plugins.epp
+++ b/templates/enabled_plugins.epp
@@ -1,3 +1,4 @@
+<% include stdlib -%>
 % This file managed by Puppet
 % Template Path: <%= $module_name %>/templates/enabled_plugins
-[<%= ($_plugins.uniq).join(',')%>].
+[<%= ($::rabbitmq::config::_plugins.unique).join(',')%>].
diff --git a/templates/rabbitmq-env.conf.epp b/templates/rabbitmq-env.conf.epp
index fa1124e..e9b5eef 100644
--- a/templates/rabbitmq-env.conf.epp
+++ b/templates/rabbitmq-env.conf.epp
@@ -1,5 +1,6 @@
-<% $environment_variables.keys.sort.each |$k| { -%>
-  <%- unless $environment_variables[$k] == Undef {-%>
-<%= $k %>=<%= $environment_variables[$k] %>
+<% include stdlib -%>
+<% $::rabbitmq::config::environment_variables.keys.sort.each |$k| { -%>
+  <%- unless $::rabbitmq::config::environment_variables[$k] == undef {-%>
+    <%= $k %>=<%= $::rabbitmq::config::environment_variables[$k] %>
   <%-} -%>
 <% } -%>
diff --git a/templates/rabbitmqadmin.conf.epp b/templates/rabbitmqadmin.conf.epp
index 209b226..3c92c81 100644
--- a/templates/rabbitmqadmin.conf.epp
+++ b/templates/rabbitmqadmin.conf.epp
@@ -1,16 +1,16 @@
 [default]
-<% if $ssl and $management_ssl {-%>
+<% if $rabbitmq::ssl and $rabbitmq::management_ssl {-%>
 ssl = True
-ssl_ca_cert_file = <%= $ssl_management_cacert %>
-ssl_cert_file = <%= $ssl_management_cert %>
-ssl_key_file = <%= $ssl_management_key %>
-port = <%= $ssl_management_port %>
-<% unless $management_hostname {-%>
+ssl_ca_cert_file = <%= $rabbitmq::ssl_management_cacert %>
+ssl_cert_file = <%= $rabbitmq::ssl_management_cert %>
+ssl_key_file = <%= $rabbitmq::ssl_management_key %>
+port = <%= $rabbitmq::ssl_management_port %>
+<% unless $rabbitmq::management_hostname {-%>
 hostname = <%= $fqdn %>
 <% } -%>
 <% } else {-%>
-port = <%= $management_port %>
+port = <%= $rabbitmq::management_port %>
 <% } -%>
-<% if $management_hostname { %>
-hostname = <%= $management_hostname %>
+<% if $rabbitmq::management_hostname { %>
+hostname = <%= $rabbitmq::management_hostname %>
 <% } -%>

[wyardley]

Were you able to get the tests working locally? It’s going to be a slog getting the remaining issues resolved without that.

[nosrio]

Thanks, I have merged all your comments. I will try to rewrite as epp the rabbitmq.config.erb file.

[wyardley]

Thanks, I have merged all your comments. I will try to rewrite as epp the rabbitmq.config.erb file.

Thanks - let me know when you want me to do a final review -- I'm Ok doing this more incrementally too, though, if you'd rather do that part separately. In the meantime, I tagged a couple other reviewers; you could also try asking for feedback on IRC.

Once you've got stuff where you want it, squashing down to one commit might also be a good idea.

[nosrio]

@wyardley I have pushed the epp template for rabbitmq.config file. Let me known if there anything else I should change or review.

Update: I see that acceptance tests are failling, mostly due to sintax error on rendered rabbitmq.config, I will check that later. Let me know if you find any other thing I should fix.

[nosrio]

I believe I had fixed all the failing tests and used the code suggested by @ekohl for the plugin configs.

@wyardley let me know if I can squash all the commit before merging this PR or any more change is needed.

[nosrio]

Commits squashed. Thanks for the help :)