Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expose more objects in the configuration rendering feature #12814

Closed
MajesticFalcon opened this issue Jun 5, 2023 · 17 comments
Closed

Expose more objects in the configuration rendering feature #12814

MajesticFalcon opened this issue Jun 5, 2023 · 17 comments
Assignees
Labels
status: accepted This issue has been accepted for implementation type: feature Introduction of new functionality to the application
Milestone

Comments

@MajesticFalcon
Copy link

NetBox version

v3.5.3

Feature type

Change to existing functionality

Proposed functionality

Configuration rendering should expose the route-targets so more advanced configurations can be generated.

Use case

Unless I am missing something, currently, the implementation of configuration rendering seems to only provide access to the "device" object. Technically, you can loop through the device.interfaces.all() and create a new array of L2vpn terminations, however, I don't believe you can get the route-targets using this method.

Database changes

None.

External dependencies

None.

@MajesticFalcon MajesticFalcon added the type: feature Introduction of new functionality to the application label Jun 5, 2023
@jeremystretch
Copy link
Member

Expanded more generally, I think the underlying proposal here is to make NetBox models available directly as context variables when rendering a template. This would enable the template author to do e.g.

{% for rt in RouteTarget.objects.all() %}

This was a consideration raised during the initial work on the config rendering feature and ultimately punted simply because we weren't sure if it made sense at the time. I don't think there's anything necessarily precluding us from adding this, but would like to leave this open for discussion for a while.

We may also want to consider namespacing the models in some fashion, so that generic models like Device and Site don't conflict with user-provided context variables. (It may also be necessary to avoid naming collisions among core and plugin models.)

@jeremystretch jeremystretch added the status: under review Further discussion is needed to determine this issue's scope and/or implementation label Jun 6, 2023
@MajesticFalcon
Copy link
Author

MajesticFalcon commented Jun 6, 2023

Expanded more generally, I think the underlying proposal here is to make NetBox models available directly as context variables when rendering a template. This would enable the template author to do e.g.

{% for rt in RouteTarget.objects.all() %}

This was a consideration raised during the initial work on the config rendering feature and ultimately punted simply because we weren't sure if it made sense at the time. I don't think there's anything necessarily precluding us from adding this, but would like to leave this open for discussion for a while.

We may also want to consider namespacing the models in some fashion, so that generic models like Device and Site don't conflict with user-provided context variables. (It may also be necessary to avoid naming collisions among core and plugin models.)

Sounds good. In the meantime, is there a different method I could use to get the RT info usable inside the configuration rendering? The configuration snippet you provided would be great, but I suspect/hope there is a round-a-bout way to get the same information through the device object?

@DanSheps
Copy link
Member

DanSheps commented Jun 6, 2023

I am assuming this is for L2VPNs?

Two ways:

  • Iterate interfaces, iterate vlans, iterate l2vpn terminations, iterate route targets
  • Iterate interfaces, iterate l2vpn terminations, iterate route target

This is because l2vpns can be attached to both a single interface or a vlan. There is no direct way from devices itself, unfortunately.

@MajesticFalcon
Copy link
Author

MajesticFalcon commented Jun 6, 2023

@DanSheps, thank you! I just saw your Slack message as well. Apologies for the duplicate work. I'm going to leave this open for now in hopes to get some more discussion.

@ITJamie
Copy link
Contributor

ITJamie commented Jun 7, 2023

maybe it would be worth passing in a dummy object which had references to all the classes. eg a "netbox" object or similar, that way it would guarantee no confusion when it comes to the existing device object.
context render would look something like:

{% for rt in netbox.RouteTarget.objects.all() %}

@DanSheps
Copy link
Member

DanSheps commented Jun 7, 2023

I like that idea, and honestly I thought of roughly the same. We would have to find a way to capture all the plugins that could be required too however.

@ITJamie
Copy link
Contributor

ITJamie commented Jun 11, 2023

Maybe "netbox.plugins.classname" work for plugins. It could act as a safety wrapper too incase a context was using a pluginclass that got removed and safely error out/log? (Its actually kind of important that we make sure that works, otherwise some important config could be missing in a deployment config...)

@do9xe
Copy link

do9xe commented Jun 13, 2023

Expanded more generally, I think the underlying proposal here is to make NetBox models available directly as context variables when rendering a template. This would enable the template author to do e.g.

{% for rt in RouteTarget.objects.all() %}

This was a consideration raised during the initial work on the config rendering feature and ultimately punted simply because we weren't sure if it made sense at the time. I don't think there's anything necessarily precluding us from adding this, but would like to leave this open for discussion for a while.

I was also just looking into how I could get all the VLANs for a device. In my case I'd like to loop through all the VLANs that match the device and create them in the configuration. If you use dynamic VLAN assignment through 802.1X most devices need the VLANs created before they can be dynamically assigned to a port. Also they don't show in the running-config of the device.

@abhi1693
Copy link
Member

abhi1693 commented Aug 4, 2023

Here is a rough draft that provides the models in the context data. However, for this to work, I have renamed device to object

diff --git a/netbox/dcim/views.py b/netbox/dcim/views.py
index 5b93e5f0b..89964c376 100644
--- a/netbox/dcim/views.py
+++ b/netbox/dcim/views.py
@@ -1,5 +1,6 @@
 import traceback
 
+from django.apps import apps
 from django.contrib import messages
 from django.contrib.contenttypes.models import ContentType
 from django.core.paginator import EmptyPage, PageNotAnInteger
@@ -19,6 +20,7 @@ from circuits.models import Circuit, CircuitTermination
 from extras.views import ObjectConfigContextView
 from ipam.models import ASN, IPAddress, Prefix, VLAN, VLANGroup
 from ipam.tables import InterfaceVLANTable
+from netbox.registry import registry
 from netbox.views import generic
 from tenancy.views import ObjectContactsView
 from utilities.forms import ConfirmationForm
@@ -2078,7 +2080,12 @@ class DeviceRenderConfigView(generic.ObjectView):
     def get_extra_context(self, request, instance):
         # Compile context data
         context_data = instance.get_config_context()
-        context_data.update({'device': instance})
+        context_data.update({'object': instance})
+        app_ns = registry['model_features']['custom_fields'].keys()
+        for app in app_ns:
+            models = apps.get_app_config(app).get_models()
+            for model in models:
+                context_data.update({model._meta.model_name: model})
 
         # Render the config template
         rendered_config = None

image

@jeremystretch
Copy link
Member

IMO it would make sense to expose the models with their native names (e.g. ConsolePort rather than console_port) in the template context.

@jeremystretch
Copy link
Member

Thinking about this further, we probably want to incorporate the application name space in the context, to avoid potential future naming conflicts. For example, suppose a plugin introduces its own Site model, which would conflict with the native model.

We could namespace the models to avoid this, e.g. dcim.Site instead of Site. It breaks from convention a bit but does so to provide a minimally inconvenient solution for potential naming conflicts.

@abhi1693
Copy link
Member

abhi1693 commented Aug 7, 2023

Would you want a flat structure like dcim.Site, dcim.Rack or a grouped structure like {dcim: {Site: ..., Rack: ...}...}?

@DanSheps
Copy link
Member

DanSheps commented Aug 8, 2023

I am for the namespacing. Seems like a good plan, yes it does deviate but I think it will prevent future isssues.

@jeremystretch
Copy link
Member

@abhi1693 I think it would need to be nested, in order to access e.g. dcim.Site.objects.all() within a Jinja2 template.

@abhi1693
Copy link
Member

abhi1693 commented Aug 8, 2023

I have added the app namespace to the context data
image

@jeremystretch jeremystretch added status: accepted This issue has been accepted for implementation and removed status: under review Further discussion is needed to determine this issue's scope and/or implementation labels Aug 9, 2023
@jeremystretch jeremystretch added this to the v3.6 milestone Aug 9, 2023
jeremystretch added a commit that referenced this issue Aug 9, 2023
* exposes all models in device context data #12814

* added app namespaces to the context data

* revert object to device in context data

* moved context to render method of ConfigTemplate

* removed print

* Include only registered models; permit passed context data to overwrite apps

---------

Co-authored-by: Jeremy Stretch <jstretch@netboxlabs.com>
@Omega-Networks
Copy link

Please consider including the ipam models as well:

ipam.Vlans.objects.all()
ipam.VlanGroups.objects.all()
etc

Traceback:


Traceback (most recent call last):
  File "/opt/netbox/netbox/dcim/views.py", line 2066, in get_extra_context
    rendered_config = config_template.render(context=context_data)
                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/netbox/netbox/extras/models/configs.py", line 280, in render
    output = template.render(**_context)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/netbox/venv/lib/python3.11/site-packages/jinja2/environment.py", line 1301, in render
    self.environment.handle_exception()
  File "/opt/netbox/venv/lib/python3.11/site-packages/jinja2/environment.py", line 936, in handle_exception
    raise rewrite_traceback_stack(source=source)
  File "<template>", line 37, in top-level template code
  File "/opt/netbox/venv/lib/python3.11/site-packages/jinja2/sandbox.py", line 326, in getattr
    value = getattr(obj, attribute)
            ^^^^^^^^^^^^^^^^^^^^^^^
jinja2.exceptions.UndefinedError: 'dict object' has no attribute 'Vlans'

@DanSheps
Copy link
Member

DanSheps commented Oct 10, 2023

The model is ipam.VLAN not ipam.Vlans

Please open a discussion for any further assistance with this.

@netbox-community netbox-community locked as resolved and limited conversation to collaborators Oct 10, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status: accepted This issue has been accepted for implementation type: feature Introduction of new functionality to the application
Projects
None yet
Development

No branches or pull requests

7 participants