Merge pull request #46 from carolynvs/user-agent-env-var

Set azure user agent environment variable

README.md

@@ -37,6 +37,28 @@ mixins:
     - EXTENSION_NAME
 ```
 
+### User Agent Opt Out
+
+When you declare the mixin, you can disable the mixin from customizing the az user agent string
+
+```yaml
+mixins:
+- az:
+    userAgentOptOut: true
+```
+
+By default, the az mixin adds the porter and mixin version to the user agent string used by the az CLI.
+We use this to understand which version of porter and the mixin are being used by a bundle, and assist with troubleshooting.
+Below is an example of what the user agent string looks like:
+
+```
+AZURE_HTTP_USER_AGENT="getporter/porter/v1.0.0 getporter/az/v1.2.3"
+```
+
+You can add your own custom strings to the user agent string by editing your [template Dockerfile] and setting the AZURE_HTTP_USER_AGENT environment variable.
+
+[template Dockerfile]: https://getporter.org/bundle/custom-dockerfile/
+
 ## Mixin Syntax
 
 The format below is for executing any arbitrary az CLI command.

cmd/az/main.go

@@ -16,12 +16,7 @@ import (
 func main() {
 	run := func() int {
 		ctx := context.Background()
-		m, err := az.New()
-		if err != nil {
-			fmt.Fprintln(os.Stderr, err)
-			os.Exit(cli.ExitCodeErr)
-		}
-
+		m := az.New()
 		if err := m.ConfigureLogging(ctx); err != nil {
 			fmt.Println(err)
 			os.Exit(cli.ExitCodeErr)

pkg/az/build.go

@@ -20,14 +20,30 @@ type BuildInput struct {
 //     extensions:
 //     - NAME
 type MixinConfig struct {
-	ClientVersion string   `yaml:"clientVersion,omitempty"`
-	Extensions    []string `yaml:"extensions,omitempty"`
+	// UserAgentOptOut allows a bundle author to opt out from adding porter and the mixin's version to the az CLI user agent string.
+	UserAgentOptOut bool `yaml:"userAgentOptOut,omitempty"`
+
+	// ClientVersion is the version of the az CLI to install.
+	ClientVersion string `yaml:"clientVersion,omitempty"`
+
+	// Extensions is a list of az CLI extensions to install.
+	Extensions []string `yaml:"extensions,omitempty"`
+}
+
+// buildConfig is the set of configuration options for the mixin's portion of the Dockerfile
+type buildConfig struct {
+	MixinConfig
+
+	// AzureUserAgent is the contents of the az CLI user agent environment variable.
+	AzureUserAgent string
 }
 
 // The package version of the az cli follows this format:
 // VERSION-1~DISTRO_CODENAME So if we are running on debian stretch and have a
 // version of 1.2.3, the package version would be 1.2.3-1~stretch.
 const buildTemplate string = `
+ENV PORTER_AZ_MIXIN_USER_AGENT_OPT_OUT="{{ .UserAgentOptOut}}"
+ENV AZURE_HTTP_USER_AGENT="{{ .AzureUserAgent }}"
 RUN --mount=type=cache,target=/var/cache/apt --mount=type=cache,target=/var/lib/apt \
 	apt-get update && apt-get install -y apt-transport-https lsb-release gnupg curl
 RUN curl -sL https://packages.microsoft.com/keys/microsoft.asc | gpg --dearmor > /etc/apt/trusted.gpg.d/microsoft.asc.gpg
@@ -56,7 +72,13 @@ func (m *Mixin) Build(ctx context.Context) error {
 		return fmt.Errorf("error parsing Dockerfile template for the az mixin: %w", err)
 	}
 
-	if err = tmpl.Execute(m.Out, input.Config); err != nil {
+	cfg := buildConfig{MixinConfig: input.Config}
+	if !input.Config.UserAgentOptOut {
+		// If they opt out, then the user agent string defaulted in the bundle will be empty
+		cfg.AzureUserAgent = m.userAgent
+	}
+
+	if err = tmpl.Execute(m.Out, cfg); err != nil {
 		return fmt.Errorf("error generating Dockerfile lines for the az mixin: %w", err)
 	}
 

pkg/az/build_test.go

@@ -6,13 +6,12 @@ import (
 	"io/ioutil"
 	"testing"
 
+	"get.porter.sh/mixin/az/pkg"
 	"get.porter.sh/porter/pkg/test"
-
 	"github.com/stretchr/testify/require"
 )
 
 func TestMixin_Build(t *testing.T) {
-
 	testcases := []struct {
 		name           string
 		inputFile      string
@@ -23,7 +22,10 @@ func TestMixin_Build(t *testing.T) {
 	}
 
 	for _, tc := range testcases {
-		t.Run("build with config", func(t *testing.T) {
+		t.Run(tc.name, func(t *testing.T) {
+			// Set a fake version of the mixin and porter for our user agent
+			pkg.Version = "v1.2.3"
+
 			b, err := ioutil.ReadFile(tc.inputFile)
 			require.NoError(t, err)
 

pkg/az/config.go

@@ -1,24 +1,54 @@
 package az
 
 import (
+	"strconv"
 	"strings"
 
 	"get.porter.sh/porter/pkg"
 )
 
-const AZURE_HTTP_USER_AGENT = "AZURE_HTTP_USER_AGENT"
+const (
+	// AzureUserAgentEnvVar is the environment variable used by the az CLI to set
+	// the user agent string sent to Azure.
+	AzureUserAgentEnvVar = "AZURE_HTTP_USER_AGENT"
 
+	// UserAgentOptOutEnvVar is the name of the environment variable that disables
+	// user agent reporting.
+	UserAgentOptOutEnvVar = "PORTER_AZ_MIXIN_USER_AGENT_OPT_OUT"
+)
+
+// SetUserAgent sets the AZURE_HTTP_USER_AGENT environment variable with
+// the full user agent string, which includes both a portion for porter and the
+// mixin.
 func (m *Mixin) SetUserAgent() {
-	value := []string{pkg.UserAgent(), m.UserAgent()}
+	// Check if PORTER_AZ_MIXIN_USER_AGENT_OPT_OUT=true, which disables editing the user agent string
+	if optOut, _ := strconv.ParseBool(m.Getenv(UserAgentOptOutEnvVar)); optOut {
+		return
+	}
 
-	if agentStr, ok := m.LookupEnv(AZURE_HTTP_USER_AGENT); ok {
+	// Check if we have already set the user agent
+	if m.userAgent != "" {
+		return
+	}
+
+	// Append porter and the mixin's version to the user agent string. Some clouds and
+	// environments will have set the environment variable already and we don't want
+	// to clobber it.
+	porterUserAgent := pkg.UserAgent()
+	value := []string{porterUserAgent, m.GetMixinUserAgent()}
+	if agentStr, ok := m.LookupEnv(AzureUserAgentEnvVar); ok {
 		value = append(value, agentStr)
 	}
 
-	m.Setenv(AZURE_HTTP_USER_AGENT, strings.Join(value, " "))
+	m.userAgent = strings.Join(value, " ")
+
+	// Set the az CLI user agent as an environment variable so that when we call the
+	// az CLI, it's automatically passed too.
+	m.Setenv(AzureUserAgentEnvVar, m.userAgent)
 }
 
-func (m *Mixin) UserAgent() string {
+// GetMixinUserAgent returns the portion of the user agent string for the mixin.
+func (m *Mixin) GetMixinUserAgent() string {
 	v := m.Version()
 	return "getporter/" + v.Name + "/" + v.Version
 }

pkg/az/config_test.go

@@ -1,19 +1,81 @@
 package az
 
 import (
+	"os"
 	"testing"
 
 	"get.porter.sh/mixin/az/pkg"
+	"get.porter.sh/porter/pkg/runtime"
 	"github.com/stretchr/testify/require"
 )
 
-func TestSetUserAgent(t *testing.T) {
-	pkg.Commit = "abc123"
-	pkg.Version = "v1.2.3"
+func TestMixinSetsUserAgentEnvVar(t *testing.T) {
+	// CI sets this value and we need to clear it out to make the test reproducible
+	os.Unsetenv(AzureUserAgentEnvVar)
 
-	m := NewTestMixin(t)
-	m.SetUserAgent()
+	t.Run("sets env var", func(t *testing.T) {
+		pkg.Commit = "abc123"
+		pkg.Version = "v1.2.3"
 
-	expected := "getporter/porter getporter/az/" + pkg.Version
-	require.Contains(t, m.Getenv(AZURE_HTTP_USER_AGENT), expected)
+		m := New()
+		expected := "getporter/porter getporter/az/" + pkg.Version
+		require.Equal(t, expected, m.Getenv(AzureUserAgentEnvVar))
+		require.Equal(t, expected, m.userAgent, "validate we remember the user agent string for later")
+	})
+
+	t.Run("edits env var", func(t *testing.T) {
+		// Validate that if the user customizations of the env var are preserved
+		pkg.Commit = "abc123"
+		pkg.Version = "v1.2.3"
+
+		cfg := runtime.NewConfig()
+		customUserAgent := "mycustom/v1.2.3"
+		cfg.Setenv(AzureUserAgentEnvVar, customUserAgent)
+		m := NewFor(cfg)
+
+		expected := "getporter/porter getporter/az/v1.2.3 mycustom/v1.2.3"
+		require.Equal(t, expected, m.Getenv(AzureUserAgentEnvVar))
+		require.Equal(t, expected, m.userAgent, "validate we remember the user agent string for later")
+	})
+
+	t.Run("call multiple times", func(t *testing.T) {
+		// Validate that calling multiple times doesn't make a messed up env var
+		pkg.Commit = "abc123"
+		pkg.Version = "v1.2.3"
+
+		m := New()
+		m.SetUserAgent()
+		m.SetUserAgent()
+
+		expected := "getporter/porter getporter/az/v1.2.3"
+		require.Equal(t, expected, m.Getenv(AzureUserAgentEnvVar))
+		require.Equal(t, expected, m.userAgent, "validate we remember the user agent string for later")
+	})
+}
+
+func TestMixinSetsUserAgentEnvVar_OptOut(t *testing.T) {
+	// CI sets this value and we need to clear it out to make the test reproducible
+	os.Unsetenv(AzureUserAgentEnvVar)
+
+	t.Run("opt-out", func(t *testing.T) {
+		// Validate that at runtime when we are calling the az cli, that if the bundle author opted-out, we don't set the user agent string
+		cfg := runtime.NewConfig()
+		cfg.Setenv(UserAgentOptOutEnvVar, "true")
+		m := NewFor(cfg)
+
+		_, hasEnv := m.LookupEnv(AzureUserAgentEnvVar)
+		require.False(t, hasEnv, "expected the opt out to skip setting the AZURE_HTTP_USER_AGENT environment variable")
+	})
+
+	t.Run("opt-out preserves original value", func(t *testing.T) {
+		// Validate that at runtime when we are calling the az cli, that if the bundle author opted-out, we don't set the user agent string
+		cfg := runtime.NewConfig()
+		cfg.Setenv(UserAgentOptOutEnvVar, "true")
+		customUserAgent := "mycustom/v1.2.3"
+		cfg.Setenv(AzureUserAgentEnvVar, customUserAgent)
+		m := NewFor(cfg)
+
+		require.Equal(t, customUserAgent, m.Getenv(AzureUserAgentEnvVar), "expected opting out to not prevent the user from setting a custom user agent")
+		require.Empty(t, m.userAgent, "validate we remember that we opted out")
+	})
 }

pkg/az/execute_test.go

@@ -3,7 +3,9 @@ package az
 import (
 	"bytes"
 	"context"
+	"fmt"
 	"io/ioutil"
+	"os"
 	"path"
 	"testing"
 
@@ -14,6 +16,13 @@ import (
 
 func TestMain(m *testing.M) {
 	test.TestMainWithMockedCommandHandlers(m)
+
+	// Validate that we passed in the user agent environment variable when we called the az cli
+	_, hasEnv := os.LookupEnv(AzureUserAgentEnvVar)
+	if !hasEnv {
+		fmt.Println("expected the az cli to be called with the AZURE_HTTP_USER_AGENT environment variable set")
+		os.Exit(127)
+	}
 }
 
 func TestMixin_Execute(t *testing.T) {

pkg/az/helpers.go

@@ -16,11 +16,13 @@ type TestMixin struct {
 // NewTestMixin initializes a mixin test client, with the output buffered, and an in-memory file system.
 func NewTestMixin(t *testing.T) *TestMixin {
 	c := portercontext.NewTestContext(t)
+
+	// Clear this out when testing since our CI environment has modifications to it
+	c.Unsetenv(AzureUserAgentEnvVar)
+
 	cfg := runtime.NewConfigFor(c.Context)
 	m := &TestMixin{
-		Mixin: &Mixin{
-			RuntimeConfig: cfg,
-		},
+		Mixin:       NewFor(cfg),
 		TestContext: c,
 	}
 	t.Cleanup(func() {

pkg/az/mixin.go

@@ -6,15 +6,19 @@ import (
 
 type Mixin struct {
 	runtime.RuntimeConfig
+	userAgent string
 }
 
 // New azure mixin client, initialized with useful defaults.
-func New() (*Mixin, error) {
+func New() *Mixin {
+	return NewFor(runtime.NewConfig())
+}
+
+func NewFor(cfg runtime.RuntimeConfig) *Mixin {
 	m := &Mixin{
-		RuntimeConfig: runtime.NewConfig(),
+		RuntimeConfig: cfg,
 	}
 
 	m.SetUserAgent()
-	return m, nil
-
+	return m
 }

pkg/az/testdata/build-input-with-config.yaml

@@ -1,4 +1,5 @@
 config:
+  userAgentOptOut: true
   clientVersion: 1.2.3
   extensions:
     - iot

pkg/az/testdata/build-with-config.txt

@@ -1,4 +1,6 @@
 
+ENV PORTER_AZ_MIXIN_USER_AGENT_OPT_OUT="true"
+ENV AZURE_HTTP_USER_AGENT=""
 RUN --mount=type=cache,target=/var/cache/apt --mount=type=cache,target=/var/lib/apt \
 	apt-get update && apt-get install -y apt-transport-https lsb-release gnupg curl
 RUN curl -sL https://packages.microsoft.com/keys/microsoft.asc | gpg --dearmor > /etc/apt/trusted.gpg.d/microsoft.asc.gpg

pkg/az/testdata/build-without-config.txt

@@ -1,4 +1,6 @@
 
+ENV PORTER_AZ_MIXIN_USER_AGENT_OPT_OUT="false"
+ENV AZURE_HTTP_USER_AGENT="getporter/porter getporter/az/v1.2.3"
 RUN --mount=type=cache,target=/var/cache/apt --mount=type=cache,target=/var/lib/apt \
 	apt-get update && apt-get install -y apt-transport-https lsb-release gnupg curl
 RUN curl -sL https://packages.microsoft.com/keys/microsoft.asc | gpg --dearmor > /etc/apt/trusted.gpg.d/microsoft.asc.gpg