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

Dynamic Remediation Functions and Mapping to override default hier_config #770

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from

Conversation

sdoiron0330
Copy link

CLOSES #755

Adds DynamicRemediationFunction and DynamicRemediationMapping models to import functions from a Git Repository that should be run during Config Compliance when remediation config is generated.

image image image

STILL TODO:

  • documentation for users
  • unit tests

Copy link
Contributor

@jeffkala jeffkala left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do a deeper review shortly; however, first thing that is a must before we can merge is:

  1. Must have docs
  2. Should have at least a few TestCases.



class DynamicRemediationMappingForm(NautobotModelForm):
"""Form for ConfigPlan instances."""
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fix the docstring.

@mzbroch
Copy link
Contributor

mzbroch commented Jun 12, 2024

I am trying to find a right balance between usability and configuration overhead -when we introduced Remediation, we also implemented Custom Remediation which could solve this use case too.

What I understand from example functions is what we try to solve by implementing those remediation methods, will probably be applicable for 80% use cases. I don't think each user should create Remediation settings first , then also create multiple "Dynamic Remediation Mappings" - perhaps we could integrate this under "remediation settings" jsonField, or straight (natively) into HierConfig ?

@itdependsnetworks
Copy link
Contributor

Can I get a bit of a better explanation as to what this is doing? I am not really understanding the use case or the strong driver for an additional model.

@jeffkala
Copy link
Contributor

Can I get a bit of a better explanation as to what this is doing? I am not really understanding the use case or the strong driver for an additional model.

#755 covers what this is trying to solve. A few months back there was an ask in public slack around customizing the way ACLs were being generated/remediated via Heir_config. James mentioned a custom remediation function from the heir_config side of the house, this allows for creating and syncing in custom remediation functions from Git.

@itdependsnetworks
Copy link
Contributor

Can we just set it up to have a flavor of custom remediation? E.g. add custom hierconfig to the list of hier_config and customer remediation?

@@ -36,6 +42,51 @@ def refresh_git_backup(repository_record, job_result, delete=False): # pylint:
)


def refresh_git_gc_dynamic_remediations(repository_record, job_result, delete=False): # pylint: disable=unused-argument
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't looked at the code, but this part makes sense, I think that this is mostly what needs to be updated, more specifically there shouldn't be a reason to create a new model here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it should be possible to have a similar solution :

  • custom remediation method is provided via nautobot_config.py or packaged
  • custom remediation points to a dispatcher provided by git.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

made some changes to remove the model and just load modules from the git repo when doing remediation; lmk what you think

@itdependsnetworks
Copy link
Contributor

Would like to pursue 2 things.

  1. Review with Nautobot Core if this is the best approach for integration with Git, as this is a newer (if not first) pattern within the app ecosystem.
  2. Review on Zoom how this works.

@jeffkala to take lead at point 1.

@mzbroch
Copy link
Contributor

mzbroch commented Jul 8, 2024

I still think what is needed is the integration under "Custom Remediation". We have already built a pattern that now can be extended for git , without the need of introducing the third Remediation Type.

The existing _get_hierconfig_remediation does not have to be re-written (duplicated code), but can be re-used.

@@ -210,6 +224,7 @@ def _get_hierconfig_remediation(obj):
ComplianceRuleConfigTypeChoice.TYPE_JSON: _get_json_compliance,
ComplianceRuleConfigTypeChoice.TYPE_XML: _get_xml_compliance,
RemediationTypeChoice.TYPE_HIERCONFIG: _get_hierconfig_remediation,
RemediationTypeChoice.TYPE_DYNAMIC_HIERCONFIG: _get_hierconfig_remediation,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not tracking to me, we are pointing it to the same function of _get_hierconfig_remediation(), which would indicated to me, it's the same feature with a different name, but I think I am missing something, can you elaborate on this one?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i added that as an option so that if your environment had dynamic functions loaded via git and for a particular remediation setting you didn't need or want to use any dynamic functions at all, it bypassed that loop over the dynamic functions entirely

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.

Support a new custom remediation specifically to allow for dynamic rememdation overrides heir_config exposes
4 participants