diff --git a/charmcraft.yaml b/charmcraft.yaml index 79bd42e4..23349ffd 100644 --- a/charmcraft.yaml +++ b/charmcraft.yaml @@ -212,3 +212,14 @@ config: automatically deduced from it). See https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/ type: string + reporting_enabled: + description: | + When disabled, Grafana will be configured to not send anonymous usage statistics to stats.grafana.org, nor + periodically check for updates. + It is very helpful to the Grafana project, so please leave this enabled. + + When enabled, Grafana will use its default values for analytics. + + Ref: https://grafana.com/docs/grafana/latest/setup-grafana/configure-grafana/#analytics + type: boolean + default: true diff --git a/requirements.txt b/requirements.txt index fc139c8b..7eac6b3a 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,8 +1,6 @@ jinja2 < 3 lightkube >= 0.11 -markupsafe == 2.0.1 -# pinned to 2.16 as 2.17 breaks our unittests -ops == 2.16 +ops pyyaml urllib3 jsonschema diff --git a/src/charm.py b/src/charm.py index bee5a36f..f202209a 100755 --- a/src/charm.py +++ b/src/charm.py @@ -752,7 +752,7 @@ def _generate_grafana_config(self) -> str: can be set in ENV variables, but leave for expansion later so we can hide auth secrets """ - configs = [self._generate_tracing_config()] + configs = [self._generate_tracing_config(), self._generate_analytics_config()] if self.has_db: configs.append(self._generate_database_config()) else: @@ -799,6 +799,27 @@ def _generate_tracing_config(self) -> str: ret = data.getvalue() return ret + def _generate_analytics_config(self) -> str: + """Generate analytics configuration. + + Returns: + A string containing the analytics config to be stubbed into the config file. + """ + if self.config["reporting_enabled"]: + return "" + config_ini = configparser.ConfigParser() + # Ref: https://grafana.com/docs/grafana/latest/setup-grafana/configure-grafana/#analytics + config_ini["analytics"] = { + "reporting_enabled": "false", + "check_for_updates": "false", + "check_for_plugin_updates": "false", + } + + data = StringIO() + config_ini.write(data) + ret = data.getvalue() + return ret + def _generate_database_config(self) -> str: """Generate a database configuration. diff --git a/tests/scenario/conftest.py b/tests/scenario/conftest.py index 06370c77..24caf6e3 100644 --- a/tests/scenario/conftest.py +++ b/tests/scenario/conftest.py @@ -1,7 +1,7 @@ from unittest.mock import patch import pytest -from scenario import Context +from ops.testing import Context from charm import GrafanaCharm diff --git a/tests/scenario/test_charm.py b/tests/scenario/test_charm.py deleted file mode 100644 index f20d036d..00000000 --- a/tests/scenario/test_charm.py +++ /dev/null @@ -1,21 +0,0 @@ -import pytest -from scenario import State, PeerRelation - - -@pytest.mark.parametrize("leader", (True, False)) -def test_peer_data_does_not_raise_if_no_peers(ctx, leader): - state = State(leader=leader) - with ctx.manager("update-status", state) as mgr: - charm = mgr.charm - assert not charm.peers - charm.set_peer_data("1", 2) - assert charm.get_peer_data("1") == {} - - -def test_peer_data_if_peers(ctx): - state = State(leader=True, relations=[PeerRelation("grafana")]) - with ctx.manager("update-status", state) as mgr: - charm = mgr.charm - assert charm.peers - charm.set_peer_data("1", 2) - assert charm.get_peer_data("1") == 2 diff --git a/tests/scenario/test_config_reporting_enabled.py b/tests/scenario/test_config_reporting_enabled.py new file mode 100644 index 00000000..14b4edb5 --- /dev/null +++ b/tests/scenario/test_config_reporting_enabled.py @@ -0,0 +1,49 @@ +from ops.testing import State, Container +from configparser import ConfigParser + + +containers = [ + Container(name="grafana", can_connect=True), + Container(name="litestream", can_connect=True), +] + + +def test_reporting_enabled(ctx): + # GIVEN the "reporting_enabled" config option is set to True + state = State(leader=True, config={"reporting_enabled": True}, containers=containers) + + # WHEN config-changed fires + out = ctx.run(ctx.on.config_changed(), state) + + # THEN the config file is written WITHOUT the [analytics] section being rendered + simulated_pebble_filesystem = out.get_container("grafana").get_filesystem(ctx) + grafana_config_path = simulated_pebble_filesystem / "etc/grafana/grafana-config.ini" + + config = ConfigParser() + config.read(grafana_config_path) + assert "analytics" not in config + + +def test_reporting_disabled(ctx): + # GIVEN the "reporting_enabled" config option is set to False + state = State(leader=True, config={"reporting_enabled": False}, containers=containers) + + # WHEN config-changed fires + out = ctx.run(ctx.on.config_changed(), state) + + # THEN the config file is written WITH the [analytics] section being rendered + simulated_pebble_filesystem = out.get_container("grafana").get_filesystem(ctx) + grafana_config_path = simulated_pebble_filesystem / "etc/grafana/grafana-config.ini" + + config = ConfigParser() + config.read(grafana_config_path) + assert "analytics" in config + assert dict(config["analytics"]) == { + "reporting_enabled": "false", + "check_for_updates": "false", + "check_for_plugin_updates": "false", + } + + # AND the "grafana" service is restarted + # TODO Does it make sense to check this if the charm under test's lifetime is only for the config-changed? + # TODO How to assert this? diff --git a/tox.ini b/tox.ini index ed2129d5..b28352ce 100644 --- a/tox.ini +++ b/tox.ini @@ -87,7 +87,7 @@ deps = pytest<8.2.0 # https://github.com/pytest-dev/pytest/issues/12263 responses cosl - ops-scenario<7.0.0 + ops[testing] -r{toxinidir}/requirements.txt commands = /usr/bin/env sh -c 'stat sqlite-static > /dev/null 2>&1 || curl -L https://github.com/CompuRoot/static-sqlite3/releases/latest/download/sqlite3 -o sqlite-static && chmod +x sqlite-static' @@ -105,6 +105,8 @@ deps = asyncstdlib # Libjuju needs to track the juju version juju<=3.3.0,>=3.0 + # https://github.com/juju/python-libjuju/issues/1184 + websockets<14.0 pytest<8.2.0 # https://github.com/pytest-dev/pytest/issues/12263 pytest-operator pytest-playwright