From bd4dda3dadca9e9c82c1430f77c1c94943c335c2 Mon Sep 17 00:00:00 2001 From: Raphael Randschau Date: Mon, 20 Jun 2016 19:37:47 +0200 Subject: [PATCH 1/2] Extract logger --- pkg/api/api.go | 149 ++++++++++++++------------------------------ pkg/api/api_test.go | 2 +- pkg/api/logger.go | 36 +++++++++++ pkg/cli/main.go | 35 ++++++++++- 4 files changed, 116 insertions(+), 106 deletions(-) create mode 100644 pkg/api/logger.go diff --git a/pkg/api/api.go b/pkg/api/api.go index cd6ce22b2f..99ff2fd43b 100644 --- a/pkg/api/api.go +++ b/pkg/api/api.go @@ -23,10 +23,6 @@ import ( "text/tabwriter" "text/template" "time" - - log "github.com/Sirupsen/logrus" - "github.com/moul/anonuuid" - "github.com/moul/http2curl" ) // Default values @@ -68,9 +64,11 @@ type ScalewayAPI struct { // Cache is used to quickly resolve identifiers from names Cache *ScalewayCache - client *http.Client - anonuuid anonuuid.AnonUUID - verbose bool + client *http.Client + verbose bool + + // + Logger Logger } // ScalewayAPIError represents a Scaleway API Error @@ -93,30 +91,31 @@ type ScalewayAPIError struct { // Error returns a string representing the error func (e ScalewayAPIError) Error() string { - if e.Message != "" { - return e.Message - } - if e.APIMessage != "" { - return e.APIMessage - } - if e.StatusCode != 0 { - return fmt.Sprintf("Invalid return code, got %d", e.StatusCode) - } - panic(e) -} - -// Debug create a debug log entry with HTTP error informations -func (e ScalewayAPIError) Debug() { - log.WithFields(log.Fields{ + var b bytes.Buffer + for k, v := range map[string]interface{}{ "StatusCode": e.StatusCode, "Type": e.Type, "Message": e.Message, - }).Debug(e.APIMessage) + "APIMessage": e.APIMessage, + } { + fmt.Fprintf(&b, " %-30s %s", fmt.Sprintf("%s: ", k), v) + } + return b.String() +} - // error.Fields handling - for k, v := range e.Fields { - log.Debugf(" %-30s %s", fmt.Sprintf("%s: ", k), v) +// HideAPICredentials removes API credentials from a string +func (s *ScalewayAPI) HideAPICredentials(input string) string { + output := input + if s.Token != "" { + output = strings.Replace(output, s.Token, "00000000-0000-4000-8000-000000000000", -1) + } + if s.Organization != "" { + output = strings.Replace(output, s.Organization, "00000000-0000-5000-9000-000000000000", -1) + } + if s.password != "" { + output = strings.Replace(output, s.password, "XX-XX-XX-XX", -1) } + return output } // ScalewayIPAddress represents a Scaleway IP address @@ -832,7 +831,7 @@ type MarketImages struct { } // NewScalewayAPI creates a ready-to-use ScalewayAPI client -func NewScalewayAPI(organization, token, userAgent string) (*ScalewayAPI, error) { +func NewScalewayAPI(organization, token, userAgent string, options ...func(*ScalewayAPI)) (*ScalewayAPI, error) { cache, err := NewScalewayCache() if err != nil { return nil, err @@ -842,13 +841,17 @@ func NewScalewayAPI(organization, token, userAgent string) (*ScalewayAPI, error) Organization: organization, Token: token, Cache: cache, + Logger: NewDefaultLogger(), verbose: os.Getenv("SCW_VERBOSE_API") != "", password: "", userAgent: userAgent, // internal - anonuuid: *anonuuid.New(), - client: &http.Client{}, + client: &http.Client{}, + } + + for _, option := range options { + option(s) } if os.Getenv("SCW_TLSVERIFY") == "0" { @@ -882,15 +885,8 @@ func (s *ScalewayAPI) GetResponse(apiURL, resource string) (*http.Response, erro req.Header.Set("Content-Type", "application/json") req.Header.Set("User-Agent", s.userAgent) - curl, err := http2curl.GetCurlCommand(req) - if err != nil { - return nil, err - } - if os.Getenv("SCW_SENSITIVE") != "1" { - log.Debug(s.HideAPICredentials(curl.String())) - } else { - log.Debug(curl.String()) - } + s.Logger.LogHTTP(req) + return s.client.Do(req) } @@ -911,15 +907,7 @@ func (s *ScalewayAPI) PostResponse(apiURL, resource string, data interface{}) (* req.Header.Set("Content-Type", "application/json") req.Header.Set("User-Agent", s.userAgent) - curl, err := http2curl.GetCurlCommand(req) - if err != nil { - return nil, err - } - if os.Getenv("SCW_SENSITIVE") != "1" { - log.Debug(s.HideAPICredentials(curl.String())) - } else { - log.Debug(curl.String()) - } + s.Logger.LogHTTP(req) return s.client.Do(req) } @@ -941,15 +929,7 @@ func (s *ScalewayAPI) PatchResponse(apiURL, resource string, data interface{}) ( req.Header.Set("Content-Type", "application/json") req.Header.Set("User-Agent", s.userAgent) - curl, err := http2curl.GetCurlCommand(req) - if err != nil { - return nil, err - } - if os.Getenv("SCW_SENSITIVE") != "1" { - log.Debug(s.HideAPICredentials(curl.String())) - } else { - log.Debug(curl.String()) - } + s.Logger.LogHTTP(req) return s.client.Do(req) } @@ -971,15 +951,7 @@ func (s *ScalewayAPI) PutResponse(apiURL, resource string, data interface{}) (*h req.Header.Set("Content-Type", "application/json") req.Header.Set("User-Agent", s.userAgent) - curl, err := http2curl.GetCurlCommand(req) - if err != nil { - return nil, err - } - if os.Getenv("SCW_SENSITIVE") != "1" { - log.Debug(s.HideAPICredentials(curl.String())) - } else { - log.Debug(curl.String()) - } + s.Logger.LogHTTP(req) return s.client.Do(req) } @@ -996,15 +968,7 @@ func (s *ScalewayAPI) DeleteResponse(apiURL, resource string) (*http.Response, e req.Header.Set("Content-Type", "application/json") req.Header.Set("User-Agent", s.userAgent) - curl, err := http2curl.GetCurlCommand(req) - if err != nil { - return nil, err - } - if os.Getenv("SCW_SENSITIVE") != "1" { - log.Debug(s.HideAPICredentials(curl.String())) - } else { - log.Debug(curl.String()) - } + s.Logger.LogHTTP(req) return s.client.Do(req) } @@ -1032,7 +996,7 @@ func (s *ScalewayAPI) handleHTTPError(goodStatusCode []int, resp *http.Response) return nil, err } scwError.StatusCode = resp.StatusCode - scwError.Debug() + s.Logger.Log(scwError.Error()) return nil, scwError } if s.verbose { @@ -1040,9 +1004,9 @@ func (s *ScalewayAPI) handleHTTPError(goodStatusCode []int, resp *http.Response) err = json.Indent(&js, body, "", " ") if err != nil { - log.Debug(string(body)) + s.Logger.Log(string(body)) } else { - log.Debug(js.String()) + s.Logger.Log(js.String()) } } return body, nil @@ -1069,12 +1033,11 @@ func (s *ScalewayAPI) GetServers(all bool, limit int) (*[]ScalewayServer, error) return nil, err } - var servers ScalewayServers - body, err := s.handleHTTPError([]int{200}, resp) if err != nil { return nil, err } + var servers ScalewayServers if err = json.Unmarshal(body, &servers); err != nil { return nil, err } @@ -1708,8 +1671,6 @@ func (s *ScalewayUserdata) String() string { // GetUserdata gets a specific userdata for a server func (s *ScalewayAPI) GetUserdata(serverID, key string, metadata bool) (*ScalewayUserdata, error) { - var data ScalewayUserdata - var err error var url, endpoint string endpoint = ComputeAPI @@ -1720,6 +1681,7 @@ func (s *ScalewayAPI) GetUserdata(serverID, key string, metadata bool) (*Scalewa url = fmt.Sprintf("servers/%s/user_data/%s", serverID, key) } + var err error resp, err := s.GetResponse(endpoint, url) if resp != nil { defer resp.Body.Close() @@ -1731,6 +1693,7 @@ func (s *ScalewayAPI) GetUserdata(serverID, key string, metadata bool) (*Scalewa if resp.StatusCode != 200 { return nil, fmt.Errorf("no such user_data %q (%d)", key, resp.StatusCode) } + var data ScalewayUserdata data, err = ioutil.ReadAll(resp.Body) return &data, err } @@ -1760,12 +1723,7 @@ func (s *ScalewayAPI) PatchUserdata(serverID, key string, value []byte, metadata req.Header.Set("Content-Type", "text/plain") req.Header.Set("User-Agent", s.userAgent) - curl, err := http2curl.GetCurlCommand(req) - if os.Getenv("SCW_SENSITIVE") != "1" { - log.Debug(s.HideAPICredentials(curl.String())) - } else { - log.Debug(curl.String()) - } + s.Logger.LogHTTP(req) resp, err := s.client.Do(req) if resp != nil { @@ -2420,7 +2378,7 @@ func (s *ScalewayAPI) GetQuotas() (*ScalewayGetQuotas, error) { // GetBootscriptID returns exactly one bootscript matching func (s *ScalewayAPI) GetBootscriptID(needle, arch string) (string, error) { // Parses optional type prefix, i.e: "bootscript:name" -> "name" - if anonuuid.IsUUID(needle) == nil { + if len(strings.Split(needle, ":")) == 1 { return needle, nil } @@ -2440,21 +2398,6 @@ func (s *ScalewayAPI) GetBootscriptID(needle, arch string) (string, error) { return "", showResolverResults(needle, bootscripts) } -// HideAPICredentials removes API credentials from a string -func (s *ScalewayAPI) HideAPICredentials(input string) string { - output := input - if s.Token != "" { - output = strings.Replace(output, s.Token, s.anonuuid.FakeUUID(s.Token), -1) - } - if s.Organization != "" { - output = strings.Replace(output, s.Organization, s.anonuuid.FakeUUID(s.Organization), -1) - } - if s.password != "" { - output = strings.Replace(output, s.password, "XX-XX-XX-XX", -1) - } - return output -} - func rootNetDial(network, addr string) (net.Conn, error) { dialer := net.Dialer{ Timeout: 10 * time.Second, diff --git a/pkg/api/api_test.go b/pkg/api/api_test.go index b16d648327..de127c9472 100644 --- a/pkg/api/api_test.go +++ b/pkg/api/api_test.go @@ -16,6 +16,6 @@ func TestNewScalewayAPI(t *testing.T) { So(api.Organization, ShouldEqual, "my-organization") So(api.Cache, ShouldNotBeNil) So(api.client, ShouldNotBeNil) - So(api.anonuuid, ShouldNotBeNil) + So(api.Logger, ShouldNotBeNil) }) } diff --git a/pkg/api/logger.go b/pkg/api/logger.go new file mode 100644 index 0000000000..f2b4a9dc4d --- /dev/null +++ b/pkg/api/logger.go @@ -0,0 +1,36 @@ +// Copyright (C) 2015 Scaleway. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE.md file. + +package api + +import ( + "log" + "net/http" + "os" +) + +// Logger handles logging concerns for the Scaleway API SDK +type Logger interface { + LogHTTP(*http.Request) + Log(...interface{}) +} + +// NewDefaultLogger returns a logger which is configured for stdout +func NewDefaultLogger() Logger { + return &defaultLogger{ + logger: log.New(os.Stdout, "", log.LstdFlags), + } +} + +type defaultLogger struct { + logger *log.Logger +} + +func (l defaultLogger) LogHTTP(r *http.Request) { + l.logger.Printf("%s %s", r.Method, r.URL.RawPath) +} + +func (l defaultLogger) Log(args ...interface{}) { + l.logger.Println(args...) +} diff --git a/pkg/cli/main.go b/pkg/cli/main.go index 78e0c40044..b1a330a52d 100644 --- a/pkg/cli/main.go +++ b/pkg/cli/main.go @@ -15,8 +15,9 @@ import ( "github.com/Sirupsen/logrus" flag "github.com/docker/docker/pkg/mflag" - "github.com/hashicorp/go-version" + "github.com/moul/http2curl" + version "github.com/hashicorp/go-version" "github.com/scaleway/scaleway-cli/pkg/api" "github.com/scaleway/scaleway-cli/pkg/commands" "github.com/scaleway/scaleway-cli/pkg/config" @@ -140,7 +141,37 @@ func getScalewayAPI() (*api.ScalewayAPI, error) { if err != nil { return nil, err } - return api.NewScalewayAPI(config.Organization, config.Token, scwversion.UserAgent()) + return api.NewScalewayAPI(config.Organization, config.Token, scwversion.UserAgent(), func(s *api.ScalewayAPI) { + s.Logger = newCliLogger(s) + }) +} + +type cliLogger struct { + logrus.Logger + s *api.ScalewayAPI +} + +func (l *cliLogger) Log(args ...interface{}) { + l.Log(args...) +} + +func (l cliLogger) LogHTTP(req *http.Request) { + curl, err := http2curl.GetCurlCommand(req) + if err != nil { + l.Fatalf("Failed to convert to curl request: %q", err) + } + + if os.Getenv("SCW_SENSITIVE") != "1" { + l.Debug(l.s.HideAPICredentials(curl.String())) + } else { + l.Debug(curl.String()) + } +} + +func newCliLogger(s *api.ScalewayAPI) api.Logger { + return &cliLogger{ + s: s, + } } func initLogging(debug bool, verbose bool, streams *commands.Streams) { From 088ab895a45b93739e232f48a810bcb2f680ddca Mon Sep 17 00:00:00 2001 From: Quentin Perez Date: Tue, 21 Jun 2016 12:26:06 +0200 Subject: [PATCH 2/2] Fix some issues --- pkg/api/api.go | 24 +++++++++++------------- pkg/api/logger.go | 29 +++++++++++++++++++++-------- pkg/cli/main.go | 11 ++++------- 3 files changed, 36 insertions(+), 28 deletions(-) diff --git a/pkg/api/api.go b/pkg/api/api.go index 99ff2fd43b..cd3a81e28a 100644 --- a/pkg/api/api.go +++ b/pkg/api/api.go @@ -68,7 +68,7 @@ type ScalewayAPI struct { verbose bool // - Logger Logger + Logger } // ScalewayAPIError represents a Scaleway API Error @@ -849,7 +849,6 @@ func NewScalewayAPI(organization, token, userAgent string, options ...func(*Scal // internal client: &http.Client{}, } - for _, option := range options { option(s) } @@ -885,7 +884,7 @@ func (s *ScalewayAPI) GetResponse(apiURL, resource string) (*http.Response, erro req.Header.Set("Content-Type", "application/json") req.Header.Set("User-Agent", s.userAgent) - s.Logger.LogHTTP(req) + s.LogHTTP(req) return s.client.Do(req) } @@ -907,7 +906,7 @@ func (s *ScalewayAPI) PostResponse(apiURL, resource string, data interface{}) (* req.Header.Set("Content-Type", "application/json") req.Header.Set("User-Agent", s.userAgent) - s.Logger.LogHTTP(req) + s.LogHTTP(req) return s.client.Do(req) } @@ -929,7 +928,7 @@ func (s *ScalewayAPI) PatchResponse(apiURL, resource string, data interface{}) ( req.Header.Set("Content-Type", "application/json") req.Header.Set("User-Agent", s.userAgent) - s.Logger.LogHTTP(req) + s.LogHTTP(req) return s.client.Do(req) } @@ -951,7 +950,7 @@ func (s *ScalewayAPI) PutResponse(apiURL, resource string, data interface{}) (*h req.Header.Set("Content-Type", "application/json") req.Header.Set("User-Agent", s.userAgent) - s.Logger.LogHTTP(req) + s.LogHTTP(req) return s.client.Do(req) } @@ -968,7 +967,7 @@ func (s *ScalewayAPI) DeleteResponse(apiURL, resource string) (*http.Response, e req.Header.Set("Content-Type", "application/json") req.Header.Set("User-Agent", s.userAgent) - s.Logger.LogHTTP(req) + s.LogHTTP(req) return s.client.Do(req) } @@ -991,12 +990,11 @@ func (s *ScalewayAPI) handleHTTPError(goodStatusCode []int, resp *http.Response) if !good { var scwError ScalewayAPIError - err := json.Unmarshal(body, &scwError) - if err != nil { + if err := json.Unmarshal(body, &scwError); err != nil { return nil, err } scwError.StatusCode = resp.StatusCode - s.Logger.Log(scwError.Error()) + s.Debugf("%s", scwError.Error()) return nil, scwError } if s.verbose { @@ -1004,9 +1002,9 @@ func (s *ScalewayAPI) handleHTTPError(goodStatusCode []int, resp *http.Response) err = json.Indent(&js, body, "", " ") if err != nil { - s.Logger.Log(string(body)) + s.Debugf("%s", string(body)) } else { - s.Logger.Log(js.String()) + s.Debugf("%s", js.String()) } } return body, nil @@ -1723,7 +1721,7 @@ func (s *ScalewayAPI) PatchUserdata(serverID, key string, value []byte, metadata req.Header.Set("Content-Type", "text/plain") req.Header.Set("User-Agent", s.userAgent) - s.Logger.LogHTTP(req) + s.LogHTTP(req) resp, err := s.client.Do(req) if resp != nil { diff --git a/pkg/api/logger.go b/pkg/api/logger.go index f2b4a9dc4d..e6959c3e5a 100644 --- a/pkg/api/logger.go +++ b/pkg/api/logger.go @@ -5,6 +5,7 @@ package api import ( + "fmt" "log" "net/http" "os" @@ -13,24 +14,36 @@ import ( // Logger handles logging concerns for the Scaleway API SDK type Logger interface { LogHTTP(*http.Request) - Log(...interface{}) + Fatalf(format string, v ...interface{}) + Debugf(format string, v ...interface{}) + Infof(format string, v ...interface{}) + Warnf(format string, v ...interface{}) } // NewDefaultLogger returns a logger which is configured for stdout func NewDefaultLogger() Logger { return &defaultLogger{ - logger: log.New(os.Stdout, "", log.LstdFlags), + Logger: log.New(os.Stdout, "", log.LstdFlags), } } type defaultLogger struct { - logger *log.Logger + *log.Logger } -func (l defaultLogger) LogHTTP(r *http.Request) { - l.logger.Printf("%s %s", r.Method, r.URL.RawPath) +func (l *defaultLogger) LogHTTP(r *http.Request) { + l.Printf("%s %s\n", r.Method, r.URL.RawPath) } - -func (l defaultLogger) Log(args ...interface{}) { - l.logger.Println(args...) +func (l *defaultLogger) Fatalf(format string, v ...interface{}) { + l.Printf("[FATAL] %s\n", fmt.Sprintf(format, v)) + os.Exit(1) +} +func (l *defaultLogger) Debugf(format string, v ...interface{}) { + l.Printf("[DEBUG] %s\n", fmt.Sprintf(format, v)) +} +func (l *defaultLogger) Infof(format string, v ...interface{}) { + l.Printf("[INFO ] %s\n", fmt.Sprintf(format, v)) +} +func (l *defaultLogger) Warnf(format string, v ...interface{}) { + l.Printf("[WARN ] %s\n", fmt.Sprintf(format, v)) } diff --git a/pkg/cli/main.go b/pkg/cli/main.go index b1a330a52d..596339887e 100644 --- a/pkg/cli/main.go +++ b/pkg/cli/main.go @@ -147,15 +147,11 @@ func getScalewayAPI() (*api.ScalewayAPI, error) { } type cliLogger struct { - logrus.Logger + *logrus.Logger s *api.ScalewayAPI } -func (l *cliLogger) Log(args ...interface{}) { - l.Log(args...) -} - -func (l cliLogger) LogHTTP(req *http.Request) { +func (l *cliLogger) LogHTTP(req *http.Request) { curl, err := http2curl.GetCurlCommand(req) if err != nil { l.Fatalf("Failed to convert to curl request: %q", err) @@ -170,7 +166,8 @@ func (l cliLogger) LogHTTP(req *http.Request) { func newCliLogger(s *api.ScalewayAPI) api.Logger { return &cliLogger{ - s: s, + Logger: logrus.StandardLogger(), + s: s, } }