Refactory config code to add validation

Author: Diogo Nicoleti

.gitignore

@@ -1,4 +1,5 @@
 vendor
+coverage.out
 test_config.yml
 postgresql_exporter
 dist

config/config.go

@@ -1,9 +1,10 @@
 package config
 
 import (
+	"errors"
+	"fmt"
 	"io/ioutil"
 
-	"github.com/apex/log"
 	"gopkg.in/yaml.v2"
 )
 
@@ -16,25 +17,34 @@ type Config struct {
 	Databases []Database `yaml:"databases,omitempty"`
 }
 
-func Parse(path string) Config {
+func Parse(path string) (Config, error) {
 	var cfg Config
 	bts, err := ioutil.ReadFile(path)
 	if err != nil {
-		log.WithError(err).Fatalf("failed to read config file: %s", path)
+		return cfg, fmt.Errorf("failed to read config file '%s': %s", path, err)
 	}
 	if err := yaml.Unmarshal(bts, &cfg); err != nil {
-		log.WithError(err).Fatalf("failed to unmarshall config file: %s", path)
+		return cfg, fmt.Errorf("failed to unmarshall config file '%s': %s", path, err)
 	}
-	return validate(cfg)
+	if err := validate(cfg); err != nil {
+		return cfg, err
+	}
+	return cfg, nil
 }
 
-func validate(config Config) Config {
+func validate(config Config) error {
 	names := make(map[string]bool)
 	for _, conf := range config.Databases {
+		if conf.Name == "" {
+			return errors.New("failed to validate configuration. Database name cannot be empty")
+		}
+		if conf.URL == "" {
+			return fmt.Errorf("failed to validate configuration. URL for database '%s' cannot be empty", conf.Name)
+		}
 		if names[conf.Name] {
-			log.Fatalf("failed to validate configuration. A database named '%s' has already been declared'", conf.Name)
+			return fmt.Errorf("failed to validate configuration. A database named '%s' has already been declared", conf.Name)
 		}
 		names[conf.Name] = true
 	}
-	return config
+	return nil
 }

config/config_test.go

@@ -0,0 +1,51 @@
+package config
+
+import (
+	"testing"
+)
+
+func TestParseConfig(t *testing.T) {
+	_, err := Parse("testdata/valid-config.yml")
+	if err != nil {
+		t.Errorf("Error loading config: %v", err)
+	}
+}
+
+func TestParseBadConfigs(t *testing.T) {
+	tests := []struct {
+		ConfigFile    string
+		ExpectedError string
+	}{
+		{
+			ConfigFile:    "testdata/unknown-config.yml",
+			ExpectedError: "failed to read config file 'testdata/unknown-config.yml': open testdata/unknown-config.yml: no such file or directory",
+		},
+		{
+			ConfigFile:    "testdata/invalid-duplicated-config.yml",
+			ExpectedError: "failed to validate configuration. A database named 'dba' has already been declared",
+		},
+		{
+			ConfigFile:    "testdata/invalid-empty-name-config.yml",
+			ExpectedError: "failed to validate configuration. Database name cannot be empty",
+		},
+		{
+			ConfigFile:    "testdata/invalid-empty-url-config.yml",
+			ExpectedError: "failed to validate configuration. URL for database 'dba' cannot be empty",
+		},
+		{
+			ConfigFile:    "testdata/invalid-yaml-config.yml",
+			ExpectedError: "failed to unmarshall config file 'testdata/invalid-yaml-config.yml': yaml: unmarshal errors:\n  line 1: cannot unmarshal !!seq into config.Config",
+		},
+	}
+
+	for _, test := range tests {
+		_, err := Parse(test.ConfigFile)
+		if err == nil {
+			t.Errorf("In case %v:\nExpected: %v\nGot: nil", test.ConfigFile, test.ExpectedError)
+			continue
+		}
+		if err.Error() != test.ExpectedError {
+			t.Errorf("In case %v:\nExpected: %v\nGot: %v", test.ConfigFile, test.ExpectedError, err.Error())
+		}
+	}
+}

config/testdata/invalid-duplicated-config.yml

@@ -0,0 +1,5 @@
+databases:
+- name: dba
+  url: postgres://localhost:5432/dba?sslmode=disable
+- name: dba
+  url: postgres://user:[email protected]:5432/dbb

config/testdata/invalid-empty-name-config.yml

@@ -0,0 +1,4 @@
+databases:
+- name: dba
+  url: postgres://localhost:5432/dba?sslmode=disable
+- url: postgres://user:[email protected]:5432/dbb

config/testdata/invalid-empty-url-config.yml

@@ -0,0 +1,4 @@
+databases:
+- name: dba
+- name: dbb
+  url: postgres://user:[email protected]:5432/dbb

config/testdata/invalid-yaml-config.yml

@@ -0,0 +1,2 @@
+- name: dba
+  url: postgres://localhost:5432/dba?sslmode=disable

config/testdata/valid-config.yml

@@ -0,0 +1,5 @@
+databases:
+- name: dba
+  url: postgres://localhost:5432/dba?sslmode=disable
+- name: dbb
+  url: postgres://user:[email protected]:5432/dbb

main.go

@@ -49,7 +49,12 @@ func main() {
 		`))
 	})
 	http.Handle("/metrics", promhttp.Handler())
-	var config = config.Parse(*configFile)
+
+	var config, err = config.Parse(*configFile)
+	if err != nil {
+		log.WithError(err).Fatalf("error to parse config file")
+	}
+
 	for _, con := range config.Databases {
 		var log = log.WithField("db", con.Name)
 		log.Info("started monitoring")