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

Rule Check: CopyIgnoredFiles #5135

Merged
merged 1 commit into from
Jul 10, 2024
Merged

Rule Check: CopyIgnoredFiles #5135

merged 1 commit into from
Jul 10, 2024

Conversation

daghack
Copy link
Collaborator

@daghack daghack commented Jul 8, 2024

Trying to COPY/ADD files excluded by dockerignore will cause the build to fail because they are missing from the context. This adds a rule check to catch this case.

@daghack daghack marked this pull request as draft July 8, 2024 23:24
@daghack daghack self-assigned this Jul 8, 2024
@daghack daghack force-pushed the copy-ignored-file-check branch 4 times, most recently from 735cbb4 to d099d2d Compare July 8, 2024 23:28
tonistiigi
tonistiigi reviewed Jul 8, 2024
View reviewed changes
frontend/dockerui/config.go Outdated Show resolved Hide resolved
frontend/dockerfile/builder/build.go Outdated Show resolved Hide resolved
frontend/dockerfile/linter/ruleset.go Outdated Show resolved Hide resolved
frontend/dockerfile/linter/ruleset.go Outdated
Description: "File is ignored by .dockerignore",
URL: "",
Format: func(file string) string {
return fmt.Sprintf("File %q is ignored by .dockerignore", file)
Copy link
Member

Choose a reason for hiding this comment

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

Ideally we could show the pattern that causes the issue but that might be tricky with current libs so can be left for follow-ups.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated with more detail, and added the needs/follow-up flag. :)

frontend/dockerfile/dockerfile2llb/convert.go Outdated Show resolved Hide resolved
frontend/dockerfile/dockerfile2llb/convert.go Outdated Show resolved Hide resolved
@thompson-shaun thompson-shaun added this to the v0.15.0 milestone Jul 9, 2024
@daghack daghack force-pushed the copy-ignored-file-check branch 3 times, most recently from 87cb599 to a8116c6 Compare July 9, 2024 05:45
@thompson-shaun thompson-shaun linked an issue Jul 9, 2024 that may be closed by this pull request
Add check for COPY/Add of files excluded by dockerignore (CopyingIgnoredFile) #5132
Closed
@daghack daghack force-pushed the copy-ignored-file-check branch 2 times, most recently from c5f2f48 to 92e5431 Compare July 10, 2024 01:23
tonistiigi
tonistiigi reviewed Jul 10, 2024
View reviewed changes
frontend/dockerui/context.go Outdated Show resolved Hide resolved
frontend/dockerui/config.go Outdated Show resolved Hide resolved
frontend/dockerfile/dockerfile2llb/convert.go Outdated Show resolved Hide resolved
frontend/dockerfile/dockerfile2llb/convert.go Outdated Show resolved Hide resolved
frontend/dockerfile/dockerfile2llb/convert.go Outdated Show resolved Hide resolved
frontend/dockerfile/dockerfile2llb/convert.go Outdated Show resolved Hide resolved
@daghack daghack force-pushed the copy-ignored-file-check branch 9 times, most recently from 1882905 to 1986a33 Compare July 10, 2024 16:17
@daghack daghack force-pushed the copy-ignored-file-check branch 4 times, most recently from 9fa8d35 to f52fe0b Compare July 10, 2024 19:42
@daghack daghack marked this pull request as ready for review July 10, 2024 19:56
@daghack daghack requested a review from tonistiigi July 10, 2024 19:57
@daghack daghack force-pushed the copy-ignored-file-check branch 3 times, most recently from 71ac772 to 2abe516 Compare July 10, 2024 20:03
@daghack daghack force-pushed the copy-ignored-file-check branch 3 times, most recently from 277848b to 0783fd2 Compare July 10, 2024 20:21
@tonistiigi
Copy link
Member 

2024-07-10T20:44:46.1289789Z --- FAIL: TestBaseImageConfig (0.90s)
2024-07-10T20:44:46.1308433Z panic: runtime error: invalid memory address or nil pointer dereference [recovered]
2024-07-10T20:44:46.1309621Z 	panic: runtime error: invalid memory address or nil pointer dereference
2024-07-10T20:44:46.1310465Z [signal 0xc0000005 code=0x0 addr=0xa8 pc=0x11fea7e]
2024-07-10T20:44:46.1311129Z 
2024-07-10T20:44:46.1311301Z goroutine 26 [running]:
2024-07-10T20:44:46.1311771Z testing.tRunner.func1.2({0x12e19c0, 0x1b2b6b0})
2024-07-10T20:44:46.1312952Z 	C:/hostedtoolcache/windows/go/1.22.5/x64/src/testing/testing.go:1631 +0x24a
2024-07-10T20:44:46.1313780Z testing.tRunner.func1()
2024-07-10T20:44:46.1314621Z 	C:/hostedtoolcache/windows/go/1.22.5/x64/src/testing/testing.go:1634 +0x377
2024-07-10T20:44:46.1315617Z panic({0x12e19c0?, 0x1b2b6b0?})
2024-07-10T20:44:46.1316442Z 	C:/hostedtoolcache/windows/go/1.22.5/x64/src/runtime/panic.go:770 +0x132
2024-07-10T20:44:46.1318122Z github.com/moby/buildkit/frontend/dockerfile/dockerfile2llb.toDispatchState({_, _}, {_, _, _}, {{0x0, {0x0, 0x0}, {0x0, 0x0}, ...}, ...})
2024-07-10T20:44:46.1319863Z 	D:/a/buildkit/buildkit/frontend/dockerfile/dockerfile2llb/convert.go:599 +0x2b1e
2024-07-10T20:44:46.1322649Z github.com/moby/buildkit/frontend/dockerfile/dockerfile2llb.Dockerfile2LLB({_, _}, {_, _, _}, {{0x0, {0x0, 0x0}, {0x0, 0x0}, ...}, ...})
2024-07-10T20:44:46.1325311Z 	D:/a/buildkit/buildkit/frontend/dockerfile/dockerfile2llb/convert.go:89 +0xa5
2024-07-10T20:44:46.1327573Z github.com/moby/buildkit/frontend/dockerfile/dockerfile2llb.TestBaseImageConfig(0xc000303380)
2024-07-10T20:44:46.1329611Z 	D:/a/buildkit/buildkit/frontend/dockerfile/dockerfile2llb/convert_test.go:227 +0x91
2024-07-10T20:44:46.1330638Z testing.tRunner(0xc000303380, 0x1455f70)

opt.Client needs a nil check as well.

@daghack @tonistiigi
Adds a rule check for copying files which match the .dockerignore pat…
07fe324 
…terns

Signed-off-by: Talon Bowler <talon.bowler@docker.com>
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
@tonistiigi tonistiigi merged commit e83d79a into moby:master Jul 10, 2024
76 checks passed
@daghack daghack deleted the copy-ignored-file-check branch July 11, 2024 14:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aaronlehmann aaronlehmann aaronlehmann left review comments

@tonistiigi tonistiigi tonistiigi approved these changes

Assignees

@daghack daghack

Labels
needs/follow-up
Projects
None yet
Milestone
v0.15.0
Development

Successfully merging this pull request may close these issues.

Add check for COPY/Add of files excluded by dockerignore (CopyingIgnoredFile)
4 participants
@daghack @tonistiigi @aaronlehmann @thompson-shaun