Skip to content

First attempt at kea dhcp support#1636

Open
void-spark wants to merge 7 commits into
netalertx:mainfrom
void-spark:main
Open

First attempt at kea dhcp support#1636
void-spark wants to merge 7 commits into
netalertx:mainfrom
void-spark:main

Conversation

@void-spark
Copy link
Copy Markdown

@void-spark void-spark commented May 10, 2026

This is code that's working for me currently.
README.md included with very basic instructions.

Tested on debian13 with kea 2.6.3 (current version in debian stable repos).

Summary by CodeRabbit

  • New Features

    • Added Kea DHCP API device-import plugin: scheduleable runs, basic-auth support, configurable command/timeout, field mappings to scans, status HTML indicators, hidden comment field, and notification controls.
    • Plugin now reports when no entries are found.
  • Documentation

    • Expanded setup and usage guide with examples, notes (test/setup and password behavior), required settings, metadata, and configuration guidance.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 10, 2026

Important

Review skipped

This PR was authored by the user configured for CodeRabbit reviews. CodeRabbit does not review PRs authored by this user. It's recommended to use a dedicated user account to post CodeRabbit review feedback.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 7105e142-0f3b-4161-9c32-b2fc75e60db2

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

This PR adds a Kea DHCP API device-scanner plugin: plugin config/schema, a Python importer that fetches and imports IPv4 leases from Kea via its control-agent API, and README documentation for setup and required settings.

Changes

Kea API Plugin Feature

Layer / File(s) Summary
Plugin Configuration & Contract
front/plugins/kea_api/config.json
Plugin metadata, execution order, data source specification, database column definitions for MAC/IP/state/hostname fields, and settings for API URL, credentials, scheduling, timeout, overwrite policies, and notification configuration.
Script Entrypoint & Globals
front/plugins/kea_api/script.py
Module header, sys.path adjustments, pluginName, log/result path constants, and Plugin_Objects(RESULT_FILE) initialization.
Lease Importer Implementation
front/plugins/kea_api/script.py
main() reads KEA API settings, issues authenticated POST for lease4-get-all (dhcp4), parses JSON, validates MACs, maps leases into plugin_objects (primaryId, secondaryId, watched fields, foreignKey), writes result file, and logs per-result statuses.
Exception Handling & Entrypoint Guard
front/plugins/kea_api/script.py
Top-level exception logging via mylog and if __name__ == '__main__': main() guard.
User Documentation & Setup
front/plugins/kea_api/README.md, docs/PLUGINS.md
Overview, example Kea control-socket/hooks and kea-ctrl-agent basic-auth example, required settings KEALSS_URL, KEALSS_USER, KEALSS_PASS, added Notes, and registry entry for the KEALSS plugin.
sequenceDiagram
  participant Importer as Kea Importer
  participant KeaAPI as Kea Control Agent API
  participant Validator as is_mac
  participant PluginObjects as Plugin_Objects
  Importer->>Importer: Read KEALSS_URL/USER/PASS and timeout
  Importer->>KeaAPI: POST {"command":"lease4-get-all","service":"dhcp4"} with BasicAuth
  KeaAPI-->>Importer: JSON response (result, leases, text)
  alt result == 0
    loop leases
      Importer->>Validator: validate hw-address
      Validator-->>Importer: valid/invalid
      Importer->>PluginObjects: add_object(primaryId=MAC, secondaryId=IP, watched fields, foreignKey=MAC)
    end
    Importer->>PluginObjects: write_result_file(RESULT_FILE)
  else result == 1
    Importer->>Importer: log API error
  else result == 3
    Importer->>Importer: log no entries found
  end
  Importer->>Importer: log completion/count/text
Loading

"A rabbit hops with keyboard bright,
I fetch the leases through the night,
MACs and IPs in careful rows,
Hostnames too — the inventory grows,
NetAlertX wakes, all devices in sight."

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'First attempt at kea dhcp support' accurately reflects the primary objective of the PR, which adds initial Kea DHCP API support with plugin implementation, documentation, and configuration.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (4)
front/plugins/kea_api/README.md (1)

70-71: 💤 Low value

Consider adding a troubleshooting section.

The documentation could benefit from a brief troubleshooting section covering common issues such as connection failures, authentication errors, or timeout problems.

📚 Example section to add
### Troubleshooting

**Connection errors:**
- Verify the Kea Control Agent is running and accessible at the configured URL
- Check firewall rules if connecting to a remote host
- Confirm the control socket path matches between `kea-dhcp4` and `kea-ctrl-agent` configurations

**Authentication failures:**
- Ensure the username and password match the values in the password file
- Verify the password file path is correct in the ctrl-agent configuration

**Timeout issues:**
- If imports are failing, increase the `RUN_TIMEOUT` setting in the plugin configuration
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@front/plugins/kea_api/README.md` around lines 70 - 71, Add a
"Troubleshooting" section to front/plugins/kea_api/README.md that covers common
connection, authentication, and timeout issues: explain verifying the Kea
Control Agent is running and reachable at the configured URL, checking firewall
rules and control socket path consistency between kea-dhcp4 and kea-ctrl-agent,
validating username/password and the kea-api-password file path used by the
ctrl-agent, and advise increasing the plugin RUN_TIMEOUT setting for
import/timeouts; keep the section brief with bullet-style checks and concrete
configuration symbols (kea-dhcp4, kea-ctrl-agent, kea-api-password, RUN_TIMEOUT)
for easy discovery.
front/plugins/kea_api/script.py (3)

40-40: ⚡ Quick win

Rename the loop variable for clarity.

The variable l is ambiguous. Use a descriptive name like lease to improve readability.

♻️ Proposed fix
-                for l in leases:
-                    mac = l['hw-address']                    
-                    state = l['state']
+                for lease in leases:
+                    mac = lease['hw-address']                    
+                    state = lease['state']
                     if is_mac(mac):
                         plugin_objects.add_object(
                             primaryId   = mac,
-                            secondaryId = l['ip-address'],
+                            secondaryId = lease['ip-address'],
                             # Active or not, similar to watched 1 of DHCPLSS plugin
                             watched1    = state == 0, 
-                            watched2    = l['hostname'],
+                            watched2    = lease['hostname'],
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@front/plugins/kea_api/script.py` at line 40, The loop variable `l` in the
iteration `for l in leases:` is ambiguous; rename it to a descriptive name like
`lease` throughout that loop (change `l` references inside the loop body to
`lease`) to improve readability and maintainability; ensure any inner
references, function calls, or assignments that used `l` are updated to `lease`
so behavior remains unchanged.

23-25: ⚡ Quick win

Consider validating the configuration values.

The script retrieves URL, USER, and PASS but does not validate them. Empty or malformed values could cause runtime errors.

🛡️ Proposed validation
         url = get_setting_value(f'{pluginName}_URL')
         user = get_setting_value(f'{pluginName}_USER')
         password = get_setting_value(f'{pluginName}_PASS')
+        
+        if not url or not user or not password:
+            mylog('none', [f'[{pluginName}] ⚠ ERROR: Missing required configuration (URL, USER, or PASS)'])
+            return
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@front/plugins/kea_api/script.py` around lines 23 - 25, The code fetches
plugin config values (url, user, password) via get_setting_value but does not
validate them; add validation after those calls to ensure url, user, and
password are non-empty and that url is a well-formed URL (e.g., parse with a URL
parser and check scheme/netloc) and if invalid, log a clear error (including
pluginName) and raise/exit so the caller is not given malformed config; update
the block around get_setting_value(...) to perform these checks and fail-fast
with descriptive messages.

57-58: ⚡ Quick win

Include error details from the Kea API response.

When result == 1, the Kea API response may include additional error information in the text or arguments fields. Logging these details would help with troubleshooting.

📝 Proposed enhancement
             elif entry.get("result") == 1:
-                mylog('none', [f'[{pluginName}] ⚠ ERROR: Kea API indicated error'])
+                error_text = entry.get("text", "unknown error")
+                mylog('none', [f'[{pluginName}] ⚠ ERROR: Kea API indicated error: {error_text}'])
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@front/plugins/kea_api/script.py` around lines 57 - 58, In the branch checking
elif entry.get("result") == 1, update the mylog call so it includes the Kea API
error details from the response (e.g., entry.get("text") and
entry.get("arguments")) alongside pluginName; locate the conditional on
entry.get("result") in script.py and change the log invocation in that block
(where mylog is called) to concatenate or format a message that includes the
text and arguments values when present so the log shows both the original
warning and the API-provided error details.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@front/plugins/kea_api/config.json`:
- Line 309: The RUN_TIMEOUT default in front/plugins/kea_api/config.json is too
low (currently "default_value": 5) and will kill the plugin before its 10s HTTP
request finishes; update the default_value for RUN_TIMEOUT to a larger value
(e.g., 15) so it comfortably exceeds the 10-second HTTP timeout plus processing
overhead, ensuring the plugin's HTTP requests can complete without premature
termination.
- Line 332: In the plugin config JSON, update the elementOptions entries so the
key reads "ordeable" instead of "orderable" to match frontend compatibility;
locate each elementOptions object that currently contains { "multiple": "true",
"orderable": "true" } and change the "orderable" key to "ordeable" for all
occurrences (including the instance shown in the diff and the other similar
elementOptions blocks present elsewhere in the file).

In `@front/plugins/kea_api/script.py`:
- Line 31: The requests.post call currently uses a hardcoded timeout of 10
seconds which exceeds the default RUN_TIMEOUT in config.json; change the timeout
to respect the configured run timeout (e.g., read RUN_TIMEOUT from configuration
and pass a timeout value safely below it, or set a smaller fixed timeout such as
3–4 seconds) so the HTTP request in the line invoking requests.post (the
assignment to response) completes within the process run budget; ensure you
reference the RUN_TIMEOUT config key when computing the timeout and adjust any
callers that assume a 10s timeout.

---

Nitpick comments:
In `@front/plugins/kea_api/README.md`:
- Around line 70-71: Add a "Troubleshooting" section to
front/plugins/kea_api/README.md that covers common connection, authentication,
and timeout issues: explain verifying the Kea Control Agent is running and
reachable at the configured URL, checking firewall rules and control socket path
consistency between kea-dhcp4 and kea-ctrl-agent, validating username/password
and the kea-api-password file path used by the ctrl-agent, and advise increasing
the plugin RUN_TIMEOUT setting for import/timeouts; keep the section brief with
bullet-style checks and concrete configuration symbols (kea-dhcp4,
kea-ctrl-agent, kea-api-password, RUN_TIMEOUT) for easy discovery.

In `@front/plugins/kea_api/script.py`:
- Line 40: The loop variable `l` in the iteration `for l in leases:` is
ambiguous; rename it to a descriptive name like `lease` throughout that loop
(change `l` references inside the loop body to `lease`) to improve readability
and maintainability; ensure any inner references, function calls, or assignments
that used `l` are updated to `lease` so behavior remains unchanged.
- Around line 23-25: The code fetches plugin config values (url, user, password)
via get_setting_value but does not validate them; add validation after those
calls to ensure url, user, and password are non-empty and that url is a
well-formed URL (e.g., parse with a URL parser and check scheme/netloc) and if
invalid, log a clear error (including pluginName) and raise/exit so the caller
is not given malformed config; update the block around get_setting_value(...) to
perform these checks and fail-fast with descriptive messages.
- Around line 57-58: In the branch checking elif entry.get("result") == 1,
update the mylog call so it includes the Kea API error details from the response
(e.g., entry.get("text") and entry.get("arguments")) alongside pluginName;
locate the conditional on entry.get("result") in script.py and change the log
invocation in that block (where mylog is called) to concatenate or format a
message that includes the text and arguments values when present so the log
shows both the original warning and the API-provided error details.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 4fcbc007-3562-4014-ac25-4a0429e3b765

📥 Commits

Reviewing files that changed from the base of the PR and between 781ae2f and b290d3c.

📒 Files selected for processing (3)
  • front/plugins/kea_api/README.md
  • front/plugins/kea_api/config.json
  • front/plugins/kea_api/script.py

Comment thread front/plugins/kea_api/config.json Outdated
Comment thread front/plugins/kea_api/config.json
Comment thread front/plugins/kea_api/script.py Outdated
@jokob-sk
Copy link
Copy Markdown
Collaborator

Thanks a lot! Can you also add a line in the plugins table? https://github.com/netalertx/NetAlertX/blob/main/docs/PLUGINS.md#available-plugins

Copy link
Copy Markdown
Collaborator

@jokob-sk jokob-sk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looking good, some small improvements and we are good to merge I think

Comment thread front/plugins/kea_api/README.md
Comment thread front/plugins/kea_api/script.py Outdated
Comment thread front/plugins/kea_api/script.py Outdated
Comment thread front/plugins/kea_api/README.md
Use 'l' instead of 'lease' (I see you little rabbit)
Use [] over get in most places, if expected fields are missing an error is an acceptable outcome.
Include 'text' field in logging.
Add logging for result '3'
match request timeout to script timeout -1 second.
Always use at least 1 sec. timeout for request.
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (1)
front/plugins/kea_api/script.py (1)

59-59: ⚡ Quick win

Consider moving write_result_file() outside the loop.

Currently, write_result_file() is called inside the for entry in data: loop, which means the result file is written once for each entry with result == 0. While Kea typically returns a single entry for lease4-get-all, if multiple entries are present, this results in redundant I/O. Moving the call outside the loop would be more efficient.

♻️ Proposed refactor
                         count += 1
-                plugin_objects.write_result_file()

                 mylog('verbose', [f'[{pluginName}] Kea API response: {text}'])
-                mylog('verbose', [f'[{pluginName}] Successfully imported {count} devices reported by Kea API'])
             elif entry['result'] == 1:
                 mylog('none', [f'[{pluginName}] ⚠ ERROR: Kea API indicated error: {text}'])
             elif entry['result'] == 3:
                 mylog('verbose', [f'[{pluginName}] Kea API indicates no entries found: {text}'])
-
+        
+        # Write results once after processing all entries
+        if count > 0:
+            plugin_objects.write_result_file()
+            mylog('verbose', [f'[{pluginName}] Successfully imported {count} devices reported by Kea API'])

     except Exception as e:
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@front/plugins/kea_api/script.py` at line 59, The loop iterates over "for
entry in data:" and calls plugin_objects.write_result_file() inside the branch
that checks entry["result"] == 0, causing repeated writes; move the single call
to plugin_objects.write_result_file() out of the loop so the file is written
once after processing all entries (keep the existing per-entry processing and
any accumulation of results, then invoke write_result_file() once after the loop
completes).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@front/plugins/kea_api/script.py`:
- Line 38: Remove the unnecessary semicolon at the end of the assignment to the
variable `text` (the statement using `entry.get('text', '[API provided no
text]')`) in front/plugins/kea_api/script.py so the line reads as a normal
Python assignment without a trailing semicolon; locate the assignment to `text`
and delete the semicolon character only.
- Around line 40-41: The code directly indexes entry['result'] and
entry['arguments']['leases'] which can KeyError on malformed API responses;
before accessing, validate the top-level keys exist (e.g. check 'result' in
entry and that 'arguments' is a dict containing 'leases') and only then perform
the comparison and assign leases; on missing/invalid structure, handle
gracefully (skip this entry and log/debug a warning) so the loop using entry
remains robust.
- Around line 23-32: The code reads url, user, password, and timeout without
validation before calling requests.post; add checks after retrieving settings
(url, user, password, timeout) to ensure url/user/password are non-empty strings
and to coerce/validate timeout to a numeric value (int or float) with a safe
default (e.g., DEFAULT_TIMEOUT) if None or invalid; on invalid required settings
call mylog('error', ...) and abort/return before the requests.post call, and for
timeout use a validated numeric value (ensure >=2 or clamp) when computing
timeout=max(1, timeout-1) so the arithmetic cannot raise a TypeError. Ensure the
changes are applied around the variables named url, user, password, timeout and
before the requests.post(...) invocation.

---

Nitpick comments:
In `@front/plugins/kea_api/script.py`:
- Line 59: The loop iterates over "for entry in data:" and calls
plugin_objects.write_result_file() inside the branch that checks entry["result"]
== 0, causing repeated writes; move the single call to
plugin_objects.write_result_file() out of the loop so the file is written once
after processing all entries (keep the existing per-entry processing and any
accumulation of results, then invoke write_result_file() once after the loop
completes).
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: f8e5fe09-60a1-4bc0-9519-40ed1c9372e4

📥 Commits

Reviewing files that changed from the base of the PR and between b290d3c and 76612e5.

📒 Files selected for processing (3)
  • front/plugins/kea_api/README.md
  • front/plugins/kea_api/config.json
  • front/plugins/kea_api/script.py
✅ Files skipped from review due to trivial changes (1)
  • front/plugins/kea_api/README.md
🚧 Files skipped from review as they are similar to previous changes (1)
  • front/plugins/kea_api/config.json

Comment thread front/plugins/kea_api/script.py
Comment thread front/plugins/kea_api/script.py Outdated
Comment thread front/plugins/kea_api/script.py
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@front/plugins/kea_api/script.py`:
- Around line 59-62: The plugin currently calls
plugin_objects.write_result_file() and logs success via mylog('verbose', [...])
inside the for entry in data: loop causing repeated writes/logs; move the
plugin_objects.write_result_file() call and the success mylog lines (the two
mylog calls that reference Kea API response and "Successfully imported {count}
devices") out of the loop so they execute once after the loop completes,
ensuring the result file is written once and the final success log reflects the
total processed count.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: f6ec11ac-fc4b-4d4e-bde9-cd17ba0dade9

📥 Commits

Reviewing files that changed from the base of the PR and between 39068e9 and 6d661dd.

📒 Files selected for processing (1)
  • front/plugins/kea_api/script.py

Comment thread front/plugins/kea_api/script.py
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (2)
front/plugins/kea_api/script.py (2)

23-32: 💤 Low value

Consider logging a warning when using HTTP instead of HTTPS.

Basic authentication sends credentials base64-encoded in headers, which are transmitted in plaintext over HTTP. While some deployments may use HTTP in trusted networks, logging a warning when the URL scheme is http:// would alert users to this security risk.

🔒 Optional: Add HTTPS check
     url = get_setting_value(f'{pluginName}_URL')
     user = get_setting_value(f'{pluginName}_USER')
     password = get_setting_value(f'{pluginName}_PASS')
     timeout = get_setting_value(f'{pluginName}_RUN_TIMEOUT')
+    
+    if url and not url.startswith('https://'):
+        mylog('none', [f'[{pluginName}] ⚠ WARNING: Using HTTP instead of HTTPS may expose credentials'])

     mylog('verbose', [f'[{pluginName}] Querying Kea API at {url}'])
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@front/plugins/kea_api/script.py` around lines 23 - 32, The code calls
requests.post with basic auth using the variable url (constructed from
get_setting_value with f'{pluginName}_URL') but does not warn when the URL uses
plain HTTP; add a check of the url's scheme (e.g., inspect
url.startswith('http://') or parse with urllib.parse.urlparse) before making the
request and call mylog('warning', [...]) with a clear message referencing
pluginName and url if the scheme is "http" to alert about plaintext credential
transmission; keep the existing request flow (requests.post(..., auth=(user,
password), ...)) but ensure the warning is logged prior to the request.

45-58: 💤 Low value

Optionally log when invalid MAC addresses are skipped.

The code silently skips leases with invalid MAC addresses. Adding a verbose log message would help operators understand if any leases are being filtered out during import.

📝 Optional: Log skipped MACs
                 for lease in leases:
                     mac = lease['hw-address']                    
                     state = lease['state']
                     if is_mac(mac):
                         plugin_objects.add_object(
                             primaryId   = mac,
                             secondaryId = lease['ip-address'],
                             # Active or not, similar to watched1 of DHCPLSS plugin
                             watched1    = state == 0, 
                             watched2    = lease['hostname'],
                             watched3    = None,
                             # Default (or assigned) (0), declined (1), expired-reclaimed (2), released (3), and registered (4)).
                             watched4    = state,
                             extra       = None,
                             foreignKey  = mac
                         )
                         count += 1
+                    else:
+                        mylog('verbose', [f'[{pluginName}] Skipping lease with invalid MAC: {mac}'])
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@front/plugins/kea_api/script.py` around lines 45 - 58, The loop that calls
is_mac(mac) currently silently skips invalid MACs; add an else branch next to
the is_mac(mac) check to log skipped leases (include mac value,
lease['ip-address'] and lease.get('hostname')) so operators can see which MACs
were filtered out; use the module or plugin logger (e.g., an existing plugin
logger or logging.getLogger(__name__)) and emit a debug/info log before
continuing, leaving plugin_objects.add_object, count increment, and the rest
unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@front/plugins/kea_api/script.py`:
- Line 51: The code accesses lease['hostname'] directly (seen in the assignment
to watched2) which can raise KeyError because Kea's lease 'hostname' is
optional; change the access to use a safe lookup like lease.get('hostname', '')
where watched2 is assigned (the lease processing block that sets watched2 =
lease['hostname']) so missing hostnames are handled gracefully without crashing
the import.

---

Nitpick comments:
In `@front/plugins/kea_api/script.py`:
- Around line 23-32: The code calls requests.post with basic auth using the
variable url (constructed from get_setting_value with f'{pluginName}_URL') but
does not warn when the URL uses plain HTTP; add a check of the url's scheme
(e.g., inspect url.startswith('http://') or parse with urllib.parse.urlparse)
before making the request and call mylog('warning', [...]) with a clear message
referencing pluginName and url if the scheme is "http" to alert about plaintext
credential transmission; keep the existing request flow (requests.post(...,
auth=(user, password), ...)) but ensure the warning is logged prior to the
request.
- Around line 45-58: The loop that calls is_mac(mac) currently silently skips
invalid MACs; add an else branch next to the is_mac(mac) check to log skipped
leases (include mac value, lease['ip-address'] and lease.get('hostname')) so
operators can see which MACs were filtered out; use the module or plugin logger
(e.g., an existing plugin logger or logging.getLogger(__name__)) and emit a
debug/info log before continuing, leaving plugin_objects.add_object, count
increment, and the rest unchanged.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 9128ee6b-f410-435a-9cef-d0bb806b4630

📥 Commits

Reviewing files that changed from the base of the PR and between 39068e9 and 6d661dd.

📒 Files selected for processing (1)
  • front/plugins/kea_api/script.py

secondaryId = lease['ip-address'],
# Active or not, similar to watched1 of DHCPLSS plugin
watched1 = state == 0,
watched2 = lease['hostname'],
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

Is the hostname field always present in Kea DHCP lease4-get-all API responses, or can it be missing?

💡 Result:

The hostname field can be missing in Kea DHCP lease4-get-all API responses. The Lease4 structure includes hostname_ as std::string, with constructors defaulting it to an empty string ("")[1][2]. Database schemas define hostname as VARCHAR(255), allowing NULL or empty values[3][4]. Memfile CSV format supports empty hostname fields[3]. API responses mirror lease data, including hostname only if set; otherwise, it is absent or empty[2]. Commands like lease4-add accept optional hostname[5][6]. No sources indicate hostname is always present; it is populated from client DHCP requests or reservations but absent if unspecified[1].

Citations:


Use defensive access for the optional 'hostname' field in Kea lease responses.

The code directly accesses lease['hostname'] at line 51, which will raise KeyError if the field is missing. According to the Kea API specification, the hostname field in lease4-get-all responses is optional and can be absent when not specified in client DHCP requests or reservations. This will cause the entire import to fail when processing a lease without a hostname. Use lease.get('hostname', '') instead to handle missing hostnames gracefully.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@front/plugins/kea_api/script.py` at line 51, The code accesses
lease['hostname'] directly (seen in the assignment to watched2) which can raise
KeyError because Kea's lease 'hostname' is optional; change the access to use a
safe lookup like lease.get('hostname', '') where watched2 is assigned (the lease
processing block that sets watched2 = lease['hostname']) so missing hostnames
are handled gracefully without crashing the import.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants