Merge pull request #10775 from duglin/BuilderFlags

Add support for Dockerfile CMD options

builder/bflag.go

@@ -0,0 +1,155 @@
+package builder
+
+import (
+	"fmt"
+	"strings"
+)
+
+type FlagType int
+
+const (
+	boolType FlagType = iota
+	stringType
+)
+
+type BuilderFlags struct {
+	Args  []string // actual flags/args from cmd line
+	flags map[string]*Flag
+	used  map[string]*Flag
+	Err   error
+}
+
+type Flag struct {
+	bf       *BuilderFlags
+	name     string
+	flagType FlagType
+	Value    string
+}
+
+func NewBuilderFlags() *BuilderFlags {
+	return &BuilderFlags{
+		flags: make(map[string]*Flag),
+		used:  make(map[string]*Flag),
+	}
+}
+
+func (bf *BuilderFlags) AddBool(name string, def bool) *Flag {
+	flag := bf.addFlag(name, boolType)
+	if flag == nil {
+		return nil
+	}
+	if def {
+		flag.Value = "true"
+	} else {
+		flag.Value = "false"
+	}
+	return flag
+}
+
+func (bf *BuilderFlags) AddString(name string, def string) *Flag {
+	flag := bf.addFlag(name, stringType)
+	if flag == nil {
+		return nil
+	}
+	flag.Value = def
+	return flag
+}
+
+func (bf *BuilderFlags) addFlag(name string, flagType FlagType) *Flag {
+	if _, ok := bf.flags[name]; ok {
+		bf.Err = fmt.Errorf("Duplicate flag defined: %s", name)
+		return nil
+	}
+
+	newFlag := &Flag{
+		bf:       bf,
+		name:     name,
+		flagType: flagType,
+	}
+	bf.flags[name] = newFlag
+
+	return newFlag
+}
+
+func (fl *Flag) IsUsed() bool {
+	if _, ok := fl.bf.used[fl.name]; ok {
+		return true
+	}
+	return false
+}
+
+func (fl *Flag) IsTrue() bool {
+	if fl.flagType != boolType {
+		// Should never get here
+		panic(fmt.Errorf("Trying to use IsTrue on a non-boolean: %s", fl.name))
+	}
+	return fl.Value == "true"
+}
+
+func (bf *BuilderFlags) Parse() error {
+	// If there was an error while defining the possible flags
+	// go ahead and bubble it back up here since we didn't do it
+	// earlier in the processing
+	if bf.Err != nil {
+		return fmt.Errorf("Error setting up flags: %s", bf.Err)
+	}
+
+	for _, arg := range bf.Args {
+		if !strings.HasPrefix(arg, "--") {
+			return fmt.Errorf("Arg should start with -- : %s", arg)
+		}
+
+		if arg == "--" {
+			return nil
+		}
+
+		arg = arg[2:]
+		value := ""
+
+		index := strings.Index(arg, "=")
+		if index >= 0 {
+			value = arg[index+1:]
+			arg = arg[:index]
+		}
+
+		flag, ok := bf.flags[arg]
+		if !ok {
+			return fmt.Errorf("Unknown flag: %s", arg)
+		}
+
+		if _, ok = bf.used[arg]; ok {
+			return fmt.Errorf("Duplicate flag specified: %s", arg)
+		}
+
+		bf.used[arg] = flag
+
+		switch flag.flagType {
+		case boolType:
+			// value == "" is only ok if no "=" was specified
+			if index >= 0 && value == "" {
+				return fmt.Errorf("Missing a value on flag: %s", arg)
+			}
+
+			lower := strings.ToLower(value)
+			if lower == "" {
+				flag.Value = "true"
+			} else if lower == "true" || lower == "false" {
+				flag.Value = lower
+			} else {
+				return fmt.Errorf("Expecting boolean value for flag %s, not: %s", arg, value)
+			}
+
+		case stringType:
+			if index < 0 {
+				return fmt.Errorf("Missing a value on flag: %s", arg)
+			}
+			flag.Value = value
+
+		default:
+			panic(fmt.Errorf("No idea what kind of flag we have! Should never get here!"))
+		}
+
+	}
+
+	return nil
+}

builder/bflag_test.go

@@ -0,0 +1,187 @@
+package builder
+
+import (
+	"testing"
+)
+
+func TestBuilderFlags(t *testing.T) {
+	var expected string
+	var err error
+
+	// ---
+
+	bf := NewBuilderFlags()
+	bf.Args = []string{}
+	if err := bf.Parse(); err != nil {
+		t.Fatalf("Test1 of %q was supposed to work: %s", bf.Args, err)
+	}
+
+	// ---
+
+	bf = NewBuilderFlags()
+	bf.Args = []string{"--"}
+	if err := bf.Parse(); err != nil {
+		t.Fatalf("Test2 of %q was supposed to work: %s", bf.Args, err)
+	}
+
+	// ---
+
+	bf = NewBuilderFlags()
+	flStr1 := bf.AddString("str1", "")
+	flBool1 := bf.AddBool("bool1", false)
+	bf.Args = []string{}
+	if err = bf.Parse(); err != nil {
+		t.Fatalf("Test3 of %q was supposed to work: %s", bf.Args, err)
+	}
+
+	if flStr1.IsUsed() == true {
+		t.Fatalf("Test3 - str1 was not used!")
+	}
+	if flBool1.IsUsed() == true {
+		t.Fatalf("Test3 - bool1 was not used!")
+	}
+
+	// ---
+
+	bf = NewBuilderFlags()
+	flStr1 = bf.AddString("str1", "HI")
+	flBool1 = bf.AddBool("bool1", false)
+	bf.Args = []string{}
+
+	if err = bf.Parse(); err != nil {
+		t.Fatalf("Test4 of %q was supposed to work: %s", bf.Args, err)
+	}
+
+	if flStr1.Value != "HI" {
+		t.Fatalf("Str1 was supposed to default to: HI")
+	}
+	if flBool1.IsTrue() {
+		t.Fatalf("Bool1 was supposed to default to: false")
+	}
+	if flStr1.IsUsed() == true {
+		t.Fatalf("Str1 was not used!")
+	}
+	if flBool1.IsUsed() == true {
+		t.Fatalf("Bool1 was not used!")
+	}
+
+	// ---
+
+	bf = NewBuilderFlags()
+	flStr1 = bf.AddString("str1", "HI")
+	bf.Args = []string{"--str1"}
+
+	if err = bf.Parse(); err == nil {
+		t.Fatalf("Test %q was supposed to fail", bf.Args)
+	}
+
+	// ---
+
+	bf = NewBuilderFlags()
+	flStr1 = bf.AddString("str1", "HI")
+	bf.Args = []string{"--str1="}
+
+	if err = bf.Parse(); err != nil {
+		t.Fatalf("Test %q was supposed to work: %s", bf.Args, err)
+	}
+
+	expected = ""
+	if flStr1.Value != expected {
+		t.Fatalf("Str1 (%q) should be: %q", flStr1.Value, expected)
+	}
+
+	// ---
+
+	bf = NewBuilderFlags()
+	flStr1 = bf.AddString("str1", "HI")
+	bf.Args = []string{"--str1=BYE"}
+
+	if err = bf.Parse(); err != nil {
+		t.Fatalf("Test %q was supposed to work: %s", bf.Args, err)
+	}
+
+	expected = "BYE"
+	if flStr1.Value != expected {
+		t.Fatalf("Str1 (%q) should be: %q", flStr1.Value, expected)
+	}
+
+	// ---
+
+	bf = NewBuilderFlags()
+	flBool1 = bf.AddBool("bool1", false)
+	bf.Args = []string{"--bool1"}
+
+	if err = bf.Parse(); err != nil {
+		t.Fatalf("Test %q was supposed to work: %s", bf.Args, err)
+	}
+
+	if !flBool1.IsTrue() {
+		t.Fatalf("Test-b1 Bool1 was supposed to be true")
+	}
+
+	// ---
+
+	bf = NewBuilderFlags()
+	flBool1 = bf.AddBool("bool1", false)
+	bf.Args = []string{"--bool1=true"}
+
+	if err = bf.Parse(); err != nil {
+		t.Fatalf("Test %q was supposed to work: %s", bf.Args, err)
+	}
+
+	if !flBool1.IsTrue() {
+		t.Fatalf("Test-b2 Bool1 was supposed to be true")
+	}
+
+	// ---
+
+	bf = NewBuilderFlags()
+	flBool1 = bf.AddBool("bool1", false)
+	bf.Args = []string{"--bool1=false"}
+
+	if err = bf.Parse(); err != nil {
+		t.Fatalf("Test %q was supposed to work: %s", bf.Args, err)
+	}
+
+	if flBool1.IsTrue() {
+		t.Fatalf("Test-b3 Bool1 was supposed to be false")
+	}
+
+	// ---
+
+	bf = NewBuilderFlags()
+	flBool1 = bf.AddBool("bool1", false)
+	bf.Args = []string{"--bool1=false1"}
+
+	if err = bf.Parse(); err == nil {
+		t.Fatalf("Test %q was supposed to fail", bf.Args)
+	}
+
+	// ---
+
+	bf = NewBuilderFlags()
+	flBool1 = bf.AddBool("bool1", false)
+	bf.Args = []string{"--bool2"}
+
+	if err = bf.Parse(); err == nil {
+		t.Fatalf("Test %q was supposed to fail", bf.Args)
+	}
+
+	// ---
+
+	bf = NewBuilderFlags()
+	flStr1 = bf.AddString("str1", "HI")
+	flBool1 = bf.AddBool("bool1", false)
+	bf.Args = []string{"--bool1", "--str1=BYE"}
+
+	if err = bf.Parse(); err != nil {
+		t.Fatalf("Test %q was supposed to work: %s", bf.Args, err)
+	}
+
+	if flStr1.Value != "BYE" {
+		t.Fatalf("Teset %s, str1 should be BYE", bf.Args)
+	}
+	if !flBool1.IsTrue() {
+		t.Fatalf("Teset %s, bool1 should be true", bf.Args)
+	}
+}

builder/dispatchers.go

@@ -47,6 +47,22 @@ func env(b *Builder, args []string, attributes map[string]bool, original string)
 		return fmt.Errorf("Bad input to ENV, too many args")
 	}
 
+	// TODO/FIXME/NOT USED
+	// Just here to show how to use the builder flags stuff within the
+	// context of a builder command. Will remove once we actually add
+	// a builder command to something!
+	/*
+		flBool1 := b.BuilderFlags.AddBool("bool1", false)
+		flStr1 := b.BuilderFlags.AddString("str1", "HI")
+
+		if err := b.BuilderFlags.Parse(); err != nil {
+			return err
+		}
+
+		fmt.Printf("Bool1:%v\n", flBool1)
+		fmt.Printf("Str1:%v\n", flStr1)
+	*/
+
 	commitStr := "ENV"
 
 	for j := 0; j < len(args); j++ {

builder/evaluator.go

@@ -116,6 +116,7 @@ type Builder struct {
 	image          string        // image name for commit processing
 	maintainer     string        // maintainer name. could probably be removed.
 	cmdSet         bool          // indicates is CMD was set in current Dockerfile
+	BuilderFlags   *BuilderFlags // current cmd's BuilderFlags - temporary
 	context        tarsum.TarSum // the context is a tarball that is uploaded by the client
 	contextPath    string        // the path of the temporary directory the local context is unpacked to (server side)
 	noBaseImage    bool          // indicates that this build does not start from any base image, but is being built from an empty file system.
@@ -276,8 +277,9 @@ func (b *Builder) dispatch(stepN int, ast *parser.Node) error {
 	cmd := ast.Value
 	attrs := ast.Attributes
 	original := ast.Original
+	flags := ast.Flags
 	strs := []string{}
-	msg := fmt.Sprintf("Step %d : %s", stepN, strings.ToUpper(cmd))
+	msg := fmt.Sprintf("Step %d : %s", stepN, original)
 
 	if cmd == "onbuild" {
 		if ast.Next == nil {
@@ -325,6 +327,8 @@ func (b *Builder) dispatch(stepN int, ast *parser.Node) error {
 	// XXX yes, we skip any cmds that are not valid; the parser should have
 	// picked these out already.
 	if f, ok := evaluateTable[cmd]; ok {
+		b.BuilderFlags = NewBuilderFlags()
+		b.BuilderFlags.Args = flags
 		return f(b, strList, attrs, original)
 	}
 

builder/parser/parser.go

@@ -29,6 +29,7 @@ type Node struct {
 	Children   []*Node         // the children of this sexp
 	Attributes map[string]bool // special attributes for this node
 	Original   string          // original line used before parsing
+	Flags      []string        // only top Node should have this set
 }
 
 var (
@@ -75,7 +76,7 @@ func parseLine(line string) (string, *Node, error) {
 		return line, nil, nil
 	}
 
-	cmd, args, err := splitCommand(line)
+	cmd, flags, args, err := splitCommand(line)
 	if err != nil {
 		return "", nil, err
 	}
@@ -91,6 +92,7 @@ func parseLine(line string) (string, *Node, error) {
 	node.Next = sexp
 	node.Attributes = attrs
 	node.Original = line
+	node.Flags = flags
 
 	return "", node, nil
 }

builder/parser/testfiles/flags/Dockerfile

@@ -0,0 +1,10 @@
+FROM scratch
+COPY foo /tmp/
+COPY --user=me foo /tmp/
+COPY --doit=true foo /tmp/
+COPY --user=me --doit=true foo /tmp/
+COPY --doit=true -- foo /tmp/
+COPY -- foo /tmp/
+CMD --doit [ "a", "b" ]
+CMD --doit=true -- [ "a", "b" ]
+CMD --doit -- [ ]

builder/parser/testfiles/flags/result

@@ -0,0 +1,10 @@
+(from "scratch")
+(copy "foo" "/tmp/")
+(copy ["--user=me"] "foo" "/tmp/")
+(copy ["--doit=true"] "foo" "/tmp/")
+(copy ["--user=me" "--doit=true"] "foo" "/tmp/")
+(copy ["--doit=true"] "foo" "/tmp/")
+(copy "foo" "/tmp/")
+(cmd ["--doit"] "a" "b")
+(cmd ["--doit=true"] "a" "b")
+(cmd ["--doit"])

builder/parser/utils.go

@@ -1,8 +1,10 @@
 package parser
 
 import (
+	"fmt"
 	"strconv"
 	"strings"
+	"unicode"
 )
 
 // dumps the AST defined by `node` as a list of sexps. Returns a string
@@ -11,6 +13,10 @@ func (node *Node) Dump() string {
 	str := ""
 	str += node.Value
 
+	if len(node.Flags) > 0 {
+		str += fmt.Sprintf(" %q", node.Flags)
+	}
+
 	for _, n := range node.Children {
 		str += "(" + n.Dump() + ")\n"
 	}
@@ -48,20 +54,23 @@ func fullDispatch(cmd, args string) (*Node, map[string]bool, error) {
 
 // splitCommand takes a single line of text and parses out the cmd and args,
 // which are used for dispatching to more exact parsing functions.
-func splitCommand(line string) (string, string, error) {
+func splitCommand(line string) (string, []string, string, error) {
 	var args string
+	var flags []string
 
 	// Make sure we get the same results irrespective of leading/trailing spaces
 	cmdline := TOKEN_WHITESPACE.Split(strings.TrimSpace(line), 2)
 	cmd := strings.ToLower(cmdline[0])
 
 	if len(cmdline) == 2 {
-		args = strings.TrimSpace(cmdline[1])
+		var err error
+		args, flags, err = extractBuilderFlags(cmdline[1])
+		if err != nil {
+			return "", nil, "", err
+		}
 	}
 
-	// the cmd should never have whitespace, but it's possible for the args to
-	// have trailing whitespace.
-	return cmd, args, nil
+	return cmd, flags, strings.TrimSpace(args), nil
 }
 
 // covers comments and empty lines. Lines should be trimmed before passing to
@@ -74,3 +83,94 @@ func stripComments(line string) string {
 
 	return line
 }
+
+func extractBuilderFlags(line string) (string, []string, error) {
+	// Parses the BuilderFlags and returns the remaining part of the line
+
+	const (
+		inSpaces = iota // looking for start of a word
+		inWord
+		inQuote
+	)
+
+	words := []string{}
+	phase := inSpaces
+	word := ""
+	quote := '\000'
+	blankOK := false
+	var ch rune
+
+	for pos := 0; pos <= len(line); pos++ {
+		if pos != len(line) {
+			ch = rune(line[pos])
+		}
+
+		if phase == inSpaces { // Looking for start of word
+			if pos == len(line) { // end of input
+				break
+			}
+			if unicode.IsSpace(ch) { // skip spaces
+				continue
+			}
+
+			// Only keep going if the next word starts with --
+			if ch != '-' || pos+1 == len(line) || rune(line[pos+1]) != '-' {
+				return line[pos:], words, nil
+			}
+
+			phase = inWord // found someting with "--", fall thru
+		}
+		if (phase == inWord || phase == inQuote) && (pos == len(line)) {
+			if word != "--" && (blankOK || len(word) > 0) {
+				words = append(words, word)
+			}
+			break
+		}
+		if phase == inWord {
+			if unicode.IsSpace(ch) {
+				phase = inSpaces
+				if word == "--" {
+					return line[pos:], words, nil
+				}
+				if blankOK || len(word) > 0 {
+					words = append(words, word)
+				}
+				word = ""
+				blankOK = false
+				continue
+			}
+			if ch == '\'' || ch == '"' {
+				quote = ch
+				blankOK = true
+				phase = inQuote
+				continue
+			}
+			if ch == '\\' {
+				if pos+1 == len(line) {
+					continue // just skip \ at end
+				}
+				pos++
+				ch = rune(line[pos])
+			}
+			word += string(ch)
+			continue
+		}
+		if phase == inQuote {
+			if ch == quote {
+				phase = inWord
+				continue
+			}
+			if ch == '\\' {
+				if pos+1 == len(line) {
+					phase = inWord
+					continue // just skip \ at end
+				}
+				pos++
+				ch = rune(line[pos])
+			}
+			word += string(ch)
+		}
+	}
+
+	return "", words, nil
+}