We're hoping to add some new commands that don't have any args so this
PR will enable that by removing all of the hard-coded checks that require
commands to have at least one arg. It also adds some checks to each
command so we're consistent in the error message we get. Added a test
for this too.
We actually had this check in at least 3 different places (twice in the
parser and once in most cmds), this removes 2 of them (the parser ones).
Had to remove/modify some testcases because its now legal to have certain
commands w/o args - e.g. RUN. This was actually inconsistent because
we used to allow "RUN []" but not "RUN" even though they would generate
(almost) the same net result. Now we're consistent.
Signed-off-by: Doug Davis <dug@us.ibm.com>
| ... | ... |
@@ -39,7 +39,7 @@ func nullDispatch(b *Builder, args []string, attributes map[string]bool, origina |
| 39 | 39 |
// |
| 40 | 40 |
func env(b *Builder, args []string, attributes map[string]bool, original string) error {
|
| 41 | 41 |
if len(args) == 0 {
|
| 42 |
- return fmt.Errorf("ENV is missing arguments")
|
|
| 42 |
+ return fmt.Errorf("ENV requires at least one argument")
|
|
| 43 | 43 |
} |
| 44 | 44 |
|
| 45 | 45 |
if len(args)%2 != 0 {
|
| ... | ... |
@@ -78,7 +78,7 @@ func env(b *Builder, args []string, attributes map[string]bool, original string) |
| 78 | 78 |
// Sets the maintainer metadata. |
| 79 | 79 |
func maintainer(b *Builder, args []string, attributes map[string]bool, original string) error {
|
| 80 | 80 |
if len(args) != 1 {
|
| 81 |
- return fmt.Errorf("MAINTAINER requires only one argument")
|
|
| 81 |
+ return fmt.Errorf("MAINTAINER requires exactly one argument")
|
|
| 82 | 82 |
} |
| 83 | 83 |
|
| 84 | 84 |
b.maintainer = args[0] |
| ... | ... |
@@ -159,6 +159,10 @@ func from(b *Builder, args []string, attributes map[string]bool, original string |
| 159 | 159 |
// cases. |
| 160 | 160 |
// |
| 161 | 161 |
func onbuild(b *Builder, args []string, attributes map[string]bool, original string) error {
|
| 162 |
+ if len(args) == 0 {
|
|
| 163 |
+ return fmt.Errorf("ONBUILD requires at least one argument")
|
|
| 164 |
+ } |
|
| 165 |
+ |
|
| 162 | 166 |
triggerInstruction := strings.ToUpper(strings.TrimSpace(args[0])) |
| 163 | 167 |
switch triggerInstruction {
|
| 164 | 168 |
case "ONBUILD": |
| ... | ... |
@@ -327,6 +331,10 @@ func entrypoint(b *Builder, args []string, attributes map[string]bool, original |
| 327 | 327 |
func expose(b *Builder, args []string, attributes map[string]bool, original string) error {
|
| 328 | 328 |
portsTab := args |
| 329 | 329 |
|
| 330 |
+ if len(args) == 0 {
|
|
| 331 |
+ return fmt.Errorf("EXPOSE requires at least one argument")
|
|
| 332 |
+ } |
|
| 333 |
+ |
|
| 330 | 334 |
if b.Config.ExposedPorts == nil {
|
| 331 | 335 |
b.Config.ExposedPorts = make(nat.PortSet) |
| 332 | 336 |
} |
| ... | ... |
@@ -373,7 +381,7 @@ func user(b *Builder, args []string, attributes map[string]bool, original string |
| 373 | 373 |
// |
| 374 | 374 |
func volume(b *Builder, args []string, attributes map[string]bool, original string) error {
|
| 375 | 375 |
if len(args) == 0 {
|
| 376 |
- return fmt.Errorf("Volume cannot be empty")
|
|
| 376 |
+ return fmt.Errorf("VOLUME requires at least one argument")
|
|
| 377 | 377 |
} |
| 378 | 378 |
|
| 379 | 379 |
if b.Config.Volumes == nil {
|
| ... | ... |
@@ -240,6 +240,9 @@ func (b *Builder) dispatch(stepN int, ast *parser.Node) error {
|
| 240 | 240 |
msg := fmt.Sprintf("Step %d : %s", stepN, strings.ToUpper(cmd))
|
| 241 | 241 |
|
| 242 | 242 |
if cmd == "onbuild" {
|
| 243 |
+ if ast.Next == nil {
|
|
| 244 |
+ return fmt.Errorf("ONBUILD requires at least one argument")
|
|
| 245 |
+ } |
|
| 243 | 246 |
ast = ast.Next.Children[0] |
| 244 | 247 |
strs = append(strs, ast.Value) |
| 245 | 248 |
msg += " " + ast.Value |
| ... | ... |
@@ -30,6 +30,10 @@ func parseIgnore(rest string) (*Node, map[string]bool, error) {
|
| 30 | 30 |
// ONBUILD RUN foo bar -> (onbuild (run foo bar)) |
| 31 | 31 |
// |
| 32 | 32 |
func parseSubCommand(rest string) (*Node, map[string]bool, error) {
|
| 33 |
+ if rest == "" {
|
|
| 34 |
+ return nil, nil, nil |
|
| 35 |
+ } |
|
| 36 |
+ |
|
| 33 | 37 |
_, child, err := parseLine(rest) |
| 34 | 38 |
if err != nil {
|
| 35 | 39 |
return nil, nil, err |
| ... | ... |
@@ -133,7 +137,7 @@ func parseEnv(rest string) (*Node, map[string]bool, error) {
|
| 133 | 133 |
} |
| 134 | 134 |
|
| 135 | 135 |
if len(words) == 0 {
|
| 136 |
- return nil, nil, fmt.Errorf("ENV must have some arguments")
|
|
| 136 |
+ return nil, nil, fmt.Errorf("ENV requires at least one argument")
|
|
| 137 | 137 |
} |
| 138 | 138 |
|
| 139 | 139 |
// Old format (ENV name value) |
| ... | ... |
@@ -181,6 +185,10 @@ func parseEnv(rest string) (*Node, map[string]bool, error) {
|
| 181 | 181 |
// parses a whitespace-delimited set of arguments. The result is effectively a |
| 182 | 182 |
// linked list of string arguments. |
| 183 | 183 |
func parseStringsWhitespaceDelimited(rest string) (*Node, map[string]bool, error) {
|
| 184 |
+ if rest == "" {
|
|
| 185 |
+ return nil, nil, nil |
|
| 186 |
+ } |
|
| 187 |
+ |
|
| 184 | 188 |
node := &Node{}
|
| 185 | 189 |
rootnode := node |
| 186 | 190 |
prevnode := node |
| ... | ... |
@@ -201,6 +209,9 @@ func parseStringsWhitespaceDelimited(rest string) (*Node, map[string]bool, error |
| 201 | 201 |
|
| 202 | 202 |
// parsestring just wraps the string in quotes and returns a working node. |
| 203 | 203 |
func parseString(rest string) (*Node, map[string]bool, error) {
|
| 204 |
+ if rest == "" {
|
|
| 205 |
+ return nil, nil, nil |
|
| 206 |
+ } |
|
| 204 | 207 |
n := &Node{}
|
| 205 | 208 |
n.Value = rest |
| 206 | 209 |
return n, nil, nil |
| ... | ... |
@@ -235,7 +246,9 @@ func parseJSON(rest string) (*Node, map[string]bool, error) {
|
| 235 | 235 |
// so, passes to parseJSON; if not, quotes the result and returns a single |
| 236 | 236 |
// node. |
| 237 | 237 |
func parseMaybeJSON(rest string) (*Node, map[string]bool, error) {
|
| 238 |
- rest = strings.TrimSpace(rest) |
|
| 238 |
+ if rest == "" {
|
|
| 239 |
+ return nil, nil, nil |
|
| 240 |
+ } |
|
| 239 | 241 |
|
| 240 | 242 |
node, attrs, err := parseJSON(rest) |
| 241 | 243 |
|
| ... | ... |
@@ -255,8 +268,6 @@ func parseMaybeJSON(rest string) (*Node, map[string]bool, error) {
|
| 255 | 255 |
// so, passes to parseJSON; if not, attmpts to parse it as a whitespace |
| 256 | 256 |
// delimited string. |
| 257 | 257 |
func parseMaybeJSONToList(rest string) (*Node, map[string]bool, error) {
|
| 258 |
- rest = strings.TrimSpace(rest) |
|
| 259 |
- |
|
| 260 | 258 |
node, attrs, err := parseJSON(rest) |
| 261 | 259 |
|
| 262 | 260 |
if err == nil {
|
| ... | ... |
@@ -3,7 +3,6 @@ package parser |
| 3 | 3 |
|
| 4 | 4 |
import ( |
| 5 | 5 |
"bufio" |
| 6 |
- "fmt" |
|
| 7 | 6 |
"io" |
| 8 | 7 |
"regexp" |
| 9 | 8 |
"strings" |
| ... | ... |
@@ -78,10 +77,6 @@ func parseLine(line string) (string, *Node, error) {
|
| 78 | 78 |
return "", nil, err |
| 79 | 79 |
} |
| 80 | 80 |
|
| 81 |
- if len(args) == 0 {
|
|
| 82 |
- return "", nil, fmt.Errorf("Instruction %q is empty; cannot continue", cmd)
|
|
| 83 |
- } |
|
| 84 |
- |
|
| 85 | 81 |
node := &Node{}
|
| 86 | 82 |
node.Value = cmd |
| 87 | 83 |
|
| ... | ... |
@@ -1,7 +1,6 @@ |
| 1 | 1 |
package parser |
| 2 | 2 |
|
| 3 | 3 |
import ( |
| 4 |
- "fmt" |
|
| 5 | 4 |
"strconv" |
| 6 | 5 |
"strings" |
| 7 | 6 |
) |
| ... | ... |
@@ -50,17 +49,19 @@ func fullDispatch(cmd, args string) (*Node, map[string]bool, error) {
|
| 50 | 50 |
// splitCommand takes a single line of text and parses out the cmd and args, |
| 51 | 51 |
// which are used for dispatching to more exact parsing functions. |
| 52 | 52 |
func splitCommand(line string) (string, string, error) {
|
| 53 |
+ var args string |
|
| 54 |
+ |
|
| 53 | 55 |
// Make sure we get the same results irrespective of leading/trailing spaces |
| 54 | 56 |
cmdline := TOKEN_WHITESPACE.Split(strings.TrimSpace(line), 2) |
| 57 |
+ cmd := strings.ToLower(cmdline[0]) |
|
| 55 | 58 |
|
| 56 |
- if len(cmdline) != 2 {
|
|
| 57 |
- return "", "", fmt.Errorf("We do not understand this file. Please ensure it is a valid Dockerfile. Parser error at %q", line)
|
|
| 59 |
+ if len(cmdline) == 2 {
|
|
| 60 |
+ args = strings.TrimSpace(cmdline[1]) |
|
| 58 | 61 |
} |
| 59 | 62 |
|
| 60 |
- cmd := strings.ToLower(cmdline[0]) |
|
| 61 | 63 |
// the cmd should never have whitespace, but it's possible for the args to |
| 62 | 64 |
// have trailing whitespace. |
| 63 |
- return cmd, strings.TrimSpace(cmdline[1]), nil |
|
| 65 |
+ return cmd, args, nil |
|
| 64 | 66 |
} |
| 65 | 67 |
|
| 66 | 68 |
// covers comments and empty lines. Lines should be trimmed before passing to |
| ... | ... |
@@ -49,14 +49,14 @@ func TestBuildEmptyWhitespace(t *testing.T) {
|
| 49 | 49 |
name, |
| 50 | 50 |
` |
| 51 | 51 |
FROM busybox |
| 52 |
- RUN |
|
| 52 |
+ COPY |
|
| 53 | 53 |
quux \ |
| 54 | 54 |
bar |
| 55 | 55 |
`, |
| 56 | 56 |
true) |
| 57 | 57 |
|
| 58 | 58 |
if err == nil {
|
| 59 |
- t.Fatal("no error when dealing with a RUN statement with no content on the same line")
|
|
| 59 |
+ t.Fatal("no error when dealing with a COPY statement with no content on the same line")
|
|
| 60 | 60 |
} |
| 61 | 61 |
|
| 62 | 62 |
logDone("build - statements with whitespace and no content should generate a parse error")
|
| ... | ... |
@@ -4822,3 +4822,51 @@ func TestBuildVolumeFileExistsinContainer(t *testing.T) {
|
| 4822 | 4822 |
|
| 4823 | 4823 |
logDone("build - errors when volume is specified where a file exists")
|
| 4824 | 4824 |
} |
| 4825 |
+ |
|
| 4826 |
+func TestBuildMissingArgs(t *testing.T) {
|
|
| 4827 |
+ // test to make sure these cmds w/o any args will generate an error |
|
| 4828 |
+ // Notice some commands are missing because its ok for them to |
|
| 4829 |
+ // not have any args - like: CMD, RUN, ENTRYPOINT |
|
| 4830 |
+ cmds := []string{
|
|
| 4831 |
+ "ADD", |
|
| 4832 |
+ "COPY", |
|
| 4833 |
+ "ENV", |
|
| 4834 |
+ "EXPOSE", |
|
| 4835 |
+ "FROM", |
|
| 4836 |
+ "MAINTAINER", |
|
| 4837 |
+ "ONBUILD", |
|
| 4838 |
+ "USER", |
|
| 4839 |
+ "VOLUME", |
|
| 4840 |
+ "WORKDIR", |
|
| 4841 |
+ } |
|
| 4842 |
+ |
|
| 4843 |
+ defer deleteAllContainers() |
|
| 4844 |
+ |
|
| 4845 |
+ for _, cmd := range cmds {
|
|
| 4846 |
+ var dockerfile string |
|
| 4847 |
+ |
|
| 4848 |
+ if cmd == "FROM" {
|
|
| 4849 |
+ dockerfile = cmd |
|
| 4850 |
+ } else {
|
|
| 4851 |
+ // Add FROM to make sure we don't complain about it missing |
|
| 4852 |
+ dockerfile = "FROM busybox\n" + cmd |
|
| 4853 |
+ } |
|
| 4854 |
+ |
|
| 4855 |
+ ctx, err := fakeContext(dockerfile, map[string]string{})
|
|
| 4856 |
+ if err != nil {
|
|
| 4857 |
+ t.Fatal(err) |
|
| 4858 |
+ } |
|
| 4859 |
+ defer ctx.Close() |
|
| 4860 |
+ var out string |
|
| 4861 |
+ if out, err = buildImageFromContext("args", ctx, true); err == nil {
|
|
| 4862 |
+ t.Fatalf("%s was supposed to fail:%s", cmd, out)
|
|
| 4863 |
+ } |
|
| 4864 |
+ if !strings.Contains(err.Error(), cmd+" requires") {
|
|
| 4865 |
+ t.Fatalf("%s returned the wrong type of error:%s", cmd, err)
|
|
| 4866 |
+ } |
|
| 4867 |
+ |
|
| 4868 |
+ ctx.Close() |
|
| 4869 |
+ } |
|
| 4870 |
+ |
|
| 4871 |
+ logDone("build - verify missing args")
|
|
| 4872 |
+} |