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

Ability to set interface to listen for SSL connections #470

Merged
merged 1 commit into from Jan 9, 2018

Conversation

Projects
None yet
2 participants
@k-popov
Copy link
Contributor

commented Jan 3, 2018

Attribute node['rabbitmq']['ssl_listen_interface'] is set will define specific interface to bind.
May be filled with "0.0.0.0" (this will result in listening on IPv4 interfaces).
Will not affect existing config if left default (nil)

@michaelklishin

This comment has been minimized.

Copy link
Member

commented Jan 3, 2018

Please take a look at why the unit and lint Travis run fails. It's usually a relatively easy part to debug compared to integration tests.

@@ -113,6 +113,7 @@
# ssl
default['rabbitmq']['ssl'] = false
default['rabbitmq']['ssl_port'] = 5671
default['rabbitmq']['ssl_listen_interface'] = nil # will not be mentioned in config if left nil

This comment has been minimized.

Copy link
@michaelklishin

michaelklishin Jan 3, 2018

Member

That's the way it works for many attributes in many cookbooks, please leave this comment out.

This comment has been minimized.

Copy link
@michaelklishin

michaelklishin Jan 3, 2018

Member

Or, at least move the comment above the line and make it say that the default will make RabbitMQ listen on all interfaces: that may or may not be obvious.

This comment has been minimized.

Copy link
@k-popov

k-popov Jan 3, 2018

Author Contributor

Updated. Waiting for CI to complete.

This comment has been minimized.

Copy link
@k-popov

k-popov Jan 3, 2018

Author Contributor

The failure looks not being related to changes:

 1) rabbitmq::default ubuntu installs the rabbitmq-server deb_package with the default action

Will try to re-trigger test.

@k-popov k-popov force-pushed the k-popov:set_ssl_interface branch from 820113b to 5a980b1 Jan 3, 2018

@k-popov k-popov closed this Jan 3, 2018

@k-popov k-popov reopened this Jan 3, 2018

@k-popov

This comment has been minimized.

Copy link
Contributor Author

commented Jan 3, 2018

@michaelklishin could you please review #471 ? It fixes the unit test that's failing builds for this PR.

@k-popov k-popov closed this Jan 9, 2018

@michaelklishin

This comment has been minimized.

Copy link
Member

commented Jan 9, 2018

Was this really meant to be closed? Looks like #471 is an addition to this PR, not a replacement for it?

@michaelklishin michaelklishin reopened this Jan 9, 2018

@michaelklishin

This comment has been minimized.

Copy link
Member

commented Jan 9, 2018

I'm going to merge this and #471 at the same time manually.

@michaelklishin michaelklishin merged commit 5a980b1 into rabbitmq:master Jan 9, 2018

1 check failed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.