diff --git a/frontend/dockerfile/dockerfile2llb/convert.go b/frontend/dockerfile/dockerfile2llb/convert.go index faf054c8952ed..e1aa8e6aeec56 100644 --- a/frontend/dockerfile/dockerfile2llb/convert.go +++ b/frontend/dockerfile/dockerfile2llb/convert.go @@ -40,6 +40,7 @@ import ( "github.com/moby/buildkit/util/suggest" "github.com/moby/buildkit/util/system" dockerspec "github.com/moby/docker-image-spec/specs-go/v1" + "github.com/moby/patternmatcher" "github.com/moby/sys/signal" digest "github.com/opencontainers/go-digest" ocispecs "github.com/opencontainers/image-spec/specs-go/v1" @@ -594,6 +595,18 @@ func toDispatchState(ctx context.Context, dt []byte, opt ConvertOpt) (*dispatchS buildContext := &mutableOutput{} ctxPaths := map[string]struct{}{} + var dockerIgnoreMatcher *patternmatcher.PatternMatcher + dockerIgnorePatterns, err := opt.Client.DockerIgnorePatterns(ctx) + if err != nil { + return nil, err + } + if len(dockerIgnorePatterns) > 0 { + dockerIgnoreMatcher, err = patternmatcher.New(dockerIgnorePatterns) + if err != nil { + return nil, err + } + } + for _, d := range allDispatchStates.states { if !opt.AllStages { if _, ok := allReachable[d]; !ok || d.noinit { @@ -634,24 +647,28 @@ func toDispatchState(ctx context.Context, dt []byte, opt ConvertOpt) (*dispatchS return nil, parser.WithLocation(err, d.stage.Location) } } + d.state = d.state.Network(opt.NetworkMode) + opt := dispatchOpt{ - allDispatchStates: allDispatchStates, - metaArgs: optMetaArgs, - buildArgValues: opt.BuildArgs, - shlex: shlex, - buildContext: llb.NewState(buildContext), - proxyEnv: proxyEnv, - cacheIDNamespace: opt.CacheIDNamespace, - buildPlatforms: platformOpt.buildPlatforms, - targetPlatform: platformOpt.targetPlatform, - extraHosts: opt.ExtraHosts, - shmSize: opt.ShmSize, - ulimit: opt.Ulimits, - cgroupParent: opt.CgroupParent, - llbCaps: opt.LLBCaps, - sourceMap: opt.SourceMap, - lint: lint, + allDispatchStates: allDispatchStates, + metaArgs: optMetaArgs, + buildArgValues: opt.BuildArgs, + shlex: shlex, + buildContext: llb.NewState(buildContext), + proxyEnv: proxyEnv, + cacheIDNamespace: opt.CacheIDNamespace, + buildPlatforms: platformOpt.buildPlatforms, + targetPlatform: platformOpt.targetPlatform, + extraHosts: opt.ExtraHosts, + shmSize: opt.ShmSize, + ulimit: opt.Ulimits, + cgroupParent: opt.CgroupParent, + llbCaps: opt.LLBCaps, + sourceMap: opt.SourceMap, + lint: lint, + dockerIgnoreMatcher: dockerIgnoreMatcher, + copyIgnoredCheckEnabled: opt.Client.CopyIgnoredCheckEnabled, } if err = dispatchOnBuildTriggers(d, d.image.Config.OnBuild, opt); err != nil { @@ -810,22 +827,24 @@ func toCommand(ic instructions.Command, allDispatchStates *dispatchStates) (comm } type dispatchOpt struct { - allDispatchStates *dispatchStates - metaArgs []instructions.KeyValuePairOptional - buildArgValues map[string]string - shlex *shell.Lex - buildContext llb.State - proxyEnv *llb.ProxyEnv - cacheIDNamespace string - targetPlatform ocispecs.Platform - buildPlatforms []ocispecs.Platform - extraHosts []llb.HostIP - shmSize int64 - ulimit []pb.Ulimit - cgroupParent string - llbCaps *apicaps.CapSet - sourceMap *llb.SourceMap - lint *linter.Linter + allDispatchStates *dispatchStates + metaArgs []instructions.KeyValuePairOptional + buildArgValues map[string]string + shlex *shell.Lex + buildContext llb.State + proxyEnv *llb.ProxyEnv + cacheIDNamespace string + targetPlatform ocispecs.Platform + buildPlatforms []ocispecs.Platform + extraHosts []llb.HostIP + shmSize int64 + ulimit []pb.Ulimit + cgroupParent string + llbCaps *apicaps.CapSet + sourceMap *llb.SourceMap + lint *linter.Linter + dockerIgnoreMatcher *patternmatcher.PatternMatcher + copyIgnoredCheckEnabled bool } func getEnv(state llb.State) shell.EnvGetter { @@ -912,6 +931,7 @@ func dispatch(d *dispatchState, cmd command, opt dispatchOpt) error { keepGitDir: c.KeepGitDir, checksum: checksum, location: c.Location(), + ignoreMatcher: opt.dockerIgnoreMatcher, opt: opt, }) } @@ -946,12 +966,15 @@ func dispatch(d *dispatchState, cmd command, opt dispatchOpt) error { err = dispatchArg(d, c, &opt) case *instructions.CopyCommand: l := opt.buildContext + var ignoreMatcher *patternmatcher.PatternMatcher if len(cmd.sources) != 0 { src := cmd.sources[0] if !src.noinit { return errors.Errorf("cannot copy from stage %q, it needs to be defined before current stage %q", c.From, d.stageName) } l = src.state + } else { + ignoreMatcher = opt.dockerIgnoreMatcher } err = dispatchCopy(d, copyConfig{ params: c.SourcesAndDest, @@ -964,6 +987,7 @@ func dispatch(d *dispatchState, cmd command, opt dispatchOpt) error { link: c.Link, parents: c.Parents, location: c.Location(), + ignoreMatcher: ignoreMatcher, opt: opt, }) if err == nil { @@ -1442,6 +1466,7 @@ func dispatchCopy(d *dispatchState, cfg copyConfig) error { a = a.Copy(st, f, dest, opts...) } } else { + validateCopySourcePath(src, &cfg) var patterns []string if cfg.parents { // detect optional pivot point @@ -1558,6 +1583,7 @@ type copyConfig struct { checksum digest.Digest parents bool location []parser.Range + ignoreMatcher *patternmatcher.PatternMatcher opt dispatchOpt } @@ -1871,6 +1897,27 @@ func addReachableStages(s *dispatchState, stages map[*dispatchState]struct{}) { } } +func validateCopySourcePath(src string, cfg *copyConfig) error { + if cfg.ignoreMatcher == nil || !cfg.opt.copyIgnoredCheckEnabled { + return nil + } + cmd := "Copy" + if cfg.isAddCommand { + cmd = "Add" + } + + ok, err := cfg.ignoreMatcher.MatchesOrParentMatches(src) + if err != nil { + return err + } + if ok { + msg := linter.RuleCopyIgnoredFile.Format(cmd, src) + cfg.opt.lint.Run(&linter.RuleCopyIgnoredFile, cfg.location, msg) + } + + return nil +} + func validateCircularDependency(states []*dispatchState) error { var visit func(*dispatchState, []instructions.Command) []instructions.Command if states == nil { diff --git a/frontend/dockerfile/dockerfile_lint_test.go b/frontend/dockerfile/dockerfile_lint_test.go index 04d66fa8dfd6f..92660bb88a178 100644 --- a/frontend/dockerfile/dockerfile_lint_test.go +++ b/frontend/dockerfile/dockerfile_lint_test.go @@ -6,6 +6,7 @@ import ( "fmt" "maps" "os" + "regexp" "sort" "testing" "time" @@ -44,8 +45,68 @@ var lintTests = integration.TestFuncs( testSecretsUsedInArgOrEnv, testInvalidDefaultArgInFrom, testFromPlatformFlagConstDisallowed, + testCopyIgnoredFiles, ) +func testCopyIgnoredFiles(t *testing.T, sb integration.Sandbox) { + dockerignore := []byte(` +Dockerfile +`) + dockerfile := []byte(` +FROM scratch +COPY Dockerfile . +ADD Dockerfile /windy +`) + checkLinterWarnings(t, sb, &lintTestParams{ + Dockerfile: dockerfile, + DockerIgnore: dockerignore, + }) + + checkLinterWarnings(t, sb, &lintTestParams{ + Dockerfile: dockerfile, + DockerIgnore: dockerignore, + FrontendAttrs: map[string]string{ + "build-arg:BUILDKIT_DOCKERFILE_CHECK_COPYIGNORED_EXPERIMENT": "true", + }, + BuildErrLocation: 3, + StreamBuildErrRegexp: regexp.MustCompile(`failed to solve: failed to compute cache key: failed to calculate checksum of ref [^\s]+ "/Dockerfile": not found`), + Warnings: []expectedLintWarning{ + { + RuleName: "CopyIgnoredFile", + Description: "Attempting to Copy file that is excluded by .dockerignore", + Detail: `Attempting to Copy file "Dockerfile" that is excluded by .dockerignore`, + URL: "https://docs.docker.com/go/dockerfile/rule/copy-ignored-file/", + Level: 1, + Line: 3, + }, + { + RuleName: "CopyIgnoredFile", + Description: "Attempting to Copy file that is excluded by .dockerignore", + Detail: `Attempting to Add file "Dockerfile" that is excluded by .dockerignore`, + URL: "https://docs.docker.com/go/dockerfile/rule/copy-ignored-file/", + Level: 1, + Line: 4, + }, + }, + }) + + dockerignore = []byte(` +foobar +`) + dockerfile = []byte(` +FROM scratch AS base +COPY Dockerfile /foobar +ADD Dockerfile /windy + +FROM base +COPY --from=base /foobar /Dockerfile +`) + checkLinterWarnings(t, sb, &lintTestParams{ + Dockerfile: dockerfile, + DockerIgnore: dockerignore, + }) +} + func testSecretsUsedInArgOrEnv(t *testing.T, sb integration.Sandbox) { dockerfile := []byte(` FROM scratch @@ -1206,11 +1267,19 @@ func checkUnmarshal(t *testing.T, sb integration.Sandbox, lintTest *lintTestPara lintResults, err := unmarshalLintResults(res) require.NoError(t, err) - if lintResults.Error != nil { - require.Equal(t, lintTest.UnmarshalBuildErr, lintResults.Error.Message) + if lintTest.UnmarshalBuildErr == "" && lintTest.UnmarshalBuildErrRegexp == nil { + require.Nil(t, lintResults.Error) + } else { + require.NotNil(t, lintResults.Error) + if lintTest.UnmarshalBuildErr != "" { + require.Equal(t, lintTest.UnmarshalBuildErr, lintResults.Error.Message) + } else if !lintTest.UnmarshalBuildErrRegexp.MatchString(lintResults.Error.Message) { + t.Fatalf("error %q does not match %q", lintResults.Error.Message, lintTest.UnmarshalBuildErrRegexp.String()) + } require.Greater(t, lintResults.Error.Location.SourceIndex, int32(-1)) require.Less(t, lintResults.Error.Location.SourceIndex, int32(len(lintResults.Sources))) } + require.Equal(t, len(warnings), len(lintResults.Warnings)) sort.Slice(lintResults.Warnings, func(i, j int) bool { @@ -1230,6 +1299,7 @@ func checkUnmarshal(t *testing.T, sb integration.Sandbox, lintTest *lintTestPara _, err = lintTest.Client.Build(sb.Context(), client.SolveOpt{ LocalMounts: map[string]fsutil.FS{ dockerui.DefaultLocalNameDockerfile: lintTest.TmpDir, + dockerui.DefaultLocalNameContext: lintTest.TmpDir, }, }, "", frontend, nil) require.NoError(t, err) @@ -1276,10 +1346,14 @@ func checkProgressStream(t *testing.T, sb integration.Sandbox, lintTest *lintTes dockerui.DefaultLocalNameContext: lintTest.TmpDir, }, }, status) - if lintTest.StreamBuildErr == "" { + if lintTest.StreamBuildErr == "" && lintTest.StreamBuildErrRegexp == nil { require.NoError(t, err) } else { - require.EqualError(t, err, lintTest.StreamBuildErr) + if lintTest.StreamBuildErr != "" { + require.EqualError(t, err, lintTest.StreamBuildErr) + } else if !lintTest.StreamBuildErrRegexp.MatchString(err.Error()) { + t.Fatalf("error %q does not match %q", err.Error(), lintTest.StreamBuildErrRegexp.String()) + } } select { @@ -1313,9 +1387,15 @@ func checkLinterWarnings(t *testing.T, sb integration.Sandbox, lintTest *lintTes integration.SkipOnPlatform(t, "windows") if lintTest.TmpDir == nil { + testfiles := []fstest.Applier{ + fstest.CreateFile("Dockerfile", lintTest.Dockerfile, 0600), + } + if lintTest.DockerIgnore != nil { + testfiles = append(testfiles, fstest.CreateFile(".dockerignore", lintTest.DockerIgnore, 0600)) + } lintTest.TmpDir = integration.Tmpdir( t, - fstest.CreateFile("Dockerfile", lintTest.Dockerfile, 0600), + testfiles..., ) } @@ -1374,13 +1454,16 @@ type expectedLintWarning struct { } type lintTestParams struct { - Client *client.Client - TmpDir *integration.TmpDirWithName - Dockerfile []byte - Warnings []expectedLintWarning - UnmarshalWarnings []expectedLintWarning - StreamBuildErr string - UnmarshalBuildErr string - BuildErrLocation int32 - FrontendAttrs map[string]string + Client *client.Client + TmpDir *integration.TmpDirWithName + Dockerfile []byte + DockerIgnore []byte + Warnings []expectedLintWarning + UnmarshalWarnings []expectedLintWarning + StreamBuildErr string + StreamBuildErrRegexp *regexp.Regexp + UnmarshalBuildErr string + UnmarshalBuildErrRegexp *regexp.Regexp + BuildErrLocation int32 + FrontendAttrs map[string]string } diff --git a/frontend/dockerfile/docs/rules/_index.md b/frontend/dockerfile/docs/rules/_index.md index bc65573ba045b..0508c34de9b0d 100644 --- a/frontend/dockerfile/docs/rules/_index.md +++ b/frontend/dockerfile/docs/rules/_index.md @@ -96,5 +96,9 @@ $ docker build --check .