Merge pull request #44622 from monopole/jr_patch_to_upstream_release-1.6

Automatic merge from submit-queue Use OS-specific libs when computing client User-Agent in kubectl, etc. This PR fixes issue 44419 in the release branch. This PR is a patchable version of mainline PR #44423 for 1.6 release branch The original full PR #44423 came after a large PR #40777 that split /vendor/BUILD into hundreds of BUILD files. Thus PR 44423's version of rest/BUILD does not exist in the 1.6 release branch, and had to be tweaked here. ```release-note Fix for [Windows kubectl sending full path to binary in User Agent](#44419). ```
kubernetes · Apr 18, 2017 · ec5b632 · ec5b632
2 parents e1d21e0 + a26452b
commit ec5b632
Show file tree

Hide file tree

Showing 3 changed files with 83 additions and 12 deletions.
diff --git a/staging/src/k8s.io/client-go/rest/config.go b/staging/src/k8s.io/client-go/rest/config.go
@@ -22,7 +22,7 @@ import (
 	"net"
 	"net/http"
 	"os"
-	"path"
+	"path/filepath"
 	gruntime "runtime"
 	"strings"
 	"time"
@@ -255,19 +255,51 @@ func SetKubernetesDefaults(config *Config) error {
 	return nil
 }
 
-// DefaultKubernetesUserAgent returns the default user agent that clients can use.
-func DefaultKubernetesUserAgent() string {
-	commit := version.Get().GitCommit
-	if len(commit) > 7 {
-		commit = commit[:7]
+// adjustCommit returns sufficient significant figures of the commit's git hash.
+func adjustCommit(c string) string {
+	if len(c) == 0 {
+		return "unknown"
+	}
+	if len(c) > 7 {
+		return c[:7]
+	}
+	return c
+}
+
+// adjustVersion strips "alpha", "beta", etc. from version in form
+// major.minor.patch-[alpha|beta|etc].
+func adjustVersion(v string) string {
+	if len(v) == 0 {
+		return "unknown"
 	}
-	if len(commit) == 0 {
-		commit = "unknown"
+	seg := strings.SplitN(v, "-", 2)
+	return seg[0]
+}
+
+// adjustCommand returns the last component of the
+// OS-specific command path for use in User-Agent.
+func adjustCommand(p string) string {
+	// Unlikely, but better than returning "".
+	if len(p) == 0 {
+		return "unknown"
 	}
-	version := version.Get().GitVersion
-	seg := strings.SplitN(version, "-", 2)
-	version = seg[0]
-	return fmt.Sprintf("%s/%s (%s/%s) kubernetes/%s", path.Base(os.Args[0]), version, gruntime.GOOS, gruntime.GOARCH, commit)
+	return filepath.Base(p)
+}
+
+// buildUserAgent builds a User-Agent string from given args.
+func buildUserAgent(command, version, os, arch, commit string) string {
+	return fmt.Sprintf(
+		"%s/%s (%s/%s) kubernetes/%s", command, version, os, arch, commit)
+}
+
+// DefaultKubernetesUserAgent returns a User-Agent string built from static global vars.
+func DefaultKubernetesUserAgent() string {
+	return buildUserAgent(
+		adjustCommand(os.Args[0]),
+		adjustVersion(version.Get().GitVersion),
+		gruntime.GOOS,
+		gruntime.GOARCH,
+		adjustCommit(version.Get().GitCommit))
 }
 
 // InClusterConfig returns a config object which uses the service account

diff --git a/staging/src/k8s.io/client-go/rest/config_test.go b/staging/src/k8s.io/client-go/rest/config_test.go
@@ -19,6 +19,7 @@ package rest
 import (
 	"io"
 	"net/http"
+	"path/filepath"
 	"reflect"
 	"strings"
 	"testing"
@@ -32,6 +33,7 @@ import (
 	clientcmdapi "k8s.io/client-go/tools/clientcmd/api"
 	"k8s.io/client-go/util/flowcontrol"
 
+	"github.com/stretchr/testify/assert"
 	_ "k8s.io/client-go/pkg/api/install"
 )
 
@@ -100,6 +102,42 @@ func TestSetKubernetesDefaultsUserAgent(t *testing.T) {
 	}
 }
 
+func TestAdjustVersion(t *testing.T) {
+	assert := assert.New(t)
+	assert.Equal("1.2.3", adjustVersion("1.2.3-alpha4"))
+	assert.Equal("1.2.3", adjustVersion("1.2.3-alpha"))
+	assert.Equal("1.2.3", adjustVersion("1.2.3"))
+	assert.Equal("unknown", adjustVersion(""))
+}
+
+func TestAdjustCommit(t *testing.T) {
+	assert := assert.New(t)
+	assert.Equal("1234567", adjustCommit("1234567890"))
+	assert.Equal("123456", adjustCommit("123456"))
+	assert.Equal("unknown", adjustCommit(""))
+}
+
+func TestAdjustCommand(t *testing.T) {
+	assert := assert.New(t)
+	assert.Equal("beans", adjustCommand(filepath.Join("home", "bob", "Downloads", "beans")))
+	assert.Equal("beans", adjustCommand(filepath.Join(".", "beans")))
+	assert.Equal("beans", adjustCommand("beans"))
+	assert.Equal("unknown", adjustCommand(""))
+}
+
+func TestBuildUserAgent(t *testing.T) {
+	assert.New(t).Equal(
+		"lynx/nicest (beos/itanium) kubernetes/baaaaaaaaad",
+		buildUserAgent(
+			"lynx", "nicest",
+			"beos", "itanium", "baaaaaaaaad"))
+}
+
+// This function untestable since it doesn't accept arguments.
+func TestDefaultKubernetesUserAgent(t *testing.T) {
+	assert.New(t).Contains(DefaultKubernetesUserAgent(), "kubernetes")
+}
+
 func TestRESTClientRequires(t *testing.T) {
 	if _, err := RESTClientFor(&Config{Host: "127.0.0.1", ContentConfig: ContentConfig{NegotiatedSerializer: api.Codecs}}); err == nil {
 		t.Errorf("unexpected non-error")

diff --git a/vendor/BUILD b/vendor/BUILD
@@ -14565,6 +14565,7 @@ go_test(
     tags = ["automanaged"],
     deps = [
         "//vendor:github.com/google/gofuzz",
+        "//vendor:github.com/stretchr/testify/assert",
         "//vendor:k8s.io/apimachinery/pkg/api/equality",
         "//vendor:k8s.io/apimachinery/pkg/api/errors",
         "//vendor:k8s.io/apimachinery/pkg/apis/meta/v1",