From 7dd8a472258e88531dece69c5f350e1f1ca5d404 Mon Sep 17 00:00:00 2001 From: Roland Shoemaker Date: Thu, 18 May 2017 14:27:50 -0700 Subject: [PATCH] Revert flag.FlagSet changes and instead manually set glog flags if provided in config --- configs/configs.go | 60 +++++++++++++++++++++++-- configs/configs_test.go | 45 +++++++++++-------- examples/ct/ct_server/main.go | 33 +++++++------- server/trillian_log_server/main.go | 32 +++++++------ server/trillian_log_signer/main.go | 49 ++++++++++---------- server/vmap/trillian_map_server/main.go | 26 +++++------ 6 files changed, 150 insertions(+), 95 deletions(-) diff --git a/configs/configs.go b/configs/configs.go index 8bb895af59..f7ce3499d3 100644 --- a/configs/configs.go +++ b/configs/configs.go @@ -16,6 +16,8 @@ package configs import ( "encoding/json" + "errors" + "flag" "fmt" "io/ioutil" "time" @@ -31,8 +33,7 @@ type JSONDuration struct { // UnmarshalJSON parses a time.Duration field from a string func (jd *JSONDuration) UnmarshalJSON(data []byte) error { s := "" - err := json.Unmarshal(data, &s) - if err != nil { + if err := json.Unmarshal(data, &s); err != nil { return err } d, err := time.ParseDuration(s) @@ -44,8 +45,13 @@ func (jd *JSONDuration) UnmarshalJSON(data []byte) error { } // LoadConfig loads a configuration JSON file from the provided path and -// unmarshals it into the provided object. +// unmarshals it into the provided object. LoadConfig also enforces the +// exclusivity of the --config flag, if any other flags are used it will +// throw an error. func LoadConfig(path string, obj interface{}) error { + if flag.CommandLine.NFlag() > 1 { + return errors.New("Cannot use --config with any other flags") + } f, err := ioutil.ReadFile(path) if err != nil { return fmt.Errorf("Failed to read config file: %v", err) @@ -55,3 +61,51 @@ func LoadConfig(path string, obj interface{}) error { } return nil } + +// GLogFlags contains the values of the flags that github.com/golang/glog +// registers. +type GLogFlags struct { + LogToStderr bool + AlsoLogToStderr bool + LogDir string + LogBacktraceAt string + V int + VModule string +} + +// SetGLogFlags sets the flags that github.com/golang/glog registers from +// the config struct if they contain non-default values so that they can +// be set from a file. +func SetGLogFlags(gf GLogFlags) error { + if gf.LogToStderr { + if err := flag.Set("logtostderr", fmt.Sprintf("%t", gf.LogToStderr)); err != nil { + return err + } + } + if gf.AlsoLogToStderr { + if err := flag.Set("alsologtostderr", fmt.Sprintf("%t", gf.AlsoLogToStderr)); err != nil { + return err + } + } + if gf.LogDir != "" { + if err := flag.Set("log_dir", gf.LogDir); err != nil { + return err + } + } + if gf.LogBacktraceAt != "" { + if err := flag.Set("log_backtrace_at", gf.LogBacktraceAt); err != nil { + return err + } + } + if gf.V != 0 { + if err := flag.Set("v", fmt.Sprintf("%d", gf.V)); err != nil { + return err + } + } + if gf.VModule != "" { + if err := flag.Set("vmodule", gf.VModule); err != nil { + return err + } + } + return nil +} diff --git a/configs/configs_test.go b/configs/configs_test.go index 8c9ace53ce..623093dfc8 100644 --- a/configs/configs_test.go +++ b/configs/configs_test.go @@ -16,30 +16,37 @@ package configs import ( "encoding/json" + "fmt" "testing" "time" ) func TestJSONDuration(t *testing.T) { - var s struct { - Dur JSONDuration + testCases := []struct { + json []byte + wantErr string + wantDuration time.Duration + }{ + {[]byte(`{"dur":"10s"}`), "", time.Second * 10}, + {[]byte(`{"dur": 1}`), "json: cannot unmarshal number into Go struct field .Dur of type string", 0}, + {[]byte(`{"dur": ""}`), "time: invalid duration ", 0}, } - - a := []byte(`{"dur":"10s"}`) - if err := json.Unmarshal(a, &s); err != nil { - t.Errorf("Failed to unmarshal duration: %v", err) - } - if s.Dur.Duration != (time.Second * 10) { - t.Errorf("Unexpected duration, want: 10s, got: %s", s.Dur) - } - - b := []byte(`{"dur": 1}`) - if err := json.Unmarshal(b, &s); err == nil { - t.Error("Didn't fail when unmarshalling a non-string type into JSONDuration") - } - - c := []byte(`{"dur": ""}`) - if err := json.Unmarshal(c, &s); err == nil { - t.Error("Didn't fail when unmarshalling an empty string into JSONDuration") + for _, tc := range testCases { + t.Run(fmt.Sprintf("json: %s", tc.json), func(t *testing.T) { + var s struct { + Dur JSONDuration + } + err := json.Unmarshal(tc.json, &s) + if err != nil && tc.wantErr == "" { + t.Errorf("failed to unmarshal into JSONDuration: %s", err) + } else if err != nil && err.Error() != tc.wantErr { + t.Errorf("unexpected error: want: %q, got %q", tc.wantErr, err) + } else if err == nil && tc.wantErr != "" { + t.Errorf("json.Unmarshal didn't fail: want %q", tc.wantErr) + } + if tc.wantDuration != 0 && tc.wantDuration != s.Dur.Duration { + t.Errorf("unexpected duration unmarshalled: want: %s, got %s", tc.wantDuration, s.Dur.Duration) + } + }) } } diff --git a/examples/ct/ct_server/main.go b/examples/ct/ct_server/main.go index 228285e089..69c6cd35bb 100644 --- a/examples/ct/ct_server/main.go +++ b/examples/ct/ct_server/main.go @@ -35,6 +35,9 @@ import ( "google.golang.org/grpc/naming" ) +// config represents all configurable settings of {CT,Log,Map} server and Log signer. +// Fields may be set either via flag or a configuration file in JSON format. +// Fields must be public due to a requirement of json.Unmarshal. type config struct { Host string Port int @@ -43,34 +46,30 @@ type config struct { LogConfig string MaxGetEntriesAllowed int64 EtcdServers string + + configs.GLogFlags } -var configFlag = flag.String("config", "", "Path to JSON config file to load instead of providing config via flags, if provided may only be used along with the following flags: --logtostderr, --alsologtostderr, --stderrthreshold, --log_dir, --log_backtrace_at, --v, --vmodule") +var configFlag = flag.String("config", "", "Path to JSON config file to load instead of providing config via flags, if provided no other flags may be used") func main() { c := config{} - flags := flag.NewFlagSet(os.Args[0], flag.ExitOnError) - flags.StringVar(&c.Host, "host", "localhost", "Address to serve CT log requests on") - flags.IntVar(&c.Port, "port", 6962, "Port to serve CT log requests on") - flags.StringVar(&c.LogRPCServer, "log_rpc_server", "localhost:8090", "Backend specification; comma-separated list or etcd service name (if --etcd_servers specified)") - flags.DurationVar(&c.RPCDeadline.Duration, "rpc_deadline", time.Second*10, "Deadline for backend RPC requests") - flags.StringVar(&c.LogConfig, "log_config", "", "File holding log config in JSON") - flags.Int64Var(&c.MaxGetEntriesAllowed, "maxGetEntriesAllowed", 0, "Max number of entries we allow in a get-entries request (default 50)") - flags.StringVar(&c.EtcdServers, "etcd_servers", "", "A comma-separated list of etcd servers") - // Both the default flag set and the binary specific flag set need to have Parse() called - // so that we can also initialize the flags registered by github.com/golang/glog as well - // as the ones defined above - flags.Parse(os.Args[1:]) + flag.StringVar(&c.Host, "host", "localhost", "Address to serve CT log requests on") + flag.IntVar(&c.Port, "port", 6962, "Port to serve CT log requests on") + flag.StringVar(&c.LogRPCServer, "log_rpc_server", "localhost:8090", "Backend specification; comma-separated list or etcd service name (if --etcd_servers specified)") + flag.DurationVar(&c.RPCDeadline.Duration, "rpc_deadline", time.Second*10, "Deadline for backend RPC requests") + flag.StringVar(&c.LogConfig, "log_config", "", "File holding log config in JSON") + flag.Int64Var(&c.MaxGetEntriesAllowed, "maxGetEntriesAllowed", 0, "Max number of entries we allow in a get-entries request (default 50)") + flag.StringVar(&c.EtcdServers, "etcd_servers", "", "A comma-separated list of etcd servers") flag.Parse() if *configFlag != "" { - // Only allow either --config OR any other flags - if flags.NFlag() > 1 { - glog.Exit("Cannot use --config with any other flags") - } if err := configs.LoadConfig(*configFlag, &c); err != nil { glog.Exit(err) } + if err := configs.SetGLogFlags(c.GLogFlags); err != nil { + glog.Exit(err) + } } if c.MaxGetEntriesAllowed > 0 { diff --git a/server/trillian_log_server/main.go b/server/trillian_log_server/main.go index b2df491412..b858a15a2d 100644 --- a/server/trillian_log_server/main.go +++ b/server/trillian_log_server/main.go @@ -18,7 +18,6 @@ import ( "context" "flag" _ "net/http/pprof" - "os" "strings" "time" @@ -42,6 +41,9 @@ import ( "google.golang.org/grpc/naming" ) +// config represents all configurable settings of {CT,Log,Map} server and Log signer. +// Fields may be set either via flag or a configuration file in JSON format. +// Fields must be public due to a requirement of json.Unmarshal. type config struct { MySQLURI string RPCEndpoint string @@ -49,33 +51,29 @@ type config struct { DumpMetricsInterval configs.JSONDuration EtcdServers string EtcdService string + + configs.GLogFlags } -var configFlag = flag.String("config", "", "Path to JSON config file to load instead of providing config via flags, if provided may only be used along with the following flags: --logtostderr, --alsologtostderr, --stderrthreshold, --log_dir, --log_backtrace_at, --v, --vmodule") +var configFlag = flag.String("config", "", "Path to JSON config file to load instead of providing config via flags, if provided no other flags may be used") func main() { c := config{} - flags := flag.NewFlagSet(os.Args[0], flag.ExitOnError) - flags.StringVar(&c.MySQLURI, "mysql_uri", "test:zaphod@tcp(127.0.0.1:3306)/test", "Connection URI for MySQL database") - flags.StringVar(&c.RPCEndpoint, "rpc_endpoint", "localhost:8090", "Endpoint for RPC requests (host:port)") - flags.StringVar(&c.HTTPEndpoint, "http_endpoint", "localhost:8091", "Endpoint for HTTP metrics and REST requests on (host:port, empty means disabled)") - flags.DurationVar(&c.DumpMetricsInterval.Duration, "dump_metrics_interval", 0, "If greater than 0, how often to dump metrics to the logs.") - flags.StringVar(&c.EtcdServers, "etcd_servers", "", "A comma-separated list of etcd servers; no etcd registration if empty") - flags.StringVar(&c.EtcdService, "etcd_service", "trillian-log", "Service name to announce ourselves under") - // Both the default flag set and the binary specific flag set need to have Parse() called - // so that we can also initialize the flags registered by github.com/golang/glog as well - // as the ones defined above - flags.Parse(os.Args[1:]) + flag.StringVar(&c.MySQLURI, "mysql_uri", "test:zaphod@tcp(127.0.0.1:3306)/test", "Connection URI for MySQL database") + flag.StringVar(&c.RPCEndpoint, "rpc_endpoint", "localhost:8090", "Endpoint for RPC requests (host:port)") + flag.StringVar(&c.HTTPEndpoint, "http_endpoint", "localhost:8091", "Endpoint for HTTP metrics and REST requests on (host:port, empty means disabled)") + flag.DurationVar(&c.DumpMetricsInterval.Duration, "dump_metrics_interval", 0, "If greater than 0, how often to dump metrics to the logs.") + flag.StringVar(&c.EtcdServers, "etcd_servers", "", "A comma-separated list of etcd servers; no etcd registration if empty") + flag.StringVar(&c.EtcdService, "etcd_service", "trillian-log", "Service name to announce ourselves under") flag.Parse() if *configFlag != "" { - // Only allow either --config OR any other flags - if flags.NFlag() > 1 { - glog.Exit("Cannot use --config with any other flags") - } if err := configs.LoadConfig(*configFlag, &c); err != nil { glog.Exit(err) } + if err := configs.SetGLogFlags(c.GLogFlags); err != nil { + glog.Exit(err) + } } ctx := context.Background() diff --git a/server/trillian_log_signer/main.go b/server/trillian_log_signer/main.go index 2062759605..667823a747 100644 --- a/server/trillian_log_signer/main.go +++ b/server/trillian_log_signer/main.go @@ -35,6 +35,9 @@ import ( "golang.org/x/net/context" ) +// config represents all configurable settings of {CT,Log,Map} server and Log signer. +// Fields may be set either via flag or a configuration file in JSON format. +// Fields must be public due to a requirement of json.Unmarshal. type config struct { MySQLURI string HTTPEndpoint string @@ -50,42 +53,38 @@ type config struct { MasterCheckInterval configs.JSONDuration MasterHoldInterval configs.JSONDuration ResignOdds int + + configs.GLogFlags } -var configFlag = flag.String("config", "", "Path to JSON config file to load instead of providing config via flags, if provided may only be used along with the following flags: --logtostderr, --alsologtostderr, --stderrthreshold, --log_dir, --log_backtrace_at, --v, --vmodule") +var configFlag = flag.String("config", "", "Path to JSON config file to load instead of providing config via flags, if provided no other flags may be used") func main() { c := config{} - flags := flag.NewFlagSet(os.Args[0], flag.ExitOnError) - flags.StringVar(&c.MySQLURI, "mysql_uri", "test:zaphod@tcp(127.0.0.1:3306)/test", "Connection URI for MySQL database") - flags.StringVar(&c.HTTPEndpoint, "http_endpoint", "localhost:8091", "Endpoint for HTTP (host:port, empty means disabled)") - flags.DurationVar(&c.SequencerInterval.Duration, "sequencer_interval", time.Second*10, "Time between each sequencing pass through all logs") - flags.IntVar(&c.BatchSize, "batch_size", 50, "Max number of leaves to process per batch") - flags.IntVar(&c.NumSequencers, "num_sequencers", 10, "Number of sequencer workers to run in parallel") - flags.DurationVar(&c.SequencerGuardWindow.Duration, "sequencer_guard_window", 0, "If set, the time elapsed before submitted leaves are eligible for sequencing") - flags.DurationVar(&c.DumpMetricsInterval.Duration, "dump_metrics_interval", 0, "If greater than 0, how often to dump metrics to the logs.") - flags.BoolVar(&c.ForceMaster, "force_master", false, "If true, assume master for all logs") - flags.StringVar(&c.EtcdServers, "etcd_servers", "localhost:2379", "A comma-separated list of etcd servers") - flags.StringVar(&c.LockFilePath, "lock_file_path", "/test/multimaster", "etcd lock file directory path") - - flags.DurationVar(&c.PreElectionPause.Duration, "pre_election_pause", 1*time.Second, "Maximum time to wait before starting elections") - flags.DurationVar(&c.MasterCheckInterval.Duration, "master_check_interval", 5*time.Second, "Interval between checking mastership still held") - flags.DurationVar(&c.MasterHoldInterval.Duration, "master_hold_interval", 60*time.Second, "Minimum interval to hold mastership for") - flags.IntVar(&c.ResignOdds, "resign_odds", 10, "Chance of resigning mastership after each check, the N in 1-in-N") - // Both the default flag set and the binary specific flag set need to have Parse() called - // so that we can also initialize the flags registered by github.com/golang/glog as well - // as the ones defined above - flags.Parse(os.Args[1:]) + flag.StringVar(&c.MySQLURI, "mysql_uri", "test:zaphod@tcp(127.0.0.1:3306)/test", "Connection URI for MySQL database") + flag.StringVar(&c.HTTPEndpoint, "http_endpoint", "localhost:8091", "Endpoint for HTTP (host:port, empty means disabled)") + flag.DurationVar(&c.SequencerInterval.Duration, "sequencer_interval", time.Second*10, "Time between each sequencing pass through all logs") + flag.IntVar(&c.BatchSize, "batch_size", 50, "Max number of leaves to process per batch") + flag.IntVar(&c.NumSequencers, "num_sequencers", 10, "Number of sequencer workers to run in parallel") + flag.DurationVar(&c.SequencerGuardWindow.Duration, "sequencer_guard_window", 0, "If set, the time elapsed before submitted leaves are eligible for sequencing") + flag.DurationVar(&c.DumpMetricsInterval.Duration, "dump_metrics_interval", 0, "If greater than 0, how often to dump metrics to the logs.") + flag.BoolVar(&c.ForceMaster, "force_master", false, "If true, assume master for all logs") + flag.StringVar(&c.EtcdServers, "etcd_servers", "localhost:2379", "A comma-separated list of etcd servers") + flag.StringVar(&c.LockFilePath, "lock_file_path", "/test/multimaster", "etcd lock file directory path") + + flag.DurationVar(&c.PreElectionPause.Duration, "pre_election_pause", 1*time.Second, "Maximum time to wait before starting elections") + flag.DurationVar(&c.MasterCheckInterval.Duration, "master_check_interval", 5*time.Second, "Interval between checking mastership still held") + flag.DurationVar(&c.MasterHoldInterval.Duration, "master_hold_interval", 60*time.Second, "Minimum interval to hold mastership for") + flag.IntVar(&c.ResignOdds, "resign_odds", 10, "Chance of resigning mastership after each check, the N in 1-in-N") flag.Parse() if *configFlag != "" { - // Only allow either --config OR any other flags - if flags.NFlag() > 1 { - glog.Exit("Cannot use --config with any other flags") - } if err := configs.LoadConfig(*configFlag, &c); err != nil { glog.Exit(err) } + if err := configs.SetGLogFlags(c.GLogFlags); err != nil { + glog.Exit(err) + } } glog.CopyStandardLogTo("WARNING") diff --git a/server/vmap/trillian_map_server/main.go b/server/vmap/trillian_map_server/main.go index 8bf2ebfb9e..538448bd13 100644 --- a/server/vmap/trillian_map_server/main.go +++ b/server/vmap/trillian_map_server/main.go @@ -18,7 +18,6 @@ import ( "context" "flag" _ "net/http/pprof" - "os" _ "github.com/go-sql-driver/mysql" // Load MySQL driver @@ -36,34 +35,33 @@ import ( "google.golang.org/grpc" ) +// config represents all configurable settings of {CT,Log,Map} server and Log signer. +// Fields may be set either via flag or a configuration file in JSON format. +// Fields must be public due to a requirement of json.Unmarshal. type config struct { MySQLURI string RPCEndpoint string HTTPEndpoint string + + configs.GLogFlags } -var configFlag = flag.String("config", "", "Path to JSON config file to load instead of providing config via flags, if provided may only be used along with the following flags: --logtostderr, --alsologtostderr, --stderrthreshold, --log_dir, --log_backtrace_at, --v, --vmodule") +var configFlag = flag.String("config", "", "Path to JSON config file to load instead of providing config via flags, if provided no other flags may be used") func main() { c := config{} - flags := flag.NewFlagSet(os.Args[0], flag.ExitOnError) - flags.StringVar(&c.MySQLURI, "mysql_uri", "test:zaphod@tcp(127.0.0.1:3306)/test", "Connection URI for MySQL database") - flags.StringVar(&c.RPCEndpoint, "rpc_endpoint", "localhost:8090", "Endpoint for RPC requests (host:port)") - flags.StringVar(&c.HTTPEndpoint, "http_endpoint", "localhost:8091", "Endpoint for HTTP metrics and REST requests on (host:port, empty means disabled)") - // Both the default flag set and the binary specific flag set need to have Parse() called - // so that we can also initialize the flags registered by github.com/golang/glog as well - // as the ones defined above - flags.Parse(os.Args[1:]) + flag.StringVar(&c.MySQLURI, "mysql_uri", "test:zaphod@tcp(127.0.0.1:3306)/test", "Connection URI for MySQL database") + flag.StringVar(&c.RPCEndpoint, "rpc_endpoint", "localhost:8090", "Endpoint for RPC requests (host:port)") + flag.StringVar(&c.HTTPEndpoint, "http_endpoint", "localhost:8091", "Endpoint for HTTP metrics and REST requests on (host:port, empty means disabled)") flag.Parse() if *configFlag != "" { - // Only allow either --config OR any other flags - if flags.NFlag() > 1 { - glog.Exit("Cannot use --config with any other flags") - } if err := configs.LoadConfig(*configFlag, &c); err != nil { glog.Exit(err) } + if err := configs.SetGLogFlags(c.GLogFlags); err != nil { + glog.Exit(err) + } } db, err := mysql.OpenDB(c.MySQLURI)