-
-
Notifications
You must be signed in to change notification settings - Fork 10
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
Add support for Unifi Network Server running remotely #65
Conversation
Task/fix auth url
Signed-off-by: aki263 <aakash@nappinggeek.com>
Signed-off-by: aki263 <aakash@nappinggeek.com>
# Conflicts: # internal/unifi/client.go
CC @onedr0p |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Incomplete review, please look into these changes for now.
Very helpful for people who want to get started using it instead of DM-ing me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Personally I'd like to see logging improvements in a separate PR.
internal/unifi/client.go
Outdated
log.Debug("Making request", zap.String("method", method), zap.String("path", path)) | ||
|
||
// Convert body to bytes for logging and reuse | ||
var bodyBytes []byte |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Little worried about reading the body here, I tried doing it last time and it messed up exdns
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not very comfortable with GO so if you think this can cause trouble, please remove it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not very comfortable with GO so if you think this can cause trouble, please remove it.
It's fine, we will test and verify it functions.
@aki263 Testing to ensure gateway functionality isn't impaired. Please test in your situation. |
I just tested, it works for me with env On unifi side |
Latest commit is working fine for me. Deleted all records and webhook recovered in less than a couple seconds. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
Added new env variable called
CONTROLLER_TYPE: NETWORK_SERVER
which will trigger the code path that will login and update dns record on remote unifi network server. This is applicable for gateway max and lite which dont have unifi server build in.This is tested locally, test results are in #63 (comment)
I have also added lot more logging so its easier to catch errors in http requests.
Image that can be tested:
ghcr.io/aki263/external-dns-unifi-webhook:main