From 6240cc38ba9dbbb463ff2fb5d6dae2ab09f94460 Mon Sep 17 00:00:00 2001 From: Cory LaNou Date: Mon, 13 Apr 2015 10:45:39 -0600 Subject: [PATCH 1/9] refactor parse config on startup --- cmd/influxd/main.go | 18 ++++++++-- cmd/influxd/restore.go | 11 +++++-- cmd/influxd/run.go | 74 +++++++++++++++++++----------------------- 3 files changed, 57 insertions(+), 46 deletions(-) diff --git a/cmd/influxd/main.go b/cmd/influxd/main.go index 5a3e124813a..ed0a8e48a03 100644 --- a/cmd/influxd/main.go +++ b/cmd/influxd/main.go @@ -136,15 +136,27 @@ func execConfig(args []string) { } var ( - configPath = fs.String("config", "", "") - hostname = fs.String("hostname", "", "") + configPath string + hostname string ) + fs.StringVar(&configPath, "config", "", "") + fs.StringVar(&hostname, "hostname", "", "") fs.Parse(args) - config, err := parseConfig(*configPath, *hostname) + var config *Config + var err error + if configPath == "" { + config, err = NewTestConfig() + } else { + config, err = ParseConfigFile(configPath) + } if err != nil { log.Fatalf("parse config: %s", err) } + // Override config properties. + if hostname != "" { + config.Hostname = hostname + } config.Write(os.Stdout) } diff --git a/cmd/influxd/restore.go b/cmd/influxd/restore.go index ce3390537d5..00c5547fd9a 100644 --- a/cmd/influxd/restore.go +++ b/cmd/influxd/restore.go @@ -98,11 +98,16 @@ func (cmd *RestoreCommand) parseFlags(args []string) (*Config, string, error) { } // Parse configuration file from disk. - config, err := parseConfig(*configPath, "") + var config *Config + var err error + if *configPath == "" { + config, err = NewTestConfig() + log.Println("No config provided, using default settings") + } else { + config, err = ParseConfig(*configPath) + } if err != nil { log.Fatal(err) - } else if *configPath == "" { - log.Println("No config provided, using default settings") } // Require output path. diff --git a/cmd/influxd/run.go b/cmd/influxd/run.go index 8c982533c7b..b49235249b5 100644 --- a/cmd/influxd/run.go +++ b/cmd/influxd/run.go @@ -28,7 +28,6 @@ import ( type RunCommand struct { // The logger passed to the ticker during execution. - Logger *log.Logger logWriter *os.File config *Config hostname string @@ -160,10 +159,34 @@ func (s *Node) closeSnapshotListener() error { return err } -func (cmd *RunCommand) Run(args ...string) error { - // Set up logger. - cmd.Logger = log.New(os.Stderr, "", log.LstdFlags) +func (cmd *RunCommand) ParseConfig(path, hostname string) error { + var err error + + // Parse configuration file from disk. + if path != "" { + cmd.config, err = ParseConfigFile(path) + + if err != nil { + return fmt.Errorf("error parsing config %s - %s\n", path, err) + } + // Override config properties. + if hostname != "" { + cmd.config.Hostname = hostname + } + + log.Printf("using config: %s\n", path) + return nil + } + cmd.config, err = NewTestConfig() + if err != nil { + return fmt.Errorf("error parsing default config: %s\n", err) + } + log.Println("no config provided, using default settings") + return nil +} + +func (cmd *RunCommand) Run(args ...string) error { // Parse command flags. fs := flag.NewFlagSet("", flag.ExitOnError) var configPath, pidfile, hostname, join, cpuprofile, memprofile string @@ -191,19 +214,9 @@ func (cmd *RunCommand) Run(args ...string) error { runtime.GOMAXPROCS(runtime.NumCPU()) log.Printf("GOMAXPROCS set to %d", runtime.GOMAXPROCS(0)) - var err error - - // Parse configuration file from disk. - if configPath != "" { - cmd.config, err = parseConfig(configPath, hostname) - } else { - cmd.config, err = NewTestConfig() - } - - if err != nil { - cmd.Logger.Fatal(err) - } else if configPath == "" { - cmd.Logger.Println("No config provided, using default settings") + // Parse config + if err := cmd.ParseConfig(configPath, hostname); err != nil { + log.Fatal(err) } // Use the config JoinURLs by default @@ -224,15 +237,15 @@ func (cmd *RunCommand) Run(args ...string) error { // CheckConfig validates the configuration func (cmd *RunCommand) CheckConfig() { if !(cmd.config.Data.Enabled || cmd.config.Broker.Enabled) { - cmd.Logger.Fatal("Node must be configured as a broker node, data node, or as both. Run `influxd config` to generate a valid configuration.") + log.Fatal("Node must be configured as a broker node, data node, or as both. Run `influxd config` to generate a valid configuration.") } if cmd.config.Broker.Enabled && cmd.config.Broker.Dir == "" { - cmd.Logger.Fatal("Broker.Dir must be specified. Run `influxd config` to generate a valid configuration.") + log.Fatal("Broker.Dir must be specified. Run `influxd config` to generate a valid configuration.") } if cmd.config.Data.Enabled && cmd.config.Data.Dir == "" { - cmd.Logger.Fatal("Data.Dir must be specified. Run `influxd config` to generate a valid configuration.") + log.Fatal("Data.Dir must be specified. Run `influxd config` to generate a valid configuration.") } } @@ -274,6 +287,7 @@ func (cmd *RunCommand) Open(config *Config, join string) *Node { //FIXME: Need to also pass in dataURLs to bootstrap a data node s = cmd.openServer(joinURLs) s.SetAuthenticationEnabled(cmd.config.Authentication.Enabled) + log.Printf("authentication enabled: %v\n", cmd.config.Authentication.Enabled) // Enable retention policy enforcement if requested. if cmd.config.Data.RetentionCheckEnabled { @@ -455,26 +469,6 @@ func writePIDFile(path string) { } } -// parseConfig parses the configuration from a given path. Sets overrides as needed. -func parseConfig(path, hostname string) (*Config, error) { - if path == "" { - return NewConfig(), nil - } - - // Parse configuration. - config, err := ParseConfigFile(path) - if err != nil { - return nil, fmt.Errorf("config: %s", err) - } - - // Override config properties. - if hostname != "" { - config.Hostname = hostname - } - - return config, nil -} - // creates and initializes a broker. func (cmd *RunCommand) openBroker(brokerURLs []url.URL) { path := cmd.config.BrokerDir() From 1ae5c09f61ec9e218d1da54b5915d7c2f355f399 Mon Sep 17 00:00:00 2001 From: Cory LaNou Date: Mon, 13 Apr 2015 11:07:45 -0600 Subject: [PATCH 2/9] let the client library handle setting auth --- cmd/influx/main.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cmd/influx/main.go b/cmd/influx/main.go index 654a5888e15..5d316e03f75 100644 --- a/cmd/influx/main.go +++ b/cmd/influx/main.go @@ -205,9 +205,6 @@ func (c *CommandLine) connect(cmd string) { } else { u.Host = c.Host } - if c.Username != "" { - u.User = url.UserPassword(c.Username, c.Password) - } cl, err := client.NewClient( client.Config{ URL: u, @@ -243,6 +240,9 @@ func (c *CommandLine) SetAuth() { return } c.Password = p + + // Update the client as well + c.Client.SetAuth(c.Username, c.Password) } func (c *CommandLine) use(cmd string) { From 4319ac051b0426b9e798c5573f012f96b47a7e79 Mon Sep 17 00:00:00 2001 From: Cory LaNou Date: Mon, 13 Apr 2015 11:08:26 -0600 Subject: [PATCH 3/9] use basic auth instead of the uri --- client/influxdb.go | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/client/influxdb.go b/client/influxdb.go index 8bd48cc89e8..a12cde87110 100644 --- a/client/influxdb.go +++ b/client/influxdb.go @@ -54,14 +54,17 @@ func NewClient(c Config) (*Client, error) { return &client, nil } +// SetAuth will update the username and passwords +func (c *Client) SetAuth(u, p string) { + c.username = u + c.password = p +} + // Query sends a command to the server and returns the Response func (c *Client) Query(q Query) (*Response, error) { u := c.url u.Path = "query" - if c.username != "" { - u.User = url.UserPassword(c.username, c.password) - } values := u.Query() values.Set("q", q.Command) values.Set("db", q.Database) @@ -72,6 +75,10 @@ func (c *Client) Query(q Query) (*Response, error) { return nil, err } req.Header.Set("User-Agent", c.userAgent) + if c.username != "" { + req.SetBasicAuth(c.username, c.password) + } + resp, err := c.httpClient.Do(req) if err != nil { return nil, err From 0ecffa1075c4ac28f9bfdd8f10d6cf2c641970d7 Mon Sep 17 00:00:00 2001 From: Cory LaNou Date: Mon, 13 Apr 2015 11:23:07 -0600 Subject: [PATCH 4/9] update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index aecda3e4b79..510c31d19e5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ - [#2257](https://github.com/influxdb/influxdb/pull/2257): Add "snapshotting" pseudo state & log entry cache. - [#2261](https://github.com/influxdb/influxdb/pull/2261): Support int64 value types. - [#2191](https://github.com/influxdb/influxdb/pull/2191): Case-insensitive check for "fill" +- [#2265](https://github.com/influxdb/influxdb/pull/2265): Fix auth for CLI. ## v0.9.0-rc23 [2015-04-11] From 3ab660fe28d5adf6dbfa8ec7b912bfcba893c7a2 Mon Sep 17 00:00:00 2001 From: Cory LaNou Date: Mon, 13 Apr 2015 14:09:31 -0600 Subject: [PATCH 5/9] fix error message from having invalid format \n --- server.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server.go b/server.go index b1fd4dd82d2..6c4db3b5beb 100644 --- a/server.go +++ b/server.go @@ -3536,7 +3536,7 @@ func (p dataNodes) Swap(i, j int) { p[i], p[j] = p[j], p[i] } // database can be "" for queries that do not require a database. // If u is nil, this means authorization is disabled. func (s *Server) Authorize(u *User, q *influxql.Query, database string) error { - const authErrLogFmt = `unauthorized request | user: %q | query: %q | database %q\n` + const authErrLogFmt = "unauthorized request | user: %q | query: %q | database %q\n" if u == nil { s.Logger.Printf(authErrLogFmt, "", q.String(), database) From a7ae9a529a44eaf5843b80a6bb94151ac264d814 Mon Sep 17 00:00:00 2001 From: Cory LaNou Date: Mon, 13 Apr 2015 15:20:26 -0600 Subject: [PATCH 6/9] need to return an error if status code is not 200 --- client/influxdb.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/client/influxdb.go b/client/influxdb.go index a12cde87110..dbdf2c8fe99 100644 --- a/client/influxdb.go +++ b/client/influxdb.go @@ -85,6 +85,11 @@ func (c *Client) Query(q Query) (*Response, error) { } defer resp.Body.Close() + // If the status code is not 200, this query failed + if resp.StatusCode != http.StatusOK { + return nil, fmt.Errorf("received status code %d from server", resp.StatusCode) + } + var response Response dec := json.NewDecoder(resp.Body) dec.UseNumber() @@ -92,6 +97,7 @@ func (c *Client) Query(q Query) (*Response, error) { if err != nil { return nil, err } + return &response, nil } From fa103d9400f662ac03055fc19977a959f34e5f8e Mon Sep 17 00:00:00 2001 From: Cory LaNou Date: Mon, 13 Apr 2015 15:40:45 -0600 Subject: [PATCH 7/9] clean up error logic for Query endpoint --- client/influxdb.go | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/client/influxdb.go b/client/influxdb.go index dbdf2c8fe99..43ae9e64880 100644 --- a/client/influxdb.go +++ b/client/influxdb.go @@ -85,19 +85,23 @@ func (c *Client) Query(q Query) (*Response, error) { } defer resp.Body.Close() - // If the status code is not 200, this query failed - if resp.StatusCode != http.StatusOK { - return nil, fmt.Errorf("received status code %d from server", resp.StatusCode) - } - var response Response dec := json.NewDecoder(resp.Body) dec.UseNumber() - err = dec.Decode(&response) - if err != nil { - return nil, err - } + decErr := dec.Decode(&response) + // ignore this error if we got an invalid status code + if decErr != nil && decErr.Error() == "EOF" && resp.StatusCode != http.StatusOK { + decErr = nil + } + // If we got a valid decode error, send that back + if decErr != nil { + return nil, decErr + } + // If we don't have an error in our json response, and didn't get statusOK, then send back an error + if resp.StatusCode != http.StatusOK && response.Error() == nil { + return &response, fmt.Errorf("received status code %d from server", resp.StatusCode) + } return &response, nil } From 33f976a340eb0e4d0a5c892093d06d86851cc57f Mon Sep 17 00:00:00 2001 From: Cory LaNou Date: Mon, 13 Apr 2015 15:56:07 -0600 Subject: [PATCH 8/9] lot of refactoring to create integration tests for cli --- cmd/influx/main.go | 52 ++++++++----- cmd/influx/main_test.go | 165 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 198 insertions(+), 19 deletions(-) diff --git a/cmd/influx/main.go b/cmd/influx/main.go index 5d316e03f75..9a53962e9fa 100644 --- a/cmd/influx/main.go +++ b/cmd/influx/main.go @@ -92,7 +92,7 @@ func main() { } if c.Execute != "" { - if err := c.executeQuery(c.Execute); err != nil { + if err := c.ExecuteQuery(c.Execute); err != nil { os.Exit(1) } else { os.Exit(0) @@ -140,13 +140,13 @@ func (c *CommandLine) ParseCommand(cmd string) bool { // signal the program to exit return false case strings.HasPrefix(lcmd, "gopher"): - gopher() + c.gopher() case strings.HasPrefix(lcmd, "connect"): c.connect(cmd) case strings.HasPrefix(lcmd, "auth"): - c.SetAuth() + c.SetAuth(cmd) case strings.HasPrefix(lcmd, "help"): - help() + c.help() case strings.HasPrefix(lcmd, "format"): c.SetFormat(cmd) case strings.HasPrefix(lcmd, "settings"): @@ -163,7 +163,7 @@ func (c *CommandLine) ParseCommand(cmd string) bool { case lcmd == "": break default: - c.executeQuery(cmd) + c.ExecuteQuery(cmd) } return true } @@ -227,19 +227,33 @@ func (c *CommandLine) connect(cmd string) { } } -func (c *CommandLine) SetAuth() { - u, e := c.Line.Prompt("username: ") - if e != nil { - fmt.Printf("Unable to process input: %s", e) - return +func (c *CommandLine) SetAuth(cmd string) { + // If they pass in the entire command, we should parse it + // auth + args := strings.Fields(cmd) + if len(args) == 3 { + args = args[1:] + } else { + args = []string{} } - c.Username = strings.TrimSpace(u) - p, e := c.Line.PasswordPrompt("password: ") - if e != nil { - fmt.Printf("Unable to process input: %s", e) - return + + if len(args) == 2 { + c.Username = args[0] + c.Password = args[1] + } else { + u, e := c.Line.Prompt("username: ") + if e != nil { + fmt.Printf("Unable to process input: %s", e) + return + } + c.Username = strings.TrimSpace(u) + p, e := c.Line.PasswordPrompt("password: ") + if e != nil { + fmt.Printf("Unable to process input: %s", e) + return + } + c.Password = p } - c.Password = p // Update the client as well c.Client.SetAuth(c.Username, c.Password) @@ -286,7 +300,7 @@ func (c *CommandLine) dump() error { return nil } -func (c *CommandLine) executeQuery(query string) error { +func (c *CommandLine) ExecuteQuery(query string) error { response, err := c.Client.Query(client.Query{Command: query, Database: c.Database}) if err != nil { fmt.Printf("ERR: %s\n", err) @@ -473,7 +487,7 @@ func (c *CommandLine) Settings() { w.Flush() } -func help() { +func (c *CommandLine) help() { fmt.Println(`Usage: connect connect to another node auth prompt for username and password @@ -494,7 +508,7 @@ func help() { `) } -func gopher() { +func (c *CommandLine) gopher() { fmt.Println(` .-::-::://:-::- .:/++/' '://:-''/oo+//++o+/.://o- ./+: diff --git a/cmd/influx/main_test.go b/cmd/influx/main_test.go index 5de4cc8dc2d..2fab4f9969a 100644 --- a/cmd/influx/main_test.go +++ b/cmd/influx/main_test.go @@ -1,9 +1,16 @@ package main_test import ( + "fmt" + "io/ioutil" + "net/url" + "os" + "path/filepath" "testing" + "github.com/influxdb/influxdb/client" main "github.com/influxdb/influxdb/cmd/influx" + influxd "github.com/influxdb/influxdb/cmd/influxd" ) func TestParseCommand_CommandsExist(t *testing.T) { @@ -75,3 +82,161 @@ func TestParseCommand_Use(t *testing.T) { } } } + +func TestQuery_NoAuth(t *testing.T) { + if testing.Short() { + t.Skip("skipping TestQuery_NoAuth") + } + + // Create root path to server. + // Remove it to clean up past failed panics + // Defer it to clean up for successful tests + path := tempfile() + os.Remove(path) + defer os.Remove(path) + + config, _ := influxd.NewTestConfig() + + // Start server. + node, err := newNode(config, path) + if err != nil { + t.Fatal(err) + } + + c := main.CommandLine{ + Format: "column", + } + + u := url.URL{ + Scheme: "http", + Host: fmt.Sprintf("%s:%d", "localhost", 8086), + } + cl, err := client.NewClient( + client.Config{ + URL: u, + }) + + if err != nil { + t.Fatal(err) + } + + c.Client = cl + + ok := c.ParseCommand("CREATE USER admin WITH PASSWORD 'password'") + if !ok { + t.Fatal("Failed to create user") + } + node.Close() +} + +func TestQuery_Auth(t *testing.T) { + if testing.Short() { + t.Skip("skipping TestQuery_Auth") + } + + // Create root path to server. + // Remove it to clean up past failed panics + // Defer it to clean up for successful tests + path := tempfile() + os.Remove(path) + defer os.Remove(path) + + // Create the cli + c := main.CommandLine{Format: "column"} + cl, err := client.NewClient( + client.Config{ + URL: url.URL{ + Scheme: "http", + Host: fmt.Sprintf("%s:%d", "localhost", 8086), + }}) + if err != nil { + t.Fatal(err) + } + + c.Client = cl + + // spin up a server + config, _ := influxd.NewTestConfig() + config.Authentication.Enabled = true + node, err := newNode(config, path) + if err != nil { + t.Fatal(err) + } + + // Check to make sure we can't do anything + err = c.ExecuteQuery("CREATE USER admin WITH PASSWORD 'password'") + if err == nil { + t.Fatal("Should have failed to create user") + } + + // spin down server + node.Close() + + // disable auth and spin back up + config.Authentication.Enabled = false + node, err = newNode(config, path) + if err != nil { + t.Fatal(err) + } + + // create the user + err = c.ExecuteQuery("CREATE USER admin WITH PASSWORD 'password'") + if err != nil { + t.Fatalf("Should have created user: %s\n", err) + } + // Make cluster admin + err = c.ExecuteQuery("GRANT ALL PRIVILEGES TO admin") + if err != nil { + t.Fatalf("Should have made cluster admin: %s\n", err) + } + + // spin down again + node.Close() + + // enable auth, spin back up + config.Authentication.Enabled = true + node, err = newNode(config, path) + if err != nil { + t.Fatal(err) + } + + // Check to make sure we still can't do anything + err = c.ExecuteQuery("CREATE USER admin WITH PASSWORD 'password'") + if err == nil { + t.Fatal("Should have failed to create user") + } + + c.SetAuth("auth admin password") + + // Check to make sure we can't do anything + err = c.ExecuteQuery("CREATE DATABASE foo") + if err != nil { + t.Fatalf("Failed to create database: %s\n", err) + } + node.Close() +} + +// tempfile returns a temporary path. +func tempfile() string { + f, _ := ioutil.TempFile("", "influxdb-") + path := f.Name() + f.Close() + os.Remove(path) + return path +} + +func newNode(config *influxd.Config, path string) (*influxd.Node, error) { + config.Broker.Dir = filepath.Join(path, "broker") + config.Data.Dir = filepath.Join(path, "data") + + // Start server. + cmd := influxd.NewRunCommand() + + node := cmd.Open(config, "") + if node.Broker == nil { + return nil, fmt.Errorf("cannot run broker") + } else if node.DataNode == nil { + return nil, fmt.Errorf("cannot run server") + } + return node, nil +} From 96a30389f811430f64e9e63db12fae94903e8be6 Mon Sep 17 00:00:00 2001 From: Cory LaNou Date: Mon, 13 Apr 2015 16:27:43 -0600 Subject: [PATCH 9/9] pr review changes --- cmd/influx/main_test.go | 4 ---- cmd/influxd/run.go | 6 +++--- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/cmd/influx/main_test.go b/cmd/influx/main_test.go index 2fab4f9969a..47ec14f775c 100644 --- a/cmd/influx/main_test.go +++ b/cmd/influx/main_test.go @@ -89,10 +89,8 @@ func TestQuery_NoAuth(t *testing.T) { } // Create root path to server. - // Remove it to clean up past failed panics // Defer it to clean up for successful tests path := tempfile() - os.Remove(path) defer os.Remove(path) config, _ := influxd.NewTestConfig() @@ -135,10 +133,8 @@ func TestQuery_Auth(t *testing.T) { } // Create root path to server. - // Remove it to clean up past failed panics // Defer it to clean up for successful tests path := tempfile() - os.Remove(path) defer os.Remove(path) // Create the cli diff --git a/cmd/influxd/run.go b/cmd/influxd/run.go index b49235249b5..363810dfd7a 100644 --- a/cmd/influxd/run.go +++ b/cmd/influxd/run.go @@ -167,14 +167,14 @@ func (cmd *RunCommand) ParseConfig(path, hostname string) error { cmd.config, err = ParseConfigFile(path) if err != nil { - return fmt.Errorf("error parsing config %s - %s\n", path, err) + return fmt.Errorf("error parsing configuration %s - %s\n", path, err) } // Override config properties. if hostname != "" { cmd.config.Hostname = hostname } - log.Printf("using config: %s\n", path) + log.Printf("using configuration at: %s\n", path) return nil } @@ -182,7 +182,7 @@ func (cmd *RunCommand) ParseConfig(path, hostname string) error { if err != nil { return fmt.Errorf("error parsing default config: %s\n", err) } - log.Println("no config provided, using default settings") + log.Println("no configuration provided, using default settings") return nil }