From 15638ee12fcf31cef22266706e63e11c40ed2f84 Mon Sep 17 00:00:00 2001 From: Denys Zhdanov Date: Sun, 19 Feb 2023 20:07:50 +0100 Subject: [PATCH 1/2] Rebasing #1700 --- docs/config-carbon.rst | 29 +++++++++++---- docs/config-local-settings.rst | 2 +- setup.py | 2 +- tox.ini | 2 +- webapp/content/js/composer_widgets.js | 12 +++---- webapp/graphite/app_settings.py | 2 +- webapp/graphite/local_settings.py.example | 2 +- .../{whitelist => metric_filters}/__init__.py | 0 .../{whitelist => metric_filters}/urls.py | 6 ++-- .../{whitelist => metric_filters}/views.py | 36 +++++++++---------- webapp/graphite/settings.py | 6 ++-- webapp/graphite/urls.py | 2 +- 12 files changed, 58 insertions(+), 43 deletions(-) rename webapp/graphite/{whitelist => metric_filters}/__init__.py (100%) rename webapp/graphite/{whitelist => metric_filters}/urls.py (77%) rename webapp/graphite/{whitelist => metric_filters}/views.py (60%) diff --git a/docs/config-carbon.rst b/docs/config-carbon.rst index fd9867dd7..aae0c7a6a 100644 --- a/docs/config-carbon.rst +++ b/docs/config-carbon.rst @@ -269,13 +269,28 @@ aggregation. **Note:** if you plan to use the ``=`` sign in your rewrite rules. Use its octal value: ``\075``. For example ``foo=bar = foo.bar`` would be ``foo\075bar = foo.bar`` +Metric Filters: allowed and blocked Metrics +------------------------------------------- + +The metric filter functionality allows any of the carbon daemons to only accept +metrics that are explicitly allowed and/or to reject rejected metrics. The +functionality can be enabled in carbon.conf with the ``USE_METRIC_FILTERS`` +flag. This can be useful when too many metrics are being sent to a Graphite +instance or when there are metric senders sending useless or invalid metrics. + +``GRAPHITE_CONF_DIR`` is searched for ``allowed_metrics.conf`` and +``blocked_metrics.conf``. Each file contains one regular expression per line to +match against metric values. If the allowed_metrics configuration is missing or +empty, all metrics will be passed through by default. + whitelist and blacklist ----------------------- -The whitelist functionality allows any of the carbon daemons to only accept metrics that are explicitly -whitelisted and/or to reject blacklisted metrics. The functionality can be enabled in carbon.conf with -the ``USE_WHITELIST`` flag. This can be useful when too many metrics are being sent to a Graphite -instance or when there are metric senders sending useless or invalid metrics. +The whitelist/blacklist functionality has been renamed to 'allowed' and +'blocked' so as to use less ambiguous language, and remove possible connotations +associated with those terms. + +The capabilities have been renamed as of this point, but he existing ``whitelist.conf`` +and ``blacklist.conf`` will still be functional for the time being. -``GRAPHITE_CONF_DIR`` is searched for ``whitelist.conf`` and ``blacklist.conf``. Each file contains one regular -expressions per line to match against metric values. If the whitelist configuration is missing or empty, -all metrics will be passed through by default. +Additionally, the ``USE_WHITELIST`` flag in carbon.conf will still be respected, +but treated as ``USE_METRIC_FILTERS`` until it is deprecated. \ No newline at end of file diff --git a/docs/config-local-settings.rst b/docs/config-local-settings.rst index 547ad2820..7582f335d 100644 --- a/docs/config-local-settings.rst +++ b/docs/config-local-settings.rst @@ -434,7 +434,7 @@ POOL_MAX_WORKERS REMOTE_RETRY_DELAY `Default: 60` - Time in seconds to blacklist a webapp after a timed-out request. + Time in seconds to block/filter a webapp after a timed-out request. FIND_CACHE_DURATION `Default: 300` diff --git a/setup.py b/setup.py index e72ee5690..7d1dc5fdf 100644 --- a/setup.py +++ b/setup.py @@ -101,7 +101,7 @@ def read(fname): 'graphite.url_shortener', 'graphite.url_shortener.migrations', 'graphite.version', - 'graphite.whitelist', + 'graphite.metric_filters', 'graphite.worker_pool', ], package_data={'graphite': ['templates/*', 'local_settings.py.example']}, diff --git a/tox.ini b/tox.ini index 3a4c5864a..749a7d9a1 100644 --- a/tox.ini +++ b/tox.ini @@ -11,7 +11,7 @@ envlist = lint, docs [testenv] -whitelist_externals = +whitelist_externals = # Unrelated to graphite project deprecation of white/black mkdir setenv = DJANGO_SETTINGS_MODULE=tests.settings diff --git a/webapp/content/js/composer_widgets.js b/webapp/content/js/composer_widgets.js index dd8bb6a13..a6cb206c3 100644 --- a/webapp/content/js/composer_widgets.js +++ b/webapp/content/js/composer_widgets.js @@ -1041,20 +1041,20 @@ var GraphDataWindow = { addWlSelected: function (item, e) { Ext.Ajax.request({ - url: document.body.dataset.baseUrl + 'whitelist/add', + url: document.body.dataset.baseUrl + 'metric_filters/add', method: 'POST', - success: function () { Ext.Msg.alert('Result', 'Successfully added metrics to whitelist.'); }, - failure: function () { Ext.Msg.alert('Result', 'Failed to add metrics to whitelist.'); }, + success: function () { Ext.Msg.alert('Result', 'Successfully added metrics to filter.'); }, + failure: function () { Ext.Msg.alert('Result', 'Failed to add metrics to filter.'); }, params: {metrics: this.getSelectedTargets().join('\n') } }); }, removeWlSelected: function (item, e) { Ext.Ajax.request({ - url: document.body.dataset.baseUrl + 'whitelist/remove', + url: document.body.dataset.baseUrl + 'metric_filters/remove', method: 'POST', - success: function () { Ext.Msg.alert('Result', 'Successfully removed metrics from whitelist.'); }, - failure: function () { Ext.Msg.alert('Result', 'Failed to remove metrics from whitelist.'); }, + success: function () { Ext.Msg.alert('Result', 'Successfully removed metrics from filter.'); }, + failure: function () { Ext.Msg.alert('Result', 'Failed to remove metrics from filter.'); }, params: {metrics: this.getSelectedTargets().join('\n') } }); }, diff --git a/webapp/graphite/app_settings.py b/webapp/graphite/app_settings.py index 493e61b00..07d03b33d 100644 --- a/webapp/graphite/app_settings.py +++ b/webapp/graphite/app_settings.py @@ -105,7 +105,7 @@ 'graphite.render', 'graphite.tags', 'graphite.url_shortener', - 'graphite.whitelist', + 'graphite.metric_filters', 'django.contrib.auth', 'django.contrib.sessions', 'django.contrib.admin', diff --git a/webapp/graphite/local_settings.py.example b/webapp/graphite/local_settings.py.example index ea053e266..b01845379 100644 --- a/webapp/graphite/local_settings.py.example +++ b/webapp/graphite/local_settings.py.example @@ -337,7 +337,7 @@ DEFAULT_XFILES_FACTOR = 0 # #CARBONLINK_HOSTS = ["127.0.0.1:7002:a", "127.0.0.1:7102:b", "127.0.0.1:7202:c"] #CARBONLINK_TIMEOUT = 1.0 -#CARBONLINK_RETRY_DELAY = 15 # Seconds to blacklist a failed remote server +#CARBONLINK_RETRY_DELAY = 15 # Seconds to block a failed remote server # Set pickle protocol to use for Carbonlink requests, # (default of -1 is HIGHEST_AVAILABLE for your Python version) # see more: https://docs.python.org/3/library/pickle.html#data-stream-format diff --git a/webapp/graphite/whitelist/__init__.py b/webapp/graphite/metric_filters/__init__.py similarity index 100% rename from webapp/graphite/whitelist/__init__.py rename to webapp/graphite/metric_filters/__init__.py diff --git a/webapp/graphite/whitelist/urls.py b/webapp/graphite/metric_filters/urls.py similarity index 77% rename from webapp/graphite/whitelist/urls.py rename to webapp/graphite/metric_filters/urls.py index 0030f29d1..dec191926 100644 --- a/webapp/graphite/whitelist/urls.py +++ b/webapp/graphite/metric_filters/urls.py @@ -16,7 +16,7 @@ from . import views urlpatterns = [ - url(r'^/add$', views.add, name='whitelist_add'), - url(r'^/remove$', views.remove, name='whitelist_remove'), - url(r'^/?$', views.show, name='whitelist_show'), + url(r'^/add$', views.add, name='metric_filters_add'), + url(r'^/remove$', views.remove, name='metric_filters_remove'), + url(r'^/?$', views.show, name='metric_filters_show'), ] diff --git a/webapp/graphite/whitelist/views.py b/webapp/graphite/metric_filters/views.py similarity index 60% rename from webapp/graphite/whitelist/views.py rename to webapp/graphite/metric_filters/views.py index 4cb68cb3e..e1ea47a84 100644 --- a/webapp/graphite/whitelist/views.py +++ b/webapp/graphite/metric_filters/views.py @@ -23,43 +23,43 @@ def add(request): metrics = set( request.POST['metrics'].split() ) - whitelist = load_whitelist() - new_whitelist = whitelist | metrics - save_whitelist(new_whitelist) + allowed_metrics = load_allowed_metrics() + new_allowed_metrics = allowed_metrics | metrics + save_allowed_metrics(new_allowed_metrics) return HttpResponse(content_type="text/plain", content="OK") def remove(request): metrics = set( request.POST['metrics'].split() ) - whitelist = load_whitelist() - new_whitelist = whitelist - metrics - save_whitelist(new_whitelist) + allowed_metrics = load_allowed_metrics() + new_allowed_metrics = allowed_metrics - metrics + save_allowed_metrics(new_allowed_metrics) return HttpResponse(content_type="text/plain", content="OK") def show(request): - whitelist = load_whitelist() - members = '\n'.join( sorted(whitelist) ) + allowed_metrics = load_allowed_metrics() + members = '\n'.join( sorted(allowed_metrics) ) return HttpResponse(content_type="text/plain", content=members) -def load_whitelist(): - buffer = open(settings.WHITELIST_FILE, 'rb').read() - whitelist = unpickle.loads(buffer) - return whitelist +def load_allowed_metrics(): + buffer = open(settings.METRIC_FILTERS_FILE, 'rb').read() + allowed_metrics = unpickle.loads(buffer) + return allowed_metrics -def save_whitelist(whitelist): +def save_allowed_metrics(allowed_metrics): # do this instead of dump() to raise potential exceptions before open() - serialized = pickle.dumps(whitelist, protocol=-1) - tmpfile = '%s-%d' % (settings.WHITELIST_FILE, randint(0, 100000)) + serialized = pickle.dumps(allowed_metrics, protocol=-1) + tmpfile = '%s-%d' % (settings.METRIC_FILTERS_FILE, randint(0, 100000)) try: fh = open(tmpfile, 'wb') fh.write(serialized) fh.close() - if os.path.exists(settings.WHITELIST_FILE): - os.unlink(settings.WHITELIST_FILE) - os.rename(tmpfile, settings.WHITELIST_FILE) + if os.path.exists(settings.METRIC_FILTERS_FILE): + os.unlink(settings.METRIC_FILTERS_FILE) + os.rename(tmpfile, settings.METRIC_FILTERS_FILE) finally: if os.path.exists(tmpfile): os.unlink(tmpfile) diff --git a/webapp/graphite/settings.py b/webapp/graphite/settings.py index 2cfd9447d..82f4051d0 100644 --- a/webapp/graphite/settings.py +++ b/webapp/graphite/settings.py @@ -50,7 +50,7 @@ DASHBOARD_CONF = '' GRAPHTEMPLATES_CONF = '' STORAGE_DIR = '' -WHITELIST_FILE = '' +METRIC_FILTERS_FILE = '' INDEX_FILE = '' LOG_DIR = '' CERES_DIR = '' @@ -266,8 +266,8 @@ if not STORAGE_DIR: STORAGE_DIR = os.environ.get('GRAPHITE_STORAGE_DIR', join(GRAPHITE_ROOT, 'storage')) -if not WHITELIST_FILE: - WHITELIST_FILE = join(STORAGE_DIR, 'lists', 'whitelist') +if not METRIC_FILTERS_FILE: + METRIC_FILTERS_FILE = join(STORAGE_DIR, 'lists', 'allowed_metrics') if not INDEX_FILE: INDEX_FILE = join(STORAGE_DIR, 'index') if not LOG_DIR: diff --git a/webapp/graphite/urls.py b/webapp/graphite/urls.py index 5bdee3043..0be7ab4f3 100644 --- a/webapp/graphite/urls.py +++ b/webapp/graphite/urls.py @@ -25,7 +25,7 @@ url('^browser', include('graphite.browser.urls')), url('^account', include('graphite.account.urls')), url('^dashboard', include('graphite.dashboard.urls')), - url('^whitelist', include('graphite.whitelist.urls')), + url('^metric_filters/?', include('graphite.metric_filters.urls')), url('^version', include('graphite.version.urls')), url('^events', include('graphite.events.urls')), url('^tags', include('graphite.tags.urls')), From ed09c9a8e4f0cbd96c9385a3dc382b424d97f034 Mon Sep 17 00:00:00 2001 From: Denys Zhdanov Date: Sun, 19 Feb 2023 20:29:31 +0100 Subject: [PATCH 2/2] fixing tests --- webapp/tests/test_whitelist.py | 82 +++++++++++++++++----------------- 1 file changed, 41 insertions(+), 41 deletions(-) diff --git a/webapp/tests/test_whitelist.py b/webapp/tests/test_whitelist.py index 32a4dcf75..db1718ab4 100644 --- a/webapp/tests/test_whitelist.py +++ b/webapp/tests/test_whitelist.py @@ -12,108 +12,108 @@ from django.core.urlresolvers import reverse from .base import TestCase -from graphite.whitelist.views import load_whitelist, save_whitelist +from graphite.metric_filters.views import load_allowed_metrics, save_allowed_metrics class WhitelistTester(TestCase): - settings.WHITELIST_FILE = os.path.join(DATA_DIR, 'lists/whitelist') + settings.METRIC_FILTERS_FILE = os.path.join(DATA_DIR, 'lists/allowlist') - def wipe_whitelist(self): + def wipe_allowlist(self): try: - os.remove(settings.WHITELIST_FILE) + os.remove(settings.METRIC_FILTERS_FILE) except OSError: pass - def create_whitelist(self): + def create_allowlist(self): try: - os.makedirs(settings.WHITELIST_FILE.replace('whitelist', '')) + os.makedirs(settings.METRIC_FILTERS_FILE.replace('allowlist', '')) except OSError: pass - fh = open(settings.WHITELIST_FILE, 'wb') + fh = open(settings.METRIC_FILTERS_FILE, 'wb') pickle.dump({'a.b.c.d', 'e.f.g.h'}, fh) fh.close() - def test_whitelist_show_no_whitelist(self): - url = reverse('whitelist_show') + def test_allowed_metrics_show_no_allowlist(self): + url = reverse('metric_filters_show') with self.assertRaises(IOError): _ = self.client.get(url) - def test_whitelist_show(self): - url = reverse('whitelist_show') - self.create_whitelist() - self.addCleanup(self.wipe_whitelist) + def test_allowed_metrics_show(self): + url = reverse('metric_filters_show') + self.create_allowlist() + self.addCleanup(self.wipe_allowlist) response = self.client.get(url) self.assertEqual(response.status_code, 200) self.assertEqual(response.content, b"a.b.c.d\ne.f.g.h") - def test_whitelist_add(self): - self.create_whitelist() - self.addCleanup(self.wipe_whitelist) + def test_allowed_metrics_add(self): + self.create_allowlist() + self.addCleanup(self.wipe_allowlist) - url = reverse('whitelist_add') + url = reverse('metric_filters_add') response = self.client.post(url, {'metrics': ['i.j.k.l']}) self.assertEqual(response.status_code, 200) self.assertEqual(response.content, b"OK") - url = reverse('whitelist_show') + url = reverse('metric_filters_show') response = self.client.get(url) self.assertEqual(response.status_code, 200) self.assertEqual(response.content, b"a.b.c.d\ne.f.g.h\ni.j.k.l") - def test_whitelist_add_existing(self): - self.create_whitelist() - self.addCleanup(self.wipe_whitelist) + def test_allowed_metrics_add_existing(self): + self.create_allowlist() + self.addCleanup(self.wipe_allowlist) - url = reverse('whitelist_add') + url = reverse('metric_filters_add') response = self.client.post(url, {'metrics': ['a.b.c.d']}) self.assertEqual(response.status_code, 200) self.assertEqual(response.content, b"OK") - url = reverse('whitelist_show') + url = reverse('metric_filters_show') response = self.client.get(url) self.assertEqual(response.status_code, 200) self.assertEqual(response.content, b"a.b.c.d\ne.f.g.h") - def test_whitelist_remove(self): - self.create_whitelist() - self.addCleanup(self.wipe_whitelist) + def test_allowed_metrics_remove(self): + self.create_allowlist() + self.addCleanup(self.wipe_allowlist) - url = reverse('whitelist_remove') + url = reverse('metric_filters_remove') response = self.client.post(url, {'metrics': ['a.b.c.d']}) self.assertEqual(response.status_code, 200) self.assertEqual(response.content, b"OK") - url = reverse('whitelist_show') + url = reverse('metric_filters_show') response = self.client.get(url) self.assertEqual(response.status_code, 200) self.assertEqual(response.content, b"e.f.g.h") - def test_whitelist_remove_missing(self): - self.create_whitelist() - self.addCleanup(self.wipe_whitelist) + def test_allowed_metrics_remove_missing(self): + self.create_allowlist() + self.addCleanup(self.wipe_allowlist) - url = reverse('whitelist_remove') + url = reverse('metric_filters_remove') response = self.client.post(url, {'metrics': ['i.j.k.l']}) self.assertEqual(response.status_code, 200) self.assertEqual(response.content, b"OK") - url = reverse('whitelist_show') + url = reverse('metric_filters_show') response = self.client.get(url) self.assertEqual(response.status_code, 200) self.assertEqual(response.content, b"a.b.c.d\ne.f.g.h") - def test_save_whitelist(self): + def test_save_allowed_metrics(self): try: - os.makedirs(settings.WHITELIST_FILE.replace('whitelist', '')) + os.makedirs(settings.METRIC_FILTERS_FILE.replace('allowlist', '')) except OSError: pass - self.addCleanup(self.wipe_whitelist) - self.assertEqual(save_whitelist({'a.b.c.d','e.f.g.h'}), None) - self.assertEqual(load_whitelist(), {'a.b.c.d','e.f.g.h'}) + self.addCleanup(self.wipe_allowlist) + self.assertEqual(save_allowed_metrics({'a.b.c.d','e.f.g.h'}), None) + self.assertEqual(load_allowed_metrics(), {'a.b.c.d','e.f.g.h'}) @mock.patch('os.rename') - def test_save_whitelist_rename_failure(self, rename): - self.addCleanup(self.wipe_whitelist) + def test_save_allowed_metrics_rename_failure(self, rename): + self.addCleanup(self.wipe_allowlist) rename.side_effect = OSError(errno.EPERM, 'Operation not permitted') with self.assertRaises(OSError): - save_whitelist({'a.b.c.d','e.f.g.h'}) + save_allowed_metrics({'a.b.c.d','e.f.g.h'})