From 633aab4beaba27de287b09ff43d9bafc220d1444 Mon Sep 17 00:00:00 2001 From: Jeffrey Regan Date: Tue, 11 Apr 2017 14:11:52 -0700 Subject: [PATCH] Use OS-specific libs when computing client User-Agent. **What this PR does / why we need it**: The User-Agent reported by clients (e.g. kubectl) in request headers should include the name of the client executable but not the full path to that executable. This PR changes how this name is determined by using the operating-system specific package "path/filepath" (meant for working with file system paths) instead of the "path" package (meant for URL paths). This fixes a problem on the Windows OS in the case where, if the user has not set their PATH to point to the location of their client executable, the User-Agent unnecessarily includes the full path. Fixes: #44419 Kubernetes-commit: 04f993250bc7a1aef0f2874d440ddb4bec1012c5 --- rest/BUILD | 1 + rest/config.go | 58 +++++++++++++++++++++++++++++++++++---------- rest/config_test.go | 38 +++++++++++++++++++++++++++++ 3 files changed, 84 insertions(+), 13 deletions(-) diff --git a/rest/BUILD b/rest/BUILD index 48b222bc..823539a3 100644 --- a/rest/BUILD +++ b/rest/BUILD @@ -22,6 +22,7 @@ go_test( tags = ["automanaged"], deps = [ "//vendor/github.com/google/gofuzz:go_default_library", + "//vendor/github.com/stretchr/testify/assert:go_default_library", "//vendor/k8s.io/apimachinery/pkg/api/equality:go_default_library", "//vendor/k8s.io/apimachinery/pkg/api/errors:go_default_library", "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", diff --git a/rest/config.go b/rest/config.go index 2a2c03df..07e724f2 100644 --- a/rest/config.go +++ b/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. +// 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" + } + 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" + } + 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 { - commit := version.Get().GitCommit - if len(commit) > 7 { - commit = commit[:7] - } - if len(commit) == 0 { - commit = "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 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/rest/config_test.go b/rest/config_test.go index 5b9071d7..913493bd 100644 --- a/rest/config_test.go +++ b/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")