From 0287cdd6149124e39b35e4aad9c63798fd10b6f6 Mon Sep 17 00:00:00 2001
From: Mitchell Hashimoto <mitchell.hashimoto@gmail.com>
Date: Sat, 9 Nov 2013 17:07:14 -0800
Subject: [PATCH] builder/docker: config refactor

---
 builder/docker/builder.go     | 27 +++------------
 builder/docker/config.go      | 29 ++++++++++++----
 builder/docker/config_test.go | 63 +++++++++++++++++++----------------
 3 files changed, 62 insertions(+), 57 deletions(-)

diff --git a/builder/docker/builder.go b/builder/docker/builder.go
index f42e4b30d..824f1697f 100644
--- a/builder/docker/builder.go
+++ b/builder/docker/builder.go
@@ -10,33 +10,16 @@ import (
 const BuilderId = "packer.docker"
 
 type Builder struct {
-	config Config
+	config *Config
 	runner multistep.Runner
 }
 
 func (b *Builder) Prepare(raws ...interface{}) ([]string, error) {
-	md, err := common.DecodeConfig(&b.config, raws...)
-	if err != nil {
-		return nil, err
-	}
-
-	b.config.tpl, err = packer.NewConfigTemplate()
-	if err != nil {
-		return nil, err
-	}
-
-	// Accumulate any errors
-	errs := common.CheckUnusedConfig(md)
-	warnings := make([]string, 0)
-
-	// Validate the configuration
-	cwarns, cerrs := b.config.Prepare()
-	errs = packer.MultiErrorAppend(errs, cerrs...)
-	warnings = append(warnings, cwarns...)
-
-	if errs != nil && len(errs.Errors) > 0 {
+	c, warnings, errs := NewConfig(raws...)
+	if errs != nil {
 		return warnings, errs
 	}
+	b.config = c
 
 	return warnings, nil
 }
@@ -52,7 +35,7 @@ func (b *Builder) Run(ui packer.Ui, hook packer.Hook, cache packer.Cache) (packe
 
 	// Setup the state bag and initial state for the steps
 	state := new(multistep.BasicStateBag)
-	state.Put("config", &b.config)
+	state.Put("config", b.config)
 	state.Put("hook", hook)
 	state.Put("ui", ui)
 
diff --git a/builder/docker/config.go b/builder/docker/config.go
index 09ef02a06..e62930fd2 100644
--- a/builder/docker/config.go
+++ b/builder/docker/config.go
@@ -15,8 +15,19 @@ type Config struct {
 	tpl *packer.ConfigTemplate
 }
 
-func (c *Config) Prepare() ([]string, []error) {
-	errs := make([]error, 0)
+func NewConfig(raws ...interface{}) (*Config, []string, error) {
+	c := new(Config)
+	md, err := common.DecodeConfig(c, raws...)
+	if err != nil {
+		return nil, nil, err
+	}
+
+	c.tpl, err = packer.NewConfigTemplate()
+	if err != nil {
+		return nil, nil, err
+	}
+
+	errs := common.CheckUnusedConfig(md)
 
 	templates := map[string]*string{
 		"export_path": &c.ExportPath,
@@ -27,18 +38,24 @@ func (c *Config) Prepare() ([]string, []error) {
 		var err error
 		*ptr, err = c.tpl.Process(*ptr, nil)
 		if err != nil {
-			errs = append(
+			errs = packer.MultiErrorAppend(
 				errs, fmt.Errorf("Error processing %s: %s", n, err))
 		}
 	}
 
 	if c.ExportPath == "" {
-		errs = append(errs, fmt.Errorf("export_path must be specified"))
+		errs = packer.MultiErrorAppend(errs,
+			fmt.Errorf("export_path must be specified"))
 	}
 
 	if c.Image == "" {
-		errs = append(errs, fmt.Errorf("image must be specified"))
+		errs = packer.MultiErrorAppend(errs,
+			fmt.Errorf("image must be specified"))
 	}
 
-	return nil, errs
+	if errs != nil && len(errs.Errors) > 0 {
+		return nil, nil, errs
+	}
+
+	return c, nil, nil
 }
diff --git a/builder/docker/config_test.go b/builder/docker/config_test.go
index 8d083bf70..d26c21973 100644
--- a/builder/docker/config_test.go
+++ b/builder/docker/config_test.go
@@ -1,67 +1,72 @@
 package docker
 
 import (
-	"github.com/mitchellh/packer/packer"
 	"testing"
 )
 
-func testConfigStruct(t *testing.T) *Config {
-	tpl, err := packer.NewConfigTemplate()
-	if err != nil {
-		t.Fatalf("err: %s", err)
-	}
-
-	return &Config{
-		ExportPath: "foo",
-		Image:      "bar",
-		tpl:        tpl,
+func testConfig() map[string]interface{} {
+	return map[string]interface{}{
+		"export_path": "foo",
+		"image":       "bar",
 	}
 }
 
+func testConfigStruct(t *testing.T) *Config {
+	c, warns, errs := NewConfig(testConfig())
+	if len(warns) > 0 {
+		t.Fatalf("bad: %#v", len(warns))
+	}
+	if errs != nil {
+		t.Fatalf("bad: %#v", errs)
+	}
+
+	return c
+}
+
 func TestConfigPrepare_exportPath(t *testing.T) {
-	c := testConfigStruct(t)
+	raw := testConfig()
 
 	// No export path
-	c.ExportPath = ""
-	warns, errs := c.Prepare()
+	delete(raw, "export_path")
+	_, warns, errs := NewConfig(raw)
 	if len(warns) > 0 {
 		t.Fatalf("bad: %#v", warns)
 	}
-	if len(errs) <= 0 {
-		t.Fatalf("bad: %#v", errs)
+	if errs == nil {
+		t.Fatal("should error")
 	}
 
 	// Good export path
-	c.ExportPath = "path"
-	warns, errs = c.Prepare()
+	raw["export_path"] = "good"
+	_, warns, errs = NewConfig(raw)
 	if len(warns) > 0 {
 		t.Fatalf("bad: %#v", warns)
 	}
-	if len(errs) > 0 {
-		t.Fatalf("bad: %#v", errs)
+	if errs != nil {
+		t.Fatalf("bad: %s", errs)
 	}
 }
 
 func TestConfigPrepare_image(t *testing.T) {
-	c := testConfigStruct(t)
+	raw := testConfig()
 
 	// No image
-	c.Image = ""
-	warns, errs := c.Prepare()
+	delete(raw, "image")
+	_, warns, errs := NewConfig(raw)
 	if len(warns) > 0 {
 		t.Fatalf("bad: %#v", warns)
 	}
-	if len(errs) <= 0 {
-		t.Fatalf("bad: %#v", errs)
+	if errs == nil {
+		t.Fatal("should error")
 	}
 
 	// Good image
-	c.Image = "path"
-	warns, errs = c.Prepare()
+	raw["image"] = "path"
+	_, warns, errs = NewConfig(raw)
 	if len(warns) > 0 {
 		t.Fatalf("bad: %#v", warns)
 	}
-	if len(errs) > 0 {
-		t.Fatalf("bad: %#v", errs)
+	if errs != nil {
+		t.Fatalf("bad: %s", errs)
 	}
 }