fix: address PR review feedback on adguard_export plugin

- config.json: add show:true to all visible column definitions so they
  render in the plugin output table
- script.py: fix managed_names adoption bug — update/skip branches no
  longer add unowned clients to managed state; rename tracking now
  scoped to plugin-created clients only
- README.md: fix ADGUARDEXP_URL default (localhost:3000, not local IP),
  add language tags to fenced code blocks, normalise metadata block to
  Other info / Maintainer / DD-Mon-YYYY format
- test_adguard_export.py: add regression test for manual client matched
  by ID not being adopted into managed state

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
Nathan Jacobson
2026-05-23 14:22:58 -04:00
parent 197e3a3cb6
commit ca7a699ce3
4 changed files with 35 additions and 10 deletions

View File

@@ -43,7 +43,7 @@ Device types set in NetAlertX (e.g. `Smartphone`, `Laptop`, `NAS`) are automatic
|---|---|---|
| `ADGUARDEXP_RUN` | `disabled` | When to run: `disabled`, `once`, or `schedule` |
| `ADGUARDEXP_RUN_SCHD` | `0 * * * *` | Cron schedule (default: hourly) |
| `ADGUARDEXP_URL` | `http://192.168.11.1:3000` | Base URL of AdGuard Home web UI |
| `ADGUARDEXP_URL` | `http://localhost:3000` | Base URL of AdGuard Home web UI |
| `ADGUARDEXP_USER` | `admin` | AdGuard Home username |
| `ADGUARDEXP_PASSWORD` | *(empty)* | AdGuard Home password |
| `ADGUARDEXP_VERIFYSSL` | `true` | Verify TLS cert; set `false` for self-signed certs |
@@ -93,7 +93,7 @@ Devices with an unrecognised or empty type are exported without a tag.
When `ADGUARDEXP_DELETE=true`, the plugin only removes clients it previously created — it will never delete clients you added manually in AdGuard Home. Ownership is tracked in a local state file at:
```
```text
/app/db/state.ADGUARDEXP.json
```
@@ -103,13 +103,13 @@ When `ADGUARDEXP_DELETE=true`, the plugin only removes clients it previously cre
Plugin logs are written to:
```
```text
/tmp/log/plugins/script.ADGUARDEXP.log
```
Result rows (used by the NetAlertX UI) are written to:
```
```text
/tmp/log/plugins/last_result.ADGUARDEXP.log
```
@@ -133,8 +133,8 @@ Result rows (used by the NetAlertX UI) are written to:
---
## Notes
### Other info
- Version: 1.0.0
- Author: [natecj](https://github.com/natecj)
- Release Date: 2026-05-10
- Maintainer: [natecj](https://github.com/natecj)
- Release Date: 10-May-2026

View File

@@ -340,6 +340,7 @@
{
"column": "Object_PrimaryID",
"mapped_to_column": "cur_MAC",
"show": true,
"type": {
"dataType": "string",
"elements": [
@@ -366,6 +367,7 @@
{
"column": "Object_SecondaryID",
"mapped_to_column": "cur_IP",
"show": true,
"type": {
"dataType": "string",
"elements": [
@@ -387,6 +389,7 @@
{
"column": "Watched_Value1",
"mapped_to_column": "cur_Name",
"show": true,
"type": {
"dataType": "string",
"elements": [
@@ -407,6 +410,7 @@
},
{
"column": "Watched_Value2",
"show": true,
"type": {
"dataType": "string",
"elements": [
@@ -427,6 +431,7 @@
},
{
"column": "Watched_Value3",
"show": true,
"type": {
"dataType": "string",
"elements": [
@@ -447,6 +452,7 @@
},
{
"column": "Watched_Value4",
"show": true,
"type": {
"dataType": "string",
"elements": [
@@ -486,6 +492,7 @@
},
{
"column": "Extra",
"show": true,
"type": {
"dataType": "string",
"elements": [
@@ -512,6 +519,7 @@
{
"column": "ForeignKey",
"mapped_to_column": "dev_MAC",
"show": true,
"type": {
"dataType": "string",
"elements": [

View File

@@ -330,15 +330,18 @@ def sync_to_adguard(
try:
agrd.update_client(old_name, merged_data)
mylog("verbose", [f"[{pluginName}] UPDATE {old_name!r}{device['name']!r} ids={client_data['ids']}"])
managed_names.discard(old_name)
managed_names.add(device["name"])
# Only track the rename for clients we already own — never adopt a manually-created client.
if old_name in managed_names:
managed_names.discard(old_name)
managed_names.add(device["name"])
updated += 1
except requests.HTTPError as exc:
mylog("verbose", [f"[{pluginName}] ERROR updating {device['name']!r}: {exc}"])
skipped += 1
else:
mylog("verbose", [f"[{pluginName}] SKIP (no change) {device['name']!r}"])
managed_names.add(device["name"])
# No managed_names update: if we created this client it's already in the state
# file; if it's a manually-created client we must not claim ownership of it.
skipped += 1
else:
try:

View File

@@ -346,6 +346,20 @@ class TestSyncToAdguard:
sync_to_adguard(agrd, [], use_mac=True, delete_missing=True)
agrd.delete_client.assert_not_called()
def test_manual_client_matched_by_id_not_adopted(self, tmp_path):
# A manually-created AdGuard client whose IP matches a NetAlertX device
# must not be added to managed_names — so DELETE=true won't touch it later.
state = tmp_path / "state.json"
state.write_text(json.dumps({"managed": []}))
existing = [{"name": "Manual Client", "ids": ["10.0.0.5"], "tags": []}]
agrd = _mock_agrd(existing=existing)
device = {"mac": "", "name": "Manual Client", "last_ip": "10.0.0.5", "dev_type": ""}
with patch("script.STATE_FILE", str(state)):
sync_to_adguard(agrd, [device], use_mac=True, delete_missing=True)
loaded = load_managed_names()
assert "Manual Client" not in loaded
agrd.delete_client.assert_not_called()
def test_device_with_no_usable_id_is_skipped(self, tmp_path):
agrd = _mock_agrd(existing=[])
device = {"mac": "00:00:00:00:00:00", "name": "Ghost", "last_ip": "0.0.0.0", "dev_type": ""}