Merge branch 'master' into copy-ignored-file-check

Signed-off-by: Talon Bowler <talon.bowler@docker.com>

.github/workflows/docs-upstream.yml

@@ -18,13 +18,15 @@ on:
       - 'docs/attestations/slsa-definitions.md'
       - 'docs/attestations/attestation-storage.md'
       - 'frontend/dockerfile/docs/reference.md'
+      - 'frontend/dockerfile/docs/rules/**'
   pull_request:
     paths:
       - '.github/workflows/docs-upstream.yml'
       - 'docs/buildkitd.toml.md'
       - 'docs/attestations/slsa-definitions.md'
       - 'docs/attestations/attestation-storage.md'
       - 'frontend/dockerfile/docs/reference.md'
+      - 'frontend/dockerfile/docs/rules/**'
 
 jobs:
   validate:

frontend/dockerfile/dockerfile2llb/convert.go

@@ -259,6 +259,9 @@ func toDispatchState(ctx context.Context, dt []byte, opt ConvertOpt) (*dispatchS
 	if err != nil {
 		return nil, err
 	}
+	if len(stages) == 0 {
+		return nil, errors.New("dockerfile contains no stages to build")
+	}
 	validateStageNames(stages, lint)
 
 	shlex := shell.NewLex(dockerfile.EscapeToken)
@@ -316,6 +319,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)
@@ -2347,6 +2351,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

frontend/dockerfile/dockerfile_lint_test.go

@@ -45,6 +45,7 @@ var lintTests = integration.TestFuncs(
 	testSecretsUsedInArgOrEnv,
 	testInvalidDefaultArgInFrom,
 	testCopyIgnoredFiles,
+	testFromPlatformFlagConstDisallowed,
 )
 
 func testCopyIgnoredFiles(t *testing.T, sb integration.Sandbox) {
@@ -1202,6 +1203,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)

frontend/dockerfile/dockerfile_test.go

@@ -192,9 +192,11 @@ var allTests = integration.TestFuncs(
 	testFrontendDeduplicateSources,
 	testDuplicateLayersProvenance,
 	testSourcePolicyWithNamedContext,
+	testEmptyStringArgInEnv,
 	testInvalidJSONCommands,
 	testHistoryError,
 	testHistoryFinalizeTrace,
+	testEmptyStages,
 )
 
 // Tests that depend on the `security.*` entitlements
@@ -273,6 +275,50 @@ func TestIntegration(t *testing.T) {
 	)...)
 }
 
+func testEmptyStringArgInEnv(t *testing.T, sb integration.Sandbox) {
+	integration.SkipOnPlatform(t, "windows")
+	f := getFrontend(t, sb)
+
+	dockerfile := []byte(`
+FROM busybox AS build
+ARG FOO
+ARG BAR=
+RUN env > env.txt
+
+FROM scratch
+COPY --from=build env.txt .
+`)
+	dir := integration.Tmpdir(
+		t,
+		fstest.CreateFile("Dockerfile", dockerfile, 0600),
+	)
+	c, err := client.New(sb.Context(), sb.Address())
+	require.NoError(t, err)
+	defer c.Close()
+
+	destDir := t.TempDir()
+	_, err = f.Solve(sb.Context(), c, client.SolveOpt{
+		Exports: []client.ExportEntry{
+			{
+				Type:      client.ExporterLocal,
+				OutputDir: destDir,
+			},
+		},
+		LocalMounts: map[string]fsutil.FS{
+			dockerui.DefaultLocalNameDockerfile: dir,
+			dockerui.DefaultLocalNameContext:    dir,
+		},
+	}, nil)
+	require.NoError(t, err)
+
+	dt, err := os.ReadFile(filepath.Join(destDir, "env.txt"))
+	require.NoError(t, err)
+
+	envStr := string(dt)
+	require.Contains(t, envStr, "BAR=")
+	require.NotContains(t, envStr, "FOO=")
+}
+
 func testDefaultEnvWithArgs(t *testing.T, sb integration.Sandbox) {
 	integration.SkipOnPlatform(t, "windows")
 	f := getFrontend(t, sb)
@@ -5434,6 +5480,40 @@ RUN echo $(hostname) | grep foo
 	}
 }
 
+func testEmptyStages(t *testing.T, sb integration.Sandbox) {
+	integration.SkipOnPlatform(t, "windows")
+	f := getFrontend(t, sb)
+	dockerfile := []byte(`
+ARG foo=bar
+`)
+
+	dir := integration.Tmpdir(
+		t,
+		fstest.CreateFile("Dockerfile", dockerfile, 0600),
+	)
+
+	c, err := client.New(sb.Context(), sb.Address())
+	require.NoError(t, err)
+	defer c.Close()
+
+	destDir := t.TempDir()
+
+	_, err = f.Solve(sb.Context(), c, client.SolveOpt{
+		LocalMounts: map[string]fsutil.FS{
+			dockerui.DefaultLocalNameDockerfile: dir,
+			dockerui.DefaultLocalNameContext:    dir,
+		},
+		Exports: []client.ExportEntry{
+			{
+				Type:      client.ExporterLocal,
+				OutputDir: destDir,
+			},
+		},
+	}, nil)
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "dockerfile contains no stages to build")
+}
+
 func testShmSize(t *testing.T, sb integration.Sandbox) {
 	integration.SkipOnPlatform(t, "windows")
 	f := getFrontend(t, sb)
@@ -7440,6 +7520,8 @@ COPY notexist /foo
 	}, nil)
 	require.Error(t, err)
 
+	expectedError := err
+
 	cl, err := c.ControlClient().ListenBuildHistory(sb.Context(), &controlapi.BuildHistoryRequest{
 		EarlyExit: true,
 		Ref:       ref,
@@ -7450,7 +7532,7 @@ COPY notexist /foo
 	for {
 		resp, err := cl.Recv()
 		if err == io.EOF {
-			require.Equal(t, true, got)
+			require.Equal(t, true, got, "expected error was %+v", expectedError)
 			break
 		}
 		require.NoError(t, err)