Dockerignore: convert to whitelist by alxndrsn · Pull Request #738 · getodk/central

alxndrsn · 2024-10-05T08:54:12Z

Update to #361

conflicts resolved
convert .dockerignore from blacklist-based to whitelist
decrease docker build context size ~~significantly~~ (most savings would come from excluding .git dirs)
make docker builds less dependent on local state (e.g. local node_modules in submodules)
add script for checking docker build context
add CI tests to monitor if surprisingly large changes are made to the build context

To check build context locally, run:

./test/check-docker-context.sh --report

To check against previous .dockerignore:

git checkout next -- .dockerignore && ./test/check-docker-context.sh --report

* convert dockerignore from blacklist-based to whitelist * decrease docker build context size significantly * make docker builds less dependent on local state (e.g. local node_modules in submodules) * add script for checking docker build context * add CI tests to monitor if surprisingly large changes are made to the build context

alxndrsn · 2024-10-05T09:08:10Z

Unfortunately generating version.txt requires all .git directories to be included in the build context.

alxndrsn · 2024-10-05T09:30:39Z

Currently, .git in published central-service images is a file:

/usr/odk# cat .git
gitdir: ../.git/modules/server

lognaturel · 2024-10-06T04:22:21Z

Could we maybe delete it after the build step in the docker file?

alxndrsn · 2024-10-07T06:16:40Z

Could we maybe delete it after the build step in the docker file?

That could be done, but the file itself looks harmless.

.dockerignore

alxndrsn · 2025-02-13T07:43:57Z

Current stats

Note that listed context sizes are less in CI. This is demonstrated in #894.

`next`

[check-docker-context.sh] File count:  5863
[check-docker-context.sh] Total size: 85 MB

this branch

[check-docker-context.sh] File count:  4610
[check-docker-context.sh] Total size: 67 MB

without `.git` directories

[check-docker-context.sh] File count:  673
[check-docker-context.sh] Total size: 7 MB

matthew-white · 2025-02-24T23:41:45Z

@alxndrsn, it looks like you moved test/check-docker-context.sh into its own PR (#894). Does that sound right? I'm in favor of that idea. I don't totally follow the Docker things happening in that script, so it'd probably be better for someone else to review that PR.

What do you think about removing the script from this PR? I should be able to review the .dockerignore file at least.

alxndrsn · 2025-03-12T09:27:08Z

@alxndrsn, it looks like you moved test/check-docker-context.sh into its own PR (#894). Does that sound right?

👍 Currently integral to this PR is checking that the docker context size has not changed significantly. I'll revisit that idea when #894 is merged/rejected, and see how this PR can be simplified.

alxndrsn · 2026-02-09T11:03:16Z

PR updated to only change .dockerignore.

.dockerignore

Spun off from #738

.dockerignore

matthew-white

This is looking very close to me!

In the previous round of review, I was mostly thinking about files that were listed that needn't be listed. This time, I did a scan through the three Central repos for files that weren't listed here but maybe need to be.

matthew-white · 2026-02-18T05:52:11Z

.dockerignore

+# .git directories required for generating version.txt
+
+!/.git/


I feel like we should include .gitignore files as well. Those could conceivably affect what is logged to version.txt (including whether the Git working directory is clean).

Actually, now that I say that, if we're no longer copying all files that are committed to the repo, won't version.txt indicate that the working directory isn't clean?

Great question. I'll investigate. Given the importance of version.txt it would be good to have a test that it's created as expected.

I don't think version.txt is affected by changes to the git working tree.

PR at #1660

Which raises the question: should local changes be reflected in version.txt?

I don't think version.txt is affected by changes to the git working tree.

PR at #1660

I know I was the one to approve that PR, but I thought of it more as a general test of version.txt rather than a test of changes to the git working tree. Does test-images.sh make changes to the working tree before generating version.txt? Or is that what #1662 will do?

Which raises the question: should local changes be reflected in version.txt?

I'm not 100% sure that local changes are reflected in version.txt. I think they used to be, but I'm not sure what the current status is. It'd be kind of nice if they were, just because it gives us more information on the build. E.g., if a Sentry report comes in for a version marked -dirty, it could be that the error was due to a user changing the code. I'm not sure it's important enough to block this PR though.

.dockerignore

matthew-white · 2026-02-18T05:55:38Z

.dockerignore

+!/client/.browserslistrc
+!/client/.git/
+!/client/index.html
+!/client/package.json
+!/client/package-lock.json
+!/client/vite.config.js
+!/client/public/
+!/client/src/


This doesn't include /client/dist/, but is that because those files are built after the ignore step?

It's intentional - the client /dist directory should be created inside the build container rather than passed in via the docker context.

With the current configuration, anything in the host machine's client/dist directory can end up in the generated image.

This reverts commit 7212663.

matthew-white · 2026-02-22T07:24:16Z

test/test-images.sh

Maybe a merge or rebase is in order? I wouldn't expect to see this file in the diff.

You should! This PR significantly decreases the number of files in the docker context 🙂

Very exciting to see those numbers go down!

It looks like test-images.sh is no longer in the diff since the latest merge, so I think this comment can be considered resolved.

alxndrsn force-pushed the test-docker-context branch from bb9f990 to 5699ebe Compare October 5, 2024 08:54

alxndrsn requested review from matthew-white and removed request for matthew-white October 5, 2024 08:56

Include .git directories

5061301

Increase expected context size

73630f1

matthew-white reviewed Oct 8, 2024

View reviewed changes

.dockerignore Outdated Show resolved Hide resolved

matthew-white changed the base branch from master to next October 8, 2024 13:49

trim client whitelist

3b9ddda

alxndrsn requested a review from matthew-white October 15, 2024 08:05

Merge branch 'next' into test-docker-context

45a9d8c

alxndrsn added 2 commits February 13, 2025 07:45

fix for_humans() output

ee8a637

fix bugs in script

aec3a92

matthew-white mentioned this pull request Feb 24, 2025

ci/test-images: print docker context #894

Merged

2 tasks

Merge branch 'next' into test-docker-context

4454222

Merge branch 'next' into test-docker-context

9cfa725

alxndrsn mentioned this pull request Apr 1, 2025

ci: alert on significant change of docker context #948

Merged

2 tasks

alxndrsn and others added 2 commits February 9, 2026 13:32

Merge branch 'next' into test-docker-context

40221b7

ci: remove new workflow

cfb13b6

alxndrsn added 2 commits February 9, 2026 11:04

unignore client/index.html

9e59bcc

add additional important files

a6a16ac

alxndrsn marked this pull request as ready for review February 9, 2026 11:34

matthew-white reviewed Feb 11, 2026

View reviewed changes

.dockerignore Outdated Show resolved Hide resolved

.dockerignore Outdated Show resolved Hide resolved

.dockerignore Outdated Show resolved Hide resolved

.dockerignore Outdated Show resolved Hide resolved

.dockerignore Show resolved Hide resolved

Merge branch 'next' into test-docker-context

e0baab0

alxndrsn added a commit that referenced this pull request Feb 12, 2026

ci: alert on significant change of docker context (#948)

b9f09c4

Spun off from #738

alxndrsn and others added 4 commits February 12, 2026 18:29

Merge branch 'next' into test-docker-context

a92e62a

remove unnecessary files/folders per review

ffd459f

re-add docs

db6860a

reduce docker image size expectations

f55b0c2

alxndrsn mentioned this pull request Feb 17, 2026

dockerignore: ignore /test/ directory #1652

Merged

2 tasks

matthew-white reviewed Feb 18, 2026

View reviewed changes

.dockerignore Outdated Show resolved Hide resolved

matthew-white reviewed Feb 18, 2026

View reviewed changes

alxndrsn added 11 commits February 20, 2026 06:21

remove exception for /test/files

89ca470

Merge branch 'next' into test-docker-context

2445848

add test of generated version.txt file

ef89f2a

lint

4fa9f0e

cheeky indent

ea3d17a

increase expected file count

272c5b2

less indent

4ff3dd8

try to break docker contexT?

549d9ab

ci: remove bad ref

b15a101

remove .git dirs

7212663

browserslistrc

b8586e8

alxndrsn mentioned this pull request Feb 20, 2026

test-images: check contents of version.txt #1660

Merged

2 tasks

alxndrsn added 2 commits February 20, 2026 07:54

Revert "remove .git dirs"

c48dac9

This reverts commit 7212663.

lower expectations

bef70ae

matthew-white approved these changes Feb 22, 2026

View reviewed changes

Merge branch 'next' into test-docker-context

b8aaa65

		# .git directories required for generating version.txt

		!/.git/

Comments

Conversation

alxndrsn commented Oct 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alxndrsn commented Oct 5, 2024

Uh oh!

alxndrsn commented Oct 5, 2024

Uh oh!

lognaturel commented Oct 6, 2024

Uh oh!

alxndrsn commented Oct 7, 2024

Uh oh!

Uh oh!

alxndrsn commented Feb 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Current stats

next

this branch

without .git directories

Uh oh!

matthew-white commented Feb 24, 2025

Uh oh!

alxndrsn commented Mar 12, 2025

Uh oh!

alxndrsn commented Feb 9, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

matthew-white left a comment

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!

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!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

alxndrsn commented Oct 5, 2024 •

edited

Loading

alxndrsn commented Feb 13, 2025 •

edited

Loading

`next`

without `.git` directories