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

Bug: VM Interface vlan list in UI uses invalid option #5058

Closed
tlourey opened this issue Jan 6, 2024 · 7 comments · Fixed by #5124 or #5286
Closed

Bug: VM Interface vlan list in UI uses invalid option #5058

tlourey opened this issue Jan 6, 2024 · 7 comments · Fixed by #5124 or #5286
Labels
type: bug Something isn't working as expected

Comments

@tlourey
Copy link
Contributor

tlourey commented Jan 6, 2024

Environment

  • Nautobot version (Docker tag too if applicable): 2.1.0
  • Python version: 3.10
  • Database platform, version: postgres, 14.10
  • Middleware(s):

Steps to Reproduce

  1. Create a VLAN
  2. Create a Cluster Type
  3. Create a Cluster
  4. Create a VM in the cluster
  5. Create a interface on the vm
  6. Selected Access, Tagged or Tagged (all)

Expected Behavior

Show a list of vlans

Observed Behavior

Dropdown shows "The results could not be loaded"

Was able to replicate on demo.nautobot.com

It appears that this dropdown tries to use an API query attribute/option called location_id however this seems to generate an 400.

Looking at the API browser with GET /api/ipam/vlans/?q=&limit=50&offset=0&depth=0&location_id=<<GUID>> or even GET /api/ipam/vlans/?q=&limit=50&offset=0&depth=0&location_id=null returns

HTTP 400 Bad Request
API-Version: 2.1
Allow: GET, POST, PUT, PATCH, DELETE, HEAD, OPTIONS
Content-Type: application/json
Vary: Accept

{
    "location_id": [
        "Unknown filter field"
    ]
}
@tlourey tlourey added triage This issue is new and has not been reviewed. type: bug Something isn't working as expected labels Jan 6, 2024
@glennmatthews
Copy link
Contributor

Thanks for the report! Looks like there are quite a few cases in nautobot/virtualization/forms.py that are using location_id as a filter parameter, which should generally be location instead.

@glennmatthews glennmatthews removed the triage This issue is new and has not been reviewed. label Jan 8, 2024
@tlourey
Copy link
Contributor Author

tlourey commented Jan 16, 2024

Hi @glennmatthews, still very very new to the model, but is it just a matter of the references under query_params being changed from

        query_params={
            "location_id": "null",
        },

to

        query_params={
            "location": "null",
        },

?

If so, I'll give it a crack and if it pass any local linting & tests, follow the contributing guide to submit a PR if it will help. If there's more to it, I'll keep out of the way :D

@glennmatthews
Copy link
Contributor

That should be the gist of it, yes. Thank you!

glennmatthews added a commit that referenced this issue Jan 22, 2024
* Bug: VM Interface vlan list in UI uses invalid option
Fixes #5058

* added changelog fragment

* grammer

* Update changes/5058.fixed

Co-authored-by: Hanlin Miao <46973263+HanlinMiao@users.noreply.github.com>

---------

Co-authored-by: TLCF <31373129+TLCF@users.noreply.github.com>
Co-authored-by: Glenn Matthews <glenn.matthews@networktocode.com>
Co-authored-by: Hanlin Miao <46973263+HanlinMiao@users.noreply.github.com>
glennmatthews added a commit that referenced this issue Jan 22, 2024
* Added global filtering to Job Result log table

* Fix integration test failure

* Apply suggestions from code review

Co-authored-by: Gary Snider <75227981+gsnider2195@users.noreply.github.com>

* use self.browser.is_text_not_present instead of time.sleep

* PR feedback

* fix migration

* fixes early return conditional in ensure_git_repository - again. (#5043)

The type returned by `git.Repo.rev_parse("HEAD")` was still off, i.e. not
a string. This commit fixes this by converting it to a string.

Co-authored-by: Hanlin Miao <46973263+HanlinMiao@users.noreply.github.com>
Co-authored-by: Glenn Matthews <glenn.matthews@networktocode.com>

* Improve IPAddressEditView and IPAddressAssignView logic (#5054)

* Improve IPAddressEditView and IPAddressAssignView logic.

* Number change fragments

* Make ruff happy

---------

Co-authored-by: Hanlin Miao <46973263+HanlinMiao@users.noreply.github.com>

* bump version

* Process CSV import one row at a time to permit back-references (#4977)

* Process CSV import one row at a time to permit back-references to earlier rows

* Fix failing test

* Change fragment

---------

Co-authored-by: Hanlin Miao <46973263+HanlinMiao@users.noreply.github.com>

* fix: devcontainer (#4906)

* fix: devcontainer

* refactor(devcontainer): remove pylint and pre-commit leftover

* fix(devcontainer): arch detection

* feat(devcontainer): replace black, flake8 and isort with ruff

* refactor(devcontainer): remove prettier and eslint extensions

* refactor: remove eslint fixall

* refactor(devcontainer): move superuser information to dev.env

* fix(devcontainer): update comment with ruff instead of black

* update docs and add changelogs

* Apply suggestions from code review

* move changelogs to housekeeping category

---------

Co-authored-by: Gary Snider <75227981+gsnider2195@users.noreply.github.com>
Co-authored-by: Glenn Matthews <glenn.matthews@networktocode.com>

* tt

* ruff fix

* Fixed bug with invoke cli and invoke nbshell. (#5079)

Co-authored-by: Glenn Matthews <glenn.matthews@networktocode.com>

* Increase grid breakpoints by 240px (#5080)

* Update nautobot/extras/tests/integration/test_jobs.py

* bump packaging (#5077)

* bump packaging

* update lock file

* create change fragment

* update packaging dependency spec to '>=23.1'

* Update changes/5076.housekeeping

Co-authored-by: Glenn Matthews <glenn.matthews@networktocode.com>

---------

Co-authored-by: Anthony House <anthony.house@networktocode.com>
Co-authored-by: Glenn Matthews <glenn.matthews@networktocode.com>

* Job-related bug fixes (#5095)

* Job-related bug fixes

* Renumber change fragment

* [Snyk] Security upgrade jinja2 from 3.1.2 to 3.1.3 (#5086)

* Bump gitpython from 3.1.40 to 3.1.41 (#5083)

* A few performance updates (#5024)

Co-authored-by: Glenn Matthews <glenn.matthews@networktocode.com>
Co-authored-by: Gary Snider <75227981+gsnider2195@users.noreply.github.com>

* Skip test

* Removes startplugin mgmt cmd. (#5082)

Co-authored-by: Glenn Matthews <glenn.matthews@networktocode.com>

* Remove `files/get` endpoint which is not used from Nautobot 2.0 (#5115)

* remove `files/get` endpoint

* Update nautobot/core/tests/test_views.py

Co-authored-by: Glenn Matthews <glenn.matthews@networktocode.com>

* update based on review

---------

Co-authored-by: Glenn Matthews <glenn.matthews@networktocode.com>

* more flexible default sanitizer pattern (#4994)

* more flexible default sanitizer pattern

* Fix re.compile syntax

* add test for new patterns create changelog fragment

* fix test cases for new pattern

* update other santization references

* add examples of matching patterns

---------

Co-authored-by: Hanlin Miao <46973263+HanlinMiao@users.noreply.github.com>
Co-authored-by: Glenn Matthews <glenn.matthews@networktocode.com>

* More job fixes (#5102)

* More job fixes

* Renumber change fragment

* Revert redundant logging

* Enhance sanitize() to handle data other than strings, fix tests

* Add checks to runjob command

* Ruff

* Additional change fragments

* Address review feedback, add test coverage

* BugFix cf table entry rendering (#5081)

* BugFix cf table entry rendering

* Update nautobot/core/tables.py

Co-authored-by: Glenn Matthews <glenn.matthews@networktocode.com>

* Create 5081.fixed

---------

Co-authored-by: Glenn Matthews <glenn.matthews@networktocode.com>

* Fix #4075 - sortability of Device Bays list view by installed device status (#5110)

Co-authored-by: Hanlin Miao <46973263+HanlinMiao@users.noreply.github.com>

* Update PULL_REQUEST_TEMPLATE.md (#5118)

* Update PULL_REQUEST_TEMPLATE.md

* Create 5118.housekeeping

---------

Co-authored-by: Glenn Matthews <glenn.matthews@networktocode.com>

* Fixed Sync Git Repository requires non-matching permissions for UI vs API (#5128)

* Adds location & rack group to device bulk edit query params (#5113)

* Sanitize `render_markdown()` output with `nh3` library (#5133)

* Fix GHSA-v4xv-795h-rv4h.

* Renumber change fragments

* Address review feedback

* Test fix

* Ruff

* Review feedback

* Update nautobot/core/forms/fields.py

* Towncrier and version bump

* Fixes #5058: use location instead of location_id in vm forms (#5124)

* Bug: VM Interface vlan list in UI uses invalid option
Fixes #5058

* added changelog fragment

* grammer

* Update changes/5058.fixed

Co-authored-by: Hanlin Miao <46973263+HanlinMiao@users.noreply.github.com>

---------

Co-authored-by: TLCF <31373129+TLCF@users.noreply.github.com>
Co-authored-by: Glenn Matthews <glenn.matthews@networktocode.com>
Co-authored-by: Hanlin Miao <46973263+HanlinMiao@users.noreply.github.com>

* Update release note

* Add `TreeManager.max_depth` cachedproperty as an alternative to `max_tree_depth()` (#5131)

* Add TreeManager.max_depth cachedproperty as an alternative to max_tree_depth()

* Change fragment

* Add signal to clear max_depth cache when needed and test case to verify it

* Update release-note

---------

Co-authored-by: Timizuo <ebideritimizuo@gmail.com>
Co-authored-by: Timizuo <94907097+timizuoebideri1@users.noreply.github.com>
Co-authored-by: Gary Snider <75227981+gsnider2195@users.noreply.github.com>
Co-authored-by: Leo Kirchner <Kircheneer@users.noreply.github.com>
Co-authored-by: Hanlin Miao <46973263+HanlinMiao@users.noreply.github.com>
Co-authored-by: Eric Jacob <erjac77@gmail.com>
Co-authored-by: housepbass <80693460+housepbass@users.noreply.github.com>
Co-authored-by: Anthony House <anthony.house@networktocode.com>
Co-authored-by: Bryan Culver <31187+bryanculver@users.noreply.github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: John Anderson <lampwins@gmail.com>
Co-authored-by: Josh VanDeraa <jv@networktocode.com>
Co-authored-by: Gerasimos Tzakis <gertzakis@gmail.com>
Co-authored-by: Jeff Kala <48843785+jeffkala@users.noreply.github.com>
Co-authored-by: Jacob McGill <9847006+jmcgill298@users.noreply.github.com>
Co-authored-by: Joe Wesch <10467633+joewesch@users.noreply.github.com>
Co-authored-by: TL <9435779+tlourey@users.noreply.github.com>
Co-authored-by: TLCF <31373129+TLCF@users.noreply.github.com>
@anton-wg2
Copy link

Hello.
I still have the problem on 2.1.3
It seems the wrong filter is still there

[Tue Feb  6 09:35:49 2024] GET /api/ipam/vlans/?q=&limit=50&offset=0&depth=0&location=null&location_id=e8573f65-41cd-40cc-a192-70515809c689 => generated 187953 bytes in 33 msecs (HTTP/1.1 400) 9 headers in 447 bytes (1 switches on core 0)

so location=null is fine, but location_id=<uuid> fails

@glennmatthews
Copy link
Contributor

Looks like the fix was incomplete - see also analysis at #5124 (comment). @tlourey were you going to be able/willing to submit a followup PR, or would you need us to run with fixing this?

@tlourey
Copy link
Contributor Author

tlourey commented Feb 6, 2024 via email

@tlourey tlourey mentioned this issue Feb 14, 2024
7 tasks
@tlourey
Copy link
Contributor Author

tlourey commented Feb 14, 2024

Looks like the fix was incomplete - see also analysis at #5124 (comment). @tlourey were you going to be able/willing to submit a followup PR, or would you need us to run with fixing this?

Draft PR #5286 running tests now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug Something isn't working as expected
Projects
No open projects
Status: Done
3 participants