Confconsole Bookworm Let's Encrypt DNS-01 challenge fixes #82
Conversation
|
Hey @OnGle can you please do a code review here ASAP? FWIW, there is a lot of discussion on the related issue: turnkeylinux/tracker#1876 You probably won't need it, but figured I might as well mention it. |
…y, but as Stefan notes, it shouldn't\!)
|
|
||
| def get_conf_value(conf: list[str], key: str) -> str: | ||
| """Given a list of config lines and a key, returns the (first) corresponding | ||
| value (non case sensititive). If nothing found, returns empty string. |
There was a problem hiding this comment.
maybe this should actually return Optional[str]?
There was a problem hiding this comment.
Yep, good call. Actually, it's not even being used, so I probably should just remove it?! I don't actually recall my intentions with that, so I'm just going to leave it there for now though...
| @@ -158,21 +162,24 @@ restart_servers() { | |||
| for servicename in $@; do | |||
| info "(Re)starting $servicename" | |||
| systemctl restart $servicename | tee -a $LOG | |||
| [ "${PIPESTATUS[0]}" -eq 0 ] || EXIT_CODE=1 | |||
| [[ "${PIPESTATUS[0]}" -eq 0 ]] || EXIT_CODE=1 | |||
| done | |||
| } | |||
|
|
|||
| clean_finish() { | |||
| # warning: do NOT use 'fatal' in this func as it will cause an inescapable recursive loop | |||
There was a problem hiding this comment.
if you add EXIT to the trap signals in L216 then fatal can just exit 1 and can be used here. the EXIT trap is executed exactly once
| [ $(which authbind) ] || fatal "Authbind not installed" | ||
| [[ "$EUID" = "0" ]] || fatal "$APP must be run as root" | ||
| [[ $(which dehydrated) ]] || fatal "Dehydrated not installed, or not in the \$PATH" | ||
| [[ $(which authbind) ]] || fatal "Authbind not installed" | ||
|
|
||
| for sig in INT TERM; do |
|
i also recommend using |
|
Thanks @a3s7p - great catches and good call to check via I've applied all of your suggested fixes, with exception of your suggestions re I'm going to merge this now. But I will circle back and add your suggestion to the tracker for future dev. |
Refactoring/bugfixing the Confconsole Let's Encrypt plugin...
Closes turnkeylinux/tracker#1876 (or at least will when I'm finished)
This work was undertaken to resolve the above noted bug for DNS-01 challenges. However I got a bit carried away and refactored and tidied the code a bit more than I originally intended...
This is currently untested, so will likely get another commit or 2.