From 3e00a37ef5ce2b55b9330c724460e70e6c0acb6b Mon Sep 17 00:00:00 2001 From: Camilo Aguilar Date: Wed, 21 May 2014 13:13:20 -0400 Subject: [PATCH] feat(util/http_client): Adds generic HTTP client Supports retries with exponential backoff as well as connection timeouts and the ability to skip SSL/TLS verification. This commit also refactors datasource and initialize packages in order to use the new HTTP client. --- datasource/datasource.go | 98 -------------- datasource/metadata_service.go | 5 +- datasource/proc_cmdline.go | 5 +- initialize/ssh_keys.go | 18 +-- test | 2 +- util/http_client.go | 122 ++++++++++++++++++ .../http_client_test.go | 26 ++-- 7 files changed, 154 insertions(+), 122 deletions(-) create mode 100644 util/http_client.go rename datasource/datasource_test.go => util/http_client_test.go (81%) diff --git a/datasource/datasource.go b/datasource/datasource.go index 41b72c5..e3d2834 100644 --- a/datasource/datasource.go +++ b/datasource/datasource.go @@ -1,104 +1,6 @@ package datasource -import ( - "errors" - "fmt" - "io/ioutil" - "log" - "math" - "net" - "net/http" - neturl "net/url" - "strings" - "time" -) - -const ( - HTTP_2xx = 2 - HTTP_4xx = 4 - - maxTimeout = time.Second * 5 - maxRetries = 15 -) - type Datasource interface { Fetch() ([]byte, error) Type() string } - -// HTTP client timeout -// This one is low since exponential backoff will kick off too. -var timeout = time.Duration(2) * time.Second - -func dialTimeout(network, addr string) (net.Conn, error) { - deadline := time.Now().Add(timeout) - c, err := net.DialTimeout(network, addr, timeout) - if err != nil { - return nil, err - } - c.SetDeadline(deadline) - return c, nil -} - -// Fetches user-data url with support for exponential backoff and maximum retries -func fetchURL(rawurl string) ([]byte, error) { - if rawurl == "" { - return nil, errors.New("user-data URL is empty. Skipping.") - } - - url, err := neturl.Parse(rawurl) - if err != nil { - return nil, err - } - - // Unfortunately, url.Parse is too generic to throw errors if a URL does not - // have a valid HTTP scheme. So, we have to do this extra validation - if !strings.HasPrefix(url.Scheme, "http") { - return nil, fmt.Errorf("user-data URL %s does not have a valid HTTP scheme. Skipping.", rawurl) - } - - userdataURL := url.String() - - // We need to create our own client in order to add timeout support. - // TODO(c4milo) Replace it once Go 1.3 is officially used by CoreOS - // More info: https://code.google.com/p/go/source/detail?r=ada6f2d5f99f - transport := &http.Transport{ - Dial: dialTimeout, - } - - client := &http.Client{ - Transport: transport, - } - - for retry := 1; retry <= maxRetries; retry++ { - log.Printf("Fetching user-data from %s. Attempt #%d", userdataURL, retry) - - resp, err := client.Get(userdataURL) - - if err == nil { - defer resp.Body.Close() - status := resp.StatusCode / 100 - - if status == HTTP_2xx { - return ioutil.ReadAll(resp.Body) - } - - if status == HTTP_4xx { - return nil, fmt.Errorf("user-data not found. HTTP status code: %d", resp.StatusCode) - } - - log.Printf("user-data not found. HTTP status code: %d", resp.StatusCode) - } else { - log.Printf("unable to fetch user-data: %s", err.Error()) - } - - duration := time.Millisecond * time.Duration((math.Pow(float64(2), float64(retry)) * 100)) - if duration > maxTimeout { - duration = maxTimeout - } - - time.Sleep(duration) - } - - return nil, fmt.Errorf("unable to fetch user-data. Maximum retries reached: %d", maxRetries) -} diff --git a/datasource/metadata_service.go b/datasource/metadata_service.go index e1376b0..6fe1c59 100644 --- a/datasource/metadata_service.go +++ b/datasource/metadata_service.go @@ -1,5 +1,7 @@ package datasource +import "github.com/coreos/coreos-cloudinit/util" + type metadataService struct { url string } @@ -9,7 +11,8 @@ func NewMetadataService(url string) *metadataService { } func (ms *metadataService) Fetch() ([]byte, error) { - return fetchURL(ms.url) + client := util.NewHttpClient() + return client.Get(ms.url) } func (ms *metadataService) Type() string { diff --git a/datasource/proc_cmdline.go b/datasource/proc_cmdline.go index 4f38fcf..c14d43f 100644 --- a/datasource/proc_cmdline.go +++ b/datasource/proc_cmdline.go @@ -5,6 +5,8 @@ import ( "io/ioutil" "log" "strings" + + "github.com/coreos/coreos-cloudinit/util" ) const ( @@ -29,7 +31,8 @@ func (self *procCmdline) Fetch() ([]byte, error) { return nil, err } - cfg, err := fetchURL(url) + client := util.NewHttpClient() + cfg, err := client.Get(url) if err != nil { return nil, err } diff --git a/initialize/ssh_keys.go b/initialize/ssh_keys.go index 954e1a9..7059c24 100644 --- a/initialize/ssh_keys.go +++ b/initialize/ssh_keys.go @@ -3,10 +3,9 @@ package initialize import ( "encoding/json" "fmt" - "io/ioutil" - "net/http" "github.com/coreos/coreos-cloudinit/system" + "github.com/coreos/coreos-cloudinit/util" ) type UserKey struct { @@ -25,22 +24,19 @@ func SSHImportKeysFromURL(system_user string, url string) error { } func fetchUserKeys(url string) ([]string, error) { - res, err := http.Get(url) + client := util.NewHttpClient() + data, err := client.Get(url) if err != nil { return nil, err } - defer res.Body.Close() - body, err := ioutil.ReadAll(res.Body) - if err != nil { - return nil, err - } - var data []UserKey - err = json.Unmarshal(body, &data) + + var userKeys []UserKey + err = json.Unmarshal(data, &userKeys) if err != nil { return nil, err } keys := make([]string, 0) - for _, key := range data { + for _, key := range userKeys { keys = append(keys, key.Key) } return keys, err diff --git a/test b/test index 8587357..41abb37 100755 --- a/test +++ b/test @@ -13,7 +13,7 @@ COVER=${COVER:-"-cover"} source ./build -declare -a TESTPKGS=(initialize system datasource) +declare -a TESTPKGS=(initialize system datasource util) if [ -z "$PKG" ]; then GOFMTPATH="$TESTPKGS coreos-cloudinit.go" diff --git a/util/http_client.go b/util/http_client.go new file mode 100644 index 0000000..1c9ec70 --- /dev/null +++ b/util/http_client.go @@ -0,0 +1,122 @@ +package util + +import ( + "crypto/tls" + "errors" + "fmt" + "io/ioutil" + "log" + "math" + "net" + "net/http" + neturl "net/url" + "strings" + "time" +) + +const ( + HTTP_2xx = 2 + HTTP_4xx = 4 +) + +type HttpClient struct { + // Maximum exp backoff duration. Defaults to 5 seconds + MaxBackoff time.Duration + + // Maximum amount of connection retries. Defaults to 15 + MaxRetries int + + // HTTP client timeout, this is suggested to be low since exponential + // backoff will kick off too. Defaults to 2 seconds + Timeout time.Duration + + //Whether or not to skip TLS verification. Defaults to false + SkipTLS bool +} + +func NewHttpClient() *HttpClient { + return &HttpClient{ + MaxBackoff: time.Second * 5, + MaxRetries: 15, + Timeout: time.Duration(2) * time.Second, + SkipTLS: false, + } +} + +// Fetches a given URL with support for exponential backoff and maximum retries +func (h *HttpClient) Get(rawurl string) ([]byte, error) { + if h == nil { + return nil, nil + } + + if rawurl == "" { + return nil, errors.New("URL is empty. Skipping.") + } + + url, err := neturl.Parse(rawurl) + if err != nil { + return nil, err + } + + // Unfortunately, url.Parse is too generic to throw errors if a URL does not + // have a valid HTTP scheme. So, we have to do this extra validation + if !strings.HasPrefix(url.Scheme, "http") { + return nil, fmt.Errorf("URL %s does not have a valid HTTP scheme. Skipping.", rawurl) + } + + dataURL := url.String() + + // We need to create our own client in order to add timeout support. + // TODO(c4milo) Replace it once Go 1.3 is officially used by CoreOS + // More info: https://code.google.com/p/go/source/detail?r=ada6f2d5f99f + transport := &http.Transport{ + TLSClientConfig: &tls.Config{ + InsecureSkipVerify: h.SkipTLS, + }, + Dial: func(network, addr string) (net.Conn, error) { + deadline := time.Now().Add(h.Timeout) + c, err := net.DialTimeout(network, addr, h.Timeout) + if err != nil { + return nil, err + } + c.SetDeadline(deadline) + return c, nil + }, + } + + client := &http.Client{ + Transport: transport, + } + + for retry := 1; retry <= h.MaxRetries; retry++ { + log.Printf("Fetching data from %s. Attempt #%d", dataURL, retry) + + resp, err := client.Get(dataURL) + + if err == nil { + defer resp.Body.Close() + status := resp.StatusCode / 100 + + if status == HTTP_2xx { + return ioutil.ReadAll(resp.Body) + } + + if status == HTTP_4xx { + return nil, fmt.Errorf("Not found. HTTP status code: %d", resp.StatusCode) + } + + log.Printf("Server error. HTTP status code: %d", resp.StatusCode) + } else { + log.Printf("Unable to fetch data: %s", err.Error()) + } + + duration := time.Millisecond * time.Duration((math.Pow(float64(2), float64(retry)) * 100)) + if duration > h.MaxBackoff { + duration = h.MaxBackoff + } + + time.Sleep(duration) + } + + return nil, fmt.Errorf("Unable to fetch data. Maximum retries reached: %d", h.MaxRetries) +} diff --git a/datasource/datasource_test.go b/util/http_client_test.go similarity index 81% rename from datasource/datasource_test.go rename to util/http_client_test.go index 76192c1..5243407 100644 --- a/datasource/datasource_test.go +++ b/util/http_client_test.go @@ -1,4 +1,4 @@ -package datasource +package util import ( "fmt" @@ -20,6 +20,8 @@ var expBackoffTests = []struct { // Test exponential backoff and that it continues retrying if a 5xx response is // received func TestFetchURLExpBackOff(t *testing.T) { + client := NewHttpClient() + for i, tt := range expBackoffTests { mux := http.NewServeMux() count := 0 @@ -34,7 +36,7 @@ func TestFetchURLExpBackOff(t *testing.T) { ts := httptest.NewServer(mux) defer ts.Close() - data, err := fetchURL(ts.URL) + data, err := client.Get(ts.URL) if err != nil { t.Errorf("Test case %d produced error: %v", i, err) } @@ -51,6 +53,7 @@ func TestFetchURLExpBackOff(t *testing.T) { // Test that it stops retrying if a 4xx response comes back func TestFetchURL4xx(t *testing.T) { + client := NewHttpClient() retries := 0 ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { retries++ @@ -58,9 +61,9 @@ func TestFetchURL4xx(t *testing.T) { })) defer ts.Close() - _, err := fetchURL(ts.URL) + _, err := client.Get(ts.URL) if err == nil { - t.Errorf("Incorrect result\ngot: %s\nwant: %s", err.Error(), "user-data not found. HTTP status code: 404") + t.Errorf("Incorrect result\ngot: %s\nwant: %s", err.Error(), "Not found. HTTP status code: 404") } if retries > 1 { @@ -83,12 +86,13 @@ coreos: reboot-strategy: best-effort ` + client := NewHttpClient() ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { fmt.Fprint(w, cloudcfg) })) defer ts.Close() - data, err := fetchURL(ts.URL) + data, err := client.Get(ts.URL) if err != nil { t.Errorf("Incorrect result\ngot: %v\nwant: %v", err, nil) } @@ -100,18 +104,20 @@ coreos: // Test attempt to fetching using malformed URL func TestFetchURLMalformed(t *testing.T) { + client := NewHttpClient() + var tests = []struct { url string want string }{ - {"boo", "user-data URL boo does not have a valid HTTP scheme. Skipping."}, - {"mailto://boo", "user-data URL mailto://boo does not have a valid HTTP scheme. Skipping."}, - {"ftp://boo", "user-data URL ftp://boo does not have a valid HTTP scheme. Skipping."}, - {"", "user-data URL is empty. Skipping."}, + {"boo", "URL boo does not have a valid HTTP scheme. Skipping."}, + {"mailto://boo", "URL mailto://boo does not have a valid HTTP scheme. Skipping."}, + {"ftp://boo", "URL ftp://boo does not have a valid HTTP scheme. Skipping."}, + {"", "URL is empty. Skipping."}, } for _, test := range tests { - _, err := fetchURL(test.url) + _, err := client.Get(test.url) if err == nil || err.Error() != test.want { t.Errorf("Incorrect result\ngot: %v\nwant: %v", err, test.want) }