Skip to content

Commit

Permalink
linter: add rule for relative path used in workdir
Browse files Browse the repository at this point in the history
Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
  • Loading branch information
jsternberg committed May 7, 2024
1 parent 51d85d7 commit 041e58d
Show file tree
Hide file tree
Showing 3 changed files with 84 additions and 0 deletions.
48 changes: 48 additions & 0 deletions frontend/dockerfile/dockerfile2llb/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -399,6 +399,8 @@ func toDispatchState(ctx context.Context, dt []byte, opt ConvertOpt) (*dispatchS
return nil, err
}

validateWorkdirs(allDispatchStates.states, platformOpt.targetPlatform, opt.Warn)

if len(allDispatchStates.states) == 1 {
allDispatchStates.states[0].stageName = ""
}
Expand Down Expand Up @@ -1124,6 +1126,8 @@ func dispatchRun(d *dispatchState, c *instructions.RunCommand, proxy *llb.ProxyE
}

func dispatchWorkdir(d *dispatchState, c *instructions.WorkdirCommand, commit bool, opt *dispatchOpt) error {
//validateWorkdir(c, d.image.Config.WorkingDir, d.platform.OS, opt.lintWarn)

wd, err := system.NormalizeWorkdir(d.image.Config.WorkingDir, c.Path, d.platform.OS)
if err != nil {
return errors.Wrap(err, "normalizing workdir")
Expand Down Expand Up @@ -1165,6 +1169,50 @@ func dispatchWorkdir(d *dispatchState, c *instructions.WorkdirCommand, commit bo
return nil
}

func validateWorkdirs(states []*dispatchState, targetPlatform ocispecs.Platform, warn linter.LintWarnFunc) {
isAbs := func(path string, platform *ocispecs.Platform) bool {
if platform == nil {
platform = &targetPlatform
}
return system.IsAbs(path, platform.OS)
}

for _, d := range states {
for _, cmd := range d.commands {
wd, ok := cmd.Command.(*instructions.WorkdirCommand)
if !ok {
continue
}

// If this command is a workdir with an absolute path, no need
// to look further.
if isAbs(wd.Path, d.platform) {
break
}

// Used a relative path. Look through previous stages
// to find a workdir command that uses an absolute path.
// We've already checked if this current stage has an absolute
// workdir so we can skip it and we can ignore relative workdirs
// in other stages since they'll be caught when they're examined.
hasAbsPath := false
for cur := d.base; cur != nil; cur = cur.base {
for _, cmd := range cur.commands {
if wd, ok := cmd.Command.(*instructions.WorkdirCommand); ok && isAbs(wd.Path, cur.platform) {
hasAbsPath = true
break
}
}
}

if !hasAbsPath {
msg := linter.RuleWorkdirRelativePath.Format(wd.Path)
linter.RuleWorkdirRelativePath.Run(warn, cmd.Location(), msg)
}
}
}
}

func dispatchCopy(d *dispatchState, cfg copyConfig) error {
dest, err := pathRelativeToWorkingDir(d.state, cfg.params.DestPath, *d.platform)
if err != nil {
Expand Down
29 changes: 29 additions & 0 deletions frontend/dockerfile/dockerfile_lint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ var lintTests = integration.TestFuncs(
testFileConsistentCommandCasing,
testDuplicateStageName,
testReservedStageName,
testWorkdirRelativePath,
testMaintainerDeprecated,
testWarningsBeforeError,
testUndeclaredArg,
Expand Down Expand Up @@ -418,6 +419,34 @@ COPY Dockerfile .
})
}

func testWorkdirRelativePath(t *testing.T, sb integration.Sandbox) {
dockerfile := []byte(`
FROM scratch
WORKDIR app/
`)
checkLinterWarnings(t, sb, &lintTestParams{
Dockerfile: dockerfile,
Warnings: []expectedLintWarning{
{
RuleName: "WorkdirRelativePath",
Description: "Relative workdir without an absolute workdir declared within the build can have unexpected results if the base image changes",
Detail: "Relative workdir \"app/\" can have unexpected results if the base image changes",
Level: 1,
Line: 3,
},
},
})

dockerfile = []byte(`
FROM scratch AS a
WORKDIR /app
FROM a AS b
WORKDIR subdir/
`)
checkLinterWarnings(t, sb, &lintTestParams{Dockerfile: dockerfile})
}

func checkUnmarshal(t *testing.T, sb integration.Sandbox, lintTest *lintTestParams) {
destDir, err := os.MkdirTemp("", "buildkit")
require.NoError(t, err)
Expand Down
7 changes: 7 additions & 0 deletions frontend/dockerfile/linter/ruleset.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,4 +70,11 @@ var (
return fmt.Sprintf("FROM argument '%s' is not declared", baseArg)
},
}
RuleWorkdirRelativePath = LinterRule[func(workdir string) string]{
Name: "WorkdirRelativePath",
Description: "Relative workdir without an absolute workdir declared within the build can have unexpected results if the base image changes",
Format: func(workdir string) string {
return fmt.Sprintf("Relative workdir %q can have unexpected results if the base image changes", workdir)
},
}
)

0 comments on commit 041e58d

Please sign in to comment.