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

use distroless/static as base image to further reduce image size #748

Merged
merged 1 commit into from
May 14, 2019
Merged

use distroless/static as base image to further reduce image size #748

merged 1 commit into from
May 14, 2019

Conversation

tariq1890
Copy link
Contributor

@tariq1890 tariq1890 commented May 10, 2019 •
edited
Loading

Motivation: Given the Alpine CVE https://www.alpinelinux.org/posts/Docker-image-vulnerability-CVE-2019-5021.html (touted as the security-focused distro ironically), I would like to go one step further in preventive action by using scratch distroless/static as the base image. This will reduce the attack vectors and the overall image size.

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels May 10, 2019
@tariq1890
Copy link
Contributor Author

/assign @andyxning

@mxinden
Copy link
Contributor

mxinden commented May 10, 2019

In regards to the CVE, given that the latest image is build after March 7th 2019, this should not apply to the latest build. Please correct me if I am wrong.

In regards to moving to scratch as a base image, would it make sense to align this effort with the current enhancement proposal updating the base image of the Kubernetes core components? Maybe @tallclair has some advice for us.

@tariq1890
Copy link
Contributor Author

Yes you are right @mxinden. We are safe. This PR is just an extra cautionary measure.

The distroless image actually ended up being up bigger in size than with alpine as base.

@brancz
Copy link
Member

brancz commented May 10, 2019

I agree with @mxinden, even if it does end up being larger, I think we should use the distroless base.

@tariq1890
Copy link
Contributor Author

Alrighty, it is a trivial change. I will update this PR then?

@tariq1890 tariq1890 changed the title use scratch as base image to further reduce image size use distroless/static as base image to further reduce image size May 10, 2019
@tariq1890
Copy link
Contributor Author

I have made the changes.

Correction: distroless/static is actually smaller than alpine. So win-win :).

brancz
brancz reviewed May 10, 2019
View reviewed changes
Copy link
Member

@brancz brancz left a comment •
edited
Loading

Choose a reason for hiding this comment

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

just one question, otherwise lgtm

Dockerfile Show resolved Hide resolved
@tariq1890
Copy link
Contributor Author

tariq1890 commented May 10, 2019 •
edited
Loading

/hold

So turns out it isn't as trivial as I thought it would be. This PR will need more work. Thanks for all your inputs @brancz @mxinden :)

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 10, 2019
@andyxning
Copy link
Member

@brancz @tariq1890 @mxinden I think we need to be consensus with the Kubernetes community in choosing base image.

@tariq1890
Copy link
Contributor Author

@andyxning As per this doc, distroless/static is k8s-endorsed.

@tariq1890 tariq1890 force-pushed the use_scratch branch 3 times, most recently from 76a285d to ca1de5c Compare May 14, 2019 06:31
@tariq1890
Copy link
Contributor Author

/hold cancel

This PR is ready for review again. I am leaving the makefiles untouched since we are using it for mult-arch builds. Hopefully, these changes should be agreeable :).

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 14, 2019
@k8s-ci-robot k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels May 14, 2019
@brancz
Copy link
Member

brancz commented May 14, 2019

Thanks for taking care of this!

/lgtm

@k8s-ci-robot k8s-ci-robot added lgtm "Looks good to me", indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels May 14, 2019
@brancz
Copy link
Member

brancz commented May 14, 2019

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: brancz, tariq1890

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot merged commit 433096d into kubernetes:master May 14, 2019
@tariq1890 tariq1890 deleted the use_scratch branch May 14, 2019 07:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@brancz brancz brancz left review comments

@reetasingh reetasingh reetasingh left review comments

@zouyee zouyee Awaiting requested review from zouyee

Assignees

@andyxning andyxning

@brancz brancz

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@tariq1890 @mxinden @brancz @andyxning @k8s-ci-robot @reetasingh