Merge ~barryprice/charm-telegraf:master into charm-telegraf:master

[Barry Price]

Make our prometheus_ip_range config option a little more resilient

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

This merge proposal is being monitored by mergebot. Change the status to Approved to merge.

[Barry Price (barryprice)]

Addressed feedback, pushed changes

review: Needs Resubmitting

[Haw Loeung (hloeung)]

LGTM

review: Approve (+1)

[Haw Loeung (hloeung)]

LGTM

review: Approve (+1)

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

Change successfully merged at revision a310706c90982b25be4b20c363cc0c99590637a4

diff --git a/src/config.yaml b/src/config.yaml
index ecdc9a7..f2bf6f6 100644
--- a/src/config.yaml
+++ b/src/config.yaml
@@ -210,8 +210,9 @@ options:
     default: ""
     type: string
     description: >
-        If set, the list of IP Ranges which are allowed to access metrics.
-        ex: prometheus_ip_range = "[192.168.0.0/24, 192.168.1.0/30]"
+        If set, the comma-separated list of IP Ranges which are allowed to access
+        metrics.
+        ex: prometheus_ip_range = "192.168.0.0/24,192.168.1.0/30"
   collect_iptables_metrics:
     default: false
     type: boolean
diff --git a/src/reactive/telegraf.py b/src/reactive/telegraf.py
index ea2af96..e3fcad3 100644
--- a/src/reactive/telegraf.py
+++ b/src/reactive/telegraf.py
@@ -17,6 +17,7 @@
 import base64
 import binascii
 import io
+import ipaddress
 import json
 import os
 import re
@@ -84,6 +85,10 @@ class InvalidInstallMethod(Exception):
     pass
 
 
+class InvalidPrometheusIPRange(Exception):
+    pass
+
+
 def write_telegraf_file(path, content):
     return host.write_file(
         path,
@@ -383,7 +388,24 @@ def get_prometheus_ip_range():
     config = hookenv.config()
     if config.get("prometheus_ip_range") == "":
         return []
-    return config.get("prometheus_ip_range").split(",")
+    # we should have a list of IPs, confirm that's the case
+    ips = []
+    for ip in config.get("prometheus_ip_range").split(","):
+        # strip any spaces
+        ip = ip.strip()
+        try:
+            ipaddress.ip_network(ip)
+        except ValueError:
+            hookenv.log(
+                "Invalid prometheus_ip_range provided: {}".format(
+                    config.get("prometheus_ip_range")
+                ),
+                level=hookenv.ERROR,
+            )
+            raise InvalidPrometheusIPRange()
+        else:
+            ips.append(ip)
+    return ips
 
 
 def get_socket_listener_port():
diff --git a/src/tests/unit/test_telegraf.py b/src/tests/unit/test_telegraf.py
index 9771618..56da00f 100644
--- a/src/tests/unit/test_telegraf.py
+++ b/src/tests/unit/test_telegraf.py
@@ -1181,6 +1181,27 @@ def test_prometheus_client_output(mocker, monkeypatch, config):
     )
 
 
+def test_get_prometheus_ip_range_with_bad_data(mocker, monkeypatch, config):
+    # if prometheus_ip_range is populated with an invalid IP, confirm we block
+    config["prometheus_ip_range"] = "1.2.3.4, 5.6.7.8, banana"
+    # we expect to raise this specific exception (and the hook to therefore fail)
+    with pytest.raises(telegraf.InvalidPrometheusIPRange):
+        telegraf.get_prometheus_ip_range()
+
+
+def test_get_prometheus_ip_range_with_good_data(mocker, monkeypatch, config):
+    # if prometheus_ip_range is populated with valid IPs, confirm we don't block
+    status_set = mocker.patch("reactive.telegraf.hookenv.status_set")
+    config["prometheus_ip_range"] = "192.168.0.0/24, 10.0.0.0/8"
+
+    our_range = telegraf.get_prometheus_ip_range()
+    # confirm we didn't block
+    assert not status_set.called
+    # and confirm we got the expected range back
+    expected_range = ["192.168.0.0/24", "10.0.0.0/8"]
+    assert our_range == expected_range
+
+
 def test_prometheus_client_output_departed(mocker, monkeypatch, config):
     configs_dir().join("prometheus_client.conf").write("empty")
     relations = [1]