diff --git a/proxysql/README.md b/proxysql/README.md index 499f53b4788e7..90b02245f40f2 100644 --- a/proxysql/README.md +++ b/proxysql/README.md @@ -14,6 +14,14 @@ The ProxySQL integration is included in the [Datadog Agent][3] package, so you d ### Configuration +#### Enabling SSL +To connect to ProxySQL using full SSL/TLS validation, enable the `tls_verify` option in `conf.yaml`. Include certificates and passwords needed to connect via SSL/TLS. + +```yaml + tls_verify: true + tls_ca_cert: ca_cert.pem +``` + diff --git a/proxysql/assets/configuration/spec.yaml b/proxysql/assets/configuration/spec.yaml index 6ba0724713f1c..4ec4d284dd4fa 100644 --- a/proxysql/assets/configuration/spec.yaml +++ b/proxysql/assets/configuration/spec.yaml @@ -34,23 +34,13 @@ files: value: type: string example: - - name: tls_verify - value: - example: false - type: boolean - description: Instructs the check to use SSL when connecting to ProxySQL - - name: tls_ca_cert - value: - example: - type: string - description: | - The path to the `proxysql-ca.pem` file generated by ProxySQL when using SSL (or your own). - More information: https://github.com/sysown/proxysql/wiki/SSL-Support#ssl-configuration-for-frontends - - name: validate_hostname - value: - example: true - type: boolean - description: Whether or not to verify the certificate was issued for the given `host` + - template: instances/tls + overrides: + tls_verify.default: false + tls_verify.value.example: false + tls_cert.hidden: true + tls_private_key.hidden: true + tls_private_key_password.hidden: true - name: connect_timeout required: false description: | diff --git a/proxysql/datadog_checks/proxysql/data/conf.yaml.example b/proxysql/datadog_checks/proxysql/data/conf.yaml.example index 549955b8606c3..2e5b1b55fa213 100644 --- a/proxysql/datadog_checks/proxysql/data/conf.yaml.example +++ b/proxysql/datadog_checks/proxysql/data/conf.yaml.example @@ -34,20 +34,24 @@ instances: password: ## @param tls_verify - boolean - optional - default: false - ## Instructs the check to use SSL when connecting to ProxySQL + ## Instructs the check to validate the TLS certificate(s) of the service(s). # # tls_verify: false ## @param tls_ca_cert - string - optional - ## The path to the `proxysql-ca.pem` file generated by ProxySQL when using SSL (or your own). - ## More information: https://github.com/sysown/proxysql/wiki/SSL-Support#ssl-configuration-for-frontends + ## The path to a file of concatenated CA certificates in PEM format or a directory + ## containing several CA certificates in PEM format. If a directory, the directory + ## must have been processed using the c_rehash utility supplied with OpenSSL. See: + ## https://www.openssl.org/docs/manmaster/man3/SSL_CTX_load_verify_locations.html + ## + ## Setting this implicitly sets `tls_verify` to true. # # tls_ca_cert: - ## @param validate_hostname - boolean - optional - default: true - ## Whether or not to verify the certificate was issued for the given `host` + ## @param tls_validate_hostname - boolean - optional - default: true + ## Verifies that the server's cert hostname matches the one requested. # - # validate_hostname: true + # tls_validate_hostname: true ## @param connect_timeout - integer - optional - default: 10 ## Timeout in seconds for connecting to ProxySQL. diff --git a/proxysql/datadog_checks/proxysql/proxysql.py b/proxysql/datadog_checks/proxysql/proxysql.py index 89bdfafa97d8d..afb4a0ddcc12a 100644 --- a/proxysql/datadog_checks/proxysql/proxysql.py +++ b/proxysql/datadog_checks/proxysql/proxysql.py @@ -6,7 +6,7 @@ import pymysql import pymysql.cursors -from datadog_checks.base import AgentCheck, ConfigurationError +from datadog_checks.base import AgentCheck, ConfigurationError, is_affirmative from datadog_checks.base.utils.db import QueryManager from .queries import ( @@ -18,7 +18,7 @@ STATS_MYSQL_USERS, VERSION_METADATA, ) -from .ssl_utils import make_insecure_ssl_client_context, make_secure_ssl_client_context +from .ssl_utils import make_insecure_ssl_client_context ADDITIONAL_METRICS_MAPPING = { 'command_counters_metrics': STATS_COMMAND_COUNTERS, @@ -34,6 +34,11 @@ class ProxysqlCheck(AgentCheck): SERVICE_CHECK_NAME = "can_connect" __NAMESPACE__ = "proxysql" + # This remapper is used to support legacy Proxysql integration config values + TLS_CONFIG_REMAPPER = { + 'validate_hostname': {'name': 'tls_validate_hostname'}, + } + def __init__(self, name, init_config, instances): super(ProxysqlCheck, self).__init__(name, init_config, instances) self.host = self.instance.get("host", "") @@ -44,9 +49,8 @@ def __init__(self, name, init_config, instances): if not all((self.host, self.port, self.user, self.password)): raise ConfigurationError("ProxySQL host, port, username and password are needed") - self.tls_verify = self.instance.get("tls_verify", False) - self.validate_hostname = self.instance.get("validate_hostname", True) - self.tls_ca_cert = self.instance.get("tls_ca_cert") + self.tls_verify = is_affirmative(self.instance.get('tls_verify', False)) + self.connect_timeout = self.instance.get("connect_timeout", 10) self.read_timeout = self.instance.get("read_timeout") @@ -91,11 +95,11 @@ def execute_query_raw(self, query): @contextmanager def connect(self): if self.tls_verify: + self.log.debug("Connecting to ProxySQL via SSL/TLS") # If ca_cert is None, will load the default certificates - ssl_context = make_secure_ssl_client_context( - ca_cert=self.tls_ca_cert, check_hostname=self.validate_hostname - ) + ssl_context = self.get_tls_context() else: + self.log.debug("Connecting to ProxySQL without SSL/TLS") ssl_context = make_insecure_ssl_client_context() db = None diff --git a/proxysql/datadog_checks/proxysql/ssl_utils.py b/proxysql/datadog_checks/proxysql/ssl_utils.py index 1643b564fa9cf..0e585963ebc85 100644 --- a/proxysql/datadog_checks/proxysql/ssl_utils.py +++ b/proxysql/datadog_checks/proxysql/ssl_utils.py @@ -1,11 +1,8 @@ # (C) Datadog, Inc. 2020-present # All rights reserved # Licensed under a 3-clause BSD style license (see LICENSE) -import os import ssl -from datadog_checks.base import ConfigurationError - def make_insecure_ssl_client_context(): """Creates an insecure ssl context for integration that requires to use TLS without checking @@ -16,53 +13,3 @@ def make_insecure_ssl_client_context(): context = ssl.SSLContext(protocol=ssl.PROTOCOL_TLS) context.verify_mode = ssl.CERT_NONE return context - - -def make_secure_ssl_client_context( - ca_cert=None, client_cert=None, client_key=None, check_hostname=True, protocol=ssl.PROTOCOL_TLS -): - """Creates a secure ssl context for integration that requires one. - :param str ca_cert: Path to a file of concatenated CA certificates in PEM format or to a directory containing - several CA certificates in PEM format - :param str client_cert: Path to a single file in PEM format containing the certificate as well as any number of - CA certificates needed to establish the certificate's authenticity. - :param str client_key: Must point to a file containing the private key. Otherwise the private key will be taken - from certfile as well. - :param bool check_hostname: Whether to match the peer cert's hostname - :param int protocol: Client side protocol (should be one of the `ssl.PROTOCOL_*` constants) - By default selects the highest protocol version possible. - - :rtype ssl.Context - """ - # https://docs.python.org/3/library/ssl.html#ssl.SSLContext - # https://docs.python.org/3/library/ssl.html#ssl.PROTOCOL_TLS - context = ssl.SSLContext(protocol=protocol) - - # https://docs.python.org/3/library/ssl.html#ssl.SSLContext.verify_mode - context.verify_mode = ssl.CERT_REQUIRED - - # https://docs.python.org/3/library/ssl.html#ssl.SSLContext.check_hostname - context.check_hostname = check_hostname - - ca_file, ca_path = None, None - if os.path.isdir(ca_cert): - ca_path = ca_cert - elif os.path.isfile(ca_cert): - ca_file = ca_cert - else: - raise ConfigurationError("Specified tls_ca_cert: {} should be a valid file or directory.".format(ca_cert)) - - # https://docs.python.org/3/library/ssl.html#ssl.SSLContext.load_verify_locations - if ca_file or ca_path: - context.load_verify_locations(ca_file, ca_path, None) - - # https://docs.python.org/3/library/ssl.html#ssl.SSLContext.load_default_certs - else: - context.load_default_certs(ssl.Purpose.SERVER_AUTH) - - # https://docs.python.org/3/library/ssl.html#ssl.SSLContext.load_cert_chain - if client_cert: - # If client_key is not defined, load_cert_chain reads the key from the client_cert - context.load_cert_chain(client_cert, keyfile=client_key) - - return context diff --git a/proxysql/setup.py b/proxysql/setup.py index f5f96d2003614..a45e32078e40a 100644 --- a/proxysql/setup.py +++ b/proxysql/setup.py @@ -27,7 +27,7 @@ def get_dependencies(): return f.readlines() -CHECKS_BASE_REQ = ['datadog-checks-base>=11.3.1'] # Needs fix integrations-core/#6146 for the QueryManager +CHECKS_BASE_REQ = ['datadog-checks-base>=15.2.0'] setup( diff --git a/proxysql/tests/common.py b/proxysql/tests/common.py index 924a8aaf69b09..4398a03a21761 100644 --- a/proxysql/tests/common.py +++ b/proxysql/tests/common.py @@ -1,6 +1,10 @@ # (C) Datadog, Inc. 2020-present # All rights reserved # Licensed under a 3-clause BSD style license (see LICENSE) +import os + +from datadog_checks.dev import get_docker_hostname + GLOBAL_METRICS = ( 'proxysql.active_transactions', 'proxysql.query_processor_time_pct', @@ -99,3 +103,71 @@ + USER_TAGS_METRICS + QUERY_RULES_TAGS_METRICS ) + +DOCKER_HOST = get_docker_hostname() +MYSQL_PORT = 6612 +PROXY_PORT = 6033 +PROXY_ADMIN_PORT = 6032 +MYSQL_USER = 'proxysql' +MYSQL_PASS = 'pass' +PROXY_ADMIN_USER = 'proxy' +PROXY_ADMIN_PASS = 'proxy' +PROXY_STATS_USER = 'proxystats' +PROXY_STATS_PASS = 'proxystats' +MYSQL_DATABASE = 'test' +PROXY_MAIN_DATABASE = 'main' +PROXYSQL_VERSION = os.environ['PROXYSQL_VERSION'] + +BASIC_INSTANCE = { + 'host': DOCKER_HOST, + 'port': PROXY_ADMIN_PORT, + 'username': PROXY_ADMIN_USER, + 'password': PROXY_ADMIN_PASS, + 'tags': ["application:test"], + 'additional_metrics': [], +} + + +BASIC_INSTANCE_TLS = { + 'host': DOCKER_HOST, + 'port': PROXY_ADMIN_PORT, + 'username': PROXY_ADMIN_USER, + 'password': PROXY_ADMIN_PASS, + 'tags': ["application:test"], + 'additional_metrics': [], + 'tls_verify': True, + 'tls_ca_cert': "/etc/ssl/certs/proxysql-ca.pem", + 'tls_validate_hostname': True, +} + + +INSTANCE_ALL_METRICS = { + 'host': DOCKER_HOST, + 'port': PROXY_ADMIN_PORT, + 'username': PROXY_ADMIN_USER, + 'password': PROXY_ADMIN_PASS, + 'tags': ["application:test"], + 'additional_metrics': [ + 'command_counters_metrics', + 'connection_pool_metrics', + 'users_metrics', + 'memory_metrics', + 'query_rules_metrics', + ], +} + +INSTANCE_ALL_METRICS_STATS = { + 'host': DOCKER_HOST, + 'port': PROXY_ADMIN_PORT, + 'username': PROXY_STATS_USER, + 'password': PROXY_STATS_PASS, + 'database_name': PROXY_MAIN_DATABASE, + 'tags': ["application:test"], + 'additional_metrics': [ + 'command_counters_metrics', + 'connection_pool_metrics', + 'users_metrics', + 'memory_metrics', + 'query_rules_metrics', + ], +} diff --git a/proxysql/tests/conftest.py b/proxysql/tests/conftest.py index d3a64389d1a12..2d8bb455ef4b5 100644 --- a/proxysql/tests/conftest.py +++ b/proxysql/tests/conftest.py @@ -4,65 +4,29 @@ import os from copy import deepcopy +import mock import pymysql import pytest -from datadog_checks.dev import TempDir, docker_run, get_docker_hostname, get_here +from datadog_checks.dev import TempDir, docker_run, get_here from datadog_checks.dev.conditions import CheckDockerLogs, WaitFor - -DOCKER_HOST = get_docker_hostname() -MYSQL_PORT = 6612 -PROXY_PORT = 6033 -PROXY_ADMIN_PORT = 6032 -MYSQL_USER = 'proxysql' -MYSQL_PASS = 'pass' -PROXY_ADMIN_USER = 'proxy' -PROXY_ADMIN_PASS = 'proxy' -PROXY_STATS_USER = 'proxystats' -PROXY_STATS_PASS = 'proxystats' -MYSQL_DATABASE = 'test' -PROXY_MAIN_DATABASE = 'main' -PROXYSQL_VERSION = os.environ['PROXYSQL_VERSION'] - -BASIC_INSTANCE = { - 'host': DOCKER_HOST, - 'port': PROXY_ADMIN_PORT, - 'username': PROXY_ADMIN_USER, - 'password': PROXY_ADMIN_PASS, - 'tags': ["application:test"], - 'additional_metrics': [], -} - -INSTANCE_ALL_METRICS = { - 'host': DOCKER_HOST, - 'port': PROXY_ADMIN_PORT, - 'username': PROXY_ADMIN_USER, - 'password': PROXY_ADMIN_PASS, - 'tags': ["application:test"], - 'additional_metrics': [ - 'command_counters_metrics', - 'connection_pool_metrics', - 'users_metrics', - 'memory_metrics', - 'query_rules_metrics', - ], -} - -INSTANCE_ALL_METRICS_STATS = { - 'host': DOCKER_HOST, - 'port': PROXY_ADMIN_PORT, - 'username': PROXY_STATS_USER, - 'password': PROXY_STATS_PASS, - 'database_name': PROXY_MAIN_DATABASE, - 'tags': ["application:test"], - 'additional_metrics': [ - 'command_counters_metrics', - 'connection_pool_metrics', - 'users_metrics', - 'memory_metrics', - 'query_rules_metrics', - ], -} +from datadog_checks.proxysql import ProxysqlCheck + +from .common import ( + ALL_METRICS, + BASIC_INSTANCE, + BASIC_INSTANCE_TLS, + DOCKER_HOST, + INSTANCE_ALL_METRICS, + INSTANCE_ALL_METRICS_STATS, + MYSQL_DATABASE, + MYSQL_PASS, + MYSQL_PORT, + MYSQL_USER, + PROXY_ADMIN_PORT, + PROXY_PORT, + PROXYSQL_VERSION, +) @pytest.fixture @@ -70,6 +34,11 @@ def instance_basic(): return deepcopy(BASIC_INSTANCE) +@pytest.fixture +def instance_basic_tls(): + return deepcopy(BASIC_INSTANCE_TLS) + + @pytest.fixture() def instance_all_metrics(instance_basic): return deepcopy(INSTANCE_ALL_METRICS) @@ -120,3 +89,28 @@ def init_mysql(): def init_proxy(): pymysql.connect(host=DOCKER_HOST, port=PROXY_PORT, user=MYSQL_USER, passwd=MYSQL_PASS) + + +def get_check(instance): + """Simple helper method to get a check instance from a config instance.""" + return ProxysqlCheck('proxysql', {}, [instance]) + + +def _assert_all_metrics(aggregator): + for metric in ALL_METRICS: + aggregator.assert_metric(metric) + + aggregator.assert_all_metrics_covered() + + +def _assert_metadata(datadog_agent, check_id=''): + raw_version = PROXYSQL_VERSION + major, minor = raw_version.split('.')[:2] + version_metadata = { + 'version.scheme': 'semver', + 'version.major': major, + 'version.minor': minor, + 'version.patch': mock.ANY, + 'version.raw': mock.ANY, + } + datadog_agent.assert_metadata(check_id, version_metadata) diff --git a/proxysql/tests/test_e2e.py b/proxysql/tests/test_e2e.py new file mode 100644 index 0000000000000..50a5f5ad9f423 --- /dev/null +++ b/proxysql/tests/test_e2e.py @@ -0,0 +1,13 @@ +# (C) Datadog, Inc. 2020-present +# All rights reserved +# Licensed under a 3-clause BSD style license (see LICENSE) +import pytest + +from .conftest import _assert_all_metrics, _assert_metadata + + +@pytest.mark.e2e +def test_e2e(dd_agent_check, datadog_agent): + aggregator = dd_agent_check(rate=True) + _assert_metadata(datadog_agent, check_id='proxysql') + _assert_all_metrics(aggregator) diff --git a/proxysql/tests/test_proxysql.py b/proxysql/tests/test_integration.py similarity index 59% rename from proxysql/tests/test_proxysql.py rename to proxysql/tests/test_integration.py index ae8a47fcf972f..1684631b4047e 100644 --- a/proxysql/tests/test_proxysql.py +++ b/proxysql/tests/test_integration.py @@ -1,15 +1,11 @@ # (C) Datadog, Inc. 2020-present # All rights reserved # Licensed under a 3-clause BSD style license (see LICENSE) -import mock import pytest from datadog_checks.base import AgentCheck -from datadog_checks.base.errors import ConfigurationError -from datadog_checks.proxysql import ProxysqlCheck from .common import ( - ALL_METRICS, COMMANDS_COUNTERS_METRICS, CONNECTION_POOL_METRICS, GLOBAL_METRICS, @@ -17,49 +13,7 @@ QUERY_RULES_TAGS_METRICS, USER_TAGS_METRICS, ) -from .conftest import PROXYSQL_VERSION - - -def get_check(instance): - """Simple helper method to get a check instance from a config instance.""" - return ProxysqlCheck('proxysql', {}, [instance]) - - -@pytest.mark.unit -def test_wrong_config(dd_run_check, instance_basic): - # Empty instance - with pytest.raises(ConfigurationError, match='ProxySQL host, port, username and password are needed'): - dd_run_check(get_check({})) - - # Only host - with pytest.raises(ConfigurationError, match='ProxySQL host, port, username and password are needed'): - dd_run_check(get_check({'host': 'localhost'})) - - # Missing password - with pytest.raises(ConfigurationError, match='ProxySQL host, port, username and password are needed'): - dd_run_check(get_check({'host': 'localhost', 'port': 6032, 'username': 'admin'})) - - # Wrong additional metrics group - with pytest.raises( - ConfigurationError, - match="There is no additional metric group called 'foo' for the ProxySQL integration, it should be one of ", - ): - instance_basic['additional_metrics'].append('foo') - dd_run_check(get_check(instance_basic)) - - -@pytest.mark.unit -def test_config_ok(dd_run_check): - check = get_check({'host': 'localhost', 'port': 6032, 'username': 'admin', 'password': 'admin'}) - connect_mock, query_executor_mock = mock.MagicMock(), mock.MagicMock() - - check.connect = connect_mock - check._query_manager.executor = query_executor_mock - - dd_run_check(check) - - connect_mock.assert_called_once() - assert query_executor_mock.call_count == 2 +from .conftest import _assert_all_metrics, _assert_metadata, get_check @pytest.mark.integration @@ -145,26 +99,6 @@ def test_metadata(datadog_agent, dd_run_check, instance_basic): _assert_metadata(datadog_agent) -def _assert_all_metrics(aggregator): - for metric in ALL_METRICS: - aggregator.assert_metric(metric) - - aggregator.assert_all_metrics_covered() - - -def _assert_metadata(datadog_agent, check_id=''): - raw_version = PROXYSQL_VERSION - major, minor = raw_version.split('.')[:2] - version_metadata = { - 'version.scheme': 'semver', - 'version.major': major, - 'version.minor': minor, - 'version.patch': mock.ANY, - 'version.raw': mock.ANY, - } - datadog_agent.assert_metadata(check_id, version_metadata) - - @pytest.mark.integration @pytest.mark.usefixtures('dd_environment') def test_all_metrics(aggregator, instance_all_metrics, dd_run_check): @@ -179,10 +113,3 @@ def test_all_metrics_stats_user(aggregator, instance_stats_user, dd_run_check): check = get_check(instance_stats_user) dd_run_check(check) _assert_all_metrics(aggregator) - - -@pytest.mark.e2e -def test_e2e(dd_agent_check, datadog_agent): - aggregator = dd_agent_check(rate=True) - _assert_metadata(datadog_agent, check_id='proxysql') - _assert_all_metrics(aggregator) diff --git a/proxysql/tests/test_unit.py b/proxysql/tests/test_unit.py new file mode 100644 index 0000000000000..b6ab69dacf2de --- /dev/null +++ b/proxysql/tests/test_unit.py @@ -0,0 +1,96 @@ +# (C) Datadog, Inc. 2020-present +# All rights reserved +# Licensed under a 3-clause BSD style license (see LICENSE) + +import mock +import pytest + +from datadog_checks.base.errors import ConfigurationError +from datadog_checks.proxysql import ProxysqlCheck + +from .conftest import get_check + + +@pytest.mark.unit +def test_empty_instance(dd_run_check): + with pytest.raises(ConfigurationError, match='ProxySQL host, port, username and password are needed'): + dd_run_check(get_check({})) + + +@pytest.mark.unit +def test_only_host(dd_run_check): + with pytest.raises(ConfigurationError, match='ProxySQL host, port, username and password are needed'): + dd_run_check(get_check({'host': 'localhost'})) + + +@pytest.mark.unit +def test_missing_password(dd_run_check): + with pytest.raises(ConfigurationError, match='ProxySQL host, port, username and password are needed'): + dd_run_check(get_check({'host': 'localhost', 'port': 6032, 'username': 'admin'})) + + +@pytest.mark.unit +def test_wrong_additional_metrics_group(dd_run_check, instance_basic): + with pytest.raises( + ConfigurationError, + match="There is no additional metric group called 'foo' for the ProxySQL integration, it should be one of ", + ): + instance_basic['additional_metrics'].append('foo') + dd_run_check(get_check(instance_basic)) + + +@pytest.mark.unit +def test_config_ok(dd_run_check, instance_basic): + check = get_check(instance_basic) + connect_mock, query_executor_mock = mock.MagicMock(), mock.MagicMock() + + check.connect = connect_mock + check._query_manager.executor = query_executor_mock + + dd_run_check(check) + + connect_mock.assert_called_once() + assert query_executor_mock.call_count == 2 + + +@pytest.mark.unit +def test_tls_config_ok(dd_run_check, instance_basic_tls): + with mock.patch('datadog_checks.base.utils.tls.ssl') as ssl: + with mock.patch('datadog_checks.proxysql.proxysql.pymysql') as pymysql: + check = get_check(instance_basic_tls) + + # mock TLS context + tls_context = mock.MagicMock() + ssl.SSLContext.return_value = tls_context + + # mock query executor + query_executor_mock = mock.MagicMock() + check._query_manager.executor = query_executor_mock + + # mock behavior of db + mock_db = mock.MagicMock() + mock_db.close.return_value = True + pymysql.connect.return_value = mock_db + + dd_run_check(check) + + assert check.tls_verify is True + assert tls_context.check_hostname is True + assert check._connection is not None + + +@pytest.mark.parametrize( + 'extra_config, expected_http_kwargs', + [ + pytest.param( + {'validate_hostname': False}, {'tls_validate_hostname': False}, id='legacy validate_hostname param' + ), + ], +) +def test_tls_config_legacy(extra_config, expected_http_kwargs, instance_all_metrics): + instance = instance_all_metrics + instance.update(extra_config) + c = ProxysqlCheck('proxysql', {}, [instance]) + c.get_tls_context() # need to call this for config values to be saved by _tls_context_wrapper + actual_options = {k: v for k, v in c._tls_context_wrapper.config.items() if k in expected_http_kwargs} + assert expected_http_kwargs == actual_options