Devcontainer update

[marcinpsk]

Summary by CodeRabbit

• New Features

  • Automatic creation of the librenms_id custom field starting with version 0.4.2 for Devices, Virtual Machines, and Interfaces.

• Improvements

  • Enhanced server configuration validation with intelligent fallback handling.
  • Expanded proxy and certificate configuration support in development environment.

• Documentation

  • Updated custom field setup guide distinguishing legacy and automatic configuration approaches.
  • Added comprehensive LibreNMS server and proxy configuration documentation.
  • New dev-help command provides development task reference.

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR introduces comprehensive proxy and certificate configuration support, auto-creation of the librenms_id custom field, GitHub CLI integration in the development container, and enhanced setup logic with workspace detection and CA trust handling.

Changes

Cohort / File(s)	Summary
Proxy & Certificate Configuration .devcontainer/.env.example, .devcontainer/README.md, .devcontainer/devcontainer.json, .devcontainer/docker-compose.yml	Adds proxy configuration documentation and environment variable support for HTTP/HTTPS proxies, NO_PROXY, and CA certificate bundles (REQUESTS_CA_BUNDLE, SSL_CERT_FILE, CURL_CA_BUNDLE). Includes MITM proxy setup instructions and Docker client configuration guidance.
Development Container Setup .devcontainer/scripts/setup.sh	Introduces detect_plugin_workspace() helper function, proxy activation logic, CA certificate installation from ca-bundle.crt, GitHub CLI installation, Git safe.directory configuration, and dynamic alias/script loading. Significantly expands setup complexity with environment initialization and tooling management.
Development Aliases & Welcome .devcontainer/scripts/load-aliases.sh, .devcontainer/scripts/welcome.sh	Adds new dev-help alias and updates help output; appends netbox-restart quick-start suggestion to welcome message.
Configuration Examples & Ignore .devcontainer/config/plugin-config.py.example, .gitignore	Corrects typo in LibreNMS production URL and comment wording; adds ca-bundle.crt, \*.pem, and .github/hooks/ to ignore rules.
Automatic Custom Field Creation netbox_librenms_plugin/__init__.py, netbox_librenms_plugin/tests/test_init.py	Introduces post_migrate signal handler to auto-create librenms_id CustomField if missing, assigns it to Device, VirtualMachine, Interface, and VMInterface content types. Comprehensive test suite covers creation, idempotency, partial content-type addition, and exception handling.
LibreNMS API Server Validation netbox_librenms_plugin/librenms_api.py	Adds server_key validation in API client initialization; raises KeyError for invalid keys, falls back to first server for "default" key with logging.
Documentation Updates docs/usage_tips/custom_field.md	Documents automatic librenms_id field creation in v0.4.2+; restructures manual setup section as "Legacy" with version-specific guidance.

Sequence Diagram(s)

sequenceDiagram
    participant Django as Django<br/>(post_migrate Signal)
    participant Handler as _ensure_librenms_id<br/>_custom_field
    participant DB as CustomField<br/>Model
    participant CT as ContentType<br/>Model
    participant Logger as Logger

    Django->>Handler: post_migrate signal fired
    Handler->>Handler: Check if already executed
    alt Not executed before
        Handler->>DB: get_or_create librenms_id field<br/>with defaults
        DB-->>Handler: field object
        Handler->>CT: Query Device, VirtualMachine,<br/>Interface, VMInterface types
        CT-->>Handler: ContentType instances
        Handler->>DB: Add content types to field
        Handler->>Logger: Log field creation
    else Already executed
        Handler->>Handler: Return early (idempotent)
    end

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

• feat: devcontainer proxy support, auto-create librenms_id … #1 — Modifies the same devcontainer setup, proxy configuration, signal handler, and server validation logic; directly overlapping implementation scope.

Poem

🐰 Carrots of config, both proxy and cert,
Custom fields bloom without breaking a sweat,
Workspaces dance, aliases leap,
While signals and safety guard secrets we keep! 🥕✨

🚥 Pre-merge checks | ✅ 3 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Title check	❓ Inconclusive	The title 'Devcontainer update' is vague and generic, using a non-descriptive term that does not convey meaningful information about the specific changes in the changeset.	Consider a more specific title that highlights the main changes, such as 'Add proxy configuration and auto-create librenms_id custom field' or 'Set up proxy/MITM support and auto-create custom fields in devcontainer'.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Merge Conflict Detection	✅ Passed	✅ No merge conflicts detected when merging into develop

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

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

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

---

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: 2

🤖 Fix all issues with AI agents

Before applying any fix, first verify the finding against the current code and
decide whether a code change is actually needed. If the finding is not valid or
no change is required, do not modify code for that item and briefly explain why
it was skipped.

In @.devcontainer/README.md:
- Line 224: The proxy-auth security warning about not embedding credentials in
proxy URLs (the "Proxy authentication:" sentence referencing
http://username:password@proxy.example.com:8080) is too hidden in the "Important
Notes" section; make it more prominent by adding a clear warning callout or
banner at the start of Step 2 (the step that begins with the cp
.devcontainer/.env.example instruction) and/or moving the sentence earlier in
the configuration steps, and explicitly mention safer alternatives (Docker's
config.json with credsStore and secret managers) so users see the warning before
they configure the proxy.

In @.devcontainer/scripts/setup.sh:
- Around line 107-121: The mkdir invocation using "mkdir -p -m 755
/etc/apt/keyrings" only applies the mode to the deepest directory, which can be
misleading; change it to create the directory and ensure permissions explicitly
by either running "mkdir -p /etc/apt/keyrings" followed by "chmod 755
/etc/apt/keyrings" or replace the command with "install -d -m 755
/etc/apt/keyrings" so the intended permissions are reliably set; update the
snippet where "mkdir -p -m 755 /etc/apt/keyrings" appears.

📜 Review details

Configuration used: Repository UI (base), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d665ea1 and 3bcf9e5.

📒 Files selected for processing (14)

• .devcontainer/.env.example
• .devcontainer/README.md
• .devcontainer/config/plugin-config.py.example
• .devcontainer/devcontainer.json
• .devcontainer/docker-compose.yml
• .devcontainer/scripts/load-aliases.sh
• .devcontainer/scripts/setup.sh
• .devcontainer/scripts/welcome.sh
• .github/workflows/lint-format.yaml
• .gitignore
• docs/usage_tips/custom_field.md
• netbox_librenms_plugin/__init__.py
• netbox_librenms_plugin/librenms_api.py
• netbox_librenms_plugin/tests/test_init.py

💤 Files with no reviewable changes (1)

• .github/workflows/lint-format.yaml

🧰 Additional context used

📓 Path-based instructions (1)

**/*.py

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.py: Reuse the librenms_api.py LibreNMS client instead of making new requests calls; it handles multi-server configs via LibreNMSSettings model and the servers plugin config, plus caching
Always call LibreNMSAPI.get_librenms_id to retrieve the device/VM LibreNMS mapping via the librenms_id custom field instead of touching the field directly
Matching must be exact-only for site, platform, device type, and role; do not add fuzzy matching; use the functions find_matching_site, match_librenms_hardware_to_device_type, and find_matching_platform from utils.py

Files:

• netbox_librenms_plugin/tests/test_init.py
• netbox_librenms_plugin/__init__.py
• netbox_librenms_plugin/librenms_api.py

🧠 Learnings (8)

📚 Learning: 2026-02-15T15:38:54.168Z

Learnt from: CR
Repo: marcinpsk/netbox-librenms-plugin PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-02-15T15:38:54.168Z
Learning: Use devcontainer commands (`netbox-run`, `netbox-run-bg`, `netbox-reload`, `netbox-logs`) to manage NetBox + plugin reloading during development

Applied to files:

• .devcontainer/README.md
• .devcontainer/scripts/load-aliases.sh
• .devcontainer/.env.example
• .devcontainer/scripts/setup.sh
• .devcontainer/scripts/welcome.sh

📚 Learning: 2026-02-15T15:38:54.168Z

Learnt from: CR
Repo: marcinpsk/netbox-librenms-plugin PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-02-15T15:38:54.168Z
Learning: Applies to {navigation.py,urls.py,api/**} : Respect NetBox plugin APIs in `navigation.py`, `urls.py`, and `api/` directories

Applied to files:

• .devcontainer/README.md

📚 Learning: 2026-02-15T15:38:54.168Z

Learnt from: CR
Repo: marcinpsk/netbox-librenms-plugin PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-02-15T15:38:54.168Z
Learning: Applies to **/*.py : Always call `LibreNMSAPI.get_librenms_id` to retrieve the device/VM LibreNMS mapping via the `librenms_id` custom field instead of touching the field directly

Applied to files:

• docs/usage_tips/custom_field.md
• netbox_librenms_plugin/__init__.py

📚 Learning: 2026-02-15T15:38:54.168Z

Learnt from: CR
Repo: marcinpsk/netbox-librenms-plugin PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-02-15T15:38:54.168Z
Learning: Follow sync pipeline flow: fetch LibreNMS data via `librenms_api.py`, cache it using `CacheMixin`, build comparison tables in `tables/`, and render HTMX fragments in `templates/netbox_librenms_plugin/htmx/`

Applied to files:

• docs/usage_tips/custom_field.md

📚 Learning: 2026-02-15T15:39:11.571Z

Learnt from: CR
Repo: marcinpsk/netbox-librenms-plugin PR: 0
File: .github/instructions/frontend.instructions.md:0-0
Timestamp: 2026-02-15T15:39:11.571Z
Learning: Applies to netbox_librenms_plugin/templates/netbox_librenms_plugin/*sync*.html : Sync pages should extend `librenms_sync_base.html`.

Applied to files:

• docs/usage_tips/custom_field.md

📚 Learning: 2026-02-15T15:39:20.734Z

Learnt from: CR
Repo: marcinpsk/netbox-librenms-plugin PR: 0
File: .github/instructions/testing.instructions.md:0-0
Timestamp: 2026-02-15T15:39:20.734Z
Learning: Applies to tests/**/*.py : Patch deferred/inline imports at their source module (e.g., `netbox_librenms_plugin.import_utils.process_device_filters`), not the consuming module

Applied to files:

• netbox_librenms_plugin/tests/test_init.py

📚 Learning: 2026-02-15T15:39:20.734Z

Learnt from: CR
Repo: marcinpsk/netbox-librenms-plugin PR: 0
File: .github/instructions/testing.instructions.md:0-0
Timestamp: 2026-02-15T15:39:20.734Z
Learning: Applies to tests/**/*.py : Mock NetBox models (Device, Job, User) with `MagicMock()` instead of creating real instances

Applied to files:

• netbox_librenms_plugin/tests/test_init.py

📚 Learning: 2026-02-15T15:38:54.168Z

Learnt from: CR
Repo: marcinpsk/netbox-librenms-plugin PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-02-15T15:38:54.168Z
Learning: Applies to **/*.py : Reuse the `librenms_api.py` LibreNMS client instead of making new `requests` calls; it handles multi-server configs via `LibreNMSSettings` model and the `servers` plugin config, plus caching

Applied to files:

• netbox_librenms_plugin/librenms_api.py

🧬 Code graph analysis (1)

netbox_librenms_plugin/tests/test_init.py (1)

netbox_librenms_plugin/__init__.py (1)

• _ensure_librenms_id_custom_field (71-127)

🪛 LanguageTool

docs/usage_tips/custom_field.md

[style] ~22-~22: Using many exclamation marks might seem excessive (in this case: 6 exclamation marks for a text that’s 3155 characters long)
Context: ... ## Manual Custom Field Setup (Legacy) !!! note This section is only needed if...

(EN_EXCESSIVE_EXCLAMATION)

🪛 Shellcheck (0.11.0)

.devcontainer/scripts/setup.sh

[warning] 110-110: When used with -p, -m only applies to the deepest directory.

(SC2174)

🔇 Additional comments (25)

netbox_librenms_plugin/librenms_api.py (1)

44-64: LGTM! Clear server key validation with helpful error messages.

The validation logic correctly distinguishes between explicit non-default keys (which should error if missing) and the auto-default case (which gracefully falls back). The error message listing available servers is useful for debugging misconfiguration.

docs/usage_tips/custom_field.md (2)

7-8: Good documentation update for the auto-creation feature.

The info box clearly communicates the version requirement and which object types receive the field. This aligns well with the implementation in __init__.py.

---

21-24: Appropriate legacy section designation.

The note box provides clear guidance on when manual setup is still needed, maintaining backwards compatibility documentation.

netbox_librenms_plugin/__init__.py (4)

71-84: Well-documented idempotency guard.

The comment explaining why _executed is never reset is valuable for future maintainers. The pattern correctly handles the per-app post_migrate signal firing.

---

104-114: Content type synchronization logic is correct.

The approach of checking existing PKs before adding missing ones avoids duplicates and handles the case where an admin manually created the field with partial object types.

---

122-127: Good defensive exception handling.

Catching all exceptions and logging without propagating ensures the plugin doesn't break NetBox startup during edge cases like initial migrations or database issues.

---

91-102: CustomField type value is valid. The type="integer" is a supported choice in NetBox 4.2.0's CustomFieldTypeChoices enum. No changes needed.

netbox_librenms_plugin/tests/test_init.py (5)

1-21: LGTM! Good test setup with proper isolation.

The setup_method correctly resets the _executed flag before each test, ensuring consistent isolation between test runs.

---

23-63: Comprehensive test for custom field creation.

The test verifies the get_or_create call arguments, content type additions for all 4 models, and logging behavior. Good coverage of the happy path.

---

65-73: Good idempotency test.

Verifies the _executed flag prevents redundant database operations.

---

100-125: Well-designed partial content type test.

Using side_effect with alternating PKs effectively tests the logic that only adds missing content types.

---

127-142: Critical test for exception handling.

This test ensures the signal handler doesn't break migrations when database issues occur. Verifying both no-raise behavior and logging is thorough.

.devcontainer/config/plugin-config.py.example (1)

16-26: LGTM!

Good improvements: fixed the typo in the URL (exampel.com → example.com) and added the interface_name_field to make the production server configuration consistent with the other example entries.

.devcontainer/.env.example (1)

34-47: LGTM!

Well-documented proxy configuration guidance with appropriate security warnings. The recommendation to prefer CA bundles over disabling SSL verification is good practice.

.gitignore (1)

288-291: LGTM!

Appropriate additions to prevent committing user-specific CA certificates and local git hooks to the repository.

.devcontainer/scripts/load-aliases.sh (1)

64-67: LGTM!

The dev-help alias provides excellent developer experience by documenting all available commands in a well-organized format. This aligns with the recommended devcontainer command workflow.

.devcontainer/scripts/welcome.sh (1)

49-49: LGTM!

Helpful addition to the quick-start guide, directing users to netbox-restart for configuration changes.

.devcontainer/devcontainer.json (1)

37-48: LGTM!

Good additions for proxy and certificate support. Setting both uppercase and lowercase proxy variables (HTTP_PROXY/http_proxy) ensures compatibility with various tools that use different conventions. The CA bundle variables properly cover Python requests, OpenSSL, and curl.

.devcontainer/docker-compose.yml (1)

25-34: LGTM!

Consistent proxy and CA bundle configuration with the devcontainer.json changes. The empty default values (${VAR:-}) are appropriate for optional settings.

.devcontainer/scripts/setup.sh (3)

15-29: LGTM!

The detect_plugin_workspace() helper is well-designed with a clear fallback hierarchy: current directory → known path → find search. The function correctly exits 0 and uses stdout for the result, making it safe for callers to check for empty output.

---

31-76: LGTM!

The proxy configuration block is well-implemented:

• Apt proxy configuration is correctly written to /etc/apt/apt.conf.d/80proxy
• CA bundle installation into the system trust store is the right approach for MITM proxies
• The ALLOW_GIT_SSL_DISABLE opt-in pattern is good security practice—it requires explicit consent rather than silently disabling SSL verification

---

253-265: LGTM!

Good use of a sentinel comment to prevent duplicate entries when setup.sh is run multiple times. The grep -qF check is efficient and the sentinel pattern is clear.

.devcontainer/README.md (3)

69-81: Clear and helpful configuration workflow.

This new section provides a straightforward workflow for configuring LibreNMS servers, with proper references to the example config file and devcontainer commands. The placement after Quick Start makes logical sense.

---

158-158: LGTM: Proxy environment variables documented.

The addition of proxy and CA bundle environment variables appropriately extends the environment variables list and aligns with the detailed proxy configuration section.

---

160-236: Comprehensive proxy configuration documentation.

This new section provides excellent coverage of MITM proxy configuration for both Docker build-time and container runtime. The step-by-step approach with examples is very helpful, and the inclusion of troubleshooting guidance is valuable.

[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)

.devcontainer/scripts/setup.sh (1)

161-161: ⚠️ Potential issue | 🟠 Major

Hardcoded /workspaces/netbox-librenms-plugin/ in config injection should use $PLUGIN_WS_DIR.

Lines 161, 175, and 188 bake in a fixed workspace path for plugin-config.py, extra-configuration.py, and codespaces-configuration.py. Meanwhile, the rest of the script uses the dynamically detected $PLUGIN_WS_DIR. If the workspace is located at a different path (e.g., a differently named fork in Codespaces), the injected Python config will reference a non-existent directory.

Suggested fix

-      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'";

Also applies to: 175-175, 188-188

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In @.devcontainer/scripts/setup.sh at line 161, Replace the hardcoded
"/workspaces/netbox-librenms-plugin/" string in the injected Python lines with
the PLUGIN_WS_DIR variable so the generated config points to the detected
workspace; specifically, update the echo that sets _pc_path, and the similar
echoes that set paths for extra-configuration.py and codespaces-configuration.py
to use shell variable expansion (e.g. echo "_pc_path =
'${PLUGIN_WS_DIR}/.devcontainer/config/plugin-config.py'") ensuring you use
quotes that allow ${PLUGIN_WS_DIR} to expand and preserve the embedded
single-quote characters for valid Python syntax.

📜 Review details

Configuration used: Repository UI (base), Organization UI (inherited)

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d665ea1 and a440fbb.

📒 Files selected for processing (13)

• .devcontainer/.env.example
• .devcontainer/README.md
• .devcontainer/config/plugin-config.py.example
• .devcontainer/devcontainer.json
• .devcontainer/docker-compose.yml
• .devcontainer/scripts/load-aliases.sh
• .devcontainer/scripts/setup.sh
• .devcontainer/scripts/welcome.sh
• .gitignore
• docs/usage_tips/custom_field.md
• netbox_librenms_plugin/__init__.py
• netbox_librenms_plugin/librenms_api.py
• netbox_librenms_plugin/tests/test_init.py

🧰 Additional context used

📓 Path-based instructions (1)

**/*.py

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.py: Reuse the librenms_api.py LibreNMS client instead of making new requests calls; it handles multi-server configs via LibreNMSSettings model and the servers plugin config, plus caching
Always call LibreNMSAPI.get_librenms_id to retrieve the device/VM LibreNMS mapping via the librenms_id custom field instead of touching the field directly
Matching must be exact-only for site, platform, device type, and role; do not add fuzzy matching; use the functions find_matching_site, match_librenms_hardware_to_device_type, and find_matching_platform from utils.py

Files:

• netbox_librenms_plugin/tests/test_init.py
• netbox_librenms_plugin/librenms_api.py
• netbox_librenms_plugin/__init__.py

🧠 Learnings (7)

📚 Learning: 2026-02-15T15:39:20.734Z

Learnt from: CR
Repo: marcinpsk/netbox-librenms-plugin PR: 0
File: .github/instructions/testing.instructions.md:0-0
Timestamp: 2026-02-15T15:39:20.734Z
Learning: Applies to tests/**/*.py : Mock NetBox models (Device, Job, User) with `MagicMock()` instead of creating real instances

Applied to files:

• netbox_librenms_plugin/tests/test_init.py

📚 Learning: 2026-02-15T15:38:54.168Z

Learnt from: CR
Repo: marcinpsk/netbox-librenms-plugin PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-02-15T15:38:54.168Z
Learning: Use devcontainer commands (`netbox-run`, `netbox-run-bg`, `netbox-reload`, `netbox-logs`) to manage NetBox + plugin reloading during development

Applied to files:

• .devcontainer/scripts/setup.sh
• .devcontainer/README.md
• .devcontainer/scripts/load-aliases.sh
• .devcontainer/.env.example
• .devcontainer/scripts/welcome.sh

📚 Learning: 2026-02-15T15:38:54.168Z

Learnt from: CR
Repo: marcinpsk/netbox-librenms-plugin PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-02-15T15:38:54.168Z
Learning: Follow sync pipeline flow: fetch LibreNMS data via `librenms_api.py`, cache it using `CacheMixin`, build comparison tables in `tables/`, and render HTMX fragments in `templates/netbox_librenms_plugin/htmx/`

Applied to files:

• .devcontainer/README.md
• docs/usage_tips/custom_field.md

📚 Learning: 2026-02-15T15:38:54.168Z

Learnt from: CR
Repo: marcinpsk/netbox-librenms-plugin PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-02-15T15:38:54.168Z
Learning: Applies to {navigation.py,urls.py,api/**} : Respect NetBox plugin APIs in `navigation.py`, `urls.py`, and `api/` directories

Applied to files:

• .devcontainer/README.md

📚 Learning: 2026-02-15T15:38:54.168Z

Learnt from: CR
Repo: marcinpsk/netbox-librenms-plugin PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-02-15T15:38:54.168Z
Learning: Applies to **/*.py : Always call `LibreNMSAPI.get_librenms_id` to retrieve the device/VM LibreNMS mapping via the `librenms_id` custom field instead of touching the field directly

Applied to files:

• docs/usage_tips/custom_field.md
• netbox_librenms_plugin/__init__.py

📚 Learning: 2026-02-15T15:39:11.571Z

Learnt from: CR
Repo: marcinpsk/netbox-librenms-plugin PR: 0
File: .github/instructions/frontend.instructions.md:0-0
Timestamp: 2026-02-15T15:39:11.571Z
Learning: Applies to netbox_librenms_plugin/templates/netbox_librenms_plugin/*sync*.html : Sync pages should extend `librenms_sync_base.html`.

Applied to files:

• docs/usage_tips/custom_field.md

📚 Learning: 2026-02-15T15:38:54.168Z

Learnt from: CR
Repo: marcinpsk/netbox-librenms-plugin PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-02-15T15:38:54.168Z
Learning: Applies to **/*.py : Reuse the `librenms_api.py` LibreNMS client instead of making new `requests` calls; it handles multi-server configs via `LibreNMSSettings` model and the `servers` plugin config, plus caching

Applied to files:

• netbox_librenms_plugin/librenms_api.py

🧬 Code graph analysis (1)

netbox_librenms_plugin/tests/test_init.py (1)

netbox_librenms_plugin/__init__.py (1)

• _ensure_librenms_id_custom_field (71-127)

🪛 LanguageTool

docs/usage_tips/custom_field.md

[style] ~22-~22: Using many exclamation marks might seem excessive (in this case: 6 exclamation marks for a text that’s 3155 characters long)
Context: ... ## Manual Custom Field Setup (Legacy) !!! note This section is only needed if...

(EN_EXCESSIVE_EXCLAMATION)

🪛 Shellcheck (0.11.0)

.devcontainer/scripts/setup.sh

[warning] 110-110: When used with -p, -m only applies to the deepest directory.

(SC2174)

🔇 Additional comments (21)

netbox_librenms_plugin/librenms_api.py (1)

44-63: LGTM — server-key validation logic is sound.

The two-tier approach (strict KeyError for explicitly requested keys, graceful fallback for the auto-default) is a good pattern that prevents silent misconfiguration while keeping backward compatibility with single-server setups.

.devcontainer/config/plugin-config.py.example (1)

16-22: Good fixes — typo correction and comment cleanup look correct.

.gitignore (1)

288-291: LGTM — sensible ignore rules for generated CA artifacts.

Minor note: *.pem is broad and would match any PEM file in the repo tree. If test fixtures or other intentional .pem files are ever added, they'd need a negation rule. Fine for now.

docs/usage_tips/custom_field.md (1)

7-8: Documentation accurately reflects the new auto-creation behavior.

The version reference (0.4.2) matches __version__ in __init__.py, and the listed object types align with the implementation.

netbox_librenms_plugin/__init__.py (3)

41-45: Signal registration in ready() is correctly implemented.

Using dispatch_uid ensures Django deduplicates the handler even if ready() is called multiple times. Good pattern.

---

71-84: The _executed flag guard is well-documented but relies on mutable function attribute state.

The pattern works correctly for CLI migrate processes. The comment on lines 78–81 clearly explains the design intent and limitations — good documentation.

---

86-102: No action needed. The type="integer" value is correct for NetBox 4.x CustomField. NetBox's CustomField.type accepts string values directly (not enum members), and "integer" is the documented and expected value for integer-type custom fields.

.devcontainer/scripts/welcome.sh (1)

49-49: Good addition — netbox-restart is defined in load-aliases.sh and useful after config changes.

.devcontainer/devcontainer.json (1)

37-46: Proxy and CA environment variable passthrough looks correct.

Both uppercase and lowercase proxy variants are provided, which is the right approach since different tools (curl, wget, Python requests, etc.) respect different casings. Unset localEnv variables will resolve to empty strings, which is benign for proxy variables.

.devcontainer/.env.example (1)

34-47: Well-documented proxy configuration section with appropriate security guidance.

The recommendation to prefer CA bundles over disabling SSL verification (line 46) is good practice.

.devcontainer/docker-compose.yml (1)

25-34: LGTM — proxy and CA environment variables are cleanly added.

The approach of defaulting to empty strings is correct and keeps the config optional. One subtle note: the lowercase variants (http_proxy, https_proxy, no_proxy) are sourced from the uppercase host variables (${HTTP_PROXY:-}, etc.), so if a user only defines lowercase versions in their host environment, those won't propagate into the container. This appears intentional (normalizing on uppercase in .env), but worth a brief mention in the README if it isn't already covered.

.devcontainer/README.md (2)

68-81: LibreNMS server configuration section is clear and actionable.

Good addition — linking to plugin-config.py setup early in the quick-start flow helps users configure the plugin before hitting runtime errors.

---

160-235: Proxy documentation is thorough and well-organized.

The two-level proxy explanation (Docker client vs. container runtime) will save users a lot of debugging time. The step-by-step flow and common issues section are practical additions.

netbox_librenms_plugin/tests/test_init.py (5)

14-21: Good test isolation with setup_method resetting _executed.

Resetting the internal flag before each test ensures proper isolation. The comment about why it exists (per-test consistency) is helpful.

---

23-63: Solid test for the creation path.

The test correctly verifies the get_or_create call with expected defaults, the 4 content type additions, and that the logger is invoked on creation. The patch target ordering and parameter mapping are correct.

One minor observation: all 4 get_for_model calls return the same mock_ct (pk=1), yet the test expects 4 add calls. This works because current_types is captured as an empty set before the loop and isn't updated during iteration — matching the real function's behavior. The test accurately reflects the implementation.

---

65-73: Correct dedup test.

Clean verification that the _executed guard short-circuits without touching the DB.

---

100-125: Well-constructed partial content-type test.

Using side_effect to return different ContentType mocks per model call is the right approach here. The assertion on both call_count and assert_any_call provides good coverage.

---

144-170: Good negative assertion on logging.

Verifying via logger_instance.info.assert_not_called() rather than checking getLogger was never called is the right approach — resilient to internal refactoring.

.devcontainer/scripts/setup.sh (3)

15-29: detect_plugin_workspace is a good addition for flexible workspace detection.

The three-tier fallback (PWD → well-known path → find search) is practical. Minor note: find order is filesystem-dependent, so if multiple pyproject.toml files exist under /workspaces, the result is non-deterministic. This is low-risk given the prior checks.

---

31-76: Proxy and CA handling block is well-structured.

The conditional activation, apt proxy injection, and the ALLOW_GIT_SSL_DISABLE gate are all good patterns. Requiring explicit opt-in before disabling SSL verification is the right security posture.

---

253-265: Sentinel-guarded .bashrc append prevents duplicate entries on re-run.

Good pattern. Note that $PLUGIN_WS_DIR is expanded at write time (unquoted heredoc delimiter), so the resolved path is baked into .bashrc. This is fine for a dev container lifecycle.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.devcontainer/scripts/load-aliases.sh:
- Around line 64-67: Replace the giant single-line alias dev-help with a shell
function (e.g., dev_help) that contains the multi-line help output, then set
alias dev-help='dev_help' so existing calls keep working; specifically, remove
the long inline alias for dev-help, add a function named dev_help that prints
the same multi-line text (using a here-doc or multiple echo/printf lines), and
then add alias dev-help='dev_help' to preserve the original alias name.

In @.devcontainer/scripts/setup.sh:
- Around line 56-57: The script currently runs update-ca-certificates under a
global set -e so any non-zero exit will abort silently (stderr redirected), so
make the command non-fatal and surface failures: modify the
update-ca-certificates invocation to either append || true to prevent exiting on
failure or capture its exit code and log an error before continuing (referencing
the existing update-ca-certificates invocation and the echo "  ✓ CA certificate
installed into system trust store" message), so failures don’t silently stop the
script and the error is logged for debugging.

In `@netbox_librenms_plugin/__init__.py`:
- Around line 116-127: Hoist the duplicate "import logging" out of the
success/exception branches by adding a single import logging at the top of the
surrounding scope (module or the try block) so both
logging.getLogger("netbox_librenms_plugin").info(...) and .exception(...) use
the same imported module; remove the two inline imports and keep the existing
logger.getLogger calls unchanged (look for the created check and the exception
handler around auto-creating the 'librenms_id' custom field).

In `@netbox_librenms_plugin/tests/test_init.py`:
- Around line 127-142: Update the test_exception_does_not_propagate test to mock
the same external dependencies used in the other tests: patch
django.contrib.contenttypes.models.ContentType and the dcim/virtualization model
imports (the same targets used in TestEnsureLibreNMSIdCustomField) before
importing or calling _ensure_librenms_id_custom_field; keep the existing
MockCustomField patch and Exception side_effect, then assert that
logger.exception was called as before so the test remains consistent and
explicit about its external dependencies.

---

Outside diff comments:
In @.devcontainer/scripts/setup.sh:
- Line 161: Replace the hardcoded "/workspaces/netbox-librenms-plugin/" string
in the injected Python lines with the PLUGIN_WS_DIR variable so the generated
config points to the detected workspace; specifically, update the echo that sets
_pc_path, and the similar echoes that set paths for extra-configuration.py and
codespaces-configuration.py to use shell variable expansion (e.g. echo "_pc_path
= '${PLUGIN_WS_DIR}/.devcontainer/config/plugin-config.py'") ensuring you use
quotes that allow ${PLUGIN_WS_DIR} to expand and preserve the embedded
single-quote characters for valid Python syntax.

---

Duplicate comments:
In @.devcontainer/scripts/setup.sh:
- Around line 107-121: Replace the mkdir invocation that uses the combined -p
and -m flags (mkdir -p -m 755 /etc/apt/keyrings) with a command that reliably
sets permissions on the target directory only; for example, use install -d -m
755 /etc/apt/keyrings or run mkdir -p /etc/apt/keyrings followed by chmod 755
/etc/apt/keyrings within the GitHub CLI install block (the if ! command -v gh
... fi section) to address the SC2174 concern.

[coderabbitai]

🧹 Nitpick | 🔵 Trivial

Comprehensive help alias — commands listed match the defined aliases.

The dev-help alias is quite long as a single line. Consider converting it to a shell function for maintainability, but this is purely cosmetic for an alias file.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In @.devcontainer/scripts/load-aliases.sh around lines 64 - 67, Replace the
giant single-line alias dev-help with a shell function (e.g., dev_help) that
contains the multi-line help output, then set alias dev-help='dev_help' so
existing calls keep working; specifically, remove the long inline alias for
dev-help, add a function named dev_help that prints the same multi-line text
(using a here-doc or multiple echo/printf lines), and then add alias
dev-help='dev_help' to preserve the original alias name.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

update-ca-certificates failure will abort the script due to set -e.

If update-ca-certificates returns non-zero (e.g., malformed certificate, missing dependency), set -e (line 2) will terminate the script immediately. Since stderr is suppressed with 2>/dev/null, the failure would be silent and hard to diagnose. The success message on line 57 would never print, but there'd be no indication of why setup stopped.

Add || true to make this non-fatal, or at least log the failure:

Suggested fix

-    update-ca-certificates 2>/dev/null
-    echo "  ✓ CA certificate installed into system trust store"
+    if update-ca-certificates 2>&1; then
+      echo "  ✓ CA certificate installed into system trust store"
+    else
+      echo "  ⚠️  update-ca-certificates failed; SSL may not trust proxy CA"
+    fi

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	update-ca-certificates 2>/dev/null
	echo " ✓ CA certificate installed into system trust store"
	if update-ca-certificates 2>&1; then
	echo " ✓ CA certificate installed into system trust store"
	else
	echo " ⚠️ update-ca-certificates failed; SSL may not trust proxy CA"
	fi

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In @.devcontainer/scripts/setup.sh around lines 56 - 57, The script currently
runs update-ca-certificates under a global set -e so any non-zero exit will
abort silently (stderr redirected), so make the command non-fatal and surface
failures: modify the update-ca-certificates invocation to either append || true
to prevent exiting on failure or capture its exit code and log an error before
continuing (referencing the existing update-ca-certificates invocation and the
echo "  ✓ CA certificate installed into system trust store" message), so
failures don’t silently stop the script and the error is logged for debugging.

[coderabbitai]

🧹 Nitpick | 🔵 Trivial

Consider hoisting the logging import to reduce duplication.

import logging appears in both the success branch (line 117) and exception branch (line 125). Moving it to the top of the try block (or the function) would be cleaner.

Proposed refactor

 def _ensure_librenms_id_custom_field(sender, **kwargs):
+    import logging
+    logger = logging.getLogger("netbox_librenms_plugin")
+
     if getattr(_ensure_librenms_id_custom_field, "_executed", False):
         return
     _ensure_librenms_id_custom_field._executed = True
 
     try:
         from django.contrib.contenttypes.models import ContentType
         from extras.models import CustomField
         # ... get_or_create and content type logic ...
 
         if created:
-            import logging
-            logging.getLogger("netbox_librenms_plugin").info(
+            logger.info(
                 "Auto-created 'librenms_id' custom field for Device, VirtualMachine, Interface, VMInterface"
             )
     except Exception as e:
-        import logging
-        logging.getLogger("netbox_librenms_plugin").exception("Failed to auto-create 'librenms_id' custom field: %s", e)
+        logger.exception("Failed to auto-create 'librenms_id' custom field: %s", e)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@netbox_librenms_plugin/__init__.py` around lines 116 - 127, Hoist the
duplicate "import logging" out of the success/exception branches by adding a
single import logging at the top of the surrounding scope (module or the try
block) so both logging.getLogger("netbox_librenms_plugin").info(...) and
.exception(...) use the same imported module; remove the two inline imports and
keep the existing logger.getLogger calls unchanged (look for the created check
and the exception handler around auto-creating the 'librenms_id' custom field).

[coderabbitai]

🧹 Nitpick | 🔵 Trivial

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if there's a conftest.py or pytest config that sets DJANGO_SETTINGS_MODULE
echo "=== Looking for pytest/django configuration ==="
fd -t f "conftest.py" --exec cat {}
echo "---"
fd -t f "pytest.ini" --exec cat {} 2>/dev/null || true
fd -t f "setup.cfg" --exec grep -A5 "\[tool:pytest\]" {} 2>/dev/null || true
fd -t f "pyproject.toml" --exec grep -A10 "\[tool.pytest" {} 2>/dev/null || true

Repository: marcinpsk/netbox-librenms-plugin

Length of output: 8900

---

🏁 Script executed:

# Now let's examine the test file to see the actual test and understand the setup
echo "=== Examining test_init.py around lines 127-142 ==="
cat -n netbox_librenms_plugin/tests/test_init.py | sed -n '110,160p'

Repository: marcinpsk/netbox-librenms-plugin

Length of output: 2757

---

🏁 Script executed:

# Let's also find and examine the _ensure_librenms_id_custom_field function
echo "=== Finding _ensure_librenms_id_custom_field implementation ==="
rg -n "_ensure_librenms_id_custom_field" --type py

Repository: marcinpsk/netbox-librenms-plugin

Length of output: 2445

---

🏁 Script executed:

echo "=== Examining _ensure_librenms_id_custom_field implementation ==="
cat -n netbox_librenms_plugin/__init__.py | sed -n '71,120p'

Repository: marcinpsk/netbox-librenms-plugin

Length of output: 2635

---

🏁 Script executed:

echo "=== Checking imports at the top of __init__.py ==="
head -n 80 netbox_librenms_plugin/__init__.py

Repository: marcinpsk/netbox-librenms-plugin

Length of output: 3403

---

🏁 Script executed:

cat -n netbox_librenms_plugin/tests/test_init.py | sed -n '1,165p'

Repository: marcinpsk/netbox-librenms-plugin

Length of output: 8406

---

🏁 Script executed:

cat -n netbox_librenms_plugin/__init__.py | sed -n '71,135p'

Repository: marcinpsk/netbox-librenms-plugin

Length of output: 3010

---

This test should patch ContentType and the dcim/virtualization model imports for consistency with the other tests in the class.

All other tests in TestEnsureLibreNMSIdCustomField (lines 23–126, 75–98, 100–125, 144–165) patch django.contrib.contenttypes.models.ContentType and the dcim/virtualization model imports, but test_exception_does_not_propagate does not. While the current test works correctly under proper pytest-django configuration (which is in place), patching these dependencies would make the test's intent clearer and consistent with the established pattern in the class.

The concern about implicit Django coupling is mitigated by the existing DJANGO_SETTINGS_MODULE = "netbox.settings" in pytest configuration, which ensures Django is fully initialized. However, the incomplete mocking makes the test less explicit about what it's testing.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@netbox_librenms_plugin/tests/test_init.py` around lines 127 - 142, Update the
test_exception_does_not_propagate test to mock the same external dependencies
used in the other tests: patch django.contrib.contenttypes.models.ContentType
and the dcim/virtualization model imports (the same targets used in
TestEnsureLibreNMSIdCustomField) before importing or calling
_ensure_librenms_id_custom_field; keep the existing MockCustomField patch and
Exception side_effect, then assert that logger.exception was called as before so
the test remains consistent and explicit about its external dependencies.