diff --git a/core/component.go b/core/component.go index 47c29ce..9bad8ac 100644 --- a/core/component.go +++ b/core/component.go @@ -63,11 +63,16 @@ func UnmarshalFile(path string, unmarshalFunc unmarshalFunction, output interfac // UnmarshalComponent finds and unmarshal the component. 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() diff --git a/core/componentConfig.go b/core/componentConfig.go index 2cabca0..c87787f 100644 --- a/core/componentConfig.go +++ b/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 } diff --git a/core/componentConfig_test.go b/core/componentConfig_test.go index 4fe7b50..ea63776 100644 --- a/core/componentConfig_test.go +++ b/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") diff --git a/core/component_test.go b/core/component_test.go index a9a141d..fd3a328 100644 --- a/core/component_test.go +++ b/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", diff --git a/testdata/badjsoncomponent/component.json b/testdata/badjsoncomponent/component.json new file mode 100644 index 0000000..bf6bb10 --- /dev/null +++ b/testdata/badjsoncomponent/component.json @@ -0,0 +1,8 @@ + "name": "microservices-workload", + "subcomponents": [ + { + "name": "infra", + "source": "./infra" + } + ] +} diff --git a/testdata/badjsonconfig/config/common.json b/testdata/badjsonconfig/config/common.json new file mode 100644 index 0000000..a30e5bc --- /dev/null +++ b/testdata/badjsonconfig/config/common.json @@ -0,0 +1 @@ +THIS IS NOT VALID JSON diff --git a/testdata/badyamlcomponent/component.yaml b/testdata/badyamlcomponent/component.yaml new file mode 100644 index 0000000..66a1e82 --- /dev/null +++ b/testdata/badyamlcomponent/component.yaml @@ -0,0 +1,2 @@ +microservices-workload +subcomponents: diff --git a/testdata/badyamlconfig/config/common.yaml b/testdata/badyamlconfig/config/common.yaml new file mode 100644 index 0000000..70da573 --- /dev/null +++ b/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