[feature] Added X.509 Certificate Generator Templates#1378
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR adds standalone Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant DeviceConfigSerializer
participant AbstractConfig
participant manage_device_certs
participant DeviceCertificate
participant ConfigContext
DeviceConfigSerializer->>AbstractConfig: update templates
AbstractConfig->>manage_device_certs: process template assignment
manage_device_certs->>DeviceCertificate: create or delete certificate rows
DeviceCertificate->>AbstractConfig: update certificate state
AbstractConfig->>ConfigContext: inject certificate context
sequenceDiagram
participant Device
participant detect_hardware_drift
participant regenerate_device_certificates_task
participant DeviceCertificate
participant Config
Device->>detect_hardware_drift: save name or MAC change
detect_hardware_drift->>regenerate_device_certificates_task: schedule regeneration
regenerate_device_certificates_task->>DeviceCertificate: revoke and replace certificates
regenerate_device_certificates_task->>Config: update status after commit
Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 4❌ Failed checks (4 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Files Reviewed (9 files)
Previous Review Summaries (11 snapshots, latest commit 1d117a2)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 1d117a2)Status: No Issues Found | Recommendation: Merge Incremental Review (since 375844d)This delta is a test-only refactor in Files Reviewed (incremental — 1 file)
Previous review (commit 375844d)Status: No Issues Found | Recommendation: Merge Incremental Review (since c12aee9)This delta refactors the hardware-drift detection (name/MAC change → certificate regeneration) to live entirely in the model rather than a separate
Ordering is correct: Tests cover the refactored paths: name change, MAC change, unrelated-field save (ignored), Files Reviewed (incremental — 2 files)
Previous review (commit c12aee9)Status: No Issues Found | Recommendation: Merge Incremental Review (since b9a9d2a)Since the last review, the branch merged the base branch ( The author's own changes in this delta are clean and well-tested:
New/updated tests cover the notes field (admin ordering, search, API create/list/patch), the cert-template type-mutation lock, the certificate-details admin rendering, and the WHOIS Selenium error helper. No critical bugs or security vulnerabilities in the changed code. Files Reviewed (incremental — feature scope)
Previous review (commit b9a9d2a)Status: No Issues Found | Recommendation: Merge Incremental Review (since 52c7544)The new commit (
No production request handlers, serializers, migrations, tenant-filtering, or security-relevant code paths are modified in this delta. No new critical bugs or security vulnerabilities. Files Reviewed (incremental, 2 files)
Previous review (commit 52c7544)Status: No Issues Found | Recommendation: Merge Incremental Review (since b486eb5)The new commit (
No production request handlers, serializers, migrations, or tenant-filtering code paths are changed in this delta. No new critical bugs or security vulnerabilities were introduced. Files Reviewed (incremental, 3 files)
Previous review (commit b486eb5)Status: No Issues Found | Recommendation: Merge Incremental Review (since b52fe63)The new commits are low-risk follow-ups:
All changes are confined to the test Files Reviewed (incremental)
Previous review (commit b52fe63)Status: No Issues Found | Recommendation: Merge Incremental Review (since fb54051)The new commit A corresponding regression test, No new critical bugs or security vulnerabilities were introduced. Files Reviewed (incremental)
Previous review (commit fb54051)Status: No Issues Found | Recommendation: Merge Incremental Review (since 8c32282)The new commit
No new critical bugs or security vulnerabilities were introduced. Files Reviewed (incremental)
Carried-forward from prior reviewAll previously reviewed production files and their findings carry forward unchanged. Prior outstanding notes (cert-template validation regression test and Previous review (commit 8c32282)Status: No Issues Found | Recommendation: Merge Incremental Review (since a819721)The new commit
No new critical bugs or security vulnerabilities were introduced by this commit. Files Reviewed (incremental)
Carried-forward from prior reviewAll previously reviewed files and their findings carry forward unchanged. The prior outstanding note (missing regression test for cert-template validation in Previous review (commit a819721)Status: No Issues Found | Recommendation: Merge Incremental Review (since 298572f)The new commits add a large body of work: certificate regeneration on hardware drift (#1359), deactivated-device enforcement across config/connection/WHOIS/geo, pagination consolidation to The previously flagged WARNING (missing regression test for cert-template validation in Review highlights:
Extensive new tests accompany each change and Files Reviewed (incremental)
Previous review (commit 298572f)Status: 1 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Other Observations (not in diff)Previous findings on No additional issues in unchanged code. Files Reviewed (5 files)
Reviewed by step-3.7-flash · Input: 142.5K · Output: 36.6K · Cached: 3.1M |
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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 `@openwisp_controller/config/base/template.py`:
- Around line 265-267: The help text for the auto_cert field is out of date (it
still says it's only valid for VPN templates) — update the auto_cert field's
help/verbose/help_text in the Template definition in
openwisp_controller/config/base/template.py so it matches the new behavior
(auto_cert is allowed when type == "cert" as well as when type == "vpn"); locate
the auto_cert attribute (and any admin/API serializer or form label/help_text
referencing it) and change the message to something like "Valid for 'vpn' and
'cert' template types" or equivalent clear wording that includes both types.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 33bb61f8-c083-446e-8e45-44d753e7ff7b
📒 Files selected for processing (3)
openwisp_controller/config/base/template.pyopenwisp_controller/config/migrations/0064_template_blueprint_cert_template_ca_and_more.pytests/openwisp2/sample_config/migrations/0010_template_blueprint_cert_template_ca_and_more.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{js,ts,tsx,jsx,py,java,go,cs,rb,php,c,cpp,h,hpp}
📄 CodeRabbit inference engine (Custom checks)
**/*.{js,ts,tsx,jsx,py,java,go,cs,rb,php,c,cpp,h,hpp}: Flag potential security vulnerabilities in code
Avoid unnecessary comments or docstrings for code that is already clear
Code formatting is compact and readable. Do not add excessive blank lines, especially inside function or method bodies
Flag unused or redundant code
Ensure variables, functions, classes, and files have descriptive and consistent names
New code must handle errors properly: log errors that cannot be resolved by the user with error level, log unusual conditions with warning level, log important background actions with info level, and provide user-facing messages for errors that the user can solve autonomously
Files:
tests/openwisp2/sample_config/migrations/0010_template_blueprint_cert_template_ca_and_more.pyopenwisp_controller/config/base/template.pyopenwisp_controller/config/migrations/0064_template_blueprint_cert_template_ca_and_more.py
**/*.{js,ts,tsx,jsx,py,java,go,cs,rb,php,c,cpp,h,hpp,sql}
📄 CodeRabbit inference engine (Custom checks)
Flag obvious performance regressions, such as heavy loops, repeated I/O, or unoptimized queries
Files:
tests/openwisp2/sample_config/migrations/0010_template_blueprint_cert_template_ca_and_more.pyopenwisp_controller/config/base/template.pyopenwisp_controller/config/migrations/0064_template_blueprint_cert_template_ca_and_more.py
**/*.{js,ts,tsx,jsx,py,java,go,cs,rb,php,c,cpp,h,hpp,sh,bash,sql}
📄 CodeRabbit inference engine (Custom checks)
Cryptic or non-obvious code (regex, complex bash commands, or hard-to-read code) must include a concise comment explaining why it is needed and why the complexity is acceptable
Files:
tests/openwisp2/sample_config/migrations/0010_template_blueprint_cert_template_ca_and_more.pyopenwisp_controller/config/base/template.pyopenwisp_controller/config/migrations/0064_template_blueprint_cert_template_ca_and_more.py
**/*.{py,html}
📄 CodeRabbit inference engine (Custom checks)
For Django pull requests, ensure all user-facing strings are marked as translatable using the Django i18n framework
Files:
tests/openwisp2/sample_config/migrations/0010_template_blueprint_cert_template_ca_and_more.pyopenwisp_controller/config/base/template.pyopenwisp_controller/config/migrations/0064_template_blueprint_cert_template_ca_and_more.py
🧠 Learnings (4)
📚 Learning: 2026-01-12T22:27:40.078Z
Learnt from: nemesifier
Repo: openwisp/openwisp-controller PR: 1175
File: tests/openwisp2/sample_config/migrations/0008_whoisinfo_organizationconfigsettings_whois_enabled.py:18-67
Timestamp: 2026-01-12T22:27:40.078Z
Learning: In test migrations under tests/openwisp2/sample_config/migrations, verify scenarios where a swappable model (CONFIG_WHOISINFO_MODEL) is extended with extra fields (e.g., an additional 'details' field) to ensure compatibility and no errors when swapping to a custom implementation. This pattern helps confirm that extending AbstractWHOISInfo via a custom model works as intended.
Applied to files:
tests/openwisp2/sample_config/migrations/0010_template_blueprint_cert_template_ca_and_more.py
📚 Learning: 2026-01-15T15:07:17.354Z
Learnt from: DragnEmperor
Repo: openwisp/openwisp-controller PR: 1175
File: openwisp_controller/geo/estimated_location/tests/tests.py:172-175
Timestamp: 2026-01-15T15:07:17.354Z
Learning: In this repository, flake8 enforces E501 (line too long) via setup.cfg (max-line-length = 88) while ruff ignores E501 via ruff.toml. Therefore, use '# noqa: E501' on lines that intentionally exceed 88 characters to satisfy flake8 without affecting ruff checks. This applies to Python files across the project (any .py) and is relevant for tests as well. Use sparingly and only where breaking lines is not feasible without hurting readability or functionality.
Applied to files:
tests/openwisp2/sample_config/migrations/0010_template_blueprint_cert_template_ca_and_more.pyopenwisp_controller/config/base/template.pyopenwisp_controller/config/migrations/0064_template_blueprint_cert_template_ca_and_more.py
📚 Learning: 2026-01-15T15:05:49.557Z
Learnt from: DragnEmperor
Repo: openwisp/openwisp-controller PR: 1175
File: openwisp_controller/config/management/commands/clear_last_ip.py:38-42
Timestamp: 2026-01-15T15:05:49.557Z
Learning: In Django projects, when using select_related() to traverse relations (for example, select_related("organization__config_settings")), the traversed relation must not be deferred. If you also use .only() in the same query, include the relation name or FK field (e.g., "organization" or "organization_id") in the .only() list to avoid the error "Field X cannot be both deferred and traversed using select_related at the same time." Apply this guideline to Django code in openwisp_controller/config/management/commands/clear_last_ip.py and similar modules by ensuring any select_related with an accompanying only() includes the related field names to prevent deferred/traversed conflicts.
Applied to files:
openwisp_controller/config/base/template.pyopenwisp_controller/config/migrations/0064_template_blueprint_cert_template_ca_and_more.py
📚 Learning: 2026-02-17T19:13:10.088Z
Learnt from: nemesifier
Repo: openwisp/openwisp-controller PR: 1175
File: openwisp_controller/config/whois/commands.py:0-0
Timestamp: 2026-02-17T19:13:10.088Z
Learning: In reviews for the openwisp/openwisp-controller repository, do not propose changes based on Ruff warnings. The project does not use Ruff as its linter; ignore Ruff-related suggestions and follow the repository’s established linting and configuration rules. This guidance applies to all Python files under the openwisp_controller directory.
Applied to files:
openwisp_controller/config/base/template.pyopenwisp_controller/config/migrations/0064_template_blueprint_cert_template_ca_and_more.py
🔇 Additional comments (3)
openwisp_controller/config/base/template.py (1)
25-29: LGTM!Also applies to: 62-83, 251-253, 271-312
openwisp_controller/config/migrations/0064_template_blueprint_cert_template_ca_and_more.py (1)
1-59: LGTM!tests/openwisp2/sample_config/migrations/0010_template_blueprint_cert_template_ca_and_more.py (1)
1-58: LGTM!
{
"failures": [
{
"type": "Build/Infrastructure/Other",
"message": "Migrations check failed! Models' changes not migrated, please run './manage.py makemigrations' to solve the issue!",
"details": "The CI environment detected that the database migrations are out of sync. This is indicated by the error message 'Migrations check failed! Models' changes not migrated, please run './manage.py makemigrations' to solve the issue!' and the mention of a specific migration file '0005_user_expiration_date_user_user_active_expiry_idx.py'."
}
]
}CI Failure AnalysisHello @stktyagi,
Fix: Run |
Updated previous tests and added new tests for implemetation. Fixes #1356
Fixed help text for auto cert and updated migration files
Migrations Check FailedHello @stktyagi, The CI failed because there are unapplied database migrations. This means that the database schema is out of sync with the current state of the code. Fix: ./manage.py makemigrations |
|
The CI is failing due to transient infrastructure issues (not related to your code). I have restarted the failed jobs automatically (1/3). |
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@openwisp_controller/config/base/template.py`:
- Around line 252-253: The validation of certificate relations (calls to
self._validate_org_relation("ca") and
self._validate_org_relation("blueprint_cert")) is running unconditionally and
can fail on stale relations when the object is being switched away from the
"cert" type; restrict these validations to the cert branch so they only run when
the template's type is "cert" (e.g., wrap or move the
self._validate_org_relation(...) calls inside the same conditional that handles
the "cert" branch or after the type check that preserves/clears ca and
blueprint_cert), ensuring they do not run when the code path clears those fields
(see the branch that clears ca and blueprint_cert).
- Around line 304-305: The current check "if not self.config" coerces any falsy
value ([], "", False) into {}, bypassing BaseConfig.clean() validation; change
the condition to only handle missing configs by checking "if self.config is
None" (or equivalent explicit None check) so only absent configs are replaced
with {} and invalid/falsy payloads are left intact for
full_clean()/BaseConfig.clean() to reject.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: d3d93328-e58d-41dd-a374-dffebd6d1e38
📒 Files selected for processing (5)
openwisp_controller/config/base/template.pyopenwisp_controller/config/migrations/0064_template_blueprint_cert_template_ca_and_more.pyopenwisp_controller/config/tests/test_template.pyopenwisp_controller/pki/tests/test_api.pytests/openwisp2/sample_config/migrations/0010_template_blueprint_cert_template_ca_and_more.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: Python==3.12 | django~=4.2.0
- GitHub Check: Python==3.12 | django~=5.1.0
- GitHub Check: Python==3.11 | django~=4.2.0
- GitHub Check: Python==3.10 | django~=5.1.0
- GitHub Check: Python==3.13 | django~=5.1.0
- GitHub Check: Python==3.10 | django~=4.2.0
- GitHub Check: Python==3.12 | django~=5.2.0
- GitHub Check: Python==3.10 | django~=5.2.0
- GitHub Check: Python==3.13 | django~=5.2.0
- GitHub Check: Python==3.11 | django~=5.2.0
- GitHub Check: Python==3.11 | django~=5.1.0
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{js,ts,tsx,jsx,py,java,go,cs,rb,php,c,cpp,h,hpp}
📄 CodeRabbit inference engine (Custom checks)
**/*.{js,ts,tsx,jsx,py,java,go,cs,rb,php,c,cpp,h,hpp}: Flag potential security vulnerabilities in code
Avoid unnecessary comments or docstrings for code that is already clear
Code formatting is compact and readable. Do not add excessive blank lines, especially inside function or method bodies
Flag unused or redundant code
Ensure variables, functions, classes, and files have descriptive and consistent names
New code must handle errors properly: log errors that cannot be resolved by the user with error level, log unusual conditions with warning level, log important background actions with info level, and provide user-facing messages for errors that the user can solve autonomously
Files:
openwisp_controller/config/migrations/0064_template_blueprint_cert_template_ca_and_more.pytests/openwisp2/sample_config/migrations/0010_template_blueprint_cert_template_ca_and_more.pyopenwisp_controller/pki/tests/test_api.pyopenwisp_controller/config/base/template.pyopenwisp_controller/config/tests/test_template.py
**/*.{js,ts,tsx,jsx,py,java,go,cs,rb,php,c,cpp,h,hpp,sql}
📄 CodeRabbit inference engine (Custom checks)
Flag obvious performance regressions, such as heavy loops, repeated I/O, or unoptimized queries
Files:
openwisp_controller/config/migrations/0064_template_blueprint_cert_template_ca_and_more.pytests/openwisp2/sample_config/migrations/0010_template_blueprint_cert_template_ca_and_more.pyopenwisp_controller/pki/tests/test_api.pyopenwisp_controller/config/base/template.pyopenwisp_controller/config/tests/test_template.py
**/*.{js,ts,tsx,jsx,py,java,go,cs,rb,php,c,cpp,h,hpp,sh,bash,sql}
📄 CodeRabbit inference engine (Custom checks)
Cryptic or non-obvious code (regex, complex bash commands, or hard-to-read code) must include a concise comment explaining why it is needed and why the complexity is acceptable
Files:
openwisp_controller/config/migrations/0064_template_blueprint_cert_template_ca_and_more.pytests/openwisp2/sample_config/migrations/0010_template_blueprint_cert_template_ca_and_more.pyopenwisp_controller/pki/tests/test_api.pyopenwisp_controller/config/base/template.pyopenwisp_controller/config/tests/test_template.py
**/*.{py,html}
📄 CodeRabbit inference engine (Custom checks)
For Django pull requests, ensure all user-facing strings are marked as translatable using the Django i18n framework
Files:
openwisp_controller/config/migrations/0064_template_blueprint_cert_template_ca_and_more.pytests/openwisp2/sample_config/migrations/0010_template_blueprint_cert_template_ca_and_more.pyopenwisp_controller/pki/tests/test_api.pyopenwisp_controller/config/base/template.pyopenwisp_controller/config/tests/test_template.py
🧠 Learnings (4)
📚 Learning: 2026-01-15T15:05:49.557Z
Learnt from: DragnEmperor
Repo: openwisp/openwisp-controller PR: 1175
File: openwisp_controller/config/management/commands/clear_last_ip.py:38-42
Timestamp: 2026-01-15T15:05:49.557Z
Learning: In Django projects, when using select_related() to traverse relations (for example, select_related("organization__config_settings")), the traversed relation must not be deferred. If you also use .only() in the same query, include the relation name or FK field (e.g., "organization" or "organization_id") in the .only() list to avoid the error "Field X cannot be both deferred and traversed using select_related at the same time." Apply this guideline to Django code in openwisp_controller/config/management/commands/clear_last_ip.py and similar modules by ensuring any select_related with an accompanying only() includes the related field names to prevent deferred/traversed conflicts.
Applied to files:
openwisp_controller/config/migrations/0064_template_blueprint_cert_template_ca_and_more.pyopenwisp_controller/pki/tests/test_api.pyopenwisp_controller/config/base/template.pyopenwisp_controller/config/tests/test_template.py
📚 Learning: 2026-02-17T19:13:10.088Z
Learnt from: nemesifier
Repo: openwisp/openwisp-controller PR: 1175
File: openwisp_controller/config/whois/commands.py:0-0
Timestamp: 2026-02-17T19:13:10.088Z
Learning: In reviews for the openwisp/openwisp-controller repository, do not propose changes based on Ruff warnings. The project does not use Ruff as its linter; ignore Ruff-related suggestions and follow the repository’s established linting and configuration rules. This guidance applies to all Python files under the openwisp_controller directory.
Applied to files:
openwisp_controller/config/migrations/0064_template_blueprint_cert_template_ca_and_more.pyopenwisp_controller/pki/tests/test_api.pyopenwisp_controller/config/base/template.pyopenwisp_controller/config/tests/test_template.py
📚 Learning: 2026-01-15T15:07:17.354Z
Learnt from: DragnEmperor
Repo: openwisp/openwisp-controller PR: 1175
File: openwisp_controller/geo/estimated_location/tests/tests.py:172-175
Timestamp: 2026-01-15T15:07:17.354Z
Learning: In this repository, flake8 enforces E501 (line too long) via setup.cfg (max-line-length = 88) while ruff ignores E501 via ruff.toml. Therefore, use '# noqa: E501' on lines that intentionally exceed 88 characters to satisfy flake8 without affecting ruff checks. This applies to Python files across the project (any .py) and is relevant for tests as well. Use sparingly and only where breaking lines is not feasible without hurting readability or functionality.
Applied to files:
openwisp_controller/config/migrations/0064_template_blueprint_cert_template_ca_and_more.pytests/openwisp2/sample_config/migrations/0010_template_blueprint_cert_template_ca_and_more.pyopenwisp_controller/pki/tests/test_api.pyopenwisp_controller/config/base/template.pyopenwisp_controller/config/tests/test_template.py
📚 Learning: 2026-01-12T22:27:40.078Z
Learnt from: nemesifier
Repo: openwisp/openwisp-controller PR: 1175
File: tests/openwisp2/sample_config/migrations/0008_whoisinfo_organizationconfigsettings_whois_enabled.py:18-67
Timestamp: 2026-01-12T22:27:40.078Z
Learning: In test migrations under tests/openwisp2/sample_config/migrations, verify scenarios where a swappable model (CONFIG_WHOISINFO_MODEL) is extended with extra fields (e.g., an additional 'details' field) to ensure compatibility and no errors when swapping to a custom implementation. This pattern helps confirm that extending AbstractWHOISInfo via a custom model works as intended.
Applied to files:
tests/openwisp2/sample_config/migrations/0010_template_blueprint_cert_template_ca_and_more.py
🔇 Additional comments (4)
openwisp_controller/pki/tests/test_api.py (1)
155-155: LGTM!Also applies to: 275-275
openwisp_controller/config/migrations/0064_template_blueprint_cert_template_ca_and_more.py (1)
12-16: LGTM!Also applies to: 19-44, 45-58, 59-74
tests/openwisp2/sample_config/migrations/0010_template_blueprint_cert_template_ca_and_more.py (1)
11-14: LGTM!Also applies to: 17-42, 43-56, 57-72
openwisp_controller/config/base/template.py (1)
25-29: LGTM!Also applies to: 62-83, 119-120
Migrations Check FailedHello @stktyagi, The CI failed because there are unapplied database migrations. Failure: Migrations check failed! Models' changes not migrated, please run './manage.py makemigrations' to solve the issue! Fix: ./manage.py makemigrations |
…s/1356-extend-abstract-template
Validate cert relations only inside the cert branch and Only coerce missing cert configs, not every falsy value. Fixes #1356
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
Added test for the validation branch that now skips ca / blueprint_cert checks for non-cert templates Fixes #1356
|
Coverage is 98.404% — issues/1356-extend-abstract-template into gsoc26-x509-certificate-generator-templates. No base build found for |
Fixed line too long flake error Fixes #1356
Updated test by joining the list of strings into one sentence. Fixes #1377
stktyagi
changed the title
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
openwisp_controller/config/apps.py (1)
93-102: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument why
manage_device_certsmust run beforetemplates_changed.The comment above
manage_vpn_clientsexplains VPN clients must be created or removed before self.config_model.templates_changed is evaluated, because the VpnClient context can influence the configuration checksum. The same rationale applies tomanage_device_certs(itsDeviceCertificaterows feedget_cert_context(), which affects the checksum), and it is correctly positioned beforetemplates_changed— but there's no comment explaining why, unlike its sibling. Without it, a future refactor could silently reorder these connects and break checksum correctness.📝 Suggested comment
+ # DeviceCertificate rows must be created or removed **before** + # self.config_model.templates_changed is evaluated, because the + # cert context can influence the configuration checksum. m2m_changed.connect( self.config_model.manage_device_certs, sender=self.config_model.templates.through, dispatch_uid="config.manage_device_certs", )🤖 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 `@openwisp_controller/config/apps.py` around lines 93 - 102, The `m2m_changed.connect` for `manage_device_certs` needs an explanatory comment like the nearby `manage_vpn_clients` block, since `DeviceCertificate` data is used by `get_cert_context()` and must be updated before `templates_changed` runs to keep the checksum correct. Add a brief note near the `manage_device_certs` connection in `config_model` setup explaining that it must execute before `templates_changed`, so future refactors do not reorder it and break checksum generation.
🤖 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 `@openwisp_controller/config/admin.py`:
- Around line 967-1011: The certificate-details queryset in
_add_certificate_details needs an explicit, deterministic order before slicing,
since the current [:51] truncation and has_more logic depend on stable row
ordering. Update the DeviceCertificate.objects.filter(...).select_related(...)
queryset to order by the same field used in Config.get_cert_context(),
specifically created, so the first 50 shown certificates are consistent across
requests and the “more” indicator is accurate.
In `@openwisp_controller/config/base/device_certificate.py`:
- Around line 12-13: Update the django-x509 dependency pin to a release that
supports custom oid extensions, since the current 1.4 branch tarball does not
handle the MAC_ADDRESS_OID and DEVICE_UUID_OID values used in
device_certificate.py. Adjust the dependency in requirements.txt to a version
with custom OID support, then verify the device certificate generation path
still works with those extension identifiers.
In `@openwisp_controller/config/base/template.py`:
- Around line 257-301: Add a regression test for the type-mutation branch in
_validate_cert_template_changes, since the existing deleted-instance coverage
does not hit current.type == "cert" and self.type != "cert". Create a test that
loads an active cert template, mutates its type away from "cert", and calls
clean() to assert a ValidationError is raised with an error on the type field.
In `@openwisp_controller/config/handlers.py`:
- Around line 98-107: The “active auto cert” queryset logic is duplicated in the
device certificate handling code and in the regeneration task, so extract the
shared filter into a single reusable API on DeviceCertificate. Add a
manager/classmethod such as DeviceCertificate.active_auto_certs_for(device) or
DeviceCertificate.objects.active_auto_certs_for(device) that encapsulates
config__device, auto_cert=True, cert__revoked=False, and template__type="cert",
then update both handlers.py and regenerate_device_certificates_task in tasks.py
to call that helper instead of repeating the filter inline.
In `@openwisp_controller/config/static/config/js/switcher.js`:
- Around line 47-63: The switcher logic contains a redundant show/hide branch
for auto_cert_field that duplicates the later conditional in the same handler.
Remove the առաջին duplicate if/else block and keep the unified val check that
both toggles auto_cert_field and updates auto_cert_label in switcher.js, using
the existing auto_cert_field and auto_cert_label symbols.
In `@openwisp_controller/config/tasks.py`:
- Around line 291-294: The exception handler in the regeneration notification
logic is redundant because `ImportError` is already covered by `Exception`.
Update the `except (ImportError, Exception) as e` block in the relevant task
function to catch only `Exception` while keeping the existing warning log
message with `device.name` and the exception details.
---
Outside diff comments:
In `@openwisp_controller/config/apps.py`:
- Around line 93-102: The `m2m_changed.connect` for `manage_device_certs` needs
an explanatory comment like the nearby `manage_vpn_clients` block, since
`DeviceCertificate` data is used by `get_cert_context()` and must be updated
before `templates_changed` runs to keep the checksum correct. Add a brief note
near the `manage_device_certs` connection in `config_model` setup explaining
that it must execute before `templates_changed`, so future refactors do not
reorder it and break checksum generation.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 08768b0a-73f9-4017-a7fc-c0025976cfcb
📒 Files selected for processing (33)
docs/developer/extending.rstdocs/index.rstdocs/user/certificate-templates.rstdocs/user/intro.rstdocs/user/rest-api.rstdocs/user/settings.rstdocs/user/templates.rstopenwisp_controller/config/admin.pyopenwisp_controller/config/api/serializers.pyopenwisp_controller/config/apps.pyopenwisp_controller/config/base/config.pyopenwisp_controller/config/base/device_certificate.pyopenwisp_controller/config/base/template.pyopenwisp_controller/config/handlers.pyopenwisp_controller/config/migrations/0064_template_blueprint_cert_template_ca_and_more.pyopenwisp_controller/config/migrations/0065_merge_20260702_1946.pyopenwisp_controller/config/models.pyopenwisp_controller/config/settings.pyopenwisp_controller/config/static/config/js/switcher.jsopenwisp_controller/config/tasks.pyopenwisp_controller/config/templates/admin/config/device/change_form.htmlopenwisp_controller/config/templates/admin/config/device_certificates_table.htmlopenwisp_controller/config/tests/test_admin.pyopenwisp_controller/config/tests/test_api.pyopenwisp_controller/config/tests/test_config.pyopenwisp_controller/config/tests/test_device.pyopenwisp_controller/config/tests/test_selenium.pyopenwisp_controller/config/tests/test_template.pyopenwisp_controller/pki/tests/test_api.pytests/openwisp2/sample_config/migrations/0010_template_blueprint_cert_template_ca_and_more.pytests/openwisp2/sample_config/migrations/0011_merge_20260702_1946.pytests/openwisp2/sample_config/models.pytests/openwisp2/settings.py
📜 Review details
⚠️ CI failures not shown inline (11)
GitHub Actions: OpenWISP Controller CI Build / 7_Python==3.12 django~=4.2.0.txt: [feature] Introduced standalone certificate templates and device bindings
Conclusion: failure
-cp312-cp312-manylinux2014_x86_64.manylinux_2_17_x86_64.manylinux_2_28_x86_64.whl.metadata (5.3 kB)
Collecting propcache>=0.2.0 (from aiohttp<4.0.0,>=3.6.2->geoip2<6.0.0,>=5.2.0->openwisp-controller==1.3a0)
Using cached propcache-0.5.2-cp312-cp312-manylinux2014_x86_64.manylinux_2_17_x86_64.manylinux_2_28_x86_64.whl.metadata (16 kB)
Requirement already satisfied: typing_extensions>=4.4 in /opt/hostedtoolcache/Python/3.12.13/x64/lib/python3.12/site-packages (from aiohttp<4.0.0,>=3.6.2->geoip2<6.0.0,>=5.2.0->openwisp-controller==1.3a0) (4.16.0)
Collecting yarl<2.0,>=1.17.0 (from aiohttp<4.0.0,>=3.6.2->geoip2<6.0.0,>=5.2.0->openwisp-controller==1.3a0)
Using cached yarl-1.24.2-cp312-cp312-manylinux2014_x86_64.manylinux_2_17_x86_64.manylinux_2_28_x86_64.whl.metadata (94 kB)
Collecting geographiclib<3,>=1.52 (from geopy~=2.4.1->django-loci @ https://github.com/openwisp/django-loci/archive/refs/heads/1.3.tar.gz->openwisp-controller==1.3a0)
Using cached geographiclib-2.1-py3-none-any.whl.metadata (1.6 kB)
Requirement already satisfied: MarkupSafe>=2.0 in /opt/hostedtoolcache/Python/3.12.13/x64/lib/python3.12/site-packages (from jinja2~=3.1.6->netjsonconfig @ https://github.com/openwisp/netjsonconfig/archive/refs/heads/1.3.tar.gz->openwisp-controller==1.3a0) (3.0.3)
Collecting jsonschema-specifications>=2023.03.6 (from jsonschema~=4.26.0->netjsonconfig @ https://github.com/openwisp/netjsonconfig/archive/refs/heads/1.3.tar.gz->openwisp-controller==1.3a0)
Using cached jsonschema_specifications-2025.9.1-py3-none-any.whl.metadata (2.9 kB)
Collecting referencing>=0.28.4 (from jsonschema~=4.26.0->netjsonconfig @ https://github.com/openwisp/netjsonconfig/archive/refs/heads/1.3.tar.gz->openwisp-controller==1.3a0)
Using cached referencing-0.37.0-py3-none-any.whl.metadata (2.8 kB)
Collecting rpds-py>=0.25.0 (from jsonschema~=4.26.0->netjsonconfig @ https://github.com/openwisp/netjsonconfig/archive/refs/heads/1.3.tar.gz->openwisp-controller==1.3a0)
Using cac...
GitHub Actions: OpenWISP Controller CI Build / 1_Python==3.11 django~=5.1.0.txt: [feature] Introduced standalone certificate templates and device bindings
Conclusion: failure
x_2_28_x86_64.whl.metadata (8.3 kB)
Collecting maxminddb<4.0.0,>=3.0.0 (from geoip2<6.0.0,>=5.2.0->openwisp-controller==1.3a0)
Using cached maxminddb-3.1.1-cp311-cp311-manylinux2014_x86_64.manylinux_2_17_x86_64.manylinux_2_28_x86_64.whl.metadata (5.3 kB)
Collecting aiohappyeyeballs>=2.5.0 (from aiohttp<4.0.0,>=3.6.2->geoip2<6.0.0,>=5.2.0->openwisp-controller==1.3a0)
Downloading aiohappyeyeballs-2.7.1-py3-none-any.whl.metadata (5.9 kB)
Collecting aiosignal>=1.4.0 (from aiohttp<4.0.0,>=3.6.2->geoip2<6.0.0,>=5.2.0->openwisp-controller==1.3a0)
Using cached aiosignal-1.4.0-py3-none-any.whl.metadata (3.7 kB)
Requirement already satisfied: attrs>=17.3.0 in /opt/hostedtoolcache/Python/3.11.15/x64/lib/python3.11/site-packages (from aiohttp<4.0.0,>=3.6.2->geoip2<6.0.0,>=5.2.0->openwisp-controller==1.3a0) (26.1.0)
Collecting frozenlist>=1.1.1 (from aiohttp<4.0.0,>=3.6.2->geoip2<6.0.0,>=5.2.0->openwisp-controller==1.3a0)
Using cached frozenlist-1.8.0-cp311-cp311-manylinux1_x86_64.manylinux_2_28_x86_64.manylinux_2_5_x86_64.whl.metadata (20 kB)
Collecting multidict<7.0,>=4.5 (from aiohttp<4.0.0,>=3.6.2->geoip2<6.0.0,>=5.2.0->openwisp-controller==1.3a0)
Using cached multidict-6.7.1-cp311-cp311-manylinux2014_x86_64.manylinux_2_17_x86_64.manylinux_2_28_x86_64.whl.metadata (5.3 kB)
Collecting propcache>=0.2.0 (from aiohttp<4.0.0,>=3.6.2->geoip2<6.0.0,>=5.2.0->openwisp-controller==1.3a0)
Using cached propcache-0.5.2-cp311-cp311-manylinux2014_x86_64.manylinux_2_17_x86_64.manylinux_2_28_x86_64.whl.metadata (16 kB)
Requirement already satisfied: typing_extensions>=4.4 in /opt/hostedtoolcache/Python/3.11.15/x64/lib/python3.11/site-packages (from aiohttp<4.0.0,>=3.6.2->geoip2<6.0.0,>=5.2.0->openwisp-controller==1.3a0) (4.16.0)
Collecting yarl<2.0,>=1.17.0 (from aiohttp<4.0.0,>=3.6.2->geoip2<6.0.0,>=5.2.0->openwisp-controller==1.3a0)
Using cached yarl-1.24.2-cp311-cp311-manylinux2014_x86_64.manylinux_2_17_x86_64.manylinux_2_28_x86_64.whl.metadata (94 kB)
Co...
GitHub Actions: OpenWISP Controller CI Build / 8_Python==3.10 django~=5.2.0.txt: [feature] Introduced standalone certificate templates and device bindings
Conclusion: failure
41021-py3-none-any.whl.metadata (5.5 kB)
Collecting tablib>=3.7.0 (from django-import-export~=4.4.1->openwisp-controller==1.3a0)
Using cached tablib-3.9.0-py3-none-any.whl.metadata (3.8 kB)
Requirement already satisfied: rjsmin<2.0.0,>=1.2.0 in /opt/hostedtoolcache/Python/3.10.20/x64/lib/python3.10/site-packages (from django-minify-compress-staticfiles~=1.1.1->openwisp-utils @ https://github.com/openwisp/openwisp-utils/archive/refs/heads/1.3.tar.gz->openwisp-utils[celery,channels] @ https://github.com/openwisp/openwisp-utils/archive/refs/heads/1.3.tar.gz->openwisp-controller==1.3a0) (1.2.5)
Requirement already satisfied: rcssmin<2.0.0,>=1.1.0 in /opt/hostedtoolcache/Python/3.10.20/x64/lib/python3.10/site-packages (from django-minify-compress-staticfiles~=1.1.1->openwisp-utils @ https://github.com/openwisp/openwisp-utils/archive/refs/heads/1.3.tar.gz->openwisp-utils[celery,channels] @ https://github.com/openwisp/openwisp-utils/archive/refs/heads/1.3.tar.gz->openwisp-controller==1.3a0) (1.2.2)
Requirement already satisfied: brotli<2.0.0,>=1.2.0 in /opt/hostedtoolcache/Python/3.10.20/x64/lib/python3.10/site-packages (from django-minify-compress-staticfiles~=1.1.1->openwisp-utils @ https://github.com/openwisp/openwisp-utils/archive/refs/heads/1.3.tar.gz->openwisp-utils[celery,channels] @ https://github.com/openwisp/openwisp-utils/archive/refs/heads/1.3.tar.gz->openwisp-controller==1.3a0) (1.2.0)
Collecting inflection>=0.3.1 (from drf-yasg<1.22.0,>=1.21.14->openwisp-utils @ https://github.com/openwisp/openwisp-utils/archive/refs/heads/1.3.tar.gz->openwisp-utils[celery,channels] @ https://github.com/openwisp/openwisp-utils/archive/refs/heads/1.3.tar.gz->openwisp-controller==1.3a0)
Using cached inflection-0.5.1-py2.py3-none-any.whl.metadata (1.7 kB)
Collecting pytz>=2021.1 (from drf-yasg<1.22.0,>=1.21.14->openwisp-utils @ https://github.com/openwisp/openwisp-utils/archive/refs/heads/1.3.tar.gz->openwisp-utils[celery,channels] @ https://github.com/openwisp/op...
GitHub Actions: OpenWISP Controller CI Build / 9_Python==3.13 django~=5.2.0.txt: [feature] Introduced standalone certificate templates and device bindings
Conclusion: failure
nwisp-utils/archive/refs/heads/1.3.tar.gz->openwisp-utils[celery,channels] @ https://github.com/openwisp/openwisp-utils/archive/refs/heads/1.3.tar.gz->openwisp-controller==1.3a0)
Using cached pytz-2026.2-py2.py3-none-any.whl.metadata (22 kB)
Requirement already satisfied: pyyaml>=5.1 in /opt/hostedtoolcache/Python/3.13.14/x64/lib/python3.13/site-packages (from drf-yasg<1.22.0,>=1.21.14->openwisp-utils @ https://github.com/openwisp/openwisp-utils/archive/refs/heads/1.3.tar.gz->openwisp-utils[celery,channels] @ https://github.com/openwisp/openwisp-utils/archive/refs/heads/1.3.tar.gz->openwisp-controller==1.3a0) (6.0.3)
Collecting uritemplate>=3.0.0 (from drf-yasg<1.22.0,>=1.21.14->openwisp-utils @ https://github.com/openwisp/openwisp-utils/archive/refs/heads/1.3.tar.gz->openwisp-utils[celery,channels] @ https://github.com/openwisp/openwisp-utils/archive/refs/heads/1.3.tar.gz->openwisp-controller==1.3a0)
Using cached uritemplate-4.2.0-py3-none-any.whl.metadata (2.6 kB)
Collecting aiohttp<4.0.0,>=3.6.2 (from geoip2<6.0.0,>=5.2.0->openwisp-controller==1.3a0)
Using cached aiohttp-3.14.1-cp313-cp313-manylinux2014_x86_64.manylinux_2_17_x86_64.manylinux_2_28_x86_64.whl.metadata (8.3 kB)
Collecting maxminddb<4.0.0,>=3.0.0 (from geoip2<6.0.0,>=5.2.0->openwisp-controller==1.3a0)
Using cached maxminddb-3.1.1-cp313-cp313-manylinux2014_x86_64.manylinux_2_17_x86_64.manylinux_2_28_x86_64.whl.metadata (5.3 kB)
Collecting aiohappyeyeballs>=2.5.0 (from aiohttp<4.0.0,>=3.6.2->geoip2<6.0.0,>=5.2.0->openwisp-controller==1.3a0)
Downloading aiohappyeyeballs-2.7.1-py3-none-any.whl.metadata (5.9 kB)
Collecting aiosignal>=1.4.0 (from aiohttp<4.0.0,>=3.6.2->geoip2<6.0.0,>=5.2.0->openwisp-controller==1.3a0)
Using cached aiosignal-1.4.0-py3-none-any.whl.metadata (3.7 kB)
Requirement already satisfied: attrs>=17.3.0 in /opt/hostedtoolcache/Python/3.13.14/x64/lib/python3.13/site-packages (from aiohttp<4.0.0,>=3.6.2->geoip2<6.0.0,>=5.2.0->openwisp-controller==1.3a0) (26.1...
GitHub Actions: OpenWISP Controller CI Build / 11_Python==3.13 django~=5.1.0.txt: [feature] Introduced standalone certificate templates and device bindings
Conclusion: failure
s/archive/refs/heads/1.3.tar.gz->openwisp-utils[celery,channels] @ https://github.com/openwisp/openwisp-utils/archive/refs/heads/1.3.tar.gz->openwisp-controller==1.3a0)
Using cached uritemplate-4.2.0-py3-none-any.whl.metadata (2.6 kB)
Collecting aiohttp<4.0.0,>=3.6.2 (from geoip2<6.0.0,>=5.2.0->openwisp-controller==1.3a0)
Using cached aiohttp-3.14.1-cp313-cp313-manylinux2014_x86_64.manylinux_2_17_x86_64.manylinux_2_28_x86_64.whl.metadata (8.3 kB)
Collecting maxminddb<4.0.0,>=3.0.0 (from geoip2<6.0.0,>=5.2.0->openwisp-controller==1.3a0)
Using cached maxminddb-3.1.1-cp313-cp313-manylinux2014_x86_64.manylinux_2_17_x86_64.manylinux_2_28_x86_64.whl.metadata (5.3 kB)
Collecting aiohappyeyeballs>=2.5.0 (from aiohttp<4.0.0,>=3.6.2->geoip2<6.0.0,>=5.2.0->openwisp-controller==1.3a0)
Downloading aiohappyeyeballs-2.7.1-py3-none-any.whl.metadata (5.9 kB)
Collecting aiosignal>=1.4.0 (from aiohttp<4.0.0,>=3.6.2->geoip2<6.0.0,>=5.2.0->openwisp-controller==1.3a0)
Using cached aiosignal-1.4.0-py3-none-any.whl.metadata (3.7 kB)
Requirement already satisfied: attrs>=17.3.0 in /opt/hostedtoolcache/Python/3.13.14/x64/lib/python3.13/site-packages (from aiohttp<4.0.0,>=3.6.2->geoip2<6.0.0,>=5.2.0->openwisp-controller==1.3a0) (26.1.0)
Collecting frozenlist>=1.1.1 (from aiohttp<4.0.0,>=3.6.2->geoip2<6.0.0,>=5.2.0->openwisp-controller==1.3a0)
Using cached frozenlist-1.8.0-cp313-cp313-manylinux1_x86_64.manylinux_2_28_x86_64.manylinux_2_5_x86_64.whl.metadata (20 kB)
Collecting multidict<7.0,>=4.5 (from aiohttp<4.0.0,>=3.6.2->geoip2<6.0.0,>=5.2.0->openwisp-controller==1.3a0)
Using cached multidict-6.7.1-cp313-cp313-manylinux2014_x86_64.manylinux_2_17_x86_64.manylinux_2_28_x86_64.whl.metadata (5.3 kB)
Collecting propcache>=0.2.0 (from aiohttp<4.0.0,>=3.6.2->geoip2<6.0.0,>=5.2.0->openwisp-controller==1.3a0)
Using cached propcache-0.5.2-cp313-cp313-manylinux2014_x86_64.manylinux_2_17_x86_64.manylinux_2_28_x86_64.whl.metadata (16 kB)
Collecting yarl<2.0,>=1.17.0 (...
GitHub Actions: OpenWISP Controller CI Build / 3_Python==3.11 django~=4.2.0.txt: [feature] Introduced standalone certificate templates and device bindings
Conclusion: failure
2.0->openwisp-controller==1.3a0)
Using cached aiosignal-1.4.0-py3-none-any.whl.metadata (3.7 kB)
Requirement already satisfied: attrs>=17.3.0 in /opt/hostedtoolcache/Python/3.11.15/x64/lib/python3.11/site-packages (from aiohttp<4.0.0,>=3.6.2->geoip2<6.0.0,>=5.2.0->openwisp-controller==1.3a0) (26.1.0)
Collecting frozenlist>=1.1.1 (from aiohttp<4.0.0,>=3.6.2->geoip2<6.0.0,>=5.2.0->openwisp-controller==1.3a0)
Using cached frozenlist-1.8.0-cp311-cp311-manylinux1_x86_64.manylinux_2_28_x86_64.manylinux_2_5_x86_64.whl.metadata (20 kB)
Collecting multidict<7.0,>=4.5 (from aiohttp<4.0.0,>=3.6.2->geoip2<6.0.0,>=5.2.0->openwisp-controller==1.3a0)
Using cached multidict-6.7.1-cp311-cp311-manylinux2014_x86_64.manylinux_2_17_x86_64.manylinux_2_28_x86_64.whl.metadata (5.3 kB)
Collecting propcache>=0.2.0 (from aiohttp<4.0.0,>=3.6.2->geoip2<6.0.0,>=5.2.0->openwisp-controller==1.3a0)
Using cached propcache-0.5.2-cp311-cp311-manylinux2014_x86_64.manylinux_2_17_x86_64.manylinux_2_28_x86_64.whl.metadata (16 kB)
Requirement already satisfied: typing_extensions>=4.4 in /opt/hostedtoolcache/Python/3.11.15/x64/lib/python3.11/site-packages (from aiohttp<4.0.0,>=3.6.2->geoip2<6.0.0,>=5.2.0->openwisp-controller==1.3a0) (4.16.0)
Collecting yarl<2.0,>=1.17.0 (from aiohttp<4.0.0,>=3.6.2->geoip2<6.0.0,>=5.2.0->openwisp-controller==1.3a0)
Using cached yarl-1.24.2-cp311-cp311-manylinux2014_x86_64.manylinux_2_17_x86_64.manylinux_2_28_x86_64.whl.metadata (94 kB)
Collecting geographiclib<3,>=1.52 (from geopy~=2.4.1->django-loci @ https://github.com/openwisp/django-loci/archive/refs/heads/1.3.tar.gz->openwisp-controller==1.3a0)
Using cached geographiclib-2.1-py3-none-any.whl.metadata (1.6 kB)
Requirement already satisfied: MarkupSafe>=2.0 in /opt/hostedtoolcache/Python/3.11.15/x64/lib/python3.11/site-packages (from jinja2~=3.1.6->netjsonconfig @ https://github.com/openwisp/netjsonconfig/archive/refs/heads/1.3.tar.gz->openwisp-controller==1.3a0) (3.0.3)
Collecting jsonschema...
GitHub Actions: OpenWISP Controller CI Build / 10_Python==3.11 django~=5.2.0.txt: [feature] Introduced standalone certificate templates and device bindings
Conclusion: failure
f-yasg<1.22.0,>=1.21.14->openwisp-utils @ https://github.com/openwisp/openwisp-utils/archive/refs/heads/1.3.tar.gz->openwisp-utils[celery,channels] @ https://github.com/openwisp/openwisp-utils/archive/refs/heads/1.3.tar.gz->openwisp-controller==1.3a0)
Using cached inflection-0.5.1-py2.py3-none-any.whl.metadata (1.7 kB)
Collecting pytz>=2021.1 (from drf-yasg<1.22.0,>=1.21.14->openwisp-utils @ https://github.com/openwisp/openwisp-utils/archive/refs/heads/1.3.tar.gz->openwisp-utils[celery,channels] @ https://github.com/openwisp/openwisp-utils/archive/refs/heads/1.3.tar.gz->openwisp-controller==1.3a0)
Using cached pytz-2026.2-py2.py3-none-any.whl.metadata (22 kB)
Requirement already satisfied: pyyaml>=5.1 in /opt/hostedtoolcache/Python/3.11.15/x64/lib/python3.11/site-packages (from drf-yasg<1.22.0,>=1.21.14->openwisp-utils @ https://github.com/openwisp/openwisp-utils/archive/refs/heads/1.3.tar.gz->openwisp-utils[celery,channels] @ https://github.com/openwisp/openwisp-utils/archive/refs/heads/1.3.tar.gz->openwisp-controller==1.3a0) (6.0.3)
Collecting uritemplate>=3.0.0 (from drf-yasg<1.22.0,>=1.21.14->openwisp-utils @ https://github.com/openwisp/openwisp-utils/archive/refs/heads/1.3.tar.gz->openwisp-utils[celery,channels] @ https://github.com/openwisp/openwisp-utils/archive/refs/heads/1.3.tar.gz->openwisp-controller==1.3a0)
Using cached uritemplate-4.2.0-py3-none-any.whl.metadata (2.6 kB)
Collecting aiohttp<4.0.0,>=3.6.2 (from geoip2<6.0.0,>=5.2.0->openwisp-controller==1.3a0)
Using cached aiohttp-3.14.1-cp311-cp311-manylinux2014_x86_64.manylinux_2_17_x86_64.manylinux_2_28_x86_64.whl.metadata (8.3 kB)
Collecting maxminddb<4.0.0,>=3.0.0 (from geoip2<6.0.0,>=5.2.0->openwisp-controller==1.3a0)
Using cached maxminddb-3.1.1-cp311-cp311-manylinux2014_x86_64.manylinux_2_17_x86_64.manylinux_2_28_x86_64.whl.metadata (5.3 kB)
Collecting aiohappyeyeballs>=2.5.0 (from aiohttp<4.0.0,>=3.6.2->geoip2<6.0.0,>=5.2.0->openwisp-controller==1.3a0)
Downloading aioha...
GitHub Actions: OpenWISP Controller CI Build / 6_Python==3.12 django~=5.2.0.txt: [feature] Introduced standalone certificate templates and device bindings
Conclusion: failure
1.14->openwisp-utils @ https://github.com/openwisp/openwisp-utils/archive/refs/heads/1.3.tar.gz->openwisp-utils[celery,channels] @ https://github.com/openwisp/openwisp-utils/archive/refs/heads/1.3.tar.gz->openwisp-controller==1.3a0) (6.0.3)
Collecting uritemplate>=3.0.0 (from drf-yasg<1.22.0,>=1.21.14->openwisp-utils @ https://github.com/openwisp/openwisp-utils/archive/refs/heads/1.3.tar.gz->openwisp-utils[celery,channels] @ https://github.com/openwisp/openwisp-utils/archive/refs/heads/1.3.tar.gz->openwisp-controller==1.3a0)
Using cached uritemplate-4.2.0-py3-none-any.whl.metadata (2.6 kB)
Collecting aiohttp<4.0.0,>=3.6.2 (from geoip2<6.0.0,>=5.2.0->openwisp-controller==1.3a0)
Using cached aiohttp-3.14.1-cp312-cp312-manylinux2014_x86_64.manylinux_2_17_x86_64.manylinux_2_28_x86_64.whl.metadata (8.3 kB)
Collecting maxminddb<4.0.0,>=3.0.0 (from geoip2<6.0.0,>=5.2.0->openwisp-controller==1.3a0)
Using cached maxminddb-3.1.1-cp312-cp312-manylinux2014_x86_64.manylinux_2_17_x86_64.manylinux_2_28_x86_64.whl.metadata (5.3 kB)
Collecting aiohappyeyeballs>=2.5.0 (from aiohttp<4.0.0,>=3.6.2->geoip2<6.0.0,>=5.2.0->openwisp-controller==1.3a0)
Downloading aiohappyeyeballs-2.7.1-py3-none-any.whl.metadata (5.9 kB)
Collecting aiosignal>=1.4.0 (from aiohttp<4.0.0,>=3.6.2->geoip2<6.0.0,>=5.2.0->openwisp-controller==1.3a0)
Using cached aiosignal-1.4.0-py3-none-any.whl.metadata (3.7 kB)
Requirement already satisfied: attrs>=17.3.0 in /opt/hostedtoolcache/Python/3.12.13/x64/lib/python3.12/site-packages (from aiohttp<4.0.0,>=3.6.2->geoip2<6.0.0,>=5.2.0->openwisp-controller==1.3a0) (26.1.0)
Collecting frozenlist>=1.1.1 (from aiohttp<4.0.0,>=3.6.2->geoip2<6.0.0,>=5.2.0->openwisp-controller==1.3a0)
Using cached frozenlist-1.8.0-cp312-cp312-manylinux1_x86_64.manylinux_2_28_x86_64.manylinux_2_5_x86_64.whl.metadata (20 kB)
Collecting multidict<7.0,>=4.5 (from aiohttp<4.0.0,>=3.6.2->geoip2<6.0.0,>=5.2.0->openwisp-controller==1.3a0)
Using cached multidict-6.7.1-cp312...
GitHub Actions: OpenWISP Controller CI Build / 2_Python==3.10 django~=4.2.0.txt: [feature] Introduced standalone certificate templates and device bindings
Conclusion: failure
openwisp-utils/archive/refs/heads/1.3.tar.gz->openwisp-controller==1.3a0)
Using cached uritemplate-4.2.0-py3-none-any.whl.metadata (2.6 kB)
Collecting aiohttp<4.0.0,>=3.6.2 (from geoip2<6.0.0,>=5.2.0->openwisp-controller==1.3a0)
Using cached aiohttp-3.14.1-cp310-cp310-manylinux2014_x86_64.manylinux_2_17_x86_64.manylinux_2_28_x86_64.whl.metadata (8.3 kB)
Collecting maxminddb<4.0.0,>=3.0.0 (from geoip2<6.0.0,>=5.2.0->openwisp-controller==1.3a0)
Using cached maxminddb-3.1.1-cp310-cp310-manylinux2014_x86_64.manylinux_2_17_x86_64.manylinux_2_28_x86_64.whl.metadata (5.3 kB)
Collecting aiohappyeyeballs>=2.5.0 (from aiohttp<4.0.0,>=3.6.2->geoip2<6.0.0,>=5.2.0->openwisp-controller==1.3a0)
Downloading aiohappyeyeballs-2.7.1-py3-none-any.whl.metadata (5.9 kB)
Collecting aiosignal>=1.4.0 (from aiohttp<4.0.0,>=3.6.2->geoip2<6.0.0,>=5.2.0->openwisp-controller==1.3a0)
Using cached aiosignal-1.4.0-py3-none-any.whl.metadata (3.7 kB)
Requirement already satisfied: async-timeout<6.0,>=4.0 in /opt/hostedtoolcache/Python/3.10.20/x64/lib/python3.10/site-packages (from aiohttp<4.0.0,>=3.6.2->geoip2<6.0.0,>=5.2.0->openwisp-controller==1.3a0) (5.0.1)
Requirement already satisfied: attrs>=17.3.0 in /opt/hostedtoolcache/Python/3.10.20/x64/lib/python3.10/site-packages (from aiohttp<4.0.0,>=3.6.2->geoip2<6.0.0,>=5.2.0->openwisp-controller==1.3a0) (26.1.0)
Collecting frozenlist>=1.1.1 (from aiohttp<4.0.0,>=3.6.2->geoip2<6.0.0,>=5.2.0->openwisp-controller==1.3a0)
Using cached frozenlist-1.8.0-cp310-cp310-manylinux1_x86_64.manylinux_2_28_x86_64.manylinux_2_5_x86_64.whl.metadata (20 kB)
Collecting multidict<7.0,>=4.5 (from aiohttp<4.0.0,>=3.6.2->geoip2<6.0.0,>=5.2.0->openwisp-controller==1.3a0)
Using cached multidict-6.7.1-cp310-cp310-manylinux2014_x86_64.manylinux_2_17_x86_64.manylinux_2_28_x86_64.whl.metadata (5.3 kB)
Collecting propcache>=0.2.0 (from aiohttp<4.0.0,>=3.6.2->geoip2<6.0.0,>=5.2.0->openwisp-controller==1.3a0)
Using cached propcache-0.5.2-cp310-cp3...
GitHub Actions: OpenWISP Controller CI Build / 5_Python==3.10 django~=5.1.0.txt: [feature] Introduced standalone certificate templates and device bindings
Conclusion: failure
1.2.2)
Requirement already satisfied: brotli<2.0.0,>=1.2.0 in /opt/hostedtoolcache/Python/3.10.20/x64/lib/python3.10/site-packages (from django-minify-compress-staticfiles~=1.1.1->openwisp-utils @ https://github.com/openwisp/openwisp-utils/archive/refs/heads/1.3.tar.gz->openwisp-utils[celery,channels] @ https://github.com/openwisp/openwisp-utils/archive/refs/heads/1.3.tar.gz->openwisp-controller==1.3a0) (1.2.0)
Collecting inflection>=0.3.1 (from drf-yasg<1.22.0,>=1.21.14->openwisp-utils @ https://github.com/openwisp/openwisp-utils/archive/refs/heads/1.3.tar.gz->openwisp-utils[celery,channels] @ https://github.com/openwisp/openwisp-utils/archive/refs/heads/1.3.tar.gz->openwisp-controller==1.3a0)
Using cached inflection-0.5.1-py2.py3-none-any.whl.metadata (1.7 kB)
Collecting pytz>=2021.1 (from drf-yasg<1.22.0,>=1.21.14->openwisp-utils @ https://github.com/openwisp/openwisp-utils/archive/refs/heads/1.3.tar.gz->openwisp-utils[celery,channels] @ https://github.com/openwisp/openwisp-utils/archive/refs/heads/1.3.tar.gz->openwisp-controller==1.3a0)
Using cached pytz-2026.2-py2.py3-none-any.whl.metadata (22 kB)
Requirement already satisfied: pyyaml>=5.1 in /opt/hostedtoolcache/Python/3.10.20/x64/lib/python3.10/site-packages (from drf-yasg<1.22.0,>=1.21.14->openwisp-utils @ https://github.com/openwisp/openwisp-utils/archive/refs/heads/1.3.tar.gz->openwisp-utils[celery,channels] @ https://github.com/openwisp/openwisp-utils/archive/refs/heads/1.3.tar.gz->openwisp-controller==1.3a0) (6.0.3)
Collecting uritemplate>=3.0.0 (from drf-yasg<1.22.0,>=1.21.14->openwisp-utils @ https://github.com/openwisp/openwisp-utils/archive/refs/heads/1.3.tar.gz->openwisp-utils[celery,channels] @ https://github.com/openwisp/openwisp-utils/archive/refs/heads/1.3.tar.gz->openwisp-controller==1.3a0)
Using cached uritemplate-4.2.0-py3-none-any.whl.metadata (2.6 kB)
Collecting aiohttp<4.0.0,>=3.6.2 (from geoip2<6.0.0,>=5.2.0->openwisp-controller==1.3a0)
Using cached aiohttp-3.14.1-cp3...
GitHub Actions: OpenWISP Controller CI Build / 4_Python==3.12 django~=5.1.0.txt: [feature] Introduced standalone certificate templates and device bindings
Conclusion: failure
oller.pki.tests.test_api.TestPkiApi.test_cert_patch_api)�[0m
�[33;1m (0.47s)�[0m test_cert_patch_api
�[33;1m(openwisp_controller.pki.tests.test_api.TestPkiApi.test_cert_post_api)�[0m
�[33;1m (0.46s)�[0m test_cert_post_api
�[33;1m(openwisp_controller.pki.tests.test_api.TestPkiApi.test_cert_post_with_date_none)�[0m
�[33;1m (0.50s)�[0m test_cert_post_with_date_none
�[33;1m(openwisp_controller.pki.tests.test_api.TestPkiApi.test_cert_post_with_extensions_field)�[0m
�[33;1m (0.50s)�[0m test_cert_post_with_extensions_field
�[33;1m(openwisp_controller.pki.tests.test_api.TestPkiApi.test_cert_put_api)�[0m
�[33;1m (0.40s)�[0m test_cert_put_api
�[33;1m(openwisp_controller.pki.tests.test_api.TestPkiApi.test_crl_download_api)�[0m
�[33;1m (0.43s)�[0m test_crl_download_api
�[33;1m(openwisp_controller.pki.tests.test_api.TestPkiApi.test_import_cert_post_api)�[0m
�[33;1m (0.41s)�[0m test_import_cert_post_api
�[33;1m(openwisp_controller.pki.tests.test_api.TestPkiApi.test_post_cert_renew_api)�[0m
�[33;1m (0.52s)�[0m test_post_cert_renew_api
�[33;1m(openwisp_controller.pki.tests.test_api.TestPkiApi.test_post_cert_revoke_api)�[0m
�[33;1m (0.47s)�[0m test_post_cert_revoke_api
�[33;1m(openwisp_controller.pki.tests.test_models.TestModels.test_ca_creation_with_org)�[0m
�[33;1m (0.30s)�[0m test_ca_creation_with_org
�[33;1m(openwisp_controller.pki.tests.test_models.TestModels.test_ca_creation_without_org)�[0m
�[33;1m (0.30s)�[0m test_ca_creation_without_org
�[33;1m(openwisp_controller.pki.tests.test_models.TestModels.test_cert_and_ca_different_organization)�[0m
�[33;1m (0.32s)�[0m test_cert_and_ca_different_organization
�[33;1m(openwisp_controller.pki.tests.test_models.TestModels.test_cert_creation)�[0m
�[33;1m (0.43s)�[0m test_cert_creation
�[33;1m(openwisp_controller.pki.tests.test_models.TestModels.test_crl_view)�[0m
�[33;1m (0.36s)�[0m test_crl_view
�[33;1m(openwisp_controller.pki.tests.test_models.TestModels.test_unique_together_org_none)�[0m
�...
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Mark user-facing strings for translation with Django i18n helpers in Django code
Avoid unnecessary blank lines inside function and method bodies
Be careful with authentication, authorization, queryset filtering, serializers, admin behavior, cache invalidation, signals, Celery tasks, and websocket updates in Django code
Preserve validation around templates, VPN/PKI material, SSH credentials, device commands, uploaded files, URLs, and subnet/IP data
Write comments and docstrings only when they explain why code is shaped a certain way, placing them before the relevant code block instead of scattering them inside it
Files:
openwisp_controller/config/migrations/0065_merge_20260702_1946.pyopenwisp_controller/config/settings.pytests/openwisp2/sample_config/migrations/0011_merge_20260702_1946.pytests/openwisp2/settings.pyopenwisp_controller/config/apps.pytests/openwisp2/sample_config/models.pyopenwisp_controller/config/tests/test_config.pyopenwisp_controller/config/models.pyopenwisp_controller/pki/tests/test_api.pyopenwisp_controller/config/tests/test_selenium.pyopenwisp_controller/config/tests/test_api.pytests/openwisp2/sample_config/migrations/0010_template_blueprint_cert_template_ca_and_more.pyopenwisp_controller/config/handlers.pyopenwisp_controller/config/base/config.pyopenwisp_controller/config/migrations/0064_template_blueprint_cert_template_ca_and_more.pyopenwisp_controller/config/api/serializers.pyopenwisp_controller/config/tasks.pyopenwisp_controller/config/tests/test_admin.pyopenwisp_controller/config/base/device_certificate.pyopenwisp_controller/config/admin.pyopenwisp_controller/config/base/template.pyopenwisp_controller/config/tests/test_template.pyopenwisp_controller/config/tests/test_device.py
**/*.{py,html}
📄 CodeRabbit inference engine (Custom checks)
For Django pull requests, ensure all user-facing strings are marked as translatable using the Django i18n framework.
Files:
openwisp_controller/config/migrations/0065_merge_20260702_1946.pyopenwisp_controller/config/settings.pyopenwisp_controller/config/templates/admin/config/device/change_form.htmltests/openwisp2/sample_config/migrations/0011_merge_20260702_1946.pyopenwisp_controller/config/templates/admin/config/device_certificates_table.htmltests/openwisp2/settings.pyopenwisp_controller/config/apps.pytests/openwisp2/sample_config/models.pyopenwisp_controller/config/tests/test_config.pyopenwisp_controller/config/models.pyopenwisp_controller/pki/tests/test_api.pyopenwisp_controller/config/tests/test_selenium.pyopenwisp_controller/config/tests/test_api.pytests/openwisp2/sample_config/migrations/0010_template_blueprint_cert_template_ca_and_more.pyopenwisp_controller/config/handlers.pyopenwisp_controller/config/base/config.pyopenwisp_controller/config/migrations/0064_template_blueprint_cert_template_ca_and_more.pyopenwisp_controller/config/api/serializers.pyopenwisp_controller/config/tasks.pyopenwisp_controller/config/tests/test_admin.pyopenwisp_controller/config/base/device_certificate.pyopenwisp_controller/config/admin.pyopenwisp_controller/config/base/template.pyopenwisp_controller/config/tests/test_template.pyopenwisp_controller/config/tests/test_device.py
🧠 Learnings (9)
📚 Learning: 2026-01-15T15:05:49.557Z
Learnt from: DragnEmperor
Repo: openwisp/openwisp-controller PR: 1175
File: openwisp_controller/config/management/commands/clear_last_ip.py:38-42
Timestamp: 2026-01-15T15:05:49.557Z
Learning: In Django projects, when using select_related() to traverse relations (for example, select_related("organization__config_settings")), the traversed relation must not be deferred. If you also use .only() in the same query, include the relation name or FK field (e.g., "organization" or "organization_id") in the .only() list to avoid the error "Field X cannot be both deferred and traversed using select_related at the same time." Apply this guideline to Django code in openwisp_controller/config/management/commands/clear_last_ip.py and similar modules by ensuring any select_related with an accompanying only() includes the related field names to prevent deferred/traversed conflicts.
Applied to files:
openwisp_controller/config/migrations/0065_merge_20260702_1946.pyopenwisp_controller/config/settings.pyopenwisp_controller/config/apps.pyopenwisp_controller/config/tests/test_config.pyopenwisp_controller/config/models.pyopenwisp_controller/pki/tests/test_api.pyopenwisp_controller/config/tests/test_selenium.pyopenwisp_controller/config/tests/test_api.pyopenwisp_controller/config/handlers.pyopenwisp_controller/config/base/config.pyopenwisp_controller/config/migrations/0064_template_blueprint_cert_template_ca_and_more.pyopenwisp_controller/config/api/serializers.pyopenwisp_controller/config/tasks.pyopenwisp_controller/config/tests/test_admin.pyopenwisp_controller/config/base/device_certificate.pyopenwisp_controller/config/admin.pyopenwisp_controller/config/base/template.pyopenwisp_controller/config/tests/test_template.pyopenwisp_controller/config/tests/test_device.py
📚 Learning: 2026-02-17T19:13:10.088Z
Learnt from: nemesifier
Repo: openwisp/openwisp-controller PR: 1175
File: openwisp_controller/config/whois/commands.py:0-0
Timestamp: 2026-02-17T19:13:10.088Z
Learning: In reviews for the openwisp/openwisp-controller repository, do not propose changes based on Ruff warnings. The project does not use Ruff as its linter; ignore Ruff-related suggestions and follow the repository’s established linting and configuration rules. This guidance applies to all Python files under the openwisp_controller directory.
Applied to files:
openwisp_controller/config/migrations/0065_merge_20260702_1946.pyopenwisp_controller/config/settings.pyopenwisp_controller/config/apps.pyopenwisp_controller/config/tests/test_config.pyopenwisp_controller/config/models.pyopenwisp_controller/pki/tests/test_api.pyopenwisp_controller/config/tests/test_selenium.pyopenwisp_controller/config/tests/test_api.pyopenwisp_controller/config/handlers.pyopenwisp_controller/config/base/config.pyopenwisp_controller/config/migrations/0064_template_blueprint_cert_template_ca_and_more.pyopenwisp_controller/config/api/serializers.pyopenwisp_controller/config/tasks.pyopenwisp_controller/config/tests/test_admin.pyopenwisp_controller/config/base/device_certificate.pyopenwisp_controller/config/admin.pyopenwisp_controller/config/base/template.pyopenwisp_controller/config/tests/test_template.pyopenwisp_controller/config/tests/test_device.py
📚 Learning: 2026-01-15T15:07:17.354Z
Learnt from: DragnEmperor
Repo: openwisp/openwisp-controller PR: 1175
File: openwisp_controller/geo/estimated_location/tests/tests.py:172-175
Timestamp: 2026-01-15T15:07:17.354Z
Learning: In this repository, flake8 enforces E501 (line too long) via setup.cfg (max-line-length = 88) while ruff ignores E501 via ruff.toml. Therefore, use '# noqa: E501' on lines that intentionally exceed 88 characters to satisfy flake8 without affecting ruff checks. This applies to Python files across the project (any .py) and is relevant for tests as well. Use sparingly and only where breaking lines is not feasible without hurting readability or functionality.
Applied to files:
openwisp_controller/config/migrations/0065_merge_20260702_1946.pyopenwisp_controller/config/settings.pytests/openwisp2/sample_config/migrations/0011_merge_20260702_1946.pytests/openwisp2/settings.pyopenwisp_controller/config/apps.pytests/openwisp2/sample_config/models.pyopenwisp_controller/config/tests/test_config.pyopenwisp_controller/config/models.pyopenwisp_controller/pki/tests/test_api.pyopenwisp_controller/config/tests/test_selenium.pyopenwisp_controller/config/tests/test_api.pytests/openwisp2/sample_config/migrations/0010_template_blueprint_cert_template_ca_and_more.pyopenwisp_controller/config/handlers.pyopenwisp_controller/config/base/config.pyopenwisp_controller/config/migrations/0064_template_blueprint_cert_template_ca_and_more.pyopenwisp_controller/config/api/serializers.pyopenwisp_controller/config/tasks.pyopenwisp_controller/config/tests/test_admin.pyopenwisp_controller/config/base/device_certificate.pyopenwisp_controller/config/admin.pyopenwisp_controller/config/base/template.pyopenwisp_controller/config/tests/test_template.pyopenwisp_controller/config/tests/test_device.py
📚 Learning: 2026-01-12T22:27:40.078Z
Learnt from: nemesifier
Repo: openwisp/openwisp-controller PR: 1175
File: tests/openwisp2/sample_config/migrations/0008_whoisinfo_organizationconfigsettings_whois_enabled.py:18-67
Timestamp: 2026-01-12T22:27:40.078Z
Learning: In test migrations under tests/openwisp2/sample_config/migrations, verify scenarios where a swappable model (CONFIG_WHOISINFO_MODEL) is extended with extra fields (e.g., an additional 'details' field) to ensure compatibility and no errors when swapping to a custom implementation. This pattern helps confirm that extending AbstractWHOISInfo via a custom model works as intended.
Applied to files:
tests/openwisp2/sample_config/migrations/0011_merge_20260702_1946.pytests/openwisp2/sample_config/migrations/0010_template_blueprint_cert_template_ca_and_more.py
📚 Learning: 2026-06-07T12:07:24.608Z
Learnt from: stktyagi
Repo: openwisp/openwisp-controller PR: 1378
File: openwisp_controller/pki/tests/test_api.py:155-155
Timestamp: 2026-06-07T12:07:24.608Z
Learning: When reviewing Python test files in this repository, avoid recommending inline comments that explain or justify `assertNumQueries` (Django query count) expectations. Query counts can change frequently as implementations evolve, and inline explanations add maintenance burden; the expected count should be understandable without added comment blocks.
Applied to files:
tests/openwisp2/sample_config/migrations/0011_merge_20260702_1946.pytests/openwisp2/settings.pytests/openwisp2/sample_config/models.pyopenwisp_controller/config/tests/test_config.pyopenwisp_controller/pki/tests/test_api.pyopenwisp_controller/config/tests/test_selenium.pyopenwisp_controller/config/tests/test_api.pytests/openwisp2/sample_config/migrations/0010_template_blueprint_cert_template_ca_and_more.pyopenwisp_controller/config/tests/test_admin.pyopenwisp_controller/config/tests/test_template.pyopenwisp_controller/config/tests/test_device.py
📚 Learning: 2026-06-25T12:20:18.414Z
Learnt from: dee077
Repo: openwisp/openwisp-controller PR: 1395
File: openwisp_controller/connection/base/models.py:571-572
Timestamp: 2026-06-25T12:20:18.414Z
Learning: When writing or reviewing tests that override pagination behavior via OpenWispPagination.paginate_queryset(), patch `view.pagination_page_size` (not `page_size`). The method uses `getattr(view, "pagination_page_size", self.page_size)`, so tests must set the attribute on the view to affect pagination. If the view class does not define `pagination_page_size`, using `unittest.mock.patch(..., create=True)` is intentional and correct because the attribute may not exist until patched.
Applied to files:
tests/openwisp2/sample_config/migrations/0011_merge_20260702_1946.pytests/openwisp2/settings.pytests/openwisp2/sample_config/models.pyopenwisp_controller/config/tests/test_config.pyopenwisp_controller/pki/tests/test_api.pyopenwisp_controller/config/tests/test_selenium.pyopenwisp_controller/config/tests/test_api.pytests/openwisp2/sample_config/migrations/0010_template_blueprint_cert_template_ca_and_more.pyopenwisp_controller/config/tests/test_admin.pyopenwisp_controller/config/tests/test_template.pyopenwisp_controller/config/tests/test_device.py
📚 Learning: 2026-06-07T12:07:08.468Z
Learnt from: stktyagi
Repo: openwisp/openwisp-controller PR: 1378
File: openwisp_controller/config/tests/test_admin.py:2335-2335
Timestamp: 2026-06-07T12:07:08.468Z
Learning: In this project’s Python test suite (files under openwisp_controller/**/tests/), don’t require or request prose/inline comments that document the breakdown of query-count changes (e.g., assertions around template/DB query counts in helpers like _verify_template_queries). Treat query-count assertions as volatile implementation details that change frequently; review should focus on whether the test asserts the expected behavior, not on explaining the specific query-count deltas in comments.
Applied to files:
openwisp_controller/config/tests/test_config.pyopenwisp_controller/pki/tests/test_api.pyopenwisp_controller/config/tests/test_selenium.pyopenwisp_controller/config/tests/test_api.pyopenwisp_controller/config/tests/test_admin.pyopenwisp_controller/config/tests/test_template.pyopenwisp_controller/config/tests/test_device.py
📚 Learning: 2026-06-07T12:07:25.164Z
Learnt from: stktyagi
Repo: openwisp/openwisp-controller PR: 1378
File: openwisp_controller/config/tests/test_config.py:864-865
Timestamp: 2026-06-07T12:07:25.164Z
Learning: When reviewing this repo’s Python test suite, treat changes to the *expected* query count in `assertNumQueries(...)` calls as routine test maintenance. If a PR updates the numeric argument (e.g., in `test_config.py`, `test_api.py`, `test_admin.py`, `test_pki.py`) and the test remains consistent with the feature changes, reviewers should not flag the increased number as a performance regression that requires investigation solely because the count went up; instead, focus on whether the update is intentional and the surrounding test/code changes justify the revised expectation.
Applied to files:
openwisp_controller/config/tests/test_config.pyopenwisp_controller/pki/tests/test_api.pyopenwisp_controller/config/tests/test_selenium.pyopenwisp_controller/config/tests/test_api.pyopenwisp_controller/config/tests/test_admin.pyopenwisp_controller/config/tests/test_template.pyopenwisp_controller/config/tests/test_device.py
📚 Learning: 2026-06-25T12:20:45.387Z
Learnt from: dee077
Repo: openwisp/openwisp-controller PR: 1395
File: openwisp_controller/connection/tests/test_api.py:916-932
Timestamp: 2026-06-25T12:20:45.387Z
Learning: When reviewing API pagination behavior in openwisp-controller, assume `OpenWispPagination.paginate_queryset()` allows a per-view page-size override via `getattr(view, "pagination_page_size", self.page_size)` (so `view.pagination_page_size`, if present, should affect pagination). In Python tests, it is valid to patch `pagination_page_size` on a view class even if the attribute isn’t declared on the class by default, by using `unittest.mock.patch.object(..., "pagination_page_size", ..., create=True)` so the override is available for the pagination logic during the test.
Applied to files:
openwisp_controller/config/tests/test_config.pyopenwisp_controller/pki/tests/test_api.pyopenwisp_controller/config/tests/test_selenium.pyopenwisp_controller/config/tests/test_api.pyopenwisp_controller/config/tests/test_admin.pyopenwisp_controller/config/tests/test_template.pyopenwisp_controller/config/tests/test_device.py
🪛 ast-grep (0.44.0)
openwisp_controller/config/tests/test_selenium.py
[warning] 25-25: Loading a Keras model from an untrusted file can execute arbitrary code via Lambda layers or custom objects. Load only trusted models and avoid deserializing custom objects from untrusted sources.
Context: load_model("openwisp_notifications", "Notification")
Note: [CWE-502] Deserialization of Untrusted Data.
(keras-load-model-python)
tests/openwisp2/sample_config/migrations/0010_template_blueprint_cert_template_ca_and_more.py
[info] 106-106: use help_text to document model columns
Context: models.CharField(blank=True, max_length=64, null=True)
Note: [CWE-710] Improper Adherence to Coding Standards.
(model-help-text)
openwisp_controller/config/handlers.py
[warning] 98-98: Loading a Keras model from an untrusted file can execute arbitrary code via Lambda layers or custom objects. Load only trusted models and avoid deserializing custom objects from untrusted sources.
Context: load_model("config", "DeviceCertificate")
Note: [CWE-502] Deserialization of Untrusted Data.
(keras-load-model-python)
openwisp_controller/config/tasks.py
[warning] 232-232: Loading a Keras model from an untrusted file can execute arbitrary code via Lambda layers or custom objects. Load only trusted models and avoid deserializing custom objects from untrusted sources.
Context: load_model("config", "Device")
Note: [CWE-502] Deserialization of Untrusted Data.
(keras-load-model-python)
[warning] 233-233: Loading a Keras model from an untrusted file can execute arbitrary code via Lambda layers or custom objects. Load only trusted models and avoid deserializing custom objects from untrusted sources.
Context: load_model("config", "DeviceCertificate")
Note: [CWE-502] Deserialization of Untrusted Data.
(keras-load-model-python)
openwisp_controller/config/base/device_certificate.py
[warning] 41-41: Loading a Keras model from an untrusted file can execute arbitrary code via Lambda layers or custom objects. Load only trusted models and avoid deserializing custom objects from untrusted sources.
Context: load_model("config", "Template")
Note: [CWE-502] Deserialization of Untrusted Data.
(keras-load-model-python)
openwisp_controller/config/base/template.py
[warning] 40-40: Loading a Keras model from an untrusted file can execute arbitrary code via Lambda layers or custom objects. Load only trusted models and avoid deserializing custom objects from untrusted sources.
Context: load_model("django_x509", "Cert")
Note: [CWE-502] Deserialization of Untrusted Data.
(keras-load-model-python)
[warning] 41-41: Loading a Keras model from an untrusted file can execute arbitrary code via Lambda layers or custom objects. Load only trusted models and avoid deserializing custom objects from untrusted sources.
Context: load_model("config", "DeviceCertificate")
Note: [CWE-502] Deserialization of Untrusted Data.
(keras-load-model-python)
[warning] 275-275: Loading a Keras model from an untrusted file can execute arbitrary code via Lambda layers or custom objects. Load only trusted models and avoid deserializing custom objects from untrusted sources.
Context: load_model("config", "Config")
Note: [CWE-502] Deserialization of Untrusted Data.
(keras-load-model-python)
[warning] 338-338: Loading a Keras model from an untrusted file can execute arbitrary code via Lambda layers or custom objects. Load only trusted models and avoid deserializing custom objects from untrusted sources.
Context: load_model("config", "DeviceCertificate")
Note: [CWE-502] Deserialization of Untrusted Data.
(keras-load-model-python)
openwisp_controller/config/tests/test_device.py
[warning] 30-30: Loading a Keras model from an untrusted file can execute arbitrary code via Lambda layers or custom objects. Load only trusted models and avoid deserializing custom objects from untrusted sources.
Context: load_model("django_x509", "Cert")
Note: [CWE-502] Deserialization of Untrusted Data.
(keras-load-model-python)
[warning] 31-31: Loading a Keras model from an untrusted file can execute arbitrary code via Lambda layers or custom objects. Load only trusted models and avoid deserializing custom objects from untrusted sources.
Context: load_model("django_x509", "Ca")
Note: [CWE-502] Deserialization of Untrusted Data.
(keras-load-model-python)
[warning] 32-32: Loading a Keras model from an untrusted file can execute arbitrary code via Lambda layers or custom objects. Load only trusted models and avoid deserializing custom objects from untrusted sources.
Context: load_model("config", "OrganizationConfigSettings")
Note: [CWE-502] Deserialization of Untrusted Data.
(keras-load-model-python)
🪛 HTMLHint (1.9.2)
openwisp_controller/config/templates/admin/config/device_certificates_table.html
[error] 1-1: Doctype must be declared before any non-comment content.
(doctype-first)
Addressed suggested refactor and additions
Simplified it to except Exception as e: since ImportError is already a subclass of Exception
Migrations Check FailedHello @stktyagi, The CI failed because of a migrations check failure. This indicates that the database schema might be out of sync with the current code. Fix: Please run the following command to generate the necessary migration files: ./manage.py makemigrationsAfter running the command, commit the generated migration files and push again. |
| try: | ||
| old_instance = sender.objects.only("name", "mac_address").get(pk=instance.pk) | ||
| instance._old_name = old_instance.name | ||
| instance._old_mac = old_instance.mac_address |
There was a problem hiding this comment.
This can be achieved inside the model, without this extra logic which also adds more queries. See https://github.com/search?q=repo%3Aopenwisp%2Fopenwisp-controller%20_initial&type=code
|
@coderabbitai any outstanding issue? |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Get current unresolved review threads on PR 1378
gh api graphql -f query='
query {
repository(owner: "openwisp", name: "openwisp-controller") {
pullRequest(number: 1378) {
reviewThreads(first: 100) {
nodes {
isResolved
isOutdated
comments(first: 1) {
nodes {
path
body
author { login }
createdAt
}
}
}
}
}
}
}' | jq '[.data.repository.pullRequest.reviewThreads.nodes[] | select(.isResolved == false) | {path: .comments.nodes[0].path, isOutdated, body: .comments.nodes[0].body[0:300], author: .comments.nodes[0].author.login}]'Length of output: 12209
🔴 Still appears open / needs attention
🟡 Likely already addressed, but threads not yet closed on GitHub (worth a quick `` pass once verified)
🔵 Not yet confirmed either way
I'd recommend triggering a fresh ✅ Action performedReview finished.
|
stktyagi
changed the title
Enhanced ui for certificate details
…s/1356-extend-abstract-template
@coderabbitai see below:
|
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== Device certificate extension construction ==='
ast-grep outline openwisp_controller/config/base/device_certificate.py --items all --match 'hardware|extension|build' || true
rg -n -A24 -B8 '_get_hardware_oid_extensions|_build_extensions|_build_cert' \
openwisp_controller/config/base/device_certificate.py
echo '=== M2M signal registration and lifecycle implementation ==='
rg -n -A12 -B8 'm2m_changed|manage_device_certs|add_default_templates|add_tagged_templates|manage_group_templates|_auto_add_to_existing_configs|def deactivate' \
openwisp_controller/config/base/config.py \
openwisp_controller/config/base/template.py \
openwisp_controller/config/apps.py
echo '=== Declared django-x509 dependency/source availability ==='
rg -n -i -C2 'django[-_]?x509|django_x509' pyproject.toml setup.cfg setup.py requirements*.txt 2>/dev/null || true
fd -HI '^models\.py$' . | while read -r f; do
if rg -q '_verify_extension_format|def _add_extensions' "$f"; then
echo "--- $f"
rg -n -A34 -B5 '_verify_extension_format|def _add_extensions' "$f"
fi
doneLength of output: 36574 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== django-x509 1.4 extension contract (declared dependency branch) ==='
gh api 'repos/openwisp/django-x509/contents/django_x509/base/models.py?ref=1.4' \
--jq '.content' | base64 --decode \
| nl -ba \
| sed -n '585,625p;760,795p'
echo '=== Certificate-template M2M call paths ==='
sed -n '219,260p' openwisp_controller/config/base/template.py
sed -n '1047,1085p' openwisp_controller/config/base/config.py
echo '=== Tagged-template assignment implementations ==='
rg -n -A24 -B5 'def add_tagged_templates|\.templates\.(add|remove|clear|set)\s*\(' \
openwisp_controller/config --glob '*.py'Length of output: 1054 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== django-x509 1.4 extension contract (declared dependency branch) ==='
gh api 'repos/openwisp/django-x509/contents/django_x509/base/models.py?ref=1.4' \
--jq '.content' | base64 --decode \
| awk 'NR >= 585 && NR <= 625 || NR >= 760 && NR <= 795 { printf "%6d %s\n", NR, $0 }'
echo '=== Certificate-template M2M call paths ==='
sed -n '219,260p' openwisp_controller/config/base/template.py
sed -n '1047,1085p' openwisp_controller/config/base/config.py
echo '=== Tagged-template assignment implementations ==='
rg -n -A24 -B5 'def add_tagged_templates|\.templates\.(add|remove|clear|set)\s*\(' \
openwisp_controller/config --glob '*.py'Length of output: 233996 🏁 Script executed: #!/bin/bash
set -euo pipefail
sed -n '1,180p' /tmp/coderabbit-shell-logs/shell-output-d0UbqULength of output: 9892
The separate documentation/comment request for the 🐇🔎 ✏️ Learnings added
|
Track mac_address in _changed_checked_fields so the model's __init__ sets _initial_mac_address automatically, matching the existing _initial_name pattern.
|
@coderabbitai full review |
✅ Action performedFull review finished. |
There was a problem hiding this comment.
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 `@openwisp_controller/config/tests/test_device.py`:
- Line 668: New hardware-drift tests create CAs without model validation. Add
the appropriate PKI/VPN test mixin to TestDevice so _create_ca() is available,
then replace every Ca.objects.create(name="test-ca", organization=org) in the
affected tests with self._create_ca(name="test-ca", organization=org),
preserving full_clean() validation.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: ba76345b-91e8-4c0b-85b3-35231934acee
📒 Files selected for processing (34)
docs/developer/extending.rstdocs/index.rstdocs/user/certificate-templates.rstdocs/user/intro.rstdocs/user/rest-api.rstdocs/user/settings.rstdocs/user/templates.rstopenwisp_controller/config/admin.pyopenwisp_controller/config/api/serializers.pyopenwisp_controller/config/apps.pyopenwisp_controller/config/base/config.pyopenwisp_controller/config/base/device.pyopenwisp_controller/config/base/device_certificate.pyopenwisp_controller/config/base/template.pyopenwisp_controller/config/handlers.pyopenwisp_controller/config/migrations/0064_template_blueprint_cert_template_ca_and_more.pyopenwisp_controller/config/migrations/0065_merge_20260702_1946.pyopenwisp_controller/config/models.pyopenwisp_controller/config/settings.pyopenwisp_controller/config/static/config/js/switcher.jsopenwisp_controller/config/tasks.pyopenwisp_controller/config/templates/admin/config/device/change_form.htmlopenwisp_controller/config/templates/admin/config/device_certificates_table.htmlopenwisp_controller/config/tests/test_admin.pyopenwisp_controller/config/tests/test_api.pyopenwisp_controller/config/tests/test_config.pyopenwisp_controller/config/tests/test_device.pyopenwisp_controller/config/tests/test_selenium.pyopenwisp_controller/config/tests/test_template.pyopenwisp_controller/pki/tests/test_api.pytests/openwisp2/sample_config/migrations/0010_template_blueprint_cert_template_ca_and_more.pytests/openwisp2/sample_config/migrations/0011_merge_20260702_1946.pytests/openwisp2/sample_config/models.pytests/openwisp2/settings.py
📜 Review details
⏰ Context from checks skipped due to timeout. (12)
- GitHub Check: CodeRabbit / Review
- GitHub Check: Python==3.12 | django~=5.2.0
- GitHub Check: Python==3.13 | django~=5.2.0
- GitHub Check: Python==3.11 | django~=4.2.0
- GitHub Check: Python==3.10 | django~=5.2.0
- GitHub Check: Python==3.10 | django~=5.1.0
- GitHub Check: Python==3.12 | django~=4.2.0
- GitHub Check: Python==3.11 | django~=5.1.0
- GitHub Check: Python==3.10 | django~=4.2.0
- GitHub Check: Python==3.12 | django~=5.1.0
- GitHub Check: Python==3.13 | django~=5.1.0
- GitHub Check: Python==3.11 | django~=5.2.0
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Mark user-facing strings for translation with Django i18n helpers in Django code
Avoid unnecessary blank lines inside function and method bodies
Be careful with authentication, authorization, queryset filtering, serializers, admin behavior, cache invalidation, signals, Celery tasks, and websocket updates in Django code
Preserve validation around templates, VPN/PKI material, SSH credentials, device commands, uploaded files, URLs, and subnet/IP data
Write comments and docstrings only when they explain why code is shaped a certain way, placing them before the relevant code block instead of scattering them inside it
Files:
openwisp_controller/config/migrations/0065_merge_20260702_1946.pytests/openwisp2/sample_config/migrations/0011_merge_20260702_1946.pyopenwisp_controller/config/apps.pyopenwisp_controller/config/models.pytests/openwisp2/settings.pyopenwisp_controller/config/tests/test_admin.pyopenwisp_controller/config/settings.pyopenwisp_controller/pki/tests/test_api.pyopenwisp_controller/config/tests/test_config.pyopenwisp_controller/config/admin.pytests/openwisp2/sample_config/models.pyopenwisp_controller/config/base/device.pyopenwisp_controller/config/handlers.pyopenwisp_controller/config/api/serializers.pytests/openwisp2/sample_config/migrations/0010_template_blueprint_cert_template_ca_and_more.pyopenwisp_controller/config/migrations/0064_template_blueprint_cert_template_ca_and_more.pyopenwisp_controller/config/tasks.pyopenwisp_controller/config/tests/test_selenium.pyopenwisp_controller/config/base/template.pyopenwisp_controller/config/tests/test_api.pyopenwisp_controller/config/base/device_certificate.pyopenwisp_controller/config/tests/test_template.pyopenwisp_controller/config/base/config.pyopenwisp_controller/config/tests/test_device.py
**/*.{py,html}
📄 CodeRabbit inference engine (Custom checks)
For Django pull requests, ensure all user-facing strings are marked as translatable using the Django i18n framework.
Files:
openwisp_controller/config/migrations/0065_merge_20260702_1946.pytests/openwisp2/sample_config/migrations/0011_merge_20260702_1946.pyopenwisp_controller/config/templates/admin/config/device/change_form.htmlopenwisp_controller/config/apps.pyopenwisp_controller/config/models.pytests/openwisp2/settings.pyopenwisp_controller/config/tests/test_admin.pyopenwisp_controller/config/templates/admin/config/device_certificates_table.htmlopenwisp_controller/config/settings.pyopenwisp_controller/pki/tests/test_api.pyopenwisp_controller/config/tests/test_config.pyopenwisp_controller/config/admin.pytests/openwisp2/sample_config/models.pyopenwisp_controller/config/base/device.pyopenwisp_controller/config/handlers.pyopenwisp_controller/config/api/serializers.pytests/openwisp2/sample_config/migrations/0010_template_blueprint_cert_template_ca_and_more.pyopenwisp_controller/config/migrations/0064_template_blueprint_cert_template_ca_and_more.pyopenwisp_controller/config/tasks.pyopenwisp_controller/config/tests/test_selenium.pyopenwisp_controller/config/base/template.pyopenwisp_controller/config/tests/test_api.pyopenwisp_controller/config/base/device_certificate.pyopenwisp_controller/config/tests/test_template.pyopenwisp_controller/config/base/config.pyopenwisp_controller/config/tests/test_device.py
🧠 Learnings (9)
📚 Learning: 2026-01-15T15:05:49.557Z
Learnt from: DragnEmperor
Repo: openwisp/openwisp-controller PR: 1175
File: openwisp_controller/config/management/commands/clear_last_ip.py:38-42
Timestamp: 2026-01-15T15:05:49.557Z
Learning: In Django projects, when using select_related() to traverse relations (for example, select_related("organization__config_settings")), the traversed relation must not be deferred. If you also use .only() in the same query, include the relation name or FK field (e.g., "organization" or "organization_id") in the .only() list to avoid the error "Field X cannot be both deferred and traversed using select_related at the same time." Apply this guideline to Django code in openwisp_controller/config/management/commands/clear_last_ip.py and similar modules by ensuring any select_related with an accompanying only() includes the related field names to prevent deferred/traversed conflicts.
Applied to files:
openwisp_controller/config/migrations/0065_merge_20260702_1946.pyopenwisp_controller/config/apps.pyopenwisp_controller/config/models.pyopenwisp_controller/config/tests/test_admin.pyopenwisp_controller/config/settings.pyopenwisp_controller/pki/tests/test_api.pyopenwisp_controller/config/tests/test_config.pyopenwisp_controller/config/admin.pyopenwisp_controller/config/base/device.pyopenwisp_controller/config/handlers.pyopenwisp_controller/config/api/serializers.pyopenwisp_controller/config/migrations/0064_template_blueprint_cert_template_ca_and_more.pyopenwisp_controller/config/tasks.pyopenwisp_controller/config/tests/test_selenium.pyopenwisp_controller/config/base/template.pyopenwisp_controller/config/tests/test_api.pyopenwisp_controller/config/base/device_certificate.pyopenwisp_controller/config/tests/test_template.pyopenwisp_controller/config/base/config.pyopenwisp_controller/config/tests/test_device.py
📚 Learning: 2026-02-17T19:13:10.088Z
Learnt from: nemesifier
Repo: openwisp/openwisp-controller PR: 1175
File: openwisp_controller/config/whois/commands.py:0-0
Timestamp: 2026-02-17T19:13:10.088Z
Learning: In reviews for the openwisp/openwisp-controller repository, do not propose changes based on Ruff warnings. The project does not use Ruff as its linter; ignore Ruff-related suggestions and follow the repository’s established linting and configuration rules. This guidance applies to all Python files under the openwisp_controller directory.
Applied to files:
openwisp_controller/config/migrations/0065_merge_20260702_1946.pyopenwisp_controller/config/apps.pyopenwisp_controller/config/models.pyopenwisp_controller/config/tests/test_admin.pyopenwisp_controller/config/settings.pyopenwisp_controller/pki/tests/test_api.pyopenwisp_controller/config/tests/test_config.pyopenwisp_controller/config/admin.pyopenwisp_controller/config/base/device.pyopenwisp_controller/config/handlers.pyopenwisp_controller/config/api/serializers.pyopenwisp_controller/config/migrations/0064_template_blueprint_cert_template_ca_and_more.pyopenwisp_controller/config/tasks.pyopenwisp_controller/config/tests/test_selenium.pyopenwisp_controller/config/base/template.pyopenwisp_controller/config/tests/test_api.pyopenwisp_controller/config/base/device_certificate.pyopenwisp_controller/config/tests/test_template.pyopenwisp_controller/config/base/config.pyopenwisp_controller/config/tests/test_device.py
📚 Learning: 2026-01-15T15:07:17.354Z
Learnt from: DragnEmperor
Repo: openwisp/openwisp-controller PR: 1175
File: openwisp_controller/geo/estimated_location/tests/tests.py:172-175
Timestamp: 2026-01-15T15:07:17.354Z
Learning: In this repository, flake8 enforces E501 (line too long) via setup.cfg (max-line-length = 88) while ruff ignores E501 via ruff.toml. Therefore, use '# noqa: E501' on lines that intentionally exceed 88 characters to satisfy flake8 without affecting ruff checks. This applies to Python files across the project (any .py) and is relevant for tests as well. Use sparingly and only where breaking lines is not feasible without hurting readability or functionality.
Applied to files:
openwisp_controller/config/migrations/0065_merge_20260702_1946.pytests/openwisp2/sample_config/migrations/0011_merge_20260702_1946.pyopenwisp_controller/config/apps.pyopenwisp_controller/config/models.pytests/openwisp2/settings.pyopenwisp_controller/config/tests/test_admin.pyopenwisp_controller/config/settings.pyopenwisp_controller/pki/tests/test_api.pyopenwisp_controller/config/tests/test_config.pyopenwisp_controller/config/admin.pytests/openwisp2/sample_config/models.pyopenwisp_controller/config/base/device.pyopenwisp_controller/config/handlers.pyopenwisp_controller/config/api/serializers.pytests/openwisp2/sample_config/migrations/0010_template_blueprint_cert_template_ca_and_more.pyopenwisp_controller/config/migrations/0064_template_blueprint_cert_template_ca_and_more.pyopenwisp_controller/config/tasks.pyopenwisp_controller/config/tests/test_selenium.pyopenwisp_controller/config/base/template.pyopenwisp_controller/config/tests/test_api.pyopenwisp_controller/config/base/device_certificate.pyopenwisp_controller/config/tests/test_template.pyopenwisp_controller/config/base/config.pyopenwisp_controller/config/tests/test_device.py
📚 Learning: 2026-01-12T22:27:40.078Z
Learnt from: nemesifier
Repo: openwisp/openwisp-controller PR: 1175
File: tests/openwisp2/sample_config/migrations/0008_whoisinfo_organizationconfigsettings_whois_enabled.py:18-67
Timestamp: 2026-01-12T22:27:40.078Z
Learning: In test migrations under tests/openwisp2/sample_config/migrations, verify scenarios where a swappable model (CONFIG_WHOISINFO_MODEL) is extended with extra fields (e.g., an additional 'details' field) to ensure compatibility and no errors when swapping to a custom implementation. This pattern helps confirm that extending AbstractWHOISInfo via a custom model works as intended.
Applied to files:
tests/openwisp2/sample_config/migrations/0011_merge_20260702_1946.pytests/openwisp2/sample_config/migrations/0010_template_blueprint_cert_template_ca_and_more.py
📚 Learning: 2026-06-07T12:07:24.608Z
Learnt from: stktyagi
Repo: openwisp/openwisp-controller PR: 1378
File: openwisp_controller/pki/tests/test_api.py:155-155
Timestamp: 2026-06-07T12:07:24.608Z
Learning: When reviewing Python test files in this repository, avoid recommending inline comments that explain or justify `assertNumQueries` (Django query count) expectations. Query counts can change frequently as implementations evolve, and inline explanations add maintenance burden; the expected count should be understandable without added comment blocks.
Applied to files:
tests/openwisp2/sample_config/migrations/0011_merge_20260702_1946.pytests/openwisp2/settings.pyopenwisp_controller/config/tests/test_admin.pyopenwisp_controller/pki/tests/test_api.pyopenwisp_controller/config/tests/test_config.pytests/openwisp2/sample_config/models.pytests/openwisp2/sample_config/migrations/0010_template_blueprint_cert_template_ca_and_more.pyopenwisp_controller/config/tests/test_selenium.pyopenwisp_controller/config/tests/test_api.pyopenwisp_controller/config/tests/test_template.pyopenwisp_controller/config/tests/test_device.py
📚 Learning: 2026-06-25T12:20:18.414Z
Learnt from: dee077
Repo: openwisp/openwisp-controller PR: 1395
File: openwisp_controller/connection/base/models.py:571-572
Timestamp: 2026-06-25T12:20:18.414Z
Learning: When writing or reviewing tests that override pagination behavior via OpenWispPagination.paginate_queryset(), patch `view.pagination_page_size` (not `page_size`). The method uses `getattr(view, "pagination_page_size", self.page_size)`, so tests must set the attribute on the view to affect pagination. If the view class does not define `pagination_page_size`, using `unittest.mock.patch(..., create=True)` is intentional and correct because the attribute may not exist until patched.
Applied to files:
tests/openwisp2/sample_config/migrations/0011_merge_20260702_1946.pytests/openwisp2/settings.pyopenwisp_controller/config/tests/test_admin.pyopenwisp_controller/pki/tests/test_api.pyopenwisp_controller/config/tests/test_config.pytests/openwisp2/sample_config/models.pytests/openwisp2/sample_config/migrations/0010_template_blueprint_cert_template_ca_and_more.pyopenwisp_controller/config/tests/test_selenium.pyopenwisp_controller/config/tests/test_api.pyopenwisp_controller/config/tests/test_template.pyopenwisp_controller/config/tests/test_device.py
📚 Learning: 2026-06-07T12:07:08.468Z
Learnt from: stktyagi
Repo: openwisp/openwisp-controller PR: 1378
File: openwisp_controller/config/tests/test_admin.py:2335-2335
Timestamp: 2026-06-07T12:07:08.468Z
Learning: In this project’s Python test suite (files under openwisp_controller/**/tests/), don’t require or request prose/inline comments that document the breakdown of query-count changes (e.g., assertions around template/DB query counts in helpers like _verify_template_queries). Treat query-count assertions as volatile implementation details that change frequently; review should focus on whether the test asserts the expected behavior, not on explaining the specific query-count deltas in comments.
Applied to files:
openwisp_controller/config/tests/test_admin.pyopenwisp_controller/pki/tests/test_api.pyopenwisp_controller/config/tests/test_config.pyopenwisp_controller/config/tests/test_selenium.pyopenwisp_controller/config/tests/test_api.pyopenwisp_controller/config/tests/test_template.pyopenwisp_controller/config/tests/test_device.py
📚 Learning: 2026-06-07T12:07:25.164Z
Learnt from: stktyagi
Repo: openwisp/openwisp-controller PR: 1378
File: openwisp_controller/config/tests/test_config.py:864-865
Timestamp: 2026-06-07T12:07:25.164Z
Learning: When reviewing this repo’s Python test suite, treat changes to the *expected* query count in `assertNumQueries(...)` calls as routine test maintenance. If a PR updates the numeric argument (e.g., in `test_config.py`, `test_api.py`, `test_admin.py`, `test_pki.py`) and the test remains consistent with the feature changes, reviewers should not flag the increased number as a performance regression that requires investigation solely because the count went up; instead, focus on whether the update is intentional and the surrounding test/code changes justify the revised expectation.
Applied to files:
openwisp_controller/config/tests/test_admin.pyopenwisp_controller/pki/tests/test_api.pyopenwisp_controller/config/tests/test_config.pyopenwisp_controller/config/tests/test_selenium.pyopenwisp_controller/config/tests/test_api.pyopenwisp_controller/config/tests/test_template.pyopenwisp_controller/config/tests/test_device.py
📚 Learning: 2026-06-25T12:20:45.387Z
Learnt from: dee077
Repo: openwisp/openwisp-controller PR: 1395
File: openwisp_controller/connection/tests/test_api.py:916-932
Timestamp: 2026-06-25T12:20:45.387Z
Learning: When reviewing API pagination behavior in openwisp-controller, assume `OpenWispPagination.paginate_queryset()` allows a per-view page-size override via `getattr(view, "pagination_page_size", self.page_size)` (so `view.pagination_page_size`, if present, should affect pagination). In Python tests, it is valid to patch `pagination_page_size` on a view class even if the attribute isn’t declared on the class by default, by using `unittest.mock.patch.object(..., "pagination_page_size", ..., create=True)` so the override is available for the pagination logic during the test.
Applied to files:
openwisp_controller/config/tests/test_admin.pyopenwisp_controller/pki/tests/test_api.pyopenwisp_controller/config/tests/test_config.pyopenwisp_controller/config/tests/test_selenium.pyopenwisp_controller/config/tests/test_api.pyopenwisp_controller/config/tests/test_template.pyopenwisp_controller/config/tests/test_device.py
🪛 ast-grep (0.44.1)
openwisp_controller/config/handlers.py
[warning] 82-82: Loading a Keras model from an untrusted file can execute arbitrary code via Lambda layers or custom objects. Load only trusted models and avoid deserializing custom objects from untrusted sources.
Context: load_model("config", "DeviceCertificate")
Note: [CWE-502] Deserialization of Untrusted Data.
(keras-load-model-python)
tests/openwisp2/sample_config/migrations/0010_template_blueprint_cert_template_ca_and_more.py
[info] 106-106: use help_text to document model columns
Context: models.CharField(blank=True, max_length=64, null=True)
Note: [CWE-710] Improper Adherence to Coding Standards.
(model-help-text)
openwisp_controller/config/tasks.py
[warning] 232-232: Loading a Keras model from an untrusted file can execute arbitrary code via Lambda layers or custom objects. Load only trusted models and avoid deserializing custom objects from untrusted sources.
Context: load_model("config", "Device")
Note: [CWE-502] Deserialization of Untrusted Data.
(keras-load-model-python)
[warning] 233-233: Loading a Keras model from an untrusted file can execute arbitrary code via Lambda layers or custom objects. Load only trusted models and avoid deserializing custom objects from untrusted sources.
Context: load_model("config", "DeviceCertificate")
Note: [CWE-502] Deserialization of Untrusted Data.
(keras-load-model-python)
openwisp_controller/config/tests/test_selenium.py
[warning] 25-25: Loading a Keras model from an untrusted file can execute arbitrary code via Lambda layers or custom objects. Load only trusted models and avoid deserializing custom objects from untrusted sources.
Context: load_model("openwisp_notifications", "Notification")
Note: [CWE-502] Deserialization of Untrusted Data.
(keras-load-model-python)
openwisp_controller/config/base/template.py
[warning] 40-40: Loading a Keras model from an untrusted file can execute arbitrary code via Lambda layers or custom objects. Load only trusted models and avoid deserializing custom objects from untrusted sources.
Context: load_model("django_x509", "Cert")
Note: [CWE-502] Deserialization of Untrusted Data.
(keras-load-model-python)
[warning] 41-41: Loading a Keras model from an untrusted file can execute arbitrary code via Lambda layers or custom objects. Load only trusted models and avoid deserializing custom objects from untrusted sources.
Context: load_model("config", "DeviceCertificate")
Note: [CWE-502] Deserialization of Untrusted Data.
(keras-load-model-python)
[warning] 275-275: Loading a Keras model from an untrusted file can execute arbitrary code via Lambda layers or custom objects. Load only trusted models and avoid deserializing custom objects from untrusted sources.
Context: load_model("config", "Config")
Note: [CWE-502] Deserialization of Untrusted Data.
(keras-load-model-python)
[warning] 338-338: Loading a Keras model from an untrusted file can execute arbitrary code via Lambda layers or custom objects. Load only trusted models and avoid deserializing custom objects from untrusted sources.
Context: load_model("config", "DeviceCertificate")
Note: [CWE-502] Deserialization of Untrusted Data.
(keras-load-model-python)
openwisp_controller/config/base/device_certificate.py
[warning] 41-41: Loading a Keras model from an untrusted file can execute arbitrary code via Lambda layers or custom objects. Load only trusted models and avoid deserializing custom objects from untrusted sources.
Context: load_model("config", "Template")
Note: [CWE-502] Deserialization of Untrusted Data.
(keras-load-model-python)
openwisp_controller/config/tests/test_device.py
[warning] 30-30: Loading a Keras model from an untrusted file can execute arbitrary code via Lambda layers or custom objects. Load only trusted models and avoid deserializing custom objects from untrusted sources.
Context: load_model("django_x509", "Cert")
Note: [CWE-502] Deserialization of Untrusted Data.
(keras-load-model-python)
[warning] 31-31: Loading a Keras model from an untrusted file can execute arbitrary code via Lambda layers or custom objects. Load only trusted models and avoid deserializing custom objects from untrusted sources.
Context: load_model("django_x509", "Ca")
Note: [CWE-502] Deserialization of Untrusted Data.
(keras-load-model-python)
[warning] 32-32: Loading a Keras model from an untrusted file can execute arbitrary code via Lambda layers or custom objects. Load only trusted models and avoid deserializing custom objects from untrusted sources.
Context: load_model("config", "OrganizationConfigSettings")
Note: [CWE-502] Deserialization of Untrusted Data.
(keras-load-model-python)
🪛 HTMLHint (1.9.2)
openwisp_controller/config/templates/admin/config/device_certificates_table.html
[error] 1-1: Doctype must be declared before any non-comment content.
(doctype-first)
🔇 Additional comments (35)
docs/developer/extending.rst (1)
345-345: LGTM!docs/index.rst (1)
40-40: LGTM!docs/user/certificate-templates.rst (1)
1-291: LGTM!docs/user/intro.rst (1)
38-39: LGTM!docs/user/rest-api.rst (1)
1114-1115: LGTM!docs/user/settings.rst (1)
310-313: LGTM!Also applies to: 333-350, 371-377
docs/user/templates.rst (1)
212-229: LGTM!openwisp_controller/config/templates/admin/config/device_certificates_table.html (1)
1-49: LGTM!openwisp_controller/config/base/template.py (1)
25-29: LGTM!All previously flagged issues (NULL handling in
get_unassigned_certs, revoked-cert rejection, active-template CA/blueprint/type mutation locks with regression test coverage,config is Nonevs falsy check) are present and correctly implemented in the current state of the file. No new issues found.Also applies to: 40-51, 74-97, 134-137, 257-394
openwisp_controller/config/migrations/0064_template_blueprint_cert_template_ca_and_more.py (1)
1-153: LGTM!Migration correctly mirrors the model definitions in
template.py(blueprint_cert, ca, updated auto_cert help text, updated type choices) and correctly includes the"swappable": "CONFIG_DEVICECERTIFICATE_MODEL"option on the productionDeviceCertificatemodel.openwisp_controller/config/migrations/0065_merge_20260702_1946.py (1)
1-13: LGTM!Trivial merge migration with no schema operations.
openwisp_controller/config/models.py (1)
5-5: LGTM!The concrete
DeviceCertificatemodel correctly subclassesAbstractDeviceCertificateand registers the swappable setting forconfig.DeviceCertificate.Also applies to: 97-106
tests/openwisp2/sample_config/migrations/0010_template_blueprint_cert_template_ca_and_more.py (1)
1-151: LGTM!The sample migration intentionally omits the
swappableoption (correct, sincesample_config.DeviceCertificateis the concrete replacement, not the swappable base) and the additionaldetailsfield follows the documented django-x509-style extension pattern for demonstrating custom model extension in the test app.openwisp_controller/config/api/serializers.py (1)
20-20: LGTM!All previously flagged concerns are addressed in the current state:
cert_fields_providedis captured beforeca/blueprint_certnormalization (avoiding the performance regression), model-instance comparisons (not PK vs instance mismatches) are used for the active-template mutation lock, thetypefield is included in the mutation lock, and the explicitDeviceCertificatecleanup in both_update_configand the organization-change path inDeviceDetailSerializer.updateare confirmed intentional/necessary givenmanage_device_certs'spost_clearno-op for active devices.Also applies to: 42-43, 53-62, 79-87, 88-173, 324-326, 440-452
openwisp_controller/config/admin.py (1)
23-23: LGTM!
_add_certificate_detailscorrectly orders the queryset bycreatedbefore slicing (previously-flagged pagination-stability concern is fixed), andselect_related("cert__ca", "template")avoids N+1 queries when buildingcert_data. I verified that adding"ca"and"blueprint_cert"toTemplateAdmin.autocomplete_fieldsis safe: django-x509'sCaAdmin/CertAdmininheritsearch_fields = ['name', 'serial_number', 'common_name']fromBaseAdminand are registered viaadmin.site.register, so this will not trigger a Django admin system-check error (admin.E039/E040).Also applies to: 64-64, 117-126, 928-929, 967-1013, 1117-1118, 1132-1141, 1154-1154
openwisp_controller/config/base/device_certificate.py (2)
41-42: False positive: not a Keras model load.The static analysis hint about untrusted Keras model deserialization on
load_model("config", "Template")is a false positive — this isswapper.load_model, used to resolve the swappableTemplatemodel class, unrelated to Keras/ML model loading.Source: Linters/SAST tools
1-186: LGTM!The remaining known items (django-x509 OID-extension version pin, upstream
django-x509#228-blocked custom OID injection) are already tracked in prior discussion and explicitly deferred by the author; no new issues found in this revision.tests/openwisp2/sample_config/migrations/0011_merge_20260702_1946.py (1)
1-14: LGTM!tests/openwisp2/sample_config/models.py (1)
5-5: LGTM!Also applies to: 99-107
tests/openwisp2/settings.py (1)
284-284: LGTM!openwisp_controller/config/base/config.py (1)
66-72: LGTM!
certificate_updatednow correctly resolves configs impacted throughDeviceCertificate(not justvpnclient), andmanage_device_certsnow carries explanatory comments for each branch as previously requested. No new issues found in this revision.Also applies to: 474-489, 502-548, 978-1004, 1032-1032
openwisp_controller/config/templates/admin/config/device/change_form.html (1)
13-28: LGTM!openwisp_controller/config/static/config/js/switcher.js (1)
4-15: LGTM!Also applies to: 42-62
openwisp_controller/config/apps.py (1)
72-72: LGTM!Also applies to: 98-102, 124-128
openwisp_controller/config/tasks.py (1)
226-289: LGTM!openwisp_controller/config/tests/test_config.py (1)
861-871: LGTM!Based on learnings, query-count changes in
assertNumQueriescalls across this repo's test files are treated as routine maintenance as the feature set evolves, not as regressions requiring investigation.openwisp_controller/config/tests/test_selenium.py (1)
25-26: LGTM!Also applies to: 491-529, 530-561, 846-878
openwisp_controller/pki/tests/test_api.py (1)
155-155: LGTM!Based on learnings,
assertNumQueriescount changes in this repo's test files are routine maintenance as features evolve and don't warrant inline explanatory comments or regression investigation.Also applies to: 256-256, 267-278, 292-292
openwisp_controller/config/tests/test_device.py (1)
6-6: 🩺 Stability & AvailabilityVerify
TransactionTestCaseis still needed. Remove the import if no test in this file depends on it; otherwise point it at the test that uses it.openwisp_controller/config/settings.py (1)
37-39: 📐 Maintainability & Code Quality | ⚡ Quick winConfirm
docs/user/settings.rstwas updated for this new setting.A prior reviewer noted that
OPENWISP_CONTROLLER_REGENERATE_CERTS_ON_HARDWARE_CHANGEwasn't documented indocs/user/settings.rst, and thatOPENWISP_CONTROLLER_COMMON_NAME_FORMAT's docs still only mention VPN client certificates even though standalone cert templates now use it too. No confirmation of a fix is visible in this batch (the docs layer isn't included here).openwisp_controller/config/base/device.py (1)
33-39: LGTM!Also applies to: 372-380
openwisp_controller/config/handlers.py (1)
48-95: LGTM!openwisp_controller/config/tests/test_template.py (1)
973-1589: LGTM!openwisp_controller/config/tests/test_admin.py (1)
1824-1857: LGTM!Based on learnings, query-count assertions in
_verify_template_queriesare treated as volatile implementation details that don't need inline explanation, so theexpected_count = 24change needs no further comment.Also applies to: 2389-2394
openwisp_controller/config/tests/test_api.py (1)
1338-1545: LGTM!
Used _create_ca()/full_clean() like the rest of the file does for Cert.
|
The CI is failing due to transient infrastructure issues (not related to your code). I have restarted the failed jobs automatically (1/3). |
| :target: https://raw.githubusercontent.com/openwisp/openwisp-controller/docs/certificate-templates/certificate-template.png | ||
| :alt: Certificate Template admin form | ||
|
|
||
| :guilabel:`Certificate Authority` (required) |
There was a problem hiding this comment.
@stktyagi are you testing the build of openwisp-docs with this document? If you haven't yet, please do, as it will be one of the next things I will start testing once I tested all the other aspects.
| TYPE_CHOICES = ( | ||
| ("generic", _("Generic")), | ||
| ("vpn", _("VPN-client")), | ||
| ("cert", _("Certificate")), |
There was a problem hiding this comment.
shouldn't we call this "Certificate generator" ?
I think "Certificate" here is too vague and bad for UX as it's confusing and I also think the same for the docs text which is not using the "Certificate generator" wording nor any equivalent meaning. We must indicate that these templates automate the creation of X509 certificates.
| "by this template." | ||
| ), | ||
| ) | ||
|
|
| if certs_regenerated > 0: | ||
| try: | ||
| message = _( | ||
| "Hardware drift detected on device {device_name}. " |
There was a problem hiding this comment.
The notification text used here reads AI sloppy to me. Who says "hardware drift" in every day language? A user is going to read this and think "WTF"?
|
|
||
|
|
||
| @shared_task(soft_time_limit=1200) | ||
| def regenerate_device_certificates_task(device_id, expected_cert_ids=None): |
There was a problem hiding this comment.
this task seems too big to me, can you move this logic to the most suited Django model (eg AbstractDeviceCertificate)? A class should work fine.
| ("config", "0064_template_notes"), | ||
| ] | ||
|
|
||
| operations = [] |
nemesifier
left a comment
nemesifier
left a comment
There was a problem hiding this comment.
Migration 0064 is not conflicting with config.0064_template_notes.
Due to the important changes done in #1418, I think it's time to sync your work with the latest master.
nemesifier
left a comment
nemesifier
left a comment
There was a problem hiding this comment.
Same error message for both fields?
Please make the error message specialized for each field.
There was a problem hiding this comment.
- Certificate details > X.509 Certs
- Eliminate "Certificates" heading, redundant
- From: I am not sure this is clear, I'd call it "Template" and add a help icon (we should have one) next to it and wrap both (template word and help icon) with a span and add a title attribute providing a tooltip which explains this column, eg: indicates which templates generated the x509 certificate.
- the table doesn't use the available width
Let's look at the certs admin:
Can we make it look identical? Except the "organization" field which is implicit (we can omit it and replace it with the "Template" column).
Let's also link the CA (with target="_blank").
| instance.config.device.organization = validated_data.get( | ||
| "organization" | ||
| ) | ||
| DeviceCertificate.objects.filter(config=instance.config).delete() |
There was a problem hiding this comment.
This cleanup only runs in the branch where no config payload was supplied. If a request changes both organization and config, _update_config() runs first while device.organization still points to the old organization, and the organization-change cleanup is skipped. With certificate templates this path now also has to keep DeviceCertificate rows in sync with the final organization/templates state. Please cover this combined organization-plus-config update path so old templates and device certificates are cleaned consistently before applying the new configuration.
| if self._has_config(): | ||
| self.config.set_status_modified() | ||
|
|
||
| self._initial_mac_address = self.mac_address |
There was a problem hiding this comment.
This advances _initial_mac_address after every save, regardless of update_fields. If mac_address is dirty in memory but saved with save(update_fields=["name"]), the new MAC is not persisted but _initial_mac_address is still updated to the dirty value. A later save that actually includes mac_address will compare against the already-advanced initial value and skip certificate regeneration, leaving the certificate CN and MAC OID stale. Please make the initial-value update respect update_fields, or pass the saved fields into _check_changed_fields() so fields that were not persisted do not advance their _initial_* values.
| vpn_list = config.templates.filter(type="vpn").values_list("vpn") | ||
| if vpn_list: | ||
| config.vpnclient_set.exclude(vpn__in=vpn_list).delete() | ||
| DeviceCertificate.objects.filter(config=config).exclude( |
There was a problem hiding this comment.
Deleting DeviceCertificate rows here happens before config.templates.set(..., clear=True) has enforced required templates. If a required certificate template is omitted from the submitted template list, it will be re-added later, but its old DeviceCertificate has already been deleted and the auto-generated cert revoked. The template remains assigned, but certificate material is unnecessarily rotated. Please wait until the final template set is known, including required templates, or rely on the M2M lifecycle handler to remove only rows that are truly no longer assigned.
| context.update(self.device._get_group().get_context()) | ||
| # Add predefined variables | ||
| context.update(self.get_vpn_context()) | ||
| context.update(self.get_cert_context()) |
There was a problem hiding this comment.
This adds a devicecertificate_set query to every config context render, including configs that do not use certificate templates. In bulk status updates, _update_related_config_status() prefetches VPN clients and templates but not device certificates, so each config can now pay an extra query. Please consider reusing prefetches where available and prefetching device certificates in bulk paths, or avoiding this query when the config has no certificate templates.
| ) | ||
| return value | ||
|
|
||
| def validate(self, data): |
There was a problem hiding this comment.
TemplateSerializer already inherits from ValidatedModelSerializer, which calls model full_clean(). This method repeats much of the same certificate-template validation that already lives in AbstractTemplate.clean() and helper methods, causing duplicate active-config and blueprint assignment checks and increasing the chance API behavior diverges from model/admin behavior. Please keep the core validation in the model and let the serializer rely on model validation, using only a small shared normalization helper if the API needs normalized values copied into validated_data.
| cn = self._get_common_name() | ||
| self._auto_create_cert(name=self.config.device.name, common_name=cn) | ||
|
|
||
| def _get_common_name(self): |
There was a problem hiding this comment.
This common-name generation duplicates the existing VPN client logic in AbstractVpnClient._get_common_name(): same COMMON_NAME_FORMAT, same truncation rules, same MAC fallback, and same shortuuid suffix. Please extract this to a shared helper that accepts a device and returns the generated common name, then use it from both VPN clients and standalone device certificates.
| }, | ||
| ] | ||
|
|
||
| def _auto_create_cert(self, name, common_name): |
There was a problem hiding this comment.
A lot of the certificate lifecycle code in this model repeats existing VPN certificate provisioning behavior: auto X.509 creation, organization assignment, default client extensions, CA-to-cert attribute copying, and revoke-on-delete. These are security-sensitive rules, so maintaining multiple copies is error-prone. Please factor out the stable shared pieces, at least common-name generation, certificate attribute copying, default client extensions, organization assignment, and revocation behavior.
| if self._state.adding: | ||
| return | ||
| try: | ||
| current = self.__class__.objects.get(pk=self.pk) |
There was a problem hiding this comment.
This extra query runs on every existing template validation just to know whether protected fields changed. The project already uses an _initial_* pattern for this in AbstractDevice. Please apply the same approach to the protected template fields, likely type, ca_id, and blueprint_cert_id, so validation can compare against initial values without fetching the current row each time.
| @mock.patch( | ||
| "openwisp_controller.config.tasks.regenerate_device_certificates_task.delay" | ||
| ) | ||
| def test_hardware_drift_signal_triggers_on_name_change(self, mocked_task): |
There was a problem hiding this comment.
Please avoid the current unclear wording around this behavior in tests, user-facing text, and docs. The feature is certificate regeneration after device identity fields change, currently name and MAC address, and that wording is much clearer. More generally, these new tests would be easier to maintain if split by feature area, for example signal-trigger tests, regeneration task tests, and DeviceCertificate model tests instead of adding all of them to the broad TestDevice class.
…s/1356-extend-abstract-template
CI Failure AnalysisHello @stktyagi,
|
Fixed failing tests after resolving merge conflict
Fixed formatting via formatter
4 participants
Checklist
Reference to Existing Issue
Closes #1356
Closes #1377
Closes #1357
Closes #1361
Closes #1358
Closes #1360
Closes #1359
Description of Changes
This PR establishes the database architecture, UI, API and lifecycle for standalone X.509 certificate templates.
Manual test plan
Setup
PKI->Certification Authoritiesand create two CAs:CA-1andCA-2.PKI->Certificatesand create two certificates to act as blueprints:Blueprint-1(Must useCA-1)Blueprint-2(Must useCA-2)Devicesand create a device (test-device).Template Creation and Validation
Configuration->Templatesand clickADD TEMPLATE.Certificate.CA-1.Blueprint-2(which belongs toCA-2). Try to save.Blueprint-1. Name the templateActive-Cert-Template. Save it.Device Provisioning
test-device.Active-Cert-Template. Save.PKI->Certificates.test-device. Its status should be valid (not revoked).Active Mutation Locks
Configuration->Templatesand editActive-Cert-Template(which is now assigned to an active device).Generic. Try to save.CA-2. Try to save.Blueprint-2(ensure you also change the CA so they match, triggering the active lock). Try to save.Revocation on Removal
test-device.Active-Cert-Templateentirely from the templates list. Save.PKI->Certificatesand locate the device's certificate.Context Configuration Injection
Go to
Configuration->Templates, openActive-Cert-Template, and copy its UUID from the URL bar (removing the dashes so it is a 32-character hex string).In the JSON configuration editor for the template, add a configuration block that references the certificate's UUID variables:
(Note: Replace
<uuid>with the actual 32-character hex string of the template).Click Save.
Go back to the Configuration page for
test-device(which has this template assigned) and click thePreview configurationbutton.Expected Result: The variables should be successfully resolved. In the preview, you should see the generated path (e.g.,
/etc/x509/cert-<uuid>.pem) and the literal-----BEGIN CERTIFICATE-----text instead of the raw{{ }}template tags.output.mp4