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.

Sign up for GitHub

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 loopback_users parameter (adds ability to allow guest user to login remotely) #699

Merged
merged 13 commits into from
Apr 24, 2018
Merged

Add loopback_users parameter (adds ability to allow guest user to login remotely) #699

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
d06fd80
Introduces a new parameter allow_remote_connections, to change the de…
Apr 19, 2018
70fa0c6
Default value for the allow_remote_connections parameter, this doesn'…
Apr 19, 2018
ee01521
Pass the new allow_remote_connections parameter
Apr 19, 2018
cd0d316
Handles the configuration file with the new parameter
Apr 19, 2018
b00acb6
The test for the new parameter
Apr 19, 2018
371c818
Renames the new module variable to allow_remote_guest_connections
Apr 23, 2018
01241d5
Adds the new parameter loopback_users, to manage the access to the lo…
Apr 24, 2018
1d24a6f
Gets the new lookback_user parameter
Apr 24, 2018
99e993b
Configures the default value for the loopback_users list with the 'gu…
Apr 24, 2018
1aebcf7
Handles the loopback_users list in the configuration template
Apr 24, 2018
45b94fb
Adds a new test case for a configured list of users
Apr 24, 2018
42044fb
Solves a rubocop offense related with a list
Apr 24, 2018
84b65d9
Makes not optional the loopback_users parameter
Apr 24, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions manifests/config.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@
$inetrc_config = $rabbitmq::inetrc_config
$inetrc_config_path = $rabbitmq::inetrc_config_path
$ssl_erl_dist = $rabbitmq::ssl_erl_dist
$loopback_users = $rabbitmq::loopback_users

if $ssl_only {
$default_ssl_env_variables = {}
Expand Down
2 changes: 2 additions & 0 deletions manifests/init.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,7 @@
# @param rabbitmq_home OS dependent. default defined in param.pp. The home directory of the rabbitmq deamon.
# @param $rabbitmqadmin_package OS dependent. default defined in param.pp. If undef: install rabbitmqadmin via archive, otherwise via package
# @param $archive_options. default defined in param.pp. Extra options to Archive resource to download rabbitmqadmin file
# @param $loopback_users. default defined in param.pp. This option configures a list of users to allow access via the loopback interfaces
class rabbitmq(
Boolean $admin_enable = $rabbitmq::params::admin_enable,
Enum['ram', 'disk', 'disc'] $cluster_node_type = $rabbitmq::params::cluster_node_type,
Expand Down Expand Up @@ -278,6 +279,7 @@
Boolean $ssl_erl_dist = $rabbitmq::params::ssl_erl_dist,
Optional[String] $rabbitmqadmin_package = $rabbitmq::params::rabbitmqadmin_package,
Array $archive_options = $rabbitmq::params::archive_options,
Optional[Array] $loopback_users = $rabbitmq::params::loopback_users,
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a strange quirk of how Puppet works, but a param can't be Optional if it has a default. Well, technically, it can, but it can only be overridden with undef using hiera, so it's safer to not make it optional, and pass in an empty array (many people's preferred pattern anyway) to set it to an empty list when necessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, you're right, I'm working with puppet 4 and the automatic hiera bind then I forgot the "old" params pattern; I think an empty list as default value for this parameter is not the best choice to achieve the behaviour that you could expect, so I could make not optional this parameter a fix the default value to the list with the guest user, let's see

Copy link
Contributor

Choose a reason for hiding this comment

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

Right, I think the right fix is to make the parameter not optional, and if users want to allow the guest user to login remotely as well, they can pass in an empty array. You could explicitly mention that in the param docs if you want to make it more clear.

Copy link
Contributor

Choose a reason for hiding this comment

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

But the default value should be ['guest'] and then just Array $loopback_users. I think we're saying the same thing 😃

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, we're in the same page

) inherits rabbitmq::params {

if $ssl_only and ! $ssl {
Expand Down
1 change: 1 addition & 0 deletions manifests/params.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,4 +145,5 @@
$inetrc_config = 'rabbitmq/inetrc.erb'
$inetrc_config_path = '/etc/rabbitmq/inetrc'
$archive_options = []
$loopback_users = ['guest']
}
25 changes: 25 additions & 0 deletions spec/classes/rabbitmq_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1411,6 +1411,31 @@
end
end

describe 'rabbitmq-loopback_users by default' do
it 'sets the loopback_users parameter in the config file' do
is_expected.to contain_file('rabbitmq.config'). \
with_content(%r{\{loopback_users, \[<<"guest">>\]\}})
end
end

describe 'rabbitmq-loopback_users allow connections via loopback interfaces' do
let(:params) { { loopback_users: [] } }

it 'sets the loopback_users parameter in the config file' do
is_expected.to contain_file('rabbitmq.config'). \
with_content(%r{\{loopback_users, \[\]\}})
end
end

describe 'rabbitmq-loopback_users allow connections via loopback interfaces to a group of users' do
let(:params) { { loopback_users: ["user1", "user2"] } }

it 'sets the loopback_users parameter in the config file' do
is_expected.to contain_file('rabbitmq.config'). \
with_content(%r{\{loopback_users, \[<<\"user1\">>, <<\"user2\">>\]\}})
end
end

##
## rabbitmq::service
##
Expand Down
1 change: 1 addition & 0 deletions templates/rabbitmq.config.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
<%- if @heartbeat -%>
{heartbeat, <%=@heartbeat%>},
<% end -%>
{loopback_users, [<%= @loopback_users.map { |u| "<<\"#{u}\">>" }.join(', ') %>]},
<% if @auth_backends -%>
{auth_backends, [<%= @auth_backends.map { |v| "#{v}" }.join(', ') %>]},
<% elsif @ldap_auth -%>
Expand Down