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.

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

Conversation

CC007
Copy link

@CC007 CC007 commented May 19, 2022

Description

This change adds the ability to provide a GIT_TOKEN as a build argument. This git token is then set as an environment variable.
Also the pip install code is moved to a builder stage.

Motivation and Context

The reason to make this GIT_TOKEN environment variable available, is so that it can be used when adding the following kind of dependency to requirements.txt:

git+https://${GITHUB_TOKEN}@github.com/user/project.git@{version}
  • I have raised an issue to propose this change (required)

Which issue(s) this PR fixes

Fixes openfaas/faas#1723

How Has This Been Tested?

It has been tested by creating a python module in a private github repo (using this video as an instruction). Then I created a python project with a requirements.txt file that uses this private git repo as a dependency.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Version change (see: Impact to existing users)

Impact to existing users

This doesn't have any direct impact on the end users, as the resulting image will contain the same files. The image layers are slightly different though, due to the pip install commands being moved to a builder stage. This has a very small impact on how the layers are cached, but since the the default requirements.txt in the template doesn't have any dependencies, this doesn't matter all that much.

Checklist:

  • My code follows the code style of this project. (I think? Is there such a thing as a Dockerfile code style?)
  • My change requires a change to the documentation. (Though explaining the ability to use the GIT_TOKEN would be nice)
  • I have updated the documentation accordingly.
  • I've read the CONTRIBUTION guide
  • I have signed-off my commits with git commit -s
  • I have added tests to cover my changes. (Not applicable I think?)
  • All new and existing tests passed.

@derek
Copy link

derek bot commented May 19, 2022

Thank you for your contribution. unfortunately, one or more of your commits are missing the required "Signed-off-by:" statement. Signing off is part of the Developer Certificate of Origin (DCO) which is used by this project.

Read the DCO and project contributing guide carefully, and amend your commits using the git CLI. Note that this does not require any cryptography, keys or special steps to be taken.

💡 Shall we fix this?

This will only take a few moments.

First, clone your fork and checkout this branch using the git CLI.

Next, set up your real name and email address:

git config --global user.name "Your Full Name"
git config --global user.email "[email protected]"

Finally, run one of these commands to add the "Signed-off-by" line to your commits.

If you only have one commit so far then run: git commit --amend --signoff and then git push --force.
If you have multiple commits, watch this video.

Check that the message has been added properly by running "git log".

@derek derek bot added the no-dco label May 19, 2022
@derek
Copy link

derek bot commented May 19, 2022

Thank you for your contribution. unfortunately, one or more of your commits are missing the required "Signed-off-by:" statement. Signing off is part of the Developer Certificate of Origin (DCO) which is used by this project.

Read the DCO and project contributing guide carefully, and amend your commits using the git CLI. Note that this does not require any cryptography, keys or special steps to be taken.

💡 Shall we fix this?

This will only take a few moments.

First, clone your fork and checkout this branch using the git CLI.

Next, set up your real name and email address:

git config --global user.name "Your Full Name"
git config --global user.email "[email protected]"

Finally, run one of these commands to add the "Signed-off-by" line to your commits.

If you only have one commit so far then run: git commit --amend --signoff and then git push --force.
If you have multiple commits, watch this video.

Check that the message has been added properly by running "git log".

These tokens can be used for python dependencies
 for private git repos.
 These changes are made in the main python template.

Signed-off-by: CC007 <[email protected]>
@CC007 CC007 force-pushed the feature/private-git-dependency-support branch from 032868c to 61bcda0 Compare May 20, 2022 12:04
@derek derek bot removed the no-dco label May 20, 2022
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

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

# 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

…ment variable for the whole builder stage

Signed-off-by: CC007 <[email protected]>
@CC007
Copy link
Author

CC007 commented May 30, 2022

Hmm, I found this article: https://pythonspeed.com/articles/docker-build-secrets/

Unfortunately build arguments are also embedded in the image [...] Technically you can work around this leak by using multi-stage builds, but that will result in slow builds, so I don’t recommend it.

So it would be safe with a multi-stage build like in this pull request, but it is recommended to use Buildkit's --secret feature instead...

I'm going to look into this, because this will also make sure that C/C++ dependencies like numpy and pandas work out of the box, because there is no need for a builder stage.

Since we are using an alpine-based image, Buildkit should be available, since it works for linux-based images.

@alexellis alexellis closed this Mar 14, 2023
@alexellis
Copy link
Member

Thanks for the suggestions.

We had a customer ask for this, and we implemented it in OpenFaaS Pro

Introducing our new Python template for production.

See also: Private npm modules

@alexellis
Copy link
Member

/lock: resolved

@derek derek bot locked and limited conversation to collaborators Mar 14, 2023
@openfaas openfaas deleted a comment from CC007 Mar 14, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for using git tokens in python dependencies
3 participants