make the config direct from yaml and json (#206)

* make the config direct from yaml and json

refactor to keep a deprectated LoadFromFile

fix imports

* fix tests

* use the args loader

* make test for parsing the default configuration

* misleading error

* use the Duration in the test

* default more values

* use the util.Duration for timeout values

* import cleanups

Author: rybit

featureflag/config.go

@@ -1,17 +1,16 @@
 package featureflag
 
 import (
-	"time"
-
+	"github.com/netlify/netlify-commons/util"
 	ld "gopkg.in/launchdarkly/go-server-sdk.v4"
 )
 
 type Config struct {
 	Key            string        `json:"key" yaml:"key"`
-	RequestTimeout time.Duration `json:"request_timeout" yaml:"request_timeout" mapstructure:"request_timeout" split_words:"true" default:"5s"`
+	RequestTimeout util.Duration `json:"request_timeout" yaml:"request_timeout" mapstructure:"request_timeout" split_words:"true" default:"5s"`
 	Enabled        bool          `json:"enabled" yaml:"enabled" default:"false"`
 
-	updateProcessorFactory ld.UpdateProcessorFactory `json:"-"`
+	updateProcessorFactory ld.UpdateProcessorFactory
 
 	// Drop telemetry events (not needed in local-dev/CI environments)
 	DisableEvents bool `json:"disable_events" yaml:"disable_events" mapstructure:"disable_events" split_words:"true"`

featureflag/featureflag.go

@@ -50,7 +50,7 @@ func NewClient(cfg *Config, logger logrus.FieldLogger) (Client, error) {
 		config.SendEvents = false
 	}
 
-	inner, err := ld.MakeCustomClient(cfg.Key, config, cfg.RequestTimeout)
+	inner, err := ld.MakeCustomClient(cfg.Key, config, cfg.RequestTimeout.Duration)
 	if err != nil {
 		logger.WithError(err).Error("Unable to construct LD client")
 	}

featureflag/featureflag_test.go

@@ -5,6 +5,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/netlify/netlify-commons/util"
 	"github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -14,7 +15,7 @@ import (
 func TestOfflineClient(t *testing.T) {
 	cfg := Config{
 		Key:            "ABCD",
-		RequestTimeout: time.Second,
+		RequestTimeout: util.Duration{time.Second},
 		Enabled:        false,
 	}
 	client, err := NewClient(&cfg, nil)
@@ -48,7 +49,7 @@ func TestAllEnabledFlags(t *testing.T) {
 	fileSource := ldfiledata.NewFileDataSourceFactory(ldfiledata.FilePaths("./fixtures/flags.yml"))
 	cfg := Config{
 		Key:                    "ABCD",
-		RequestTimeout:         time.Second,
+		RequestTimeout:         util.Duration{time.Second},
 		Enabled:                true,
 		updateProcessorFactory: fileSource,
 	}
@@ -63,7 +64,7 @@ func TestAllEnabledFlags(t *testing.T) {
 func TestLogging(t *testing.T) {
 	cfg := Config{
 		Key:            "ABCD",
-		RequestTimeout: time.Second,
+		RequestTimeout: util.Duration{time.Second},
 		Enabled:        false,
 	}
 

nconf/args.go

@@ -19,26 +19,9 @@ type RootArgs struct {
 }
 
 func (args *RootArgs) Setup(config interface{}, serviceName, version string) (logrus.FieldLogger, error) {
-	// first load the logger and BugSnag config
-	rootConfig := &struct {
-		Log         *LoggingConfig
-		BugSnag     *BugSnagConfig
-		Metrics     metriks.Config
-		Tracing     tracing.Config
-		FeatureFlag featureflag.Config
-	}{}
-
-	loader := func(cfg interface{}) error {
-		return LoadFromEnv(args.Prefix, args.ConfigFile, cfg)
-	}
-	if !strings.HasSuffix(args.ConfigFile, ".env") {
-		loader = func(cfg interface{}) error {
-			return LoadFromFile(args.ConfigFile, cfg)
-		}
-	}
-
-	if err := loader(rootConfig); err != nil {
-		return nil, errors.Wrap(err, "Failed to load the logging configuration")
+	rootConfig, err := args.loadDefaultConfig()
+	if err != nil {
+		return nil, err
 	}
 
 	log, err := ConfigureLogging(rootConfig.Log)
@@ -71,14 +54,26 @@ func (args *RootArgs) Setup(config interface{}, serviceName, version string) (lo
 
 	if config != nil {
 		// second load the config for this project
-		if err := loader(config); err != nil {
+		if err := args.load(config); err != nil {
 			return log, errors.Wrap(err, "Failed to load the config object")
 		}
 		log.Debug("Loaded configuration")
 	}
 	return log, nil
 }
 
+func (args *RootArgs) load(cfg interface{}) error {
+	loader := func(cfg interface{}) error {
+		return LoadFromEnv(args.Prefix, args.ConfigFile, cfg)
+	}
+	if !strings.HasSuffix(args.ConfigFile, ".env") {
+		loader = func(cfg interface{}) error {
+			return LoadConfigFromFile(args.ConfigFile, cfg)
+		}
+	}
+	return loader(cfg)
+}
+
 func (args *RootArgs) MustSetup(config interface{}, serviceName, version string) logrus.FieldLogger {
 	logger, err := args.Setup(config, serviceName, version)
 	if err != nil {
@@ -92,6 +87,16 @@ func (args *RootArgs) MustSetup(config interface{}, serviceName, version string)
 	return logger
 }
 
+func (args *RootArgs) loadDefaultConfig() (*RootConfig, error) {
+	c := DefaultConfig()
+
+	if err := args.load(&c); err != nil {
+		return nil, errors.Wrap(err, "Failed to load the default configuration")
+	}
+
+	return &c, nil
+}
+
 func (args *RootArgs) AddFlags(cmd *cobra.Command) *cobra.Command {
 	cmd.Flags().AddFlag(args.ConfigFlag())
 	cmd.Flags().AddFlag(args.PrefixFlag())

nconf/args_test.go

@@ -1,15 +1,17 @@
 package nconf
 
 import (
+	"encoding/json"
 	"io/ioutil"
+	"os"
 	"testing"
-
-	"github.com/spf13/cobra"
+	"time"
 
 	"github.com/sirupsen/logrus"
+	"github.com/spf13/cobra"
 	"github.com/stretchr/testify/assert"
-
 	"github.com/stretchr/testify/require"
+	"gopkg.in/yaml.v3"
 )
 
 func TestArgsLoad(t *testing.T) {
@@ -67,3 +69,98 @@ func TestArgsAddToCmd(t *testing.T) {
 	require.NoError(t, cmd.Execute())
 	assert.Equal(t, 1, called)
 }
+
+func TestArgsLoadDefault(t *testing.T) {
+	configVals := map[string]interface{}{
+		"log": map[string]interface{}{
+			"level": "debug",
+			"fields": map[string]interface{}{
+				"something": 1,
+			},
+		},
+		"bugsnag": map[string]interface{}{
+			"api_key":         "secrets",
+			"project_package": "package",
+		},
+		"metrics": map[string]interface{}{
+			"enabled": true,
+			"port":    8125,
+			"tags": map[string]string{
+				"env": "prod",
+			},
+		},
+		"tracing": map[string]interface{}{
+			"enabled":      true,
+			"port":         "9125",
+			"enable_debug": true,
+		},
+		"featureflag": map[string]interface{}{
+			"key":             "magicalkey",
+			"request_timeout": "10s",
+			"enabled":         true,
+		},
+	}
+
+	scenes := []struct {
+		ext string
+		enc func(v interface{}) ([]byte, error)
+	}{
+		{"json", json.Marshal},
+		{"yaml", yaml.Marshal},
+	}
+	for _, s := range scenes {
+		t.Run(s.ext, func(t *testing.T) {
+			f, err := ioutil.TempFile("", "test-config-*."+s.ext)
+			require.NoError(t, err)
+			defer os.Remove(f.Name())
+
+			b, err := s.enc(&configVals)
+			require.NoError(t, err)
+			_, err = f.Write(b)
+			require.NoError(t, err)
+
+			args := RootArgs{
+				ConfigFile: f.Name(),
+			}
+			cfg, err := args.loadDefaultConfig()
+			require.NoError(t, err)
+
+			// logging
+			assert.Equal(t, "debug", cfg.Log.Level)
+			assert.Equal(t, true, cfg.Log.QuoteEmptyFields)
+			assert.Equal(t, "", cfg.Log.File)
+			assert.Equal(t, false, cfg.Log.DisableColors)
+			assert.Equal(t, "", cfg.Log.TSFormat)
+
+			assert.Len(t, cfg.Log.Fields, 1)
+			assert.EqualValues(t, 1, cfg.Log.Fields["something"])
+			assert.Equal(t, false, cfg.Log.UseNewLogger)
+
+			// bugsnag
+			assert.Equal(t, "", cfg.BugSnag.Environment)
+			assert.Equal(t, "secrets", cfg.BugSnag.APIKey)
+			assert.Equal(t, false, cfg.BugSnag.LogHook)
+			assert.Equal(t, "package", cfg.BugSnag.ProjectPackage)
+
+			// metrics
+			assert.Equal(t, true, cfg.Metrics.Enabled)
+			assert.Equal(t, "localhost", cfg.Metrics.Host)
+			assert.Equal(t, 8125, cfg.Metrics.Port)
+			assert.Equal(t, map[string]string{"env": "prod"}, cfg.Metrics.Tags)
+
+			// tracing
+			assert.Equal(t, true, cfg.Tracing.Enabled)
+			assert.Equal(t, "localhost", cfg.Tracing.Host)
+			assert.Equal(t, "9125", cfg.Tracing.Port)
+			assert.Empty(t, cfg.Tracing.Tags)
+			assert.Equal(t, true, cfg.Tracing.EnableDebug)
+
+			// featureflag
+			assert.Equal(t, "magicalkey", cfg.FeatureFlag.Key)
+			assert.Equal(t, 10*time.Second, cfg.FeatureFlag.RequestTimeout.Duration)
+			assert.Equal(t, true, cfg.FeatureFlag.Enabled)
+			assert.Equal(t, false, cfg.FeatureFlag.DisableEvents)
+			assert.Equal(t, "", cfg.FeatureFlag.RelayHost)
+		})
+	}
+}

nconf/bugsnag.go

@@ -8,28 +8,28 @@ import (
 
 type BugSnagConfig struct {
 	Environment    string
-	APIKey         string `envconfig:"api_key"`
-	LogHook        bool   `envconfig:"log_hook"`
-	ProjectPackage string `envconfig:"project_package"`
+	APIKey         string `envconfig:"api_key" json:"api_key" yaml:"api_key"`
+	LogHook        bool   `envconfig:"log_hook" json:"log_hook" yaml:"log_hook"`
+	ProjectPackage string `envconfig:"project_package" json:"project_package" yaml:"project_package"`
 }
 
 func SetupBugSnag(config *BugSnagConfig, version string) error {
 	if config == nil || config.APIKey == "" {
 		return nil
 	}
-	
+
 	projectPackages := make([]string, 0, 2)
 	projectPackages = append(projectPackages, "main")
 	if config.ProjectPackage != "" {
 		projectPackages = append(projectPackages, config.ProjectPackage)
 	}
 
 	bugsnag.Configure(bugsnag.Configuration{
-		APIKey:       config.APIKey,
-		ReleaseStage: config.Environment,
-		AppVersion:   version,
-		ProjectPackages: projectPackages, 
-		PanicHandler: func() {}, // this is to disable panic handling. The lib was forking and restarting the process (causing races)
+		APIKey:          config.APIKey,
+		ReleaseStage:    config.Environment,
+		AppVersion:      version,
+		ProjectPackages: projectPackages,
+		PanicHandler:    func() {}, // this is to disable panic handling. The lib was forking and restarting the process (causing races)
 	})
 
 	if config.LogHook {

nconf/configuration.go

@@ -1,15 +1,60 @@
 package nconf
 
 import (
+	"encoding/json"
+	"fmt"
+	"io/ioutil"
 	"os"
+	"path/filepath"
 	"strings"
 
 	"github.com/joho/godotenv"
 	"github.com/kelseyhightower/envconfig"
+	"github.com/netlify/netlify-commons/featureflag"
+	"github.com/netlify/netlify-commons/metriks"
+	"github.com/netlify/netlify-commons/tracing"
 	"github.com/pkg/errors"
 	"github.com/spf13/viper"
+	"gopkg.in/yaml.v3"
 )
 
+// ErrUnknownConfigFormat indicates the extension of the config file is not supported as a config source
+type ErrUnknownConfigFormat struct {
+	ext string
+}
+
+func (e *ErrUnknownConfigFormat) Error() string {
+	return fmt.Sprintf("Unknown config format: %s", e.ext)
+}
+
+type RootConfig struct {
+	Log         LoggingConfig
+	BugSnag     *BugSnagConfig
+	Metrics     metriks.Config
+	Tracing     tracing.Config
+	FeatureFlag featureflag.Config
+}
+
+func DefaultConfig() RootConfig {
+	return RootConfig{
+		Log: LoggingConfig{
+			QuoteEmptyFields: true,
+		},
+		Tracing: tracing.Config{
+			Host: "localhost",
+			Port: "8126",
+		},
+		Metrics: metriks.Config{
+			Host: "localhost",
+			Port: 8125,
+		},
+	}
+}
+
+/*
+ Deprecated: This method relies on parsing the json/yaml to a map, then running it through mapstructure.
+ This required that both tags exist (annoying!). And so there is now LoadConfigFromFile.
+*/
 // LoadFromFile will load the configuration from the specified file based on the file type
 // There is only support for .json and .yml now
 func LoadFromFile(configFile string, input interface{}) error {
@@ -37,6 +82,30 @@ func LoadFromFile(configFile string, input interface{}) error {
 	return viper.Unmarshal(input)
 }
 
+// LoadConfigFromFile will load the configuration from the specified file based on the file type
+// There is only support for .json and .yml now. It will use the underlying json/yaml packages directly.
+// meaning those should be the only required tags.
+func LoadConfigFromFile(configFile string, input interface{}) error {
+	if configFile == "" {
+		return nil
+	}
+
+	// read in all the bytes
+	data, err := ioutil.ReadFile(configFile)
+	if err != nil {
+		return err
+	}
+
+	configExt := filepath.Ext(configFile)
+	switch configExt {
+	case ".json":
+		return json.Unmarshal(data, input)
+	case ".yaml", ".yml":
+		return yaml.Unmarshal(data, input)
+	}
+	return &ErrUnknownConfigFormat{configExt}
+}
+
 func LoadFromEnv(prefix, filename string, face interface{}) error {
 	var err error
 	if filename == "" {