From d675f3e92440753bb5240453a8336da28d507973 Mon Sep 17 00:00:00 2001
From: Lachlan Donald <lachlan@buildkite.com>
Date: Mon, 20 May 2019 16:41:56 +1000
Subject: [PATCH 1/4] Fold all api services down into the one client

---
 api/agents.go                   |  24 +++-----
 api/annotations.go              |  14 ++---
 api/artifacts.go                |  30 ++++-----
 api/chunks.go                   |  12 +---
 api/{buildkite.go => client.go} | 104 +++++++++++---------------------
 api/header_times.go             |  14 ++---
 api/heartbeats.go               |  14 ++---
 api/jobs.go                     |  54 +++++++++--------
 api/meta_data.go                |  24 +++-----
 api/pings.go                    |  12 +---
 api/pipelines.go                |  12 +---
 api/steps.go                    |   9 ---
 12 files changed, 114 insertions(+), 209 deletions(-)
 rename api/{buildkite.go => client.go} (72%)
 delete mode 100644 api/steps.go

diff --git a/api/agents.go b/api/agents.go
index 254a9d5b29..26fd51cb7a 100644
--- a/api/agents.go
+++ b/api/agents.go
@@ -1,11 +1,5 @@
 package api
 
-// AgentsService handles communication with the agent related methods of the
-// Buildkite Agent API.
-type AgentsService struct {
-	client *Client
-}
-
 // AgentRegisterRequest is a call to register on the Buildkite Agent API
 type AgentRegisterRequest struct {
 	Name              string   `json:"name"`
@@ -35,14 +29,14 @@ type AgentRegisterResponse struct {
 
 // Registers the agent against the Buildkite Agent API. The client for this
 // call must be authenticated using an Agent Registration Token
-func (as *AgentsService) Register(regReq *AgentRegisterRequest) (*AgentRegisterResponse, *Response, error) {
-	req, err := as.client.NewRequest("POST", "register", regReq)
+func (c *Client) Register(regReq *AgentRegisterRequest) (*AgentRegisterResponse, *Response, error) {
+	req, err := c.newRequest("POST", "register", regReq)
 	if err != nil {
 		return nil, nil, err
 	}
 
 	a := new(AgentRegisterResponse)
-	resp, err := as.client.Do(req, a)
+	resp, err := c.doRequest(req, a)
 	if err != nil {
 		return nil, resp, err
 	}
@@ -51,21 +45,21 @@ func (as *AgentsService) Register(regReq *AgentRegisterRequest) (*AgentRegisterR
 }
 
 // Connects the agent to the Buildkite Agent API
-func (as *AgentsService) Connect() (*Response, error) {
-	req, err := as.client.NewRequest("POST", "connect", nil)
+func (c *Client) Connect() (*Response, error) {
+	req, err := c.newRequest("POST", "connect", nil)
 	if err != nil {
 		return nil, err
 	}
 
-	return as.client.Do(req, nil)
+	return c.doRequest(req, nil)
 }
 
 // Disconnects the agent to the Buildkite Agent API
-func (as *AgentsService) Disconnect() (*Response, error) {
-	req, err := as.client.NewRequest("POST", "disconnect", nil)
+func (c *Client) Disconnect() (*Response, error) {
+	req, err := c.newRequest("POST", "disconnect", nil)
 	if err != nil {
 		return nil, err
 	}
 
-	return as.client.Do(req, nil)
+	return c.doRequest(req, nil)
 }
diff --git a/api/annotations.go b/api/annotations.go
index 497e369063..3d7aa87429 100644
--- a/api/annotations.go
+++ b/api/annotations.go
@@ -2,12 +2,6 @@ package api
 
 import "fmt"
 
-// AnnotationsService handles communication with the annotation related methods of the
-// Buildkite Agent API.
-type AnnotationsService struct {
-	client *Client
-}
-
 // Annotation represents a Buildkite Agent API Annotation
 type Annotation struct {
 	Body    string `json:"body,omitempty"`
@@ -16,14 +10,14 @@ type Annotation struct {
 	Append  bool   `json:"append,omitempty"`
 }
 
-// Annotates a build in the Buildkite UI
-func (cs *AnnotationsService) Create(jobId string, annotation *Annotation) (*Response, error) {
+// Annotate a build in the Buildkite UI
+func (c *Client) Annotate(jobId string, annotation *Annotation) (*Response, error) {
 	u := fmt.Sprintf("jobs/%s/annotations", jobId)
 
-	req, err := cs.client.NewRequest("POST", u, annotation)
+	req, err := c.newRequest("POST", u, annotation)
 	if err != nil {
 		return nil, err
 	}
 
-	return cs.client.Do(req, nil)
+	return c.doRequest(req, nil)
 }
diff --git a/api/artifacts.go b/api/artifacts.go
index 07fb6c644a..de281da0b4 100644
--- a/api/artifacts.go
+++ b/api/artifacts.go
@@ -4,12 +4,6 @@ import (
 	"fmt"
 )
 
-// ArtifactsService handles communication with the artifact related methods of
-// the Buildkite Artifact API.
-type ArtifactsService struct {
-	client *Client
-}
-
 // Artifact represents an artifact on the Buildkite Agent API
 type Artifact struct {
 	// The ID of the artifact. The ID is assigned to it after a successful
@@ -83,17 +77,17 @@ type ArtifactBatchUpdateRequest struct {
 	Artifacts []*ArtifactBatchUpdateArtifact `json:"artifacts"`
 }
 
-// Accepts a slice of artifacts, and creates them on Buildkite as a batch.
-func (as *ArtifactsService) Create(jobId string, batch *ArtifactBatch) (*ArtifactBatchCreateResponse, *Response, error) {
+// CreateArtifacts takes a slice of artifacts, and creates them on Buildkite as a batch.
+func (c *Client) CreateArtifacts(jobId string, batch *ArtifactBatch) (*ArtifactBatchCreateResponse, *Response, error) {
 	u := fmt.Sprintf("jobs/%s/artifacts", jobId)
 
-	req, err := as.client.NewRequest("POST", u, batch)
+	req, err := c.newRequest("POST", u, batch)
 	if err != nil {
 		return nil, nil, err
 	}
 
 	createResponse := new(ArtifactBatchCreateResponse)
-	resp, err := as.client.Do(req, createResponse)
+	resp, err := c.doRequest(req, createResponse)
 	if err != nil {
 		return nil, resp, err
 	}
@@ -101,8 +95,8 @@ func (as *ArtifactsService) Create(jobId string, batch *ArtifactBatch) (*Artifac
 	return createResponse, resp, err
 }
 
-// Updates a paticular artifact
-func (as *ArtifactsService) Update(jobId string, artifactStates map[string]string) (*Response, error) {
+// Updates a particular artifact
+func (c *Client) UpdateArtifacts(jobId string, artifactStates map[string]string) (*Response, error) {
 	u := fmt.Sprintf("jobs/%s/artifacts", jobId)
 	payload := ArtifactBatchUpdateRequest{}
 
@@ -110,12 +104,12 @@ func (as *ArtifactsService) Update(jobId string, artifactStates map[string]strin
 		payload.Artifacts = append(payload.Artifacts, &ArtifactBatchUpdateArtifact{id, state})
 	}
 
-	req, err := as.client.NewRequest("PUT", u, payload)
+	req, err := c.newRequest("PUT", u, payload)
 	if err != nil {
 		return nil, err
 	}
 
-	resp, err := as.client.Do(req, nil)
+	resp, err := c.doRequest(req, nil)
 	if err != nil {
 		return resp, err
 	}
@@ -123,21 +117,21 @@ func (as *ArtifactsService) Update(jobId string, artifactStates map[string]strin
 	return resp, err
 }
 
-// Searches Buildkite for a set of artifacts
-func (as *ArtifactsService) Search(buildId string, opt *ArtifactSearchOptions) ([]*Artifact, *Response, error) {
+// SearchArtifacts searches Buildkite for a set of artifacts
+func (c *Client) SearchArtifacts(buildId string, opt *ArtifactSearchOptions) ([]*Artifact, *Response, error) {
 	u := fmt.Sprintf("builds/%s/artifacts/search", buildId)
 	u, err := addOptions(u, opt)
 	if err != nil {
 		return nil, nil, err
 	}
 
-	req, err := as.client.NewRequest("GET", u, nil)
+	req, err := c.newRequest("GET", u, nil)
 	if err != nil {
 		return nil, nil, err
 	}
 
 	a := []*Artifact{}
-	resp, err := as.client.Do(req, &a)
+	resp, err := c.doRequest(req, &a)
 	if err != nil {
 		return nil, resp, err
 	}
diff --git a/api/chunks.go b/api/chunks.go
index 5b8cfc0a36..43df0be321 100644
--- a/api/chunks.go
+++ b/api/chunks.go
@@ -6,12 +6,6 @@ import (
 	"fmt"
 )
 
-// ChunksService handles communication with the chunk related methods of the
-// Buildkite Agent API.
-type ChunksService struct {
-	client *Client
-}
-
 // Chunk represents a Buildkite Agent API Chunk
 type Chunk struct {
 	Data     string
@@ -22,7 +16,7 @@ type Chunk struct {
 
 // Uploads the chunk to the Buildkite Agent API. This request sends the
 // compressed log directly as a request body.
-func (cs *ChunksService) Upload(jobId string, chunk *Chunk) (*Response, error) {
+func (c *Client) UploadChunks(jobId string, chunk *Chunk) (*Response, error) {
 	// Create a compressed buffer of the log content
 	body := &bytes.Buffer{}
 	gzipper := gzip.NewWriter(body)
@@ -33,7 +27,7 @@ func (cs *ChunksService) Upload(jobId string, chunk *Chunk) (*Response, error) {
 
 	// Pass most params as query
 	u := fmt.Sprintf("jobs/%s/chunks?sequence=%d&offset=%d&size=%d", jobId, chunk.Sequence, chunk.Offset, chunk.Size)
-	req, err := cs.client.NewFormRequest("POST", u, body)
+	req, err := c.newFormRequest("POST", u, body)
 	if err != nil {
 		return nil, err
 	}
@@ -42,5 +36,5 @@ func (cs *ChunksService) Upload(jobId string, chunk *Chunk) (*Response, error) {
 	req.Header.Add("Content-Type", "text/plain")
 	req.Header.Add("Content-Encoding", "gzip")
 
-	return cs.client.Do(req, nil)
+	return c.doRequest(req, nil)
 }
diff --git a/api/buildkite.go b/api/client.go
similarity index 72%
rename from api/buildkite.go
rename to api/client.go
index f8ec8aef7a..884949f402 100644
--- a/api/buildkite.go
+++ b/api/client.go
@@ -7,10 +7,8 @@ import (
 	"fmt"
 	"io"
 	"io/ioutil"
-	"mime/multipart"
 	"net/http"
 	"net/http/httputil"
-	"net/textproto"
 	"net/url"
 	"reflect"
 	"strings"
@@ -25,14 +23,8 @@ const (
 	defaultUserAgent = "buildkite-agent/api"
 )
 
-// A Client manages communication with the Buildkite Agent API.
-type Client struct {
-	// HTTP client used to communicate with the API.
-	client *http.Client
-
-	// The logger used
-	logger logger.Logger
-
+// ClientConfig is configuration for Client
+type ClientConfig struct {
 	// Base URL for API requests. Defaults to the public Buildkite Agent API.
 	// The URL should always be specified with a trailing slash.
 	BaseURL *url.URL
@@ -42,43 +34,35 @@ type Client struct {
 
 	// If true, requests and responses will be dumped and set to the logger
 	DebugHTTP bool
+}
+
+// A Client manages communication with the Buildkite Agent API.
+type Client struct {
+	// The client configuration
+	conf ClientConfig
 
-	// Services used for talking to different parts of the Buildkite Agent API.
-	Agents      *AgentsService
-	Pings       *PingsService
-	Jobs        *JobsService
-	Chunks      *ChunksService
-	MetaData    *MetaDataService
-	HeaderTimes *HeaderTimesService
-	Artifacts   *ArtifactsService
-	Pipelines   *PipelinesService
-	Heartbeats  *HeartbeatsService
-	Annotations *AnnotationsService
+	// HTTP client used to communicate with the API.
+	client *http.Client
+
+	// The logger used
+	logger logger.Logger
 }
 
 // NewClient returns a new Buildkite Agent API Client.
-func NewClient(httpClient *http.Client, l logger.Logger) *Client {
-	baseURL, _ := url.Parse(defaultBaseURL)
-
-	c := &Client{
-		logger:    l,
-		client:    httpClient,
-		BaseURL:   baseURL,
-		UserAgent: defaultUserAgent,
+func NewClient(httpClient *http.Client, l logger.Logger, conf ClientConfig) *Client {
+	if conf.BaseURL == nil {
+		conf.BaseURL, _ = url.Parse(defaultBaseURL)
+	}
+
+	if conf.UserAgent == "" {
+		conf.UserAgent = defaultUserAgent
 	}
 
-	c.Agents = &AgentsService{c}
-	c.Pings = &PingsService{c}
-	c.Jobs = &JobsService{c}
-	c.Chunks = &ChunksService{c}
-	c.MetaData = &MetaDataService{c}
-	c.HeaderTimes = &HeaderTimesService{c}
-	c.Artifacts = &ArtifactsService{c}
-	c.Pipelines = &PipelinesService{c}
-	c.Heartbeats = &HeartbeatsService{c}
-	c.Annotations = &AnnotationsService{c}
-
-	return c
+	return &Client{
+		logger: l,
+		client: httpClient,
+		conf:   conf,
+	}
 }
 
 // NewRequest creates an API request. A relative URL can be provided in urlStr,
@@ -86,8 +70,8 @@ func NewClient(httpClient *http.Client, l logger.Logger) *Client {
 // Relative URLs should always be specified without a preceding slash. If
 // specified, the value pointed to by body is JSON encoded and included as the
 // request body.
-func (c *Client) NewRequest(method, urlStr string, body interface{}) (*http.Request, error) {
-	u := joinURL(c.BaseURL.String(), urlStr)
+func (c *Client) newRequest(method, urlStr string, body interface{}) (*http.Request, error) {
+	u := joinURL(c.conf.BaseURL.String(), urlStr)
 
 	buf := new(bytes.Buffer)
 	if body != nil {
@@ -102,7 +86,7 @@ func (c *Client) NewRequest(method, urlStr string, body interface{}) (*http.Requ
 		return nil, err
 	}
 
-	req.Header.Add("User-Agent", c.UserAgent)
+	req.Header.Add("User-Agent", c.conf.UserAgent)
 
 	if body != nil {
 		req.Header.Add("Content-Type", "application/json")
@@ -115,16 +99,16 @@ func (c *Client) NewRequest(method, urlStr string, body interface{}) (*http.Requ
 // provided in urlStr, in which case it is resolved relative to the UploadURL
 // of the Client. Relative URLs should always be specified without a preceding
 // slash.
-func (c *Client) NewFormRequest(method, urlStr string, body *bytes.Buffer) (*http.Request, error) {
-	u := joinURL(c.BaseURL.String(), urlStr)
+func (c *Client) newFormRequest(method, urlStr string, body *bytes.Buffer) (*http.Request, error) {
+	u := joinURL(c.conf.BaseURL.String(), urlStr)
 
 	req, err := http.NewRequest(method, u, body)
 	if err != nil {
 		return nil, err
 	}
 
-	if c.UserAgent != "" {
-		req.Header.Add("User-Agent", c.UserAgent)
+	if c.conf.UserAgent != "" {
+		req.Header.Add("User-Agent", c.conf.UserAgent)
 	}
 
 	return req, nil
@@ -147,10 +131,10 @@ func newResponse(r *http.Response) *Response {
 // error if an API error has occurred.  If v implements the io.Writer
 // interface, the raw response body will be written to v, without attempting to
 // first decode it.
-func (c *Client) Do(req *http.Request, v interface{}) (*Response, error) {
+func (c *Client) doRequest(req *http.Request, v interface{}) (*Response, error) {
 	var err error
 
-	if c.DebugHTTP {
+	if c.conf.DebugHTTP {
 		// If the request is a multi-part form, then it's probably a
 		// file upload, in which case we don't want to spewing out the
 		// file contents into the debug log (especially if it's been
@@ -185,7 +169,7 @@ func (c *Client) Do(req *http.Request, v interface{}) (*Response, error) {
 
 	response := newResponse(resp)
 
-	if c.DebugHTTP {
+	if c.conf.DebugHTTP {
 		responseDump, err := httputil.DumpResponse(resp, true)
 		c.logger.Debug("\nERR: %s\n%s", err, string(responseDump))
 	}
@@ -266,24 +250,6 @@ func addOptions(s string, opt interface{}) (string, error) {
 	return u.String(), nil
 }
 
-// Copied from http://golang.org/src/mime/multipart/writer.go
-var quoteEscaper = strings.NewReplacer("\\", "\\\\", `"`, "\\\"")
-
-func escapeQuotes(s string) string {
-	return quoteEscaper.Replace(s)
-}
-
-// createFormFileWithContentType is a copy of the CreateFormFile method, except
-// you can change the content type it uses (by default you can't)
-func createFormFileWithContentType(w *multipart.Writer, fieldname, filename, contentType string) (io.Writer, error) {
-	h := make(textproto.MIMEHeader)
-	h.Set("Content-Disposition",
-		fmt.Sprintf(`form-data; name="%s"; filename="%s"`,
-			escapeQuotes(fieldname), escapeQuotes(filename)))
-	h.Set("Content-Type", contentType)
-	return w.CreatePart(h)
-}
-
 func joinURL(endpoint string, path string) string {
 	return strings.TrimRight(endpoint, "/") + "/" + strings.TrimLeft(path, "/")
 }
diff --git a/api/header_times.go b/api/header_times.go
index ed9c2e8ecc..057cf22161 100644
--- a/api/header_times.go
+++ b/api/header_times.go
@@ -4,28 +4,22 @@ import (
 	"fmt"
 )
 
-// HeaderTimesService handles communication with the meta data related methods
-// of the Buildkite Agent API.
-type HeaderTimesService struct {
-	client *Client
-}
-
 // HeaderTimes represents a set of header times that are associated with a job
 // log.
 type HeaderTimes struct {
 	Times map[string]string `json:"header_times"`
 }
 
-// Saves the header times to the job
-func (hs *HeaderTimesService) Save(jobId string, headerTimes *HeaderTimes) (*Response, error) {
+// SaveHeaderTimes saves the header times to the job
+func (c *Client) SaveHeaderTimes(jobId string, headerTimes *HeaderTimes) (*Response, error) {
 	u := fmt.Sprintf("jobs/%s/header_times", jobId)
 
-	req, err := hs.client.NewRequest("POST", u, headerTimes)
+	req, err := c.newRequest("POST", u, headerTimes)
 	if err != nil {
 		return nil, err
 	}
 
-	resp, err := hs.client.Do(req, nil)
+	resp, err := c.doRequest(req, nil)
 	if err != nil {
 		return resp, err
 	}
diff --git a/api/heartbeats.go b/api/heartbeats.go
index 37c97e8989..5de4a0b137 100644
--- a/api/heartbeats.go
+++ b/api/heartbeats.go
@@ -2,30 +2,24 @@ package api
 
 import "time"
 
-// HeartbeatsService handles communication with the ping related methods of the
-// Buildkite Agent API.
-type HeartbeatsService struct {
-	client *Client
-}
-
 // Heartbeat represents a Buildkite Agent API Heartbeat
 type Heartbeat struct {
 	SentAt     string `json:"sent_at"`
 	ReceivedAt string `json:"received_at,omitempty"`
 }
 
-// Heartbeats the API which keeps the agent connected to Buildkite
-func (hs *HeartbeatsService) Beat() (*Heartbeat, *Response, error) {
+// Heartbeat notifies Buildkite that an agent is still connected
+func (c *Client) Heartbeat() (*Heartbeat, *Response, error) {
 	// Include the current time in the heartbeat, and include the operating
 	// systems timezone.
 	heartbeat := &Heartbeat{SentAt: time.Now().Format(time.RFC3339Nano)}
 
-	req, err := hs.client.NewRequest("POST", "heartbeat", &heartbeat)
+	req, err := c.newRequest("POST", "heartbeat", &heartbeat)
 	if err != nil {
 		return nil, nil, err
 	}
 
-	resp, err := hs.client.Do(req, heartbeat)
+	resp, err := c.doRequest(req, heartbeat)
 	if err != nil {
 		return nil, resp, err
 	}
diff --git a/api/jobs.go b/api/jobs.go
index f5e832c6d3..8a0ec527c5 100644
--- a/api/jobs.go
+++ b/api/jobs.go
@@ -4,12 +4,6 @@ import (
 	"fmt"
 )
 
-// JobsService handles communication with the job related methods of the
-// Buildkite Agent API.
-type JobsService struct {
-	client *Client
-}
-
 // Job represents a Buildkite Agent API Job
 type Job struct {
 	ID                 string            `json:"id,omitempty"`
@@ -37,17 +31,17 @@ type jobFinishRequest struct {
 	ChunksFailedCount int    `json:"chunks_failed_count"`
 }
 
-// Fetches a job
-func (js *JobsService) GetState(id string) (*JobState, *Response, error) {
+// GetJobState returns the state of a given job
+func (c *Client) GetJobState(id string) (*JobState, *Response, error) {
 	u := fmt.Sprintf("jobs/%s", id)
 
-	req, err := js.client.NewRequest("GET", u, nil)
+	req, err := c.newRequest("GET", u, nil)
 	if err != nil {
 		return nil, nil, err
 	}
 
 	s := new(JobState)
-	resp, err := js.client.Do(req, s)
+	resp, err := c.doRequest(req, s)
 	if err != nil {
 		return nil, resp, err
 	}
@@ -55,19 +49,19 @@ func (js *JobsService) GetState(id string) (*JobState, *Response, error) {
 	return s, resp, err
 }
 
-// Accepts the passed in job. Returns the job with it's finalized set of
+// AcceptJob accepts the passed in job. Returns the job with it's finalized set of
 // environment variables (when a job is accepted, the agents environment is
 // applied to the job)
-func (js *JobsService) Accept(job *Job) (*Job, *Response, error) {
+func (c *Client) AcceptJob(job *Job) (*Job, *Response, error) {
 	u := fmt.Sprintf("jobs/%s/accept", job.ID)
 
-	req, err := js.client.NewRequest("PUT", u, nil)
+	req, err := c.newRequest("PUT", u, nil)
 	if err != nil {
 		return nil, nil, err
 	}
 
 	j := new(Job)
-	resp, err := js.client.Do(req, j)
+	resp, err := c.doRequest(req, j)
 	if err != nil {
 		return nil, resp, err
 	}
@@ -75,25 +69,25 @@ func (js *JobsService) Accept(job *Job) (*Job, *Response, error) {
 	return j, resp, err
 }
 
-// Starts the passed in job
-func (js *JobsService) Start(job *Job) (*Response, error) {
+// StartJob starts the passed in job
+func (c *Client) StartJob(job *Job) (*Response, error) {
 	u := fmt.Sprintf("jobs/%s/start", job.ID)
 
-	req, err := js.client.NewRequest("PUT", u, &jobStartRequest{
+	req, err := c.newRequest("PUT", u, &jobStartRequest{
 		StartedAt: job.StartedAt,
 	})
 	if err != nil {
 		return nil, err
 	}
 
-	return js.client.Do(req, nil)
+	return c.doRequest(req, nil)
 }
 
-// Finishes the passed in job
-func (js *JobsService) Finish(job *Job) (*Response, error) {
+// FinishJob finishes the passed in job
+func (c *Client) FinishJob(job *Job) (*Response, error) {
 	u := fmt.Sprintf("jobs/%s/finish", job.ID)
 
-	req, err := js.client.NewRequest("PUT", u, &jobFinishRequest{
+	req, err := c.newRequest("PUT", u, &jobFinishRequest{
 		FinishedAt:        job.FinishedAt,
 		ExitStatus:        job.ExitStatus,
 		ChunksFailedCount: job.ChunksFailedCount,
@@ -102,17 +96,25 @@ func (js *JobsService) Finish(job *Job) (*Response, error) {
 		return nil, err
 	}
 
-	return js.client.Do(req, nil)
+	return c.doRequest(req, nil)
+}
+
+// StepUpdate represents a change request to a step
+type StepUpdate struct {
+	UUID      string `json:"uuid,omitempty"`
+	Attribute string `json:"attribute,omitempty"`
+	Value     string `json:"value,omitempty"`
+	Append    bool   `json:"append,omitempty"`
 }
 
-// Updates a step
-func (js *JobsService) StepUpdate(jobId string, stepUpdate *StepUpdate) (*Response, error) {
+// StepUpdate updates a step
+func (c *Client) StepUpdate(jobId string, stepUpdate *StepUpdate) (*Response, error) {
 	u := fmt.Sprintf("jobs/%s/step_update", jobId)
 
-	req, err := js.client.NewRequest("PUT", u, stepUpdate)
+	req, err := c.newRequest("PUT", u, stepUpdate)
 	if err != nil {
 		return nil, err
 	}
 
-	return js.client.Do(req, nil)
+	return c.doRequest(req, nil)
 }
diff --git a/api/meta_data.go b/api/meta_data.go
index 88fd5c2a2a..e5ead8458b 100644
--- a/api/meta_data.go
+++ b/api/meta_data.go
@@ -4,12 +4,6 @@ import (
 	"fmt"
 )
 
-// MetaDataService handles communication with the meta data related methods of
-// the Buildkite Agent API.
-type MetaDataService struct {
-	client *Client
-}
-
 // MetaData represents a Buildkite Agent API MetaData
 type MetaData struct {
 	Key   string `json:"key,omitempty"`
@@ -23,28 +17,28 @@ type MetaDataExists struct {
 }
 
 // Sets the meta data value
-func (ps *MetaDataService) Set(jobId string, metaData *MetaData) (*Response, error) {
+func (c *Client) SetMetaData(jobId string, metaData *MetaData) (*Response, error) {
 	u := fmt.Sprintf("jobs/%s/data/set", jobId)
 
-	req, err := ps.client.NewRequest("POST", u, metaData)
+	req, err := c.newRequest("POST", u, metaData)
 	if err != nil {
 		return nil, err
 	}
 
-	return ps.client.Do(req, nil)
+	return c.doRequest(req, nil)
 }
 
 // Gets the meta data value
-func (ps *MetaDataService) Get(jobId string, key string) (*MetaData, *Response, error) {
+func (c *Client) GetMetaData(jobId string, key string) (*MetaData, *Response, error) {
 	u := fmt.Sprintf("jobs/%s/data/get", jobId)
 	m := &MetaData{Key: key}
 
-	req, err := ps.client.NewRequest("POST", u, m)
+	req, err := c.newRequest("POST", u, m)
 	if err != nil {
 		return nil, nil, err
 	}
 
-	resp, err := ps.client.Do(req, m)
+	resp, err := c.doRequest(req, m)
 	if err != nil {
 		return nil, resp, err
 	}
@@ -53,17 +47,17 @@ func (ps *MetaDataService) Get(jobId string, key string) (*MetaData, *Response,
 }
 
 // Returns true if the meta data key has been set, false if it hasn't.
-func (ps *MetaDataService) Exists(jobId string, key string) (*MetaDataExists, *Response, error) {
+func (c *Client) ExistsMetaData(jobId string, key string) (*MetaDataExists, *Response, error) {
 	u := fmt.Sprintf("jobs/%s/data/exists", jobId)
 	m := &MetaData{Key: key}
 
-	req, err := ps.client.NewRequest("POST", u, m)
+	req, err := c.newRequest("POST", u, m)
 	if err != nil {
 		return nil, nil, err
 	}
 
 	e := new(MetaDataExists)
-	resp, err := ps.client.Do(req, e)
+	resp, err := c.doRequest(req, e)
 	if err != nil {
 		return nil, resp, err
 	}
diff --git a/api/pings.go b/api/pings.go
index 3e82d0f58a..3dc8e8ade8 100644
--- a/api/pings.go
+++ b/api/pings.go
@@ -1,11 +1,5 @@
 package api
 
-// PingsService handles communication with the ping related methods of the
-// Buildkite Agent API.
-type PingsService struct {
-	client *Client
-}
-
 // Ping represents a Buildkite Agent API Ping
 type Ping struct {
 	Action   string `json:"action,omitempty"`
@@ -15,14 +9,14 @@ type Ping struct {
 }
 
 // Pings the API and returns any work the client needs to perform
-func (ps *PingsService) Get() (*Ping, *Response, error) {
-	req, err := ps.client.NewRequest("GET", "ping", nil)
+func (c *Client) Ping() (*Ping, *Response, error) {
+	req, err := c.newRequest("GET", "ping", nil)
 	if err != nil {
 		return nil, nil, err
 	}
 
 	ping := new(Ping)
-	resp, err := ps.client.Do(req, ping)
+	resp, err := c.doRequest(req, ping)
 	if err != nil {
 		return nil, resp, err
 	}
diff --git a/api/pipelines.go b/api/pipelines.go
index 862121707d..57d9a6c12e 100644
--- a/api/pipelines.go
+++ b/api/pipelines.go
@@ -2,12 +2,6 @@ package api
 
 import "fmt"
 
-// PipelinesService handles communication with the pipeline related methods of the
-// Buildkite Agent API.
-type PipelinesService struct {
-	client *Client
-}
-
 // Pipeline represents a Buildkite Agent API Pipeline
 type Pipeline struct {
 	UUID     string      `json:"uuid"`
@@ -17,13 +11,13 @@ type Pipeline struct {
 
 // Uploads the pipeline to the Buildkite Agent API. This request doesn't use JSON,
 // but a multi-part HTTP form upload
-func (cs *PipelinesService) Upload(jobId string, pipeline *Pipeline) (*Response, error) {
+func (c *Client) UploadPipeline(jobId string, pipeline *Pipeline) (*Response, error) {
 	u := fmt.Sprintf("jobs/%s/pipelines", jobId)
 
-	req, err := cs.client.NewRequest("POST", u, pipeline)
+	req, err := c.newRequest("POST", u, pipeline)
 	if err != nil {
 		return nil, err
 	}
 
-	return cs.client.Do(req, nil)
+	return c.doRequest(req, nil)
 }
diff --git a/api/steps.go b/api/steps.go
deleted file mode 100644
index baa1eb5dac..0000000000
--- a/api/steps.go
+++ /dev/null
@@ -1,9 +0,0 @@
-package api
-
-// StepUpdate represents a change request to a step
-type StepUpdate struct {
-	UUID      string `json:"uuid,omitempty"`
-	Attribute string `json:"attribute,omitempty"`
-	Value     string `json:"value,omitempty"`
-	Append    bool   `json:"append,omitempty"`
-}

From 5508feb2f5fa63ef062579af7d4a988f963ba2a1 Mon Sep 17 00:00:00 2001
From: Lachlan Donald <lachlan@buildkite.com>
Date: Mon, 20 May 2019 17:05:31 +1000
Subject: [PATCH 2/4] Update agent package to use iface for api

---
 agent/agent_worker.go           | 14 +++++++-------
 agent/api_client.go             | 11 ++++++-----
 agent/api_iface.go              | 31 +++++++++++++++++++++++++++++++
 agent/artifact_batch_creator.go |  6 +++---
 agent/artifact_downloader.go    | 18 ++++++++++--------
 agent/artifact_searcher.go      |  6 +++---
 agent/artifact_uploader.go      | 17 ++++++++++-------
 agent/job_runner.go             | 12 ++++++------
 agent/register.go               |  4 ++--
 api/chunks.go                   |  2 +-
 api/client.go                   |  2 ++
 clicommand/annotate.go          |  4 ++--
 clicommand/artifact_download.go |  1 +
 clicommand/artifact_upload.go   |  1 +
 clicommand/meta_data_exists.go  |  2 +-
 clicommand/meta_data_get.go     |  2 +-
 clicommand/meta_data_set.go     |  2 +-
 clicommand/pipeline_upload.go   |  2 +-
 clicommand/step_update.go       |  2 +-
 19 files changed, 90 insertions(+), 49 deletions(-)
 create mode 100755 agent/api_iface.go

diff --git a/agent/agent_worker.go b/agent/agent_worker.go
index b167cb7c97..e15e2e691f 100644
--- a/agent/agent_worker.go
+++ b/agent/agent_worker.go
@@ -37,7 +37,7 @@ type AgentWorker struct {
 	lastPing, lastHeartbeat int64
 
 	// The API Client used when this agent is communicating with the API
-	apiClient *api.Client
+	apiClient APIClient
 
 	// The logger instance to use
 	logger logger.Logger
@@ -257,7 +257,7 @@ func (a *AgentWorker) Connect() error {
 	a.UpdateProcTitle("connecting")
 
 	return retry.Do(func(s *retry.Stats) error {
-		_, err := a.apiClient.Agents.Connect()
+		_, err := a.apiClient.Connect()
 		if err != nil {
 			a.logger.Warn("%s (%s)", err, s)
 		}
@@ -273,7 +273,7 @@ func (a *AgentWorker) Heartbeat() error {
 
 	// Retry the heartbeat a few times
 	err = retry.Do(func(s *retry.Stats) error {
-		beat, _, err = a.apiClient.Heartbeats.Beat()
+		beat, _, err = a.apiClient.Heartbeat()
 		if err != nil {
 			a.logger.Warn("%s (%s)", err, s)
 		}
@@ -297,7 +297,7 @@ func (a *AgentWorker) Ping() (*api.Job, error) {
 	// Update the proc title
 	a.UpdateProcTitle("pinging")
 
-	ping, _, err := a.apiClient.Pings.Get()
+	ping, _, err := a.apiClient.Ping()
 	if err != nil {
 		// Get the last ping time to the nearest microsecond
 		lastPing := time.Unix(atomic.LoadInt64(&a.lastPing), 0)
@@ -320,7 +320,7 @@ func (a *AgentWorker) Ping() (*api.Job, error) {
 			Token:    a.agent.AccessToken,
 		})
 
-		newPing, _, err := newAPIClient.Pings.Get()
+		newPing, _, err := newAPIClient.Ping()
 		if err != nil {
 			a.logger.Warn("Failed to ping the new endpoint %s - ignoring switch for now (%s)", ping.Endpoint, err)
 		} else {
@@ -364,7 +364,7 @@ func (a *AgentWorker) AcceptAndRun(job *api.Job) error {
 	var accepted *api.Job
 	err := retry.Do(func(s *retry.Stats) error {
 		var err error
-		accepted, _, err = a.apiClient.Jobs.Accept(job)
+		accepted, _, err = a.apiClient.AcceptJob(job)
 		if err != nil {
 			if api.IsRetryableError(err) {
 				a.logger.Warn("%s (%s)", err, s)
@@ -422,7 +422,7 @@ func (a *AgentWorker) Disconnect() error {
 	// Update the proc title
 	a.UpdateProcTitle("disconnecting")
 
-	_, err := a.apiClient.Agents.Disconnect()
+	_, err := a.apiClient.Disconnect()
 	if err != nil {
 		a.logger.Warn("There was an error sending the disconnect API call to Buildkite. If this agent still appears online, you may have to manually stop it (%s)", err)
 	}
diff --git a/agent/api_client.go b/agent/api_client.go
index 2321fab252..ed752b5fa8 100644
--- a/agent/api_client.go
+++ b/agent/api_client.go
@@ -26,7 +26,7 @@ func APIClientEnableHTTPDebug() {
 	debugHTTP = true
 }
 
-func NewAPIClient(l logger.Logger, c APIClientConfig) *api.Client {
+func NewAPIClient(l logger.Logger, c APIClientConfig) APIClient {
 	httpTransport := &http.Transport{
 		Proxy:              http.ProxyFromEnvironment,
 		DisableCompression: false,
@@ -57,10 +57,11 @@ func NewAPIClient(l logger.Logger, c APIClientConfig) *api.Client {
 	}
 
 	// Create the Buildkite Agent API Client
-	client := api.NewClient(httpClient, l)
-	client.BaseURL = u
-	client.UserAgent = userAgent()
-	client.DebugHTTP = debugHTTP
+	client := api.NewClient(httpClient, l, api.ClientConfig{
+		BaseURL:   u,
+		UserAgent: userAgent(),
+		DebugHTTP: debugHTTP,
+	})
 
 	return client
 }
diff --git a/agent/api_iface.go b/agent/api_iface.go
new file mode 100755
index 0000000000..f36192c923
--- /dev/null
+++ b/agent/api_iface.go
@@ -0,0 +1,31 @@
+// Created by interfacer; DO NOT EDIT
+
+package agent
+
+import (
+	"github.com/buildkite/agent/api"
+)
+
+// APIClient is an interface generated for "github.com/buildkite/agent/api.Client".
+type APIClient interface {
+	AcceptJob(*api.Job) (*api.Job, *api.Response, error)
+	Annotate(string, *api.Annotation) (*api.Response, error)
+	Connect() (*api.Response, error)
+	CreateArtifacts(string, *api.ArtifactBatch) (*api.ArtifactBatchCreateResponse, *api.Response, error)
+	Disconnect() (*api.Response, error)
+	ExistsMetaData(string, string) (*api.MetaDataExists, *api.Response, error)
+	FinishJob(*api.Job) (*api.Response, error)
+	GetJobState(string) (*api.JobState, *api.Response, error)
+	GetMetaData(string, string) (*api.MetaData, *api.Response, error)
+	Heartbeat() (*api.Heartbeat, *api.Response, error)
+	Ping() (*api.Ping, *api.Response, error)
+	Register(*api.AgentRegisterRequest) (*api.AgentRegisterResponse, *api.Response, error)
+	SaveHeaderTimes(string, *api.HeaderTimes) (*api.Response, error)
+	SearchArtifacts(string, *api.ArtifactSearchOptions) ([]*api.Artifact, *api.Response, error)
+	SetMetaData(string, *api.MetaData) (*api.Response, error)
+	StartJob(*api.Job) (*api.Response, error)
+	StepUpdate(string, *api.StepUpdate) (*api.Response, error)
+	UpdateArtifacts(string, map[string]string) (*api.Response, error)
+	UploadChunk(string, *api.Chunk) (*api.Response, error)
+	UploadPipeline(string, *api.Pipeline) (*api.Response, error)
+}
diff --git a/agent/artifact_batch_creator.go b/agent/artifact_batch_creator.go
index be6748758d..83c9534819 100644
--- a/agent/artifact_batch_creator.go
+++ b/agent/artifact_batch_creator.go
@@ -27,10 +27,10 @@ type ArtifactBatchCreator struct {
 	logger logger.Logger
 
 	// The APIClient that will be used when uploading jobs
-	apiClient *api.Client
+	apiClient APIClient
 }
 
-func NewArtifactBatchCreator(l logger.Logger, ac *api.Client, c ArtifactBatchCreatorConfig) *ArtifactBatchCreator {
+func NewArtifactBatchCreator(l logger.Logger, ac APIClient, c ArtifactBatchCreatorConfig) *ArtifactBatchCreator {
 	return &ArtifactBatchCreator{
 		logger:    l,
 		conf:      c,
@@ -67,7 +67,7 @@ func (a *ArtifactBatchCreator) Create() ([]*api.Artifact, error) {
 
 		// Retry the batch upload a couple of times
 		err = retry.Do(func(s *retry.Stats) error {
-			creation, resp, err = a.apiClient.Artifacts.Create(a.conf.JobID, batch)
+			creation, resp, err = a.apiClient.CreateArtifacts(a.conf.JobID, batch)
 			if resp != nil && (resp.StatusCode == 401 || resp.StatusCode == 404 || resp.StatusCode == 500) {
 				s.Break()
 			}
diff --git a/agent/artifact_downloader.go b/agent/artifact_downloader.go
index ca3cdbae30..b4c0b77bce 100644
--- a/agent/artifact_downloader.go
+++ b/agent/artifact_downloader.go
@@ -8,7 +8,6 @@ import (
 	"path/filepath"
 	"strings"
 
-	"github.com/buildkite/agent/api"
 	"github.com/buildkite/agent/logger"
 	"github.com/buildkite/agent/pool"
 )
@@ -25,6 +24,9 @@ type ArtifactDownloaderConfig struct {
 
 	// Where we'll be downloading artifacts to
 	Destination string
+
+	// Whether to show HTTP debugging
+	DebugHTTP bool
 }
 
 type ArtifactDownloader struct {
@@ -34,11 +36,11 @@ type ArtifactDownloader struct {
 	// The logger instance to use
 	logger logger.Logger
 
-	// The *api.Client that will be used when uploading jobs
-	apiClient *api.Client
+	// The APIClient that will be used when uploading jobs
+	apiClient APIClient
 }
 
-func NewArtifactDownloader(l logger.Logger, ac *api.Client, c ArtifactDownloaderConfig) ArtifactDownloader {
+func NewArtifactDownloader(l logger.Logger, ac APIClient, c ArtifactDownloaderConfig) ArtifactDownloader {
 	return ArtifactDownloader{
 		logger:    l,
 		apiClient: ac,
@@ -90,7 +92,7 @@ func (a *ArtifactDownloader) Download() error {
 						Bucket:      artifact.UploadDestination,
 						Destination: downloadDestination,
 						Retries:     5,
-						DebugHTTP:   a.apiClient.DebugHTTP,
+						DebugHTTP:   a.conf.DebugHTTP,
 					}).Start()
 				} else if strings.HasPrefix(artifact.UploadDestination, "gs://") {
 					err = NewGSDownloader(a.logger, GSDownloaderConfig{
@@ -98,7 +100,7 @@ func (a *ArtifactDownloader) Download() error {
 						Bucket:      artifact.UploadDestination,
 						Destination: downloadDestination,
 						Retries:     5,
-						DebugHTTP:   a.apiClient.DebugHTTP,
+						DebugHTTP:   a.conf.DebugHTTP,
 					}).Start()
 				} else if strings.HasPrefix(artifact.UploadDestination, "rt://") {
 					err = NewArtifactoryDownloader(a.logger, ArtifactoryDownloaderConfig{
@@ -106,7 +108,7 @@ func (a *ArtifactDownloader) Download() error {
 						Repository:  artifact.UploadDestination,
 						Destination: downloadDestination,
 						Retries:     5,
-						DebugHTTP:   a.apiClient.DebugHTTP,
+						DebugHTTP:   a.conf.DebugHTTP,
 					}).Start()
 				} else {
 					err = NewDownload(a.logger, http.DefaultClient, DownloadConfig{
@@ -114,7 +116,7 @@ func (a *ArtifactDownloader) Download() error {
 						Path:        artifact.Path,
 						Destination: downloadDestination,
 						Retries:     5,
-						DebugHTTP:   a.apiClient.DebugHTTP,
+						DebugHTTP:   a.conf.DebugHTTP,
 					}).Start()
 				}
 
diff --git a/agent/artifact_searcher.go b/agent/artifact_searcher.go
index d8fae01563..6d81ea2385 100644
--- a/agent/artifact_searcher.go
+++ b/agent/artifact_searcher.go
@@ -10,13 +10,13 @@ type ArtifactSearcher struct {
 	logger logger.Logger
 
 	// The APIClient that will be used when uploading jobs
-	apiClient *api.Client
+	apiClient APIClient
 
 	// The ID of the Build that these artifacts belong to
 	buildID string
 }
 
-func NewArtifactSearcher(l logger.Logger, ac *api.Client, buildID string) *ArtifactSearcher {
+func NewArtifactSearcher(l logger.Logger, ac APIClient, buildID string) *ArtifactSearcher {
 	return &ArtifactSearcher{
 		logger:    l,
 		apiClient: ac,
@@ -31,7 +31,7 @@ func (a *ArtifactSearcher) Search(query string, scope string) ([]*api.Artifact,
 		a.logger.Info("Searching for artifacts: \"%s\" within step: \"%s\"", query, scope)
 	}
 
-	artifacts, _, err := a.apiClient.Artifacts.Search(a.buildID, &api.ArtifactSearchOptions{
+	artifacts, _, err := a.apiClient.SearchArtifacts(a.buildID, &api.ArtifactSearchOptions{
 		Query: query,
 		Scope: scope,
 	})
diff --git a/agent/artifact_uploader.go b/agent/artifact_uploader.go
index fb94090612..c76ac1c9f3 100644
--- a/agent/artifact_uploader.go
+++ b/agent/artifact_uploader.go
@@ -37,6 +37,9 @@ type ArtifactUploaderConfig struct {
 
 	// A specific Content-Type to use for all artifacts
 	ContentType string
+
+	// Whether to show HTTP debugging
+	DebugHTTP bool
 }
 
 type ArtifactUploader struct {
@@ -47,10 +50,10 @@ type ArtifactUploader struct {
 	logger logger.Logger
 
 	// The APIClient that will be used when uploading jobs
-	apiClient *api.Client
+	apiClient APIClient
 }
 
-func NewArtifactUploader(l logger.Logger, ac *api.Client, c ArtifactUploaderConfig) *ArtifactUploader {
+func NewArtifactUploader(l logger.Logger, ac APIClient, c ArtifactUploaderConfig) *ArtifactUploader {
 	return &ArtifactUploader{
 		logger:    l,
 		apiClient: ac,
@@ -208,24 +211,24 @@ func (a *ArtifactUploader) upload(artifacts []*api.Artifact) error {
 		if strings.HasPrefix(a.conf.Destination, "s3://") {
 			uploader, err = NewS3Uploader(a.logger, S3UploaderConfig{
 				Destination: a.conf.Destination,
-				DebugHTTP:   a.apiClient.DebugHTTP,
+				DebugHTTP:   a.conf.DebugHTTP,
 			})
 		} else if strings.HasPrefix(a.conf.Destination, "gs://") {
 			uploader, err = NewGSUploader(a.logger, GSUploaderConfig{
 				Destination: a.conf.Destination,
-				DebugHTTP:   a.apiClient.DebugHTTP,
+				DebugHTTP:   a.conf.DebugHTTP,
 			})
 		} else if strings.HasPrefix(a.conf.Destination, "rt://") {
 			uploader, err = NewArtifactoryUploader(a.logger, ArtifactoryUploaderConfig{
 				Destination: a.conf.Destination,
-				DebugHTTP:   a.apiClient.DebugHTTP,
+				DebugHTTP:   a.conf.DebugHTTP,
 			})
 		} else {
 			return errors.New(fmt.Sprintf("Invalid upload destination: '%v'. Only s3://, gs:// or rt:// upload destinations are allowed. Did you forget to surround your artifact upload pattern in double quotes?", a.conf.Destination))
 		}
 	} else {
 		uploader = NewFormUploader(a.logger, FormUploaderConfig{
-			DebugHTTP: a.apiClient.DebugHTTP,
+			DebugHTTP: a.conf.DebugHTTP,
 		})
 	}
 
@@ -293,7 +296,7 @@ func (a *ArtifactUploader) upload(artifacts []*api.Artifact) error {
 
 				// Update the states of the artifacts in bulk.
 				err = retry.Do(func(s *retry.Stats) error {
-					_, err = a.apiClient.Artifacts.Update(a.conf.JobID, statesToUpload)
+					_, err = a.apiClient.UpdateArtifacts(a.conf.JobID, statesToUpload)
 					if err != nil {
 						a.logger.Warn("%s (%s)", err, s)
 					}
diff --git a/agent/job_runner.go b/agent/job_runner.go
index 9aa3996c78..44fea55898 100644
--- a/agent/job_runner.go
+++ b/agent/job_runner.go
@@ -45,7 +45,7 @@ type JobRunner struct {
 	job *api.Job
 
 	// The APIClient that will be used when updating the job
-	apiClient *api.Client
+	apiClient APIClient
 
 	// A scope for metrics within a job
 	metrics *metrics.Scope
@@ -466,7 +466,7 @@ func (r *JobRunner) startJob(startedAt time.Time) error {
 	r.job.StartedAt = startedAt.UTC().Format(time.RFC3339Nano)
 
 	return retry.Do(func(s *retry.Stats) error {
-		_, err := r.apiClient.Jobs.Start(r.job)
+		_, err := r.apiClient.StartJob(r.job)
 
 		if err != nil {
 			if api.IsRetryableError(err) {
@@ -489,7 +489,7 @@ func (r *JobRunner) finishJob(finishedAt time.Time, exitStatus string, failedChu
 	r.job.ChunksFailedCount = failedChunkCount
 
 	return retry.Do(func(s *retry.Stats) error {
-		response, err := r.apiClient.Jobs.Finish(r.job)
+		response, err := r.apiClient.FinishJob(r.job)
 		if err != nil {
 			// If the API returns with a 422, that means that we
 			// succesfully tried to finish the job, but Buildkite
@@ -555,7 +555,7 @@ func (r *JobRunner) onProcessStartCallback() {
 		for {
 			// Re-get the job and check it's status to see if it's been
 			// cancelled
-			jobState, _, err := r.apiClient.Jobs.GetState(r.job.ID)
+			jobState, _, err := r.apiClient.GetJobState(r.job.ID)
 			if err != nil {
 				// We don't really care if it fails, we'll just
 				// try again soon anyway
@@ -578,7 +578,7 @@ func (r *JobRunner) onProcessStartCallback() {
 
 func (r *JobRunner) onUploadHeaderTime(cursor int, total int, times map[string]string) {
 	retry.Do(func(s *retry.Stats) error {
-		response, err := r.apiClient.HeaderTimes.Save(r.job.ID, &api.HeaderTimes{Times: times})
+		response, err := r.apiClient.SaveHeaderTimes(r.job.ID, &api.HeaderTimes{Times: times})
 		if err != nil {
 			if response != nil && (response.StatusCode >= 400 && response.StatusCode <= 499) {
 				r.logger.Warn("Buildkite rejected the header times (%s)", err)
@@ -603,7 +603,7 @@ func (r *JobRunner) onUploadChunk(chunk *LogStreamerChunk) error {
 	// from Buildkite that it's considered the chunk (a 4xx will be
 	// returned if the chunk is invalid, and we shouldn't retry on that)
 	return retry.Do(func(s *retry.Stats) error {
-		response, err := r.apiClient.Chunks.Upload(r.job.ID, &api.Chunk{
+		response, err := r.apiClient.UploadChunk(r.job.ID, &api.Chunk{
 			Data:     chunk.Data,
 			Sequence: chunk.Order,
 			Offset:   chunk.Offset,
diff --git a/agent/register.go b/agent/register.go
index 25cb6cab48..3a216389e2 100644
--- a/agent/register.go
+++ b/agent/register.go
@@ -23,7 +23,7 @@ var (
 
 // Register takes an api.Agent and registers it with the Buildkite API
 // and populates the result of the register call
-func Register(l logger.Logger, ac *api.Client, req api.AgentRegisterRequest) (*api.AgentRegisterResponse, error) {
+func Register(l logger.Logger, ac APIClient, req api.AgentRegisterRequest) (*api.AgentRegisterResponse, error) {
 	var registered *api.AgentRegisterResponse
 	var err error
 	var resp *api.Response
@@ -41,7 +41,7 @@ func Register(l logger.Logger, ac *api.Client, req api.AgentRegisterRequest) (*a
 	req.OS = osVersionDump
 
 	register := func(s *retry.Stats) error {
-		registered, resp, err = ac.Agents.Register(&req)
+		registered, resp, err = ac.Register(&req)
 		if err != nil {
 			if resp != nil && resp.StatusCode == 401 {
 				l.Warn("Buildkite rejected the registration (%s)", err)
diff --git a/api/chunks.go b/api/chunks.go
index 43df0be321..4ba4279d9d 100644
--- a/api/chunks.go
+++ b/api/chunks.go
@@ -16,7 +16,7 @@ type Chunk struct {
 
 // Uploads the chunk to the Buildkite Agent API. This request sends the
 // compressed log directly as a request body.
-func (c *Client) UploadChunks(jobId string, chunk *Chunk) (*Response, error) {
+func (c *Client) UploadChunk(jobId string, chunk *Chunk) (*Response, error) {
 	// Create a compressed buffer of the log content
 	body := &bytes.Buffer{}
 	gzipper := gzip.NewWriter(body)
diff --git a/api/client.go b/api/client.go
index 884949f402..e046b060b5 100644
--- a/api/client.go
+++ b/api/client.go
@@ -1,5 +1,7 @@
 package api
 
+//go:generate interfacer -for github.com/buildkite/agent/api.Client -as agent.APIClient -o ../agent/api_iface.go
+
 import (
 	"bytes"
 	"encoding/json"
diff --git a/clicommand/annotate.go b/clicommand/annotate.go
index 001f94e2c7..fc3163f642 100644
--- a/clicommand/annotate.go
+++ b/clicommand/annotate.go
@@ -147,8 +147,8 @@ var AnnotateCommand = cli.Command{
 
 		// Retry the annotation a few times before giving up
 		err = retry.Do(func(s *retry.Stats) error {
-			// Attempt ot create the annotation
-			resp, err := client.Annotations.Create(cfg.Job, annotation)
+			// Attempt to create the annotation
+			resp, err := client.Annotate(cfg.Job, annotation)
 
 			// Don't bother retrying if the response was one of these statuses
 			if resp != nil && (resp.StatusCode == 401 || resp.StatusCode == 404 || resp.StatusCode == 400) {
diff --git a/clicommand/artifact_download.go b/clicommand/artifact_download.go
index a63cfc191e..fdafb8796e 100644
--- a/clicommand/artifact_download.go
+++ b/clicommand/artifact_download.go
@@ -99,6 +99,7 @@ var ArtifactDownloadCommand = cli.Command{
 			Destination: cfg.Destination,
 			BuildID:     cfg.Build,
 			Step:        cfg.Step,
+			DebugHTTP:   cfg.DebugHTTP,
 		})
 
 		// Download the artifacts
diff --git a/clicommand/artifact_upload.go b/clicommand/artifact_upload.go
index 4d04712506..6b88989650 100644
--- a/clicommand/artifact_upload.go
+++ b/clicommand/artifact_upload.go
@@ -103,6 +103,7 @@ var ArtifactUploadCommand = cli.Command{
 			Paths:       cfg.UploadPaths,
 			Destination: cfg.Destination,
 			ContentType: cfg.ContentType,
+			DebugHTTP:   cfg.DebugHTTP,
 		})
 
 		// Upload the artifacts
diff --git a/clicommand/meta_data_exists.go b/clicommand/meta_data_exists.go
index e616ea3f53..4e88b7af18 100644
--- a/clicommand/meta_data_exists.go
+++ b/clicommand/meta_data_exists.go
@@ -83,7 +83,7 @@ var MetaDataExistsCommand = cli.Command{
 		var exists *api.MetaDataExists
 		var resp *api.Response
 		err = retry.Do(func(s *retry.Stats) error {
-			exists, resp, err = client.MetaData.Exists(cfg.Job, cfg.Key)
+			exists, resp, err = client.ExistsMetaData(cfg.Job, cfg.Key)
 			if resp != nil && (resp.StatusCode == 401 || resp.StatusCode == 404) {
 				s.Break()
 			}
diff --git a/clicommand/meta_data_get.go b/clicommand/meta_data_get.go
index 422471cef6..146ac2185c 100644
--- a/clicommand/meta_data_get.go
+++ b/clicommand/meta_data_get.go
@@ -88,7 +88,7 @@ var MetaDataGetCommand = cli.Command{
 		var err error
 		var resp *api.Response
 		err = retry.Do(func(s *retry.Stats) error {
-			metaData, resp, err = client.MetaData.Get(cfg.Job, cfg.Key)
+			metaData, resp, err = client.GetMetaData(cfg.Job, cfg.Key)
 			// Don't bother retrying if the response was one of these statuses
 			if resp != nil && (resp.StatusCode == 401 || resp.StatusCode == 404 || resp.StatusCode == 400) {
 				s.Break()
diff --git a/clicommand/meta_data_set.go b/clicommand/meta_data_set.go
index a3ceaf2d3f..9eed43a974 100644
--- a/clicommand/meta_data_set.go
+++ b/clicommand/meta_data_set.go
@@ -103,7 +103,7 @@ var MetaDataSetCommand = cli.Command{
 
 		// Set the meta data
 		err := retry.Do(func(s *retry.Stats) error {
-			resp, err := client.MetaData.Set(cfg.Job, metaData)
+			resp, err := client.SetMetaData(cfg.Job, metaData)
 			if resp != nil && (resp.StatusCode == 401 || resp.StatusCode == 404) {
 				s.Break()
 			}
diff --git a/clicommand/pipeline_upload.go b/clicommand/pipeline_upload.go
index ff55cb4cdc..b8ba4fde37 100644
--- a/clicommand/pipeline_upload.go
+++ b/clicommand/pipeline_upload.go
@@ -241,7 +241,7 @@ var PipelineUploadCommand = cli.Command{
 
 		// Retry the pipeline upload a few times before giving up
 		err = retry.Do(func(s *retry.Stats) error {
-			_, err = client.Pipelines.Upload(cfg.Job, &api.Pipeline{UUID: uuid, Pipeline: result, Replace: cfg.Replace})
+			_, err = client.UploadPipeline(cfg.Job, &api.Pipeline{UUID: uuid, Pipeline: result, Replace: cfg.Replace})
 			if err != nil {
 				l.Warn("%s (%s)", err, s)
 
diff --git a/clicommand/step_update.go b/clicommand/step_update.go
index 521b5be759..f2d531cc45 100644
--- a/clicommand/step_update.go
+++ b/clicommand/step_update.go
@@ -114,7 +114,7 @@ var StepUpdateCommand = cli.Command{
 
 		// Post the change
 		err := retry.Do(func(s *retry.Stats) error {
-			resp, err := client.Jobs.StepUpdate(cfg.Job, update)
+			resp, err := client.StepUpdate(cfg.Job, update)
 			if resp != nil && (resp.StatusCode == 400 || resp.StatusCode == 401 || resp.StatusCode == 404) {
 				s.Break()
 			}

From b6167f73dd0e82723adf39103f0e942ea476626f Mon Sep 17 00:00:00 2001
From: Lachlan Donald <lachlan@buildkite.com>
Date: Tue, 21 May 2019 16:23:36 +1000
Subject: [PATCH 3/4] Move api client creation into api package

---
 agent/agent_worker.go           | 35 ++----------
 agent/{api_iface.go => api.go}  |  3 +
 agent/api_client.go             | 71 ------------------------
 agent/job_runner.go             | 28 ++++------
 agent/version.go                |  6 ++
 api/artifacts.go                |  2 +-
 api/auth.go                     | 26 +++------
 api/client.go                   | 98 ++++++++++++++++++++++++++++-----
 api/client_test.go              | 75 +++++++++++++++++++++++++
 clicommand/agent_start.go       | 17 ++----
 clicommand/annotate.go          |  3 +-
 clicommand/artifact_download.go |  3 +-
 clicommand/artifact_shasum.go   |  3 +-
 clicommand/artifact_upload.go   |  3 +-
 clicommand/global.go            | 19 ++++---
 clicommand/meta_data_exists.go  |  3 +-
 clicommand/meta_data_get.go     |  3 +-
 clicommand/meta_data_set.go     |  3 +-
 clicommand/pipeline_upload.go   |  2 +-
 clicommand/step_update.go       |  3 +-
 20 files changed, 222 insertions(+), 184 deletions(-)
 rename agent/{api_iface.go => api.go} (92%)
 delete mode 100644 agent/api_client.go
 create mode 100644 api/client_test.go

diff --git a/agent/agent_worker.go b/agent/agent_worker.go
index e15e2e691f..6f1fdcf4f7 100644
--- a/agent/agent_worker.go
+++ b/agent/agent_worker.go
@@ -19,12 +19,6 @@ type AgentWorkerConfig struct {
 	// Whether to set debug in the job
 	Debug bool
 
-	// The endpoint that should be used when communicating with the API
-	Endpoint string
-
-	// Whether to disable http for the API
-	DisableHTTP2 bool
-
 	// The configuration of the agent from the CLI
 	AgentConfiguration AgentConfiguration
 }
@@ -68,26 +62,12 @@ type AgentWorker struct {
 }
 
 // Creates the agent worker and initializes it's API Client
-func NewAgentWorker(l logger.Logger, a *api.AgentRegisterResponse, m *metrics.Collector, c AgentWorkerConfig) *AgentWorker {
-	var endpoint string
-	if a.Endpoint != "" {
-		endpoint = a.Endpoint
-	} else {
-		endpoint = c.Endpoint
-	}
-
-	// Create an APIClient with the agent's access token
-	apiClient := NewAPIClient(l, APIClientConfig{
-		Endpoint:     endpoint,
-		Token:        a.AccessToken,
-		DisableHTTP2: c.DisableHTTP2,
-	})
-
+func NewAgentWorker(l logger.Logger, a *api.AgentRegisterResponse, m *metrics.Collector, apiClient APIClient, c AgentWorkerConfig) *AgentWorker {
 	return &AgentWorker{
 		logger:             l,
 		agent:              a,
 		metricsCollector:   m,
-		apiClient:          apiClient,
+		apiClient:          apiClient.FromAgentRegisterResponse(a),
 		debug:              c.Debug,
 		agentConfiguration: c.AgentConfiguration,
 		stop:               make(chan struct{}),
@@ -312,14 +292,10 @@ func (a *AgentWorker) Ping() (*api.Job, error) {
 
 	// Should we switch endpoints?
 	if ping.Endpoint != "" && ping.Endpoint != a.agent.Endpoint {
+		newAPIClient := a.apiClient.FromPing(ping)
+
 		// Before switching to the new one, do a ping test to make sure it's
 		// valid. If it is, switch and carry on, otherwise ignore the switch
-		// for now.
-		newAPIClient := NewAPIClient(a.logger, APIClientConfig{
-			Endpoint: ping.Endpoint,
-			Token:    a.agent.AccessToken,
-		})
-
 		newPing, _, err := newAPIClient.Ping()
 		if err != nil {
 			a.logger.Warn("Failed to ping the new endpoint %s - ignoring switch for now (%s)", ping.Endpoint, err)
@@ -395,9 +371,8 @@ func (a *AgentWorker) AcceptAndRun(job *api.Job) error {
 	}()
 
 	// Now that the job has been accepted, we can start it.
-	a.jobRunner, err = NewJobRunner(a.logger, jobMetricsScope, a.agent, accepted, JobRunnerConfig{
+	a.jobRunner, err = NewJobRunner(a.logger, jobMetricsScope, a.agent, accepted, a.apiClient, JobRunnerConfig{
 		Debug:              a.debug,
-		Endpoint:           accepted.Endpoint,
 		AgentConfiguration: a.agentConfiguration,
 	})
 
diff --git a/agent/api_iface.go b/agent/api.go
similarity index 92%
rename from agent/api_iface.go
rename to agent/api.go
index f36192c923..b8acbb29e1 100755
--- a/agent/api_iface.go
+++ b/agent/api.go
@@ -10,11 +10,14 @@ import (
 type APIClient interface {
 	AcceptJob(*api.Job) (*api.Job, *api.Response, error)
 	Annotate(string, *api.Annotation) (*api.Response, error)
+	Config() api.Config
 	Connect() (*api.Response, error)
 	CreateArtifacts(string, *api.ArtifactBatch) (*api.ArtifactBatchCreateResponse, *api.Response, error)
 	Disconnect() (*api.Response, error)
 	ExistsMetaData(string, string) (*api.MetaDataExists, *api.Response, error)
 	FinishJob(*api.Job) (*api.Response, error)
+	FromAgentRegisterResponse(*api.AgentRegisterResponse) *api.Client
+	FromPing(*api.Ping) *api.Client
 	GetJobState(string) (*api.JobState, *api.Response, error)
 	GetMetaData(string, string) (*api.MetaData, *api.Response, error)
 	Heartbeat() (*api.Heartbeat, *api.Response, error)
diff --git a/agent/api_client.go b/agent/api_client.go
deleted file mode 100644
index ed752b5fa8..0000000000
--- a/agent/api_client.go
+++ /dev/null
@@ -1,71 +0,0 @@
-package agent
-
-import (
-	"crypto/tls"
-	"net"
-	"net/http"
-	"net/url"
-	"runtime"
-	"time"
-
-	"github.com/buildkite/agent/api"
-	"github.com/buildkite/agent/logger"
-)
-
-var (
-	debugHTTP = false
-)
-
-type APIClientConfig struct {
-	Endpoint     string
-	Token        string
-	DisableHTTP2 bool
-}
-
-func APIClientEnableHTTPDebug() {
-	debugHTTP = true
-}
-
-func NewAPIClient(l logger.Logger, c APIClientConfig) APIClient {
-	httpTransport := &http.Transport{
-		Proxy:              http.ProxyFromEnvironment,
-		DisableCompression: false,
-		DisableKeepAlives:  false,
-		DialContext: (&net.Dialer{
-			Timeout:   30 * time.Second,
-			KeepAlive: 30 * time.Second,
-		}).DialContext,
-		MaxIdleConns:        100,
-		IdleConnTimeout:     90 * time.Second,
-		TLSHandshakeTimeout: 30 * time.Second,
-	}
-
-	if c.DisableHTTP2 {
-		httpTransport.TLSNextProto = make(map[string]func(string, *tls.Conn) http.RoundTripper)
-	}
-
-	// Configure the HTTP client
-	httpClient := &http.Client{Transport: &api.AuthenticatedTransport{
-		Token:     c.Token,
-		Transport: httpTransport,
-	}}
-	httpClient.Timeout = 60 * time.Second
-
-	u, err := url.Parse(c.Endpoint)
-	if err != nil {
-		l.Warn("Failed to parse %q: %v", c.Endpoint, err)
-	}
-
-	// Create the Buildkite Agent API Client
-	client := api.NewClient(httpClient, l, api.ClientConfig{
-		BaseURL:   u,
-		UserAgent: userAgent(),
-		DebugHTTP: debugHTTP,
-	})
-
-	return client
-}
-
-func userAgent() string {
-	return "buildkite-agent/" + Version() + "." + BuildVersion() + " (" + runtime.GOOS + "; " + runtime.GOARCH + ")"
-}
diff --git a/agent/job_runner.go b/agent/job_runner.go
index 44fea55898..9d766b5db4 100644
--- a/agent/job_runner.go
+++ b/agent/job_runner.go
@@ -21,9 +21,6 @@ import (
 )
 
 type JobRunnerConfig struct {
-	// The endpoint that should be used when communicating with the API
-	Endpoint string
-
 	// The configuration of the agent from the CLI
 	AgentConfiguration AgentConfiguration
 
@@ -80,23 +77,18 @@ type JobRunner struct {
 }
 
 // Initializes the job runner
-func NewJobRunner(l logger.Logger, scope *metrics.Scope, ag *api.AgentRegisterResponse, j *api.Job, conf JobRunnerConfig) (*JobRunner, error) {
+func NewJobRunner(l logger.Logger, scope *metrics.Scope, ag *api.AgentRegisterResponse, j *api.Job, apiClient APIClient, conf JobRunnerConfig) (*JobRunner, error) {
 	runner := &JobRunner{
-		agent:   ag,
-		job:     j,
-		logger:  l,
-		conf:    conf,
-		metrics: scope,
+		agent:     ag,
+		job:       j,
+		logger:    l,
+		conf:      conf,
+		metrics:   scope,
+		apiClient: apiClient,
 	}
 
 	runner.context, runner.contextCancel = context.WithCancel(context.Background())
 
-	// Our own APIClient using the endpoint and the agents access token
-	runner.apiClient = NewAPIClient(l, APIClientConfig{
-		Endpoint: runner.conf.Endpoint,
-		Token:    ag.AccessToken,
-	})
-
 	// Create our header times struct
 	runner.headerTimesStreamer = newHeaderTimesStreamer(l, runner.onUploadHeaderTime)
 
@@ -404,8 +396,10 @@ func (r *JobRunner) createEnvironment() ([]string, error) {
 		env["BUILDKITE_IGNORED_ENV"] = strings.Join(ignoredEnv, ",")
 	}
 
-	env["BUILDKITE_AGENT_ENDPOINT"] = r.conf.Endpoint
-	env["BUILDKITE_AGENT_ACCESS_TOKEN"] = r.agent.AccessToken
+	// Add the API configuration
+	apiConfig := r.apiClient.Config()
+	env["BUILDKITE_AGENT_ENDPOINT"] = apiConfig.Endpoint
+	env["BUILDKITE_AGENT_ACCESS_TOKEN"] = apiConfig.Token
 
 	// Add agent environment variables
 	env["BUILDKITE_AGENT_DEBUG"] = fmt.Sprintf("%t", r.conf.Debug)
diff --git a/agent/version.go b/agent/version.go
index 409e9bd4a9..7df30093a9 100644
--- a/agent/version.go
+++ b/agent/version.go
@@ -1,5 +1,7 @@
 package agent
 
+import "runtime"
+
 // You can overridden buildVersion at compile time by using:
 //
 //  go run -ldflags "-X github.com/buildkite/agent/agent.buildVersion abc" *.go --version
@@ -20,3 +22,7 @@ func BuildVersion() string {
 		return "x"
 	}
 }
+
+func UserAgent() string {
+	return "buildkite-agent/" + Version() + "." + BuildVersion() + " (" + runtime.GOOS + "; " + runtime.GOARCH + ")"
+}
diff --git a/api/artifacts.go b/api/artifacts.go
index de281da0b4..e69639e11d 100644
--- a/api/artifacts.go
+++ b/api/artifacts.go
@@ -46,7 +46,7 @@ type ArtifactBatch struct {
 }
 
 type ArtifactUploadInstructions struct {
-	Data   map[string]string `json: "data"`
+	Data   map[string]string `json:"data"`
 	Action struct {
 		URL       string `json:"url,omitempty"`
 		Method    string `json:"method"`
diff --git a/api/auth.go b/api/auth.go
index e49fae0d72..1fb28da103 100644
--- a/api/auth.go
+++ b/api/auth.go
@@ -9,39 +9,29 @@ type canceler interface {
 	CancelRequest(*http.Request)
 }
 
-// Transport manages injection of the API token
-type AuthenticatedTransport struct {
+// authenticatedTransport manages injection of the API token
+type authenticatedTransport struct {
 	// The Token used for authentication. This can either the be
 	// organizations registration token, or the agents access token.
 	Token string
 
-	// Transport is the underlying HTTP transport to use when making
-	// requests. It will default to http.DefaultTransport if nil.
-	Transport http.RoundTripper
+	// Delegate is the underlying HTTP transport
+	Delegate http.RoundTripper
 }
 
 // RoundTrip invoked each time a request is made
-func (t AuthenticatedTransport) RoundTrip(req *http.Request) (*http.Response, error) {
+func (t authenticatedTransport) RoundTrip(req *http.Request) (*http.Response, error) {
 	if t.Token == "" {
 		return nil, fmt.Errorf("Invalid token, empty string supplied")
 	}
 
 	req.Header.Set("Authorization", fmt.Sprintf("Token %s", t.Token))
 
-	return t.transport().RoundTrip(req)
+	return t.Delegate.RoundTrip(req)
 }
 
 // CancelRequest cancels an in-flight request by closing its connection.
-func (t *AuthenticatedTransport) CancelRequest(req *http.Request) {
-	cancelableTransport := t.Transport.(canceler)
+func (t *authenticatedTransport) CancelRequest(req *http.Request) {
+	cancelableTransport := t.Delegate.(canceler)
 	cancelableTransport.CancelRequest(req)
 }
-
-func (t *AuthenticatedTransport) transport() http.RoundTripper {
-	// Use the custom transport if one was provided
-	if t.Transport != nil {
-		return t.Transport
-	}
-
-	return http.DefaultTransport
-}
diff --git a/api/client.go b/api/client.go
index e046b060b5..c311662ee1 100644
--- a/api/client.go
+++ b/api/client.go
@@ -1,14 +1,16 @@
 package api
 
-//go:generate interfacer -for github.com/buildkite/agent/api.Client -as agent.APIClient -o ../agent/api_iface.go
+//go:generate interfacer -for github.com/buildkite/agent/api.Client -as agent.APIClient -o ../agent/api.go
 
 import (
 	"bytes"
+	"crypto/tls"
 	"encoding/json"
 	"errors"
 	"fmt"
 	"io"
 	"io/ioutil"
+	"net"
 	"net/http"
 	"net/http/httputil"
 	"net/url"
@@ -21,27 +23,36 @@ import (
 )
 
 const (
-	defaultBaseURL   = "https://agent.buildkite.com/"
+	defaultEndpoint  = "https://agent.buildkite.com/"
 	defaultUserAgent = "buildkite-agent/api"
 )
 
-// ClientConfig is configuration for Client
-type ClientConfig struct {
-	// Base URL for API requests. Defaults to the public Buildkite Agent API.
+// Config is configuration for the API Client
+type Config struct {
+	// Endpoint for API requests. Defaults to the public Buildkite Agent API.
 	// The URL should always be specified with a trailing slash.
-	BaseURL *url.URL
+	Endpoint string
+
+	// The authentication token to use, either a registration or access token
+	Token string
 
 	// User agent used when communicating with the Buildkite Agent API.
 	UserAgent string
 
+	// If true, only HTTP2 is disabled
+	DisableHTTP2 bool
+
 	// If true, requests and responses will be dumped and set to the logger
 	DebugHTTP bool
+
+	// The http client used, leave nil for the default
+	HTTPClient *http.Client
 }
 
 // A Client manages communication with the Buildkite Agent API.
 type Client struct {
 	// The client configuration
-	conf ClientConfig
+	conf Config
 
 	// HTTP client used to communicate with the API.
 	client *http.Client
@@ -51,15 +62,43 @@ type Client struct {
 }
 
 // NewClient returns a new Buildkite Agent API Client.
-func NewClient(httpClient *http.Client, l logger.Logger, conf ClientConfig) *Client {
-	if conf.BaseURL == nil {
-		conf.BaseURL, _ = url.Parse(defaultBaseURL)
+func NewClient(l logger.Logger, conf Config) *Client {
+	if conf.Endpoint == "" {
+		conf.Endpoint = defaultEndpoint
 	}
 
 	if conf.UserAgent == "" {
 		conf.UserAgent = defaultUserAgent
 	}
 
+	httpClient := conf.HTTPClient
+	if conf.HTTPClient == nil {
+		t := &http.Transport{
+			Proxy:              http.ProxyFromEnvironment,
+			DisableCompression: false,
+			DisableKeepAlives:  false,
+			DialContext: (&net.Dialer{
+				Timeout:   30 * time.Second,
+				KeepAlive: 30 * time.Second,
+			}).DialContext,
+			MaxIdleConns:        100,
+			IdleConnTimeout:     90 * time.Second,
+			TLSHandshakeTimeout: 30 * time.Second,
+		}
+
+		if conf.DisableHTTP2 {
+			t.TLSNextProto = make(map[string]func(string, *tls.Conn) http.RoundTripper)
+		}
+
+		httpClient = &http.Client{
+			Timeout: 60 * time.Second,
+			Transport: &authenticatedTransport{
+				Token:    conf.Token,
+				Delegate: t,
+			},
+		}
+	}
+
 	return &Client{
 		logger: l,
 		client: httpClient,
@@ -67,13 +106,46 @@ func NewClient(httpClient *http.Client, l logger.Logger, conf ClientConfig) *Cli
 	}
 }
 
+// Config returns the internal configuration for the Client
+func (c *Client) Config() Config {
+	return c.conf
+}
+
+// FromAgentRegisterResponse returns a new instance using the access token and endpoint
+// from the registration response
+func (c *Client) FromAgentRegisterResponse(resp *AgentRegisterResponse) *Client {
+	conf := c.conf
+
+	// Override the registration token with the access token
+	conf.Token = resp.AccessToken
+
+	// If Buildkite told us to use a new Endpoint, respect that
+	if resp.Endpoint != "" {
+		conf.Endpoint = resp.Endpoint
+	}
+
+	return NewClient(c.logger, conf)
+}
+
+// FromPing returns a new instance using a new endpoint from a ping response
+func (c *Client) FromPing(resp *Ping) *Client {
+	conf := c.conf
+
+	// If Buildkite told us to use a new Endpoint, respect that
+	if resp.Endpoint != "" {
+		conf.Endpoint = resp.Endpoint
+	}
+
+	return NewClient(c.logger, conf)
+}
+
 // NewRequest creates an API request. A relative URL can be provided in urlStr,
 // in which case it is resolved relative to the BaseURL of the Client.
 // Relative URLs should always be specified without a preceding slash. If
 // specified, the value pointed to by body is JSON encoded and included as the
 // request body.
 func (c *Client) newRequest(method, urlStr string, body interface{}) (*http.Request, error) {
-	u := joinURL(c.conf.BaseURL.String(), urlStr)
+	u := joinURLPath(c.conf.Endpoint, urlStr)
 
 	buf := new(bytes.Buffer)
 	if body != nil {
@@ -102,7 +174,7 @@ func (c *Client) newRequest(method, urlStr string, body interface{}) (*http.Requ
 // of the Client. Relative URLs should always be specified without a preceding
 // slash.
 func (c *Client) newFormRequest(method, urlStr string, body *bytes.Buffer) (*http.Request, error) {
-	u := joinURL(c.conf.BaseURL.String(), urlStr)
+	u := joinURLPath(c.conf.Endpoint, urlStr)
 
 	req, err := http.NewRequest(method, u, body)
 	if err != nil {
@@ -252,6 +324,6 @@ func addOptions(s string, opt interface{}) (string, error) {
 	return u.String(), nil
 }
 
-func joinURL(endpoint string, path string) string {
+func joinURLPath(endpoint string, path string) string {
 	return strings.TrimRight(endpoint, "/") + "/" + strings.TrimLeft(path, "/")
 }
diff --git a/api/client_test.go b/api/client_test.go
new file mode 100644
index 0000000000..2f713d7dd8
--- /dev/null
+++ b/api/client_test.go
@@ -0,0 +1,75 @@
+package api
+
+import (
+	"fmt"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/buildkite/agent/logger"
+)
+
+func TestRegisteringAndConnectingClient(t *testing.T) {
+	server := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
+		switch req.URL.Path {
+		case `/register`:
+			if !checkAuthToken(t, req, "llamas") {
+				http.Error(rw, "Bad auth", http.StatusUnauthorized)
+				return
+			}
+			rw.WriteHeader(http.StatusOK)
+			fmt.Fprintf(rw, `{"id":"12-34-56-78-91", "name":"agent-1", "access_token":"alpacas"}`)
+
+		case `/connect`:
+			if !checkAuthToken(t, req, "alpacas") {
+				http.Error(rw, "Bad auth", http.StatusUnauthorized)
+				return
+			}
+			rw.WriteHeader(http.StatusOK)
+			fmt.Fprintf(rw, `{}`)
+
+		default:
+			t.Errorf("Unknown endpoint %s %s", req.Method, req.URL.Path)
+			http.Error(rw, "Not found", http.StatusNotFound)
+		}
+	}))
+	defer server.Close()
+
+	// Initial client with a registration token
+	c := NewClient(logger.Discard, Config{
+		Endpoint: server.URL,
+		Token:    "llamas",
+	})
+
+	// Check a register works
+	regResp, _, err := c.Register(&AgentRegisterRequest{})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if regResp.Name != "agent-1" {
+		t.Fatalf("Bad name %q", regResp.Name)
+	}
+
+	if regResp.AccessToken != "alpacas" {
+		t.Fatalf("Bad access token %q", regResp.AccessToken)
+	}
+
+	// New client with the access token
+	c2 := c.FromAgentRegisterResponse(regResp)
+
+	// Check a connect works
+	_, err = c2.Connect()
+	if err != nil {
+		t.Fatal(err)
+	}
+}
+
+func checkAuthToken(t *testing.T, req *http.Request, token string) bool {
+	t.Helper()
+	if auth := req.Header.Get(`Authorization`); auth != fmt.Sprintf("Token %s", token) {
+		t.Errorf("Bad Authorization header %q", auth)
+		return false
+	}
+	return true
+}
diff --git a/clicommand/agent_start.go b/clicommand/agent_start.go
index 8fb6c484aa..5d965cdc89 100644
--- a/clicommand/agent_start.go
+++ b/clicommand/agent_start.go
@@ -587,10 +587,8 @@ var AgentStartCommand = cli.Command{
 			l.Info("Agents will disconnect after %d seconds of inactivity", agentConf.DisconnectAfterIdleTimeout)
 		}
 
-		apiClientConf := loadAPIClientConfig(cfg, `Token`)
-
 		// Create the API client
-		client := agent.NewAPIClient(l, apiClientConf)
+		client := api.NewClient(l, loadAPIClientConfig(cfg, `Token`))
 
 		// The registration request for all agents
 		registerReq := api.AgentRegisterRequest{
@@ -609,14 +607,6 @@ var AgentStartCommand = cli.Command{
 			}),
 		}
 
-		// The common configuration for all workers
-		workerConf := agent.AgentWorkerConfig{
-			AgentConfiguration: agentConf,
-			Debug:              cfg.Debug,
-			Endpoint:           apiClientConf.Endpoint,
-			DisableHTTP2:       apiClientConf.DisableHTTP2,
-		}
-
 		var workers []*agent.AgentWorker
 
 		for i := 1; i <= cfg.Spawn; i++ {
@@ -635,7 +625,10 @@ var AgentStartCommand = cli.Command{
 			// Create an agent worker to run the agent
 			workers = append(workers,
 				agent.NewAgentWorker(
-					l.WithFields(logger.StringField(`agent`, ag.Name)), ag, mc, workerConf))
+					l.WithFields(logger.StringField(`agent`, ag.Name)), ag, mc, client, agent.AgentWorkerConfig{
+						AgentConfiguration: agentConf,
+						Debug:              cfg.Debug,
+					}))
 		}
 
 		// Setup the agent pool that spawns agent workers
diff --git a/clicommand/annotate.go b/clicommand/annotate.go
index fc3163f642..b2c3ff6f4f 100644
--- a/clicommand/annotate.go
+++ b/clicommand/annotate.go
@@ -7,7 +7,6 @@ import (
 
 	"github.com/buildkite/agent/stdin"
 
-	"github.com/buildkite/agent/agent"
 	"github.com/buildkite/agent/api"
 	"github.com/buildkite/agent/cliconfig"
 	"github.com/buildkite/agent/retry"
@@ -135,7 +134,7 @@ var AnnotateCommand = cli.Command{
 		}
 
 		// Create the API client
-		client := agent.NewAPIClient(l, loadAPIClientConfig(cfg, `AgentAccessToken`))
+		client := api.NewClient(l, loadAPIClientConfig(cfg, `AgentAccessToken`))
 
 		// Create the annotation we'll send to the Buildkite API
 		annotation := &api.Annotation{
diff --git a/clicommand/artifact_download.go b/clicommand/artifact_download.go
index fdafb8796e..257e22152f 100644
--- a/clicommand/artifact_download.go
+++ b/clicommand/artifact_download.go
@@ -2,6 +2,7 @@ package clicommand
 
 import (
 	"github.com/buildkite/agent/agent"
+	"github.com/buildkite/agent/api"
 	"github.com/buildkite/agent/cliconfig"
 	"github.com/urfave/cli"
 )
@@ -91,7 +92,7 @@ var ArtifactDownloadCommand = cli.Command{
 		HandleGlobalFlags(l, cfg)
 
 		// Create the API client
-		client := agent.NewAPIClient(l, loadAPIClientConfig(cfg, `AgentAccessToken`))
+		client := api.NewClient(l, loadAPIClientConfig(cfg, `AgentAccessToken`))
 
 		// Setup the downloader
 		downloader := agent.NewArtifactDownloader(l, client, agent.ArtifactDownloaderConfig{
diff --git a/clicommand/artifact_shasum.go b/clicommand/artifact_shasum.go
index 54c4cc52ce..21cb42bc40 100644
--- a/clicommand/artifact_shasum.go
+++ b/clicommand/artifact_shasum.go
@@ -4,6 +4,7 @@ import (
 	"fmt"
 
 	"github.com/buildkite/agent/agent"
+	"github.com/buildkite/agent/api"
 	"github.com/buildkite/agent/cliconfig"
 	"github.com/urfave/cli"
 )
@@ -93,7 +94,7 @@ var ArtifactShasumCommand = cli.Command{
 		HandleGlobalFlags(l, cfg)
 
 		// Create the API client
-		client := agent.NewAPIClient(l, loadAPIClientConfig(cfg, `AgentAccessToken`))
+		client := api.NewClient(l, loadAPIClientConfig(cfg, `AgentAccessToken`))
 
 		// Find the artifact we want to show the SHASUM for
 		searcher := agent.NewArtifactSearcher(l, client, cfg.Build)
diff --git a/clicommand/artifact_upload.go b/clicommand/artifact_upload.go
index 6b88989650..986321e6ba 100644
--- a/clicommand/artifact_upload.go
+++ b/clicommand/artifact_upload.go
@@ -2,6 +2,7 @@ package clicommand
 
 import (
 	"github.com/buildkite/agent/agent"
+	"github.com/buildkite/agent/api"
 	"github.com/buildkite/agent/cliconfig"
 	"github.com/urfave/cli"
 )
@@ -95,7 +96,7 @@ var ArtifactUploadCommand = cli.Command{
 		HandleGlobalFlags(l, cfg)
 
 		// Create the API client
-		client := agent.NewAPIClient(l, loadAPIClientConfig(cfg, `AgentAccessToken`))
+		client := api.NewClient(l, loadAPIClientConfig(cfg, `AgentAccessToken`))
 
 		// Setup the uploader
 		uploader := agent.NewArtifactUploader(l, client, agent.ArtifactUploaderConfig{
diff --git a/clicommand/global.go b/clicommand/global.go
index e5991d153e..15606aff31 100644
--- a/clicommand/global.go
+++ b/clicommand/global.go
@@ -7,6 +7,7 @@ import (
 	"strings"
 
 	"github.com/buildkite/agent/agent"
+	"github.com/buildkite/agent/api"
 	"github.com/buildkite/agent/experiments"
 	"github.com/buildkite/agent/logger"
 	"github.com/oleiade/reflections"
@@ -146,29 +147,31 @@ func UnsetConfigFromEnvironment(c *cli.Context) {
 	}
 }
 
-func loadAPIClientConfig(cfg interface{}, tokenField string) agent.APIClientConfig {
+func loadAPIClientConfig(cfg interface{}, tokenField string) api.Config {
+	conf := api.Config{
+		UserAgent: agent.UserAgent(),
+	}
+
 	// Enable HTTP debugging
 	debugHTTP, err := reflections.GetField(cfg, "DebugHTTP")
 	if debugHTTP == true && err == nil {
-		agent.APIClientEnableHTTPDebug()
+		conf.DebugHTTP = true
 	}
 
-	var a agent.APIClientConfig
-
 	endpoint, err := reflections.GetField(cfg, "Endpoint")
 	if endpoint != "" && err == nil {
-		a.Endpoint = endpoint.(string)
+		conf.Endpoint = endpoint.(string)
 	}
 
 	token, err := reflections.GetField(cfg, tokenField)
 	if token != "" && err == nil {
-		a.Token = token.(string)
+		conf.Token = token.(string)
 	}
 
 	noHTTP2, err := reflections.GetField(cfg, "NoHTTP2")
 	if err == nil {
-		a.DisableHTTP2 = noHTTP2.(bool)
+		conf.DisableHTTP2 = noHTTP2.(bool)
 	}
 
-	return a
+	return conf
 }
diff --git a/clicommand/meta_data_exists.go b/clicommand/meta_data_exists.go
index 4e88b7af18..1ce64a92d0 100644
--- a/clicommand/meta_data_exists.go
+++ b/clicommand/meta_data_exists.go
@@ -4,7 +4,6 @@ import (
 	"os"
 	"time"
 
-	"github.com/buildkite/agent/agent"
 	"github.com/buildkite/agent/api"
 	"github.com/buildkite/agent/cliconfig"
 	"github.com/buildkite/agent/retry"
@@ -76,7 +75,7 @@ var MetaDataExistsCommand = cli.Command{
 		HandleGlobalFlags(l, cfg)
 
 		// Create the API client
-		client := agent.NewAPIClient(l, loadAPIClientConfig(cfg, `AgentAccessToken`))
+		client := api.NewClient(l, loadAPIClientConfig(cfg, `AgentAccessToken`))
 
 		// Find the meta data value
 		var err error
diff --git a/clicommand/meta_data_get.go b/clicommand/meta_data_get.go
index 146ac2185c..e2e42639ba 100644
--- a/clicommand/meta_data_get.go
+++ b/clicommand/meta_data_get.go
@@ -4,7 +4,6 @@ import (
 	"fmt"
 	"time"
 
-	"github.com/buildkite/agent/agent"
 	"github.com/buildkite/agent/api"
 	"github.com/buildkite/agent/cliconfig"
 	"github.com/buildkite/agent/retry"
@@ -81,7 +80,7 @@ var MetaDataGetCommand = cli.Command{
 		HandleGlobalFlags(l, cfg)
 
 		// Create the API client
-		client := agent.NewAPIClient(l, loadAPIClientConfig(cfg, `AgentAccessToken`))
+		client := api.NewClient(l, loadAPIClientConfig(cfg, `AgentAccessToken`))
 
 		// Find the meta data value
 		var metaData *api.MetaData
diff --git a/clicommand/meta_data_set.go b/clicommand/meta_data_set.go
index 9eed43a974..7af9e824e3 100644
--- a/clicommand/meta_data_set.go
+++ b/clicommand/meta_data_set.go
@@ -5,7 +5,6 @@ import (
 	"os"
 	"time"
 
-	"github.com/buildkite/agent/agent"
 	"github.com/buildkite/agent/api"
 	"github.com/buildkite/agent/cliconfig"
 	"github.com/buildkite/agent/retry"
@@ -93,7 +92,7 @@ var MetaDataSetCommand = cli.Command{
 		}
 
 		// Create the API client
-		client := agent.NewAPIClient(l, loadAPIClientConfig(cfg, `AgentAccessToken`))
+		client := api.NewClient(l, loadAPIClientConfig(cfg, `AgentAccessToken`))
 
 		// Create the meta data to set
 		metaData := &api.MetaData{
diff --git a/clicommand/pipeline_upload.go b/clicommand/pipeline_upload.go
index b8ba4fde37..0c7a29af7f 100644
--- a/clicommand/pipeline_upload.go
+++ b/clicommand/pipeline_upload.go
@@ -232,7 +232,7 @@ var PipelineUploadCommand = cli.Command{
 		}
 
 		// Create the API client
-		client := agent.NewAPIClient(l, loadAPIClientConfig(cfg, `AgentAccessToken`))
+		client := api.NewClient(l, loadAPIClientConfig(cfg, `AgentAccessToken`))
 
 		// Generate a UUID that will identifiy this pipeline change. We
 		// do this outside of the retry loop because we want this UUID
diff --git a/clicommand/step_update.go b/clicommand/step_update.go
index f2d531cc45..d6f9d5cf56 100644
--- a/clicommand/step_update.go
+++ b/clicommand/step_update.go
@@ -5,7 +5,6 @@ import (
 	"os"
 	"time"
 
-	"github.com/buildkite/agent/agent"
 	"github.com/buildkite/agent/api"
 	"github.com/buildkite/agent/cliconfig"
 	"github.com/buildkite/agent/retry"
@@ -97,7 +96,7 @@ var StepUpdateCommand = cli.Command{
 		}
 
 		// Create the API client
-		client := agent.NewAPIClient(l, loadAPIClientConfig(cfg, `AgentAccessToken`))
+		client := api.NewClient(l, loadAPIClientConfig(cfg, `AgentAccessToken`))
 
 		// Generate a UUID that will identifiy this change. We do this
 		// outside of the retry loop because we want this UUID to be

From da1853e4c1e3d9348dfe060a03dd25b40f3bbbb5 Mon Sep 17 00:00:00 2001
From: Lachlan Donald <lachlan@buildkite.com>
Date: Tue, 21 May 2019 16:38:22 +1000
Subject: [PATCH 4/4] Fix tests for new api client

---
 agent/artifact_downloader_test.go                | 6 ++++--
 agent/integration/job_runner_integration_test.go | 8 ++++++--
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/agent/artifact_downloader_test.go b/agent/artifact_downloader_test.go
index 397ebb0ad7..fcaaad72c1 100644
--- a/agent/artifact_downloader_test.go
+++ b/agent/artifact_downloader_test.go
@@ -2,10 +2,12 @@ package agent
 
 import (
 	"fmt"
-	"github.com/buildkite/agent/logger"
 	"net/http"
 	"net/http/httptest"
 	"testing"
+
+	"github.com/buildkite/agent/api"
+	"github.com/buildkite/agent/logger"
 )
 
 func TestArtifactDownloaderConnectsToEndpoint(t *testing.T) {
@@ -27,7 +29,7 @@ func TestArtifactDownloaderConnectsToEndpoint(t *testing.T) {
 	}))
 	defer server.Close()
 
-	ac := NewAPIClient(logger.Discard, APIClientConfig{
+	ac := api.NewClient(logger.Discard, api.Config{
 		Endpoint: server.URL,
 		Token:    `llamasforever`,
 	})
diff --git a/agent/integration/job_runner_integration_test.go b/agent/integration/job_runner_integration_test.go
index 4528e9641c..b7b8d43e22 100644
--- a/agent/integration/job_runner_integration_test.go
+++ b/agent/integration/job_runner_integration_test.go
@@ -89,8 +89,12 @@ func runJob(t *testing.T, ag *api.AgentRegisterResponse, j *api.Job, cfg agent.A
 	// set the bootstrap into the config
 	cfg.BootstrapScript = bs.Path
 
-	jr, err := agent.NewJobRunner(l, scope, ag, j, agent.JobRunnerConfig{
-		Endpoint:           server.URL,
+	client := api.NewClient(l, api.Config{
+		Endpoint: server.URL,
+		Token:    ag.AccessToken,
+	})
+
+	jr, err := agent.NewJobRunner(l, scope, ag, j, client, agent.JobRunnerConfig{
 		AgentConfiguration: cfg,
 	})
 	if err != nil {