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

[Barry Price]

Add support for ip_range in the prometheus-client output plugin

[Laurent Sesquès (sajoupa)]

pre-req MP: https://code.launchpad.net/~sajoupa/charm-telegraf/+git/telegraf-charm/+merge/397005

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

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

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

Unable to determine commit message from repository - please click "Set commit message" and enter the commit message manually.

[Paul Goins (vultaire)]

Just a quick note in passing so far:

* Generally looks good
* I need to take a proper look at the tests for this later, unit and functional - it seems sane, but I likely need more context than I'm seeing here to properly review. I'll try to come back to this when I have more time.

Left one minor code comment re: code which could be done in less lines.

[Laurent Sesquès (sajoupa)]

Thanks. Replied to the comment inline.

[Haw Loeung (hloeung)]

LGTM, comment inline.

review: Approve (+1)

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

Failed to merge change (unable to merge source repository due to conflicts), setting status to needs review.

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

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

[Haw Loeung (hloeung)]

LGTM

review: Approve (+1)

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

Change successfully merged at revision 562a3cc1aaeb05a0e28874ddbd6a597459c44f8f

diff --git a/src/config.yaml b/src/config.yaml
index 66f6105..b94071b 100644
--- a/src/config.yaml
+++ b/src/config.yaml
@@ -206,3 +206,9 @@ options:
         Note: The complete datasource inserted into dashboard will be
         "$prometheus_datasource - Juju generated source"
         This matches how the Prometheus2/Grafana charms format the datasource.
+  prometheus_ip_range:
+    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]"
diff --git a/src/reactive/telegraf.py b/src/reactive/telegraf.py
index 82cf10e..47819cd 100644
--- a/src/reactive/telegraf.py
+++ b/src/reactive/telegraf.py
@@ -362,6 +362,13 @@ def get_prometheus_port():
         return int(config.get("prometheus_output_port"))
 
 
+def get_prometheus_ip_range():
+    config = hookenv.config()
+    if config.get("prometheus_ip_range") == "":
+        return []
+    return config.get("prometheus_ip_range").split(",")
+
+
 def get_socket_listener_port():
     config = hookenv.config()
     if not config.get("socket_listener_port", False):
@@ -699,7 +706,9 @@ def handle_config_changes():
         clear_flag("plugins.prometheus-client.configured")
         clear_flag("prometheus-client.relation.configured")
 
-    if config.changed("prometheus_output_port"):
+    if config.changed("prometheus_output_port") or config.changed(
+        "prometheus_ip_range"
+    ):
         clear_flag("plugins.prometheus-client.configured")
         clear_flag("prometheus-client.relation.configured")
     clear_flag("telegraf.configured")
@@ -1220,7 +1229,7 @@ def influxdb_api_output(influxdb):
     set_flag("telegraf.needs_reload")
 
 
-def generate_prometheus_output_config(prometheus_output_port):
+def generate_prometheus_output_config(prometheus_output_port, prometheus_ip_range):
     # If extra_options are set for prometheus_client, let's integrate them
     extra_options = get_extra_options()
     options = extra_options["outputs"].get("prometheus_client", {})
@@ -1239,21 +1248,27 @@ def generate_prometheus_output_config(prometheus_output_port):
         )
         listen = "{}:{}".format(listen.split(":", 1)[0], prometheus_output_port)
 
+    options_ip_range = options.pop("ip_range", [])
+    ip_range = options_ip_range + prometheus_ip_range
+
     return {
         "listen": listen,
+        "ip_range": ip_range,
         "extra_options": render_extra_options(
             "outputs", "prometheus_client", extra_options=extra_options
         ),
     }
 
 
-def render_prometheus_client_config(port):
+def render_prometheus_client_config(port, ip_range):
     config_path = "{}/{}.conf".format(get_configs_dir(), "prometheus_client")
     hookenv.log(
-        "Updating {} plugin config file. Port is {}".format("prometheus_client", port),
+        "Updating {} plugin config file. Port is {} and ip_range is {}".format(
+            "prometheus_client", port, ip_range
+        ),
         level=hookenv.INFO,
     )
-    context = generate_prometheus_output_config(port)
+    context = generate_prometheus_output_config(port, ip_range)
     render(
         source="prometheus_client.tmpl",
         templates_dir=get_templates_dir(),
@@ -1301,7 +1316,11 @@ def configure_prometheus_client_with_relation(prometheus):
             ip_addr = None
         prometheus.configure(port, hostname=ip_addr, private_address=ip_addr)
     check_prometheus_port("prometheus_output", port)
-    render_prometheus_client_config(port)
+    # If prometheus_ip_range is empty, all remote IPs are allowed
+    ip_range = get_prometheus_ip_range()
+    if ip_range != []:
+        ip_range = ip_range + remote_egress_subnets
+    render_prometheus_client_config(port, ip_range)
     set_flag("plugins.prometheus-client.configured")
     set_flag("prometheus-client.relation.configured")
     set_flag("telegraf.needs_reload")
@@ -1318,7 +1337,8 @@ def configure_prometheus_client():
         set_flag("plugins.prometheus-client.configured")
         return
     check_prometheus_port("prometheus_output", port)
-    render_prometheus_client_config(port)
+    ip_range = get_prometheus_ip_range()
+    render_prometheus_client_config(port, ip_range)
     set_flag("plugins.prometheus-client.configured")
     set_flag("telegraf.needs_reload")
     clear_flag("prometheus-client.relation.configured")
diff --git a/src/templates/prometheus_client.tmpl b/src/templates/prometheus_client.tmpl
index add2099..a6e3c06 100644
--- a/src/templates/prometheus_client.tmpl
+++ b/src/templates/prometheus_client.tmpl
@@ -1,3 +1,4 @@
 [[outputs.prometheus_client]]
   listen = "{{ listen }}"
+  ip_range = {{ ip_range }}
 {%- if extra_options %}{{ extra_options }}{%- endif %}
diff --git a/src/tests/functional/tests/bundles/bionic-monitoring.yaml b/src/tests/functional/tests/bundles/bionic-monitoring.yaml
index 120eab8..c7a487b 100644
--- a/src/tests/functional/tests/bundles/bionic-monitoring.yaml
+++ b/src/tests/functional/tests/bundles/bionic-monitoring.yaml
@@ -8,6 +8,7 @@ applications:
     num_units: 0
     options:
       prometheus_datasource: prometheus2
+      prometheus_ip_range: '0.0.0.0/0'
   grafana:
     charm: cs:grafana
     num_units: 1
diff --git a/src/tests/unit/test_telegraf.py b/src/tests/unit/test_telegraf.py
index 88603c8..8146f3f 100644
--- a/src/tests/unit/test_telegraf.py
+++ b/src/tests/unit/test_telegraf.py
@@ -810,8 +810,9 @@ def test_prometheus_global(monkeypatch, config):
     monkeypatch.setattr(telegraf.hookenv, "close_port", lambda p: open_ports.remove(p))
     config["prometheus_output_port"] = "default"
     telegraf.configure_prometheus_client()
-    expected = '''[[outputs.prometheus_client]]
-  listen = ":9103"'''
+    expected = """[[outputs.prometheus_client]]
+  listen = ":9103"
+  ip_range = []"""
     config_file = base_dir().join("telegraf.d", "prometheus_client.conf")
     assert expected in config_file.read()
 
@@ -834,6 +835,7 @@ outputs:
     telegraf.configure_prometheus_client()
     expected = """[[outputs.prometheus_client]]
   listen = ":9103"
+  ip_range = []
   namedrop = ["aerospike*"]
   [outputs.prometheus_client.tagpass]
     cpu = ["cpu0"]"""
@@ -1147,18 +1149,36 @@ def test_prometheus_client_output(mocker, monkeypatch, config):
 
     interface = mocker.Mock(spec=RelationBase)
     interface.configure = mocker.Mock()
+
+    # Test that telegraf will allow scraping from related prometheus, and from ranges in
+    # prometheus_ip_range
+    config["prometheus_ip_range"] = "127.0.0.1/32,10.0.0.0/8"
     telegraf.configure_prometheus_client_with_relation(interface)
     expected = """
     [[outputs.prometheus_client]]
   listen = ":9103"
+  ip_range = ['127.0.0.1/32', '10.0.0.0/8', '5.6.7.8/32']
 """
     assert (
         configs_dir().join("prometheus_client.conf").read().strip() == expected.strip()
     )
+    # prometheus_ip_range is empty, test that all IPs are allowed, not just the related
+    # prometheus
+    # prometheus_output_port is also empty, we should get 9103 by default
     network_get_primary_address.assert_called_once_with("prometheus-client")
     interface.configure.assert_called_once_with(
         "9103", hostname="foo", private_address="foo"
     )
+    config["prometheus_ip_range"] = ""
+    telegraf.configure_prometheus_client_with_relation(interface)
+    expected = """
+    [[outputs.prometheus_client]]
+  listen = ":9103"
+  ip_range = []
+"""
+    assert (
+        configs_dir().join("prometheus_client.conf").read().strip() == expected.strip()
+    )
 
 
 def test_prometheus_client_output_departed(mocker, monkeypatch, config):