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

fix!: handle changed opnsense api since 22.7.x #9

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

jdreffein
Copy link

This MR addresses Issue #6 and correctly handles the changed return codes by the api of latest opnsense versions.

It is breaking for users of opnsense prior 22.7.x since there is no version handling by the check.

@nbuchwitz
Copy link
Owner

Thanks for the MR in this repo too! I will have a look later today, when I have access to my test environment. Maybe we can find a way to check the version and thus make it backward compatible (eg. check if certain attributes are part of the response and parse either this or that in the check)

@nbuchwitz
Copy link
Owner

I've slightly reworked your patch and rebased it against current master branch (some changes in #10). In my setup everything looks fine. Please test in your setup and if it is also good, I will merge it and publish the v0.2.0 release.

@jdreffein
Copy link
Author

i had to make changes to the needs reboot state, which is returned reboot flag in different keys by old and new api versions. in our environment with v 24.1.9 my last commit works as expected.

@nbuchwitz nbuchwitz force-pushed the api_change branch 2 times, most recently from 3479c0c to 9bf9e13 Compare July 1, 2024 23:20
@nbuchwitz
Copy link
Owner

I've updated the PR again, after I did some digging into opnsense's API code (https://github.com/opnsense/core/blob/master/src/opnsense/mvc/app/controllers/OPNsense/Core/Api/FirmwareController.php). Hopefully this should cover everything with recent versions. Happy to hear about your tests!

@jdreffein
Copy link
Author

Sorry for the long time off. Just managed to test the code in my fork. It is working with my current version of OPNsense (24.1.10_8)

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.

2 participants