Merge pull request #2649 from pedronis/log-dev-mgr-http

many: extract the logging http client and user-agent handling for use in devicestate

cmd/snap/main.go

@@ -35,15 +35,15 @@ import (
 	"github.com/snapcore/snapd/client"
 	"github.com/snapcore/snapd/cmd"
 	"github.com/snapcore/snapd/dirs"
+	"github.com/snapcore/snapd/httputil"
 	"github.com/snapcore/snapd/i18n"
 	"github.com/snapcore/snapd/logger"
 	"github.com/snapcore/snapd/osutil"
-	"github.com/snapcore/snapd/store"
 )
 
 func init() {
 	// set User-Agent for when 'snap' talks to the store directly (snap download etc...)
-	store.SetUserAgentFromVersion(cmd.Version, "snap")
+	httputil.SetUserAgentFromVersion(cmd.Version, "snap")
 }
 
 // Standard streams, redirected for testing.

cmd/snapd/main.go

@@ -27,8 +27,8 @@ import (
 
 	"github.com/snapcore/snapd/cmd"
 	"github.com/snapcore/snapd/daemon"
+	"github.com/snapcore/snapd/httputil"
 	"github.com/snapcore/snapd/logger"
-	"github.com/snapcore/snapd/store"
 )
 
 func init() {
@@ -47,7 +47,7 @@ func main() {
 }
 
 func run() error {
-	store.SetUserAgentFromVersion(cmd.Version)
+	httputil.SetUserAgentFromVersion(cmd.Version)
 
 	d, err := daemon.New()
 	if err != nil {

daemon/daemon.go

@@ -35,13 +35,13 @@ import (
 	"gopkg.in/tomb.v2"
 
 	"github.com/snapcore/snapd/dirs"
+	"github.com/snapcore/snapd/httputil"
 	"github.com/snapcore/snapd/i18n/dumb"
 	"github.com/snapcore/snapd/logger"
 	"github.com/snapcore/snapd/osutil"
 	"github.com/snapcore/snapd/overlord"
 	"github.com/snapcore/snapd/overlord/auth"
 	"github.com/snapcore/snapd/overlord/state"
-	"github.com/snapcore/snapd/store"
 )
 
 // A Daemon listens for requests and routes them to the right command
@@ -248,7 +248,7 @@ func (d *Daemon) Init() error {
 	d.addRoutes()
 
 	logger.Debugf("init done in %s", time.Now().Sub(t0))
-	logger.Noticef("started %v.", store.UserAgent())
+	logger.Noticef("started %v.", httputil.UserAgent())
 
 	return nil
 }

httputil/export_test.go

@@ -0,0 +1,30 @@
+// -*- Mode: Go; indent-tabs-mode: t -*-
+
+/*
+ * Copyright (C) 2016-2017 Canonical Ltd
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 3 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+package httputil
+
+var GetFlags = (*LoggedTransport).getFlags
+
+func MockUserAgent(mock string) (restore func()) {
+	old := userAgent
+	userAgent = mock
+	return func() {
+		userAgent = old
+	}
+}

store/logger.go → httputil/logger.go

@@ -1,7 +1,7 @@
 // -*- Mode: Go; indent-tabs-mode: t -*-
 
 /*
- * Copyright (C) 2016 Canonical Ltd
+ * Copyright (C) 2016-2017 Canonical Ltd
  *
  * This program is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
@@ -17,7 +17,7 @@
  *
  */
 
-package store
+package httputil
 
 import (
 	"errors"
@@ -87,16 +87,16 @@ func (tr *LoggedTransport) getFlags() debugflag {
 	return debugflag(flags)
 }
 
-type httpClientOpts struct {
+type ClientOpts struct {
 	Timeout    time.Duration
 	MayLogBody bool
 }
 
-// returns a new http.Client with a LoggedTransport, a Timeout and preservation
-// of range requests across redirects
-func newHTTPClient(opts *httpClientOpts) *http.Client {
+// NewHTTPCLient returns a new http.Client with a LoggedTransport, a
+// Timeout and preservation of range requests across redirects
+func NewHTTPClient(opts *ClientOpts) *http.Client {
 	if opts == nil {
-		opts = &httpClientOpts{}
+		opts = &ClientOpts{}
 	}
 
 	return &http.Client{

store/logger_test.go → httputil/logger_test.go

@@ -1,7 +1,7 @@
 // -*- Mode: Go; indent-tabs-mode: t -*-
 
 /*
- * Copyright (C) 2016 Canonical Ltd
+ * Copyright (C) 2016-2017 Canonical Ltd
  *
  * This program is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
@@ -17,7 +17,7 @@
  *
  */
 
-package store_test
+package httputil_test
 
 import (
 	"bytes"
@@ -26,14 +26,17 @@ import (
 	"net/http"
 	"os"
 	"strings"
+	"testing"
 
 	"gopkg.in/check.v1"
 
+	"github.com/snapcore/snapd/httputil"
 	"github.com/snapcore/snapd/logger"
-	"github.com/snapcore/snapd/store"
 	"github.com/snapcore/snapd/testutil"
 )
 
+func TestHTTPUtil(t *testing.T) { check.TestingT(t) }
+
 type loggerSuite struct {
 	logbuf *bytes.Buffer
 }
@@ -54,17 +57,17 @@ func (s *loggerSuite) SetUpTest(c *check.C) {
 
 func (loggerSuite) TestFlags(c *check.C) {
 	for _, f := range []interface{}{
-		store.DebugRequest,
-		store.DebugResponse,
-		store.DebugBody,
-		store.DebugRequest | store.DebugResponse | store.DebugBody,
+		httputil.DebugRequest,
+		httputil.DebugResponse,
+		httputil.DebugBody,
+		httputil.DebugRequest | httputil.DebugResponse | httputil.DebugBody,
 	} {
 		os.Setenv("TEST_FOO", fmt.Sprintf("%d", f))
-		tr := &store.LoggedTransport{
+		tr := &httputil.LoggedTransport{
 			Key: "TEST_FOO",
 		}
 
-		c.Check(store.GetFlags(tr), check.Equals, f)
+		c.Check(httputil.GetFlags(tr), check.Equals, f)
 	}
 }
 
@@ -85,7 +88,7 @@ func (s loggerSuite) TestLogging(c *check.C) {
 		Status:     "999 WAT",
 		StatusCode: 999,
 	}
-	tr := &store.LoggedTransport{
+	tr := &httputil.LoggedTransport{
 		Transport: &fakeTransport{
 			rsp: rsp,
 		},
@@ -113,7 +116,7 @@ func (s loggerSuite) TestNotLoggingOctetStream(c *check.C) {
 		},
 		Body: ioutil.NopCloser(strings.NewReader(needle)),
 	}
-	tr := &store.LoggedTransport{
+	tr := &httputil.LoggedTransport{
 		Transport: &fakeTransport{
 			rsp: rsp,
 		},

httputil/useragent.go

@@ -0,0 +1,69 @@
+// -*- Mode: Go; indent-tabs-mode: t -*-
+
+/*
+ * Copyright (C) 2016-2017 Canonical Ltd
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 3 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+package httputil
+
+import (
+	"fmt"
+	"strings"
+
+	"github.com/snapcore/snapd/arch"
+	"github.com/snapcore/snapd/osutil"
+	"github.com/snapcore/snapd/release"
+)
+
+// UserAgent to send
+// TODO: this should actually be set per client request, and include the client user agent
+var userAgent = "unset"
+
+var isTesting bool
+
+func init() {
+	if osutil.GetenvBool("SNAPPY_TESTING") {
+		isTesting = true
+	}
+}
+
+// SetUserAgentFromVersion sets up a user-agent string.
+func SetUserAgentFromVersion(version string, extraProds ...string) {
+	extras := make([]string, 1, 3)
+	extras[0] = "series " + release.Series
+	if release.OnClassic {
+		extras = append(extras, "classic")
+	}
+	if release.ReleaseInfo.ForceDevMode() {
+		extras = append(extras, "devmode")
+	}
+	if isTesting {
+		extras = append(extras, "testing")
+	}
+	extraProdStr := ""
+	if len(extraProds) != 0 {
+		extraProdStr = " " + strings.Join(extraProds, " ")
+	}
+	// xxx this assumes ReleaseInfo's ID and VersionID don't have weird characters
+	// (see rfc 7231 for values of weird)
+	// assumption checks out in practice, q.v. https://github.com/zyga/os-release-zoo
+	userAgent = fmt.Sprintf("snapd/%v (%s)%s %s/%s (%s)", version, strings.Join(extras, "; "), extraProdStr, release.ReleaseInfo.ID, release.ReleaseInfo.VersionID, string(arch.UbuntuArchitecture()))
+}
+
+// UserAgent returns the user-agent string setup through SetUserAgentFromVersion.
+func UserAgent() string {
+	return userAgent
+}

httputil/useragent_test.go

@@ -0,0 +1,52 @@
+// -*- Mode: Go; indent-tabs-mode: t -*-
+
+/*
+ * Copyright (C) 2016-2017 Canonical Ltd
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 3 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+package httputil_test
+
+import (
+	"strings"
+
+	. "gopkg.in/check.v1"
+
+	"github.com/snapcore/snapd/httputil"
+)
+
+type UASuite struct {
+	restore func()
+}
+
+var _ = Suite(&UASuite{})
+
+func (s *UASuite) SetUpTest(c *C) {
+	s.restore = httputil.MockUserAgent("-")
+}
+
+func (s *UASuite) TearDownTest(c *C) {
+	s.restore()
+}
+
+func (s *UASuite) TestUserAgent(c *C) {
+	httputil.SetUserAgentFromVersion("10")
+	ua := httputil.UserAgent()
+	c.Check(strings.HasPrefix(ua, "snapd/10 "), Equals, true)
+
+	httputil.SetUserAgentFromVersion("10", "extraProd")
+	ua = httputil.UserAgent()
+	c.Check(strings.Contains(ua, "extraProd"), Equals, true)
+}

store/withtestkeys.go → httputil/withtestkeys.go

@@ -2,7 +2,7 @@
 // +build withtestkeys
 
 /*
- * Copyright (C) 2016 Canonical Ltd
+ * Copyright (C) 2016-2017 Canonical Ltd
  *
  * This program is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
@@ -18,7 +18,7 @@
  *
  */
 
-package store
+package httputil
 
 func init() {
 	// mark as testing if we carry testing keys

osutil/mount.go

@@ -16,6 +16,7 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  *
  */
+
 package osutil
 
 import (

overlord/devicestate/devicemgr.go

@@ -40,6 +40,7 @@ import (
 
 	"github.com/snapcore/snapd/asserts"
 	"github.com/snapcore/snapd/dirs"
+	"github.com/snapcore/snapd/httputil"
 	"github.com/snapcore/snapd/i18n/dumb"
 	"github.com/snapcore/snapd/osutil"
 	"github.com/snapcore/snapd/overlord/assertstate"
@@ -455,6 +456,7 @@ func prepareSerialRequest(t *state.Task, privKey asserts.PrivateKey, device *aut
 	if err != nil {
 		return "", fmt.Errorf("internal error: cannot create request-id request %q", cfg.requestIDURL)
 	}
+	req.Header.Set("User-Agent", httputil.UserAgent())
 	cfg.applyHeaders(req)
 
 	resp, err := client.Do(req)
@@ -508,6 +510,7 @@ func submitSerialRequest(t *state.Task, serialRequest string, client *http.Clien
 	if err != nil {
 		return nil, fmt.Errorf("internal error: cannot create serial-request request %q", cfg.serialRequestURL)
 	}
+	req.Header.Set("User-Agent", httputil.UserAgent())
 	cfg.applyHeaders(req)
 	req.Header.Set("Content-Type", asserts.MediaType)
 
@@ -556,7 +559,10 @@ func getSerial(t *state.Task, privKey asserts.PrivateKey, device *auth.DeviceSta
 		return a.(*asserts.Serial), nil
 	}
 
-	client := &http.Client{Timeout: 30 * time.Second}
+	client := httputil.NewHTTPClient(&httputil.ClientOpts{
+		Timeout:    30 * time.Second,
+		MayLogBody: true,
+	})
 
 	// NB: until we get at least an Accepted (202) we need to
 	// retry from scratch creating a new request-id because the