Skip to content

Commit

Permalink
frontend/dockerfile: BFlags.Parse: include flag with "--" prefix in e…
Browse files Browse the repository at this point in the history
…rrors

Before this change, only the name of the flag would be printed as part
of errors. Change the errors to include the flag-name _including_ the
"--" prefix to make it more clear the error is about a flag that was
provided but invalid.

Before this change:

    Dockerfile:2
    --------------------
    1 |     FROM alpine
    2 | >>> COPY --nosuchflag . .
    3 |
    --------------------
    ERROR: failed to solve: dockerfile parse error on line 2: unknown flag: nosuchflag

    Dockerfile:4
    --------------------
    2 |     FROM alpine
    3 |     WORKDIR /test
    4 | >>> COPY --exclude /.git . .
    5 |
    --------------------
    ERROR: failed to solve: dockerfile parse error on line 4: missing a value on flag: exclude

With this change;

    Dockerfile:2
    --------------------
    1 |     FROM alpine
    2 | >>> COPY --nosuchflag . .
    3 |
    --------------------
    ERROR: failed to solve: dockerfile parse error on line 2: unknown flag: --nosuchflag

    Dockerfile:4
    --------------------
    2 |     FROM alpine
    3 |     WORKDIR /test
    4 | >>> COPY --exclude /.git . .
    5 |
    --------------------
    ERROR: failed to solve: dockerfile parse error on line 4: missing a value on flag: --exclude

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
  • Loading branch information
thaJeztah committed Oct 1, 2024
1 parent b300989 commit 31a7c17
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 14 deletions.
2 changes: 1 addition & 1 deletion frontend/dockerfile/dockerfile_mount_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ RUN --mont=target=/mytmp,type=tmpfs /bin/true
},
}, nil)
require.Error(t, err)
require.Contains(t, err.Error(), "unknown flag: mont")
require.Contains(t, err.Error(), "unknown flag: --mont")
require.Contains(t, err.Error(), "did you mean mount?")

dockerfile = []byte(`
Expand Down
24 changes: 12 additions & 12 deletions frontend/dockerfile/instructions/bflag.go
Original file line number Diff line number Diff line change
Expand Up @@ -147,8 +147,8 @@ func (bf *BFlags) Parse() error {
return errors.Wrap(bf.Err, "error setting up flags")
}

for _, arg := range bf.Args {
if arg == "--" {
for _, a := range bf.Args {
if a == "--" {
// Stop processing further arguments as flags. We're matching
// the POSIX Utility Syntax Guidelines here;
// https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap12.html#tag_12_02
Expand All @@ -158,21 +158,21 @@ func (bf *BFlags) Parse() error {
// > should be treated as operands, even if they begin with the '-' character.
return nil
}

if !strings.HasPrefix(arg, "--") {
return errors.Errorf("arg should start with -- : %s", arg)
if !strings.HasPrefix(a, "--") {
return errors.Errorf("arg should start with -- : %s", a)
}

arg, value, hasValue := strings.Cut(arg[2:], "=")
flagName, value, hasValue := strings.Cut(a, "=")
arg := flagName[2:]

flag, ok := bf.flags[arg]
if !ok {
err := errors.Errorf("unknown flag: %s", arg)
err := errors.Errorf("unknown flag: %s", flagName)
return suggest.WrapError(err, arg, allFlags(bf.flags), true)
}

if _, ok = bf.used[arg]; ok && flag.flagType != stringsType {
return errors.Errorf("duplicate flag specified: %s", arg)
return errors.Errorf("duplicate flag specified: %s", flagName)
}

bf.used[arg] = flag
Expand All @@ -181,7 +181,7 @@ func (bf *BFlags) Parse() error {
case boolType:
// value == "" is only ok if no "=" was specified
if hasValue && value == "" {
return errors.Errorf("missing a value on flag: %s", arg)
return errors.Errorf("missing a value on flag: %s", flagName)
}

switch strings.ToLower(value) {
Expand All @@ -190,18 +190,18 @@ func (bf *BFlags) Parse() error {
case "false":
flag.Value = "false"
default:
return errors.Errorf("expecting boolean value for flag %s, not: %s", arg, value)
return errors.Errorf("expecting boolean value for flag %s, not: %s", flagName, value)
}

case stringType:
if !hasValue {
return errors.Errorf("missing a value on flag: %s", arg)
return errors.Errorf("missing a value on flag: %s", flagName)
}
flag.Value = value

case stringsType:
if !hasValue {
return errors.Errorf("missing a value on flag: %s", arg)
return errors.Errorf("missing a value on flag: %s", flagName)
}
flag.StringValues = append(flag.StringValues, value)

Expand Down
2 changes: 1 addition & 1 deletion frontend/dockerfile/instructions/parse_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ func TestErrorCases(t *testing.T) {
{
name: "MAINTAINER unknown flag",
dockerfile: "MAINTAINER --boo joe@example.com",
expectedError: "unknown flag: boo",
expectedError: "unknown flag: --boo",
},
{
name: "Chaining ONBUILD",
Expand Down

0 comments on commit 31a7c17

Please sign in to comment.