diff --git a/frontend/dockerfile/dockerfile2llb/convert.go b/frontend/dockerfile/dockerfile2llb/convert.go index 1d0623f039e8..34990b89eecd 100644 --- a/frontend/dockerfile/dockerfile2llb/convert.go +++ b/frontend/dockerfile/dockerfile2llb/convert.go @@ -881,7 +881,7 @@ func dispatch(d *dispatchState, cmd command, opt dispatchOpt) error { case *instructions.MaintainerCommand: err = dispatchMaintainer(d, c) case *instructions.EnvCommand: - err = dispatchEnv(d, c) + err = dispatchEnv(d, c, opt.lint) case *instructions.RunCommand: err = dispatchRun(d, c, opt.proxyEnv, cmd.sources, opt) case *instructions.WorkdirCommand: @@ -915,7 +915,7 @@ func dispatch(d *dispatchState, cmd command, opt dispatchOpt) error { } } case *instructions.LabelCommand: - err = dispatchLabel(d, c) + err = dispatchLabel(d, c, opt.lint) case *instructions.OnbuildCommand: err = dispatchOnbuild(d, c) case *instructions.CmdCommand: @@ -1104,9 +1104,13 @@ func dispatchOnBuildTriggers(d *dispatchState, triggers []string, opt dispatchOp return nil } -func dispatchEnv(d *dispatchState, c *instructions.EnvCommand) error { +func dispatchEnv(d *dispatchState, c *instructions.EnvCommand, lint *linter.Linter) error { commitMessage := bytes.NewBufferString("ENV") for _, e := range c.Env { + if e.NoDelim { + msg := linter.RuleLegacyKeyValueFormat.Format(c.Name()) + lint.Run(&linter.RuleLegacyKeyValueFormat, c.Location(), msg) + } commitMessage.WriteString(" " + e.String()) d.state = d.state.AddEnv(e.Key, e.Value) d.image.Config.Env = addEnv(d.image.Config.Env, e.Key, e.Value) @@ -1562,12 +1566,16 @@ func dispatchMaintainer(d *dispatchState, c *instructions.MaintainerCommand) err return commitToHistory(&d.image, fmt.Sprintf("MAINTAINER %v", c.Maintainer), false, nil, d.epoch) } -func dispatchLabel(d *dispatchState, c *instructions.LabelCommand) error { +func dispatchLabel(d *dispatchState, c *instructions.LabelCommand, lint *linter.Linter) error { commitMessage := bytes.NewBufferString("LABEL") if d.image.Config.Labels == nil { d.image.Config.Labels = make(map[string]string, len(c.Labels)) } for _, v := range c.Labels { + if v.NoDelim { + msg := linter.RuleLegacyKeyValueFormat.Format(c.Name()) + lint.Run(&linter.RuleLegacyKeyValueFormat, c.Location(), msg) + } d.image.Config.Labels[v.Key] = v.Value commitMessage.WriteString(" " + v.String()) } diff --git a/frontend/dockerfile/dockerfile_lint_test.go b/frontend/dockerfile/dockerfile_lint_test.go index 965fa19d18c7..ce0b3da76a4a 100644 --- a/frontend/dockerfile/dockerfile_lint_test.go +++ b/frontend/dockerfile/dockerfile_lint_test.go @@ -858,6 +858,37 @@ ENV key=value LABEL key=value `) checkLinterWarnings(t, sb, &lintTestParams{Dockerfile: dockerfile}) + + // Warnings only happen in unmarshal if the lint happens in a + // stage that's not reachable. + dockerfile = []byte(` +FROM scratch AS a + +FROM a AS b +ENV key value +LABEL key value + +FROM a AS c +`) + checkLinterWarnings(t, sb, &lintTestParams{ + Dockerfile: dockerfile, + UnmarshalWarnings: []expectedLintWarning{ + { + RuleName: "LegacyKeyValueFormat", + Description: "Legacy key/value format with whitespace separator should not be used", + Detail: "\"ENV key=value\" should be used instead of legacy \"ENV key value\" format", + Line: 3, + Level: 1, + }, + { + RuleName: "LegacyKeyValueFormat", + Description: "Legacy key/value format with whitespace separator should not be used", + Detail: "\"LABEL key=value\" should be used instead of legacy \"LABEL key value\" format", + Line: 4, + Level: 1, + }, + }, + }) } func checkUnmarshal(t *testing.T, sb integration.Sandbox, lintTest *lintTestParams) { diff --git a/frontend/dockerfile/instructions/commands.go b/frontend/dockerfile/instructions/commands.go index 8fb69380523e..c0e7abe7f787 100644 --- a/frontend/dockerfile/instructions/commands.go +++ b/frontend/dockerfile/instructions/commands.go @@ -13,8 +13,9 @@ import ( // This is useful for commands containing key-value maps that want to preserve // the order of insertion, instead of map[string]string which does not. type KeyValuePair struct { - Key string - Value string + Key string + Value string + NoDelim bool } func (kvp *KeyValuePair) String() string { @@ -108,8 +109,9 @@ func expandKvp(kvp KeyValuePair, expander SingleWordExpander) (KeyValuePair, err if err != nil { return KeyValuePair{}, err } - return KeyValuePair{Key: key, Value: value}, nil + return KeyValuePair{Key: key, Value: value, NoDelim: kvp.NoDelim}, nil } + func expandKvpsInPlace(kvps KeyValuePairs, expander SingleWordExpander) error { for i, kvp := range kvps { newKvp, err := expandKvp(kvp, expander) diff --git a/frontend/dockerfile/instructions/parse.go b/frontend/dockerfile/instructions/parse.go index 709474ae84f3..2933e0ba0ea6 100644 --- a/frontend/dockerfile/instructions/parse.go +++ b/frontend/dockerfile/instructions/parse.go @@ -78,13 +78,13 @@ func ParseInstructionWithLinter(node *parser.Node, lint *linter.Linter) (v inter req := newParseRequestFromNode(node) switch strings.ToLower(node.Value) { case command.Env: - return parseEnv(req, lint) + return parseEnv(req) case command.Maintainer: msg := linter.RuleMaintainerDeprecated.Format() lint.Run(&linter.RuleMaintainerDeprecated, node.Location(), msg) return parseMaintainer(req) case command.Label: - return parseLabel(req, lint) + return parseLabel(req) case command.Add: return parseAdd(req) case command.Copy: @@ -193,7 +193,7 @@ func Parse(ast *parser.Node, lint *linter.Linter) (stages []Stage, metaArgs []Ar return stages, metaArgs, nil } -func parseKvps(args []string, cmdName string, location []parser.Range, lint *linter.Linter) (KeyValuePairs, error) { +func parseKvps(args []string, cmdName string) (KeyValuePairs, error) { if len(args) == 0 { return nil, errAtLeastOneArgument(cmdName) } @@ -206,21 +206,17 @@ func parseKvps(args []string, cmdName string, location []parser.Range, lint *lin if len(args[j]) == 0 { return nil, errBlankCommandNames(cmdName) } - name, value, sep := args[j], args[j+1], args[j+2] - if sep == "" { - msg := linter.RuleLegacyKeyValueFormat.Format(cmdName) - lint.Run(&linter.RuleLegacyKeyValueFormat, location, msg) - } - res = append(res, KeyValuePair{Key: name, Value: value}) + name, value, delim := args[j], args[j+1], args[j+2] + res = append(res, KeyValuePair{Key: name, Value: value, NoDelim: delim == ""}) } return res, nil } -func parseEnv(req parseRequest, lint *linter.Linter) (*EnvCommand, error) { +func parseEnv(req parseRequest) (*EnvCommand, error) { if err := req.flags.Parse(); err != nil { return nil, err } - envs, err := parseKvps(req.args, "ENV", req.location, lint) + envs, err := parseKvps(req.args, "ENV") if err != nil { return nil, err } @@ -244,12 +240,12 @@ func parseMaintainer(req parseRequest) (*MaintainerCommand, error) { }, nil } -func parseLabel(req parseRequest, lint *linter.Linter) (*LabelCommand, error) { +func parseLabel(req parseRequest) (*LabelCommand, error) { if err := req.flags.Parse(); err != nil { return nil, err } - labels, err := parseKvps(req.args, "LABEL", req.location, lint) + labels, err := parseKvps(req.args, "LABEL") if err != nil { return nil, err }