The following pull request was submitted through Github. It can be accessed and reviewed at: https://github.com/lxc/lxd/pull/3672
This e-mail was sent by the LXC bot, direct replies will not reach the author unless they happen to be subscribed to this list. === Description (from pull-request) === This branch moves the Daemon.httpClient method (which really depends only on the Daemon.proxy attribute) into a standalone function. The goal is to progressively streamline the Daemon structure and factor out ancillary code, to eventually be able to improve unit-testability of both Deamon itself and its main consumer, the api*.go files. While at it, the util.go file has also been moved to the util/ package, and split into two (encoding.go, and kernel.go). Signed-off-by: Free Ekanayaka <[email protected]>
From 1fe9822d360185d80244c513eebac8443a5aa93a Mon Sep 17 00:00:00 2001 From: Free Ekanayaka <[email protected]> Date: Wed, 16 Aug 2017 22:39:26 +0000 Subject: [PATCH] Extract Daemon.httpClient into a standalone HTTPClient function This branch moves the Daemon.httpClient method (which really depends only on the Daemon.proxy attribute) into a standalone function. The goal is to progressively streamline the Daemon structure and factor out ancillary code, to eventually be able to improve unit-testability of both Deamon itself and its main consumer, the api*.go files. While at it, the util.go file has also been moved to the util/ package, and split into two (encoding.go, and kernel.go). Signed-off-by: Free Ekanayaka <[email protected]> --- lxd/api_1.0.go | 5 ++-- lxd/certificates.go | 5 ++-- lxd/container_instance_types.go | 3 +- lxd/container_lxc.go | 5 ++-- lxd/container_patch.go | 3 +- lxd/container_put.go | 3 +- lxd/daemon.go | 44 ----------------------------- lxd/daemon_images.go | 3 +- lxd/devlxd.go | 3 +- lxd/images.go | 11 ++++---- lxd/networks.go | 5 ++-- lxd/profiles.go | 5 ++-- lxd/response.go | 7 +++-- lxd/storage_pools.go | 5 ++-- lxd/storage_volumes.go | 5 ++-- lxd/storage_zfs.go | 3 +- lxd/{util.go => util/encoding.go} | 19 ++++--------- lxd/util/http.go | 58 +++++++++++++++++++++++++++++++++++++++ lxd/util/kernel.go | 18 ++++++++++++ 19 files changed, 124 insertions(+), 86 deletions(-) rename lxd/{util.go => util/encoding.go} (64%) create mode 100644 lxd/util/http.go create mode 100644 lxd/util/kernel.go diff --git a/lxd/api_1.0.go b/lxd/api_1.0.go index 58668a089..b29ccf148 100644 --- a/lxd/api_1.0.go +++ b/lxd/api_1.0.go @@ -11,6 +11,7 @@ import ( "gopkg.in/lxc/go-lxc.v2" "github.com/lxc/lxd/lxd/db" + "github.com/lxc/lxd/lxd/util" "github.com/lxc/lxd/shared" "github.com/lxc/lxd/shared/api" "github.com/lxc/lxd/shared/osarch" @@ -248,7 +249,7 @@ func api10Put(d *Daemon, r *http.Request) Response { return SmartError(err) } - err = etagCheck(r, daemonConfigRender()) + err = util.EtagCheck(r, daemonConfigRender()) if err != nil { return PreconditionFailed(err) } @@ -267,7 +268,7 @@ func api10Patch(d *Daemon, r *http.Request) Response { return SmartError(err) } - err = etagCheck(r, daemonConfigRender()) + err = util.EtagCheck(r, daemonConfigRender()) if err != nil { return PreconditionFailed(err) } diff --git a/lxd/certificates.go b/lxd/certificates.go index 008c93c27..d15f0fb53 100644 --- a/lxd/certificates.go +++ b/lxd/certificates.go @@ -12,6 +12,7 @@ import ( "github.com/gorilla/mux" "github.com/lxc/lxd/lxd/db" + "github.com/lxc/lxd/lxd/util" "github.com/lxc/lxd/shared" "github.com/lxc/lxd/shared/api" "github.com/lxc/lxd/shared/logger" @@ -193,7 +194,7 @@ func certificateFingerprintPut(d *Daemon, r *http.Request) Response { } fingerprint = oldEntry.Fingerprint - err = etagCheck(r, oldEntry) + err = util.EtagCheck(r, oldEntry) if err != nil { return PreconditionFailed(err) } @@ -215,7 +216,7 @@ func certificateFingerprintPatch(d *Daemon, r *http.Request) Response { } fingerprint = oldEntry.Fingerprint - err = etagCheck(r, oldEntry) + err = util.EtagCheck(r, oldEntry) if err != nil { return PreconditionFailed(err) } diff --git a/lxd/container_instance_types.go b/lxd/container_instance_types.go index 77259e3bf..c5225b7f8 100644 --- a/lxd/container_instance_types.go +++ b/lxd/container_instance_types.go @@ -9,6 +9,7 @@ import ( "gopkg.in/yaml.v2" + "github.com/lxc/lxd/lxd/util" "github.com/lxc/lxd/shared" "github.com/lxc/lxd/shared/logger" "github.com/lxc/lxd/shared/version" @@ -67,7 +68,7 @@ func instanceRefreshTypes(d *Daemon) error { downloadParse := func(filename string, target interface{}) error { url := fmt.Sprintf("https://images.linuxcontainers.org/meta/instance-types/%s";, filename) - httpClient, err := d.httpClient("") + httpClient, err := util.HTTPClient("", d.proxy) if err != nil { return err } diff --git a/lxd/container_lxc.go b/lxd/container_lxc.go index 6e21c0be5..edac0c2e3 100644 --- a/lxd/container_lxc.go +++ b/lxd/container_lxc.go @@ -26,6 +26,7 @@ import ( "github.com/lxc/lxd/lxd/db" "github.com/lxc/lxd/lxd/types" + "github.com/lxc/lxd/lxd/util" "github.com/lxc/lxd/shared" "github.com/lxc/lxd/shared/api" "github.com/lxc/lxd/shared/logger" @@ -1627,7 +1628,7 @@ func (c *containerLXC) startCommon() (string, error) { if kernelModules != "" { for _, module := range strings.Split(kernelModules, ",") { module = strings.TrimPrefix(module, " ") - err := loadModule(module) + err := util.LoadModule(module) if err != nil { return "", fmt.Errorf("Failed to load kernel module '%s': %s", module, err) } @@ -3487,7 +3488,7 @@ func (c *containerLXC) Update(args db.ContainerArgs, userRequested bool) error { } else if key == "linux.kernel_modules" && value != "" { for _, module := range strings.Split(value, ",") { module = strings.TrimPrefix(module, " ") - err := loadModule(module) + err := util.LoadModule(module) if err != nil { return fmt.Errorf("Failed to load kernel module '%s': %s", module, err) } diff --git a/lxd/container_patch.go b/lxd/container_patch.go index 02fd3a0aa..3d042163d 100644 --- a/lxd/container_patch.go +++ b/lxd/container_patch.go @@ -10,6 +10,7 @@ import ( "github.com/gorilla/mux" "github.com/lxc/lxd/lxd/db" + "github.com/lxc/lxd/lxd/util" "github.com/lxc/lxd/shared" "github.com/lxc/lxd/shared/api" "github.com/lxc/lxd/shared/osarch" @@ -25,7 +26,7 @@ func containerPatch(d *Daemon, r *http.Request) Response { // Validate the ETag etag := []interface{}{c.Architecture(), c.LocalConfig(), c.LocalDevices(), c.IsEphemeral(), c.Profiles()} - err = etagCheck(r, etag) + err = util.EtagCheck(r, etag) if err != nil { return PreconditionFailed(err) } diff --git a/lxd/container_put.go b/lxd/container_put.go index 84bc7b99f..351cce765 100644 --- a/lxd/container_put.go +++ b/lxd/container_put.go @@ -9,6 +9,7 @@ import ( "github.com/gorilla/mux" "github.com/lxc/lxd/lxd/db" + "github.com/lxc/lxd/lxd/util" "github.com/lxc/lxd/shared" "github.com/lxc/lxd/shared/api" "github.com/lxc/lxd/shared/osarch" @@ -28,7 +29,7 @@ func containerPut(d *Daemon, r *http.Request) Response { // Validate the ETag etag := []interface{}{c.Architecture(), c.LocalConfig(), c.LocalDevices(), c.IsEphemeral(), c.Profiles()} - err = etagCheck(r, etag) + err = util.EtagCheck(r, etag) if err != nil { return PreconditionFailed(err) } diff --git a/lxd/daemon.go b/lxd/daemon.go index 186985219..bc728d098 100644 --- a/lxd/daemon.go +++ b/lxd/daemon.go @@ -6,7 +6,6 @@ import ( "crypto/x509" "database/sql" "encoding/hex" - "encoding/pem" "fmt" "io" "io/ioutil" @@ -107,49 +106,6 @@ type Command struct { patch func(d *Daemon, r *http.Request) Response } -func (d *Daemon) httpClient(certificate string) (*http.Client, error) { - var err error - var cert *x509.Certificate - - if certificate != "" { - certBlock, _ := pem.Decode([]byte(certificate)) - if certBlock == nil { - return nil, fmt.Errorf("Invalid certificate") - } - - cert, err = x509.ParseCertificate(certBlock.Bytes) - if err != nil { - return nil, err - } - } - - tlsConfig, err := shared.GetTLSConfig("", "", "", cert) - if err != nil { - return nil, err - } - - tr := &http.Transport{ - TLSClientConfig: tlsConfig, - Dial: shared.RFC3493Dialer, - Proxy: d.proxy, - DisableKeepAlives: true, - } - - myhttp := http.Client{ - Transport: tr, - } - - // Setup redirect policy - myhttp.CheckRedirect = func(req *http.Request, via []*http.Request) error { - // Replicate the headers - req.Header = via[len(via)-1].Header - - return nil - } - - return &myhttp, nil -} - func readMyCert() (string, string, error) { certf := shared.VarPath("server.crt") keyf := shared.VarPath("server.key") diff --git a/lxd/daemon_images.go b/lxd/daemon_images.go index 659bb0d1a..145e45ed5 100644 --- a/lxd/daemon_images.go +++ b/lxd/daemon_images.go @@ -16,6 +16,7 @@ import ( "github.com/lxc/lxd/client" "github.com/lxc/lxd/lxd/db" + "github.com/lxc/lxd/lxd/util" "github.com/lxc/lxd/shared" "github.com/lxc/lxd/shared/api" "github.com/lxc/lxd/shared/cancel" @@ -423,7 +424,7 @@ func (d *Daemon) ImageDownload(op *operation, server string, protocol string, ce } } else if protocol == "direct" { // Setup HTTP client - httpClient, err := d.httpClient(certificate) + httpClient, err := util.HTTPClient(certificate, d.proxy) if err != nil { return nil, err } diff --git a/lxd/devlxd.go b/lxd/devlxd.go index 5dd17b454..68f2ca518 100644 --- a/lxd/devlxd.go +++ b/lxd/devlxd.go @@ -16,6 +16,7 @@ import ( "github.com/gorilla/mux" "github.com/lxc/lxd/lxd/db" + "github.com/lxc/lxd/lxd/util" "github.com/lxc/lxd/shared" "github.com/lxc/lxd/shared/logger" "github.com/lxc/lxd/shared/version" @@ -119,7 +120,7 @@ func hoistReq(f func(container, *http.Request) *devLxdResponse, d *Daemon) func( http.Error(w, fmt.Sprintf("%s", resp.content), resp.code) } else if resp.ctype == "json" { w.Header().Set("Content-Type", "application/json") - WriteJSON(w, resp.content) + util.WriteJSON(w, resp.content, debug) } else { w.Header().Set("Content-Type", "application/octet-stream") fmt.Fprintf(w, resp.content.(string)) diff --git a/lxd/images.go b/lxd/images.go index ce016c2cc..c9dc99ae7 100644 --- a/lxd/images.go +++ b/lxd/images.go @@ -23,6 +23,7 @@ import ( "gopkg.in/yaml.v2" "github.com/lxc/lxd/lxd/db" + "github.com/lxc/lxd/lxd/util" "github.com/lxc/lxd/shared" "github.com/lxc/lxd/shared/api" "github.com/lxc/lxd/shared/logger" @@ -358,7 +359,7 @@ func imgPostURLInfo(d *Daemon, req api.ImagesPost, op *operation) (*api.Image, e return nil, fmt.Errorf("Missing URL") } - myhttp, err := d.httpClient("") + myhttp, err := util.HTTPClient("", d.proxy) if err != nil { return nil, err } @@ -1207,7 +1208,7 @@ func imagePut(d *Daemon, r *http.Request) Response { // Validate ETag etag := []interface{}{info.Public, info.AutoUpdate, info.Properties} - err = etagCheck(r, etag) + err = util.EtagCheck(r, etag) if err != nil { return PreconditionFailed(err) } @@ -1235,7 +1236,7 @@ func imagePatch(d *Daemon, r *http.Request) Response { // Validate ETag etag := []interface{}{info.Public, info.AutoUpdate, info.Properties} - err = etagCheck(r, etag) + err = util.EtagCheck(r, etag) if err != nil { return PreconditionFailed(err) } @@ -1392,7 +1393,7 @@ func aliasPut(d *Daemon, r *http.Request) Response { } // Validate ETag - err = etagCheck(r, alias) + err = util.EtagCheck(r, alias) if err != nil { return PreconditionFailed(err) } @@ -1428,7 +1429,7 @@ func aliasPatch(d *Daemon, r *http.Request) Response { } // Validate ETag - err = etagCheck(r, alias) + err = util.EtagCheck(r, alias) if err != nil { return PreconditionFailed(err) } diff --git a/lxd/networks.go b/lxd/networks.go index a567de196..6f81691b3 100644 --- a/lxd/networks.go +++ b/lxd/networks.go @@ -15,6 +15,7 @@ import ( log "gopkg.in/inconshreveable/log15.v2" "github.com/lxc/lxd/lxd/db" + "github.com/lxc/lxd/lxd/util" "github.com/lxc/lxd/shared" "github.com/lxc/lxd/shared/api" "github.com/lxc/lxd/shared/logger" @@ -305,7 +306,7 @@ func networkPut(d *Daemon, r *http.Request) Response { // Validate the ETag etag := []interface{}{dbInfo.Name, dbInfo.Managed, dbInfo.Type, dbInfo.Description, dbInfo.Config} - err = etagCheck(r, etag) + err = util.EtagCheck(r, etag) if err != nil { return PreconditionFailed(err) } @@ -330,7 +331,7 @@ func networkPatch(d *Daemon, r *http.Request) Response { // Validate the ETag etag := []interface{}{dbInfo.Name, dbInfo.Managed, dbInfo.Type, dbInfo.Description, dbInfo.Config} - err = etagCheck(r, etag) + err = util.EtagCheck(r, etag) if err != nil { return PreconditionFailed(err) } diff --git a/lxd/profiles.go b/lxd/profiles.go index 9c94633fb..57e6641f9 100644 --- a/lxd/profiles.go +++ b/lxd/profiles.go @@ -12,6 +12,7 @@ import ( _ "github.com/mattn/go-sqlite3" "github.com/lxc/lxd/lxd/db" + "github.com/lxc/lxd/lxd/util" "github.com/lxc/lxd/shared" "github.com/lxc/lxd/shared/api" "github.com/lxc/lxd/shared/logger" @@ -165,7 +166,7 @@ func profilePut(d *Daemon, r *http.Request) Response { // Validate the ETag etag := []interface{}{profile.Config, profile.Description, profile.Devices} - err = etagCheck(r, etag) + err = util.EtagCheck(r, etag) if err != nil { return PreconditionFailed(err) } @@ -188,7 +189,7 @@ func profilePatch(d *Daemon, r *http.Request) Response { // Validate the ETag etag := []interface{}{profile.Config, profile.Description, profile.Devices} - err = etagCheck(r, etag) + err = util.EtagCheck(r, etag) if err != nil { return PreconditionFailed(err) } diff --git a/lxd/response.go b/lxd/response.go index de821a655..41629738e 100644 --- a/lxd/response.go +++ b/lxd/response.go @@ -14,6 +14,7 @@ import ( "github.com/mattn/go-sqlite3" "github.com/lxc/lxd/lxd/db" + "github.com/lxc/lxd/lxd/util" "github.com/lxc/lxd/shared" "github.com/lxc/lxd/shared/api" ) @@ -35,7 +36,7 @@ type syncResponse struct { func (r *syncResponse) Render(w http.ResponseWriter) error { // Set an appropriate ETag header if r.etag != nil { - etag, err := etagHash(r.etag) + etag, err := util.EtagHash(r.etag) if err == nil { w.Header().Set("ETag", etag) } @@ -66,7 +67,7 @@ func (r *syncResponse) Render(w http.ResponseWriter) error { Metadata: r.metadata, } - return WriteJSON(w, resp) + return util.WriteJSON(w, resp, debug) } func (r *syncResponse) String() string { @@ -237,7 +238,7 @@ func (r *operationResponse) Render(w http.ResponseWriter) error { w.Header().Set("Location", url) w.WriteHeader(202) - return WriteJSON(w, body) + return util.WriteJSON(w, body, debug) } func (r *operationResponse) String() string { diff --git a/lxd/storage_pools.go b/lxd/storage_pools.go index 6a89f3ebb..a616566a5 100644 --- a/lxd/storage_pools.go +++ b/lxd/storage_pools.go @@ -9,6 +9,7 @@ import ( "github.com/gorilla/mux" "github.com/lxc/lxd/lxd/db" + "github.com/lxc/lxd/lxd/util" "github.com/lxc/lxd/shared/api" "github.com/lxc/lxd/shared/version" ) @@ -123,7 +124,7 @@ func storagePoolPut(d *Daemon, r *http.Request) Response { // Validate the ETag etag := []interface{}{dbInfo.Name, dbInfo.Driver, dbInfo.Config} - err = etagCheck(r, etag) + err = util.EtagCheck(r, etag) if err != nil { return PreconditionFailed(err) } @@ -161,7 +162,7 @@ func storagePoolPatch(d *Daemon, r *http.Request) Response { // Validate the ETag etag := []interface{}{dbInfo.Name, dbInfo.Driver, dbInfo.Config} - err = etagCheck(r, etag) + err = util.EtagCheck(r, etag) if err != nil { return PreconditionFailed(err) } diff --git a/lxd/storage_volumes.go b/lxd/storage_volumes.go index 22c1f6497..556b3450e 100644 --- a/lxd/storage_volumes.go +++ b/lxd/storage_volumes.go @@ -8,6 +8,7 @@ import ( "github.com/gorilla/mux" "github.com/lxc/lxd/lxd/db" + "github.com/lxc/lxd/lxd/util" "github.com/lxc/lxd/shared" "github.com/lxc/lxd/shared/api" "github.com/lxc/lxd/shared/version" @@ -261,7 +262,7 @@ func storagePoolVolumeTypePut(d *Daemon, r *http.Request) Response { // Validate the ETag etag := []interface{}{volume.Name, volume.Type, volume.Config} - err = etagCheck(r, etag) + err = util.EtagCheck(r, etag) if err != nil { return PreconditionFailed(err) } @@ -323,7 +324,7 @@ func storagePoolVolumeTypePatch(d *Daemon, r *http.Request) Response { // Validate the ETag etag := []interface{}{volume.Name, volume.Type, volume.Config} - err = etagCheck(r, etag) + err = util.EtagCheck(r, etag) if err != nil { return PreconditionFailed(err) } diff --git a/lxd/storage_zfs.go b/lxd/storage_zfs.go index 2177282a1..0ebcc81a7 100644 --- a/lxd/storage_zfs.go +++ b/lxd/storage_zfs.go @@ -14,6 +14,7 @@ import ( "github.com/gorilla/websocket" "github.com/lxc/lxd/lxd/db" + "github.com/lxc/lxd/lxd/util" "github.com/lxc/lxd/shared" "github.com/lxc/lxd/shared/api" "github.com/lxc/lxd/shared/logger" @@ -46,7 +47,7 @@ func (s *storageZfs) StorageCoreInit() error { } s.sTypeName = typeName - loadModule("zfs") + util.LoadModule("zfs") if !zfsIsEnabled() { return fmt.Errorf("the \"zfs\" tool is not enabled") diff --git a/lxd/util.go b/lxd/util/encoding.go similarity index 64% rename from lxd/util.go rename to lxd/util/encoding.go index e85a5a849..71596ba0c 100644 --- a/lxd/util.go +++ b/lxd/util/encoding.go @@ -1,4 +1,4 @@ -package main +package util import ( "bytes" @@ -11,7 +11,7 @@ import ( "github.com/lxc/lxd/shared" ) -func WriteJSON(w http.ResponseWriter, body interface{}) error { +func WriteJSON(w http.ResponseWriter, body interface{}, debug bool) error { var output io.Writer var captured *bytes.Buffer @@ -30,7 +30,7 @@ func WriteJSON(w http.ResponseWriter, body interface{}) error { return err } -func etagHash(data interface{}) (string, error) { +func EtagHash(data interface{}) (string, error) { etag := sha256.New() err := json.NewEncoder(etag).Encode(data) if err != nil { @@ -40,13 +40,13 @@ func etagHash(data interface{}) (string, error) { return fmt.Sprintf("%x", etag.Sum(nil)), nil } -func etagCheck(r *http.Request, data interface{}) error { +func EtagCheck(r *http.Request, data interface{}) error { match := r.Header.Get("If-Match") if match == "" { return nil } - hash, err := etagHash(data) + hash, err := EtagHash(data) if err != nil { return err } @@ -57,12 +57,3 @@ func etagCheck(r *http.Request, data interface{}) error { return nil } - -func loadModule(module string) error { - if shared.PathExists(fmt.Sprintf("/sys/module/%s", module)) { - return nil - } - - _, err := shared.RunCommand("modprobe", module) - return err -} diff --git a/lxd/util/http.go b/lxd/util/http.go new file mode 100644 index 000000000..5f3eff9ff --- /dev/null +++ b/lxd/util/http.go @@ -0,0 +1,58 @@ +package util + +import ( + "crypto/x509" + "encoding/pem" + "fmt" + "net/http" + "net/url" + + "github.com/lxc/lxd/shared" +) + +// HTTPClient returns an http.Client using the given certificate and proxy. +func HTTPClient(certificate string, proxy proxyFunc) (*http.Client, error) { + var err error + var cert *x509.Certificate + + if certificate != "" { + certBlock, _ := pem.Decode([]byte(certificate)) + if certBlock == nil { + return nil, fmt.Errorf("Invalid certificate") + } + + cert, err = x509.ParseCertificate(certBlock.Bytes) + if err != nil { + return nil, err + } + } + + tlsConfig, err := shared.GetTLSConfig("", "", "", cert) + if err != nil { + return nil, err + } + + tr := &http.Transport{ + TLSClientConfig: tlsConfig, + Dial: shared.RFC3493Dialer, + Proxy: proxy, + DisableKeepAlives: true, + } + + myhttp := http.Client{ + Transport: tr, + } + + // Setup redirect policy + myhttp.CheckRedirect = func(req *http.Request, via []*http.Request) error { + // Replicate the headers + req.Header = via[len(via)-1].Header + + return nil + } + + return &myhttp, nil +} + +// A function capable of proxing an HTTP request. +type proxyFunc func(req *http.Request) (*url.URL, error) diff --git a/lxd/util/kernel.go b/lxd/util/kernel.go new file mode 100644 index 000000000..6f75915ba --- /dev/null +++ b/lxd/util/kernel.go @@ -0,0 +1,18 @@ +package util + +import ( + "fmt" + + "github.com/lxc/lxd/shared" +) + +// LoadModule loads the kernel module with the given name, by invoking +// modprobe. +func LoadModule(module string) error { + if shared.PathExists(fmt.Sprintf("/sys/module/%s", module)) { + return nil + } + + _, err := shared.RunCommand("modprobe", module) + return err +}
_______________________________________________ lxc-devel mailing list [email protected] http://lists.linuxcontainers.org/listinfo/lxc-devel
