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.

Sign up for GitHub

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

Jump to bottom

Add PADD to the image #1410

Merged
merged 4 commits into from
Aug 2, 2023
Merged

Add PADD to the image #1410

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
76b6d5b
Add PADD to the image
yubiuser Jul 31, 2023
d89d826
Add padd alias
yubiuser Aug 1, 2023
81e7671
Revert alias and make branch configurable
yubiuser Aug 2, 2023
f24dd34
Update src/start.sh
PromoFaux Aug 2, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ 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

# download a the main repos from github
RUN git clone --depth 1 --single-branch --branch ${WEB_BRANCH} https://github.com/pi-hole/AdminLTE.git /var/www/html/admin && \
git clone --depth 1 --single-branch --branch ${CORE_BRANCH} https://github.com/pi-hole/pi-hole.git /etc/.pihole ;\
Expand Down
4 changes: 4 additions & 0 deletions src/start.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor

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.

Copy link
Member

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

Copy link
Contributor

@edgd1er edgd1er Aug 2, 2023 •
edited
Loading

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.

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.

Copy link
Member

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!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've just noticed that the slash before the $ was removed in the reply when publishing the comment, there is may be a misunderstanding : here is a picture of the dockerfile of how it should look:
image

and the result in the image:
image

port="${FTLCONF_webserver_port%%,*}"
echo "alias padd='padd --port ${port:-8080} --secret ${FTLCONF_webserver_api_password}'" > /root/.bashrc

PromoFaux marked this conversation as resolved.
Show resolved Hide resolved
# Remove possible leftovers from previous pihole-FTL processes
rm -f /dev/shm/FTL-* 2> /dev/null
rm -f /run/pihole/FTL.sock
Expand Down