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 groups should fail closed if filter is invalid #4559

Merged
merged 3 commits into from
Sep 29, 2023

Conversation

glennmatthews
Copy link
Contributor

Closes: #N/A but see also nautobot/nautobot-app-golden-config#626

What's Changed

  • If a DynamicGroup has an invalid filter, its members() was failing open (all objects returned) instead of closed (no objects returned). This adds logic to call filterset.is_valid() and fail closed if the filterset is invalid.
  • Enhance the audit_dynamic_groups command to call DynamicGroup.clean_filter() to also flag groups with invalid filter values.
>>> Auditing existing DynamicGroup data for invalid filters ...
    DynamicGroup "Bad Filter" has an invalid filter: {'platform': ['Enter a list of values.']}

>>> Please fix the broken filters stated above according to the documentation available at:
<nautobot-home>/static/docs/installation/upgrading-from-nautobot-v1.html#ui-graphql-and-rest-api-filter-changes

This is important for 2.0 as many existing DGs may have filters that become invalid as a result of the upgrade and should fail closed instead of failing open.

TODO

  • Explanation of Change(s)
  • Added change log fragment(s) (for more information see the documentation)
  • Attached Screenshots, Payload Example
  • Unit, Integration Tests
  • n/a Documentation Updates (when adding/changing features)
  • n/a Example Plugin Updates (when adding/changing features)
  • Outline Remaining Work, Constraints from Design

@bryanculver bryanculver merged commit 302c038 into develop Sep 29, 2023
21 checks passed
@bryanculver bryanculver deleted the u/glennmatthews-dg-fail-closed branch September 29, 2023 01:42
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.

None yet

2 participants