Skip to content

#98 - Add optional ConfigContext to Custom Objects#602

Merged
bctiemann merged 8 commits into
featurefrom
98-config-context
Jul 2, 2026
Merged

#98 - Add optional ConfigContext to Custom Objects#602
bctiemann merged 8 commits into
featurefrom
98-config-context

Conversation

@arthanson

@arthanson arthanson commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Fixes: #98

Adds a config_context_enabled checkbox to Custom Object Types. When enabled, instances of that type gain NetBox's local_context_data JSON field — a per-object data blob usable for downstream config generation (e.g. Ansible, per the issue).

  • Defaults to off.
  • Can only be set at creation - disabled when editing an existing type, since the backing column is created once when the type's table is built (managed = False thereafter).

Inherit config context

Config-context-enabled custom objects can now inherit the ConfigContexts that apply to NetBox objects they reference - the same site/tenant/role/etc. contexts a Device there would get — merged under their own local_context_data.

How: by field-naming convention, no configuration. If a type has a single object field named site, tenant, role, platform, location, device_type, or cluster pointing at the matching model, get_config_context() builds a proxy from those values and reuses NetBox's ConfigContextQuerySet.get_for_object() — so all of NetBox's matching, hierarchy (region/site-group/tenant-group/cluster derivation), weight ordering, and tag matching apply. local_context_data is merged last and always wins. A type with none of these fields stays local-only.

Notes:

  • Field names are unique per type, so there's no dimension ambiguity by construction.
  • A field must match both name and target model; a mistyped/mispointed field is ignored.
  • The Config Context tab gains the standard 3-panel view (Rendered / Local / Source Contexts); the source list is RBAC-filtered like NetBox's own view.

@netboxlabs netboxlabs deleted a comment from github-actions Bot Jun 30, 2026
@netboxlabs netboxlabs deleted a comment from github-actions Bot Jul 1, 2026
@netboxlabs netboxlabs deleted a comment from github-actions Bot Jul 1, 2026
@arthanson

Copy link
Copy Markdown
Contributor Author

@claude /review Only report on issues of potential concern; we don't need commentary on anything that doesn't require further consideration.

@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown

Claude finished @arthanson's task in 4m 12s —— View job


PR Review — #98 Add optional ConfigContext to Custom Objects

  • Read changed files
  • Analyze for issues of concern
  • Post review findings

Issues of Concern

1. deepmerge(data, context) — likely wrong argument (models.py:1044)

contexts = ConfigContext.objects.get_for_object(source, aggregate_data=True) or []
for context in contexts:
    data = deepmerge(data, context)          # ← context or context.data ?

Standard NetBox pattern (e.g. Device.get_config_context()) calls get_for_object() without aggregate_data=True and does deepmerge(data, context.data). The aggregate_data=True variant is non-standard here. If it returns ConfigContext model instances (not raw dicts), deepmerge will iterate over the model object's keys incorrectly (or raise AttributeError), silently producing empty/wrong output. This should be verified against the actual NetBox queryset implementation or changed to the known-safe form:

for context in ConfigContext.objects.get_for_object(source):
    data = deepmerge(data, context.data)

The tests cover the happy path but will only confirm correctness if run against real NetBox — recommend verifying this explicitly. Fix this →


2. SimpleNamespace proxy may not satisfy get_for_object attribute access (models.py:1024-1025)

_config_context_source() builds the proxy with:

proxy = SimpleNamespace(**dims)   # dims["site"] = <Site instance>, not <site_id>
proxy.tags = self.tags

If ConfigContextQuerySet.get_for_object() accesses FK IDs internally (e.g. obj.site_id, obj.tenant_id) the proxy will return None for those lookups (since SimpleNamespace has no *_id attributes), silently producing no aggregation. The site test case covers the happy path, but there's no test for tenant, role, platform, location, device_type, or cluster dimensions individually. A change in NetBox's internal queryset access pattern could silently break this without test coverage catching it. At minimum, the proxy should be verified to expose *_id attributes where needed, or test coverage should be extended to other dimensions.


3. config_context_enabled schema desync via QuerySet.update() or shell save()

The flag is enforced immutable only through clean(). Any of the following bypass it silently:

CustomObjectType.objects.filter(pk=pk).update(config_context_enabled=True)
cot.config_context_enabled = True; cot.save(update_fields=["config_context_enabled"])

If an operator does this on an existing type, every subsequent query on that type fails with column "local_context_data" does not exist, breaking the type entirely. The code comments acknowledge the bypass, but there's no DB-level guard (e.g. a migration that adds a check constraint, or a pre_save signal that enforces it). This is an operational footgun worth protecting against, or at minimum warning about in docs.


4. Global ConfigContexts excluded from types with no convention fields — docs gap

The PR description notes this is intentional, but the user-facing docs (docs/index.md) don't mention it. A user who configures a global ConfigContext (no site/tenant assignment) will find that custom objects without convention-named fields receive no config context at all — unlike Devices, which always inherit global contexts. This should be called out in the docs so it's not a silent surprise.


@arthanson

arthanson commented Jul 1, 2026

Copy link
Copy Markdown
Contributor Author

claude feedback:

1 - Not valid.

2 - *_id concern not valid; added tenant + role dimension tests for coverage.

3 - Valid; docs note added then removed per your request. Enforcement stays at clean().

4 - already addressed in docs.

@arthanson arthanson marked this pull request as ready for review July 1, 2026 18:38
@arthanson arthanson requested a review from bctiemann July 1, 2026 18:38
@bctiemann bctiemann merged commit 5b269bc into feature Jul 2, 2026
7 checks passed
@bctiemann bctiemann deleted the 98-config-context branch July 2, 2026 14:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants