Skip to content

Commit cb40f3f

Browse files
lucia-sbclaude
lucia-sb
and
claude
authored
Validate appliance IP addresses from Orchestrator response in HPE Aruba EdgeConnect (#24130)
* Validate appliance IP addresses from Orchestrator response in HPE Aruba EdgeConnect Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Add changelog entry for PR #24130 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Use neutral names in non-IP appliance test fixtures Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Fix E2E test setup: use static appliance IP on docker network The fake appliance now listens on port 443 inside the container and is assigned a static IP (192.168.200.10) on the docker-compose network. The orchestrator returns this plain IP, which passes strict ipaddress.ip_address() validation. The conftest health check uses the host-exposed port mapping separately. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Rework E2E appliance setup: DNS resolution, no hardcoded subnet The fake orchestrator now resolves the appliance IP via Docker DNS (socket.gethostbyname("dd-appliance")) at request time instead of reading a hardcoded IP from an env var. The fake appliance listens on port 443 inside the container so the check connects via standard HTTPS. Removes the custom network subnet and static IP assignment from docker-compose. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Use get_container_ip to resolve appliance container IP for E2E tests Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Remove socket fallback from fake_orch now that APPLIANCE_IP is always set Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Revert to DNS resolution for appliance IP in fake orchestrator Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Add recommendation to configure appliance_ips allowlist in README Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Extract appliance IP validation into _parse_appliances method Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 752e179 commit cb40f3f

9 files changed

Lines changed: 67 additions & 17 deletions

File tree

‎hpe_aruba_edgeconnect/README.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,8 @@ No additional installation is needed on your server.
3535

3636
**Note**: Monitor permissions are enough to collect most metrics. Admin permissions are required to collect the `hpe_aruba_edgeconnect.device.cpu.usage` metric on the appliances. If the configured credentials do not have admin access, the Agent skips this metric, but collects the rest of the metrics.
3737

38+
It is recommended to configure `appliance_ips` with an explicit include list that matches your known appliance IP ranges. This helps ensure that the check only connects to expected appliances.
39+
3840
2. [Restart the Agent][5].
3941

4042
### Validation

‎hpe_aruba_edgeconnect/changelog.d/24130.fixed‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Validate appliance IP addresses from Orchestrator response before connecting to appliances.

‎hpe_aruba_edgeconnect/datadog_checks/hpe_aruba_edgeconnect/check.py‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
# Licensed under a 3-clause BSD style license (see LICENSE)
44
from __future__ import annotations
55

6+
import ipaddress
67
import json
78
import threading
89
from collections.abc import Callable
@@ -111,13 +112,29 @@ def _submit_metadata(
111112
"network-devices-metadata",
112113
)
113114

115+
def _parse_appliances(self, raw_appliances: list[dict]) -> list[Appliance]:
116+
appliances = []
117+
for raw in raw_appliances:
118+
raw_ip = raw.get('ip', '')
119+
try:
120+
ipaddress.ip_address(raw_ip)
121+
except ValueError:
122+
self.log.warning(
123+
"Skipping appliance with non-IP address %r from orchestrator %s",
124+
raw_ip,
125+
self.config.orchestrator_ip,
126+
)
127+
continue
128+
appliances.append(Appliance(raw))
129+
return appliances
130+
114131
def _collect_appliances_from_orch(self, client: OrchestratorClient) -> Appliances:
115132
raw_appliances = client.get_appliances()
116133
if not raw_appliances:
117134
self.log.warning("No appliances returned from orchestrator %s", self.config.orchestrator_ip)
118135
return Appliances([], self.log)
119136
self.log.debug("Found %d appliances from orchestrator before filtering", len(raw_appliances))
120-
all_appliances = [Appliance(a) for a in raw_appliances]
137+
all_appliances = self._parse_appliances(raw_appliances)
121138
self._peer_lookup = {ap.host_name: (ap.ip, ap.site or 'unknown') for ap in all_appliances if ap.host_name}
122139
appliances = Appliances(all_appliances, self.log)
123140
appliances.filter(self.config.appliance_ips)

‎hpe_aruba_edgeconnect/tests/conftest.py‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -62,14 +62,13 @@ def dd_environment(instance, dd_save_state):
6262
orch_port = find_free_port(HOST)
6363
appliance_port = find_free_port(HOST)
6464
orch_ip = f'{HOST}:{orch_port}'
65-
appliance_ip = f'{HOST}:{appliance_port}'
6665

6766
def _ready():
6867
ctx = ssl.create_default_context()
6968
ctx.check_hostname = False
7069
ctx.verify_mode = ssl.CERT_NONE
7170
urlopen(f'https://{orch_ip}/health', timeout=2, context=ctx)
72-
urlopen(f'https://{appliance_ip}/health', timeout=2, context=ctx)
71+
urlopen(f'https://{HOST}:{appliance_port}/health', timeout=2, context=ctx)
7372

7473
inst = instance(
7574
orch_ip,
@@ -88,7 +87,6 @@ def _ready():
8887
'ORCH_PASSWORD': '',
8988
'APPLIANCE_USERNAME': 'admin',
9089
'APPLIANCE_PASSWORD': '',
91-
'APPLIANCE_IP': appliance_ip,
9290
},
9391
):
9492
yield {'instances': [inst]}

‎hpe_aruba_edgeconnect/tests/docker/docker-compose.yaml‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ services:
99
environment:
1010
ORCH_USERNAME: "${ORCH_USERNAME}"
1111
ORCH_PASSWORD: "${ORCH_PASSWORD}"
12-
APPLIANCE_IP: "${APPLIANCE_IP}"
1312
restart: "no"
1413

1514
appliance:
@@ -18,7 +17,7 @@ services:
1817
dockerfile: tests/docker/fake_appliance/Dockerfile
1918
container_name: dd-appliance
2019
ports:
21-
- "${APPLIANCE_PORT}:8444"
20+
- "${APPLIANCE_PORT}:443"
2221
environment:
2322
APPLIANCE_USERNAME: "${APPLIANCE_USERNAME}"
2423
APPLIANCE_PASSWORD: "${APPLIANCE_PASSWORD}"

‎hpe_aruba_edgeconnect/tests/docker/fake_appliance/Dockerfile‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,6 @@ RUN mkdir -p /app/certs \
2323

2424
COPY tests/docker/fake_appliance/app.py app.py
2525

26-
EXPOSE 8444
26+
EXPOSE 443
2727

2828
CMD ["python", "app.py"]

‎hpe_aruba_edgeconnect/tests/docker/fake_appliance/app.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -190,4 +190,4 @@ def health():
190190
app.config["start_time"] = time.time()
191191
ctx = ssl.SSLContext(ssl.PROTOCOL_TLS_SERVER)
192192
ctx.load_cert_chain(CERT_FILE, KEY_FILE)
193-
app.run(host="0.0.0.0", port=8444, ssl_context=ctx)
193+
app.run(host="0.0.0.0", port=443, ssl_context=ctx)

‎hpe_aruba_edgeconnect/tests/docker/fake_orch/app.py‎

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
"""Fake HPE Aruba EdgeConnect orchestrator for E2E tests."""
55

66
import os
7+
import socket
78
import ssl
89

910
from flask import Flask, jsonify, request
@@ -16,11 +17,14 @@
1617
ORCH_USERNAME = os.environ.get("ORCH_USERNAME", "admin")
1718
ORCH_PASSWORD = os.environ.get("ORCH_PASSWORD", "")
1819

19-
APPLIANCE_IP = os.environ.get("APPLIANCE_IP", "172.16.3.21")
2020
PEER_NEWYORK_IP = "10.0.0.2"
2121
PEER_SANFRAN_IP = "10.0.0.3"
2222

2323

24+
def _appliance_ip():
25+
return socket.gethostbyname("dd-appliance")
26+
27+
2428
def _appliance(ip, ne_pk, host_name, site, startup_time=None):
2529
return {
2630
"id": ne_pk,
@@ -60,13 +64,6 @@ def _appliance(ip, ne_pk, host_name, site, startup_time=None):
6064
}
6165

6266

63-
APPLIANCE_LIST = [
64-
_appliance(APPLIANCE_IP, "1.NE", "SydneySP01", "SYD", startup_time=86400),
65-
_appliance(PEER_NEWYORK_IP, "4.NE", "NewYorkSP01", "NYC"),
66-
_appliance(PEER_SANFRAN_IP, "5.NE", "SanFranSP02", "SFO"),
67-
]
68-
69-
7067
@app.route("/gms/rest/authentication/login", methods=["POST"])
7168
def login():
7269
data = request.get_json(silent=True) or {}
@@ -77,7 +74,13 @@ def login():
7774

7875
@app.route("/gms/rest/appliance")
7976
def appliances():
80-
return jsonify(APPLIANCE_LIST)
77+
return jsonify(
78+
[
79+
_appliance(_appliance_ip(), "1.NE", "SydneySP01", "SYD", startup_time=86400),
80+
_appliance(PEER_NEWYORK_IP, "4.NE", "NewYorkSP01", "NYC"),
81+
_appliance(PEER_SANFRAN_IP, "5.NE", "SanFranSP02", "SFO"),
82+
]
83+
)
8184

8285

8386
@app.route("/gms/rest/gms/overlays/config")

‎hpe_aruba_edgeconnect/tests/test_unit.py‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,36 @@ def test_appliances_filter(dd_run_check, aggregator, mocker, instance, ips, filt
110110
assert monitored_ips == sorted(expected_ips)
111111

112112

113+
@pytest.mark.parametrize(
114+
'bad_ip',
115+
[
116+
pytest.param('appliance.example.com', id='hostname'),
117+
pytest.param('appliance.example.com:8443', id='host_port'),
118+
pytest.param('localhost:9999', id='localhost_port'),
119+
pytest.param('', id='empty'),
120+
],
121+
)
122+
def test_appliances_with_non_ip_address_are_skipped(dd_run_check, aggregator, mocker, instance, bad_ip):
123+
inst = instance('localhost:8443', max_backfill_minutes=10)
124+
check = HpeArubaEdgeconnectCheck('hpe_aruba_edgeconnect', {}, [inst])
125+
126+
payload = [
127+
{**APPLIANCE_PAYLOAD[0], 'ip': '10.0.0.1'},
128+
{**APPLIANCE_PAYLOAD[0], 'ip': bad_ip, 'hostName': 'invalid-appliance'},
129+
]
130+
_setup_mocks(mocker, check, payload, appliance_client=_mock_appliance_client(TGZ_DATA))
131+
132+
dd_run_check(check)
133+
134+
monitored_ips = [
135+
tag.split(':', 1)[1]
136+
for metric in aggregator.metrics(f'{NS}.device.reachability')
137+
for tag in metric.tags
138+
if tag.startswith('device_ip:')
139+
]
140+
assert monitored_ips == ['10.0.0.1']
141+
142+
113143
@pytest.mark.parametrize(
114144
'value, expected',
115145
[

0 commit comments

Comments
 (0)