From 58753e89212e07d54d3e0825b28b67b7ac89d506 Mon Sep 17 00:00:00 2001 From: "Jonathan A. Sternberg" Date: Wed, 10 Jul 2024 09:50:16 -0500 Subject: [PATCH] checks: add check for constant in from platform flag 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 --- frontend/dockerfile/dockerfile2llb/convert.go | 26 ++++++++ frontend/dockerfile/dockerfile_lint_test.go | 34 +++++++++++ frontend/dockerfile/docs/rules/_index.md | 4 ++ .../from-platform-flag-const-disallowed.md | 59 +++++++++++++++++++ .../docs/FromPlatformFlagConstDisallowed.md | 51 ++++++++++++++++ frontend/dockerfile/linter/ruleset.go | 8 +++ 6 files changed, 182 insertions(+) create mode 100644 frontend/dockerfile/docs/rules/from-platform-flag-const-disallowed.md create mode 100644 frontend/dockerfile/linter/docs/FromPlatformFlagConstDisallowed.md diff --git a/frontend/dockerfile/dockerfile2llb/convert.go b/frontend/dockerfile/dockerfile2llb/convert.go index 84716ec3ee91..b34180eff1fd 100644 --- a/frontend/dockerfile/dockerfile2llb/convert.go +++ b/frontend/dockerfile/dockerfile2llb/convert.go @@ -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) @@ -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 diff --git a/frontend/dockerfile/dockerfile_lint_test.go b/frontend/dockerfile/dockerfile_lint_test.go index a23b2b6a1c2b..04d66fa8dfd6 100644 --- a/frontend/dockerfile/dockerfile_lint_test.go +++ b/frontend/dockerfile/dockerfile_lint_test.go @@ -43,6 +43,7 @@ var lintTests = integration.TestFuncs( testRedundantTargetPlatform, testSecretsUsedInArgOrEnv, testInvalidDefaultArgInFrom, + testFromPlatformFlagConstDisallowed, ) func testSecretsUsedInArgOrEnv(t *testing.T, sb integration.Sandbox) { @@ -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) diff --git a/frontend/dockerfile/docs/rules/_index.md b/frontend/dockerfile/docs/rules/_index.md index 0980d5d398ec..bc65573ba045 100644 --- a/frontend/dockerfile/docs/rules/_index.md +++ b/frontend/dockerfile/docs/rules/_index.md @@ -92,5 +92,9 @@ $ docker build --check . InvalidDefaultArgInFrom Default value for global ARG results in an empty or invalid base image name + + FromPlatformFlagConstDisallowed + FROM --platform flag should not use a constant value + diff --git a/frontend/dockerfile/docs/rules/from-platform-flag-const-disallowed.md b/frontend/dockerfile/docs/rules/from-platform-flag-const-disallowed.md new file mode 100644 index 000000000000..9e4f7878871a --- /dev/null +++ b/frontend/dockerfile/docs/rules/from-platform-flag-const-disallowed.md @@ -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 +... +``` + diff --git a/frontend/dockerfile/linter/docs/FromPlatformFlagConstDisallowed.md b/frontend/dockerfile/linter/docs/FromPlatformFlagConstDisallowed.md new file mode 100644 index 000000000000..5186fc9f1022 --- /dev/null +++ b/frontend/dockerfile/linter/docs/FromPlatformFlagConstDisallowed.md @@ -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 +... +``` diff --git a/frontend/dockerfile/linter/ruleset.go b/frontend/dockerfile/linter/ruleset.go index b333c4c7c225..cc9249b0dc57 100644 --- a/frontend/dockerfile/linter/ruleset.go +++ b/frontend/dockerfile/linter/ruleset.go @@ -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) + }, + } )