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

Add support for git tokens, to be used for python dependencies from private git repos, in the main python template. #292

Closed
wants to merge 2 commits into from
Closed

Add support for git tokens, to be used for python dependencies from private git repos, in the main python template. #292

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
61bcda0
Add support for git tokens
CC007 May 18, 2022
65b6c21
Use inline variables for GITHUB_TOKEN, instead of setting the environ…
CC007 May 30, 2022
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
76 changes: 54 additions & 22 deletions template/python3/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,56 +1,88 @@
FROM --platform=${TARGETPLATFORM:-linux/amd64} ghcr.io/openfaas/classic-watchdog:0.2.0 as watchdog
FROM --platform=${TARGETPLATFORM:-linux/amd64} python:3-alpine

ARG TARGETPLATFORM
ARG BUILDPLATFORM
# Builder stage that allows you to use git modules from private repos
FROM --platform=${TARGETPLATFORM:-linux/amd64} python:3-alpine as builder

# Allows you to add additional packages via build-arg
# Basic user, python and certificate setup
ARG ADDITIONAL_PACKAGE

COPY --from=watchdog /fwatchdog /usr/bin/fwatchdog
RUN chmod +x /usr/bin/fwatchdog
RUN apk --no-cache add ca-certificates ${ADDITIONAL_PACKAGE}
RUN addgroup -S app && adduser app -S -G app
WORKDIR /home/app/
RUN chown -R app /home/app && \
mkdir -p /home/app/python && chown -R app /home/app
USER app
ENV PATH=$PATH:/home/app/.local/bin:/home/app/python/bin/
ENV PYTHONPATH=$PYTHONPATH:/home/app/python

# Token to be provided as argument
ARG GIT_TOKEN=no_token_set

# Add non root user
RUN addgroup -S app && adduser app -S -G app
# Install git and make the git token available as environment variable
USER root
RUN apk --no-cache add git
ENV GIT_TOKEN=${GIT_TOKEN}
Copy link
Member

Choose a reason for hiding this comment

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

It may be better to avoid leaking the ENV into the build, and instead prefixing the value in the pip install command.

Copy link
Member

Choose a reason for hiding this comment

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

I was going to say the same thing, like this

RUN GIT_TOKEN=${GIT_TOKEN} pip install -r requirements.txt --target=/home/app/python

Copy link
Author

Choose a reason for hiding this comment

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

I didn't think that the env var would be leaked if used in the builder stage, but I do agree that it's neater the way that @LucasRoesler proposes. That way it won't unnecessarily be available to all the other commands.

Copy link
Author

Choose a reason for hiding this comment

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

Changed


# Install template requirements
USER app
WORKDIR /home/app/

COPY index.py .
COPY requirements.txt .
RUN pip install -r requirements.txt --target=/home/app/python

# Install function specific requirements
RUN mkdir -p function
WORKDIR /home/app/function/
COPY function/requirements.txt .
RUN pip install -r requirements.txt --target=/home/app/python

FROM --platform=${TARGETPLATFORM:-linux/amd64} ghcr.io/openfaas/classic-watchdog:0.2.0 as watchdog

# Actual image
FROM --platform=${TARGETPLATFORM:-linux/amd64} python:3-alpine

# Basic user, python and certificate setup
ARG ADDITIONAL_PACKAGE
RUN apk --no-cache add ca-certificates ${ADDITIONAL_PACKAGE}
RUN addgroup -S app && adduser app -S -G app
WORKDIR /home/app/
RUN chown -R app /home/app && \
mkdir -p /home/app/python && chown -R app /home/app
mkdir -p /home/app/python && chown -R app /home/app
USER app
ENV PATH=$PATH:/home/app/.local/bin:/home/app/python/bin/
ENV PYTHONPATH=$PYTHONPATH:/home/app/python

RUN pip install -r requirements.txt --target=/home/app/python
# Copy over watchdog
USER root
COPY --from=watchdog /fwatchdog /usr/bin/fwatchdog
RUN chmod +x /usr/bin/fwatchdog

# Copy over template files
USER app
WORKDIR /home/app/
COPY index.py .
COPY requirements.txt .

# Mark the function dir as a module
RUN mkdir -p function
RUN touch ./function/__init__.py

# Copy over the function specific requirements file
WORKDIR /home/app/function/
COPY function/requirements.txt .

RUN pip install -r requirements.txt --target=/home/app/python

# Copy over resolved dependencies from builder stage
WORKDIR /home/app/
COPY --from=builder /home/app/.cache /home/app/.cache
Copy link
Member

Choose a reason for hiding this comment

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

My concern would be whether any C/C++ libraries that were built or installed into the system are still available at this point such as numpy, pillow or pandas.

Copy link
Author

Choose a reason for hiding this comment

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

Good point. I'll see if I can test this. I believe requests also requires C/C++ libs.

Copy link
Author

Choose a reason for hiding this comment

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

Confirmed that numpy only installs to /home/app/.cache and /home/app/python. I'll try pillow and pandas next.

Copy link
Author

@CC007 CC007 May 30, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Pillow doesn't correctly install, due to a missing zlib dependency (and even when that is added, it is installed to /lib instead of /usr/lib, so python still can't find it)

See: link

Copy link
Author

Choose a reason for hiding this comment

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

Pandas seems to work fine too, only using those 2 folders

COPY --from=builder /home/app/python /home/app/python

# Copy over the specific function code
USER root

COPY function function

# Allow any user-id for OpenShift users.
RUN chown -R app:app ./ && \
chmod -R 777 /home/app/python
chmod -R 777 /home/app/python

# Prepare and run the watchdog
USER app

ENV fprocess="python3 index.py"
EXPOSE 8080

HEALTHCHECK --interval=3s CMD [ -e /tmp/.lock ] || exit 1

CMD ["fwatchdog"]