Differentiate between failures to load config file not existing vs. p…

…arse errors (#242)

core/component.go

@@ -63,11 +63,16 @@ func UnmarshalFile(path string, unmarshalFunc unmarshalFunction, output interfac
 
 // UnmarshalComponent finds and unmarshal the component.<format> of a component using the
 // provided `unmarshalFunc` function.
-func (c *Component) UnmarshalComponent(marshaledType string, unmarshalFunc unmarshalFunction, component *Component) error {
-	componentFilename := fmt.Sprintf("component.%s", marshaledType)
+func (c *Component) UnmarshalComponent(serializationType string, unmarshalFunc unmarshalFunction, component *Component) error {
+	log.Debugf("Attempting to unmarshal %s for component '%s'", serializationType, c.Name)
+
+	componentFilename := fmt.Sprintf("component.%s", serializationType)
 	componentPath := path.Join(c.PhysicalPath, componentFilename)
 
-	return UnmarshalFile(componentPath, unmarshalFunc, component)
+	err := UnmarshalFile(componentPath, unmarshalFunc, component)
+	component.Serialization = serializationType
+
+	return err
 }
 
 func (c *Component) applyDefaultsAndMigrations() {
@@ -85,18 +90,24 @@ func (c *Component) applyDefaultsAndMigrations() {
 	}
 }
 
+// LoadComponent loads a component definition in either YAML or JSON formats.
 func (c *Component) LoadComponent() (loadedComponent Component, err error) {
-	log.Debugf("Attempting to unmarshal yaml for component '%s'", c.Name)
-	if err = c.UnmarshalComponent("yaml", yaml.Unmarshal, &loadedComponent); err != nil {
-		log.Debugf("Failed to unmarshal yaml for component '%s'; attempting json", c.Name)
+
+	// If success or loading or parsing the yaml component failed for reasons other than it didn't exist, return.
+	if err = c.UnmarshalComponent("yaml", yaml.Unmarshal, &loadedComponent); err != nil && !os.IsNotExist(err) {
+		return loadedComponent, err
+	}
+
+	// If YAML component definition did not exist, try JSON.
+	if err != nil {
 		if err = c.UnmarshalComponent("json", json.Unmarshal, &loadedComponent); err != nil {
-			log.Debugf("Failed to unmarshal json for component '%s'", c.Name)
+			if !os.IsNotExist(err) {
+				return loadedComponent, err
+			}
+
 			errorMessage := fmt.Sprintf("Error loading component in path %s", c.PhysicalPath)
 			return loadedComponent, errors.New(errorMessage)
 		}
-		loadedComponent.Serialization = "json"
-	} else {
-		loadedComponent.Serialization = "yaml"
 	}
 
 	loadedComponent.applyDefaultsAndMigrations()

core/componentConfig.go

@@ -64,18 +64,18 @@ func (cc *ComponentConfig) MergeConfigFile(path string, environment string) (err
 
 // Load loads the config for the specified environment.
 func (cc *ComponentConfig) Load(environment string) (err error) {
-	err = cc.UnmarshalYAMLConfig(environment)
-
-	// fall back to looking for JSON if loading YAML fails.
-	if err != nil {
-		err = cc.UnmarshalJSONConfig(environment)
+	// If success or loading or parsing the file failed for reasons other than it didn't exist, return.
+	if err = cc.UnmarshalYAMLConfig(environment); err == nil || !os.IsNotExist(err) {
+		return err
+	}
 
-		if err != nil {
-			// couldn't find any config files, so default back to yaml serialization
-			cc.Serialization = "yaml"
-		}
+	// If success or loading or parsing the file failed for reasons other than it didn't exist, return.
+	if err = cc.UnmarshalJSONConfig(environment); err == nil || !os.IsNotExist(err) {
+		return err
 	}
 
+	// Otherwise, no config files were found, so default to yaml serialization and return.
+	cc.Serialization = "yaml"
 	return nil
 }
 

core/componentConfig_test.go

@@ -20,6 +20,24 @@ func TestLoad(t *testing.T) {
 	assert.Equal(t, "myapp", config.Namespace)
 }
 
+func TestFailedYAMLLoad(t *testing.T) {
+	config := ComponentConfig{
+		Path: "../testdata/badyamlconfig",
+	}
+
+	err := config.Load("common")
+	assert.NotNil(t, err)
+}
+
+func TestFailedJSONLoad(t *testing.T) {
+	config := ComponentConfig{
+		Path: "../testdata/badjsonconfig",
+	}
+
+	err := config.Load("common")
+	assert.NotNil(t, err)
+}
+
 func TestMerge(t *testing.T) {
 	currentConfig := NewComponentConfig("../testdata/merge")
 	err := currentConfig.Load("current")

core/component_test.go

@@ -42,6 +42,26 @@ func TestLoadComponent(t *testing.T) {
 	assert.Equal(t, component.Subcomponents[0].Method, "git")
 }
 
+func TestLoadBadYAMLComponent(t *testing.T) {
+	component := Component{
+		PhysicalPath: "../testdata/badyamldefinition",
+		LogicalPath:  "",
+	}
+
+	component, err := component.LoadComponent()
+	assert.NotNil(t, err)
+}
+
+func TestLoadBadJSONComponent(t *testing.T) {
+	component := Component{
+		PhysicalPath: "../testdata/badjsondefinition",
+		LogicalPath:  "",
+	}
+
+	component, err := component.LoadComponent()
+	assert.NotNil(t, err)
+}
+
 func TestLoadConfig(t *testing.T) {
 	component := Component{
 		PhysicalPath: "../testdata/generate/infra",

testdata/badjsoncomponent/component.json

@@ -0,0 +1,8 @@
+    "name": "microservices-workload",
+    "subcomponents": [
+        {
+            "name": "infra",
+            "source": "./infra"
+        }
+    ]
+}

testdata/badjsonconfig/config/common.json

@@ -0,0 +1 @@
+THIS IS NOT VALID JSON

testdata/badyamlcomponent/component.yaml

@@ -0,0 +1,2 @@
+microservices-workload
+subcomponents: 

testdata/badyamlconfig/config/common.yaml

@@ -0,0 +1,17 @@
+subcomponents:
+  anotherservice:
+    config:
+      image:
+        tag: "744"
+      replicaCount: 1
+  screenprofiler:
+    config:
+      envVar:
+        important: true
+        fabshouldfail: true
+        is_this_a_bug: definitely
+        culprit:"this is where it all goes wrong due to the lack of space between key and value"
+      image:
+        repository: somewheregood.azurecr.io/screenprofiler
+        tag: "73"
+      replicaCount: 1