Parse shuttle.yaml and plan.yaml strictly (#64)

Currently we silently ignore unknown fields in shuttle.yaml and plan.yaml. This
can lead to odd behaviour and hard to find bugs from typos.

This change adds strict parsing of shuttle.yaml and plan.yaml failing commands
if any unexpected fields are found.

pkg/config/shuttleconfig.go

@@ -2,7 +2,6 @@ package config
 
 import (
 	"fmt"
-	"io/ioutil"
 	"os"
 	"path"
 
@@ -16,7 +15,7 @@ type DynamicYaml = map[string]interface{}
 
 // ShuttleConfig describes the actual config for each project
 type ShuttleConfig struct {
-	Plan      string                       `yaml:"not_plan"`
+	Plan      string                       `yaml:"-"`
 	PlanRaw   interface{}                  `yaml:"plan"`
 	Variables DynamicYaml                  `yaml:"vars"`
 	Scripts   map[string]ShuttlePlanScript `yaml:"scripts"`
@@ -36,7 +35,7 @@ type ShuttleProjectContext struct {
 
 // Setup the ShuttleProjectContext for a specific path
 func (c *ShuttleProjectContext) Setup(projectPath string, uii ui.UI, clean bool, skipGitPlanPulling bool, planArgument string) (*ShuttleProjectContext, error) {
-	_, err := c.Config.getConf(uii, projectPath)
+	_, err := c.Config.getConf(projectPath)
 	if err != nil {
 		return nil, err
 	}
@@ -61,7 +60,10 @@ func (c *ShuttleProjectContext) Setup(projectPath string, uii ui.UI, clean bool,
 	if err != nil {
 		return nil, err
 	}
-	c.Plan.Load(c.LocalPlanPath)
+	_, err = c.Plan.Load(c.LocalPlanPath)
+	if err != nil {
+		return nil, err
+	}
 	c.Scripts = make(map[string]ShuttlePlanScript)
 	for scriptName, script := range c.Plan.Scripts {
 		c.Scripts[scriptName] = script
@@ -73,18 +75,24 @@ func (c *ShuttleProjectContext) Setup(projectPath string, uii ui.UI, clean bool,
 }
 
 // getConf loads the ShuttleConfig from yaml file in the project path
-func (c *ShuttleConfig) getConf(uii ui.UI, projectPath string) (*ShuttleConfig, error) {
-	var configPath = path.Join(projectPath, "shuttle.yaml")
+func (c *ShuttleConfig) getConf(projectPath string) (*ShuttleConfig, error) {
+	if projectPath == "" {
+		return c, nil
+	}
 
-	//log.Printf("configpath: %s", configPath)
+	configPath := path.Join(projectPath, "shuttle.yaml")
 
-	yamlFile, err := ioutil.ReadFile(configPath)
+	file, err := os.Open(configPath)
 	if err != nil {
-		return nil, errors.NewExitCode(2, "Failed to load shuttle configuration: %s\n\nMake sure you are in a project using shuttle and that a 'shuttle.yaml' file is available.", err)
+		return c, errors.NewExitCode(2, "Failed to load shuttle configuration: %s\n\nMake sure you are in a project using shuttle and that a 'shuttle.yaml' file is available.", err)
 	}
-	err = yaml.Unmarshal(yamlFile, c)
+	defer file.Close()
+
+	decoder := yaml.NewDecoder(file)
+	decoder.SetStrict(true)
+	err = decoder.Decode(c)
 	if err != nil {
-		return nil, errors.NewExitCode(2, "Failed to parse shuttle configuration: %s\n\nMake sure your 'shuttle.yaml' is valid.", err)
+		return c, errors.NewExitCode(2, "Failed to parse shuttle configuration: %s\n\nMake sure your 'shuttle.yaml' is valid.", err)
 	}
 
 	switch c.PlanRaw {

pkg/config/shuttleconfig_test.go

@@ -0,0 +1,68 @@
+package config
+
+import (
+	"errors"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestShuttleConfig_getConf(t *testing.T) {
+	tt := []struct {
+		name   string
+		input  string
+		err    error
+		config ShuttleConfig
+	}{
+		{
+			name:  "empty path",
+			input: "",
+		},
+		{
+			name:  "unknown field",
+			input: "testdata/unknown_field",
+			err:   errors.New("exit code 2 - Failed to parse shuttle configuration: yaml: unmarshal errors:\n  line 1: field nothing not found in type config.ShuttleConfig\n\nMake sure your 'shuttle.yaml' is valid."),
+		},
+		{
+			name:  "unknown file",
+			input: "testdata/unknown_file",
+			err:   errors.New("exit code 2 - Failed to load shuttle configuration: open testdata/unknown_file/shuttle.yaml: no such file or directory\n\nMake sure you are in a project using shuttle and that a 'shuttle.yaml' file is available."),
+		},
+		{
+			name:  "valid",
+			input: "testdata/valid",
+			err:   nil,
+			config: ShuttleConfig{
+				Plan:    ".",
+				PlanRaw: ".",
+				Variables: map[string]interface{}{
+					"squad": "nasa",
+				},
+				Scripts: map[string]ShuttlePlanScript{
+					"shout": {
+						Description: "Shout hello",
+						Actions: []ShuttleAction{
+							{
+								Shell: `echo "HELLO WORLD"`,
+							},
+						},
+					},
+				},
+			},
+		},
+	}
+	for _, tc := range tt {
+		t.Run(tc.name, func(t *testing.T) {
+			c := &ShuttleConfig{}
+			var err error
+			c, err = c.getConf(tc.input)
+
+			if tc.err != nil {
+				assert.EqualError(t, err, tc.err.Error(), "error not as expected")
+			} else {
+				assert.NoError(t, err, "unexpected error")
+			}
+			assert.Equal(t, tc.config, *c, "config not as expected")
+		})
+	}
+}

pkg/config/shuttleplan.go

@@ -2,8 +2,7 @@ package config
 
 import (
 	"fmt"
-	"io/ioutil"
-	"log"
+	"os"
 	"path"
 	"path/filepath"
 	"regexp"
@@ -61,22 +60,27 @@ type ShuttlePlan struct {
 }
 
 // Load loads a plan from project path and shuttle config
-func (p *ShuttlePlanConfiguration) Load(planPath string) *ShuttlePlanConfiguration {
+func (p *ShuttlePlanConfiguration) Load(planPath string) (*ShuttlePlanConfiguration, error) {
 	if planPath == "" {
-		return p
+		return p, nil
 	}
-	var configPath = path.Join(planPath, "plan.yaml")
-	//log.Printf("configpath: %s", configPath)
-	yamlFile, err := ioutil.ReadFile(configPath)
+
+	configPath := path.Join(planPath, "plan.yaml")
+
+	file, err := os.Open(configPath)
 	if err != nil {
-		log.Printf("yamlFile.Get err   #%v ", err)
+		return p, errors.NewExitCode(2, "Failed to open plan configuration: %s\n\nMake sure you are in a project using shuttle and that a 'shuttle.yaml' file is available.", err)
 	}
-	err = yaml.Unmarshal(yamlFile, p)
+	defer file.Close()
+
+	decoder := yaml.NewDecoder(file)
+	decoder.SetStrict(true)
+	err = decoder.Decode(p)
 	if err != nil {
-		log.Fatalf("Unmarshal: %v", err)
+		return p, errors.NewExitCode(1, "Failed to load plan configuration: %s\n\nThis is likely an issue with the referenced plan. Please, contact the plan maintainers.", err)
 	}
 
-	return p
+	return p, nil
 }
 
 // FetchPlan so it exists locally and return path to that plan

pkg/config/shuttleplan_test.go

@@ -0,0 +1,63 @@
+package config
+
+import (
+	"errors"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestShuttlePlanConfiguration_Load(t *testing.T) {
+	tt := []struct {
+		name   string
+		input  string
+		err    error
+		config ShuttlePlanConfiguration
+	}{
+		{
+			name:  "empty path",
+			input: "",
+		},
+		{
+			name:  "unknown field",
+			input: "testdata/unknown_field",
+			err:   errors.New("exit code 1 - Failed to load plan configuration: yaml: unmarshal errors:\n  line 1: field unknown not found in type config.ShuttlePlanConfiguration\n\nThis is likely an issue with the referenced plan. Please, contact the plan maintainers."),
+		},
+		{
+			name:  "unknown file",
+			input: "testdata/unknown_file",
+			err:   errors.New("exit code 2 - Failed to open plan configuration: open testdata/unknown_file/plan.yaml: no such file or directory\n\nMake sure you are in a project using shuttle and that a 'shuttle.yaml' file is available."),
+		},
+		{
+			name:  "valid",
+			input: "testdata/valid",
+			err:   nil,
+			config: ShuttlePlanConfiguration{
+				Scripts: map[string]ShuttlePlanScript{
+					"hello": {
+						Description: "Say hello",
+						Actions: []ShuttleAction{
+							{
+								Shell: `echo "Hello world"`,
+							},
+						},
+					},
+				},
+			},
+		},
+	}
+	for _, tc := range tt {
+		t.Run(tc.name, func(t *testing.T) {
+			c := &ShuttlePlanConfiguration{}
+			var err error
+			c, err = c.Load(tc.input)
+
+			if tc.err != nil {
+				assert.EqualError(t, err, tc.err.Error(), "error not as expected")
+			} else {
+				assert.NoError(t, err, "unexpected error")
+			}
+			assert.Equal(t, tc.config, *c, "config not as expected")
+		})
+	}
+}

pkg/config/testdata/unknown_field/plan.yaml

@@ -0,0 +1 @@
+unknown: field

pkg/config/testdata/unknown_field/shuttle.yaml

@@ -0,0 +1 @@
+nothing: important

pkg/config/testdata/valid/plan.yaml

@@ -0,0 +1,5 @@
+scripts:
+  hello:
+    description: Say hello
+    actions:
+    - shell: echo "Hello world"

pkg/config/testdata/valid/shuttle.yaml

@@ -0,0 +1,8 @@
+plan: .
+vars:
+  squad: nasa
+scripts:
+  shout:
+    description: Shout hello
+    actions:
+    - shell: echo "HELLO WORLD"