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

Auth fails getting API version with Netbox 4.0.0 #612

Closed
terraflubb opened this issue May 7, 2024 · 25 comments · Fixed by #616
Closed

Auth fails getting API version with Netbox 4.0.0 #612

terraflubb opened this issue May 7, 2024 · 25 comments · Fixed by #616
Assignees
Labels
type: bug A confirmed report of unexpected behavior in the application

Comments

@terraflubb
Copy link

terraflubb commented May 7, 2024

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! 🎉

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

@terraflubb terraflubb added the type: bug A confirmed report of unexpected behavior in the application label May 7, 2024
@Fredouye
Copy link

Fredouye commented May 9, 2024

Hi

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

@kotso
Copy link

kotso commented May 15, 2024

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

@mto2
Copy link

mto2 commented May 17, 2024

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

@tyldum
Copy link

tyldum commented May 23, 2024

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

netbox-community/netbox#16122

@markkuleinio
Copy link
Contributor

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

@btriller
Copy link
Contributor

btriller commented May 27, 2024

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
Copy link

xibriz commented May 29, 2024

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

if req.ok:
return req.json()
else:
raise RequestError(req)

EDIT: as @btriller already stated 😆

@terraflubb
Copy link
Author

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
Copy link

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 #613 but in my case that didn't help.
Thank you

@xibriz
Copy link

xibriz commented May 30, 2024

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

@xibriz
Copy link

xibriz commented May 30, 2024

@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
Copy link
Author

terraflubb commented Jun 3, 2024

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

@alensfor
Copy link

alensfor commented Jun 3, 2024

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
Copy link

xibriz commented Jun 3, 2024

@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
Copy link

alensfor commented Jun 10, 2024

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
Copy link

xibriz commented Jun 11, 2024

@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
Copy link

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
Copy link

bluikko commented Jun 26, 2024

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
Copy link

@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
Copy link

bluikko commented Jun 26, 2024

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
Copy link

@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
Copy link

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
Copy link

TheNetworkGuy commented Jul 1, 2024

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
Copy link

MCM-Mike commented Jul 2, 2024

@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:

  • netbox (v4.0.3)

@jeffgdotorg
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug A confirmed report of unexpected behavior in the application
Projects
None yet