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.

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

linter: add rule for relative path used in workdir #4903

Merged
merged 1 commit into from
May 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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 set 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)
},
}
)
Loading