Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
[DO NOT MERGE] Use distroless to build the python container #29001
[DO NOT MERGE] Use distroless to build the python container #29001
Changes from all commits
9d7dbf8
6e355ae
ad1720b
f7b6009
ca64035
c2383b5
53bdcb4
95026a5
a8dbd7b
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
for my information:
apt
) ? Will users be able to install additional software into these images if they want?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.
/bin contains some system commands like
ls
. We do not need them in theory.no apt on distroless. If you are talking about the customer container, it will be better for users to use other images as base or use the way in this PR to copy the packages over.
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 think we have to announce this in breaking changes in CHANGES.MD and in make adjustments to custom container documentation in Beam / Dataflow docs. I do see that this could complicate the UX for some custom container users.
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.
Sounds good. I will update CHANGES.md after we all agree with using distroless.
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.
Not a blocker here, just a general comment: I'd rather pick specific binaries rather than copying every
/bin
binary. I understand it probably means more breakages, but it's also a smaller vulnerability surface.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 think lack of
apt
is the main inconvenience for me. I think this is a real friction point, while the vulnerabilities reported by docker image checkers more often than not, are not applicable to execution of Beam pipelines.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 do not think we need
/bin
. I added it here only for the convenience. @tvalentyn I doubt users care aboutapt
since if they need to touch this file, they usually build their own images. The purpose for us to adopt distorless is only to reduce the vulnerabilities when users use our containers as the default.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.
Before custom containers we also mentioned running custom commands in:
https://beam.apache.org/documentation/sdks/python-pipeline-dependencies/#nonpython
Not sure how much usage this has though.