Merge pull request #4883 from jsternberg/lint-reserved-stage-names

linter: add linter rule for reserved stage names and duplicate stage names

frontend/dockerfile/dockerfile2llb/convert.go

@@ -217,6 +217,7 @@ func toDispatchState(ctx context.Context, dt []byte, opt ConvertOpt) (*dispatchS
 	if err != nil {
 		return nil, err
 	}
+	validateStageNames(stages, opt.Warn)
 
 	shlex := shell.NewLex(dockerfile.EscapeToken)
 	outline := newOutlineCapture()
@@ -1991,3 +1992,26 @@ func validateCommandCasing(dockerfile *parser.Result, warn linter.LintWarnFunc)
 		}
 	}
 }
+
+var reservedStageNames = map[string]struct{}{
+	"context": {},
+	"scratch": {},
+}
+
+func validateStageNames(stages []instructions.Stage, warn linter.LintWarnFunc) {
+	stageNames := make(map[string]struct{})
+	for _, stage := range stages {
+		if stage.Name != "" {
+			if _, ok := reservedStageNames[stage.Name]; ok {
+				msg := linter.RuleReservedStageName.Format(stage.Name)
+				linter.RuleReservedStageName.Run(warn, stage.Location, msg)
+			}
+
+			if _, ok := stageNames[stage.Name]; ok {
+				msg := linter.RuleDuplicateStageName.Format(stage.Name)
+				linter.RuleDuplicateStageName.Run(warn, stage.Location, msg)
+			}
+			stageNames[stage.Name] = struct{}{}
+		}
+	}
+}

frontend/dockerfile/dockerfile_lint_test.go

@@ -26,6 +26,8 @@ var lintTests = integration.TestFuncs(
 	testNoEmptyContinuations,
 	testSelfConsistentCommandCasing,
 	testFileConsistentCommandCasing,
+	testDuplicateStageName,
+	testReservedStageName,
 	testMaintainerDeprecated,
 )
 
@@ -209,6 +211,59 @@ COPY Dockerfile /bar
 	checkLinterWarnings(t, sb, dockerfile, []expectedLintWarning{})
 }
 
+func testDuplicateStageName(t *testing.T, sb integration.Sandbox) {
+	dockerfile := []byte(`
+FROM scratch AS b
+FROM scratch AS b
+`)
+	checkLinterWarnings(t, sb, dockerfile, []expectedLintWarning{
+		{
+			RuleName:    "DuplicateStageName",
+			Description: "Stage names should be unique",
+			Detail:      "Duplicate stage name \"b\", stage names should be unique",
+			Level:       1,
+			Line:        3,
+		},
+	})
+
+	dockerfile = []byte(`
+FROM scratch AS b1
+FROM scratch AS b2
+`)
+	checkLinterWarnings(t, sb, dockerfile, []expectedLintWarning{})
+}
+
+func testReservedStageName(t *testing.T, sb integration.Sandbox) {
+	dockerfile := []byte(`
+FROM scratch AS scratch
+FROM scratch AS context
+`)
+	checkLinterWarnings(t, sb, dockerfile, []expectedLintWarning{
+		{
+			RuleName:    "ReservedStageName",
+			Description: "Reserved stage names should not be used to name a stage",
+			Detail:      "Stage name should not use the same name as reserved stage \"scratch\"",
+			Level:       1,
+			Line:        2,
+		},
+		{
+			RuleName:    "ReservedStageName",
+			Description: "Reserved stage names should not be used to name a stage",
+			Detail:      "Stage name should not use the same name as reserved stage \"context\"",
+			Level:       1,
+			Line:        3,
+		},
+	})
+
+	// Using a reserved name as the base without a set name
+	// or a non-reserved name shouldn't trigger a lint warning.
+	dockerfile = []byte(`
+FROM scratch
+FROM scratch AS a
+`)
+	checkLinterWarnings(t, sb, dockerfile, []expectedLintWarning{})
+}
+
 func testMaintainerDeprecated(t *testing.T, sb integration.Sandbox) {
 	dockerfile := []byte(`
 FROM scratch

frontend/dockerfile/linter/ruleset.go

@@ -41,6 +41,20 @@ var (
 			return fmt.Sprintf("Command '%s' should match the case of the command majority (%s)", violatingCommand, correctCasing)
 		},
 	}
+	RuleDuplicateStageName = LinterRule[func(string) string]{
+		Name:        "DuplicateStageName",
+		Description: "Stage names should be unique",
+		Format: func(stageName string) string {
+			return fmt.Sprintf("Duplicate stage name %q, stage names should be unique", stageName)
+		},
+	}
+	RuleReservedStageName = LinterRule[func(string) string]{
+		Name:        "ReservedStageName",
+		Description: "Reserved stage names should not be used to name a stage",
+		Format: func(reservedStageName string) string {
+			return fmt.Sprintf("Stage name should not use the same name as reserved stage %q", reservedStageName)
+		},
+	}
 	RuleMaintainerDeprecated = LinterRule[func() string]{
 		Name:        "MaintainerDeprecated",
 		Description: "The maintainer instruction is deprecated, use a label instead to define an image author",