Merge lp:~niedbalski/charms/trusty/rabbitmq-server/fix-1442443 into lp:~openstack-charmers-archive/charms/trusty/rabbitmq-server/next
- Trusty Tahr (14.04)
- fix-1442443
- Merge into next
Status: | Superseded | ||||
---|---|---|---|---|---|
Proposed branch: | lp:~niedbalski/charms/trusty/rabbitmq-server/fix-1442443 | ||||
Merge into: | lp:~openstack-charmers-archive/charms/trusty/rabbitmq-server/next | ||||
Diff against target: |
707 lines (+392/-152) 9 files modified
config.yaml (+23/-1) hooks/rabbit_utils.py (+37/-39) hooks/rabbitmq_context.py (+122/-0) hooks/rabbitmq_server_relations.py (+7/-99) hooks/ssl_utils.py (+66/-0) templates/rabbitmq.config (+25/-11) tests/50_test_cluster_partition.py (+31/-0) unit_tests/test_rabbitmq_context.py (+79/-0) unit_tests/test_rabbitmq_server_relations.py (+2/-2) |
||||
To merge this branch: | bzr merge lp:~niedbalski/charms/trusty/rabbitmq-server/fix-1442443 | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
James Page | Needs Fixing | ||
Billy Olsen | Pending | ||
Review via email: mp+255872@code.launchpad.net |
This proposal supersedes a proposal from 2015-04-10.
This proposal has been superseded by a proposal from 2015-04-16.
Commit message
Description of the change
- Added the cluster_
- Refactored the ssl handling code.
- Added a generic config loader
Billy Olsen (billy-olsen) wrote : Posted in a previous version of this proposal | # |
uosci-testing-bot (uosci-testing-bot) wrote : Posted in a previous version of this proposal | # |
charm_lint_check #3202 rabbitmq-server for niedbalski mp255770
LINT OK: passed
uosci-testing-bot (uosci-testing-bot) wrote : Posted in a previous version of this proposal | # |
charm_unit_test #2990 rabbitmq-server for niedbalski mp255770
UNIT OK: passed
uosci-testing-bot (uosci-testing-bot) wrote : Posted in a previous version of this proposal | # |
charm_amulet_test #3019 rabbitmq-server for niedbalski mp255770
AMULET FAIL: amulet-test failed
AMULET Results (max last 2 lines):
make: *** [functional_test] Error 1
ERROR:root:Make target returned non-zero.
Full amulet test output: http://
Build: http://
uosci-testing-bot (uosci-testing-bot) wrote : Posted in a previous version of this proposal | # |
charm_lint_check #3210 rabbitmq-
LINT OK: passed
uosci-testing-bot (uosci-testing-bot) wrote : Posted in a previous version of this proposal | # |
charm_unit_test #2998 rabbitmq-
UNIT OK: passed
uosci-testing-bot (uosci-testing-bot) wrote : Posted in a previous version of this proposal | # |
charm_amulet_test #3027 rabbitmq-
AMULET FAIL: amulet-test failed
AMULET Results (max last 2 lines):
make: *** [functional_test] Error 1
ERROR:root:Make target returned non-zero.
Full amulet test output: http://
Build: http://
Edward Hope-Morley (hopem) : Posted in a previous version of this proposal | # |
uosci-testing-bot (uosci-testing-bot) wrote : Posted in a previous version of this proposal | # |
charm_unit_test #3005 rabbitmq-
UNIT OK: passed
uosci-testing-bot (uosci-testing-bot) wrote : Posted in a previous version of this proposal | # |
charm_lint_check #3217 rabbitmq-
LINT OK: passed
uosci-testing-bot (uosci-testing-bot) wrote : | # |
charm_lint_check #3224 rabbitmq-
LINT OK: passed
uosci-testing-bot (uosci-testing-bot) wrote : | # |
charm_unit_test #3012 rabbitmq-
UNIT OK: passed
uosci-testing-bot (uosci-testing-bot) wrote : | # |
charm_amulet_test #3041 rabbitmq-
AMULET FAIL: amulet-test failed
AMULET Results (max last 2 lines):
make: *** [functional_test] Error 1
ERROR:root:Make target returned non-zero.
Full amulet test output: http://
Build: http://
James Page (james-page) wrote : | # |
Please can you review the amulet test failures - they appear to be related to SSL.
Jorge Niedbalski (niedbalski) wrote : | # |
@james-page:
Seems that this tests has been failing since Feb 23 (I tested rev 86), because the certificates being used in amulet tests expired:
verify error:num=
notAfter=Feb 23 20:08:18 2015 GMT
This is not directly implied to my proposed change, and seems that https:/
I would appreciate if you can review this proposal.
uosci-testing-bot (uosci-testing-bot) wrote : | # |
charm_unit_test #3086 rabbitmq-
UNIT OK: passed
uosci-testing-bot (uosci-testing-bot) wrote : | # |
charm_lint_check #3298 rabbitmq-
LINT OK: passed
uosci-testing-bot (uosci-testing-bot) wrote : | # |
charm_amulet_test #3115 rabbitmq-
AMULET FAIL: amulet-test failed
AMULET Results (max last 2 lines):
make: *** [functional_test] Error 1
ERROR:root:Make target returned non-zero.
Full amulet test output: http://
Build: http://
Billy Olsen (billy-olsen) wrote : | # |
With these changes, this LGTM. I tested some of the partitioning logic here and things seem fine by me. It appears that an unknown value for cluster_
The fix for LP#1436014 is already in the queue and I don't think it should particularly hold this up, however osci bails as soon as the first test fails and I can't see the full output.
I'm running the amulet tests against your change + the fix for 1436014 and will mark it based on the findings of that.
As always, thanks for the submission Jorge!
- 89. By Liam Young
-
[gnuoy,trivial] Pre-release charmhelper sync
- 90. By James Page
-
[wolsen,
r=james- page] Fixup SSL tests to not fail when certs expire! - 91. By Jorge Niedbalski
-
Rebase commit
- 92. By Jorge Niedbalski
-
Rebase
- 93. By Jorge Niedbalski
-
addressing @dosaboy comments
- 94. By Jorge Niedbalski
-
addressing @dosaboy comments
- 95. By Jorge Niedbalski
-
addressing @dosaboy comments
- 96. By Jorge Niedbalski
-
addressing comments
- 97. By Jorge Niedbalski
-
Addressed @beisner comments
- 98. By Jorge Niedbalski
-
Addressed @beisner comments
Unmerged revisions
Preview Diff
1 | === modified file 'config.yaml' | |||
2 | --- config.yaml 2015-03-03 06:05:14 +0000 | |||
3 | +++ config.yaml 2015-04-13 17:16:27 +0000 | |||
4 | @@ -8,7 +8,7 @@ | |||
5 | 8 | type: string | 8 | type: string |
6 | 9 | default: "off" | 9 | default: "off" |
7 | 10 | description: | | 10 | description: | |
9 | 11 | Enable SSL connections on rabbitmq, valid values are 'off', 'on', 'only'. If ssl_key, | 11 | Enable SSL connections on rabbitmq, valid values are 'off', 'on', 'only'. If ssl_key, |
10 | 12 | ssl_cert, ssl_ca are provided then then those values will be used. Otherwise | 12 | ssl_cert, ssl_ca are provided then then those values will be used. Otherwise |
11 | 13 | the service will act as its own certificate authority and pass its ca cert to clients. | 13 | the service will act as its own certificate authority and pass its ca cert to clients. |
12 | 14 | For HA or clustered rabbits ssl key/cert must be provided. | 14 | For HA or clustered rabbits ssl key/cert must be provided. |
13 | @@ -92,6 +92,28 @@ | |||
14 | 92 | description: | | 92 | description: | |
15 | 93 | When set to true the 'ha-mode: all' policy is applied to all the exchages | 93 | When set to true the 'ha-mode: all' policy is applied to all the exchages |
16 | 94 | that match the expression '^(?!amq\.).*' | 94 | that match the expression '^(?!amq\.).*' |
17 | 95 | cluster-partition-handling: | ||
18 | 96 | type: string | ||
19 | 97 | default: ignore | ||
20 | 98 | description: | | ||
21 | 99 | RabbitMQ offers three ways to deal with network partitions automatically. | ||
22 | 100 | Available modes: | ||
23 | 101 | |||
24 | 102 | ignore - Your network is reliable. All your nodes are in a rack, | ||
25 | 103 | connected with a switch, and that switch is also the route to the outside world. | ||
26 | 104 | You don't want to run any risk of any of your cluster shutting down if any other part of it fails | ||
27 | 105 | (or you have a two node cluster). | ||
28 | 106 | |||
29 | 107 | pause_minority - Your network is maybe less reliable. You have clustered across 3 AZs in EC2, | ||
30 | 108 | and you assume that only one AZ will fail at once. | ||
31 | 109 | In that scenario you want the remaining two AZs to continue working and the nodes from the failed AZ | ||
32 | 110 | to rejoin automatically and without fuss when the AZ comes back. | ||
33 | 111 | |||
34 | 112 | autoheal - Your network may not be reliable. | ||
35 | 113 | You are more concerned with continuity of service than with data integrity. | ||
36 | 114 | You may have a two node cluster. | ||
37 | 115 | |||
38 | 116 | For more information see http://www.rabbitmq.com/partitions.html. | ||
39 | 95 | rbd-size: | 117 | rbd-size: |
40 | 96 | type: string | 118 | type: string |
41 | 97 | default: 5G | 119 | default: 5G |
42 | 98 | 120 | ||
43 | === modified file 'hooks/rabbit_utils.py' | |||
44 | --- hooks/rabbit_utils.py 2015-03-26 13:07:43 +0000 | |||
45 | +++ hooks/rabbit_utils.py 2015-04-13 17:16:27 +0000 | |||
46 | @@ -1,6 +1,4 @@ | |||
47 | 1 | import os | 1 | import os |
48 | 2 | import pwd | ||
49 | 3 | import grp | ||
50 | 4 | import re | 2 | import re |
51 | 5 | import socket | 3 | import socket |
52 | 6 | import sys | 4 | import sys |
53 | @@ -8,6 +6,13 @@ | |||
54 | 8 | import glob | 6 | import glob |
55 | 9 | import tempfile | 7 | import tempfile |
56 | 10 | 8 | ||
57 | 9 | from rabbitmq_context import ( | ||
58 | 10 | RabbitMQSSLContext, | ||
59 | 11 | RabbitMQClusterContext | ||
60 | 12 | ) | ||
61 | 13 | |||
62 | 14 | from charmhelpers.core.templating import render | ||
63 | 15 | |||
64 | 11 | from charmhelpers.contrib.openstack.utils import ( | 16 | from charmhelpers.contrib.openstack.utils import ( |
65 | 12 | get_hostname, | 17 | get_hostname, |
66 | 13 | ) | 18 | ) |
67 | @@ -30,8 +35,6 @@ | |||
68 | 30 | cmp_pkgrevno | 35 | cmp_pkgrevno |
69 | 31 | ) | 36 | ) |
70 | 32 | 37 | ||
71 | 33 | from charmhelpers.core.templating import render | ||
72 | 34 | |||
73 | 35 | from charmhelpers.contrib.peerstorage import ( | 38 | from charmhelpers.contrib.peerstorage import ( |
74 | 36 | peer_store, | 39 | peer_store, |
75 | 37 | peer_retrieve | 40 | peer_retrieve |
76 | @@ -57,7 +60,10 @@ | |||
77 | 57 | # the charm doesn't concern itself with template specifics etc. | 60 | # the charm doesn't concern itself with template specifics etc. |
78 | 58 | CONFIG_FILES = OrderedDict([ | 61 | CONFIG_FILES = OrderedDict([ |
79 | 59 | (RABBITMQ_CONF, { | 62 | (RABBITMQ_CONF, { |
81 | 60 | 'hook_contexts': None, | 63 | 'hook_contexts': [ |
82 | 64 | RabbitMQSSLContext(), | ||
83 | 65 | RabbitMQClusterContext(), | ||
84 | 66 | ], | ||
85 | 61 | 'services': ['rabbitmq-server'] | 67 | 'services': ['rabbitmq-server'] |
86 | 62 | }), | 68 | }), |
87 | 63 | (ENV_CONF, { | 69 | (ENV_CONF, { |
88 | @@ -71,6 +77,32 @@ | |||
89 | 71 | ]) | 77 | ]) |
90 | 72 | 78 | ||
91 | 73 | 79 | ||
92 | 80 | class ConfigRenderer(): | ||
93 | 81 | |||
94 | 82 | def __init__(self, config): | ||
95 | 83 | self.config_data = {} | ||
96 | 84 | |||
97 | 85 | for config_path, data in config.items(): | ||
98 | 86 | if 'hook_contexts' in data and data['hook_contexts']: | ||
99 | 87 | ctxt = {} | ||
100 | 88 | for svc_context in data['hook_contexts']: | ||
101 | 89 | ctxt.update(svc_context()) | ||
102 | 90 | self.config_data[config_path] = ctxt | ||
103 | 91 | |||
104 | 92 | def write(self, config_path): | ||
105 | 93 | data = self.config_data.get(config_path, None) | ||
106 | 94 | if data: | ||
107 | 95 | log("writing config file: %s , data: %s" % (config_path, | ||
108 | 96 | str(data)), level='DEBUG') | ||
109 | 97 | |||
110 | 98 | render(os.path.basename(config_path), config_path, | ||
111 | 99 | data, perms=0o644) | ||
112 | 100 | |||
113 | 101 | def write_all(self): | ||
114 | 102 | for service in self.config_data.keys(): | ||
115 | 103 | self.write(service) | ||
116 | 104 | |||
117 | 105 | |||
118 | 74 | class RabbitmqError(Exception): | 106 | class RabbitmqError(Exception): |
119 | 75 | pass | 107 | pass |
120 | 76 | 108 | ||
121 | @@ -390,40 +422,6 @@ | |||
122 | 390 | def disable_plugin(plugin): | 422 | def disable_plugin(plugin): |
123 | 391 | _manage_plugin(plugin, 'disable') | 423 | _manage_plugin(plugin, 'disable') |
124 | 392 | 424 | ||
125 | 393 | ssl_key_file = "/etc/rabbitmq/rabbit-server-privkey.pem" | ||
126 | 394 | ssl_cert_file = "/etc/rabbitmq/rabbit-server-cert.pem" | ||
127 | 395 | ssl_ca_file = "/etc/rabbitmq/rabbit-server-ca.pem" | ||
128 | 396 | |||
129 | 397 | |||
130 | 398 | def enable_ssl(ssl_key, ssl_cert, ssl_port, | ||
131 | 399 | ssl_ca=None, ssl_only=False, ssl_client=None): | ||
132 | 400 | uid = pwd.getpwnam("root").pw_uid | ||
133 | 401 | gid = grp.getgrnam("rabbitmq").gr_gid | ||
134 | 402 | |||
135 | 403 | for contents, path in ( | ||
136 | 404 | (ssl_key, ssl_key_file), | ||
137 | 405 | (ssl_cert, ssl_cert_file), | ||
138 | 406 | (ssl_ca, ssl_ca_file)): | ||
139 | 407 | if not contents: | ||
140 | 408 | continue | ||
141 | 409 | with open(path, 'w') as fh: | ||
142 | 410 | fh.write(contents) | ||
143 | 411 | os.chmod(path, 0o640) | ||
144 | 412 | os.chown(path, uid, gid) | ||
145 | 413 | |||
146 | 414 | data = { | ||
147 | 415 | "ssl_port": ssl_port, | ||
148 | 416 | "ssl_cert_file": ssl_cert_file, | ||
149 | 417 | "ssl_key_file": ssl_key_file, | ||
150 | 418 | "ssl_client": ssl_client, | ||
151 | 419 | "ssl_ca_file": "", | ||
152 | 420 | "ssl_only": ssl_only} | ||
153 | 421 | |||
154 | 422 | if ssl_ca: | ||
155 | 423 | data["ssl_ca_file"] = ssl_ca_file | ||
156 | 424 | |||
157 | 425 | render(os.path.basename(RABBITMQ_CONF), RABBITMQ_CONF, data, perms=0o644) | ||
158 | 426 | |||
159 | 427 | 425 | ||
160 | 428 | def execute(cmd, die=False, echo=False): | 426 | def execute(cmd, die=False, echo=False): |
161 | 429 | """ Executes a command | 427 | """ Executes a command |
162 | 430 | 428 | ||
163 | === added file 'hooks/rabbitmq_context.py' | |||
164 | --- hooks/rabbitmq_context.py 1970-01-01 00:00:00 +0000 | |||
165 | +++ hooks/rabbitmq_context.py 2015-04-13 17:16:27 +0000 | |||
166 | @@ -0,0 +1,122 @@ | |||
167 | 1 | from charmhelpers.contrib.ssl.service import ServiceCA | ||
168 | 2 | |||
169 | 3 | from charmhelpers.core.hookenv import ( | ||
170 | 4 | open_port, | ||
171 | 5 | close_port, | ||
172 | 6 | config, | ||
173 | 7 | log, | ||
174 | 8 | ERROR, | ||
175 | 9 | ) | ||
176 | 10 | |||
177 | 11 | import sys | ||
178 | 12 | import pwd | ||
179 | 13 | import grp | ||
180 | 14 | import os | ||
181 | 15 | import base64 | ||
182 | 16 | |||
183 | 17 | import ssl_utils | ||
184 | 18 | |||
185 | 19 | ssl_key_file = "/etc/rabbitmq/rabbit-server-privkey.pem" | ||
186 | 20 | ssl_cert_file = "/etc/rabbitmq/rabbit-server-cert.pem" | ||
187 | 21 | ssl_ca_file = "/etc/rabbitmq/rabbit-server-ca.pem" | ||
188 | 22 | |||
189 | 23 | |||
190 | 24 | def convert_from_base64(v): | ||
191 | 25 | # Rabbit originally supported pem encoded key/cert in config, play | ||
192 | 26 | # nice on upgrades as we now expect base64 encoded key/cert/ca. | ||
193 | 27 | if not v: | ||
194 | 28 | return v | ||
195 | 29 | if v.startswith('-----BEGIN'): | ||
196 | 30 | return v | ||
197 | 31 | try: | ||
198 | 32 | return base64.b64decode(v) | ||
199 | 33 | except TypeError: | ||
200 | 34 | return v | ||
201 | 35 | |||
202 | 36 | |||
203 | 37 | class RabbitMQSSLContext(object): | ||
204 | 38 | |||
205 | 39 | def enable_ssl(self, ssl_key, ssl_cert, ssl_port, | ||
206 | 40 | ssl_ca=None, ssl_only=False, ssl_client=None): | ||
207 | 41 | |||
208 | 42 | uid = pwd.getpwnam("root").pw_uid | ||
209 | 43 | gid = grp.getgrnam("rabbitmq").gr_gid | ||
210 | 44 | |||
211 | 45 | for contents, path in ( | ||
212 | 46 | (ssl_key, ssl_key_file), | ||
213 | 47 | (ssl_cert, ssl_cert_file), | ||
214 | 48 | (ssl_ca, ssl_ca_file)): | ||
215 | 49 | |||
216 | 50 | if not contents: | ||
217 | 51 | continue | ||
218 | 52 | |||
219 | 53 | with open(path, 'w') as fh: | ||
220 | 54 | fh.write(contents) | ||
221 | 55 | |||
222 | 56 | os.chmod(path, 0o640) | ||
223 | 57 | os.chown(path, uid, gid) | ||
224 | 58 | |||
225 | 59 | data = { | ||
226 | 60 | "ssl_port": ssl_port, | ||
227 | 61 | "ssl_cert_file": ssl_cert_file, | ||
228 | 62 | "ssl_key_file": ssl_key_file, | ||
229 | 63 | "ssl_client": ssl_client, | ||
230 | 64 | "ssl_ca_file": "", | ||
231 | 65 | "ssl_only": ssl_only | ||
232 | 66 | } | ||
233 | 67 | |||
234 | 68 | if ssl_ca: | ||
235 | 69 | data["ssl_ca_file"] = ssl_ca_file | ||
236 | 70 | |||
237 | 71 | return data | ||
238 | 72 | |||
239 | 73 | def __call__(self): | ||
240 | 74 | """ | ||
241 | 75 | The legacy config support adds some additional complications. | ||
242 | 76 | |||
243 | 77 | ssl_enabled = True, ssl = off -> ssl enabled | ||
244 | 78 | ssl_enabled = False, ssl = on -> ssl enabled | ||
245 | 79 | """ | ||
246 | 80 | ssl_mode, external_ca = ssl_utils.get_ssl_mode() | ||
247 | 81 | |||
248 | 82 | ctxt = { | ||
249 | 83 | 'ssl_mode': ssl_mode, | ||
250 | 84 | } | ||
251 | 85 | |||
252 | 86 | if ssl_mode == 'off': | ||
253 | 87 | close_port(config('ssl_port')) | ||
254 | 88 | ssl_utils.reconfigure_client_ssl() | ||
255 | 89 | return ctxt | ||
256 | 90 | |||
257 | 91 | ssl_key = convert_from_base64(config('ssl_key')) | ||
258 | 92 | ssl_cert = convert_from_base64(config('ssl_cert')) | ||
259 | 93 | ssl_ca = convert_from_base64(config('ssl_ca')) | ||
260 | 94 | ssl_port = config('ssl_port') | ||
261 | 95 | |||
262 | 96 | # If external managed certs then we need all the fields. | ||
263 | 97 | if (ssl_mode in ('on', 'only') and any((ssl_key, ssl_cert)) and | ||
264 | 98 | not all((ssl_key, ssl_cert))): | ||
265 | 99 | log('If ssl_key or ssl_cert are specified both are required.', | ||
266 | 100 | level=ERROR) | ||
267 | 101 | sys.exit(1) | ||
268 | 102 | |||
269 | 103 | if not external_ca: | ||
270 | 104 | ssl_cert, ssl_key, ssl_ca = ServiceCA.get_service_cert() | ||
271 | 105 | |||
272 | 106 | ctxt.update(self.enable_ssl( | ||
273 | 107 | ssl_key, ssl_cert, ssl_port, ssl_ca, | ||
274 | 108 | ssl_only=(ssl_mode == "only"), ssl_client=False | ||
275 | 109 | )) | ||
276 | 110 | |||
277 | 111 | ssl_utils.reconfigure_client_ssl(True) | ||
278 | 112 | open_port(ssl_port) | ||
279 | 113 | |||
280 | 114 | return ctxt | ||
281 | 115 | |||
282 | 116 | |||
283 | 117 | class RabbitMQClusterContext(object): | ||
284 | 118 | |||
285 | 119 | def __call__(self): | ||
286 | 120 | return { | ||
287 | 121 | 'cluster_partition_handling': config('cluster-partition-handling'), | ||
288 | 122 | } | ||
289 | 0 | 123 | ||
290 | === modified file 'hooks/rabbitmq_server_relations.py' | |||
291 | --- hooks/rabbitmq_server_relations.py 2015-04-01 09:33:46 +0000 | |||
292 | +++ hooks/rabbitmq_server_relations.py 2015-04-13 17:16:27 +0000 | |||
293 | @@ -1,5 +1,5 @@ | |||
294 | 1 | #!/usr/bin/python | 1 | #!/usr/bin/python |
296 | 2 | import base64 | 2 | |
297 | 3 | import os | 3 | import os |
298 | 4 | import shutil | 4 | import shutil |
299 | 5 | import sys | 5 | import sys |
300 | @@ -8,6 +8,8 @@ | |||
301 | 8 | import socket | 8 | import socket |
302 | 9 | 9 | ||
303 | 10 | import rabbit_utils as rabbit | 10 | import rabbit_utils as rabbit |
304 | 11 | import ssl_utils | ||
305 | 12 | |||
306 | 11 | from lib.utils import ( | 13 | from lib.utils import ( |
307 | 12 | chown, chmod, | 14 | chown, chmod, |
308 | 13 | is_newer, | 15 | is_newer, |
309 | @@ -60,7 +62,6 @@ | |||
310 | 60 | service_restart, | 62 | service_restart, |
311 | 61 | ) | 63 | ) |
312 | 62 | from charmhelpers.contrib.charmsupport import nrpe | 64 | from charmhelpers.contrib.charmsupport import nrpe |
313 | 63 | from charmhelpers.contrib.ssl.service import ServiceCA | ||
314 | 64 | 65 | ||
315 | 65 | from charmhelpers.contrib.peerstorage import ( | 66 | from charmhelpers.contrib.peerstorage import ( |
316 | 66 | peer_echo, | 67 | peer_echo, |
317 | @@ -175,7 +176,7 @@ | |||
318 | 175 | get_address_in_network(config('access-network'), | 176 | get_address_in_network(config('access-network'), |
319 | 176 | unit_get('private-address')) | 177 | unit_get('private-address')) |
320 | 177 | 178 | ||
322 | 178 | configure_client_ssl(relation_settings) | 179 | ssl_utils.configure_client_ssl(relation_settings) |
323 | 179 | 180 | ||
324 | 180 | if is_clustered(): | 181 | if is_clustered(): |
325 | 181 | relation_settings['clustered'] = 'true' | 182 | relation_settings['clustered'] = 'true' |
326 | @@ -512,103 +513,10 @@ | |||
327 | 512 | MAN_PLUGIN = 'rabbitmq_management' | 513 | MAN_PLUGIN = 'rabbitmq_management' |
328 | 513 | 514 | ||
329 | 514 | 515 | ||
330 | 515 | def configure_client_ssl(relation_data): | ||
331 | 516 | """Configure client with ssl | ||
332 | 517 | """ | ||
333 | 518 | ssl_mode, external_ca = _get_ssl_mode() | ||
334 | 519 | if ssl_mode == 'off': | ||
335 | 520 | return | ||
336 | 521 | relation_data['ssl_port'] = config('ssl_port') | ||
337 | 522 | if external_ca: | ||
338 | 523 | if config('ssl_ca'): | ||
339 | 524 | relation_data['ssl_ca'] = base64.b64encode( | ||
340 | 525 | config('ssl_ca')) | ||
341 | 526 | return | ||
342 | 527 | ca = ServiceCA.get_ca() | ||
343 | 528 | relation_data['ssl_ca'] = base64.b64encode(ca.get_ca_bundle()) | ||
344 | 529 | |||
345 | 530 | |||
346 | 531 | def _get_ssl_mode(): | ||
347 | 532 | ssl_mode = config('ssl') | ||
348 | 533 | external_ca = False | ||
349 | 534 | # Legacy config boolean option | ||
350 | 535 | ssl_on = config('ssl_enabled') | ||
351 | 536 | if ssl_mode == 'off' and ssl_on is False: | ||
352 | 537 | ssl_mode = 'off' | ||
353 | 538 | elif ssl_mode == 'off' and ssl_on: | ||
354 | 539 | ssl_mode = 'on' | ||
355 | 540 | ssl_key = config('ssl_key') | ||
356 | 541 | ssl_cert = config('ssl_cert') | ||
357 | 542 | if all((ssl_key, ssl_cert)): | ||
358 | 543 | external_ca = True | ||
359 | 544 | return ssl_mode, external_ca | ||
360 | 545 | |||
361 | 546 | |||
362 | 547 | def _convert_from_base64(v): | ||
363 | 548 | # Rabbit originally supported pem encoded key/cert in config, play | ||
364 | 549 | # nice on upgrades as we now expect base64 encoded key/cert/ca. | ||
365 | 550 | if not v: | ||
366 | 551 | return v | ||
367 | 552 | if v.startswith('-----BEGIN'): | ||
368 | 553 | return v | ||
369 | 554 | try: | ||
370 | 555 | return base64.b64decode(v) | ||
371 | 556 | except TypeError: | ||
372 | 557 | return v | ||
373 | 558 | |||
374 | 559 | |||
375 | 560 | def reconfigure_client_ssl(ssl_enabled=False): | ||
376 | 561 | ssl_config_keys = set(('ssl_key', 'ssl_cert', 'ssl_ca')) | ||
377 | 562 | for rid in relation_ids('amqp'): | ||
378 | 563 | rdata = relation_get(rid=rid, unit=os.environ['JUJU_UNIT_NAME']) | ||
379 | 564 | if not ssl_enabled and ssl_config_keys.intersection(rdata): | ||
380 | 565 | # No clean way to remove entirely, but blank them. | ||
381 | 566 | relation_set(relation_id=rid, ssl_key='', ssl_cert='', ssl_ca='') | ||
382 | 567 | elif ssl_enabled and not ssl_config_keys.intersection(rdata): | ||
383 | 568 | configure_client_ssl(rdata) | ||
384 | 569 | relation_set(relation_id=rid, **rdata) | ||
385 | 570 | |||
386 | 571 | |||
387 | 572 | def configure_rabbit_ssl(): | ||
388 | 573 | """ | ||
389 | 574 | The legacy config support adds some additional complications. | ||
390 | 575 | |||
391 | 576 | ssl_enabled = True, ssl = off -> ssl enabled | ||
392 | 577 | ssl_enabled = False, ssl = on -> ssl enabled | ||
393 | 578 | """ | ||
394 | 579 | ssl_mode, external_ca = _get_ssl_mode() | ||
395 | 580 | |||
396 | 581 | if ssl_mode == 'off': | ||
397 | 582 | if os.path.exists(rabbit.RABBITMQ_CONF): | ||
398 | 583 | os.remove(rabbit.RABBITMQ_CONF) | ||
399 | 584 | close_port(config('ssl_port')) | ||
400 | 585 | reconfigure_client_ssl() | ||
401 | 586 | return | ||
402 | 587 | ssl_key = _convert_from_base64(config('ssl_key')) | ||
403 | 588 | ssl_cert = _convert_from_base64(config('ssl_cert')) | ||
404 | 589 | ssl_ca = _convert_from_base64(config('ssl_ca')) | ||
405 | 590 | ssl_port = config('ssl_port') | ||
406 | 591 | |||
407 | 592 | # If external managed certs then we need all the fields. | ||
408 | 593 | if (ssl_mode in ('on', 'only') and any((ssl_key, ssl_cert)) and | ||
409 | 594 | not all((ssl_key, ssl_cert))): | ||
410 | 595 | log('If ssl_key or ssl_cert are specified both are required.', | ||
411 | 596 | level=ERROR) | ||
412 | 597 | sys.exit(1) | ||
413 | 598 | |||
414 | 599 | if not external_ca: | ||
415 | 600 | ssl_cert, ssl_key, ssl_ca = ServiceCA.get_service_cert() | ||
416 | 601 | |||
417 | 602 | rabbit.enable_ssl( | ||
418 | 603 | ssl_key, ssl_cert, ssl_port, ssl_ca, | ||
419 | 604 | ssl_only=(ssl_mode == "only"), ssl_client=False) | ||
420 | 605 | reconfigure_client_ssl(True) | ||
421 | 606 | open_port(ssl_port) | ||
422 | 607 | |||
423 | 608 | |||
424 | 609 | @hooks.hook('config-changed') | 516 | @hooks.hook('config-changed') |
425 | 610 | @restart_on_change(rabbit.restart_map()) | 517 | @restart_on_change(rabbit.restart_map()) |
426 | 611 | def config_changed(): | 518 | def config_changed(): |
427 | 519 | |||
428 | 612 | if config('prefer-ipv6'): | 520 | if config('prefer-ipv6'): |
429 | 613 | rabbit.assert_charm_supports_ipv6() | 521 | rabbit.assert_charm_supports_ipv6() |
430 | 614 | 522 | ||
431 | @@ -638,9 +546,9 @@ | |||
432 | 638 | rabbit.disable_plugin(MAN_PLUGIN) | 546 | rabbit.disable_plugin(MAN_PLUGIN) |
433 | 639 | close_port(55672) | 547 | close_port(55672) |
434 | 640 | 548 | ||
435 | 641 | configure_rabbit_ssl() | ||
436 | 642 | |||
437 | 643 | rabbit.set_all_mirroring_queues(config('mirroring-queues')) | 549 | rabbit.set_all_mirroring_queues(config('mirroring-queues')) |
438 | 550 | rabbit.ConfigRenderer( | ||
439 | 551 | rabbit.CONFIG_FILES).write_all() | ||
440 | 644 | 552 | ||
441 | 645 | if is_relation_made("ha"): | 553 | if is_relation_made("ha"): |
442 | 646 | ha_is_active_active = config("ha-vip-only") | 554 | ha_is_active_active = config("ha-vip-only") |
443 | 647 | 555 | ||
444 | === added file 'hooks/ssl_utils.py' | |||
445 | --- hooks/ssl_utils.py 1970-01-01 00:00:00 +0000 | |||
446 | +++ hooks/ssl_utils.py 2015-04-13 17:16:27 +0000 | |||
447 | @@ -0,0 +1,66 @@ | |||
448 | 1 | from charmhelpers.contrib.ssl.service import ServiceCA | ||
449 | 2 | |||
450 | 3 | from charmhelpers.core.hookenv import ( | ||
451 | 4 | config, | ||
452 | 5 | relation_ids, | ||
453 | 6 | relation_set, | ||
454 | 7 | relation_get, | ||
455 | 8 | log, | ||
456 | 9 | WARNING, | ||
457 | 10 | local_unit, | ||
458 | 11 | ) | ||
459 | 12 | |||
460 | 13 | import base64 | ||
461 | 14 | |||
462 | 15 | |||
463 | 16 | def get_ssl_mode(): | ||
464 | 17 | ssl_mode = config('ssl') | ||
465 | 18 | external_ca = False | ||
466 | 19 | # Legacy config boolean option | ||
467 | 20 | ssl_enabled = config('ssl_enabled') | ||
468 | 21 | |||
469 | 22 | if ssl_enabled: | ||
470 | 23 | log("Deprecated ssl_enabled config option is True -\ | ||
471 | 24 | this should be set to False and 'ssl' config set to 'on' instead.", | ||
472 | 25 | level=WARNING) | ||
473 | 26 | ssl_mode = 'on' | ||
474 | 27 | elif ssl_mode == 'on': | ||
475 | 28 | ssl_mode = 'on' | ||
476 | 29 | else: | ||
477 | 30 | ssl_mode = 'off' | ||
478 | 31 | |||
479 | 32 | ssl_key = config('ssl_key') | ||
480 | 33 | ssl_cert = config('ssl_cert') | ||
481 | 34 | |||
482 | 35 | if all((ssl_key, ssl_cert)): | ||
483 | 36 | external_ca = True | ||
484 | 37 | |||
485 | 38 | return ssl_mode, external_ca | ||
486 | 39 | |||
487 | 40 | |||
488 | 41 | def configure_client_ssl(relation_data): | ||
489 | 42 | """Configure client with ssl | ||
490 | 43 | """ | ||
491 | 44 | ssl_mode, external_ca = get_ssl_mode() | ||
492 | 45 | if ssl_mode == 'off': | ||
493 | 46 | return | ||
494 | 47 | relation_data['ssl_port'] = config('ssl_port') | ||
495 | 48 | if external_ca: | ||
496 | 49 | if config('ssl_ca'): | ||
497 | 50 | relation_data['ssl_ca'] = base64.b64encode( | ||
498 | 51 | config('ssl_ca')) | ||
499 | 52 | return | ||
500 | 53 | ca = ServiceCA.get_ca() | ||
501 | 54 | relation_data['ssl_ca'] = base64.b64encode(ca.get_ca_bundle()) | ||
502 | 55 | |||
503 | 56 | |||
504 | 57 | def reconfigure_client_ssl(ssl_enabled=False): | ||
505 | 58 | ssl_config_keys = set(('ssl_key', 'ssl_cert', 'ssl_ca')) | ||
506 | 59 | for rid in relation_ids('amqp'): | ||
507 | 60 | rdata = relation_get(rid=rid, unit=local_unit()) | ||
508 | 61 | if not ssl_enabled and ssl_config_keys.intersection(rdata): | ||
509 | 62 | # No clean way to remove entirely, but blank them. | ||
510 | 63 | relation_set(relation_id=rid, ssl_key='', ssl_cert='', ssl_ca='') | ||
511 | 64 | elif ssl_enabled and not ssl_config_keys.intersection(rdata): | ||
512 | 65 | configure_client_ssl(rdata) | ||
513 | 66 | relation_set(relation_id=rid, **rdata) | ||
514 | 0 | 67 | ||
515 | === modified file 'templates/rabbitmq.config' | |||
516 | --- templates/rabbitmq.config 2014-05-23 08:13:05 +0000 | |||
517 | +++ templates/rabbitmq.config 2015-04-13 17:16:27 +0000 | |||
518 | @@ -1,21 +1,35 @@ | |||
519 | 1 | [ | 1 | [ |
522 | 2 | {rabbit, [ | 2 | {rabbit, [ |
523 | 3 | {% if ssl_only %} | 3 | {% if ssl_only %} |
524 | 4 | {tcp_listeners, []}, | 4 | {tcp_listeners, []}, |
525 | 5 | {% else %} | 5 | {% else %} |
526 | 6 | {tcp_listeners, [5672]}, | 6 | {tcp_listeners, [5672]}, |
527 | 7 | {% endif %} | 7 | {% endif %} |
529 | 8 | {ssl_listeners, [{{ ssl_port }}]}, | 8 | {% if ssl_port %} |
530 | 9 | {ssl_listeners, [{{ ssl_port }}]}, | ||
531 | 10 | {% endif %} | ||
532 | 11 | {% if ssl_mode == "on" %} | ||
533 | 9 | {ssl_options, [ | 12 | {ssl_options, [ |
534 | 10 | {verify, verify_peer}, | 13 | {verify, verify_peer}, |
537 | 11 | {% if ssl_client %} | 14 | {% if ssl_client %} |
538 | 12 | {fail_if_no_peer_cert, true}, | 15 | {fail_if_no_peer_cert, true}, |
539 | 13 | {% else %} | 16 | {% else %} |
540 | 14 | {fail_if_no_peer_cert, false}, | 17 | {fail_if_no_peer_cert, false}, |
546 | 15 | {% endif %}{% if ssl_ca_file %} | 18 | {% endif %} |
547 | 16 | {cacertfile, "{{ ssl_ca_file }}"}, {% endif %} | 19 | {% if ssl_ca_file %} |
548 | 17 | {certfile, "{{ ssl_cert_file }}"}, | 20 | {cacertfile, "{{ ssl_ca_file }}"}, |
549 | 18 | {keyfile, "{{ ssl_key_file }}"} | 21 | {% endif %} |
550 | 19 | ]} | 22 | {% if ssl_cert_file %} |
551 | 23 | {certfile, "{{ ssl_cert_file }}"}, | ||
552 | 24 | {% endif %} | ||
553 | 25 | {% if ssl_key_file %} | ||
554 | 26 | {keyfile, "{{ ssl_key_file }}"} | ||
555 | 27 | {% endif %} | ||
556 | 28 | ]}, | ||
557 | 29 | {% endif %} | ||
558 | 30 | |||
559 | 31 | {% if cluster_partition_handling %} | ||
560 | 32 | {cluster_partition_handling, {{ cluster_partition_handling }}} | ||
561 | 33 | {% endif %} | ||
562 | 20 | ]} | 34 | ]} |
563 | 21 | ]. | ||
564 | 22 | \ No newline at end of file | 35 | \ No newline at end of file |
565 | 36 | ]. | ||
566 | 23 | 37 | ||
567 | === added file 'tests/50_test_cluster_partition.py' | |||
568 | --- tests/50_test_cluster_partition.py 1970-01-01 00:00:00 +0000 | |||
569 | +++ tests/50_test_cluster_partition.py 2015-04-13 17:16:27 +0000 | |||
570 | @@ -0,0 +1,31 @@ | |||
571 | 1 | #!/usr/bin/python | ||
572 | 2 | # | ||
573 | 3 | # This Amulet test deploys rabbitmq-server | ||
574 | 4 | # | ||
575 | 5 | # Note: We use python2, because pika doesn't support python3 | ||
576 | 6 | import amulet | ||
577 | 7 | |||
578 | 8 | # The number of seconds to wait for the environment to setup. | ||
579 | 9 | seconds = 1200 | ||
580 | 10 | d = amulet.Deployment(series="trusty") | ||
581 | 11 | |||
582 | 12 | d.add('rabbitmq-server', units=1) | ||
583 | 13 | # Create a configuration. | ||
584 | 14 | configuration = {'cluster-partition-handling': "autoheal"} | ||
585 | 15 | d.configure('rabbitmq-server', configuration) | ||
586 | 16 | |||
587 | 17 | d.expose('rabbitmq-server') | ||
588 | 18 | try: | ||
589 | 19 | d.setup(timeout=seconds) | ||
590 | 20 | d.sentry.wait(seconds) | ||
591 | 21 | except amulet.helpers.TimeoutError: | ||
592 | 22 | message = 'The environment did not setup in %d seconds.' % seconds | ||
593 | 23 | amulet.raise_status(amulet.SKIP, msg=message) | ||
594 | 24 | except: | ||
595 | 25 | raise | ||
596 | 26 | |||
597 | 27 | rabbit_unit = d.sentry.unit['rabbitmq-server/0'] | ||
598 | 28 | output, code = rabbit_unit.run("grep autoheal /etc/rabbitmq/rabbitmq.config") | ||
599 | 29 | |||
600 | 30 | if code != 0 or output == "": | ||
601 | 31 | amulet.raise_status(amulet.FAIL, msg="didn't find autoheal") | ||
602 | 0 | 32 | ||
603 | === added file 'unit_tests/test_rabbitmq_context.py' | |||
604 | --- unit_tests/test_rabbitmq_context.py 1970-01-01 00:00:00 +0000 | |||
605 | +++ unit_tests/test_rabbitmq_context.py 2015-04-13 17:16:27 +0000 | |||
606 | @@ -0,0 +1,79 @@ | |||
607 | 1 | import rabbitmq_context | ||
608 | 2 | |||
609 | 3 | import mock | ||
610 | 4 | import unittest | ||
611 | 5 | |||
612 | 6 | |||
613 | 7 | class TestRabbitMQSSLContext(unittest.TestCase): | ||
614 | 8 | |||
615 | 9 | @mock.patch("rabbitmq_context.config") | ||
616 | 10 | @mock.patch("rabbitmq_context.close_port") | ||
617 | 11 | @mock.patch("rabbitmq_context.ssl_utils.reconfigure_client_ssl") | ||
618 | 12 | @mock.patch("rabbitmq_context.ssl_utils.get_ssl_mode") | ||
619 | 13 | def test_context_ssl_off(self, get_ssl_mode, reconfig_ssl, close_port, | ||
620 | 14 | config): | ||
621 | 15 | get_ssl_mode.return_value = ("off", "off") | ||
622 | 16 | self.assertEqual(rabbitmq_context.RabbitMQSSLContext().__call__(), { | ||
623 | 17 | "ssl_mode": "off" | ||
624 | 18 | }) | ||
625 | 19 | |||
626 | 20 | close_port.assert_called_once() | ||
627 | 21 | reconfig_ssl.assert_called_once() | ||
628 | 22 | |||
629 | 23 | @mock.patch("rabbitmq_context.open_port") | ||
630 | 24 | @mock.patch("rabbitmq_context.os.chmod") | ||
631 | 25 | @mock.patch("rabbitmq_context.os.chown") | ||
632 | 26 | @mock.patch("rabbitmq_context.pwd.getpwnam") | ||
633 | 27 | @mock.patch("rabbitmq_context.grp.getgrnam") | ||
634 | 28 | @mock.patch("rabbitmq_context.config") | ||
635 | 29 | @mock.patch("rabbitmq_context.close_port") | ||
636 | 30 | @mock.patch("rabbitmq_context.ssl_utils.reconfigure_client_ssl") | ||
637 | 31 | @mock.patch("rabbitmq_context.ssl_utils.get_ssl_mode") | ||
638 | 32 | def test_context_ssl_on(self, get_ssl_mode, reconfig_ssl, close_port, | ||
639 | 33 | config, gr, pw, chown, chmod, open_port): | ||
640 | 34 | |||
641 | 35 | get_ssl_mode.return_value = ("on", "on") | ||
642 | 36 | |||
643 | 37 | def config_get(n): | ||
644 | 38 | return None | ||
645 | 39 | |||
646 | 40 | config.side_effect = config_get | ||
647 | 41 | |||
648 | 42 | def pw(name): | ||
649 | 43 | class Uid(object): | ||
650 | 44 | pw_uid = 1 | ||
651 | 45 | gr_gid = 100 | ||
652 | 46 | return Uid() | ||
653 | 47 | |||
654 | 48 | pw.side_effect = pw | ||
655 | 49 | gr.side_effect = pw | ||
656 | 50 | |||
657 | 51 | m = mock.mock_open() | ||
658 | 52 | with mock.patch('rabbitmq_context.open', m, create=True): | ||
659 | 53 | self.assertEqual( | ||
660 | 54 | rabbitmq_context.RabbitMQSSLContext().__call__(), { | ||
661 | 55 | "ssl_port": None, | ||
662 | 56 | "ssl_cert_file": "/etc/rabbitmq/rabbit-server-cert.pem", | ||
663 | 57 | "ssl_key_file": '/etc/rabbitmq/rabbit-server-privkey.pem', | ||
664 | 58 | "ssl_client": False, | ||
665 | 59 | "ssl_ca_file": "", | ||
666 | 60 | "ssl_only": False, | ||
667 | 61 | "ssl_mode": "on", | ||
668 | 62 | }) | ||
669 | 63 | |||
670 | 64 | reconfig_ssl.assert_called_once() | ||
671 | 65 | open_port.assert_called_once() | ||
672 | 66 | |||
673 | 67 | |||
674 | 68 | class TestRabbitMQClusterContext(unittest.TestCase): | ||
675 | 69 | |||
676 | 70 | @mock.patch("rabbitmq_context.config") | ||
677 | 71 | def test_context_ssl_off(self, config): | ||
678 | 72 | config.return_value = "ignore" | ||
679 | 73 | |||
680 | 74 | self.assertEqual( | ||
681 | 75 | rabbitmq_context.RabbitMQClusterContext().__call__(), { | ||
682 | 76 | 'cluster_partition_handling': "ignore" | ||
683 | 77 | }) | ||
684 | 78 | |||
685 | 79 | config.assert_called_once_with("cluster-partition-handling") | ||
686 | 0 | 80 | ||
687 | === modified file 'unit_tests/test_rabbitmq_server_relations.py' | |||
688 | --- unit_tests/test_rabbitmq_server_relations.py 2015-03-03 22:59:25 +0000 | |||
689 | +++ unit_tests/test_rabbitmq_server_relations.py 2015-04-13 17:16:27 +0000 | |||
690 | @@ -37,7 +37,7 @@ | |||
691 | 37 | @patch('rabbitmq_server_relations.relation_set') | 37 | @patch('rabbitmq_server_relations.relation_set') |
692 | 38 | @patch('apt_pkg.Cache') | 38 | @patch('apt_pkg.Cache') |
693 | 39 | @patch('rabbitmq_server_relations.is_clustered') | 39 | @patch('rabbitmq_server_relations.is_clustered') |
695 | 40 | @patch('rabbitmq_server_relations.configure_client_ssl') | 40 | @patch('rabbitmq_server_relations.ssl_utils.configure_client_ssl') |
696 | 41 | @patch('rabbitmq_server_relations.unit_get') | 41 | @patch('rabbitmq_server_relations.unit_get') |
697 | 42 | @patch('rabbitmq_server_relations.relation_get') | 42 | @patch('rabbitmq_server_relations.relation_get') |
698 | 43 | @patch('rabbitmq_server_relations.is_elected_leader') | 43 | @patch('rabbitmq_server_relations.is_elected_leader') |
699 | @@ -87,7 +87,7 @@ | |||
700 | 87 | @patch('rabbitmq_server_relations.relation_set') | 87 | @patch('rabbitmq_server_relations.relation_set') |
701 | 88 | @patch('apt_pkg.Cache') | 88 | @patch('apt_pkg.Cache') |
702 | 89 | @patch('rabbitmq_server_relations.is_clustered') | 89 | @patch('rabbitmq_server_relations.is_clustered') |
704 | 90 | @patch('rabbitmq_server_relations.configure_client_ssl') | 90 | @patch('rabbitmq_server_relations.ssl_utils.configure_client_ssl') |
705 | 91 | @patch('rabbitmq_server_relations.unit_get') | 91 | @patch('rabbitmq_server_relations.unit_get') |
706 | 92 | @patch('rabbitmq_server_relations.relation_get') | 92 | @patch('rabbitmq_server_relations.relation_get') |
707 | 93 | @patch('rabbitmq_server_relations.is_elected_leader') | 93 | @patch('rabbitmq_server_relations.is_elected_leader') |
Jorge,
I think some of the refactor in here is a definite improvement and makes things cleaner, thanks! I've got a few inline comments included, but the biggest thing is that the rabbitmq-server charm now has a /next and /trunk branch to match the rest of the flow of the openstack charms (since this has recently come under ownership of the ~openstack-charmers team). Can you retarget for /next?
Also, I think the README file should be updated with some discussions regarding high availability.
I'll run through some tests on this but wanted to drop some feedback now.