Use shared configuration parsing tools in notary-server.

This changes the 'addr' parameter of notary-server's config to
'http_addr', so we can add a GRPC server to notary-server if
necessary.  This also allows environment variables to override
the notary-server config file entries, as notary-signer already
does.

The bugsnag configuration has also been changed so that the
bugsnag parameters are under the "bugsnag" key.

Signed-off-by: Ying Li <ying.li@docker.com>

cmd/notary-server/config.json

@@ -1,6 +1,6 @@
 {
 	"server": {
-		"addr": ":4443",
+		"http_addr": ":4443",
 		"tls_key_file": "./fixtures/notary-server.key",
 		"tls_cert_file": "./fixtures/notary-server.crt"
 	},

cmd/notary-server/main.go

@@ -13,18 +13,16 @@ import (
 	"time"
 
 	"github.com/Sirupsen/logrus"
-	"github.com/bugsnag/bugsnag-go"
 	"github.com/docker/distribution/health"
 	_ "github.com/docker/distribution/registry/auth/htpasswd"
 	_ "github.com/docker/distribution/registry/auth/token"
+	"github.com/docker/notary/server/storage"
 	"github.com/docker/notary/signer/client"
 	"github.com/docker/notary/tuf/signed"
 	_ "github.com/go-sql-driver/mysql"
 	"golang.org/x/net/context"
 
-	bugsnag_hook "github.com/Sirupsen/logrus/hooks/bugsnag"
 	"github.com/docker/notary/server"
-	"github.com/docker/notary/server/storage"
 	"github.com/docker/notary/utils"
 	"github.com/docker/notary/version"
 	"github.com/spf13/viper"
@@ -36,38 +34,41 @@ const DebugAddress = "localhost:8080"
 var (
 	debug      bool
 	configFile string
+	envPrefix  = "NOTARY_SERVER"
 	mainViper  = viper.New()
 )
 
 func init() {
-	// set default log level to Error
+	utils.SetupViper(mainViper, envPrefix)
-	mainViper.SetDefault("logging", map[string]interface{}{"level": 2})
-
 	// Setup flags
 	flag.StringVar(&configFile, "config", "", "Path to configuration file")
 	flag.BoolVar(&debug, "debug", false, "Enable the debugging server on localhost:8080")
 }
 
-// optionally sets up TLS for the server - if no TLS configuration is
+// get the address for the HTTP server, and parses the optional TLS
-// specified, TLS is not enabled.
+// configuration for the server - if no TLS configuration is specified,
-func serverTLS(configuration *viper.Viper) (*tls.Config, error) {
+// TLS is not enabled.
-	tlsCertFile := configuration.GetString("server.tls_cert_file")
+func getAddrAndTLSConfig(configuration *viper.Viper) (string, *tls.Config, error) {
-	tlsKeyFile := configuration.GetString("server.tls_key_file")
+	httpAddr := configuration.GetString("server.http_addr")
-
+	if httpAddr == "" {
-	if tlsCertFile == "" && tlsKeyFile == "" {
+		return "", nil, fmt.Errorf("http listen address required for server")
-		return nil, nil
-	} else if tlsCertFile == "" || tlsKeyFile == "" {
-		return nil, fmt.Errorf("Partial TLS configuration found. Either include both a cert and key file in the configuration, or include neither to disable TLS.")
 	}
 
-	tlsConfig, err := utils.ConfigureServerTLS(&utils.ServerTLSOpts{
+	tlsOpts, err := utils.ParseServerTLS(configuration, false)
-		ServerCertFile: tlsCertFile,
-		ServerKeyFile:  tlsKeyFile,
-	})
 	if err != nil {
-		return nil, fmt.Errorf("Unable to set up TLS: %s", err.Error())
+		return "", nil, fmt.Errorf(err.Error())
 	}
-	return tlsConfig, nil
+	// do not support this yet since the client doesn't have client cert support
+	if tlsOpts != nil {
+		tlsOpts.ClientCAFile = ""
+		tlsConfig, err := utils.ConfigureServerTLS(tlsOpts)
+		if err != nil {
+			return "", nil, fmt.Errorf(
+				"unable to set up TLS for server: %s", err.Error())
+		}
+		return httpAddr, tlsConfig, nil
+	}
+	return httpAddr, nil, nil
 }
 
 // sets up TLS for the GRPC connection to notary-signer
@@ -94,6 +95,28 @@ func grpcTLS(configuration *viper.Viper) (*tls.Config, error) {
 	return tlsConfig, nil
 }
 
+func getStore(configuration *viper.Viper, allowedBackends []string) (
+	storage.MetaStore, error) {
+
+	storeConfig, err := utils.ParseStorage(configuration, allowedBackends)
+	if err != nil {
+		return nil, err
+	}
+	if storeConfig != nil {
+		logrus.Infof("Using %s backend", storeConfig.Backend)
+		store, err := storage.NewSQLStorage(storeConfig.Backend, storeConfig.Source)
+		if err != nil {
+			return nil, fmt.Errorf("Error starting DB driver: ", err.Error())
+		}
+		health.RegisterPeriodicFunc(
+			"DB operational", store.CheckHealth, time.Second*60)
+		return store, nil
+	}
+
+	logrus.Debug("Using memory backend")
+	return storage.NewMemStorage(), nil
+}
+
 func main() {
 	flag.Usage = usage
 	flag.Parse()
@@ -115,40 +138,27 @@ func main() {
 	mainViper.SetConfigName(strings.TrimSuffix(filename, ext))
 	mainViper.AddConfigPath(configPath)
 
-	// Automatically accept configuration options from the environment
-	mainViper.SetEnvPrefix("NOTARY_SERVER")
-	mainViper.SetEnvKeyReplacer(strings.NewReplacer(".", "_"))
-	mainViper.AutomaticEnv()
-
 	err := mainViper.ReadInConfig()
 	if err != nil {
 		logrus.Error("Viper Error: ", err.Error())
 		logrus.Error("Could not read config at ", configFile)
 		os.Exit(1)
 	}
-	lvl, err := logrus.ParseLevel(mainViper.GetString("logging.level"))
+
+	// default is error level
+	lvl, err := utils.ParseLogLevel(mainViper, logrus.ErrorLevel)
 	if err != nil {
-		lvl = logrus.ErrorLevel
+		logrus.Fatal(err.Error())
-		logrus.Error("Could not parse log level from config. Defaulting to ErrorLevel")
 	}
 	logrus.SetLevel(lvl)
 
-	// set up bugsnag and attach to logrus
+	// parse bugsnag config
-	bugs := mainViper.GetString("reporting.bugsnag")
+	bugsnagConf, err := utils.ParseBugsnag(mainViper)
-	if bugs != "" {
+	if err != nil {
-		apiKey := mainViper.GetString("reporting.bugsnag_api_key")
+		logrus.Fatal(err.Error())
-		releaseStage := mainViper.GetString("reporting.bugsnag_release_stage")
-		bugsnag.Configure(bugsnag.Configuration{
-			APIKey:       apiKey,
-			ReleaseStage: releaseStage,
-		})
-		hook, err := bugsnag_hook.NewBugsnagHook()
-		if err != nil {
-			logrus.Error("Could not attach bugsnag to logrus: ", err.Error())
-		} else {
-			logrus.AddHook(hook)
-		}
 	}
+	utils.SetUpBugsnag(bugsnagConf)
+
 	keyAlgo := mainViper.GetString("trust_service.key_algorithm")
 	if keyAlgo == "" {
 		logrus.Fatal("no key algorithm configured.")
@@ -188,23 +198,13 @@ func main() {
 		trust = signed.NewEd25519()
 	}
 
-	if mainViper.GetString("storage.backend") == "mysql" {
+	store, err := getStore(mainViper, []string{"mysql"})
-		logrus.Info("Using mysql backend")
+	if err != nil {
-		dbURL := mainViper.GetString("storage.db_url")
+		logrus.Fatal(err.Error())
-		store, err := storage.NewSQLStorage("mysql", dbURL)
-		if err != nil {
-			logrus.Fatal("Error starting DB driver: ", err.Error())
-			return // not strictly needed but let's be explicit
-		}
-		health.RegisterPeriodicFunc(
-			"DB operational", store.CheckHealth, time.Second*60)
-		ctx = context.WithValue(ctx, "metaStore", store)
-	} else {
-		logrus.Debug("Using memory backend")
-		ctx = context.WithValue(ctx, "metaStore", storage.NewMemStorage())
 	}
+	ctx = context.WithValue(ctx, "metaStore", store)
 
-	tlsConfig, err := serverTLS(mainViper)
+	httpAddr, tlsConfig, err := getAddrAndTLSConfig(mainViper)
 	if err != nil {
 		logrus.Fatal(err.Error())
 	}
@@ -212,7 +212,7 @@ func main() {
 	logrus.Info("Starting Server")
 	err = server.Run(
 		ctx,
-		mainViper.GetString("server.addr"),
+		httpAddr,
 		tlsConfig,
 		trust,
 		mainViper.GetString("auth.type"),

cmd/notary-server/main_test.go

@@ -4,9 +4,12 @@ import (
 	"bytes"
 	"crypto/tls"
 	"fmt"
+	"io/ioutil"
+	"os"
 	"strings"
 	"testing"
 
+	"github.com/docker/notary/server/storage"
 	"github.com/spf13/viper"
 	"github.com/stretchr/testify/assert"
 )
@@ -18,63 +21,71 @@ const (
 )
 
 // initializes a viper object with test configuration
-func configure(jsonConfig []byte) *viper.Viper {
+func configure(jsonConfig string) *viper.Viper {
 	config := viper.New()
 	config.SetConfigType("json")
-	config.ReadConfig(bytes.NewBuffer(jsonConfig))
+	config.ReadConfig(bytes.NewBuffer([]byte(jsonConfig)))
 	return config
 }
 
-// If neither the cert nor the key are provided, a nil tls config is returned.
+func TestGetAddrAndTLSConfigInvalidTLS(t *testing.T) {
-func TestServerTLSMissingCertAndKey(t *testing.T) {
+	invalids := []string{
-	tlsConfig, err := serverTLS(configure([]byte(`{"server": {}}`)))
+		`{"server": {
-	assert.NoError(t, err)
+				"http_addr": ":1234",
-	assert.Nil(t, tlsConfig)
+				"tls_key_file": "nope"
-}
+		}}`,
-
-// Cert and Key either both have to be empty or both have to be provided.
-func TestServerTLSMissingCertAndOrKey(t *testing.T) {
-	configs := []string{
-		fmt.Sprintf(`{"tls_cert_file": "%s"}`, Cert),
-		fmt.Sprintf(`{"tls_key_file": "%s"}`, Key),
 	}
-	for _, serverConfig := range configs {
+	for _, configJSON := range invalids {
-		config := configure(
+		_, _, err := getAddrAndTLSConfig(configure(configJSON))
-			[]byte(fmt.Sprintf(`{"server": %s}`, serverConfig)))
-		tlsConfig, err := serverTLS(config)
 		assert.Error(t, err)
-		assert.Nil(t, tlsConfig)
-		assert.True(t,
-			strings.Contains(err.Error(), "Partial TLS configuration found."))
 	}
 }
 
-// The rest of the functionality of serverTLS depends upon
+func TestGetAddrAndTLSConfigNoHTTPAddr(t *testing.T) {
-// utils.ConfigureServerTLS, so this test just asserts that if successful,
+	_, _, err := getAddrAndTLSConfig(configure(fmt.Sprintf(`{
-// the correct tls.Config is returned based on all the configuration parameters
+		"server": {
-func TestServerTLSSuccess(t *testing.T) {
+			"tls_cert_file": "%s",
-	keypair, err := tls.LoadX509KeyPair(Cert, Key)
+			"tls_key_file": "%s"
-	assert.NoError(t, err, "Unable to load cert and key for testing")
+		}
-
+	}`, Cert, Key)))
-	config := fmt.Sprintf(
+	assert.Error(t, err)
-		`{"server": {"tls_cert_file": "%s", "tls_key_file": "%s"}}`,
+	assert.Contains(t, err.Error(), "http listen address required for server")
-		Cert, Key)
-	tlsConfig, err := serverTLS(configure([]byte(config)))
-	assert.NoError(t, err)
-	assert.Equal(t, []tls.Certificate{keypair}, tlsConfig.Certificates)
 }
 
-// The rest of the functionality of serverTLS depends upon
+func TestGetAddrAndTLSConfigSuccessWithTLS(t *testing.T) {
-// utils.ConfigureServerTLS, so this test just asserts that if it fails,
+	httpAddr, tlsConf, err := getAddrAndTLSConfig(configure(fmt.Sprintf(`{
-// the error is propogated.
+		"server": {
-func TestServerTLSFailure(t *testing.T) {
+			"http_addr": ":2345",
-	config := fmt.Sprintf(
+			"tls_cert_file": "%s",
-		`{"server": {"tls_cert_file": "non-exist", "tls_key_file": "%s"}}`,
+			"tls_key_file": "%s"
-		Key)
+		}
-	tlsConfig, err := serverTLS(configure([]byte(config)))
+	}`, Cert, Key)))
-	assert.Error(t, err)
+	assert.NoError(t, err)
-	assert.Nil(t, tlsConfig)
+	assert.Equal(t, ":2345", httpAddr)
-	assert.True(t, strings.Contains(err.Error(), "Unable to set up TLS"))
+	assert.NotNil(t, tlsConf)
+}
+
+func TestGetAddrAndTLSConfigSuccessWithoutTLS(t *testing.T) {
+	httpAddr, tlsConf, err := getAddrAndTLSConfig(configure(
+		`{"server": {"http_addr": ":2345"}}`))
+	assert.NoError(t, err)
+	assert.Equal(t, ":2345", httpAddr)
+	assert.Nil(t, tlsConf)
+}
+
+// We don't support client CAs yet on notary server
+func TestGetAddrAndTLSConfigSkipClientTLS(t *testing.T) {
+	httpAddr, tlsConf, err := getAddrAndTLSConfig(configure(fmt.Sprintf(`{
+		"server": {
+			"http_addr": ":2345",
+			"tls_cert_file": "%s",
+			"tls_key_file": "%s",
+			"client_ca_file": "%s"
+		}
+	}`, Cert, Key, Root)))
+	assert.NoError(t, err)
+	assert.Equal(t, ":2345", httpAddr)
+	assert.Nil(t, tlsConf.ClientCAs)
 }
 
 // Client cert and Key either both have to be empty or both have to be
@@ -88,7 +99,7 @@ func TestGrpcTLSMissingCertOrKey(t *testing.T) {
 		jsonConfig := fmt.Sprintf(
 			`{"trust_service": {"hostname": "notary-signer", %s}}`,
 			trustConfig)
-		config := configure([]byte(jsonConfig))
+		config := configure(jsonConfig)
 		tlsConfig, err := grpcTLS(config)
 		assert.Error(t, err)
 		assert.Nil(t, tlsConfig)
@@ -101,7 +112,7 @@ func TestGrpcTLSMissingCertOrKey(t *testing.T) {
 // the provided serverName is still returned.
 func TestGrpcTLSNoConfig(t *testing.T) {
 	tlsConfig, err := grpcTLS(
-		configure([]byte(`{"trust_service": {"hostname": "notary-signer"}}`)))
+		configure(`{"trust_service": {"hostname": "notary-signer"}}`))
 	assert.NoError(t, err)
 	assert.Equal(t, "notary-signer", tlsConfig.ServerName)
 	assert.Nil(t, tlsConfig.RootCAs)
@@ -121,7 +132,7 @@ func TestGrpcTLSSuccess(t *testing.T) {
             "tls_client_cert": "%s",
             "tls_client_key": "%s"}}`,
 		Cert, Key)
-	tlsConfig, err := grpcTLS(configure([]byte(config)))
+	tlsConfig, err := grpcTLS(configure(config))
 	assert.NoError(t, err)
 	assert.Equal(t, []tls.Certificate{keypair}, tlsConfig.Certificates)
 }
@@ -136,9 +147,40 @@ func TestGrpcTLSFailure(t *testing.T) {
             "tls_client_cert": "no-exist",
             "tls_client_key": "%s"}}`,
 		Key)
-	tlsConfig, err := grpcTLS(configure([]byte(config)))
+	tlsConfig, err := grpcTLS(configure(config))
 	assert.Error(t, err)
 	assert.Nil(t, tlsConfig)
 	assert.True(t, strings.Contains(err.Error(),
 		"Unable to configure TLS to the trust service"))
 }
+
+// Just to ensure that errors are propogated
+func TestGetStoreInvalid(t *testing.T) {
+	config := `{"storage": {"backend": "asdf", "db_url": "/tmp/1234"}}`
+
+	_, err := getStore(configure(config), []string{"mysql"})
+	assert.Error(t, err)
+}
+
+func TestGetStoreDBStore(t *testing.T) {
+	tmpFile, err := ioutil.TempFile("/tmp", "sqlite3")
+	assert.NoError(t, err)
+	tmpFile.Close()
+	defer os.Remove(tmpFile.Name())
+
+	config := fmt.Sprintf(`{"storage": {"backend": "sqlite3", "db_url": "%s"}}`,
+		tmpFile.Name())
+
+	store, err := getStore(configure(config), []string{"sqlite3"})
+	assert.NoError(t, err)
+	_, ok := store.(*storage.SQLStorage)
+	assert.True(t, ok)
+}
+
+func TestGetMemoryStore(t *testing.T) {
+	config := fmt.Sprintf(`{"storage": {}}`)
+	store, err := getStore(configure(config), []string{"mysql"})
+	assert.NoError(t, err)
+	_, ok := store.(*storage.MemStorage)
+	assert.True(t, ok)
+}