-
Notifications
You must be signed in to change notification settings - Fork 379
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow flag or file based configuration #620
Changes from 5 commits
4712f74
ddcc786
4c06b2e
a6656ac
cc9922c
4fd46e5
d2cc80d
54850a1
661daaf
c49bc08
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,50 @@ | ||
// Copyright 2017 Google Inc. All Rights Reserved. | ||
// | ||
// Licensed under the Apache License, Version 2.0 (the "License"); | ||
// you may not use this file except in compliance with the License. | ||
// You may obtain a copy of the License at | ||
// | ||
// http://www.apache.org/licenses/LICENSE-2.0 | ||
// | ||
// Unless required by applicable law or agreed to in writing, software | ||
// distributed under the License is distributed on an "AS IS" BASIS, | ||
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
// See the License for the specific language governing permissions and | ||
// limitations under the License. | ||
|
||
package cmd | ||
|
||
import ( | ||
"flag" | ||
"io/ioutil" | ||
|
||
"github.com/mattn/go-shellwords" | ||
) | ||
|
||
// ParseFlagFile parses a set of flags from a file at the provided | ||
// path. Re-calls flag.Parse() after parsing the flags in the file | ||
// so that flags provided on the command line take precedence over | ||
// flags provided in the file. | ||
func ParseFlagFile(path string) error { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This func could do with some tests. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
file, err := ioutil.ReadFile(path) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
p := shellwords.NewParser() | ||
p.ParseEnv = true | ||
args, err := p.Parse(string(file)) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
err = flag.CommandLine.Parse(args) | ||
if err != nil { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These two lines could be combined, e.g. |
||
return err | ||
} | ||
|
||
// Call flag.Parse() again so that command line flags | ||
// can override flags provided in the provided flag file. | ||
flag.Parse() | ||
return nil | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,7 +17,9 @@ package main | |
import ( | ||
"context" | ||
"flag" | ||
"fmt" | ||
_ "net/http/pprof" | ||
"os" | ||
"strings" | ||
"time" | ||
|
||
|
@@ -27,6 +29,7 @@ import ( | |
etcdnaming "github.com/coreos/etcd/clientv3/naming" | ||
"github.com/golang/glog" | ||
"github.com/google/trillian" | ||
"github.com/google/trillian/cmd" | ||
"github.com/google/trillian/crypto/keys" | ||
"github.com/google/trillian/extension" | ||
"github.com/google/trillian/monitoring" | ||
|
@@ -48,10 +51,20 @@ var ( | |
etcdServers = flag.String("etcd_servers", "", "A comma-separated list of etcd servers; no etcd registration if empty") | ||
etcdService = flag.String("etcd_service", "trillian-log", "Service name to announce ourselves under") | ||
maxUnsequencedRows = flag.Int("max_unsequenced_rows", mysqlq.DefaultMaxUnsequenced, "Max number of unsequenced rows before rate limiting kicks in") | ||
|
||
configFile = flag.String("config", "", "Config file containing flags, file contents can be overridden by command line flags") | ||
) | ||
|
||
func main() { | ||
flag.Parse() | ||
|
||
if *configFile != "" { | ||
if err := cmd.ParseFlagFile(*configFile); err != nil { | ||
fmt.Fprintf(os.Stderr, "Failed to parse %v: %v\n", *configFile, err) | ||
os.Exit(1) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
} | ||
} | ||
|
||
ctx := context.Background() | ||
|
||
// First make sure we can access the database, quit if not | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,6 +23,7 @@ import ( | |
_ "github.com/go-sql-driver/mysql" // Load MySQL driver | ||
|
||
"github.com/golang/glog" | ||
"github.com/google/trillian/cmd" | ||
"github.com/google/trillian/crypto/keys" | ||
"github.com/google/trillian/extension" | ||
"github.com/google/trillian/monitoring/metric" | ||
|
@@ -50,10 +51,20 @@ var ( | |
masterCheckInterval = flag.Duration("master_check_interval", 5*time.Second, "Interval between checking mastership still held") | ||
masterHoldInterval = flag.Duration("master_hold_interval", 60*time.Second, "Minimum interval to hold mastership for") | ||
resignOdds = flag.Int("resign_odds", 10, "Chance of resigning mastership after each check, the N in 1-in-N") | ||
|
||
configFile = flag.String("config", "", "Config file containing flags, file contents can be overridden by command line flags") | ||
) | ||
|
||
func main() { | ||
flag.Parse() | ||
|
||
if *configFile != "" { | ||
if err := cmd.ParseFlagFile(*configFile); err != nil { | ||
fmt.Fprintf(os.Stderr, "Failed to parse %v: %v\n", *configFile, err) | ||
os.Exit(1) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
} | ||
} | ||
|
||
glog.CopyStandardLogTo("WARNING") | ||
glog.Info("**** Log Signer Starting ****") | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,12 +17,15 @@ package main | |
import ( | ||
"context" | ||
"flag" | ||
"fmt" | ||
_ "net/http/pprof" | ||
"os" | ||
|
||
_ "github.com/go-sql-driver/mysql" // Load MySQL driver | ||
|
||
"github.com/golang/glog" | ||
"github.com/google/trillian" | ||
"github.com/google/trillian/cmd" | ||
"github.com/google/trillian/crypto/keys" | ||
"github.com/google/trillian/extension" | ||
mysqlq "github.com/google/trillian/quota/mysql" | ||
|
@@ -39,11 +42,20 @@ var ( | |
rpcEndpoint = flag.String("rpc_endpoint", "localhost:8090", "Endpoint for RPC requests (host:port)") | ||
httpEndpoint = flag.String("http_endpoint", "localhost:8091", "Endpoint for HTTP metrics and REST requests on (host:port, empty means disabled)") | ||
maxUnsequencedRows = flag.Int("max_unsequenced_rows", mysqlq.DefaultMaxUnsequenced, "Max number of unsequenced rows before rate limiting kicks in") | ||
|
||
configFile = flag.String("config", "", "Config file containing flags, file contents can be overridden by command line flags") | ||
) | ||
|
||
func main() { | ||
flag.Parse() | ||
|
||
if *configFile != "" { | ||
if err := cmd.ParseFlagFile(*configFile); err != nil { | ||
fmt.Fprintf(os.Stderr, "Failed to parse %v: %v\n", *configFile, err) | ||
os.Exit(1) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
} | ||
} | ||
|
||
db, err := mysql.OpenDB(*mySQLURI) | ||
if err != nil { | ||
glog.Exitf("Failed to open database: %v", err) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Having looked at what is available within Google, could we swap this for https://bitbucket.org/creachadair/shell instead please? We'll lose the ability to use environment variables in the flag file, which is a bit of a shame, but it saves having to get https://github.com/mattn/go-shellwords reviewed by our security team. I can go down that road if supporting environment variables is important to you though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works for me.