-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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 PADD to the image #1410
Add PADD to the image #1410
Conversation
Signed-off-by: Christian König <ckoenig@posteo.de>
101ca37
to
23572ef
Compare
src/Dockerfile
Outdated
@@ -34,6 +34,10 @@ RUN apk add --no-cache \ | |||
ADD https://ftl.pi-hole.net/macvendor.db /macvendor.db | |||
COPY crontab.txt /crontab.txt | |||
|
|||
# Add PADD to the container, too. | |||
ADD --chmod=0755 https://raw.githubusercontent.com/pi-hole/PADD/PADD_FTLv6/padd.sh /usr/local/bin/padd | |||
RUN alias padd='port=${FTLCONF_webserver_port%%,*} padd --port ${port:-8080} --secret ${FTLCONF_webserver_api_password}' |
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.
The alias is not much use here, it will need to be in start.sh like the following (tested)
# Add an alias for padd to the root user's bashrc
port="${FTLCONF_webserver_port%%,*}"
echo "alias padd='padd --port ${port:-8080} --secret ${FTLCONF_webserver_api_password}'" > /root/.bashrc
Another reason for it going into start.sh
is that the FTLCONF_
variables will be defined by the user at runtime and we don't know them when we build the image
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.
Thanks, I FP'd it to the right place.
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'm sorry my suggestion was incomplete. I put $ to prevent variable substitution at build time.
the complete line to put in dockerfile is:
echo -e "alias padd='port=\${FTLCONF_webserver_port%%,*} padd --port \${port:-8080} --secret \${FTLCONF_webserver_api_password}'" >>/root/.bashrc
without the redirection to bashrc and placed in start.sh will do also, but bashrc will be rewritten at each container start, erasing possible user aliases and/or bash user defined settings.
Signed-off-by: Christian König <ckoenig@posteo.de>
src/start.sh
Outdated
@@ -87,6 +87,10 @@ if [ "${INSTALL_DEV_TOOLS:-0}" -gt 0 ] ; then | |||
apk add --no-cache nano less | |||
fi | |||
|
|||
# Add an alias for padd to the root user's bashrc |
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.
placed in start.sh will do also, but bashrc will be rewritten at each container start, erasing possible user aliases and/or bash user defined settings.
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.
It needs to be in start.sh
because the user-defined FTLCONF_
environment variables will not be defined at build-time, so it need to be applied at run time.
@yubiuser another thought, to ensure we're not munging any user-defined .bashrc
(though, one would argue there isn't much of a need to do so in a container such as this) Perhaps we cna do something like:
Dockerfile:
ADD --chmod=0755 https://raw.githubusercontent.com/pi-hole/PADD/PADD_FTLv6/padd.sh /usr/bin/padd
start.sh
port="${FTLCONF_webserver_port%%,*}"
echo "/usr/bin/padd --port ${port:-8080} --secret ${FTLCONF_webserver_api_password}'" > /usr/local/bin/padd
chmod +x /usr/local/bin/padd
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.
It needs to be in
start.sh
because the user-definedFTLCONF_
environment variables will not be defined at build-time, so it need to be applied at run time.
well, it doesn't have to be in start.sh, as bashrc is sourced when session starts, and using $ prevent bash to substitute variable at buildtime. that being said, defining a dynamic alias in start.sh will eventually add the same feature as an alias in bashrc.
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.
using $ prevent bash to substitute variable at buildtime.
Ah OK thanks, when I was tinkering about before it wasn't working - but that could have been for all manner of reasons, to be honest!
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.
With the addition of pi-hole/PADD#362, the user need only pass in Internally discussed, but as there is no guarantee that the password will be set via @yubiuser -can we have it just in the dockerfile please, no alias - and also if possible please make the padd branch a variable that can be passwed in at build time. (like the other repo branch arguments) |
Signed-off-by: Christian König <ckoenig@posteo.de>
Signed-off-by: Adam Warner <github@adamwarner.co.uk>
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
Add the v6 development version of PADD to the image.
PADD expects the API at port 80, currently FTL still defaults to 8080. Therefore, PADD needs to be started with
padd --port 8080