Ensure config values roundtrip properly through config_dump (#1266)

- Set the appropriate yaml tags for dumping
- Add test to verify roundtrip

Fixes #1261

server/config/config.go

@@ -16,15 +16,15 @@ const (
 
 // MysqlConfig defines configs related to MySQL
 type MysqlConfig struct {
-	Address    string
+	Address       string
-	Username   string
+	Username      string
-	Password   string
+	Password      string
-	Database   string
+	Database      string
-	Cert       string
+	TLSCert       string `yaml:"tls_cert"`
-	Key        string
+	TLSKey        string `yaml:"tls_key"`
-	CA         string
+	TLSCA         string `yaml:"tls_ca"`
-	ServerName string
+	TLSServerName string `yaml:"tls_server_name"`
-	TLSConfig  string //tls=customValue in DSN
+	TLSConfig     string `yaml:"tls_config"` //tls=customValue in DSN
 }
 
 // RedisConfig defines configs related to Redis
@@ -43,37 +43,37 @@ type ServerConfig struct {
 
 // AuthConfig defines configs related to user authorization
 type AuthConfig struct {
-	JwtKey      string
+	JwtKey      string `yaml:"jwt_key"`
-	BcryptCost  int
+	BcryptCost  int    `yaml:"bcrypt_cost"`
-	SaltKeySize int
+	SaltKeySize int    `yaml:"salt_key_size"`
 }
 
 // AppConfig defines configs related to HTTP
 type AppConfig struct {
-	TokenKeySize              int
+	TokenKeySize              int           `yaml:"token_key_size"`
-	TokenKey                  string
+	TokenKey                  string        `yaml:"token_key"`
-	InviteTokenValidityPeriod time.Duration
+	InviteTokenValidityPeriod time.Duration `yaml:"invite_token_validity_period"`
 }
 
 // SessionConfig defines configs related to user sessions
 type SessionConfig struct {
-	KeySize  int
+	KeySize  int `yaml:"key_size"`
 	Duration time.Duration
 }
 
 // OsqueryConfig defines configs related to osquery
 type OsqueryConfig struct {
-	NodeKeySize         int
+	NodeKeySize         int           `yaml:"node_key_size"`
-	StatusLogFile       string
+	StatusLogFile       string        `yaml:"status_log_file"`
-	ResultLogFile       string
+	ResultLogFile       string        `yaml:"result_log_file"`
-	LabelUpdateInterval time.Duration
+	LabelUpdateInterval time.Duration `yaml:"label_update_interval"`
 }
 
 // LoggingConfig defines configs related to logging
 type LoggingConfig struct {
 	Debug         bool
 	JSON          bool
-	DisableBanner bool
+	DisableBanner bool `yaml:"disable_banner"`
 }
 
 // KolideConfig stores the application configuration. Each subcategory is
@@ -103,14 +103,14 @@ func (man Manager) addConfigs() {
 		"MySQL server password (prefer env variable for security)")
 	man.addConfigString("mysql.database", "kolide",
 		"MySQL database name")
-	man.addConfigString("mysql.tls_certificate", "",
+	man.addConfigString("mysql.tls_cert", "",
-		"MySQL TLS client certificate")
+		"MySQL TLS client certificate path")
 	man.addConfigString("mysql.tls_key", "",
-		"MySQL TLS client key")
+		"MySQL TLS client key path")
 	man.addConfigString("mysql.tls_ca", "",
 		"MySQL TLS server CA")
 	man.addConfigString("mysql.tls_server_name", "",
-		"MySQL TLS client key")
+		"MySQL TLS server name")
 	man.addConfigString("mysql.tls_config", "",
 		"MySQL TLS config value. Use skip-verify, true, false or custom key.")
 
@@ -178,15 +178,15 @@ func (man Manager) LoadConfig() KolideConfig {
 
 	return KolideConfig{
 		Mysql: MysqlConfig{
-			Address:    man.getConfigString("mysql.address"),
+			Address:       man.getConfigString("mysql.address"),
-			Username:   man.getConfigString("mysql.username"),
+			Username:      man.getConfigString("mysql.username"),
-			Password:   man.getConfigString("mysql.password"),
+			Password:      man.getConfigString("mysql.password"),
-			Database:   man.getConfigString("mysql.database"),
+			Database:      man.getConfigString("mysql.database"),
-			Cert:       man.getConfigString("mysql.tls_certificate"),
+			TLSCert:       man.getConfigString("mysql.tls_cert"),
-			Key:        man.getConfigString("mysql.tls_key"),
+			TLSKey:        man.getConfigString("mysql.tls_key"),
-			CA:         man.getConfigString("mysql.tls_ca"),
+			TLSCA:         man.getConfigString("mysql.tls_ca"),
-			ServerName: man.getConfigString("mysql.tls_server_name"),
+			TLSServerName: man.getConfigString("mysql.tls_server_name"),
-			TLSConfig:  man.getConfigString("mysql.tls_config"),
+			TLSConfig:     man.getConfigString("mysql.tls_config"),
 		},
 		Redis: RedisConfig{
 			Address:  man.getConfigString("redis.address"),

server/config/config_test.go

@@ -0,0 +1,62 @@
+package config
+
+import (
+	"bytes"
+	"reflect"
+	"testing"
+	"time"
+
+	"github.com/spf13/cobra"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	yaml "gopkg.in/yaml.v2"
+)
+
+func TestConfigRoundtrip(t *testing.T) {
+	// This test verifies that a config can be roundtripped through yaml.
+	// Doing so ensures that config_dump will provide the correct config.
+	// Newly added config values will automatically be tested in this
+	// function because of the reflection on the config struct.
+
+	cmd := &cobra.Command{}
+	// Leaving this flag unset means that no attempt will be made to load
+	// the config file
+	cmd.PersistentFlags().StringP("config", "c", "", "Path to a configuration file")
+	man := NewManager(cmd)
+
+	// Use reflection magic to walk the config struct, setting unique
+	// values to be verified on the roundtrip. Note that bools are always
+	// set to true, which could false positive if the default value is
+	// true.
+	original := &KolideConfig{}
+	v := reflect.ValueOf(original)
+	for conf_index := 0; conf_index < v.Elem().NumField(); conf_index++ {
+		conf_v := v.Elem().Field(conf_index)
+		for key_index := 0; key_index < conf_v.NumField(); key_index++ {
+			key_v := conf_v.Field(key_index)
+			switch key_v.Interface().(type) {
+			case string:
+				key_v.SetString(v.Elem().Type().Field(conf_index).Name + "_" + conf_v.Type().Field(key_index).Name)
+			case int:
+				key_v.SetInt(int64(conf_index*100 + key_index))
+			case bool:
+				key_v.SetBool(true)
+			case time.Duration:
+				d := time.Duration(conf_index*100 + key_index)
+				key_v.Set(reflect.ValueOf(d))
+			}
+		}
+	}
+
+	// Marshal the generated config
+	buf, err := yaml.Marshal(original)
+	require.Nil(t, err)
+
+	// Manually load the serialized config
+	man.viper.SetConfigType("yaml")
+	err = man.viper.ReadConfig(bytes.NewReader(buf))
+	require.Nil(t, err)
+
+	// Ensure the read config is the same as the original
+	assert.Equal(t, *original, man.LoadConfig())
+}

server/datastore/mysql/mysql.go

@@ -187,7 +187,7 @@ func appendListOptionsToSQL(sql string, opts kolide.ListOptions) string {
 // registerTLS adds client certificate configuration to the mysql connection.
 func registerTLS(config config.MysqlConfig) error {
 	rootCertPool := x509.NewCertPool()
-	pem, err := ioutil.ReadFile(config.CA)
+	pem, err := ioutil.ReadFile(config.TLSCA)
 	if err != nil {
 		return errors.Wrap(err, "read server-ca pem")
 	}
@@ -195,7 +195,7 @@ func registerTLS(config config.MysqlConfig) error {
 		return errors.New("failed to append PEM.")
 	}
 	clientCert := make([]tls.Certificate, 0, 1)
-	certs, err := tls.LoadX509KeyPair(config.Cert, config.Key)
+	certs, err := tls.LoadX509KeyPair(config.TLSCert, config.TLSKey)
 	if err != nil {
 		return errors.Wrap(err, "load mysql client cert and key")
 	}
@@ -204,8 +204,8 @@ func registerTLS(config config.MysqlConfig) error {
 		RootCAs:      rootCertPool,
 		Certificates: clientCert,
 	}
-	if config.ServerName != "" {
+	if config.TLSServerName != "" {
-		cfg.ServerName = config.ServerName
+		cfg.ServerName = config.TLSServerName
 	}
 	if err := mysql.RegisterTLSConfig(config.TLSConfig, &cfg); err != nil {
 		return errors.Wrap(err, "register mysql tls config")