Skip to content

Commit

Permalink
checks: add check for constant in from platform flag
Browse files Browse the repository at this point in the history
This linter rule triggers if a constant value has been used in the
`FROM --platform` flag and the stage name doesn't contain the OS or
architecture mentioned.

Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
  • Loading branch information
jsternberg committed Jul 10, 2024
1 parent 5b5afdb commit 58753e8
Show file tree
Hide file tree
Showing 6 changed files with 182 additions and 0 deletions.
26 changes: 26 additions & 0 deletions frontend/dockerfile/dockerfile2llb/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,7 @@ func toDispatchState(ctx context.Context, dt []byte, opt ConvertOpt) (*dispatchS
platMatch, err := shlex.ProcessWordWithMatches(v, platEnv)
reportUnusedFromArgs(metaArgsKeys(optMetaArgs), platMatch.Unmatched, st.Location, lint)
reportRedundantTargetPlatform(st.Platform, platMatch, st.Location, platEnv, lint)
reportConstPlatformDisallowed(st.Name, platMatch, st.Location, lint)

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

func reportConstPlatformDisallowed(stageName string, nameMatch shell.ProcessWordResult, location []parser.Range, lint *linter.Linter) {
if len(nameMatch.Matched) > 0 || len(nameMatch.Unmatched) > 0 {
// Some substitution happened so the platform was not a constant.
// Disable checking for this warning.
return
}

// Attempt to parse the platform result. If this fails, then it will fail
// later so just ignore.
p, err := platforms.Parse(nameMatch.Result)
if err != nil {
return
}

// Check if the platform os or architecture is used in the stage name
// at all. If it is, then disable this warning.
if strings.Contains(stageName, p.OS) || strings.Contains(stageName, p.Architecture) {
return
}

// Report the linter warning.
msg := linter.RuleFromPlatformFlagConstDisallowed.Format(nameMatch.Result)
lint.Run(&linter.RuleFromPlatformFlagConstDisallowed, location, msg)
}

type instructionTracker struct {
Loc []parser.Range
IsSet bool
Expand Down
34 changes: 34 additions & 0 deletions frontend/dockerfile/dockerfile_lint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ var lintTests = integration.TestFuncs(
testRedundantTargetPlatform,
testSecretsUsedInArgOrEnv,
testInvalidDefaultArgInFrom,
testFromPlatformFlagConstDisallowed,
)

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

func testFromPlatformFlagConstDisallowed(t *testing.T, sb integration.Sandbox) {
dockerfile := []byte(`
FROM --platform=linux/amd64 scratch
`)
checkLinterWarnings(t, sb, &lintTestParams{
Dockerfile: dockerfile,
Warnings: []expectedLintWarning{
{
RuleName: "FromPlatformFlagConstDisallowed",
Description: "FROM --platform flag should not use a constant value",
URL: "https://docs.docker.com/go/dockerfile/rule/from-platform-flag-const-disallowed/",
Detail: "FROM --platform flag should not use constant value \"linux/amd64\"",
Line: 2,
Level: 1,
},
},
})

dockerfile = []byte(`
FROM --platform=linux/amd64 scratch AS my_amd64_stage
`)
checkLinterWarnings(t, sb, &lintTestParams{
Dockerfile: dockerfile,
})

dockerfile = []byte(`
FROM --platform=linux/amd64 scratch AS linux
`)
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
4 changes: 4 additions & 0 deletions frontend/dockerfile/docs/rules/_index.md
Original file line number Diff line number Diff line change
Expand Up @@ -92,5 +92,9 @@ $ docker build --check .
<td><a href="./invalid-default-arg-in-from/">InvalidDefaultArgInFrom</a></td>
<td>Default value for global ARG results in an empty or invalid base image name</td>
</tr>
<tr>
<td><a href="./from-platform-flag-const-disallowed/">FromPlatformFlagConstDisallowed</a></td>
<td>FROM --platform flag should not use a constant value</td>
</tr>
</tbody>
</table>
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
---
title: FromPlatformFlagConstDisallowed
description: FROM --platform flag should not use a constant value
aliases:
- /go/dockerfile/rule/from-platform-flag-const-disallowed/
---

## Output

```text
FROM --platform flag should not use constant value "linux/amd64"
```

## Description

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`.

The recommended approach is to:

* Omit `FROM --platform` in the Dockerfile and use the `--platform` argument on the command line.
* Use `$BUILDPLATFORM` or some other combination of variables for the `--platform` argument.
* Stage name should include the platform, OS, or architecture name to indicate that it only contains platform-specific instructions.

## Examples

❌ Bad: using a constant argument for `--platform`

```dockerfile
FROM --platform=linux/amd64 alpine AS base
RUN apk add --no-cache git
```

✅ Good: using the default platform

```dockerfile
FROM alpine AS base
RUN apk add --no-cache git
```

✅ Good: using a meta variable

```dockerfile
FROM --platform=${BUILDPLATFORM} alpine AS base
RUN apk add --no-cache git
```

✅ Good: used in a multi-stage build with a target architecture

```dockerfile
FROM --platform=linux/amd64 alpine AS build_amd64
...

FROM --platform=linux/arm64 alpine AS build_arm64
...

FROM build_${TARGETARCH} AS build
...
```

51 changes: 51 additions & 0 deletions frontend/dockerfile/linter/docs/FromPlatformFlagConstDisallowed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
## Output

```text
FROM --platform flag should not use constant value "linux/amd64"
```

## Description

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`.

The recommended approach is to:

* Omit `FROM --platform` in the Dockerfile and use the `--platform` argument on the command line.
* Use `$BUILDPLATFORM` or some other combination of variables for the `--platform` argument.
* Stage name should include the platform, OS, or architecture name to indicate that it only contains platform-specific instructions.

## Examples

❌ Bad: using a constant argument for `--platform`

```dockerfile
FROM --platform=linux/amd64 alpine AS base
RUN apk add --no-cache git
```

✅ Good: using the default platform

```dockerfile
FROM alpine AS base
RUN apk add --no-cache git
```

✅ Good: using a meta variable

```dockerfile
FROM --platform=${BUILDPLATFORM} alpine AS base
RUN apk add --no-cache git
```

✅ Good: used in a multi-stage build with a target architecture

```dockerfile
FROM --platform=linux/amd64 alpine AS build_amd64
...

FROM --platform=linux/arm64 alpine AS build_arm64
...

FROM build_${TARGETARCH} AS build
...
```
8 changes: 8 additions & 0 deletions frontend/dockerfile/linter/ruleset.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,4 +148,12 @@ var (
return fmt.Sprintf("Default value for ARG %v results in empty or invalid base image name", baseName)
},
}
RuleFromPlatformFlagConstDisallowed = LinterRule[func(string) string]{
Name: "FromPlatformFlagConstDisallowed",
Description: "FROM --platform flag should not use a constant value",
URL: "https://docs.docker.com/go/dockerfile/rule/from-platform-flag-const-disallowed/",
Format: func(platform string) string {
return fmt.Sprintf("FROM --platform flag should not use constant value %q", platform)
},
}
)

0 comments on commit 58753e8

Please sign in to comment.