Repository: tetratelabs/getenvoy
Pull request #245 (2021-05-27 03:32:51)

Adds platform to User-Agent
Adding platform will tell us how many are using architectures besides
amd64. For example, we will see attempts of usage in analytics, even if
they end up eventually failing. This allows us to plan better.



Patch:
diff --git a/internal/cmd/app.go b/internal/cmd/app.go
index 342202b..3886470 100644
--- a/internal/cmd/app.go
+++ b/internal/cmd/app.go
@@ -29,7 +29,7 @@ import (
 // NewApp create a new root command. The globals.GlobalOpts parameter allows tests to scope overrides, which avoids
 // having to define a flag for everything needed in tests.
 func NewApp(o *globals.GlobalOpts) *cli.App {
-	var homeDir, envoyVersionsURL string
+	var homeDir, envoyVersionsURL, userAgent string
 
 	app := cli.NewApp()
 	app.Name = "getenvoy"
@@ -51,8 +51,15 @@ func NewApp(o *globals.GlobalOpts) *cli.App {
 			DefaultText: globals.DefaultEnvoyVersionsURL,
 			Destination: &envoyVersionsURL,
 			EnvVars:     []string{"ENVOY_VERSIONS_URL"},
+		},
+		&cli.StringFlag{
+			Name:        "internal-user-agent", // Hidden, but settable for release e2e tests
+			Hidden:      true,                  // Hidden, but settable for release e2e tests
+			Value:       globals.DefaultUserAgent,
+			Destination: &userAgent,
 		}}
 	app.Before = func(c *cli.Context) error {
+		o.UserAgent = userAgent
 		if err := setHomeDir(o, homeDir); err != nil {
 			return err
 		}
diff --git a/internal/cmd/install.go b/internal/cmd/install.go
index bc49227..af0e4fd 100644
--- a/internal/cmd/install.go
+++ b/internal/cmd/install.go
@@ -37,7 +37,7 @@ Example:
 $ getenvoy install %s`, version.LastKnownEnvoy),
 		Before: validateVersionArg,
 		Action: func(c *cli.Context) error {
-			_, err := envoy.InstallIfNeeded(o, envoy.CurrentPlatform(), c.Args().First())
+			_, err := envoy.InstallIfNeeded(o, globals.CurrentPlatform, c.Args().First())
 			return err
 		},
 	}
diff --git a/internal/cmd/run.go b/internal/cmd/run.go
index 1004b2a..b272f5f 100644
--- a/internal/cmd/run.go
+++ b/internal/cmd/run.go
@@ -44,7 +44,7 @@ $ getenvoy run %s --config-path ./bootstrap.yaml`, latestversion.LastKnownEnvoy)
 		Before: validateVersionArg,
 		Action: func(c *cli.Context) error {
 			args := c.Args().Slice()
-			if err := initializeRunOpts(o, envoy.CurrentPlatform(), args[0]); err != nil {
+			if err := initializeRunOpts(o, globals.CurrentPlatform, args[0]); err != nil {
 				return err
 			}
 			r := envoy.NewRuntime(&o.RunOpts)
diff --git a/internal/cmd/versions.go b/internal/cmd/versions.go
index 4fe7ab2..547655a 100644
--- a/internal/cmd/versions.go
+++ b/internal/cmd/versions.go
@@ -27,11 +27,11 @@ func NewVersionsCmd(o *globals.GlobalOpts) *cli.Command {
 		Name:  "versions",
 		Usage: "List available Envoy versions",
 		Action: func(c *cli.Context) error {
-			m, err := envoy.GetEnvoyVersions(o.EnvoyVersionsURL)
+			m, err := envoy.GetEnvoyVersions(o.EnvoyVersionsURL, o.UserAgent)
 			if err != nil {
 				return err
 			}
-			envoy.PrintVersions(m, envoy.CurrentPlatform(), c.App.Writer)
+			envoy.PrintVersions(m, globals.CurrentPlatform, c.App.Writer)
 			return nil
 		},
 	}
diff --git a/internal/envoy/http.go b/internal/envoy/http.go
index 23498a5..b2dd81b 100644
--- a/internal/envoy/http.go
+++ b/internal/envoy/http.go
@@ -15,16 +15,11 @@
 package envoy
 
 import (
-	"fmt"
 	"net/http"
-
-	"github.com/tetratelabs/getenvoy/internal/version"
 )
 
-var userAgent = fmt.Sprintf("GetEnvoy/%s", version.GetEnvoy)
-
 // httpGet adds the "User-Agent" header to the request, so that we can tell what is a dev build vs release.
-func httpGet(url string) (*http.Response, error) {
+func httpGet(url, userAgent string) (*http.Response, error) {
 	// #nosec -> url can be anywhere by design
 	req, err := http.NewRequest("GET", url, nil)
 	if err != nil {
diff --git a/internal/envoy/install.go b/internal/envoy/install.go
index c8b05a8..ade5247 100644
--- a/internal/envoy/install.go
+++ b/internal/envoy/install.go
@@ -42,7 +42,7 @@ func InstallIfNeeded(o *globals.GlobalOpts, p, v string) (string, error) {
 		}
 
 		var ev version.EnvoyVersions
-		ev, err = GetEnvoyVersions(o.EnvoyVersionsURL)
+		ev, err = GetEnvoyVersions(o.EnvoyVersionsURL, o.UserAgent)
 		if err != nil {
 			return "", err
 		}
@@ -53,7 +53,7 @@ func InstallIfNeeded(o *globals.GlobalOpts, p, v string) (string, error) {
 		}
 
 		fmt.Fprintln(o.Out, "downloading", tarballURL) //nolint
-		if err = untarEnvoy(installPath, tarballURL, o.Out); err != nil {
+		if err = untarEnvoy(installPath, tarballURL, o.UserAgent, o.Out); err != nil {
 			return "", err
 		}
 	case err == nil:
@@ -77,8 +77,8 @@ func verifyEnvoy(installPath string) (string, error) {
 	return envoyPath, nil
 }
 
-func untarEnvoy(dst, url string, out io.Writer) error { // dst, src order like io.Copy
-	resp, err := httpGet(url)
+func untarEnvoy(dst, url, userAgent string, out io.Writer) error { // dst, src order like io.Copy
+	resp, err := httpGet(url, userAgent)
 	if err != nil {
 		return err
 	}
diff --git a/internal/envoy/versions.go b/internal/envoy/versions.go
index cd65065..c4f3079 100644
--- a/internal/envoy/versions.go
+++ b/internal/envoy/versions.go
@@ -19,17 +19,16 @@ import (
 	"fmt"
 	"io"
 	"net/http"
-	"runtime"
 	"sort"
 
 	"github.com/tetratelabs/getenvoy/internal/version"
 )
 
 // GetEnvoyVersions returns a version map from a remote URL. eg globals.DefaultEnvoyVersionsURL.
-func GetEnvoyVersions(envoyVersionsURL string) (version.EnvoyVersions, error) {
+func GetEnvoyVersions(envoyVersionsURL, userAgent string) (version.EnvoyVersions, error) {
 	result := version.EnvoyVersions{}
 	// #nosec => This is by design, users can call out to wherever they like!
-	resp, err := httpGet(envoyVersionsURL)
+	resp, err := httpGet(envoyVersionsURL, userAgent)
 	if err != nil {
 		return result, err
 	}
@@ -75,8 +74,3 @@ func PrintVersions(vs version.EnvoyVersions, p string, w io.Writer) {
 		fmt.Fprintf(w, "%s\t%s\n", vr.version, vr.releaseDate) //nolint
 	}
 }
-
-// CurrentPlatform is the platform of the current process. This is used as a key in EnvoyVersion.Tarballs.
-func CurrentPlatform() string {
-	return runtime.GOOS + "/" + runtime.GOARCH
-}
diff --git a/internal/globals/globals.go b/internal/globals/globals.go
index 02a8be4..5b00fd6 100644
--- a/internal/globals/globals.go
+++ b/internal/globals/globals.go
@@ -14,7 +14,13 @@
 
 package globals
 
-import "io"
+import (
+	"fmt"
+	"io"
+	"runtime"
+
+	"github.com/tetratelabs/getenvoy/internal/version"
+)
 
 // RunOpts support invocations of "getenvoy run"
 type RunOpts struct {
@@ -42,6 +48,8 @@ type GlobalOpts struct {
 	EnvoyVersionsURL string
 	// HomeDir most importantly contains Envoy versions installed from EnvoyVersionsURL. Defaults to DefaultHomeDir
 	HomeDir string
+	// UserAgent is the "User-Agent" header added to all HTTP requests. Defaults to DefaultUserAgent
+	UserAgent string
 	// Out is where status messages are written. Defaults to os.Stdout
 	Out io.Writer
 }
@@ -52,3 +60,12 @@ const (
 	// DefaultEnvoyVersionsURL is the default value for GlobalOpts.EnvoyVersionsURL
 	DefaultEnvoyVersionsURL = "https://getenvoy.io/envoy-versions.json"
 )
+
+var (
+	// CurrentPlatform is the platform of the current process. This is used as a key in EnvoyVersion.Tarballs.
+	CurrentPlatform = runtime.GOOS + "/" + runtime.GOARCH
+	// DefaultUserAgent is the default value for GlobalOpts.UserAgent.
+	// This includes the platform to help differentiate installations in site analytics.
+	// This doesn't normalize the platform value like browsers: we use this to track usage of CurrentPlatform.
+	DefaultUserAgent = fmt.Sprintf("GetEnvoy/%s (%s)", version.GetEnvoy, CurrentPlatform)
+)


Test patch:
diff --git a/e2e/admin.go b/e2e/admin_test.go
similarity index 100%
rename from e2e/admin.go
rename to e2e/admin_test.go
diff --git a/e2e/buffer.go b/e2e/buffer_test.go
similarity index 100%
rename from e2e/buffer.go
rename to e2e/buffer_test.go
diff --git a/e2e/command.go b/e2e/command_test.go
similarity index 85%
rename from e2e/command.go
rename to e2e/command_test.go
index 1a8f2f0..1cff2c2 100644
--- a/e2e/command.go
+++ b/e2e/command_test.go
@@ -28,10 +28,8 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-var (
-	// getEnvoyPath holds a path to a 'getenvoy' binary under test.
-	getEnvoyPath = "getenvoy"
-)
+// getEnvoyPath holds a path to a 'getenvoy' binary under test.
+var getEnvoyPath = "getenvoy"
 
 // cmdBuilder represents a command builder.
 type cmdBuilder struct {
@@ -40,7 +38,16 @@ type cmdBuilder struct {
 
 // getEnvoy returns a new command builder.
 func getEnvoy(arg0 string) *cmdBuilder { //nolint:golint
-	return &cmdBuilder{exec.Command(getEnvoyPath, arg0)} //nolint:gosec
+
+	var args []string
+	if arg0 == "--version" {
+		args = []string{arg0}
+	} else {
+		args = []string{"--internal-user-agent", userAgent, arg0}
+	}
+
+	// Hard-code "User-Agent" HTTP header so that it doesn't interfere with site analytics.
+	return &cmdBuilder{exec.Command(getEnvoyPath, args...)} //nolint:gosec
 }
 
 func (b *cmdBuilder) args(args ...string) *cmdBuilder {
diff --git a/e2e/main_test.go b/e2e/main_test.go
index 3a5a8b5..af82444 100644
--- a/e2e/main_test.go
+++ b/e2e/main_test.go
@@ -31,6 +31,7 @@ const (
 	getenvoyBinaryEnvKey   = "E2E_GETENVOY_BINARY"
 	envoyVersionsURLEnvKey = "ENVOY_VERSIONS_URL"
 	envoyVersionsJSON      = "../site/envoy-versions.json"
+	userAgent              = "GetEnvoy/dev"
 )
 
 // TestMain ensures the "getenvoy" binary is valid.
@@ -55,7 +56,6 @@ func TestMain(m *testing.M) {
 		}
 		os.Setenv(envoyVersionsURLEnvKey, s.URL)
 	}
-
 	os.Exit(m.Run())
 }
 
@@ -79,6 +79,12 @@ func mockEnvoyVersionsServer() (*httptest.Server, error) {
 	}
 
 	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		// Ensure e2e tests won't eventually interfere with analytics when run against a release version
+		if r.UserAgent() != userAgent {
+			w.WriteHeader(500)
+			w.Write([]byte(fmt.Sprintln(`invalid "User-Agent":`, r.UserAgent()))) //nolint
+			return
+		}
 		w.Header().Add("Content-Type", "application/json")
 		w.WriteHeader(200)
 		w.Write(bytes) //nolint
diff --git a/internal/cmd/app_test.go b/internal/cmd/app_test.go
index 0e9a797..ce57d62 100644
--- a/internal/cmd/app_test.go
+++ b/internal/cmd/app_test.go
@@ -170,6 +170,40 @@ func TestEnvoyVersionsURL(t *testing.T) {
 	}
 }
 
+// TestGetEnvoyUserAgent tests we can re-assign the user-agent, even if this ability is internal.
+func TestGetEnvoyUserAgent(t *testing.T) {
+	type testCase struct {
+		name     string
+		args     []string
+		expected string
+	}
+
+	tests := []testCase{ // we don't test default as that depends on the runtime env
+		{
+			name:     `default is "GetEnvoy/$version ($platform)"`,
+			args:     []string{"getenvoy"},
+			expected: globals.DefaultUserAgent,
+		},
+		{
+			name:     "--internal-user-agent",
+			args:     []string{"getenvoy", "--internal-user-agent", "GetEnvoy/dev"},
+			expected: "GetEnvoy/dev",
+		},
+	}
+
+	for _, tc := range tests {
+		tc := tc // pin! see https://github.com/kyoh86/scopelint for why
+
+		t.Run(tc.name, func(t *testing.T) {
+			o := &globals.GlobalOpts{}
+			err := runTestCommand(t, o, tc.args)
+
+			require.NoError(t, err)
+			require.Equal(t, tc.expected, o.UserAgent)
+		})
+	}
+}
+
 // requireSetenv will os.Setenv the given key and value. The function returned reverts to the original.
 func requireSetenv(t *testing.T, key, value string) func() {
 	previous := os.Getenv(key)
diff --git a/internal/envoy/http_test.go b/internal/envoy/http_test.go
index 39ebb58..276c60c 100644
--- a/internal/envoy/http_test.go
+++ b/internal/envoy/http_test.go
@@ -23,12 +23,14 @@ import (
 )
 
 func TestHttpGet_AddsUserAgent(t *testing.T) {
+	userAgent := "foo"
+
 	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		require.Equal(t, userAgent, r.UserAgent())
 	}))
 	defer ts.Close()
 
-	res, err := httpGet(ts.URL)
+	res, err := httpGet(ts.URL, userAgent)
 	require.NoError(t, err)
 
 	defer res.Body.Close()
diff --git a/internal/envoy/install_test.go b/internal/envoy/install_test.go
index 0272189..3a835e3 100644
--- a/internal/envoy/install_test.go
+++ b/internal/envoy/install_test.go
@@ -35,6 +35,7 @@ import (
 )
 
 func TestUntarEnvoyError(t *testing.T) {
+	userAgent := "test"
 	tempDir, removeTempDir := morerequire.RequireNewTempDir(t)
 	dst := filepath.Join(tempDir, "dst")
 	defer removeTempDir()
@@ -51,7 +52,7 @@ func TestUntarEnvoyError(t *testing.T) {
 
 	url := server.URL + "/file.tar.gz"
 	t.Run("error on incorrect URL", func(t *testing.T) {
-		err := untarEnvoy(dst, url, io.Discard)
+		err := untarEnvoy(dst, url, userAgent, io.Discard)
 		require.EqualError(t, err, fmt.Sprintf(`received 404 status code from %s`, url))
 	})
 
@@ -59,7 +60,7 @@ func TestUntarEnvoyError(t *testing.T) {
 		w.WriteHeader(200)
 	}
 	t.Run("error on empty", func(t *testing.T) {
-		err := untarEnvoy(dst, url, io.Discard)
+		err := untarEnvoy(dst, url, userAgent, io.Discard)
 		require.EqualError(t, err, fmt.Sprintf(`error untarring %s: EOF`, url))
 	})
 
@@ -68,7 +69,7 @@ func TestUntarEnvoyError(t *testing.T) {
 		w.Write([]byte("mary had a little lamb")) //nolint
 	}
 	t.Run("error on not a tar", func(t *testing.T) {
-		err := untarEnvoy(dst, url, io.Discard)
+		err := untarEnvoy(dst, url, userAgent, io.Discard)
 		require.EqualError(t, err, fmt.Sprintf(`error untarring %s: gzip: invalid header`, url))
 	})
 }
@@ -79,7 +80,7 @@ func TestUntarEnvoy(t *testing.T) {
 	defer cleanup()
 
 	out := new(bytes.Buffer)
-	err := untarEnvoy(o.tempDir, o.tarballURL, out)
+	err := untarEnvoy(o.tempDir, o.tarballURL, o.UserAgent, out)
 	require.NoError(t, err)
 	require.FileExists(t, filepath.Join(o.tempDir, binEnvoy))
 	require.Contains(t, out.String(), `100% |████████████████████████████████████████|`)
@@ -153,7 +154,7 @@ func TestInstallIfNeeded(t *testing.T) {
 	defer cleanup()
 	out := o.Out.(*bytes.Buffer)
 
-	envoyPath, e := InstallIfNeeded(&o.GlobalOpts, CurrentPlatform(), version.LastKnownEnvoy)
+	envoyPath, e := InstallIfNeeded(&o.GlobalOpts, globals.CurrentPlatform, version.LastKnownEnvoy)
 	require.NoError(t, e)
 	require.Equal(t, o.EnvoyPath, envoyPath)
 	require.FileExists(t, envoyPath)
@@ -246,6 +247,7 @@ func setupInstallTest(t *testing.T) (*installTest, func()) {
 			GlobalOpts: globals.GlobalOpts{
 				HomeDir:          tempDir,
 				EnvoyVersionsURL: versionsServer.URL + "/envoy-versions.json",
+				UserAgent:        globals.DefaultUserAgent,
 				Out:              new(bytes.Buffer),
 				RunOpts: globals.RunOpts{
 					EnvoyPath: filepath.Join(tempDir, "versions", version.LastKnownEnvoy, "bin", "envoy"),
diff --git a/internal/test/server.go b/internal/test/server.go
index 6e7d4d8..bf20caa 100644
--- a/internal/test/server.go
+++ b/internal/test/server.go
@@ -28,6 +28,7 @@ import (
 
 	"github.com/stretchr/testify/require"
 
+	"github.com/tetratelabs/getenvoy/internal/globals"
 	"github.com/tetratelabs/getenvoy/internal/tar"
 	"github.com/tetratelabs/getenvoy/internal/test/morerequire"
 	"github.com/tetratelabs/getenvoy/internal/version"
@@ -71,6 +72,7 @@ type server struct {
 func (s *server) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	switch {
 	case r.RequestURI == "/envoy-versions.json":
+		require.Equal(s.t, globals.DefaultUserAgent, r.UserAgent())
 		w.WriteHeader(http.StatusOK)
 		_, err := w.Write(s.getEnvoyVersions())
 		require.NoError(s.t, err)