Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add management of TF_APPEND_USER_AGENT #46

Merged
merged 1 commit into from
Aug 26, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ go 1.14
require (
github.com/davecgh/go-spew v1.1.1
github.com/hashicorp/go-checkpoint v0.5.0
github.com/hashicorp/go-cleanhttp v0.5.0
github.com/hashicorp/go-getter v1.4.0
github.com/hashicorp/go-version v1.2.1
github.com/hashicorp/terraform-json v0.5.0
Expand Down
9 changes: 9 additions & 0 deletions internal/version/version.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package version

const version = "0.7.0"

// ModuleVersion returns the current version of the github.com/hashicorp/terraform-exec Go module.
// This is a function to allow for future possible enhancement using debug.BuildInfo.
func ModuleVersion() string {
return version
}
6 changes: 3 additions & 3 deletions scripts/release/release.sh
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ set -x
# release.sh will:
# 1. Modify changelog
# 2. Run changelog links script
# 3. Modify version in tfinstall/version.go
# 3. Modify version in internal/version/version.go
# 4. Commit and push changes
# 5. Create a Git tag

Expand Down Expand Up @@ -59,13 +59,13 @@ function changelogMain {
}

function modifyVersionFiles {
sed -i "s/const Version =.*/const Version = \"${TARGET_VERSION}\"/" tfinstall/version.go
sed -i "s/const version =.*/const version = \"${TARGET_VERSION}\"/" internal/version/version.go
}

function commitChanges {
git add CHANGELOG.md
modifyVersionFiles
git add tfinstall/version.go
git add internal/version/version.go

if [ "$CI" = true ] ; then
git commit --gpg-sign="${GPG_KEY_ID}" -m "v${TARGET_VERSION} [skip ci]"
Expand Down
33 changes: 33 additions & 0 deletions tfexec/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,13 @@ package tfexec

import (
"context"
"fmt"
"io"
"os"
"os/exec"
"strings"

"github.com/hashicorp/terraform-exec/internal/version"
)

const (
Expand All @@ -15,6 +18,7 @@ const (
automationEnvVar = "TF_IN_AUTOMATION"
logPathEnvVar = "TF_LOG_PATH"
reattachEnvVar = "TF_REATTACH_PROVIDERS"
appendUserAgentEnvVar = "TF_APPEND_USER_AGENT"

varEnvVarPrefix = "TF_VAR_"
)
Expand All @@ -25,6 +29,7 @@ var prohibitedEnvVars = []string{
logPathEnvVar,
logEnvVar,
reattachEnvVar,
appendUserAgentEnvVar,
}

func envMap(environ []string) map[string]string {
Expand Down Expand Up @@ -75,6 +80,14 @@ func (tf *Terraform) buildEnv(mergeEnv map[string]string) []string {
env[checkpointDisableEnvVar] = os.Getenv(checkpointDisableEnvVar)
}

// always override user agent
ua := mergeUserAgent(
os.Getenv(appendUserAgentEnvVar),
tf.appendUserAgent,
fmt.Sprintf("HashiCorp-terraform-exec/%s", version.ModuleVersion()),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't these values be in reverse order? i.e. static HashiCorp-terraform-exec first and then other values being appended, rather than prepended?

Copy link
Contributor Author

@paultyng paultyng Aug 14, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it depends. arguably "terraform cloud" or "binary testing" (as the appendUserAgent) is more significant / distinct than terraform exec I think in the case of that system? As there will be many terraform exec apps, so the app itself is the more significant identifier?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't disagree, but the problem is that the ENV variable is called append, not prepend, so we would need to reflect it there somehow (with another/changed variable), else the behaviour won't match the name.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thats the common convention in programming but not the english definition of the word. It was a poor original choice for the env var, but its what we have so no option to change it now.

)
env[appendUserAgentEnvVar] = ua

// always override logging
if tf.logPath == "" {
// so logging can't pollute our stderr output
Expand Down Expand Up @@ -123,3 +136,23 @@ func (tf *Terraform) runTerraformCmd(cmd *exec.Cmd) error {
}
return nil
}

// mergeUserAgent does some minor deduplication to ensure we aren't
// just using the same append string over and over.
func mergeUserAgent(uas ...string) string {
included := map[string]bool{}
merged := []string{}
for _, ua := range uas {
ua = strings.TrimSpace(ua)

if ua == "" {
continue
}
if included[ua] {
continue
}
included[ua] = true
merged = append(merged, ua)
}
return strings.Join(merged, " ")
}
43 changes: 37 additions & 6 deletions tfexec/cmd_test.go
Original file line number Diff line number Diff line change
@@ -1,16 +1,47 @@
package tfexec

import (
"fmt"
"os/exec"
"strings"
"testing"

"github.com/hashicorp/terraform-exec/internal/version"
)

var defaultEnv = []string{
"TF_LOG=",
"TF_LOG_PATH=",
"TF_IN_AUTOMATION=1",
"CHECKPOINT_DISABLE=",
func TestMergeUserAgent(t *testing.T) {
for i, c := range []struct {
expected string
uas []string
}{
{"foo/1 bar/2", []string{"foo/1", "bar/2"}},
{"foo/1 bar/2", []string{"foo/1 bar/2"}},
{"foo/1 bar/2", []string{"", "foo/1", "bar/2"}},
{"foo/1 bar/2", []string{"", "foo/1 bar/2"}},
{"foo/1 bar/2", []string{" ", "foo/1 bar/2"}},
{"foo/1 bar/2", []string{"foo/1", "", "bar/2"}},
{"foo/1 bar/2", []string{"foo/1", " ", "bar/2"}},

// comments
{"foo/1 (bar/1 bar/2 bar/3) bar/2", []string{"foo/1 (bar/1 bar/2 bar/3)", "bar/2"}},
} {
t.Run(fmt.Sprintf("%d %s", i, c.expected), func(t *testing.T) {
actual := mergeUserAgent(c.uas...)
if c.expected != actual {
t.Fatalf("expected %q, got %q", c.expected, actual)
}
})
}
}

func defaultEnv() []string {
return []string{
"TF_APPEND_USER_AGENT=HashiCorp-terraform-exec/" + version.ModuleVersion(),
"TF_LOG=",
"TF_LOG_PATH=",
"TF_IN_AUTOMATION=1",
"CHECKPOINT_DISABLE=",
}
}

func assertCmd(t *testing.T, expectedArgs []string, expectedEnv map[string]string, actual *exec.Cmd) {
Expand All @@ -29,7 +60,7 @@ func assertCmd(t *testing.T, expectedArgs []string, expectedEnv map[string]strin
}

// check environment
expectedEnv = envMap(append(defaultEnv, envSlice(expectedEnv)...))
expectedEnv = envMap(append(defaultEnv(), envSlice(expectedEnv)...))

// compare against raw slice len incase of duplication or something
if len(expectedEnv) != len(actual.Env) {
Expand Down
2 changes: 1 addition & 1 deletion tfexec/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ func parseError(err error, stderr string) error {
break
}
}

return &ErrMissingVar{name}
case usageRegexp.MatchString(stderr):
return &ErrCLIUsage{stderr: stderr}
Expand Down
2 changes: 1 addition & 1 deletion tfexec/internal/e2etest/errors_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ func TestMissingVar(t *testing.T) {
err := tf.Init(context.Background())
if err != nil {
t.Fatalf("err during init: %s", err)
}
}

err = tf.Plan(context.Background())
if err == nil {
Expand Down
13 changes: 10 additions & 3 deletions tfexec/terraform.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,10 @@ type printfer interface {
}

type Terraform struct {
execPath string
workingDir string
env map[string]string
execPath string
workingDir string
appendUserAgent string
env map[string]string

stdout io.Writer
stderr io.Writer
Expand Down Expand Up @@ -108,6 +109,12 @@ func (tf *Terraform) SetLogPath(path string) error {
return nil
}

// SetAppendUserAgent sets the TF_APPEND_USER_AGENT environment variable for
// Terraform CLI execution.
func (tf *Terraform) SetAppendUserAgent(ua string) {
tf.appendUserAgent = ua
}

// WorkingDir returns the working directory for Terraform.
func (tf *Terraform) WorkingDir() string {
return tf.workingDir
Expand Down
37 changes: 37 additions & 0 deletions tfinstall/http.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
package tfinstall

import (
"fmt"
"net/http"
"os"
"strings"

cleanhttp "github.com/hashicorp/go-cleanhttp"

intversion "github.com/hashicorp/terraform-exec/internal/version"
)

type userAgentRoundTripper struct {
inner http.RoundTripper
userAgent string
}

func (rt *userAgentRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) {
if _, ok := req.Header["User-Agent"]; !ok {
req.Header.Set("User-Agent", rt.userAgent)
}
return rt.inner.RoundTrip(req)
}

func newHTTPClient() *http.Client {
appendUA := os.Getenv("TF_APPEND_USER_AGENT")
userAgent := strings.TrimSpace(fmt.Sprintf("HashiCorp-tfinstall/%s %s", intversion.ModuleVersion(), appendUA))

cli := cleanhttp.DefaultPooledClient()
cli.Transport = &userAgentRoundTripper{
userAgent: userAgent,
inner: cli.Transport,
}

return cli
}
7 changes: 2 additions & 5 deletions tfinstall/tfinstall.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"fmt"
"io/ioutil"
"log"
"net/http"
"os"
"os/exec"
"path/filepath"
Expand Down Expand Up @@ -162,11 +161,9 @@ func downloadWithVerification(tfVersion string, installDir string) (string, erro

}

// setup: getter client
httpHeader := make(http.Header)
httpHeader.Set("User-Agent", "HashiCorp-tfinstall/"+Version)
httpGetter := &getter.HttpGetter{
Netrc: true,
Netrc: true,
Client: newHTTPClient(),
}
client := getter.Client{
Getters: map[string]getter.Getter{
Expand Down
4 changes: 0 additions & 4 deletions tfinstall/version.go

This file was deleted.