Merge pull request #26 from stefanotorresi/config-refactoring

refactor global config into a dedicated internal package

Author: stefanotorresi

internal/config/config.go

@@ -0,0 +1,64 @@
+package config
+
+import (
+	"net/url"
+	"regexp"
+
+	"github.com/pkg/errors"
+	flag "github.com/spf13/pflag"
+	"github.com/spf13/viper"
+)
+
+func New() (*viper.Viper, error) {
+	config := viper.New()
+
+	err := config.BindPFlags(flag.CommandLine)
+	if err != nil {
+		return nil, errors.Wrap(err, "could not bind config to CLI flags")
+	}
+
+	// try to get the "config" value from the bound "config" CLI flag
+	path := config.GetString("config")
+	if path != "" {
+		// try to manually load the configuration from the given path
+		err = loadConfigurationFromFile(config, path)
+	} else {
+		// otherwise try viper's auto-discovery
+		err = loadConfigurationAutomatically(config)
+	}
+
+	if err != nil {
+		return nil, errors.Wrap(err, "could not load configuration file")
+	}
+
+	setLogLevel(config.GetString("log-level"))
+
+	err = validateSapControlUrl(config)
+	if err != nil {
+		return nil, errors.Wrap(err, "invalid configuration value")
+	}
+
+	sanitizeSapControlUrl(config)
+
+	return config, nil
+}
+
+// returns an error in case the sap-control-url config value cannot be parsed as URL
+func validateSapControlUrl(config *viper.Viper) error {
+	sapControlUrl := config.GetString("sap-control-url")
+	if _, err := url.ParseRequestURI(sapControlUrl); err != nil {
+		return errors.Wrap(err, "invalid config value for sap-control-url")
+	}
+	return nil
+}
+
+// automatically adds an http:// prefix in case it's missing from the value, to avoid the downstream consumer
+// throw errors due to missing schema URL component
+func sanitizeSapControlUrl(config *viper.Viper) {
+	sapControlUrl := config.GetString("sap-control-url")
+	hasScheme, _ := regexp.MatchString("^https?://", sapControlUrl)
+	if !hasScheme {
+		sapControlUrl = "http://" + sapControlUrl
+		config.Set("sap-control-url", sapControlUrl)
+	}
+}

internal/config/loader.go

@@ -0,0 +1,51 @@
+package config
+
+import (
+	"os"
+
+	"github.com/pkg/errors"
+	log "github.com/sirupsen/logrus"
+	"github.com/spf13/viper"
+)
+
+func loadConfigurationAutomatically(config *viper.Viper) error {
+	config.SetConfigName("sap_host_exporter")
+	config.AddConfigPath("./")
+	config.AddConfigPath("$HOME/.config/")
+	config.AddConfigPath("/etc/")
+	config.AddConfigPath("/usr/etc/")
+
+	err := config.ReadInConfig()
+	if err == nil {
+		log.Info("Using config file: ", config.ConfigFileUsed())
+		return nil
+	}
+
+	if _, ok := err.(viper.ConfigFileNotFoundError); ok {
+		log.Infof("Could not discover configuration file: %s", err)
+		log.Info("Default configuration values will be used")
+		return nil
+	}
+
+	return errors.Wrap(err, "could not load automatically discovered config file")
+}
+
+// loads configuration from an explicit file path
+func loadConfigurationFromFile(config *viper.Viper, path string) error {
+	// we hard-code the config type to yaml, otherwise ReadConfig will not load the values
+	// see https://github.com/spf13/viper/issues/316
+	config.SetConfigType("yaml")
+
+	file, err := os.Open(path)
+	if err != nil {
+		return errors.Wrap(err, "could not open file")
+	}
+	defer file.Close()
+
+	err = config.ReadConfig(file)
+	if err != nil {
+		return errors.Wrap(err, "could not read file")
+	}
+
+	return nil
+}

internal/log.go renamed to internal/config/log.go

@@ -1,8 +1,8 @@
-package internal
+package config
 
 import log "github.com/sirupsen/logrus"
 
-func SetLogLevel(level string) {
+func setLogLevel(level string) {
 	switch level {
 	case "error":
 		log.SetLevel(log.ErrorLevel)

main.go

@@ -1,50 +1,42 @@
 package main
 
 import (
-	"bytes"
 	"fmt"
-	"io/ioutil"
 	"net/http"
 
 	"github.com/hooklift/gowsdl/soap"
 	"github.com/prometheus/client_golang/prometheus"
 	"github.com/prometheus/client_golang/prometheus/promhttp"
 	log "github.com/sirupsen/logrus"
 	flag "github.com/spf13/pflag"
-	config "github.com/spf13/viper"
 
 	"github.com/SUSE/sap_host_exporter/collector/alert"
 	"github.com/SUSE/sap_host_exporter/collector/dispatcher"
 	"github.com/SUSE/sap_host_exporter/collector/enqueue_server"
 	"github.com/SUSE/sap_host_exporter/collector/start_service"
 	"github.com/SUSE/sap_host_exporter/internal"
+	"github.com/SUSE/sap_host_exporter/internal/config"
 	"github.com/SUSE/sap_host_exporter/internal/sapcontrol"
 )
 
 func init() {
-	config.SetConfigName("sap_host_exporter")
-	config.AddConfigPath("./")
-	config.AddConfigPath("$HOME/.config/")
-	config.AddConfigPath("/etc/")
-	config.AddConfigPath("/usr/etc/")
-
 	flag.String("port", "9680", "The port number to listen on for HTTP requests")
 	flag.String("address", "0.0.0.0", "The address to listen on for HTTP requests")
 	flag.String("log-level", "info", "The minimum logging level; levels are, in ascending order: debug, info, warn, error")
-	flag.String("sap-control-url", "", "The URL of the SAPControl SOAP web service, e.g. http://$HOST:$PORT")
-	flag.String("config", "", "The path where a custom configuration.yaml file is located. NOTE: the conf must be yaml")
-
-	err := config.BindPFlags(flag.CommandLine)
-	if err != nil {
-		log.Errorf("Could not bind config to CLI flags: %v", err)
-	}
+	flag.String("sap-control-url", "localhost:50013", "The URL of the SAPControl SOAP web service, e.g. $HOST:$PORT")
+	flag.StringP("config", "c", "", "The path to a custom configuration file. NOTE: it must be in yaml format.")
 }
 
 func main() {
-	initConfig()
-
 	var err error
 
+	flag.Parse()
+
+	config, err := config.New()
+	if err != nil {
+		log.Fatalf("Could not initialize config: %s", err)
+	}
+
 	client := soap.NewClient(
 		config.GetString("sap-control-url"),
 		soap.WithBasicAuth(
@@ -99,49 +91,3 @@ func main() {
 	log.Infof("Serving metrics on %s", fullListenAddress)
 	log.Fatal(http.ListenAndServe(fullListenAddress, nil))
 }
-
-func initConfig() {
-
-	flag.Parse()
-
-	// read configuration from custom path or defaults
-	readExporterConf()
-
-	internal.SetLogLevel(config.GetString("log-level"))
-
-	if config.GetString("sap-control-url") == "" {
-		log.Fatal("sap-control-url cannot be empty, please use the --sap-control-url flag or set a value in the config")
-	}
-}
-
-func readExporterConf() {
-
-	// read first the configuration from custom file. If not provided, read default.
-	confFile := config.GetString("config")
-	if confFile != "" {
-		// hardcode for custom config file type to yaml
-		// this workaround is needed otherwise viper return empty conf
-		// see issue https://github.com/spf13/viper/issues/316
-		config.SetConfigType("yaml")
-		confData, err := ioutil.ReadFile(confFile)
-		if err != nil {
-			log.Fatal("Could not read configuration file for exporter: ", err)
-		}
-		config.ReadConfig(bytes.NewBuffer(confData))
-		if err != nil {
-			log.Fatal("Could not parse configuration:", err)
-		}
-		log.Info("Using custom configuration file provided by flag")
-		return
-	}
-
-	// if no custom file given, read configuration from default paths
-	err := config.ReadInConfig()
-	if err != nil {
-		log.Warn(err)
-		log.Info("Default config values will be used")
-	} else {
-		log.Info("Using config file: ", config.ConfigFileUsed())
-	}
-
-}