Merge ~peter-sabaini/charm-graylog:lp1758175 into charm-graylog:master
- Git
- lp:~peter-sabaini/charm-graylog
- lp1758175
- Merge into master
Status: | Superseded | ||||
---|---|---|---|---|---|
Proposed branch: | ~peter-sabaini/charm-graylog:lp1758175 | ||||
Merge into: | charm-graylog:master | ||||
Diff against target: |
847 lines (+441/-17) (has conflicts) 20 files modified
src/layer.yaml (+1/-0) src/lib/charms/layer/graylog/api.py (+11/-1) src/lib/charms/layer/graylog/constants.py (+6/-0) src/lib/charms/layer/graylog/utils.py (+9/-2) src/reactive/graylog.py (+70/-3) src/templates/default-graylog-server (+4/-0) src/templates/sync-graylog-snap (+7/-0) src/tests/functional/requirements.txt (+2/-0) src/tests/functional/tests/base.py (+48/-0) src/tests/functional/tests/bundles/bionic-graylog2.yaml (+6/-0) src/tests/functional/tests/bundles/bionic-graylog3-tls.yaml (+51/-0) src/tests/functional/tests/bundles/bionic-graylog3.yaml (+6/-0) src/tests/functional/tests/bundles/focal-graylog2.yaml (+6/-0) src/tests/functional/tests/bundles/focal-graylog3-tls.yaml (+51/-0) src/tests/functional/tests/bundles/focal-graylog3.yaml (+6/-0) src/tests/functional/tests/test_graylog_charm.py (+12/-5) src/tests/functional/tests/test_legacy.py (+29/-4) src/tests/functional/tests/tests.yaml (+20/-1) src/tests/unit/test_graylog.py (+83/-0) src/tests/unit/test_lib.py (+13/-1) Conflict in src/tests/functional/tests/tests.yaml |
||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Paul Goins | Pending | ||
Review via email: mp+396670@code.launchpad.net |
This proposal supersedes a proposal from 2020-08-26.
This proposal has been superseded by a proposal from 2021-01-21.
Commit message
Description of the change
Paul Goins (vultaire) wrote : Posted in a previous version of this proposal | # |
Paul Goins (vultaire) wrote : Posted in a previous version of this proposal | # |
Unit tests are failing; seems more mocking is needed. "make unittests" partial output showing the failure: https:/
(It's trying to do an apt-get install ca-certificates
Running functional tests now.
Paul Goins (vultaire) wrote : Posted in a previous version of this proposal | # |
Functional tests pass.
If you can run "make black" and correct the unit test failure, I think this is +1 from me.
Peter Sabaini (peter-sabaini) wrote : | # |
I've mocked out the java CA install and blackened, please take a look?
Unmerged commits
- 7fe02d3... by Peter Sabaini
-
Test and lint fix
Add mocking for the java CA install and blacken
- 189f578... by Felipe Reyes
-
Add tls-client layer to support HTTPS
This patch adds a new interface provided by the tls-client later. When related
to a certificates provider (e.g. EasyRSA) it will configure the daemon to
serve over HTTPS.Fixes-Bug: #1758175
Preview Diff
1 | diff --git a/src/layer.yaml b/src/layer.yaml |
2 | index 04e6d2a..5173d7e 100644 |
3 | --- a/src/layer.yaml |
4 | +++ b/src/layer.yaml |
5 | @@ -3,6 +3,7 @@ includes: |
6 | - 'layer:basic' |
7 | - 'layer:snap' |
8 | - 'layer:leadership' |
9 | + - 'layer:tls-client' |
10 | - 'interface:elasticsearch' |
11 | - 'interface:elastic-beats' |
12 | - 'interface:http' |
13 | diff --git a/src/lib/charms/layer/graylog/api.py b/src/lib/charms/layer/graylog/api.py |
14 | index ca9cba1..f2299dc 100644 |
15 | --- a/src/lib/charms/layer/graylog/api.py |
16 | +++ b/src/lib/charms/layer/graylog/api.py |
17 | @@ -4,6 +4,12 @@ import os |
18 | |
19 | import requests |
20 | |
21 | +# When using 'certifi' from the virtualenv, the system-wide certificates store |
22 | +# is not used, so installed certificates won't be used to validate hosts. |
23 | +# Adding the system CA bundle |
24 | +# https://git.launchpad.net/ubuntu/+source/python-certifi/tree/debian/patches/0001-Use-Debian-provided-etc-ssl-certs-ca-certificates.cr.patch |
25 | +SYSTEM_CA_BUNDLE = "/etc/ssl/certs/ca-certificates.crt" |
26 | + |
27 | # We are in a charm environment |
28 | charm = False |
29 | if os.environ.get("JUJU_UNIT_NAME"): |
30 | @@ -20,7 +26,9 @@ def get_ignore_indexer_failures_file(): # noqa: D103 |
31 | class GraylogApi: |
32 | """Manage Graylog via its API.""" |
33 | |
34 | - def __init__(self, base_url, username, password, token_name="graylog-api"): |
35 | + def __init__( |
36 | + self, base_url, username, password, token_name="graylog-api", verify=None |
37 | + ): |
38 | """Initialize HTTP values.""" |
39 | self.base_url = base_url |
40 | if not base_url.endswith("/"): |
41 | @@ -33,6 +41,7 @@ class GraylogApi: |
42 | self.input_types = None |
43 | self.req_timeout = 3 |
44 | self.req_retries = 4 |
45 | + self.verify = verify |
46 | |
47 | def request(self, path, method="GET", data={}, params=None): |
48 | """Retrieve data by URL.""" |
49 | @@ -59,6 +68,7 @@ class GraylogApi: |
50 | params=params, |
51 | headers=headers, |
52 | timeout=self.req_timeout, |
53 | + verify=SYSTEM_CA_BUNDLE if self.verify is None else self.verify, |
54 | ) |
55 | if resp.ok: |
56 | if method == "DELETE": |
57 | diff --git a/src/lib/charms/layer/graylog/constants.py b/src/lib/charms/layer/graylog/constants.py |
58 | index db38aa0..ce2bc59 100644 |
59 | --- a/src/lib/charms/layer/graylog/constants.py |
60 | +++ b/src/lib/charms/layer/graylog/constants.py |
61 | @@ -1,5 +1,9 @@ |
62 | """Options used by the Graylog charm.""" |
63 | +import os |
64 | + |
65 | + |
66 | SNAP_NAME = "graylog" |
67 | +SNAP_COMMON_DIR = "/var/snap/graylog/common/" |
68 | CONF_FILE = "/var/snap/graylog/common/server.conf" |
69 | SNAP_CONF_FILE = "/snap/graylog/current/etc/graylog/server/server.conf" |
70 | # /snap/graylog is a read-only squashfs so we use the initial settings |
71 | @@ -12,3 +16,5 @@ ELASTICSEARCH_DISCOVERY_PORT = "9300" |
72 | SERVICE_NAME = "snap.graylog.graylog" |
73 | DEFAULT_REST_API_TIMEOUT = 120 |
74 | NAGIOS_USERNAME = "nagios" |
75 | +CERT_PATH = os.path.join(SNAP_COMMON_DIR, "server.crt") |
76 | +CERT_KEY_PATH = os.path.join(SNAP_COMMON_DIR, "server.key") |
77 | diff --git a/src/lib/charms/layer/graylog/utils.py b/src/lib/charms/layer/graylog/utils.py |
78 | index 115e672..6b020c5 100644 |
79 | --- a/src/lib/charms/layer/graylog/utils.py |
80 | +++ b/src/lib/charms/layer/graylog/utils.py |
81 | @@ -4,10 +4,10 @@ from urllib.parse import urlparse |
82 | from charmhelpers.core import hookenv |
83 | |
84 | from charms.layer.graylog.constants import SNAP_NAME |
85 | +from charms.reactive import get_flags |
86 | |
87 | import netifaces |
88 | |
89 | - |
90 | # Allow mocking of other layers during unit tests |
91 | try: |
92 | from charms.layer import snap |
93 | @@ -20,7 +20,14 @@ def get_api_port(): # noqa: D103 |
94 | |
95 | |
96 | def get_api_url(): # noqa: D103 |
97 | - return "http://127.0.0.1:{}/api/".format(get_api_port()) |
98 | + return "{}://127.0.0.1:{}/api/".format(get_api_protocol(), get_api_port()) |
99 | + |
100 | + |
101 | +def get_api_protocol(): # noqa: D103 |
102 | + if "graylog.certificates.configured" in get_flags(): |
103 | + return "https" |
104 | + else: |
105 | + return "http" |
106 | |
107 | |
108 | def is_v2(): # noqa: D103 |
109 | diff --git a/src/reactive/graylog.py b/src/reactive/graylog.py |
110 | index 9cc3a93..88bef99 100644 |
111 | --- a/src/reactive/graylog.py |
112 | +++ b/src/reactive/graylog.py |
113 | @@ -2,23 +2,28 @@ |
114 | import hashlib |
115 | import os |
116 | import re |
117 | +import socket |
118 | import time |
119 | from urllib.parse import urlparse |
120 | |
121 | from charmhelpers.contrib.charmsupport import nrpe |
122 | -from charmhelpers.core import hookenv, host, unitdata |
123 | +from charmhelpers.core import hookenv, host, templating, unitdata |
124 | +from charmhelpers.fetch import filter_installed_packages, install |
125 | |
126 | import charms.leadership |
127 | -from charms.layer import snap |
128 | +from charms.layer import snap, tls_client |
129 | from charms.layer.graylog import ( |
130 | GraylogApi, |
131 | LogExtractPipeline, |
132 | get_api_port, |
133 | + get_api_protocol, |
134 | get_api_url, |
135 | is_v2, |
136 | validate_api_uri, |
137 | ) |
138 | from charms.layer.graylog.constants import ( |
139 | + CERT_KEY_PATH, |
140 | + CERT_PATH, |
141 | CONF_FILE, |
142 | DEFAULT_REST_API_TIMEOUT, |
143 | ELASTICSEARCH_DISCOVERY_PORT, |
144 | @@ -200,7 +205,8 @@ def configure_graylog(): # noqa: C901 |
145 | # intead of guessing the best IP. |
146 | set_conf("http_bind_address", "0.0.0.0:9000") |
147 | set_conf( |
148 | - "http_publish_uri", validate_api_uri("http://0.0.0.0:{}/".format(api_port)) |
149 | + "http_publish_uri", |
150 | + validate_api_uri("{}://0.0.0.0:{}/".format(get_api_protocol(), api_port)), |
151 | ) |
152 | |
153 | if conf["web_endpoint_uri"]: |
154 | @@ -1045,6 +1051,49 @@ def configure_nagios(nagios): |
155 | set_state("graylog_nagios.configured") |
156 | |
157 | |
158 | +@when("certificates.available") |
159 | +@when_not("graylog.certificates.configured") |
160 | +def tls_request_certificate(tls): |
161 | + """Create a server certificate for this server.""" |
162 | + # ca-certificates-java generates the ca-certificates in a jvm compatible |
163 | + # keystore |
164 | + if filter_installed_packages(["ca-certificates-java"]): |
165 | + install(["ca-certificates-java"], fatal=True) |
166 | + |
167 | + _maybe_install_ca_certificates_hook() |
168 | + _maybe_configure_graylog_jvm_keystore() |
169 | + |
170 | + # Use the public ip of this unit as the Common Name for the certificate. |
171 | + common_name = hookenv.unit_public_ip() |
172 | + # Get a list of Subject Alt Names for the certificate. |
173 | + sans = [] |
174 | + sans.append(hookenv.unit_public_ip()) |
175 | + sans.append(hookenv.unit_private_ip()) |
176 | + sans.append(socket.gethostname()) |
177 | + sans.append("127.0.0.1") |
178 | + sans.append("localhost") |
179 | + tls_client.request_server_cert( |
180 | + common_name, sans, crt_path=CERT_PATH, key_path=CERT_KEY_PATH |
181 | + ) |
182 | + |
183 | + set_conf("http_enable_tls", "true") |
184 | + set_conf("http_tls_cert_file", CERT_PATH) |
185 | + set_conf("http_tls_key_file", CERT_KEY_PATH) |
186 | + |
187 | + set_conf("rest_enable_tls", "true") |
188 | + set_conf("rest_tls_cert_file", CERT_PATH) |
189 | + set_conf("rest_tls_key_file", CERT_KEY_PATH) |
190 | + |
191 | + set_conf("web_enable_tls", "true") |
192 | + set_conf("web_tls_cert_file", CERT_PATH) |
193 | + set_conf("web_tls_key_file", CERT_KEY_PATH) |
194 | + |
195 | + set_conf("http_publish_uri", "https://0.0.0.0:{}/".format(get_api_port())) |
196 | + |
197 | + set_state("graylog.certificates.configured") |
198 | + set_state("graylog.needs_restart") |
199 | + |
200 | + |
201 | def get_default_graylog_client(): # noqa: D103 |
202 | db = unitdata.kv() |
203 | return GraylogApi( |
204 | @@ -1068,3 +1117,21 @@ def _verify_rest_api_is_alive(timeout=DEFAULT_REST_API_TIMEOUT): |
205 | if time.time() - start_ts > timeout: |
206 | raise ApiTimeout() |
207 | hookenv.log("REST API is up") |
208 | + |
209 | + |
210 | +def _maybe_install_ca_certificates_hook(): |
211 | + templating.render( |
212 | + "sync-graylog-snap", |
213 | + "/etc/ca-certificates/update.d/sync-graylog-snap", |
214 | + context={}, |
215 | + perms=0o755, |
216 | + ) |
217 | + |
218 | + |
219 | +def _maybe_configure_graylog_jvm_keystore(): |
220 | + templating.render( |
221 | + "default-graylog-server", |
222 | + "/var/snap/graylog/current/default-graylog-server", |
223 | + context={}, |
224 | + perms=0o644, |
225 | + ) |
226 | diff --git a/src/templates/default-graylog-server b/src/templates/default-graylog-server |
227 | new file mode 100644 |
228 | index 0000000..ace24cb |
229 | --- /dev/null |
230 | +++ b/src/templates/default-graylog-server |
231 | @@ -0,0 +1,4 @@ |
232 | +# |
233 | +# This file is managed by juju. |
234 | +# |
235 | +GRAYLOG_SERVER_JAVA_OPTS="-Djavax.net.ssl.trustStore=/var/snap/graylog/common/cacerts" |
236 | diff --git a/src/templates/sync-graylog-snap b/src/templates/sync-graylog-snap |
237 | new file mode 100755 |
238 | index 0000000..6d08b7d |
239 | --- /dev/null |
240 | +++ b/src/templates/sync-graylog-snap |
241 | @@ -0,0 +1,7 @@ |
242 | +#!/bin/sh |
243 | +# |
244 | +# This file is managed by juju. |
245 | +# |
246 | + |
247 | +set -e |
248 | +rsync /etc/ssl/certs/java/cacerts /var/snap/graylog/common/cacerts |
249 | diff --git a/src/tests/functional/requirements.txt b/src/tests/functional/requirements.txt |
250 | index 7345526..ecf87be 100644 |
251 | --- a/src/tests/functional/requirements.txt |
252 | +++ b/src/tests/functional/requirements.txt |
253 | @@ -1,2 +1,4 @@ |
254 | tenacity |
255 | git+https://github.com/openstack-charmers/zaza.git#egg=zaza |
256 | +netifaces |
257 | +charmhelpers |
258 | diff --git a/src/tests/functional/tests/base.py b/src/tests/functional/tests/base.py |
259 | new file mode 100644 |
260 | index 0000000..46eb75f |
261 | --- /dev/null |
262 | +++ b/src/tests/functional/tests/base.py |
263 | @@ -0,0 +1,48 @@ |
264 | +"""Base class for Testing.""" |
265 | +import logging |
266 | +import os |
267 | +import tempfile |
268 | +import unittest |
269 | + |
270 | +from zaza import model |
271 | + |
272 | + |
273 | +class BaseTestCase(unittest.TestCase): |
274 | + """Base class for graylog functional testing.""" |
275 | + |
276 | + @classmethod |
277 | + def setUpClass(cls): |
278 | + """Configure the test's environment. |
279 | + |
280 | + - Get the CA from easyrsa (when available) and write it on disk. |
281 | + """ |
282 | + if cls.get_api_protocol() == "https": |
283 | + rel_id = model.get_relation_id( |
284 | + "graylog", "easyrsa", remote_interface_name="client" |
285 | + ) |
286 | + easyrsa_unit = model.get_units("easyrsa")[0] |
287 | + cmd = "relation-get -r client:{} ca {}".format( |
288 | + rel_id, easyrsa_unit.entity_id |
289 | + ) |
290 | + logging.info(cmd) |
291 | + result = model.run_on_unit(easyrsa_unit.entity_id, cmd) |
292 | + |
293 | + with tempfile.NamedTemporaryFile(mode="w", delete=False) as f: |
294 | + f.write(result["Stdout"]) |
295 | + f.flush() |
296 | + cls.ca_path = f.name |
297 | + else: |
298 | + cls.ca_path = None |
299 | + |
300 | + @classmethod |
301 | + def tearDownClass(cls): |
302 | + """Remove the CA file that was written to disk during the setUp.""" |
303 | + if cls.ca_path: |
304 | + os.remove(cls.ca_path) |
305 | + |
306 | + @classmethod |
307 | + def get_api_protocol(cls): # noqa: D102 |
308 | + if model.get_relation_id("graylog", "easyrsa"): |
309 | + return "https" |
310 | + else: |
311 | + return "http" |
312 | diff --git a/src/tests/functional/tests/bundles/bionic-graylog2.yaml b/src/tests/functional/tests/bundles/bionic-graylog2.yaml |
313 | index bd256db..27703fd 100644 |
314 | --- a/src/tests/functional/tests/bundles/bionic-graylog2.yaml |
315 | +++ b/src/tests/functional/tests/bundles/bionic-graylog2.yaml |
316 | @@ -30,6 +30,10 @@ applications: |
317 | nrpe: |
318 | charm: cs:nrpe |
319 | |
320 | + nagios: |
321 | + charm: cs:nagios |
322 | + num_units: 1 |
323 | + |
324 | relations: |
325 | - - ubuntu |
326 | - filebeat |
327 | @@ -43,3 +47,5 @@ relations: |
328 | - haproxy |
329 | - - graylog |
330 | - nrpe |
331 | + - - nagios |
332 | + - nrpe |
333 | diff --git a/src/tests/functional/tests/bundles/bionic-graylog3-tls.yaml b/src/tests/functional/tests/bundles/bionic-graylog3-tls.yaml |
334 | new file mode 100644 |
335 | index 0000000..954f9c5 |
336 | --- /dev/null |
337 | +++ b/src/tests/functional/tests/bundles/bionic-graylog3-tls.yaml |
338 | @@ -0,0 +1,51 @@ |
339 | +series: bionic |
340 | + |
341 | +applications: |
342 | + ubuntu: |
343 | + charm: cs:ubuntu |
344 | + num_units: 1 |
345 | + |
346 | + filebeat: |
347 | + charm: cs:filebeat |
348 | + num_units: 0 |
349 | + |
350 | + graylog: |
351 | + num_units: 1 |
352 | + series: bionic |
353 | + options: |
354 | + channel: 3/stable |
355 | + |
356 | + elastic: |
357 | + charm: cs:elasticsearch |
358 | + num_units: 1 |
359 | + |
360 | + mongo: |
361 | + charm: cs:mongodb |
362 | + num_units: 1 |
363 | + |
364 | + nrpe: |
365 | + charm: cs:nrpe |
366 | + |
367 | + nagios: |
368 | + charm: cs:nagios |
369 | + num_units: 1 |
370 | + |
371 | + easyrsa: |
372 | + charm: cs:~containers/easyrsa |
373 | + num_units: 1 |
374 | + |
375 | +relations: |
376 | + - - ubuntu |
377 | + - filebeat |
378 | + - - graylog:beats |
379 | + - filebeat:logstash |
380 | + - - graylog |
381 | + - mongo |
382 | + - - graylog |
383 | + - elastic |
384 | + - - graylog |
385 | + - nrpe |
386 | + - - nagios |
387 | + - nrpe |
388 | + - - easyrsa:client |
389 | + - graylog:certificates |
390 | diff --git a/src/tests/functional/tests/bundles/bionic-graylog3.yaml b/src/tests/functional/tests/bundles/bionic-graylog3.yaml |
391 | index 596cf7d..33ac7e5 100644 |
392 | --- a/src/tests/functional/tests/bundles/bionic-graylog3.yaml |
393 | +++ b/src/tests/functional/tests/bundles/bionic-graylog3.yaml |
394 | @@ -30,6 +30,10 @@ applications: |
395 | nrpe: |
396 | charm: cs:nrpe |
397 | |
398 | + nagios: |
399 | + charm: cs:nagios |
400 | + num_units: 1 |
401 | + |
402 | relations: |
403 | - - ubuntu |
404 | - filebeat |
405 | @@ -43,3 +47,5 @@ relations: |
406 | - haproxy |
407 | - - graylog |
408 | - nrpe |
409 | + - - nagios |
410 | + - nrpe |
411 | diff --git a/src/tests/functional/tests/bundles/focal-graylog2.yaml b/src/tests/functional/tests/bundles/focal-graylog2.yaml |
412 | index 368432e..9cc6016 100644 |
413 | --- a/src/tests/functional/tests/bundles/focal-graylog2.yaml |
414 | +++ b/src/tests/functional/tests/bundles/focal-graylog2.yaml |
415 | @@ -30,6 +30,10 @@ applications: |
416 | nrpe: |
417 | charm: cs:nrpe |
418 | |
419 | + nagios: |
420 | + charm: cs:nagios |
421 | + num_units: 1 |
422 | + |
423 | relations: |
424 | - - ubuntu |
425 | - filebeat |
426 | @@ -43,3 +47,5 @@ relations: |
427 | - haproxy |
428 | - - graylog |
429 | - nrpe |
430 | + - - nagios |
431 | + - nrpe |
432 | diff --git a/src/tests/functional/tests/bundles/focal-graylog3-tls.yaml b/src/tests/functional/tests/bundles/focal-graylog3-tls.yaml |
433 | new file mode 100644 |
434 | index 0000000..100ccb4 |
435 | --- /dev/null |
436 | +++ b/src/tests/functional/tests/bundles/focal-graylog3-tls.yaml |
437 | @@ -0,0 +1,51 @@ |
438 | +series: bionic |
439 | + |
440 | +applications: |
441 | + ubuntu: |
442 | + charm: cs:ubuntu |
443 | + num_units: 1 |
444 | + |
445 | + filebeat: |
446 | + charm: cs:filebeat |
447 | + num_units: 0 |
448 | + |
449 | + graylog: |
450 | + num_units: 1 |
451 | + series: focal |
452 | + options: |
453 | + channel: 3/stable |
454 | + |
455 | + elastic: |
456 | + charm: cs:elasticsearch |
457 | + num_units: 1 |
458 | + |
459 | + mongo: |
460 | + charm: cs:mongodb |
461 | + num_units: 1 |
462 | + |
463 | + nrpe: |
464 | + charm: cs:nrpe |
465 | + |
466 | + nagios: |
467 | + charm: cs:nagios |
468 | + num_units: 1 |
469 | + |
470 | + easyrsa: |
471 | + charm: cs:~containers/easyrsa |
472 | + num_units: 1 |
473 | + |
474 | +relations: |
475 | + - - ubuntu |
476 | + - filebeat |
477 | + - - graylog:beats |
478 | + - filebeat:logstash |
479 | + - - graylog |
480 | + - mongo |
481 | + - - graylog |
482 | + - elastic |
483 | + - - graylog |
484 | + - nrpe |
485 | + - - nagios |
486 | + - nrpe |
487 | + - - easyrsa:client |
488 | + - graylog:certificates |
489 | diff --git a/src/tests/functional/tests/bundles/focal-graylog3.yaml b/src/tests/functional/tests/bundles/focal-graylog3.yaml |
490 | index cb26f52..e2e2a41 100644 |
491 | --- a/src/tests/functional/tests/bundles/focal-graylog3.yaml |
492 | +++ b/src/tests/functional/tests/bundles/focal-graylog3.yaml |
493 | @@ -30,6 +30,10 @@ applications: |
494 | nrpe: |
495 | charm: cs:nrpe |
496 | |
497 | + nagios: |
498 | + charm: cs:nagios |
499 | + num_units: 1 |
500 | + |
501 | relations: |
502 | - - ubuntu |
503 | - filebeat |
504 | @@ -43,3 +47,5 @@ relations: |
505 | - haproxy |
506 | - - graylog |
507 | - nrpe |
508 | + - - nagios |
509 | + - nrpe |
510 | diff --git a/src/tests/functional/tests/test_graylog_charm.py b/src/tests/functional/tests/test_graylog_charm.py |
511 | index df763b6..83d58e7 100644 |
512 | --- a/src/tests/functional/tests/test_graylog_charm.py |
513 | +++ b/src/tests/functional/tests/test_graylog_charm.py |
514 | @@ -2,12 +2,13 @@ |
515 | |
516 | import logging |
517 | import time |
518 | -import unittest |
519 | |
520 | from api import GraylogApi |
521 | |
522 | import tenacity |
523 | |
524 | +from tests.base import BaseTestCase |
525 | + |
526 | import zaza.model as model |
527 | |
528 | |
529 | @@ -17,7 +18,7 @@ DEFAULT_API_PORT = "9001" |
530 | DEFAULT_WEB_PORT = "9000" |
531 | |
532 | |
533 | -class BaseGraylogTest(unittest.TestCase): |
534 | +class BaseGraylogTest(BaseTestCase): |
535 | """Base for Graylog charm tests.""" |
536 | |
537 | @classmethod |
538 | @@ -56,10 +57,14 @@ class BaseGraylogTest(unittest.TestCase): |
539 | "org.graylog.plugins.threatintel.ThreatIntelPlugin", |
540 | } |
541 | |
542 | - api_url = "http://{}:{}/api".format(cls.graylog_ip, cls.api_port) |
543 | + api_url = "{}://{}:{}/api".format( |
544 | + cls.get_api_protocol(), cls.graylog_ip, cls.api_port |
545 | + ) |
546 | action = model.run_action_on_leader(cls.application_name, "show-admin-password") |
547 | res = action.data["results"] |
548 | - cls.api = GraylogApi(api_url, "admin", res["admin-password"]) |
549 | + cls.api = GraylogApi( |
550 | + api_url, "admin", res["admin-password"], verify=cls.ca_path |
551 | + ) |
552 | # try harder to get an api connection to cater for overloaded testsystems |
553 | cls.api.req_timeout = REQ_TIMEOUT |
554 | logging.debug("API at {}".format(api_url)) |
555 | @@ -75,7 +80,9 @@ class CharmOperationTest(BaseGraylogTest): |
556 | until the CURL_TIMEOUT because it may take a few seconds for the |
557 | graylog systemd service to start. |
558 | """ |
559 | - curl_command = "curl http://localhost:{}".format(self.api_port) |
560 | + curl_command = "curl {}://localhost:{}".format( |
561 | + self.get_api_protocol(), self.api_port |
562 | + ) |
563 | timeout = time.time() + CURL_TIMEOUT |
564 | while time.time() < timeout: |
565 | response = model.run_on_unit(self.lead_unit_name, curl_command) |
566 | diff --git a/src/tests/functional/tests/test_legacy.py b/src/tests/functional/tests/test_legacy.py |
567 | index 5ee2eea..fcea119 100644 |
568 | --- a/src/tests/functional/tests/test_legacy.py |
569 | +++ b/src/tests/functional/tests/test_legacy.py |
570 | @@ -1,9 +1,14 @@ |
571 | """Graylog v2 legacy tests.""" |
572 | +import logging |
573 | import re |
574 | import unittest |
575 | |
576 | from api import GraylogApi |
577 | |
578 | +from charmhelpers.core.decorators import retry_on_exception |
579 | + |
580 | +from tests.base import BaseTestCase |
581 | + |
582 | import yaml |
583 | |
584 | from zaza import model |
585 | @@ -14,7 +19,7 @@ DEFAULT_API_PORT = "9001" # Graylog 2 only |
586 | DEFAULT_WEB_PORT = "9000" |
587 | |
588 | |
589 | -class LegacyTests(unittest.TestCase): |
590 | +class LegacyTests(BaseTestCase): |
591 | """These tests were ported from tests/test_10_basic.py in changeset a3b54c2. |
592 | |
593 | They were temporarily deleted during the conversion from Amulet to Zaza. |
594 | @@ -24,8 +29,12 @@ class LegacyTests(unittest.TestCase): |
595 | |
596 | def test_api_ready(self): |
597 | """Curl the api endpoint on the graylog unit.""" |
598 | + protocol = self.get_api_protocol() |
599 | port = self.get_api_port() |
600 | - curl_command = "curl http://localhost:{} --connect-timeout 30".format(port) |
601 | + curl_command = ("curl {}://localhost:{} " "--connect-timeout 30").format( |
602 | + protocol, port |
603 | + ) |
604 | + logging.info(curl_command) |
605 | self.run_command("graylog/0", curl_command) |
606 | |
607 | def test_elasticsearch_active(self): |
608 | @@ -60,10 +69,16 @@ class LegacyTests(unittest.TestCase): |
609 | """Return the list of API clients to Graylog units.""" |
610 | graylog_units = juju.get_full_juju_status().applications["graylog"]["units"] |
611 | graylog_addrs = [unit["public-address"] for unit in graylog_units.values()] |
612 | + protocol = self.get_api_protocol() |
613 | port = self.get_api_port() |
614 | password = self.get_graylog_admin_password() |
615 | return [ |
616 | - GraylogApi("http://{}:{}/api".format(graylog_addr, port), "admin", password) |
617 | + GraylogApi( |
618 | + "{}://{}:{}/api".format(protocol, graylog_addr, port), |
619 | + "admin", |
620 | + password, |
621 | + verify=self.ca_path, |
622 | + ) |
623 | for graylog_addr in graylog_addrs |
624 | ] |
625 | |
626 | @@ -76,8 +91,17 @@ class LegacyTests(unittest.TestCase): |
627 | password = result.data["results"]["admin-password"] |
628 | return password |
629 | |
630 | - def test_website_active(self): |
631 | + def test_website_active_with_haproxy(self): |
632 | """Verify if the Graylog endpoints are correctly configured.""" |
633 | + try: |
634 | + model.get_application("haproxy") |
635 | + except KeyError: |
636 | + reason = ( |
637 | + "haproxy is not in the model, assuming there is " |
638 | + "no front loadbalancer" |
639 | + ) |
640 | + raise unittest.SkipTest(reason) |
641 | + |
642 | data = juju.get_relation_from_unit("haproxy/0", "graylog/0", "website") |
643 | self.assertEqual(data["port"], DEFAULT_WEB_PORT) |
644 | |
645 | @@ -117,6 +141,7 @@ class LegacyTests(unittest.TestCase): |
646 | ) |
647 | self.assertTrue(re.search(r"CRITICAL", cfg)) |
648 | |
649 | + @retry_on_exception(num_retries=5, base_delay=2, exc_type=AssertionError) |
650 | def get_file_contents(self, unit, filename): |
651 | """Retrieve content of a file in a remote unit.""" |
652 | result = self.run_command(unit, "cat '{}'".format(filename)) |
653 | diff --git a/src/tests/functional/tests/tests.yaml b/src/tests/functional/tests/tests.yaml |
654 | index c52c200..30306e4 100644 |
655 | --- a/src/tests/functional/tests/tests.yaml |
656 | +++ b/src/tests/functional/tests/tests.yaml |
657 | @@ -1,9 +1,11 @@ |
658 | charm_name: graylog |
659 | gate_bundles: |
660 | - gl2: bionic-graylog2 |
661 | - - gl3: bionic-graylog3 |
662 | - gl2: focal-graylog2 |
663 | + - gl3: bionic-graylog3 |
664 | + - gl3: bionic-graylog3-tls |
665 | - gl3: focal-graylog3 |
666 | + - gl3: focal-graylog3-tls |
667 | # - gl2: bionic-graylog2-ha |
668 | # - gl3: bionic-graylog3-ha |
669 | smoke_bundles: |
670 | @@ -22,9 +24,26 @@ target_deploy_status: |
671 | filebeat: |
672 | workload-status: active |
673 | workload-status-message: 'Filebeat ready.' |
674 | +<<<<<<< src/tests/functional/tests/tests.yaml |
675 | haproxy: |
676 | workload-status: unknown |
677 | workload-status-message: "" |
678 | nrpe: |
679 | workload-status: blocked |
680 | workload-status-message: Nagios server not configured or related |
681 | +======= |
682 | + mongo: |
683 | + workload-status: active |
684 | + workload-status-message: 'Unit is ready' |
685 | + graylog: |
686 | + workload-status: active |
687 | + workload-status-message: 'Ready with: filebeat, elasticsearch, mongodb' |
688 | + elastic: |
689 | + workload-status: active |
690 | + workload-status-message: 'Unit is ready' |
691 | + nrpe: |
692 | + workload-status: active |
693 | + workload-status-message: 'ready' |
694 | + easyrsa: |
695 | + workload-status-message: 'Certificate Authority connected.' |
696 | +>>>>>>> src/tests/functional/tests/tests.yaml |
697 | diff --git a/src/tests/unit/test_graylog.py b/src/tests/unit/test_graylog.py |
698 | index c723ce1..ffd7d79 100644 |
699 | --- a/src/tests/unit/test_graylog.py |
700 | +++ b/src/tests/unit/test_graylog.py |
701 | @@ -1,10 +1,15 @@ |
702 | """Tests around the graylog reactive script.""" |
703 | import os |
704 | +import socket |
705 | import sys |
706 | import tempfile |
707 | import unittest |
708 | from unittest import mock |
709 | |
710 | +from charms.layer.graylog.constants import ( |
711 | + CERT_KEY_PATH, |
712 | + CERT_PATH, |
713 | +) |
714 | from charms.layer.graylog.snap_change import ( |
715 | ChannelChangeStatus, |
716 | _is_channel_valid, |
717 | @@ -17,6 +22,8 @@ leader_mock = mock.Mock() |
718 | sys.modules["charms.leadership"] = leader_mock |
719 | snap_mock = mock.Mock() |
720 | sys.modules["charms.layer.snap"] = snap_mock |
721 | +tls_client_mock = mock.Mock() |
722 | +sys.modules["charms.layer.tls_client"] = tls_client_mock |
723 | |
724 | from files import check_graylog_health # noqa: E402 |
725 | |
726 | @@ -28,6 +35,7 @@ from reactive.graylog import ( # noqa: E402 |
727 | refresh_graylog, |
728 | set_conf, |
729 | set_jvm_heap_size, |
730 | + tls_request_certificate, |
731 | update_config, |
732 | upgrade_charm, |
733 | ) |
734 | @@ -555,3 +563,78 @@ class TestIsChannelValid(unittest.TestCase): |
735 | "2", # track w/o risk |
736 | ): |
737 | self.assertFalse(_is_channel_valid(channel)) |
738 | + |
739 | + |
740 | +class TestTlsClient(unittest.TestCase): |
741 | + """Test TLS Client integration.""" |
742 | + |
743 | + @mock.patch("charmhelpers.core.host.mkdir") |
744 | + @mock.patch("charmhelpers.core.host.write_file") |
745 | + @mock.patch("charmhelpers.core.hookenv.charm_dir") |
746 | + @mock.patch("charms.layer.graylog.utils.is_v2") |
747 | + @mock.patch("reactive.graylog.set_conf") |
748 | + @mock.patch("reactive.graylog.hookenv.unit_public_ip") |
749 | + @mock.patch("reactive.graylog.hookenv.unit_private_ip") |
750 | + @mock.patch("reactive.graylog.install") |
751 | + def test_request_certificate( |
752 | + self, |
753 | + mock_install, |
754 | + mock_private_ip, |
755 | + mock_public_ip, |
756 | + mock_set_conf, |
757 | + mock_is_v2, |
758 | + mock_charm_dir, |
759 | + mock_write_file, |
760 | + mock_mkdir, |
761 | + ): # noqa: D102 |
762 | + charm_dir = os.path.join(os.path.dirname(os.path.abspath(__file__)), "../../") |
763 | + mock_charm_dir.return_value = charm_dir |
764 | + mock_is_v2.return_value = False |
765 | + mock_public_ip.return_value = "1.2.3.4" |
766 | + mock_private_ip.return_value = "5.6.7.8" |
767 | + mock_tls = mock.Mock() |
768 | + tls_request_certificate(mock_tls) |
769 | + |
770 | + tls_client_mock.request_server_cert.assert_called_with( |
771 | + "1.2.3.4", |
772 | + ["1.2.3.4", "5.6.7.8", socket.gethostname(), "127.0.0.1", "localhost"], |
773 | + crt_path=CERT_PATH, |
774 | + key_path=CERT_KEY_PATH, |
775 | + ) |
776 | + mock_set_conf.assert_has_calls( |
777 | + [ |
778 | + mock.call("rest_enable_tls", "true"), |
779 | + mock.call("rest_tls_cert_file", CERT_PATH), |
780 | + mock.call("rest_tls_key_file", CERT_KEY_PATH), |
781 | + mock.call("web_enable_tls", "true"), |
782 | + mock.call("web_tls_cert_file", CERT_PATH), |
783 | + mock.call("web_tls_key_file", CERT_KEY_PATH), |
784 | + ] |
785 | + ) |
786 | + |
787 | + with open(os.path.join(charm_dir, "templates/sync-graylog-snap"), "rb") as f: |
788 | + sync_script = f.read().strip() |
789 | + |
790 | + with open( |
791 | + os.path.join(charm_dir, "templates/default-graylog-server"), "rb" |
792 | + ) as f: |
793 | + default_graylog_server = f.read().strip() |
794 | + |
795 | + mock_write_file.assert_has_calls( |
796 | + [ |
797 | + mock.call( |
798 | + "/etc/ca-certificates/update.d/sync-graylog-snap", |
799 | + sync_script, |
800 | + "root", |
801 | + "root", |
802 | + 0o755, |
803 | + ), |
804 | + mock.call( |
805 | + "/var/snap/graylog/current/default-graylog-server", |
806 | + default_graylog_server, |
807 | + "root", |
808 | + "root", |
809 | + 0o644, |
810 | + ), |
811 | + ] |
812 | + ) |
813 | diff --git a/src/tests/unit/test_lib.py b/src/tests/unit/test_lib.py |
814 | index 2f7a7c3..5849ac5 100644 |
815 | --- a/src/tests/unit/test_lib.py |
816 | +++ b/src/tests/unit/test_lib.py |
817 | @@ -12,17 +12,29 @@ from charms.layer.graylog import ( |
818 | class TestLibraryUtils(unittest.TestCase): |
819 | """Test lib.charms.layer.graylog utils.""" |
820 | |
821 | + @mock.patch("charms.layer.graylog.utils.get_flags") |
822 | @mock.patch("charms.layer.graylog.utils.is_v2") |
823 | - def test_api(self, mock_v2): |
824 | + def test_api(self, mock_v2, mock_get_flags): |
825 | """Validate the api port/url match what we expect for v2 and v3.""" |
826 | mock_v2.return_value = True |
827 | self.assertEqual(get_api_port(), "9001") |
828 | + mock_get_flags.return_value = [] |
829 | self.assertEqual(get_api_url(), "http://127.0.0.1:9001/api/") |
830 | |
831 | + mock_get_flags.reset_mock() |
832 | + mock_get_flags.return_value = ["graylog.certificates.configured"] |
833 | + self.assertEqual(get_api_url(), "https://127.0.0.1:9001/api/") |
834 | + |
835 | mock_v2.return_value = False |
836 | self.assertEqual(get_api_port(), "9000") |
837 | + mock_get_flags.reset_mock() |
838 | + mock_get_flags.return_value = [] |
839 | self.assertEqual(get_api_url(), "http://127.0.0.1:9000/api/") |
840 | |
841 | + mock_get_flags.reset_mock() |
842 | + mock_get_flags.return_value = ["graylog.certificates.configured"] |
843 | + self.assertEqual(get_api_url(), "https://127.0.0.1:9000/api/") |
844 | + |
845 | @mock.patch("charms.layer.graylog.utils.is_v2") |
846 | def test_validate_api_url(self, mock_v2): |
847 | """Validate the URI suffix is valid for v2 and v3.""" |
This review is large, but I actually found the overall review to be good. I have no real critical feedback.
Re: the zaza bundles, something we've been adopting in other charms to reduce duplication is to have a single base bundle with all the common apps/relations ("base.yaml", not listed in tests.yaml), replace the existing bundles with symlinks to that base bundle, and then have all the per-bundle customizations moved to overlays. I don't think that's critical for this MR, but I'm mentioning it as it seems a good practice to reduce duplication and the chance of accidental divergence between bundles.
Re: code changes: the only thing I would suggest is that "make black" is run; src/tests/ functional/ tests/test_ graylog_ upgrade. py is not passing "make lint" but will simply by running "make black" and committing the changes.
I'll get this running a full test suite on my charmlab box... I'm +1 once the "make black" changes are in, assuming no failures arise during tests.