Skip to content

Commit 69b501a

Browse files
authored
Adds a user-agent identifier for preflight calls (#779)
1 parent 896f866 commit 69b501a

4 files changed

Lines changed: 101 additions & 10 deletions

File tree

cmd/preflight/preflight.go

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,14 @@ import (
88
"os"
99
"os/signal"
1010
"strconv"
11+
"strings"
1112
"syscall"
1213
"time"
1314

1415
"github.com/alecthomas/kong"
1516
"github.com/google/uuid"
1617

18+
"github.com/buildkite/cli/v3/cmd/version"
1719
buildstate "github.com/buildkite/cli/v3/internal/build/state"
1820
"github.com/buildkite/cli/v3/internal/build/watch"
1921
"github.com/buildkite/cli/v3/internal/cli"
@@ -39,13 +41,28 @@ var (
3941
newFactory = factory.New
4042
)
4143

44+
func preflightUserAgentSuffix() string {
45+
major := strings.TrimPrefix(version.Version, "v")
46+
if i := strings.IndexByte(major, '.'); i >= 0 {
47+
major = major[:i]
48+
}
49+
if major == "" || major == "DEV" {
50+
major = "DEV"
51+
}
52+
return "buildkite-cli-preflight/" + major + ".x"
53+
}
54+
4255
func (c *PreflightCmd) Help() string {
4356
return `Snapshots your working tree (uncommitted, staged, and untracked changes) and pushes it to a bk/preflight/<id> branch. If there are no local changes, pushes HEAD directly.`
4457
}
4558

4659
func (c *PreflightCmd) Run(kongCtx *kong.Context, globals cli.GlobalFlags) error {
4760
rlTransport := bkhttp.NewRateLimitTransport(http.DefaultTransport)
48-
f, err := newFactory(factory.WithDebug(globals.EnableDebug()), factory.WithTransport(rlTransport))
61+
f, err := newFactory(
62+
factory.WithDebug(globals.EnableDebug()),
63+
factory.WithTransport(rlTransport),
64+
factory.WithUserAgentSuffix(preflightUserAgentSuffix()),
65+
)
4966
if err != nil {
5067
return bkErrors.NewInternalError(err, "failed to initialize CLI", "This is likely a bug", "Report to Buildkite")
5168
}

cmd/preflight/preflight_test.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,8 +78,10 @@ func TestPreflightCmd_Run(t *testing.T) {
7878
t.Setenv("BUILDKITE_EXPERIMENTS", "preflight")
7979

8080
var gotReq buildkite.CreateBuild
81+
var gotUserAgent string
8182
s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
8283
if r.Method == "POST" && strings.Contains(r.URL.Path, "/builds") {
84+
gotUserAgent = r.Header.Get("User-Agent")
8385
json.NewDecoder(r.Body).Decode(&gotReq)
8486
w.Header().Set("Content-Type", "application/json")
8587
json.NewEncoder(w).Encode(buildkite.Build{
@@ -128,6 +130,12 @@ func TestPreflightCmd_Run(t *testing.T) {
128130
if gotReq.Env["BUILDKITE_PREFLIGHT"] != "true" {
129131
t.Errorf("expected BUILDKITE_PREFLIGHT=true (deprecated), got %#v", gotReq.Env)
130132
}
133+
if !strings.Contains(gotUserAgent, buildkite.DefaultUserAgent) {
134+
t.Errorf("expected User-Agent to contain %q, got %q", buildkite.DefaultUserAgent, gotUserAgent)
135+
}
136+
if !strings.Contains(gotUserAgent, "buildkite-cli-preflight/") {
137+
t.Errorf("expected User-Agent to contain preflight token, got %q", gotUserAgent)
138+
}
131139
})
132140

133141
t.Run("falls back to git cli when factory cannot open repository", func(t *testing.T) {

pkg/cmd/factory/factory.go

Lines changed: 27 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import (
1616
git "github.com/go-git/go-git/v5"
1717
)
1818

19-
var userAgent string
19+
var baseUserAgent string
2020

2121
type Factory struct {
2222
Config *config.Config
@@ -35,9 +35,10 @@ type Factory struct {
3535
type FactoryOpt func(*factoryConfig)
3636

3737
type factoryConfig struct {
38-
debug bool
39-
orgOverride string
40-
transport http.RoundTripper
38+
debug bool
39+
orgOverride string
40+
transport http.RoundTripper
41+
userAgentSuffix string
4142
}
4243

4344
// WithDebug enables debug output for REST API calls
@@ -64,6 +65,13 @@ func WithTransport(t http.RoundTripper) FactoryOpt {
6465
}
6566
}
6667

68+
// WithUserAgentSuffix appends an extra product token to the default user agent.
69+
func WithUserAgentSuffix(suffix string) FactoryOpt {
70+
return func(c *factoryConfig) {
71+
c.userAgentSuffix = suffix
72+
}
73+
}
74+
6775
// debugTransport wraps an http.RoundTripper and logs requests/responses with sensitive headers redacted
6876
type debugTransport struct {
6977
transport http.RoundTripper
@@ -129,17 +137,25 @@ func redactHeaders(headers http.Header) {
129137
}
130138

131139
type gqlHTTPClient struct {
132-
client *http.Client
133-
token string
140+
client *http.Client
141+
token string
142+
userAgent string
134143
}
135144

136145
func init() {
137-
userAgent = fmt.Sprintf("%s buildkite-cli/%s", buildkite.DefaultUserAgent, version.Version)
146+
baseUserAgent = fmt.Sprintf("%s buildkite-cli/%s", buildkite.DefaultUserAgent, version.Version)
147+
}
148+
149+
func buildUserAgent(suffix string) string {
150+
if suffix == "" {
151+
return baseUserAgent
152+
}
153+
return fmt.Sprintf("%s %s", baseUserAgent, suffix)
138154
}
139155

140156
func (a *gqlHTTPClient) Do(req *http.Request) (*http.Response, error) {
141157
req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", a.token))
142-
req.Header.Set("User-Agent", userAgent)
158+
req.Header.Set("User-Agent", a.userAgent)
143159
return a.client.Do(req)
144160
}
145161

@@ -165,6 +181,8 @@ func New(opts ...FactoryOpt) (*Factory, error) {
165181
}
166182
}
167183

184+
userAgent := buildUserAgent(cfg.userAgentSuffix)
185+
168186
// Build client options
169187
clientOpts := []buildkite.ClientOpt{
170188
buildkite.WithBaseURL(conf.RESTAPIEndpoint()),
@@ -188,7 +206,7 @@ func New(opts ...FactoryOpt) (*Factory, error) {
188206
return nil, fmt.Errorf("creating buildkite client: %w", err)
189207
}
190208

191-
graphqlHTTPClient := &gqlHTTPClient{client: http.DefaultClient, token: token}
209+
graphqlHTTPClient := &gqlHTTPClient{client: http.DefaultClient, token: token, userAgent: userAgent}
192210

193211
return &Factory{
194212
Config: conf,

pkg/cmd/factory/factory_test.go

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ import (
66
"net/http/httptest"
77
"strings"
88
"testing"
9+
10+
buildkite "github.com/buildkite/go-buildkite/v4"
911
)
1012

1113
func TestRedactHeaders(t *testing.T) {
@@ -152,3 +154,49 @@ func TestDebugTransportHandlesNilBody(t *testing.T) {
152154
t.Errorf("expected status 200, got %d", resp.StatusCode)
153155
}
154156
}
157+
158+
func TestBuildUserAgent(t *testing.T) {
159+
t.Run("default user agent has no preflight suffix", func(t *testing.T) {
160+
got := buildUserAgent("")
161+
if !strings.Contains(got, buildkite.DefaultUserAgent) {
162+
t.Fatalf("expected default user agent %q in %q", buildkite.DefaultUserAgent, got)
163+
}
164+
if strings.Contains(got, "buildkite-cli-preflight/") {
165+
t.Fatalf("expected no preflight suffix in %q", got)
166+
}
167+
})
168+
169+
t.Run("preflight suffix is appended when requested", func(t *testing.T) {
170+
got := buildUserAgent("buildkite-cli-preflight/3.x")
171+
if !strings.Contains(got, buildkite.DefaultUserAgent) {
172+
t.Fatalf("expected default user agent %q in %q", buildkite.DefaultUserAgent, got)
173+
}
174+
if !strings.Contains(got, "buildkite-cli-preflight/3.x") {
175+
t.Fatalf("expected preflight suffix in %q", got)
176+
}
177+
})
178+
}
179+
180+
func TestNewUserAgent(t *testing.T) {
181+
t.Chdir(t.TempDir())
182+
183+
t.Run("non-preflight factory does not set preflight suffix", func(t *testing.T) {
184+
f, err := New()
185+
if err != nil {
186+
t.Fatalf("New() error = %v", err)
187+
}
188+
if strings.Contains(f.RestAPIClient.UserAgent, "buildkite-cli-preflight/") {
189+
t.Fatalf("expected no preflight suffix in %q", f.RestAPIClient.UserAgent)
190+
}
191+
})
192+
193+
t.Run("factory can opt in to preflight suffix", func(t *testing.T) {
194+
f, err := New(WithUserAgentSuffix("buildkite-cli-preflight/3.x"))
195+
if err != nil {
196+
t.Fatalf("New() error = %v", err)
197+
}
198+
if !strings.Contains(f.RestAPIClient.UserAgent, "buildkite-cli-preflight/3.x") {
199+
t.Fatalf("expected preflight suffix in %q", f.RestAPIClient.UserAgent)
200+
}
201+
})
202+
}

0 commit comments

Comments
 (0)