feat: devcontainer proxy support, auto-create librenms_id …

[marcinpsk]

…custom field

• Add proxy and CA certificate support to devcontainer setup
• Add GitHub CLI installation to setup.sh
• Add git safe.directory config for devcontainer
• Add .env.example and docker-compose.yml changes for proxy
• Update devcontainer.json with proxy compatible settings
• Add README.md for devcontainer usage documentation
• Auto-create librenms_id custom field via post_migrate signal
• Add multi-server config support to librenms_api.py
• Add tests for _ensure_librenms_id_custom_field
• Update custom_field.md documentation

Summary by CodeRabbit

• New Features

  • Automatic librenms_id custom field creation during plugin setup
  • Proxy configuration support for corporate MITM proxies in development environments
  • Server selection fallback for multi-server configurations
  • New quick-start command (netbox-restart) and dev-help alias in the dev container

• Bug Fixes

  • Corrected a typo in example plugin configuration

• Documentation

  • Added comprehensive proxy setup guide and updated custom field docs

• Tests

  • Added unit tests for the custom field creation flow

• Chores

  • Increased dev container storage, enhanced dev tooling and CI/test configs, packaging exclusions and .gitignore updates

[coderabbitai]

Warning

Rate limit exceeded

@marcinpsk has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 12 minutes and 57 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

Walkthrough

Adds automatic librenms_id custom-field creation on post-migrate, extensive devcontainer proxy/CA configuration and docs, enhanced devcontainer setup scripts (plugin discovery, CA installation, Git/GH CLI, pre-commit), a server-key fallback in librenms_api, new testing configuration and unit tests for the migration handler, CI/workflow updates, and assorted example/config fixes.

Changes

Cohort / File(s)	Summary
Devcontainer env examples & README .devcontainer/.env.example, .devcontainer/README.md	Added commented proxy/CA env examples and detailed proxy/CA setup guidance in README; explains dual-level proxy setup, CA handling, rebuild steps and common issues.
Devcontainer example config .devcontainer/config/plugin-config.py.example	Fixed production Librenms URL typo and added interface_name_field: "ifDescr" to the production example config.
Devcontainer host/container config .devcontainer/devcontainer.json, .devcontainer/docker-compose.yml	Moved/added proxy and CA-related env vars into container envs, increased host storage to 48gb, adjusted features block; ensure env propagation and storage sizing.
Devcontainer scripts & aliases .devcontainer/scripts/setup.sh, .devcontainer/scripts/load-aliases.sh, .devcontainer/scripts/welcome.sh	Added detect_plugin_workspace, apt/pip proxy wiring, CA bundle installation and trust updates, Git/GH installation (non-fatal), pre-commit/git setup, plugin dev install, remote URL normalization; added dev-help alias and netbox-restart hint. High-density script logic—review idempotence and error handling.
Plugin: post-migrate auto-init netbox_librenms_plugin/__init__.py	Adds _ensure_librenms_id_custom_field registered on post_migrate to create/associate librenms_id CustomField for Device, VirtualMachine, Interface, VMInterface with logging and exception safety.
API: server-key fallback netbox_librenms_plugin/librenms_api.py	When multi-server configured and requested server_key missing: raise for non-default keys; if default, fall back to first configured server key and log info. self.server_key assignment moved after validation.
Unit tests for migration handler netbox_librenms_plugin/tests/test_init.py	Adds extensive tests for the post_migrate handler covering creation, idempotence, partial associations, exception safety, and logging.
Packaging exclusion pyproject.toml	Excluded netbox_librenms_plugin.tests* from package discovery to avoid shipping tests.
Testing configuration media/configuration.testing.py	New Python testing config defining DB, Redis, PLUGINS, PLUGINS_CONFIG, SECRET_KEY, and API_TOKEN_PEPPERS intended for CI/testing only.
CI workflows .github/workflows/lint-format.yaml, .github/workflows/test.yaml	Updated actions versions, bumped Python to 3.12, removed branch filters and continue-on-error for Ruff, renamed/adjusted test workflow steps and paths; verify CI behavior and test paths.
Docs: custom field guidance docs/usage_tips/custom_field.md	Documented automatic creation of librenms_id (v0.4.2+) and moved manual setup to a legacy section.
Misc ignores .gitignore	Added ignores for ca-bundle.crt, *.pem, and .github/hooks/.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

🐇 I nudged certs, anchors, and fields that bloom,
I hopped through scripts to clear the dev-room gloom,
Containers sip trust from my little crate,
Tests hum, CI wakes — configurations celebrate,
A rabbit cheers the code's new tune. 🥕✨

🚥 Pre-merge checks | ✅ 3 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Merge Conflict Detection	⚠️ Warning	⚠️ Unable to check for merge conflicts: Failed to fetch base branch: fatal: cannot change to '/home/jailuser/git': No such file or directory

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the two main changes: devcontainer proxy support and auto-creation of the librenms_id custom field, matching the substantial work across multiple files.
Docstring Coverage	✅ Passed	Docstring coverage is 92.86% which is sufficient. The required threshold is 80.00%.

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

✨ Finishing touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch devcontainer-codespaces

Warning

Review ran into problems

🔥 Problems

Git: Failed to clone repository. Please run the @coderabbitai full review command to re-trigger a full review. If the issue persists, set path_filters to include or exclude specific files.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 13

🤖 Fix all issues with AI agents

In @.devcontainer/docker-compose.yml:
- Around line 49-54: Remove the unnecessary proxy environment variables from the
Postgres and Redis service environment blocks: delete the keys HTTP_PROXY,
HTTPS_PROXY, http_proxy, https_proxy, NO_PROXY and no_proxy from the postgres
and redis service definitions (also remove the duplicate set in the second
occurrences around the other service blocks). This keeps the docker-compose
service configs minimal since these local services do not perform outbound
HTTP(S) requests.

In @.devcontainer/README.md:
- Around line 190-202: Add a single blank line immediately before the fenced
code block that starts with ```bash containing the proxy/CA environment
variables so the markdown has a preceding empty line (fixing MD031); locate the
fenced block with the HTTP_PROXY/HTTPS_PROXY/NO_PROXY and CA certificate
comments and insert one blank line above it.
- Line 221: Update the line "If your proxy requires authentication, include it
in the URL: `http://username:password@proxy.example.com:8080`" to add a security
note that embedding credentials in the proxy URL is insecure because they can
appear in process listings, environment dumps, container inspect output, and
logs; instruct readers to avoid username:password URLs and instead use safer
alternatives such as Docker's config.json with credsStore, .netrc, or other
secret managers, and include a brief sentence pointing them to those
alternatives for storing proxy credentials securely.
- Around line 73-78: The fenced code block inside the numbered list item is
missing blank lines before and after, triggering MD031; update the list item so
there is an empty line before the opening ```bash and an empty line after the
closing ``` to ensure blank lines surround the fenced code block in the README's
numbered step (the code block shown: cp
.devcontainer/config/plugin-config.py.example
.devcontainer/config/plugin-config.py).

In @.devcontainer/scripts/setup.sh:
- Around line 28-29: The early workspace detection using PLUGIN_WS_DIR_EARLY
duplicates later logic and may diverge; refactor by extracting the workspace
detection heuristic into a single function (e.g., detect_plugin_workspace()) and
replace both the PLUGIN_WS_DIR_EARLY assignment and the later detection block
(the code around the current workspace discovery and the "find /workspaces"
fallback) to call that function once; ensure detect_plugin_workspace()
implements the canonical checks (current PWD with pyproject.toml, fallback to
/workspaces search) and return the resolved directory so CA bundle lookup always
uses the same canonical path.
- Around line 73-88: The gh install block chains many commands with && so
partial failures leave artifacts; modify the block around the temporary file
variable out, the keyring path (/etc/apt/keyrings/githubcli-archive-keyring.gpg)
and the sources list (/etc/apt/sources.list.d/github-cli.list) to ensure cleanup
on error: capture exit status after the chained operations, and on non-zero exit
remove the temp file, remove any written keyring and sources list, and print the
failure message (instead of relying solely on the final ||); alternatively use a
subshell or trap to remove "$out" and delete the two target files on failure,
and consider isolating wget and apt-get install into separate steps so failures
can be handled and logged individually; keep the mkdir -p -m 755 as-is but note
SC2174 is acceptable here.
- Around line 42-47: The script currently disables Git SSL verification
automatically (git config --global http.sslVerify false) when no CA bundle is
found; instead, change .devcontainer/scripts/setup.sh to NOT flip global
http.sslVerify by default — emit a prominent warning message about the security
risk and require explicit opt-in (e.g., check an environment variable like
ALLOW_GIT_SSL_DISABLE or prompt the user) before running git config; if opt-in
is given, confine the change to the local repo or clearly log that a global
change will occur and why, and only then run git config (or preferably set it
per-repo rather than --global).

In `@netbox_librenms_plugin/__init__.py`:
- Around line 77-80: The _ensure_librenms_id_custom_field handler sets a
module-level _executed attribute that persists for the life of the process,
which can cause subsequent programmatic migrate calls in long-running processes
to be skipped; update the _ensure_librenms_id_custom_field block to include a
brief inline comment next to the _executed flag (and its True assignment)
stating the assumption that migrations run in short-lived CLI processes and
explaining why the flag is intentionally not reset (or note that long-running
processes should not rely on this behavior), referencing the symbol
_ensure_librenms_id_custom_field and its _executed attribute so future readers
know why it isn’t cleared.
- Around line 118-120: Replace the silent except block that swallows all errors
with one that captures the exception (e.g., "except Exception as e") and logs it
with the module logger using logger.exception or logging.exception so the error
and traceback are recorded while still allowing startup to continue; update the
except block that follows the custom field creation logic in __init__.py (the
current "except Exception: pass") to log the exception details and a concise
context message.

In `@netbox_librenms_plugin/librenms_api.py`:
- Around line 44-57: The current silent fallback in librenms_api.py that
replaces a missing server_key with the first configured server should be
restricted: change the logic in the block that checks servers_config and
server_key (the code that computes first_key and sets self.server_key) so that
it only falls back when the requested server_key is the auto-default (e.g.,
server_key == "default"); if a non-default server_key is provided but not found,
raise a clear exception (ValueError or KeyError) with a message identifying the
missing server_key and available keys to avoid silently using the wrong LibreNMS
instance. Ensure the exception is raised before assigning self.server_key.

In `@netbox_librenms_plugin/tests/test_init.py`:
- Around line 66-76: The test leaves _ensure_librenms_id_custom_field._executed
set to True if the assertion fails because the cleanup call
self._reset_executed_flag() is skipped; fix by moving the reset into the test
class setup so it always runs before each test: add a setup_method on the test
class that calls self._reset_executed_flag() or directly sets
_ensure_librenms_id_custom_field._executed = False, and remove the per-test
cleanup in test_skips_when_already_executed to ensure consistent test isolation.
- Around line 149-166: The test currently asserts logging.getLogger was never
called which is fragile; change it to assert the logger's info method was not
invoked instead. Keep the patch("logging.getLogger") as mock_get_logger, call
_ensure_librenms_id_custom_field(sender=None), then get the logger instance via
mock_get_logger.return_value and call logger_instance.info.assert_not_called()
(or the appropriate log method) so the test verifies no informational log was
emitted without preventing other getLogger uses; reference the test name
test_no_log_when_field_already_exists and the function
_ensure_librenms_id_custom_field to locate where to update the assertion.
- Around line 14-21: The tests currently call
TestEnsureLibreNMSIdCustomField._reset_executed_flag() manually which can leave
_ensure_librenms_id_custom_field._executed in a stale state after failures;
replace those manual resets with a robust per-test reset by adding either a
pytest autouse fixture or a setup_method in the TestEnsureLibreNMSIdCustomField
test class that sets _ensure_librenms_id_custom_field._executed = False before
each test, then remove the individual self._reset_executed_flag() calls (keeping
explicit tests that set the flag to True as-is).

[coderabbitai]

Actionable comments posted: 2

🤖 Fix all issues with AI agents

In @.devcontainer/scripts/setup.sh:
- Around line 10-28: The docstring for detect_plugin_workspace() is incorrect
and the find pipeline is fragile: update the comment to reflect that the
function returns the resolved path via stdout and returns an empty string (exit
status 0) on failure rather than exiting the script, and make the fallback `find
... | head -n1 | xargs dirname` robust by adding the xargs no-run-if-empty
option (use xargs -r or platform-appropriate equivalent) so dirname isn't
invoked with no args; modify the comment text and replace the xargs invocation
accordingly in the detect_plugin_workspace function.

In `@netbox_librenms_plugin/tests/test_init.py`:
- Around line 127-135: Update test_exception_does_not_propagate to also assert
the exception was logged: patch or mock the logger used inside
_ensure_librenms_id_custom_field (e.g., patch logging.getLogger or the specific
logger name) and after calling _ensure_librenms_id_custom_field(sender=None)
assert that logger.exception (or .error/.exception as used) was called with the
expected message or that an exception-level log was emitted; keep the existing
MockCustomField.objects.get_or_create.side_effect = Exception("DB not ready")
setup and only add the logger patch/assertion to ensure logging is exercised.

[coderabbitai]

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

.github/workflows/lint-format.yaml (1)

20-22: 🧹 Nitpick | 🔵 Trivial

Pin the ruff version for reproducible CI.

pip install ruff pulls the latest version on each run. A new ruff release with stricter defaults or new rules could unexpectedly break the build.

♻️ Suggested fix

-          pip install ruff
+          pip install ruff==0.9.7

(Replace with whatever version the team currently uses.)

🤖 Fix all issues with AI agents

In @.github/workflows/lint-format.yaml:
- Around line 3-5: The workflow currently triggers on every push because the on:
push: key has no branch filters; to restrict it, add a branches (or
branches-ignore) filter under the push trigger in
.github/workflows/lint-format.yaml (e.g., add push: branches: - main) so the
workflow only runs for intended branches; update the push trigger in the YAML to
include the desired branch list or branches-ignore entries to prevent noisy runs
on all feature branches.

In @.github/workflows/test.yaml:
- Around line 58-62: The CI installs pytest and pytest-django unpinned; either
pin them in the workflow or move test deps into a project extras group and
install that. Create a [test] extras in pyproject.toml or setup.cfg listing
pytest and pytest-django (e.g., pytest, pytest-django with chosen versions),
then update the GitHub Actions step named "Install NetBox LibreNMS Plugin" to
install the package with extras (pip install -e .[test]) and remove the separate
pip install lines; alternatively, if you prefer minimal changes, pin explicit
versions for pytest and pytest-django in the workflow's pip install commands.
- Around line 64-71: The CI is symlinking configuration.testing.py from media/,
which is misleading; move configuration.testing.py out of media/ into a clearer
directory (e.g., ci/ or tests/), update the workflow's symlink command in the
.github/workflows/test.yaml step that currently references
media/configuration.testing.py to point to the new location (keep the target
filename netbox/netbox/configuration.py), and ensure any other references (tests
or docs) are updated to the new path (configuration.testing.py) so the CI and
repo consistently use the new ci/ or tests/ location.
- Around line 51-56: The workflow checkout step currently pins the
netbox-community/netbox repo to ref: main (actions/checkout@v4 using repository
"netbox-community/netbox" with path netbox), which causes CI to track the
bleeding-edge NetBox; change the checkout ref to use specific release tags
(e.g., vX.Y.Z) or convert the checkout into a matrix strategy that tests
multiple refs (stable release tags plus one entry for main) so CI runs
deterministically against pinned NetBox versions while still retaining a main
entry to catch forward-compatibility regressions.

[marcinpsk]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

.devcontainer/scripts/setup.sh (1)

151-198: ⚠️ Potential issue | 🟠 Major

Hardcoded workspace paths will break when detect_plugin_workspace resolves a different directory.

Lines 161, 175, and 188 inject /workspaces/netbox-librenms-plugin into the NetBox configuration, but $PLUGIN_WS_DIR (resolved on line 124) may point elsewhere (e.g., /workspaces/some-other-name). The dynamically detected path should be used consistently.

Proposed fix — use $PLUGIN_WS_DIR in the injected paths

-      echo "_pc_path = '/workspaces/netbox-librenms-plugin/.devcontainer/config/plugin-config.py'";
+      echo "_pc_path = '$PLUGIN_WS_DIR/.devcontainer/config/plugin-config.py'";
...
-      echo "_xc_path = '/workspaces/netbox-librenms-plugin/.devcontainer/config/extra-configuration.py'";
+      echo "_xc_path = '$PLUGIN_WS_DIR/.devcontainer/config/extra-configuration.py'";
...
-      echo "_cs_path = '/workspaces/netbox-librenms-plugin/.devcontainer/config/codespaces-configuration.py'";
+      echo "_cs_path = '$PLUGIN_WS_DIR/.devcontainer/config/codespaces-configuration.py'";

🤖 Fix all issues with AI agents

In @.devcontainer/scripts/setup.sh:
- Around line 253-261: The appended .bashrc block can duplicate entries on
repeated runs; modify the setup logic that writes to ~/.bashrc so it first
checks for an idempotency sentinel (e.g., the existing "# Devcontainer Plugins
Loader" style comment) before appending. Detect presence of that sentinel string
and only append the lines that source
"$PLUGIN_WS_DIR/.devcontainer/scripts/load-aliases.sh" and run bash
"$PLUGIN_WS_DIR/.devcontainer/scripts/welcome.sh" when the sentinel is missing,
so rerunning setup.sh will not add duplicate entries.