Support for SSL client cert authentication and server certificate validation when connecting to RabbitMQ #4541
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
server certificate using provided CA bundle for message bus (RabbitMQ) connections. Option names are consistent with the same option names for MongoDB. Update affected code so connection and URLs are only retrieved in a single place.
arm4b
reviewed
Feb 8, 2019
| self._publisher = SharedPoolPublishers().get_publisher(urls=urls) | ||
| self._exchange = exchange | ||
|
|
||
| def publish_create(self, payload): | ||
| with Timer(key='amqp.publish.create'): | ||
| self._publisher.publish(payload, self._exchange, CREATE_RK) | ||
| self._publisher.publgish(payload, self._exchange, CREATE_RK) |
Kami
changed the title
Kami
added a commit
to StackStorm/st2docs
that referenced
this pull request
Feb 11, 2019
Kami
commented
Feb 11, 2019
|
|
||
| class AnnouncementPublisher(object): | ||
| def __init__(self, urls): | ||
| def __init__(self, urls=None): |
There was a problem hiding this comment.
Connection retrieval and URL handling is now centralized in a single place so there is really no need for this url argument here anymore.
To make the code nicer, I could simply remove it from all those classes.
there is no need for this argument anymore.
|
@Kami No, we don't auto-generate SSL/CA in K8s/HA work, leaving that part for user consideration/configuration. |
arm4b
reviewed
Feb 12, 2019
| @@ -41,7 +41,7 @@ | |||
|
|
|||
| SKIP_GROUPS = ['api_pecan', 'rbac', 'results_tracker'] | |||
|
|
|||
| # We group auth options together to nake it a bit more clear what applies where | |||
…ection_ssl_options
listener on port 5671. Default non SSL listener is exposed on port 5672.
Kami
force-pushed
the
rabbitmq_connection_ssl_options
branch
from
f091833 to
3b77449
Compare
RabbitMQ on Travis tests.
uncomment to test this functionality.
|
I pushed changes so we now also expose SSL listener when running RabbitMQ on Travis (67fc03f, 3b77449, 922cb23) and added some basic tests which verify that those parameters work as expected (88a6d57). In the future, we should do the same for MongoDB (we don't have any integration tests for SSL related parameters, just unit ones which are lacking). NOTE: At the moment we only run those tests on Travis, because RabbitMQ SSL listener is not set up on local vagrant dev VMs. |
Kami
force-pushed
the
rabbitmq_connection_ssl_options
branch
from
1eaa005 to
0c69ebd
Compare
Kami
force-pushed
the
rabbitmq_connection_ssl_options
branch
from
0c69ebd to
5387ece
Compare
Kami
force-pushed
the
rabbitmq_connection_ssl_options
branch
from
36cbe7a to
44e513e
Compare
ancient Precise version.
Kami
added a commit
to StackStorm/st2docs
that referenced
this pull request
Mar 6, 2019
cognifloyd
added a commit
to cognifloyd/st2
that referenced
this pull request
Feb 21, 2020
These tests were added in StackStorm#4541. Now that we're on xenial, we can reenable them. This skips upgrading rabbitmq. reverts 44e513e originally added in 5387ece reverts 922cb23
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull request adds two of the features we were missing for fully secure and verified connections to RabbitMQ message bus:
Both of those features were already supported for database (MongoDB) connection, but not for RabbitMQ.
Keep in mind that secure SSL / TLS connections to RabbitMQ were already possible (without verifying server certificate), but that wasn't documented anywhere. StackStorm/st2docs#848 fixed that.
Nice thing is that we can use same naming for those options for
databaseandmessagingsection (ssl_keyfile,ssl_certfile,ssl_cert_reqes,ssl_ca_certs).Example config entries:
Implementation wise, I needed to refactor some code so we now retrieve a properly configured
Connectionobject in a single centralized place.TODO