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 10, 2024
1 parent 1ffc790 commit ae6a1cb
Show file tree
Hide file tree
Showing 3 changed files with 77 additions and 12 deletions.
53 changes: 41 additions & 12 deletions frontend/dockerfile/dockerfile2llb/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -549,18 +549,7 @@ func toDispatchState(ctx context.Context, dt []byte, opt ConvertOpt) (*dispatchS
if !isReachable(target, d) || d.noinit {
continue
}
// mark as initialized, used to determine states that have not been dispatched yet
d.noinit = true

if d.base != nil {
d.state = d.base.state
d.platform = d.base.platform
d.image = clone(d.base.image)
d.baseImg = cloneX(d.base.baseImg)
// Utilize the same path index as our base image so we propagate
// the paths we use back to the base image.
d.paths = d.base.paths
}
d.init()

// Ensure platform is set.
if d.platform == nil {
Expand Down Expand Up @@ -908,12 +897,35 @@ type dispatchState struct {
epoch *time.Time
scanStage bool
scanContext bool
// workdirSet is set to true if a workdir has been set
// within the current dockerfile.
workdirSet bool
}

func (ds *dispatchState) asyncLocalOpts() []llb.LocalOption {
return filterPaths(ds.paths)
}

// init is invoked when the dispatch state inherits its attributes
// from the base image.
func (ds *dispatchState) init() {
// mark as initialized, used to determine states that have not been dispatched yet
ds.noinit = true

if ds.base == nil {
return
}

ds.state = ds.base.state
ds.platform = ds.base.platform
ds.image = clone(ds.base.image)
ds.baseImg = cloneX(ds.base.baseImg)
// Utilize the same path index as our base image so we propagate
// the paths we use back to the base image.
ds.paths = ds.base.paths
ds.workdirSet = ds.base.workdirSet
}

type dispatchStates struct {
states []*dispatchState
statesByName map[string]*dispatchState
Expand Down Expand Up @@ -1128,6 +1140,23 @@ func dispatchRun(d *dispatchState, c *instructions.RunCommand, proxy *llb.ProxyE
}

func dispatchWorkdir(d *dispatchState, c *instructions.WorkdirCommand, commit bool, opt *dispatchOpt) error {
if commit {
// This linter rule checks if workdir has been est to an absolute value locally
// within the current dockerfile. Absolute paths in base images are ignored
// because they might change and it is not advised to rely on them.
//
// We only run this check when commit is true. Commit is true when we are performing
// this operation on a local call to workdir rather than one coming from
// the base image. We only check the first instance of workdir being set
// so successive relative paths are ignored because every instance is fixed
// by fixing the first one.
if !d.workdirSet && !system.IsAbs(c.Path, d.platform.OS) {
msg := linter.RuleWorkdirRelativePath.Format(c.Path)
linter.RuleWorkdirRelativePath.Run(opt.lintWarn, c.Location(), msg)
}
d.workdirSet = true
}

wd, err := system.NormalizeWorkdir(d.image.Config.WorkingDir, c.Path, d.platform.OS)
if err != nil {
return errors.Wrap(err, "normalizing workdir")
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 @@ -32,6 +32,7 @@ var lintTests = integration.TestFuncs(
testMaintainerDeprecated,
testWarningsBeforeError,
testUndeclaredArg,
testWorkdirRelativePath,
)

func testStageName(t *testing.T, sb integration.Sandbox) {
Expand Down Expand Up @@ -487,6 +488,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 @@ -77,4 +77,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 ae6a1cb

Please sign in to comment.