get prod dockerfile working properly by timothy-spencer · Pull Request #10790 · 18F/identity-idp

timothy-spencer · 2024-06-10T21:58:14Z

🛠 Summary of changes

Ensure that image is mostly read-only to the app user
make sure keys are readable by the app user
Make it so that build-essentials is not installed (smaller image, less tooling for attackers to work with)
Make sure puma knows what xff header to use
Ensure that service_providers.yml and related files are copied into the image

This image is running on https://idp.tstest.identitysandbox.gov/

mitchellhenke · 2024-06-25T17:17:17Z

dockerfiles/idp_prod.Dockerfile

 COPY .ruby-version $RAILS_ROOT/.ruby-version
 COPY Gemfile $RAILS_ROOT/Gemfile
 COPY Gemfile.lock $RAILS_ROOT/Gemfile.lock
+COPY package.json $RAILS_ROOT/package.json


We probably want to keep the ordering here for the same reasons discussed in https://gitlab.login.gov/lg/identity-dashboard/-/merge_requests/42

On this one, I'm going to stand firm, because this is not inline with development, but is aimed at production, and thus the image size reduction is important.

Yeah, I wholeheartedly agree that the slimmer image is worthwhile, but we should probably do a multi-stage build since it allows us to maximize the benefits of both caching and smaller builds. The size difference doesn't seem that significant at this point (~5-6%) for a potentially significant increase in build times.

Is it possible to try a multi-stage build?

I don't believe the build time is relevant here, though. It takes 8 minutes to build the prod image. 6 minutes to build the regular image. The overall pipeline with tests takes 16 minutes because of tests. The image builds are started at the start of the pipeline, and nothing depends on the prod image, so nothing has to wait for it to complete before it can run, and they run in parallel in a different runner pool, so don't even take up any test slots.

I will dig into the multi-stage builds to see if there's a better way to do it than when I last looked at this, but again, I think that this is not really needed.

How's that? I added a multi-stage build for you! :-)

dockerfiles/idp_prod.Dockerfile

mitchellhenke · 2024-06-26T14:19:23Z

dockerfiles/idp_prod.Dockerfile

+# yarn install
 COPY package.json $RAILS_ROOT/package.json
 COPY yarn.lock $RAILS_ROOT/yarn.lock
 RUN yarn install --production=true --frozen-lockfile --cache-folder .yarn-cache


Suggested change

RUN yarn install --production=true --frozen-lockfile --cache-folder .yarn-cache

Don't you want a yarn install? This command adds 408MB to the image, so it's doing something.

Probably mostly a bunch of node_modules which we could copy to avoid installing twice.

Though if we copy the compiled assets, we don't need to do that either.

mitchellhenke · 2024-06-26T14:21:11Z

dockerfiles/idp_prod.Dockerfile

 COPY config/service_providers.localdev.yml $RAILS_ROOT/config/service_providers.yml

 # Precompile assets
+RUN apt-get install -y make


can we move this up into the big install block?

We could also move asset compilation into the build stage as well and not install any yarn/javascript I think.

We can, though it won't make any significant difference.

I didn't move the asset compilation up, because I don't know where all the assets get put. It looked like there were a lot of assets directories in there. Is there a single one that I can copy back over?

I think it should just be the public/ folder

dockerfiles/idp_prod.Dockerfile

timothy-spencer · 2024-06-27T00:10:40Z

@mitchellhenke , I rearranged stuff a bit so that almost everything is built in the builder, and then everything in the app dir is copied over. The image is about 1gb smaller in size now. Fun!

The image is running over in https://idp.tstest.identitysandbox.gov/

Let me know what you think.

timothy-spencer · 2024-07-18T21:49:36Z

@mitchellhenke , any reason why we can't get this approved?

timothy-spencer · 2024-07-22T22:14:06Z

@mitchellhenke OK. I think it's ready for review again at last. :-)

dockerfiles/idp_prod.Dockerfile

… to actually work

remove extra commands because mitchell doesn't like them Co-authored-by: Mitchell Henke <mitchell.henke@gsa.gov>

…operly

… not the image we were building

timothy-spencer changed the title ~~WIP: get prod dockerfile working properly~~ get prod dockerfile working properly Jun 18, 2024

timothy-spencer requested review from mitchellhenke and zachmargolis June 20, 2024 15:47

mitchellhenke reviewed Jun 25, 2024

View reviewed changes

mitchellhenke reviewed Jun 26, 2024

View reviewed changes

timothy-spencer requested a review from mitchellhenke June 27, 2024 22:30

mitchellhenke reviewed Jul 23, 2024

View reviewed changes

dockerfiles/idp_prod.Dockerfile Outdated Show resolved Hide resolved

timothy-spencer requested a review from mitchellhenke July 23, 2024 20:56

mitchellhenke force-pushed the tspencer/update_prod_dockerfile branch from 3654d4c to 2acd1c2 Compare July 24, 2024 17:03

timothy-spencer and others added 18 commits August 1, 2024 10:42

make sure keys are readable

352a10c

remove build-essential in a way that will not bloat the image

9ac6f5d

yarn install needs build-essential too it seems

68859a5

add make for asset stuff

560d6aa

add some network debugging

8964ab5

tracepath is not apparently a thing

0924cb7

add more debug

1fc2d49

use proper netcat

92d27a4

remove network debugging tools

ce32d07

changelog: Internal, Platform Automation, Update production idp image…

70b4177

… to actually work

fix pwned_passwords.txt

fee5936

add puma header

ee4a8a4

trigger another build

92d37a1

move gem build and large files out into a multi-stage build

4dcb3ff

Update dockerfiles/idp_prod.Dockerfile

7c0159e

remove extra commands because mitchell doesn't like them Co-authored-by: Mitchell Henke <mitchell.henke@gsa.gov>

try removing some sqlite stuff

1e2d210

move everything up and just copy rails root over

5a18996

change where bundle install gets run and make sure it's configured pr…

f124d00

…operly

timothy-spencer and others added 25 commits August 1, 2024 10:42

remove suid/sgid from binaries

36f9268

add service_providers.yml and related files

32283fd

forgot to wire the env vars up to args

aecc5b5

trigger build

8a24005

forgot to copy keys we generated over

9041fe3

try using readonly deploy key

0aaf75f

get the host fingerprint, map the key in as an arg

13ec4b8

change perms on key so it works

2b71b59

just noticed that the service_provider stuff wasn't in the build dir

3f85789

move config copy to where workdir works

8fd25a6

some debugging

c87f906

try full path

18d83fa

I was totally missing that COPY was coming from the local filesystem,…

91dd66b

… not the image we were building

don't overwrite the identity-idp-config files with localdev ones

21192b1

exclude node_modules

09bc2d4

be more specific about node_modules

29c35f4

testing

52b110a

revert

21ded89

remove package restrictions in builder

14acf63

try some debugging of the seeder error

80f8cb1

forgot to assign error to a variable

b6fd69e

logger needs more log-ness to actually log I think

84656cf

get all exceptions

72588ab

did Agency.destroy_all and it doesn't have this issue

d56ed58

fix --no-install-recommends

363535f

mitchellhenke force-pushed the tspencer/update_prod_dockerfile branch from cf09d92 to 363535f Compare August 1, 2024 15:43

mitchellhenke approved these changes Aug 1, 2024

View reviewed changes

timothy-spencer merged commit 9365c40 into main Aug 1, 2024

timothy-spencer deleted the tspencer/update_prod_dockerfile branch August 1, 2024 17:34

matthinz mentioned this pull request Aug 6, 2024

Deploy RC 403 to Production #11038

Merged

Conversation

timothy-spencer commented Jun 10, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🛠 Summary of changes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mitchellhenke Jun 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

timothy-spencer commented Jun 27, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

timothy-spencer commented Jul 18, 2024

Uh oh!

timothy-spencer commented Jul 22, 2024

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

timothy-spencer commented Jun 10, 2024 •

edited

Loading

mitchellhenke Jun 26, 2024 •

edited

Loading

timothy-spencer commented Jun 27, 2024 •

edited

Loading