linter: add redundant target platform check

Adds a linter check that checks if `--platform=$TARGETPLATFORM` is used
and sends a linter message marking this as redundant. This is because
`$TARGETPLATFORM` is the default.

Signed-off-by: Jonathan A. Sternberg <[email protected]>

frontend/dockerfile/dockerfile2llb/convert.go

@@ -289,7 +289,8 @@ func toDispatchState(ctx context.Context, dt []byte, opt ConvertOpt) (*dispatchS
 
  // set base state for every image
  for i, st := range stages {
- nameMatch, err := shlex.ProcessWordWithMatches(st.BaseName, metaArgsToEnvs(optMetaArgs))
+ env := metaArgsToEnvs(optMetaArgs)
+ nameMatch, err := shlex.ProcessWordWithMatches(st.BaseName, env)
  reportUnusedFromArgs(metaArgsKeys(optMetaArgs), nameMatch.Unmatched, st.Location, lint)
  used := nameMatch.Matched
  if used == nil {
@@ -316,8 +317,10 @@ func toDispatchState(ctx context.Context, dt []byte, opt ConvertOpt) (*dispatchS
  }
 
  if v := st.Platform; v != "" {
- platMatch, err := shlex.ProcessWordWithMatches(v, metaArgsToEnvs(optMetaArgs))
+ platEnv := metaArgsToEnvs(optMetaArgs)
+ platMatch, err := shlex.ProcessWordWithMatches(v, platEnv)
  reportUnusedFromArgs(metaArgsKeys(optMetaArgs), platMatch.Unmatched, st.Location, lint)
+ reportRedundantTargetPlatform(st.Platform, platMatch, st.Location, platEnv, lint)
 
  if err != nil {
  return nil, parser.WithLocation(errors.Wrapf(err, "failed to process arguments for platform %s", platMatch.Result), st.Location)
@@ -2278,6 +2281,26 @@ func reportUnusedFromArgs(values []string, unmatched map[string]struct{}, locati
  }
 }
 
+func reportRedundantTargetPlatform(platform string, nameMatch shell.ProcessWordResult, location []parser.Range, env shell.EnvGetter, lint *linter.Linter) {
+ // Only match this rule if there was only one matched name.
+ // It's psosible there were multiple args and that one of them expanded to an empty
+ // string and we don't want to report a warning when that happens.
+ if len(nameMatch.Matched) == 1 {
+ const targetPlatform = "TARGETPLATFORM"
+ // If target platform is the only environment variable that was substituted and the result
+ // matches the target platform exactly, we can infer that the input was ${TARGETPLATFORM} or
+ // $TARGETPLATFORM.
+ if _, ok := nameMatch.Matched[targetPlatform]; !ok {
+ return
+ }
+
+ if result, _ := env.Get(targetPlatform); nameMatch.Result == result {
+ msg := linter.RuleRedundantTargetPlatform.Format(platform)
+ lint.Run(&linter.RuleRedundantTargetPlatform, location, msg)
+ }
+ }
+}
+
 type instructionTracker struct {
  Loc []parser.Range
  IsSet bool

frontend/dockerfile/dockerfile_lint_test.go

@@ -40,6 +40,7 @@ var lintTests = integration.TestFuncs(
  testLegacyKeyValueFormat,
  testBaseImagePlatformMismatch,
  testAllTargetUnmarshal,
+ testRedundantTargetPlatform,
 )
 
 func testAllTargetUnmarshal(t *testing.T, sb integration.Sandbox) {
@@ -923,6 +924,42 @@ FROM a AS c
  })
 }
 
+func testRedundantTargetPlatform(t *testing.T, sb integration.Sandbox) {
+ dockerfile := []byte(`
+FROM --platform=$TARGETPLATFORM scratch
+`)
+ checkLinterWarnings(t, sb, &lintTestParams{
+ Dockerfile: dockerfile,
+ UnmarshalWarnings: []expectedLintWarning{
+ {
+ RuleName: "RedundantTargetPlatform",
+ Description: "Setting platform to predefined $TARGETPLATFORM in FROM is redundant as this is the default behavior",
+ URL: "https://docs.docker.com/go/dockerfile/rule/redundant-target-platform/",
+ Detail: "Setting platform to predefined $TARGETPLATFORM in FROM is redundant as this is the default behavior",
+ Line: 2,
+ Level: 1,
+ },
+ },
+ })
+
+ dockerfile = []byte(`
+FROM --platform=${TARGETPLATFORM} scratch
+`)
+ checkLinterWarnings(t, sb, &lintTestParams{
+ Dockerfile: dockerfile,
+ UnmarshalWarnings: []expectedLintWarning{
+ {
+ RuleName: "RedundantTargetPlatform",
+ Description: "Setting platform to predefined $TARGETPLATFORM in FROM is redundant as this is the default behavior",
+ URL: "https://docs.docker.com/go/dockerfile/rule/redundant-target-platform/",
+ Detail: "Setting platform to predefined ${TARGETPLATFORM} in FROM is redundant as this is the default behavior",
+ Line: 2,
+ Level: 1,
+ },
+ },
+ })
+}
+
 func checkUnmarshal(t *testing.T, sb integration.Sandbox, lintTest *lintTestParams) {
  destDir, err := os.MkdirTemp("", "buildkit")
  require.NoError(t, err)

frontend/dockerfile/dockerfile_test.go

@@ -122,7 +122,7 @@ var allTests = integration.TestFuncs(
  testErrorsSourceMap,
  testMultiArgs,
  testFrontendSubrequests,
- testDockefileCheckHostname,
+ testDockerfileCheckHostname,
  testDefaultShellAndPath,
  testDockerfileLowercase,
  testExportCacheLoop,
@@ -199,8 +199,10 @@ var reproTests = integration.TestFuncs(
  testReproSourceDateEpoch,
 )
 
-var opts []integration.TestOpt
-var securityOpts []integration.TestOpt
+var (
+ opts []integration.TestOpt
+ securityOpts []integration.TestOpt
+)
 
 type frontend interface {
  Solve(context.Context, *client.Client, client.SolveOpt, chan *client.SolveStatus) (*client.SolveResponse, error)
@@ -4324,6 +4326,7 @@ COPY --from=base unique /
  require.NoError(t, err)
  require.Equal(t, string(dt), string(dt2))
 }
+
 func testCacheImportExport(t *testing.T, sb integration.Sandbox) {
  integration.SkipOnPlatform(t, "windows")
  workers.CheckFeatureCompat(t, sb, workers.FeatureCacheExport, workers.FeatureCacheBackendLocal)
@@ -5222,7 +5225,7 @@ COPY Dockerfile Dockerfile
 }
 
 // moby/buildkit#1301
-func testDockefileCheckHostname(t *testing.T, sb integration.Sandbox) {
+func testDockerfileCheckHostname(t *testing.T, sb integration.Sandbox) {
  integration.SkipOnPlatform(t, "windows")
  f := getFrontend(t, sb)
  dockerfile := []byte(`
@@ -7433,6 +7436,7 @@ func (f *clientFrontend) SolveGateway(ctx context.Context, c gateway.Client, req
 func (f *clientFrontend) DFCmdArgs(ctx, dockerfile string) (string, string) {
  return "", ""
 }
+
 func (f *clientFrontend) RequiresBuildctl(t *testing.T) {
  t.Skip()
 }
@@ -7493,8 +7497,10 @@ func (*secModeInsecure) UpdateConfigFile(in string) string {
  return in + "\n\ninsecure-entitlements = [\"security.insecure\"]\n"
 }
 
-var securityInsecureGranted integration.ConfigUpdater = &secModeInsecure{}
-var securityInsecureDenied integration.ConfigUpdater = &secModeSandbox{}
+var (
+ securityInsecureGranted integration.ConfigUpdater = &secModeInsecure{}
+ securityInsecureDenied integration.ConfigUpdater = &secModeSandbox{}
+)
 
 type networkModeHost struct{}
 
@@ -7508,8 +7514,10 @@ func (*networkModeSandbox) UpdateConfigFile(in string) string {
  return in
 }
 
-var networkHostGranted integration.ConfigUpdater = &networkModeHost{}
-var networkHostDenied integration.ConfigUpdater = &networkModeSandbox{}
+var (
+ networkHostGranted integration.ConfigUpdater = &networkModeHost{}
+ networkHostDenied integration.ConfigUpdater = &networkModeSandbox{}
+)
 
 func fixedWriteCloser(wc io.WriteCloser) filesync.FileOutputFunc {
  return func(map[string]string) (io.WriteCloser, error) {

frontend/dockerfile/linter/ruleset.go

@@ -124,4 +124,12 @@ var (
  return fmt.Sprintf("Base image %s was pulled with platform %q, expected %q for current build", image, actual, expected)
  },
  }
+ RuleRedundantTargetPlatform = LinterRule[func(string) string]{
+ Name: "RedundantTargetPlatform",
+ Description: "Setting platform to predefined $TARGETPLATFORM in FROM is redundant as this is the default behavior",
+ URL: "https://docs.docker.com/go/dockerfile/rule/redundant-target-platform/",
+ Format: func(baseName string) string {
+ return fmt.Sprintf("Setting platform to predefined %s in FROM is redundant as this is the default behavior", baseName)
+ },
+ }
 )