-
Notifications
You must be signed in to change notification settings - Fork 58
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
Add branch parameter to generate intended config view #871
Open
gsnider2195
wants to merge
6
commits into
develop
Choose a base branch
from
u/gas-828-generate-intended-config-branch
base: develop
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+385
−79
Open
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
f132623
Added `branch` parameter to generate intended config view.
gsnider2195 22aa6ee
Apply suggestions from code review
gsnider2195 c14c6da
docs update
gsnider2195 6cd352b
Merge branch 'u/gas-828-generate-intended-config-branch' of github.co…
gsnider2195 a2ac9f7
update docs
gsnider2195 b0e7c75
Merge branch 'develop' into u/gas-828-generate-intended-config-branch
gsnider2195 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
Added `branch` parameter to generate intended config view. |
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,6 +6,7 @@ | |
import logging | ||
from pathlib import Path | ||
|
||
from django.conf import settings as nautobot_settings | ||
from django.contrib.contenttypes.models import ContentType | ||
from django.utils.timezone import make_aware | ||
from drf_spectacular.types import OpenApiTypes | ||
|
@@ -21,13 +22,14 @@ | |
) | ||
from nautobot.dcim.models import Device | ||
from nautobot.extras.datasources.git import ensure_git_repository | ||
from nautobot.extras.models import GraphQLQuery | ||
from nautobot.extras.models import GitRepository, GraphQLQuery | ||
from nautobot_plugin_nornir.constants import NORNIR_SETTINGS | ||
from nornir import InitNornir | ||
from nornir_nautobot.plugins.tasks.dispatcher import dispatcher | ||
from packaging import version | ||
from rest_framework import mixins, status, viewsets | ||
from rest_framework.exceptions import APIException | ||
from rest_framework.generics import GenericAPIView | ||
from rest_framework.generics import GenericAPIView, RetrieveAPIView | ||
from rest_framework.mixins import DestroyModelMixin, ListModelMixin, RetrieveModelMixin, UpdateModelMixin | ||
from rest_framework.permissions import AllowAny, BasePermission, IsAuthenticated | ||
from rest_framework.response import Response | ||
|
@@ -237,20 +239,29 @@ def _get_object(self, request, model, query_param): | |
except model.DoesNotExist as exc: | ||
raise GenerateIntendedConfigException(f"{model.__name__} with id '{pk}' not found") from exc | ||
|
||
def _get_jinja_template_path(self, settings, device, git_repository): | ||
def _get_jinja_template_path(self, settings, device, git_repository, base_path=None): | ||
"""Get the Jinja template path for the device in the provided git repository.""" | ||
try: | ||
rendered_path = render_jinja2(template_code=settings.jinja_path_template, context={"obj": device}) | ||
except (TemplateSyntaxError, TemplateError) as exc: | ||
raise GenerateIntendedConfigException("Error rendering Jinja path template") from exc | ||
filesystem_path = Path(git_repository.filesystem_path) / rendered_path | ||
if base_path is None: | ||
filesystem_path = Path(git_repository.filesystem_path) / rendered_path | ||
else: | ||
filesystem_path = Path(base_path) / rendered_path | ||
if not filesystem_path.is_file(): | ||
msg = f"Jinja template {filesystem_path} not found in git repository {git_repository}" | ||
msg = f"Jinja template {rendered_path} not found in git repository {git_repository}" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like this change, should be easier for the user to understand. |
||
raise GenerateIntendedConfigException(msg) | ||
return filesystem_path | ||
|
||
@extend_schema( | ||
parameters=[ | ||
OpenApiParameter( | ||
name="branch", | ||
required=False, | ||
type=OpenApiTypes.STR, | ||
location=OpenApiParameter.QUERY, | ||
), | ||
OpenApiParameter( | ||
name="device_id", | ||
required=True, | ||
|
@@ -265,9 +276,12 @@ def _get_jinja_template_path(self, settings, device, git_repository): | |
), | ||
] | ||
) | ||
def get(self, request, *args, **kwargs): | ||
def get(self, request, *args, **kwargs): # pylint: disable=too-many-locals, too-many-branches | ||
"""Generate intended configuration for a Device.""" | ||
device = self._get_object(request, Device, "device_id") | ||
branch_param = request.query_params.get("branch") | ||
if branch_param and version.parse(nautobot_settings.VERSION) < version.parse("2.4.2"): | ||
raise GenerateIntendedConfigException("Branch support requires Nautobot v2.4.2 or later") | ||
graphql_query = None | ||
graphql_query_id_param = request.query_params.get("graphql_query_id") | ||
if graphql_query_id_param: | ||
|
@@ -298,17 +312,28 @@ def get(self, request, *args, **kwargs): | |
except Exception as exc: | ||
raise GenerateIntendedConfigException("Error trying to sync git repository") from exc | ||
|
||
filesystem_path = self._get_jinja_template_path(settings, device, git_repository) | ||
|
||
status_code, graphql_data = graph_ql_query(request, device, graphql_query.query) | ||
if status_code == status.HTTP_200_OK: | ||
try: | ||
intended_config = self._render_config_nornir_serial( | ||
device=device, | ||
jinja_template=filesystem_path.name, | ||
jinja_root_path=filesystem_path.parent, | ||
graphql_data=graphql_data, | ||
) | ||
if branch_param: | ||
with git_repository.clone_to_directory_context(branch=branch_param) as git_repo_path: | ||
filesystem_path = self._get_jinja_template_path( | ||
settings, device, git_repository, base_path=git_repo_path | ||
) | ||
intended_config = self._render_config_nornir_serial( | ||
device=device, | ||
jinja_template=filesystem_path.name, | ||
jinja_root_path=filesystem_path.parent, | ||
graphql_data=graphql_data, | ||
) | ||
else: | ||
filesystem_path = self._get_jinja_template_path(settings, device, git_repository) | ||
intended_config = self._render_config_nornir_serial( | ||
device=device, | ||
jinja_template=filesystem_path.name, | ||
jinja_root_path=filesystem_path.parent, | ||
graphql_data=graphql_data, | ||
) | ||
except Exception as exc: | ||
raise GenerateIntendedConfigException(f"Error rendering Jinja template: {exc}") from exc | ||
|
||
|
@@ -372,3 +397,18 @@ def _render_config_nornir_serial(self, device, jinja_template, jinja_root_path, | |
) | ||
else: | ||
return results[device.name][1][1][0].result["config"] | ||
|
||
|
||
@extend_schema(exclude=True) | ||
class GitRepositoryBranchesView(NautobotAPIVersionMixin, RetrieveAPIView): | ||
"""API view for extras.GitRepository with branches.""" | ||
|
||
name = "Git Repository with Branches" | ||
permission_classes = [IsAuthenticated] | ||
queryset = GitRepository.objects.all() | ||
serializer_class = serializers.GitRepositoryWithBranchesSerializer | ||
|
||
def get_queryset(self): | ||
"""Override the original get_queryset to apply permissions.""" | ||
queryset = super().get_queryset() | ||
return queryset.restrict(self.request.user, "view") |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For this whole docs section (Developing Intended Configuration Templates), could you please make a small change and flip the order? You start talking about the feature and how to use it via the API, but given we're in the "User Guide" let's start with the screenshot of the UI and what they can do there (what inputs they can provide, the 3 panels, what's shown there, noting the git pull as well etc.)
And then at the end provide the option of using the REST API endpoints (I'd even have that info under developer guide, but I don't see any of that in GC's docs so nevermind).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made these changes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, thank you!