netbox-community / pynetbox

Python API client library for Netbox.
Apache License 2.0
580 stars 172 forks source link

Auth fails getting API version with Netbox 4.0.0 #612

Closed terraflubb closed 5 months ago

terraflubb commented 6 months ago

pynetbox version

v7.3.3

NetBox version

v4.0.0

Python version

3.11

Steps to Reproduce

  1. Upgrade to Netbox 4.0.0
  2. Run this (auth is correct in the environment)
import pynetbox
import os

NETBOX_URL = os.environ.get('NETBOX_URI')
NETBOX_KEY = os.environ.get('NETBOX_API_KEY')

nb = pynetbox.api(url=NETBOX_URL, token=NETBOX_KEY)

print(nb.version)

Expected Behavior

It should say:

'4.0'

Observed Behavior

This will throw:

pynetbox.core.query.RequestError: The request failed with code 403 Forbidden: {'detail': 'Authentication credentials were not provided.'}

Fix

The issue is that in get_version, we aren't including any auth in the header, which was never a problem until Netbox 4.0.0. One could argue either way if it's an issue with Netbox or pynetbox, but the fix is pretty harmless:

By turning: https://github.com/netbox-community/pynetbox/blob/master/pynetbox/core/query.py#L188-L190

Into something more like: https://github.com/netbox-community/pynetbox/blob/master/pynetbox/core/query.py#L206-L208

This issue stops! :tada:

I have a PR to fix this which I can't create until this issue exists. Suggested fix is here #613

Fredouye commented 6 months ago

Hi

with #613, Ansible modules such as netbox.netbox.netbox_ip_address are working once again.

kotso commented 6 months ago

Spent 5+ hours debugging my automation, ended up with this error, came to open bug report and found this :(((

mto2 commented 6 months ago

I'm hitting the same problem. Implementing locally solution that is in the above PR is solving the problem.

tyldum commented 6 months ago

Netbox 4.0.1 changed the default LOGIN_REQUIRED to true, so every call should be authenticated.

https://github.com/netbox-community/netbox/pull/16122

markkuleinio commented 6 months ago

Unfortunately that's not relevant in the problem at hand.

btriller commented 6 months ago

Netbox 4.0.1 changed the default LOGIN_REQUIRED to true, so every call should be authenticated.

netbox-community/netbox#16122

API requests seems to include API-Version header for 403/404 responses.

% curl -sw '%{http_code}\n%{header_json}' -H content-type:application/json -o /dev/null https://demo.netbox.dev/api/foo | jq -r '.["api-version"]?[] // .'
404
4.0
% curl -sw '%{http_code}\n%{header_json}' -H content-type:application/json -o /dev/null https://your.authed.instance/api/ | jq -r '.["api-version"]?[] // .'
403
4.0
xibriz commented 6 months ago

It seems to me that the actual bug is not missing authentication, but that the code requires req.ok and when you get a 403 that is not OK even if the api-version is in the headers https://requests.readthedocs.io/en/latest/api/#requests.Response.ok

https://github.com/netbox-community/pynetbox/blob/1b397f04ad104970e3f7d76efe990e56da8a7727/pynetbox/core/query.py#L213-L216

EDIT: as @btriller already stated 😆

terraflubb commented 6 months ago

Ah, right. Even an unauthorized query returns the current Netbox version in the header. So passing along the auth only avoided this trap, didn't really solve the underlying issue.

alensfor commented 6 months ago

Hello all, I am facing the same bug and currently cannot use the netbox ansible module because it relies on pynetbox. I see multiple PR and fixes mentioned and that this repository was last updated in January. Is this an active project that will publish some of the mentioned fixes or should we look at workarounds for the time being ? I try to pull and copy all the files from https://github.com/netbox-community/pynetbox/pull/613 but in my case that didn't help. Thank you

xibriz commented 6 months ago

@terraflubb I think both suggestions solve the problem... It's just a matter of which way to go :)

xibriz commented 6 months ago

@alensfor I would just go for the fix in #616

We have Netbox 4.0.3 in our staging environment so I hope a solution gets merged soon.

terraflubb commented 6 months ago

@xibriz :grinning: I'll take any solution as long as it gets merged sometime soon. (even both!)

alensfor commented 6 months ago

Netbox 4.0 failures netbox-community/Device-Type-Library-Import#134

Thank you @xibriz ; that worked for some jobs running via ansible-core using CLI, but other jobs running from Ansible AWX, despite trying to apply the same code changes within the Execution Environment image, failed with the same API error. We will wait for a merge and release I think.

xibriz commented 6 months ago

@alensfor Strange. I was asked today if we could do a patch while we wait for pynetbox to be updated. We also run a custom EE inside AWX so it would be interesting to compare notes.

Have you verified that the code has changed by checking inside the container?

alensfor commented 5 months ago

Hi @xibriz , I cloned the pynetbox repo and did a pr checkout on https://github.com/netbox-community/pynetbox/pull/616/files Then copied the entire /usr/local/lib/python3.12/site-packages/pynetbox/ content in the container, I verified the fixed code was visible. Then did a commit of my docker image. From AWX unfortunately, the error output is still lacking details

msg: Failed to establish connection to NetBox API
exception: |2
    File "/tmp/ansible_netbox.netbox.netbox_device_payload_66jj3nnb/ansible_netbox.netbox.netbox_device_payload.zip/ansible_collections/netbox/netbox/plugins/module_utils/netbox_utils.py", line 777, in _connect_netbox_api
      self.version = nb.version
                     ^^^^^^^^^^
    File "/usr/local/lib/python3.12/site-packages/pynetbox/core/api.py", line 113, in version
      ).get_version()
        ^^^^^^^^^^^^^
    File "/usr/local/lib/python3.12/site-packages/pynetbox/core/query.py", line 191, in get_version
      req = self.http_session.get(
            ^^^^^^^^^^^^^^^^^^^^^^
    File "/usr/local/lib/python3.12/site-packages/requests/sessions.py", line 602, in get
      return self.request("GET", url, **kwargs)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    File "/usr/local/lib/python3.12/site-packages/requests/sessions.py", line 589, in request
      resp = self.send(prep, **send_kwargs)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    File "/usr/local/lib/python3.12/site-packages/requests/sessions.py", line 703, in send
      r = adapter.send(request, **kwargs)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    File "/usr/local/lib/python3.12/site-packages/requests/adapters.py", line 700, in send
      raise ConnectionError(e, request=request)
 I will keep hoping the multiple PR waiting approval on this repo will be merged soon. That'd greatly help us.
xibriz commented 5 months ago

@alensfor I created a diff-file between the original query.py and the patched file in #616

--- query.py    2024-06-04 07:32:12.893691318 +0000
+++ pr616.py    2024-06-04 07:35:30.227466867 +0000
@@ -192,7 +192,8 @@
             self.normalize_url(self.base),
             headers=headers,
         )
-        if req.ok:
+        # https://github.com/netbox-community/pynetbox/pull/616
+        if req.ok or req.status_code == 403:
             return req.headers.get("API-Version", "")
         else:
             raise RequestError(req)

When building the EE I had to install patch and then apply the patch under additional_build_steps. This works as expected.

---
version: 3
images:
  base_image:
    name: quay.io/centos/centos:stream9
dependencies:
  ansible_core:
    package_pip: ansible-core>=2.15
  ansible_runner:
    package_pip: ansible-runner
  galaxy: |
    ---
    roles: []
    collections: []
  system: |
    ...
    patch [platform:rpm]
  python: |
    ...
additional_build_steps:
  append_final:
    # Patch pynetbox
    - COPY pynetbox/pynetbox_patch.diff pynetbox_patch.diff
    - RUN patch /usr/local/lib/python3.9/site-packages/pynetbox/core/query.py < pynetbox_patch.diff
alensfor commented 5 months ago

Thanks a lot @xibriz , I will try your method. I was attempting to fix this query.py but also all the files changes listed in PR Still, if an approver could review the current MR after 5 months without update, that'd be great. Thank you all.

bluikko commented 5 months ago

This breakage seems to happen every so often and in the past I have tried to raise discussion about the "ecosystem" compatibility beyond NetBox proper.
I recall someone from "the team" replied something with not much substance and that was all.

There definitely could be more coordination and planning since pynetbox has broken several times after changes to NetBox. Maybe pynetbox is now just 1 guy doing this on his free time after some active maintainer(s) have left/been kicked out? Meanwhile there are no more updates (even security updates) to NetBox previous version that we are stuck at.

I wonder how anyone using the paid NetBox Cloud deals with this. Do they have a "special" pynetbox version that works with the v4 in the NetBox Cloud?
I guess all the subscribers, especially on the low tier(s), cannot demand a v3 on the NetBox Cloud due to their integrations being broken?

nicolai-hornung-bl commented 5 months ago

@jeremystretch could a pynetbox compatibility test be included in the CI of netbox-community/netbox?

It does seem like this package is more of an afterthought, which is sad since through the ansible-collection many depend on it to work after NetBox upgrades. All in all I think we would all appreciate if the PR for the current fix would be merged and released sooner rather than later.

bluikko commented 5 months ago

could a pynetbox compatibility test be included in the CI of netbox-community/netbox?

This sounds great and convenient. But I would be very surprised if there could be tests or any dependencies whatsoever with these kind of third party code in the "official" NetBox.
Remember that the official position is that pynetbox is just another third party piece that has nothing to do with NetBox proper.

nicolai-hornung-bl commented 5 months ago

@bluikko kind of defeats the slogan "The premier source of truth powering network automation." if one of the critical packages supporting the automation part is regularly borked 🤣.

alensfor commented 5 months ago

I concur with Nicolai, the ability to connect Ansible with Netbox is a core component for us. Losing this ability to use the standard Pynetbox package (or having to implement custom workarounds/fixes) is impacting our interest in the Netbox product. That said, the community is reactive and helpful, providing solution to fix this glitch, so thank you all !

TheNetworkGuy commented 5 months ago

My sync script is also affected and i got notified by it with a new issue from @patricklind. Details:

LOGIN_REQUIRED=True

Python 3.12.4
pynetbox 7.3.3
Netbox 4.0.6

I would love to see the proposed fix being pushed to master since a bunch of underlying projects are affected as well. It does not make sense to me and for me to implement a bug fix while the underlying issue still persists in the dependent pynetbox library. Especially when the fix is quite easy to implement and the fork for said fix has already been performed.

MCM-Mike commented 5 months ago

@alensfor I created a diff-file between the original query.py and the patched file in #616

--- query.py  2024-06-04 07:32:12.893691318 +0000
+++ pr616.py  2024-06-04 07:35:30.227466867 +0000
@@ -192,7 +192,8 @@
             self.normalize_url(self.base),
             headers=headers,
         )
-        if req.ok:
+        # https://github.com/netbox-community/pynetbox/pull/616
+        if req.ok or req.status_code == 403:
             return req.headers.get("API-Version", "")
         else:
             raise RequestError(req)

When building the EE I had to install patch and then apply the patch under additional_build_steps. This works as expected.

---
version: 3
images:
  base_image:
    name: quay.io/centos/centos:stream9
dependencies:
  ansible_core:
    package_pip: ansible-core>=2.15
  ansible_runner:
    package_pip: ansible-runner
  galaxy: |
    ---
    roles: []
    collections: []
  system: |
    ...
    patch [platform:rpm]
  python: |
    ...
additional_build_steps:
  append_final:
    # Patch pynetbox
    - COPY pynetbox/pynetbox_patch.diff pynetbox_patch.diff
    - RUN patch /usr/local/lib/python3.9/site-packages/pynetbox/core/query.py < pynetbox_patch.diff

This worked for me, as this issue is still opened I'd wanted to share this quickly using:

jeffgdotorg commented 5 months ago

Hi folks, I'm stepping in and merging #616 as it's more targeted, besides fixing the problem in the lab and not breaking any tests. Thanks for bearing with us as we work to bring more resources to bear on pynetbox.