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

Docker improvement #21

Merged
merged 3 commits into from Jan 21, 2019
Merged

Conversation

yaegashi
Copy link
Contributor

@yaegashi yaegashi commented Jan 19, 2019

Description

Update Makefile and Dockerfile to build a static binary and get a much smaller footprint with alpine image. It also includes CA certificates needed for practical use.

Add Dockerfile.dev in .gitignore and .dockerignore. It's to utilize cached build steps on Dockerfile development.

Optimize ordering of build steps in Dockerfile to avoid needless downloads.

Motivation and Context

This PR improves Docker building facilities to achieve a faster development cycle and a smaller footprint image for practical use.

How Has This Been Tested?

You can get build results very fast by utilizing cached build steps on development using Dockerfile.dev.

$ cp Dockerfile Dockerfile.dev
$ vi Dockerfile.dev
$ docker build -f Dockerfile.dev .

You can get 17.1MB in size by building this image.

$ docker images oauth2_proxy:latest 
REPOSITORY          TAG                 IMAGE ID            CREATED             SIZE
oauth2_proxy        latest              8d7d2c24c5bd        14 seconds ago      17.1MB

Checklist:

  • My change requires a change to the documentation or CHANGELOG.
  • I have updated the documentation/CHANGELOG accordingly.
  • I have created a feature (non-master) branch for my PR.

@yaegashi yaegashi requested a review from a team January 19, 2019 21:53
Dockerfile Outdated Show resolved Hide resolved
Copy link
Member

@JoelSpeed JoelSpeed left a comment

Choose a reason for hiding this comment

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

I can't really see any reason not to just make the default build target statically linked? Is there that much benefit to having both on a small project like this?

Dockerfile Outdated Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
@yaegashi yaegashi force-pushed the docker-improvement branch 2 times, most recently from 16606ec to ebf7b2c Compare January 21, 2019 17:16
@JoelSpeed
Copy link
Member

@yaegashi I've just checked over this and am happy with the content now, would you please squash the changes into logical commits and we'll get this merged

@yaegashi yaegashi force-pushed the docker-improvement branch 3 times, most recently from 156d4dd to 01461f8 Compare January 21, 2019 17:53
Update Makefile to build a static binary by default.
Update Dockefile to get a much smaller footprint with alpine image.

Optimize ordering of build steps to avoid needless downloads.

Include CA certificates needed for practical use.
Dockerfile.dev is ignored by both git and docker for faster development
cycle of docker build.
@yaegashi
Copy link
Contributor Author

@JoelSpeed Squashed the changes into 3 commits. Feel free to ammend the commit logs or other wording issues before merging them.

Copy link
Member

@JoelSpeed JoelSpeed left a comment

Choose a reason for hiding this comment

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

Looks great, thanks very much!

@JoelSpeed JoelSpeed merged commit c922a09 into oauth2-proxy:master Jan 21, 2019
michael-freidgeim-webjet added a commit to MNF/oauth2-proxy that referenced this pull request Sep 4, 2021
Added audit log to indicate login username
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants