Skip to content

Commit 4096e93

Browse files
authored
Merge pull request moby#5140 from jsternberg/lint-constant-platform-disallowed
checks: add check for constant in from platform flag
2 parents 5b5afdb + 58753e8 commit 4096e93

File tree

6 files changed

+182
-0
lines changed

6 files changed

+182
-0
lines changed

frontend/dockerfile/dockerfile2llb/convert.go

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -315,6 +315,7 @@ func toDispatchState(ctx context.Context, dt []byte, opt ConvertOpt) (*dispatchS
315315
platMatch, err := shlex.ProcessWordWithMatches(v, platEnv)
316316
reportUnusedFromArgs(metaArgsKeys(optMetaArgs), platMatch.Unmatched, st.Location, lint)
317317
reportRedundantTargetPlatform(st.Platform, platMatch, st.Location, platEnv, lint)
318+
reportConstPlatformDisallowed(st.Name, platMatch, st.Location, lint)
318319

319320
if err != nil {
320321
return nil, parser.WithLocation(errors.Wrapf(err, "failed to process arguments for platform %s", platMatch.Result), st.Location)
@@ -2300,6 +2301,31 @@ func reportRedundantTargetPlatform(platformVar string, nameMatch shell.ProcessWo
23002301
}
23012302
}
23022303

2304+
func reportConstPlatformDisallowed(stageName string, nameMatch shell.ProcessWordResult, location []parser.Range, lint *linter.Linter) {
2305+
if len(nameMatch.Matched) > 0 || len(nameMatch.Unmatched) > 0 {
2306+
// Some substitution happened so the platform was not a constant.
2307+
// Disable checking for this warning.
2308+
return
2309+
}
2310+
2311+
// Attempt to parse the platform result. If this fails, then it will fail
2312+
// later so just ignore.
2313+
p, err := platforms.Parse(nameMatch.Result)
2314+
if err != nil {
2315+
return
2316+
}
2317+
2318+
// Check if the platform os or architecture is used in the stage name
2319+
// at all. If it is, then disable this warning.
2320+
if strings.Contains(stageName, p.OS) || strings.Contains(stageName, p.Architecture) {
2321+
return
2322+
}
2323+
2324+
// Report the linter warning.
2325+
msg := linter.RuleFromPlatformFlagConstDisallowed.Format(nameMatch.Result)
2326+
lint.Run(&linter.RuleFromPlatformFlagConstDisallowed, location, msg)
2327+
}
2328+
23032329
type instructionTracker struct {
23042330
Loc []parser.Range
23052331
IsSet bool

frontend/dockerfile/dockerfile_lint_test.go

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ var lintTests = integration.TestFuncs(
4343
testRedundantTargetPlatform,
4444
testSecretsUsedInArgOrEnv,
4545
testInvalidDefaultArgInFrom,
46+
testFromPlatformFlagConstDisallowed,
4647
)
4748

4849
func testSecretsUsedInArgOrEnv(t *testing.T, sb integration.Sandbox) {
@@ -1141,6 +1142,39 @@ FROM busybox:stable${BUSYBOX_VARIANT}
11411142
})
11421143
}
11431144

1145+
func testFromPlatformFlagConstDisallowed(t *testing.T, sb integration.Sandbox) {
1146+
dockerfile := []byte(`
1147+
FROM --platform=linux/amd64 scratch
1148+
`)
1149+
checkLinterWarnings(t, sb, &lintTestParams{
1150+
Dockerfile: dockerfile,
1151+
Warnings: []expectedLintWarning{
1152+
{
1153+
RuleName: "FromPlatformFlagConstDisallowed",
1154+
Description: "FROM --platform flag should not use a constant value",
1155+
URL: "https://docs.docker.com/go/dockerfile/rule/from-platform-flag-const-disallowed/",
1156+
Detail: "FROM --platform flag should not use constant value \"linux/amd64\"",
1157+
Line: 2,
1158+
Level: 1,
1159+
},
1160+
},
1161+
})
1162+
1163+
dockerfile = []byte(`
1164+
FROM --platform=linux/amd64 scratch AS my_amd64_stage
1165+
`)
1166+
checkLinterWarnings(t, sb, &lintTestParams{
1167+
Dockerfile: dockerfile,
1168+
})
1169+
1170+
dockerfile = []byte(`
1171+
FROM --platform=linux/amd64 scratch AS linux
1172+
`)
1173+
checkLinterWarnings(t, sb, &lintTestParams{
1174+
Dockerfile: dockerfile,
1175+
})
1176+
}
1177+
11441178
func checkUnmarshal(t *testing.T, sb integration.Sandbox, lintTest *lintTestParams) {
11451179
destDir, err := os.MkdirTemp("", "buildkit")
11461180
require.NoError(t, err)

frontend/dockerfile/docs/rules/_index.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,5 +92,9 @@ $ docker build --check .
9292
<td><a href="./invalid-default-arg-in-from/">InvalidDefaultArgInFrom</a></td>
9393
<td>Default value for global ARG results in an empty or invalid base image name</td>
9494
</tr>
95+
<tr>
96+
<td><a href="./from-platform-flag-const-disallowed/">FromPlatformFlagConstDisallowed</a></td>
97+
<td>FROM --platform flag should not use a constant value</td>
98+
</tr>
9599
</tbody>
96100
</table>
Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
---
2+
title: FromPlatformFlagConstDisallowed
3+
description: FROM --platform flag should not use a constant value
4+
aliases:
5+
- /go/dockerfile/rule/from-platform-flag-const-disallowed/
6+
---
7+
8+
## Output
9+
10+
```text
11+
FROM --platform flag should not use constant value "linux/amd64"
12+
```
13+
14+
## Description
15+
16+
Specifying `--platform` in the Dockerfile `FROM` instruction forces the image to build on only one target platform. This prevents building a multi-platform image from this Dockerfile and you must build on the same platform as specified in `--platform`.
17+
18+
The recommended approach is to:
19+
20+
* Omit `FROM --platform` in the Dockerfile and use the `--platform` argument on the command line.
21+
* Use `$BUILDPLATFORM` or some other combination of variables for the `--platform` argument.
22+
* Stage name should include the platform, OS, or architecture name to indicate that it only contains platform-specific instructions.
23+
24+
## Examples
25+
26+
❌ Bad: using a constant argument for `--platform`
27+
28+
```dockerfile
29+
FROM --platform=linux/amd64 alpine AS base
30+
RUN apk add --no-cache git
31+
```
32+
33+
✅ Good: using the default platform
34+
35+
```dockerfile
36+
FROM alpine AS base
37+
RUN apk add --no-cache git
38+
```
39+
40+
✅ Good: using a meta variable
41+
42+
```dockerfile
43+
FROM --platform=${BUILDPLATFORM} alpine AS base
44+
RUN apk add --no-cache git
45+
```
46+
47+
✅ Good: used in a multi-stage build with a target architecture
48+
49+
```dockerfile
50+
FROM --platform=linux/amd64 alpine AS build_amd64
51+
...
52+
53+
FROM --platform=linux/arm64 alpine AS build_arm64
54+
...
55+
56+
FROM build_${TARGETARCH} AS build
57+
...
58+
```
59+
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
## Output
2+
3+
```text
4+
FROM --platform flag should not use constant value "linux/amd64"
5+
```
6+
7+
## Description
8+
9+
Specifying `--platform` in the Dockerfile `FROM` instruction forces the image to build on only one target platform. This prevents building a multi-platform image from this Dockerfile and you must build on the same platform as specified in `--platform`.
10+
11+
The recommended approach is to:
12+
13+
* Omit `FROM --platform` in the Dockerfile and use the `--platform` argument on the command line.
14+
* Use `$BUILDPLATFORM` or some other combination of variables for the `--platform` argument.
15+
* Stage name should include the platform, OS, or architecture name to indicate that it only contains platform-specific instructions.
16+
17+
## Examples
18+
19+
❌ Bad: using a constant argument for `--platform`
20+
21+
```dockerfile
22+
FROM --platform=linux/amd64 alpine AS base
23+
RUN apk add --no-cache git
24+
```
25+
26+
✅ Good: using the default platform
27+
28+
```dockerfile
29+
FROM alpine AS base
30+
RUN apk add --no-cache git
31+
```
32+
33+
✅ Good: using a meta variable
34+
35+
```dockerfile
36+
FROM --platform=${BUILDPLATFORM} alpine AS base
37+
RUN apk add --no-cache git
38+
```
39+
40+
✅ Good: used in a multi-stage build with a target architecture
41+
42+
```dockerfile
43+
FROM --platform=linux/amd64 alpine AS build_amd64
44+
...
45+
46+
FROM --platform=linux/arm64 alpine AS build_arm64
47+
...
48+
49+
FROM build_${TARGETARCH} AS build
50+
...
51+
```

frontend/dockerfile/linter/ruleset.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,4 +148,12 @@ var (
148148
return fmt.Sprintf("Default value for ARG %v results in empty or invalid base image name", baseName)
149149
},
150150
}
151+
RuleFromPlatformFlagConstDisallowed = LinterRule[func(string) string]{
152+
Name: "FromPlatformFlagConstDisallowed",
153+
Description: "FROM --platform flag should not use a constant value",
154+
URL: "https://docs.docker.com/go/dockerfile/rule/from-platform-flag-const-disallowed/",
155+
Format: func(platform string) string {
156+
return fmt.Sprintf("FROM --platform flag should not use constant value %q", platform)
157+
},
158+
}
151159
)

0 commit comments

Comments
 (0)