From 97f8f2426f9a24014dce39df66083394895e81d7 Mon Sep 17 00:00:00 2001
From: Justin Nauman <justin.r.nauman@gmail.com>
Date: Mon, 18 Sep 2017 17:30:12 -0500
Subject: [PATCH] Addressing PR feedback and adding tests

Signed-off-by: Justin Nauman <justin.r.nauman@gmail.com>
---
 pkg/buildinfo/version.go      | 10 +++++++++
 pkg/buildinfo/version_test.go | 38 +++++++++++++++++++++++++++++++++++
 pkg/client/client.go          |  7 ++++---
 pkg/client/client_test.go     | 36 +++++++++++++++++++++++++++++++++
 pkg/cmd/ark/ark.go            |  2 +-
 pkg/cmd/server/server.go      |  4 ++--
 6 files changed, 91 insertions(+), 6 deletions(-)
 create mode 100644 pkg/buildinfo/version_test.go
 create mode 100644 pkg/client/client_test.go

diff --git a/pkg/buildinfo/version.go b/pkg/buildinfo/version.go
index e23470db0..1c1c2e9cb 100644
--- a/pkg/buildinfo/version.go
+++ b/pkg/buildinfo/version.go
@@ -19,6 +19,8 @@ limitations under the License.
 // worrying about introducing circular dependencies.
 package buildinfo
 
+import "fmt"
+
 var (
 	// Version is the current version of Ark, set by the go linker's -X flag at build time.
 	Version string
@@ -34,3 +36,11 @@ var (
 	// time.
 	GitTreeState string
 )
+
+// FormattedGitSHA renders the Git SHA with an indicator of the tree state.
+func FormattedGitSHA() string {
+	if GitTreeState != "clean" {
+		return fmt.Sprintf("%s-%s", GitSHA, GitTreeState)
+	}
+	return GitSHA
+}
diff --git a/pkg/buildinfo/version_test.go b/pkg/buildinfo/version_test.go
new file mode 100644
index 000000000..5f4479991
--- /dev/null
+++ b/pkg/buildinfo/version_test.go
@@ -0,0 +1,38 @@
+package buildinfo
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestFormattedGitSHA(t *testing.T) {
+	tests := []struct {
+		name     string
+		sha      string
+		state    string
+		expected string
+	}{
+		{
+			"Clean git state has no suffix",
+			"abc123",
+			"clean",
+			"abc123",
+		},
+		{
+			"Dirty git status includes suffix",
+			"abc123",
+			"dirty",
+			"abc123-dirty",
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			GitSHA = test.sha
+			GitTreeState = test.state
+			assert.Equal(t, FormattedGitSHA(), test.expected)
+		})
+	}
+
+}
diff --git a/pkg/client/client.go b/pkg/client/client.go
index 7db58b3ff..10a363205 100644
--- a/pkg/client/client.go
+++ b/pkg/client/client.go
@@ -39,15 +39,16 @@ func Config(kubeconfig, baseName string) (*rest.Config, error) {
 	clientConfig.UserAgent = buildUserAgent(
 		baseName,
 		buildinfo.Version,
+		buildinfo.FormattedGitSHA(),
 		runtime.GOOS,
 		runtime.GOARCH,
-		buildinfo.GitSHA)
+	)
 
 	return clientConfig, nil
 }
 
 // buildUserAgent builds a User-Agent string from given args.
-func buildUserAgent(command, version, os, arch, commit string) string {
+func buildUserAgent(command, version, formattedSha, os, arch string) string {
 	return fmt.Sprintf(
-		"%s/%s (%s/%s) %s", command, version, os, arch, commit)
+		"%s/%s (%s/%s) %s", command, version, os, arch, formattedSha)
 }
diff --git a/pkg/client/client_test.go b/pkg/client/client_test.go
new file mode 100644
index 000000000..4ab5107d0
--- /dev/null
+++ b/pkg/client/client_test.go
@@ -0,0 +1,36 @@
+package client
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestBuildUserAgent(t *testing.T) {
+	tests := []struct {
+		name     string
+		command  string
+		os       string
+		arch     string
+		gitSha   string
+		version  string
+		expected string
+	}{
+		{
+			name:     "Test general interpolation in correct order",
+			command:  "ark",
+			os:       "darwin",
+			arch:     "amd64",
+			gitSha:   "abc123",
+			version:  "v0.1.1",
+			expected: "ark/v0.1.1 (darwin/amd64) abc123",
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			resp := buildUserAgent(test.command, test.version, test.gitSha, test.os, test.arch)
+			assert.Equal(t, resp, test.expected)
+		})
+	}
+}
diff --git a/pkg/cmd/ark/ark.go b/pkg/cmd/ark/ark.go
index e99c18014..26a7e0707 100644
--- a/pkg/cmd/ark/ark.go
+++ b/pkg/cmd/ark/ark.go
@@ -46,7 +46,7 @@ associated data.`,
 		backup.NewCommand(f),
 		schedule.NewCommand(f),
 		restore.NewCommand(f),
-		server.NewCommand(name),
+		server.NewCommand(),
 		version.NewCommand(),
 	)
 
diff --git a/pkg/cmd/server/server.go b/pkg/cmd/server/server.go
index 4a742e982..fb3326e80 100644
--- a/pkg/cmd/server/server.go
+++ b/pkg/cmd/server/server.go
@@ -57,7 +57,7 @@ import (
 	"github.com/heptio/ark/pkg/util/kube"
 )
 
-func NewCommand(baseName string) *cobra.Command {
+func NewCommand() *cobra.Command {
 	var kubeconfig string
 
 	var command = &cobra.Command{
@@ -65,7 +65,7 @@ func NewCommand(baseName string) *cobra.Command {
 		Short: "Run the ark server",
 		Long:  "Run the ark server",
 		Run: func(c *cobra.Command, args []string) {
-			s, err := newServer(kubeconfig, baseName+"-server")
+			s, err := newServer(kubeconfig, fmt.Sprintf("%s-%s", c.Parent().Name(), c.Name()))
 			cmd.CheckError(err)
 
 			cmd.CheckError(s.run())