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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
111 changes: 79 additions & 32 deletions frontend/dockerfile/dockerfile2llb/convert.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -594,6 +595,20 @@ func toDispatchState(ctx context.Context, dt []byte, opt ConvertOpt) (*dispatchS
buildContext := &mutableOutput{}
ctxPaths := map[string]struct{}{}

var dockerIgnoreMatcher *patternmatcher.PatternMatcher
if opt.Client != nil && opt.Client.CopyIgnoredCheckEnabled {
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 {
Expand Down Expand Up @@ -634,24 +649,27 @@ 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,
}

if err = dispatchOnBuildTriggers(d, d.image.Config.OnBuild, opt); err != nil {
Expand Down Expand Up @@ -810,22 +828,23 @@ 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
}

func getEnv(state llb.State) shell.EnvGetter {
Expand Down Expand Up @@ -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,
})
}
Expand Down Expand Up @@ -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,
Expand All @@ -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 {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -1558,6 +1583,7 @@ type copyConfig struct {
checksum digest.Digest
parents bool
location []parser.Range
ignoreMatcher *patternmatcher.PatternMatcher
opt dispatchOpt
}

Expand Down Expand Up @@ -1871,6 +1897,27 @@ func addReachableStages(s *dispatchState, stages map[*dispatchState]struct{}) {
}
}

func validateCopySourcePath(src string, cfg *copyConfig) error {
if cfg.ignoreMatcher == nil {
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 {
Expand Down
113 changes: 99 additions & 14 deletions frontend/dockerfile/dockerfile_lint_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"fmt"
"maps"
"os"
"regexp"
"sort"
"testing"
"time"
Expand Down Expand Up @@ -44,8 +45,70 @@ 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,
BuildErrLocation: 3,
StreamBuildErrRegexp: regexp.MustCompile(`failed to solve: failed to compute cache key: failed to calculate checksum of ref [^\s]+ "/Dockerfile": not found`),
})

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
Expand Down Expand Up @@ -1206,11 +1269,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 {
Expand All @@ -1230,6 +1301,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)
Expand Down Expand Up @@ -1276,10 +1348,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 {
Expand Down Expand Up @@ -1313,9 +1389,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...,
)
}

Expand Down Expand Up @@ -1374,13 +1456,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
}
4 changes: 4 additions & 0 deletions frontend/dockerfile/docs/rules/_index.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,5 +96,9 @@ $ docker build --check .
<td><a href="./from-platform-flag-const-disallowed/">FromPlatformFlagConstDisallowed</a></td>
<td>FROM --platform flag should not use a constant value</td>
</tr>
<tr>
<td><a href="./copy-ignored-file/">CopyIgnoredFile</a></td>
<td>Attempting to Copy file that is excluded by .dockerignore</td>
</tr>
</tbody>
</table>
Loading
Loading