Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Verify step signatures #2210

Merged
merged 11 commits into from
Aug 10, 2023
50 changes: 28 additions & 22 deletions agent/agent_configuration.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,28 +5,34 @@ import "time"
// AgentConfiguration is the run-time configuration for an agent that
// has been loaded from the config file and command-line params
type AgentConfiguration struct {
ConfigPath string
BootstrapScript string
BuildPath string
HooksPath string
SocketsPath string
GitMirrorsPath string
GitMirrorsLockTimeout int
GitMirrorsSkipUpdate bool
PluginsPath string
GitCheckoutFlags string
GitCloneFlags string
GitCloneMirrorFlags string
GitCleanFlags string
GitFetchFlags string
GitSubmodules bool
SSHKeyscan bool
CommandEval bool
PluginsEnabled bool
PluginValidation bool
LocalHooksEnabled bool
StrictSingleHooks bool
RunInPty bool
ConfigPath string
BootstrapScript string
BuildPath string
HooksPath string
SocketsPath string
GitMirrorsPath string
GitMirrorsLockTimeout int
GitMirrorsSkipUpdate bool
PluginsPath string
GitCheckoutFlags string
GitCloneFlags string
GitCloneMirrorFlags string
GitCleanFlags string
GitFetchFlags string
GitSubmodules bool
SSHKeyscan bool
CommandEval bool
PluginsEnabled bool
PluginValidation bool
LocalHooksEnabled bool
StrictSingleHooks bool
RunInPty bool

JobSigningKeyPath string
JobVerificationKeyPath string
JobVerificationNoSignatureBehavior string
JobVerificationInvalidSignatureBehavior string
Comment on lines +33 to +34
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe explain what these are. The other one seems obvious, but it deserves a doc comment too.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see they're explained below, but this is a higher level. I think it's good to talk about them here as well.


ANSITimestamps bool
TimestampLines bool
HealthCheckAddr string
Expand Down
139 changes: 0 additions & 139 deletions agent/integration/job_runner_integration_test.go
Original file line number Diff line number Diff line change
@@ -1,24 +1,13 @@
package integration

import (
"context"
"encoding/json"
"fmt"
"io"
"net/http"
"net/http/httptest"
"os"
"path/filepath"
"runtime"
"strconv"
"strings"
"sync"
"testing"

"github.com/buildkite/agent/v3/agent"
"github.com/buildkite/agent/v3/api"
"github.com/buildkite/agent/v3/logger"
"github.com/buildkite/agent/v3/metrics"
"github.com/buildkite/bintest/v3"
)

Expand Down Expand Up @@ -197,131 +186,3 @@ func TestJobRunnerIgnoresPipelineChangesToProtectedVars(t *testing.T) {

runJob(t, j, server, agent.AgentConfiguration{CommandEval: true}, mb)
}

func mockBootstrap(t *testing.T) *bintest.Mock {
t.Helper()

// tests run using t.Run() will have a slash in their name, which will mess with paths to bintest binaries
name := strings.ReplaceAll(t.Name(), "/", "-")
bs, err := bintest.NewMock(fmt.Sprintf("buildkite-agent-bootstrap-%s", name))
if err != nil {
t.Fatalf("bintest.NewMock() error = %v", err)
}

return bs
}

func runJob(t *testing.T, j *api.Job, server *httptest.Server, cfg agent.AgentConfiguration, bs *bintest.Mock) {
l := logger.Discard

// minimal metrics, this could be cleaner
m := metrics.NewCollector(l, metrics.CollectorConfig{})
scope := m.Scope(metrics.Tags{})

// set the bootstrap into the config
cfg.BootstrapScript = bs.Path

client := api.NewClient(l, api.Config{
Endpoint: server.URL,
Token: "llamasrock",
})

jr, err := agent.NewJobRunner(l, client, agent.JobRunnerConfig{
Job: j,
AgentConfiguration: cfg,
MetricsScope: scope,
})
if err != nil {
t.Fatalf("agent.NewJobRunner() error = %v", err)
}

if err := jr.Run(context.Background()); err != nil {
t.Errorf("jr.Run() = %v", err)
}
}

type testAgentEndpoint struct {
mu sync.RWMutex
calls map[string][][]byte
}

func createTestAgentEndpoint() *testAgentEndpoint {
return &testAgentEndpoint{
calls: make(map[string][][]byte, 4),
}
}

func (tae *testAgentEndpoint) finishesFor(t *testing.T, jobID string) []api.Job {
t.Helper()

tae.mu.RLock()
defer tae.mu.RUnlock()

endpoint := fmt.Sprintf("/jobs/%s/finish", jobID)
finishes := make([]api.Job, 0, len(tae.calls))

for _, b := range tae.calls[endpoint] {
var job api.Job
err := json.Unmarshal(b, &job)
if err != nil {
t.Fatalf("decoding accept request body: %v", err)
}
finishes = append(finishes, job)
}

return finishes
}

func (t *testAgentEndpoint) server(jobID string) *httptest.Server {
return httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
b, _ := io.ReadAll(req.Body)
t.mu.Lock()
t.calls[req.URL.Path] = append(t.calls[req.URL.Path], b)
t.mu.Unlock()

switch req.URL.Path {
case "/jobs/" + jobID:
rw.WriteHeader(http.StatusOK)
fmt.Fprintf(rw, `{"state":"running"}`)
case "/jobs/" + jobID + "/start":
rw.WriteHeader(http.StatusOK)
case "/jobs/" + jobID + "/chunks":
rw.WriteHeader(http.StatusCreated)
case "/jobs/" + jobID + "/finish":
rw.WriteHeader(http.StatusOK)
default:
http.Error(rw, fmt.Sprintf("not found; method = %q, path = %q", req.Method, req.URL.Path), http.StatusNotFound)
}
}))
}

func mockPreBootstrap(t *testing.T, hooksDir string) *bintest.Mock {
t.Helper()

mock, err := bintest.NewMock(fmt.Sprintf("buildkite-agent-pre-bootstrap-hook-%s", t.Name()))
if err != nil {
t.Fatalf("bintest.NewMock() error = %v", err)
}

hookScript := filepath.Join(hooksDir, "pre-bootstrap")
body := ""

if runtime.GOOS == "windows" {
// You may be tempted to change this to `@%q`, but please do not. bintest doesn't like it when things change.
// (%q escapes backslashes, which are windows path separators and leads to this test failing on windows)
body = fmt.Sprintf(`@"%s"`, mock.Path)
hookScript += ".bat"
} else {
body = "#!/bin/sh\n" + mock.Path
}

if err := os.MkdirAll(hooksDir, 0o700); err != nil {
t.Fatalf("creating pre-bootstrap hook mock: os.MkdirAll() error = %v", err)
}

if err := os.WriteFile(hookScript, []byte(body), 0o777); err != nil {
t.Fatalf("creating pre-bootstrap hook mock: s.WriteFile() error = %v", err)
}

return mock
}
Loading