Skip to content

Commit

Permalink
Merge branch 'main' into dependabot/go_modules/cloud.google.com/go/co…
Browse files Browse the repository at this point in the history 
…mpute/metadata-0.5.1
  • Loading branch information
@patrobinson
patrobinson authored Sep 17, 2024
2 parents acf44d2 + 12e7aa8 commit f1c232d
Show file tree
Hide file tree
Showing 15 changed files with 172 additions and 13 deletions.
1 change: 1 addition & 0 deletions agent/integration/config_allowlisting_integration_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ func TestConfigAllowlisting(t *testing.T) {
"BUILDKITE": "true",
"BUILDKITE_COMMAND": "echo hello",
},
Token: "bkaj_job-token",
}

for k, v := range tc.extraEnv {
Expand Down
1 change: 1 addition & 0 deletions agent/integration/job_environment_integration_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ func TestWhenCachePathsSetInJobStep_CachePathsEnvVarIsSet(t *testing.T) {
Paths: []string{"foo", "bar"},
},
},
Token: "bkaj_job-token",
}

mb := mockBootstrap(t)
Expand Down
9 changes: 7 additions & 2 deletions agent/integration/job_runner_integration_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ func TestPreBootstrapHookScripts(t *testing.T) {
Env: map[string]string{
"BUILDKITE_COMMAND": "echo hello world",
},
Token: "bkaj_job-token",
}

mb := mockBootstrap(t)
Expand Down Expand Up @@ -150,6 +151,7 @@ func TestPreBootstrapHookRefusesJob(t *testing.T) {
Env: map[string]string{
"BUILDKITE_COMMAND": "echo hello world",
},
Token: "bkaj_job-token",
}

// create a mock agent API
Expand Down Expand Up @@ -197,6 +199,7 @@ func TestJobRunner_WhenBootstrapExits_ItSendsTheExitStatusToTheAPI(t *testing.T)
Env: map[string]string{
"BUILDKITE_COMMAND": "echo hello world",
},
Token: "bkaj_job-token",
}

mb := mockBootstrap(t)
Expand Down Expand Up @@ -231,7 +234,7 @@ func TestJobRunner_WhenJobHasToken_ItOverridesAccessToken(t *testing.T) {
t.Parallel()
ctx := context.Background()

jobToken := "actually-llamas-are-only-okay"
jobToken := "bkaj_actually-llamas-are-only-okay"

j := &api.Job{
ID: "my-job-id",
Expand Down Expand Up @@ -280,13 +283,14 @@ func TestJobRunnerPassesAccessTokenToBootstrap(t *testing.T) {
Env: map[string]string{
"BUILDKITE_COMMAND": "echo hello world",
},
Token: "bkaj_job-token",
}

mb := mockBootstrap(t)
defer mb.CheckAndClose(t)

mb.Expect().Once().AndExitWith(0).AndCallFunc(func(c *bintest.Call) {
if got, want := c.GetEnv("BUILDKITE_AGENT_ACCESS_TOKEN"), "llamasrock"; got != want {
if got, want := c.GetEnv("BUILDKITE_AGENT_ACCESS_TOKEN"), "bkaj_job-token"; got != want {
t.Errorf("c.GetEnv(BUILDKITE_AGENT_ACCESS_TOKEN) = %q, want %q", got, want)
}
c.Exit(0)
Expand Down Expand Up @@ -319,6 +323,7 @@ func TestJobRunnerIgnoresPipelineChangesToProtectedVars(t *testing.T) {
"BUILDKITE_COMMAND": "echo hello world",
"BUILDKITE_COMMAND_EVAL": "false",
},
Token: "bkaj_job-token",
}

mb := mockBootstrap(t)
Expand Down
15 changes: 15 additions & 0 deletions agent/integration/job_verification_integration_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ var (
"BUILDKITE_REPO": defaultRepositoryURL,
"DEPLOY": "1", // step env overrides pipeline env
},
Token: "bkaj_job-token",
}

jobWithNoPluginConfig = api.Job{
Expand All @@ -67,6 +68,7 @@ var (
"BUILDKITE_REPO": defaultRepositoryURL,
"DEPLOY": "1", // step env overrides pipeline env
},
Token: "bkaj_job-token",
}

jobWithNoPlugins = api.Job{
Expand All @@ -82,6 +84,7 @@ var (
"BUILDKITE_REPO": defaultRepositoryURL,
"DEPLOY": "0",
},
Token: "bkaj_job-token",
}

jobWithNullPlugins = api.Job{
Expand All @@ -97,6 +100,7 @@ var (
"BUILDKITE_REPO": defaultRepositoryURL,
"DEPLOY": "0",
},
Token: "bkaj_job-token",
}

jobWithMismatchedStepAndJob = api.Job{
Expand All @@ -110,6 +114,7 @@ var (
"BUILDKITE_REPO": defaultRepositoryURL,
"DEPLOY": "0",
},
Token: "bkaj_job-token",
}

jobWithMismatchedPlugins = api.Job{
Expand All @@ -130,6 +135,7 @@ var (
"BUILDKITE_REPO": defaultRepositoryURL,
"DEPLOY": "0",
},
Token: "bkaj_job-token",
}

jobWithMissingPlugins = api.Job{
Expand All @@ -150,6 +156,7 @@ var (
"BUILDKITE_REPO": defaultRepositoryURL,
"DEPLOY": "0",
},
Token: "bkaj_job-token",
}

jobWithMismatchedEnv = api.Job{
Expand All @@ -163,6 +170,7 @@ var (
"BUILDKITE_REPO": defaultRepositoryURL,
"DEPLOY": "crimes",
},
Token: "bkaj_job-token",
}

jobWithStepEnvButNoCorrespondingJobEnv = api.Job{
Expand All @@ -177,6 +185,7 @@ var (
"BUILDKITE_REPO": defaultRepositoryURL,
"DEPLOY": "0",
},
Token: "bkaj_job-token",
}

jobWithPipelineEnvButNoCorrespondingJobEnv = api.Job{
Expand All @@ -189,6 +198,7 @@ var (
"BUILDKITE_COMMAND": "echo hello world",
"BUILDKITE_REPO": defaultRepositoryURL,
},
Token: "bkaj_job-token",
}

jobWithMatrix = api.Job{
Expand All @@ -206,6 +216,7 @@ var (
"greeting": "hello",
"object": "world",
},
Token: "bkaj_job-token",
}

jobWithInvalidMatrixPermutation = api.Job{
Expand All @@ -223,6 +234,7 @@ var (
"greeting": "goodbye",
"object": "mister anderson",
},
Token: "bkaj_job-token",
}

jobWithMatrixMismatch = api.Job{
Expand All @@ -240,6 +252,7 @@ var (
"greeting": "hello",
"object": "world",
},
Token: "bkaj_job-token",
}

jobWithPipelineInvariantsInEnv = api.Job{
Expand All @@ -253,6 +266,7 @@ var (
"BUILDKITE_REPO": defaultRepositoryURL,
"DEPLOY": "0",
},
Token: "bkaj_job-token",
}

jobWithInvalidPipelineInvariantsInEnv = api.Job{
Expand All @@ -266,6 +280,7 @@ var (
"BUILDKITE_REPO": "https://github.com/haxors/agent.git",
"DEPLOY": "0",
},
Token: "bkaj_job-token",
}
)

Expand Down
9 changes: 9 additions & 0 deletions agent/integration/test_helpers.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,15 @@ func (t *testAgentEndpoint) server(extraRoutes ...route) *httptest.Server {
t.calls[req.URL.Path] = append(t.calls[req.URL.Path], b)
t.mtx.Unlock()

// We also require job tokens are used for authentication
authzHeader := req.Header.Get("Authorization")
if !strings.HasPrefix(authzHeader, "Token bkaj_") {
msg := fmt.Sprintf("Authorization header = %q, want job token prefix 'Token bkaj_'", authzHeader)
fmt.Fprintln(os.Stderr, msg)
http.Error(rw, msg, http.StatusUnauthorized)
return
}

next.ServeHTTP(rw, req)
})
}
Expand Down
2 changes: 1 addition & 1 deletion agent/job_runner.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,6 @@ func NewJobRunner(ctx context.Context, l logger.Logger, apiClient APIClient, con
agentLogger: l,
conf: conf,
apiClient: apiClient,
client: &core.Client{APIClient: apiClient, Logger: l},
}

var err error
Expand All @@ -199,6 +198,7 @@ func NewJobRunner(ctx context.Context, l logger.Logger, apiClient APIClient, con
clientConf.Token = r.conf.Job.Token
r.apiClient = api.NewClient(r.agentLogger, clientConf)
}
r.client = &core.Client{APIClient: r.apiClient, Logger: l}

// Create our header times struct
r.headerTimesStreamer = newHeaderTimesStreamer(r.agentLogger, r.onUploadHeaderTime)
Expand Down
118 changes: 118 additions & 0 deletions api/client.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@ import (
"fmt"
"io"
"net/http"
"net/http/httptrace"
"net/http/httputil"
"net/textproto"
"net/url"
"reflect"
"strconv"
Expand Down Expand Up @@ -45,6 +47,9 @@ type Config struct {
// If true, requests and responses will be dumped and set to the logger
DebugHTTP bool

// If true timings for each request will be logged
TraceHTTP bool

// The http client used, leave nil for the default
HTTPClient *http.Client

Expand Down Expand Up @@ -253,12 +258,22 @@ func (c *Client) doRequest(req *http.Request, v any) (*Response, error) {
}
}

tracer := &tracer{Logger: c.logger}
if c.conf.TraceHTTP {
// Inject a custom http tracer
req = traceHTTPRequest(req, tracer)
tracer.Start()
}

ts := time.Now()

c.logger.Debug("%s %s", req.Method, req.URL)

resp, err := c.client.Do(req)
if err != nil {
if c.conf.TraceHTTP {
tracer.EmitTraceToLog(logger.ERROR)
}
return nil, err
}

Expand All @@ -282,6 +297,10 @@ func (c *Client) doRequest(req *http.Request, v any) (*Response, error) {
}
}

if c.conf.TraceHTTP {
tracer.EmitTraceToLog(logger.DEBUG)
}

err = checkResponse(resp)
if err != nil {
// even though there was an error, we still return the response
Expand All @@ -306,6 +325,105 @@ func (c *Client) doRequest(req *http.Request, v any) (*Response, error) {
return response, err
}

type traceEvent struct {
event string
since time.Duration
}

type tracer struct {
startTime time.Time
logger.Logger
}

func (t *tracer) Start() {
t.startTime = time.Now()
}

func (t *tracer) LogTiming(event string) {
t.Logger = t.Logger.WithFields(logger.DurationField(event, time.Since(t.startTime)))
}

func (t *tracer) LogField(key, value string) {
t.Logger = t.Logger.WithFields(logger.StringField(key, value))
}

func (t *tracer) LogDuration(event string, d time.Duration) {
t.Logger = t.Logger.WithFields(logger.DurationField(event, d))
}

// Currently logger.Logger doesn't give us a way to set the level we want to emit logs at dynamically
func (t *tracer) EmitTraceToLog(level logger.Level) {
msg := "HTTP Timing Trace"
switch level {
case logger.DEBUG:
t.Debug(msg)
case logger.INFO:
t.Info(msg)
case logger.WARN:
t.Warn(msg)
case logger.ERROR:
t.Error(msg)
}
}

func traceHTTPRequest(req *http.Request, t *tracer) *http.Request {
trace := &httptrace.ClientTrace{
GetConn: func(hostPort string) {
t.LogField("hostPort", hostPort)
t.LogTiming("getConn")
},
GotConn: func(info httptrace.GotConnInfo) {
t.LogTiming("gotConn")
t.LogField("reused", strconv.FormatBool(info.Reused))
t.LogField("idle", strconv.FormatBool(info.WasIdle))
t.LogDuration("idleTime", info.IdleTime)
},
PutIdleConn: func(err error) {
t.LogTiming("putIdleConn")
if err != nil {
t.LogField("putIdleConnectionError", err.Error())
}
},
GotFirstResponseByte: func() {
t.LogTiming("gotFirstResponseByte")
},
Got1xxResponse: func(code int, header textproto.MIMEHeader) error {
t.LogTiming("got1xxResponse")
return nil
},
DNSStart: func(_ httptrace.DNSStartInfo) {
t.LogTiming("dnsStart")
},
DNSDone: func(_ httptrace.DNSDoneInfo) {
t.LogTiming("dnsDone")
},
ConnectStart: func(network, addr string) {
t.LogTiming(fmt.Sprintf("connectStart.%s.%s", network, addr))
},
ConnectDone: func(network, addr string, _ error) {
t.LogTiming(fmt.Sprintf("connectDone.%s.%s", network, addr))
},
TLSHandshakeStart: func() {
t.LogTiming("tlsHandshakeStart")
},
TLSHandshakeDone: func(_ tls.ConnectionState, _ error) {
t.LogTiming("tlsHandshakeDone")
},
WroteHeaders: func() {
t.LogTiming("wroteHeaders")
},
WroteRequest: func(_ httptrace.WroteRequestInfo) {
t.LogTiming("wroteRequest")
},
}

req = req.WithContext(httptrace.WithClientTrace(req.Context(), trace))

t.LogField("uri", req.URL.String())
t.LogField("method", req.Method)
return req
}

// ErrorResponse provides a message.
type ErrorResponse struct {
Response *http.Response // HTTP response that caused this error
Expand Down
Loading

0 comments on commit f1c232d

Please sign in to comment.