Skip to content

Commit

Permalink
Update Redis Enterprise to set tls_verify to true by default in check…
Browse files Browse the repository at this point in the history
… code (ECOINT-109) (#2607)

* Update redis_enterprise to set tls_verify to false in check code and add corresponding test

* Fix linting

* Remove reduntant test code and push version to 2.0.0 for breaking change

* Update install instructions to v2.0.0

* Update tls_verify default handling, spec.yaml, and tests

* Update version to 1.1.2

* Change .get() to .setdefault(), sync ddev with latest for tls_cipher changes
  • Loading branch information
mitcherthewitcher authored Mar 3, 2025
1 parent e8d1c2a commit ebe8b41
Show file tree
Hide file tree
Showing 10 changed files with 40 additions and 6 deletions.
6 changes: 6 additions & 0 deletions redis_enterprise/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
# CHANGELOG - Redis Enterprise

## 1.1.2 / 2025-02-28

***Changed***:

* Update `tls_verify` handling in check code to ensure default is set to 'True'

## 1.1.1 / 2025-01-21

***Changed***:
Expand Down
2 changes: 1 addition & 1 deletion redis_enterprise/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ For a full list of supported metrics, see the [Metrics](#metrics) section below.

1. Run the following command to install the Agent integration:
```shell
datadog-agent integration install -t datadog-redis_enterprise==1.1.1
datadog-agent integration install -t datadog-redis_enterprise==1.1.2
```

2. Configure the integration by setting the `openmetrics_endpoint` to your cluster's master node. See [Integration][2] for further information.
Expand Down
3 changes: 2 additions & 1 deletion redis_enterprise/assets/configuration/spec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,10 @@ files:
The endpoint should be the URL of the metric exporter of the enterprise instance
openmetrics_endpoint.display_priority: 3
tls_verify.value.example: false
tls_verify.value.display_default: true
tls_verify.enabled: true
tls_verify.display_priority: 1
extra_metrics.value.example: [ foo, bar ]
extra_metrics.enabled: false
exclude_metrics.value.example: [ foo, bar ]
exclude_metrics.enabled: false
exclude_metrics.enabled: false
Original file line number Diff line number Diff line change
@@ -1 +1 @@
__version__ = '1.1.1'
__version__ = '1.1.2'
1 change: 1 addition & 0 deletions redis_enterprise/datadog_checks/redis_enterprise/check.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ def __init__(self, name, init_config, instances):
def _parse_config(self):
self.scraper_configs = []
metrics_endpoint = self.instance.get('openmetrics_endpoint')
self.instance.setdefault("tls_verify", True)
metrics = self.get_default_config()

additional = []
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ def instance_tls_use_host_header():


def instance_tls_verify():
return False
return True


def instance_use_latest_spec():
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ class InstanceConfig(BaseModel):
timeout: Optional[float] = None
tls_ca_cert: Optional[str] = None
tls_cert: Optional[str] = None
tls_ciphers: Optional[tuple[str, ...]] = None
tls_ignore_warning: Optional[bool] = None
tls_private_key: Optional[str] = None
tls_protocols_allowed: Optional[tuple[str, ...]] = None
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ instances:
#
- openmetrics_endpoint: https://<host>:8070/metrics

## @param tls_verify - boolean - optional - default: false
## @param tls_verify - boolean - optional - default: true
## Instructs the check to validate the TLS certificate of services.
#
tls_verify: false
Expand Down Expand Up @@ -501,6 +501,16 @@ instances:
# - TLSv1.2
# - TLSv1.3

## @param tls_ciphers - list of strings - optional
## The list of ciphers suites to use when connecting to an endpoint. If not specified,
## `ALL` ciphers are used. For list of ciphers see:
## https://www.openssl.org/docs/man1.0.2/man1/ciphers.html
#
# tls_ciphers:
# - TLS_AES_256_GCM_SHA384
# - TLS_CHACHA20_POLY1305_SHA256
# - TLS_AES_128_GCM_SHA256

## @param headers - mapping - optional
## The headers parameter allows you to send specific headers with every request.
## You can use it for explicitly specifying the host header or adding headers for
Expand Down
2 changes: 2 additions & 0 deletions redis_enterprise/tests/support.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@

ERSATZ_INSTANCE = {'openmetrics_endpoint': "https://localhost:8071/metrics", 'tags': ['instance']}

SSL_INSTANCE = {"openmetrics_endpoint": "https://localhost:8071/metrics", "tls_verify": True}

EPHEMERAL = [
"rdse.bdb_avg_latency",
"rdse.bdb_avg_latency_max",
Expand Down
15 changes: 14 additions & 1 deletion redis_enterprise/tests/test_redis_enterprise.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
# from datadog_checks.dev.utils import get_metadata_metrics
from datadog_checks.redis_enterprise.check import RedisEnterpriseCheck

from .support import CHECK, DEFAULT_METRICS, EPHEMERAL, ERSATZ_INSTANCE, INSTANCE, METRICS_MAP
from .support import CHECK, DEFAULT_METRICS, EPHEMERAL, ERSATZ_INSTANCE, INSTANCE, METRICS_MAP, SSL_INSTANCE

ssl._create_default_https_context = ssl._create_unverified_context

Expand Down Expand Up @@ -116,3 +116,16 @@ def test_invalid_instance(aggregator, dd_run_check, mock_http_response):
assert True

aggregator.assert_service_check(f'{RedisEnterpriseCheck.__NAMESPACE__}.node_imaginary', count=0)


@pytest.mark.unit
def test_invalid_ssl_instance(aggregator, dd_run_check, mock_http_response):
# Create instance with tls_verify: True
instance = deepcopy(SSL_INSTANCE)
instance.pop('tls_verify') # Simulating missing tls_verify in config

check = RedisEnterpriseCheck(CHECK, {}, [instance])
dd_run_check(check)

# Ensure tls_verify defaults to True
assert check.scraper_configs[0]["tls_verify"] is True, "tls_verify should default to True"

0 comments on commit ebe8b41

Please sign in to comment.