-
-
Notifications
You must be signed in to change notification settings - Fork 5k
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
deploy hook for Ruckus ZoneDirector / Unleashed #4832
base: dev
Are you sure you want to change the base?
Conversation
Just stumbled across this and verified it works for deploying certificate updates from OPNsense to my Ruckus Unleashed controller. Steps:
So, I vote for including this deployment hook. What else is needed to make it happen? It needs to be upstream (here) before OPNsense will add an option to use it, and it'd be really fantastic to have it "just work". ✨ |
deploy/ruckus.sh
Outdated
} | ||
trap cleanup EXIT | ||
|
||
LOGIN_URL=$(curl https://$RUCKUS_HOST -ksSLo /dev/null -w '%{url_effective}') |
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.
do not use curl
, please use _post()
or _get()
function instead.
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.
My reimplemention using _get() and post() has been pushed to this PR now
@uberjay I don't know if the original author is still working on this... Maybe you would be interested to finish the job? I'd suggest to apply the proposed fixes and submit a new PR. |
I'm the author of the code this is adapted from. If anyone's already working on a fix then let me know. Otherwise I'm happy to re-implement this correctly: I'm on holiday for a couple more months, so I have the time. (I had assumed this PR wouldn't be accepted as-is, since it's not idiomatic with the other acme code). |
@ms264556 I was planning on looking into it now, but if you're able to, please do go ahead. |
@ms264556 That'd be awesome! I don't know if the original PR author is following along here, and I don't personally have the bandwidth. It'd be very much appreciated! |
Sure. I'll setup some test APs and have a look this afternoon. I do remember that the acme code for existing integrations was very low level compared to my lazy 'get curl do all the thinking' code, so I don't think this is a 5 minute job. |
@kchiem my changes are in a PR to your dev branch |
Rewrite deploy/ruckus.sh to use _get() and _post()
deploy/ruckus.sh
Outdated
|
||
_debug RUCKUS_HOST "$RUCKUS_HOST" | ||
_debug RUCKUS_USER "$RUCKUS_USER" | ||
_debug RUCKUS_PASS "$RUCKUS_PASS" |
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.
use _secure_debug
for password, so that you won't leak 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.
Fixed
deploy/ruckus.sh
Outdated
_debug RUCKUS_USER "$RUCKUS_USER" | ||
_debug RUCKUS_PASS "$RUCKUS_PASS" | ||
|
||
export HTTPS_INSECURE=1 |
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.
why do we need "insecure" here?
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.
Ruckus devices ship with 1024 bit self-signed server certificates, so curl will fail with error 60 unless I set HTTPS_INSECURE=1.
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.
Overriding HTTPS_INSECURE
to always be enabled seems wrong to me -- for initial deployment one can add --insecure
: acme.sh --deploy -d your.domain.here --deploy-hook ruckus --insecure
. On subsequent cert deploy runs, HTTPS_INSECURE=1
should no longer be necessary.
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.
No problem. That's fixed now.
deploy/ruckus.sh
Outdated
_replace_cert_ajax='<ajax-request action="docmd" comp="system" updater="rid.0.5" xcmd="replace-cert" checkAbility="6" timeout="-1"><xcmd cmd="replace-cert" cn="'$RUCKUS_HOST'"/></ajax-request>' | ||
_post "$_replace_cert_ajax" "$_base_url/_cmdstat.jsp" >/dev/null | ||
|
||
info "Rebooting" |
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.
use _info
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.
Fixed, and I also fixed the unquoted _info strings
fix insecure password debug and _info typo
Oops. Would help if I quoted all of my _info() args. One more commit incoming. And this is fixed now. |
fix acme.sh PR shfmt failure
please add the usage here: |
That's done |
I noticed in the dev docs that we're not supposed to use awk. Our awk usage isn't doing anything useful, so I've given @kchiem a PR to remove this, which should be pushed here soon |
Remove awk usage & refuse host redirects
Tested on my Unleashed install, but according to the code it's adapted form, it should work for ZD installs too.