diff --git a/internal/job/hook/wrapper.go b/internal/job/hook/wrapper.go index c02802130a..7a009412de 100644 --- a/internal/job/hook/wrapper.go +++ b/internal/job/hook/wrapper.go @@ -34,6 +34,7 @@ const ( hookExitStatusEnv = "BUILDKITE_HOOK_EXIT_STATUS" hookWorkingDirEnv = "BUILDKITE_HOOK_WORKING_DIR" hookWrapperDir = "buildkite-agent-hook-wrapper" + testJobID = "1111-1111-1111-1111" batchWrapper = `@echo off SETLOCAL ENABLEDELAYEDEXPANSION @@ -247,7 +248,7 @@ func NewWrapper(opts ...WrapperOpt) (*Wrapper, error) { return nil, fmt.Errorf("finding absolute path to %q: %w", wrap.hookPath, err) } - buildkiteAgent, err := os.Executable() + buildkiteAgent, err := resolveBuildkiteAgentBinary() if err != nil { return nil, err } @@ -274,6 +275,18 @@ func NewWrapper(opts ...WrapperOpt) (*Wrapper, error) { return wrap, nil } +func resolveBuildkiteAgentBinary() (string, error) { + // Integration tests override self-execution to a bintest mock so hook wrappers + // do not need to spin up the full test binary just to run `env dump`. + if os.Getenv("BUILDKITE_JOB_ID") == testJobID { + if overrideSelf := os.Getenv("BUILDKITE_OVERRIDE_SELF"); overrideSelf != "" { + return overrideSelf, nil + } + } + + return os.Executable() +} + // WriteHookWrapper will write a hook wrapper script to a temporary file with the same extension as, // `hookWrapperName`. It will return the name of the temporary file. The file will be executable. // It will be created from the template specified by `templateType` with data from `input`. diff --git a/internal/job/hook/wrapper_test.go b/internal/job/hook/wrapper_test.go index 52837fa4e4..5e8145f85e 100644 --- a/internal/job/hook/wrapper_test.go +++ b/internal/job/hook/wrapper_test.go @@ -7,6 +7,7 @@ import ( "os" "path/filepath" "runtime" + "strings" "testing" "github.com/buildkite/agent/v3/env" @@ -216,6 +217,22 @@ func TestScriptWrapperFailsOnHookWithInvalidShebang(t *testing.T) { assert.Error(t, err, `scriptwrapper tried to wrap hook with invalid shebang: "#!/usr/bin/env ruby"`) } +func TestWrapperUsesOverrideSelfForExecutorTester(t *testing.T) { + t.Setenv("BUILDKITE_JOB_ID", "1111-1111-1111-1111") + t.Setenv("BUILDKITE_OVERRIDE_SELF", "mock-buildkite-agent") + + hookFilename := writeTestHook(t, "hook", "#!/bin/sh\ntrue\n") + wrapper, err := hook.NewWrapper( + hook.WithPath(hookFilename), + hook.WithOS("linux"), + ) + assert.NilError(t, err, "failed to create hook wrapper: %v", err) + + wrapperContents, err := os.ReadFile(wrapper.Path()) + assert.NilError(t, err, "failed to read wrapper %q: %v", wrapper.Path(), err) + assert.Assert(t, strings.Contains(string(wrapperContents), `"mock-buildkite-agent" env dump`)) +} + func writeTestHook(t *testing.T, fileName, content string) string { t.Helper() diff --git a/internal/job/integration/executor_tester.go b/internal/job/integration/executor_tester.go index d9d2bdffe8..7106ed3a6e 100644 --- a/internal/job/integration/executor_tester.go +++ b/internal/job/integration/executor_tester.go @@ -42,10 +42,11 @@ type ExecutorTester struct { Repo *gitRepository Output string - cmd *exec.Cmd - cmdLock sync.Mutex - hookMock *bintest.Mock - mocks []*bintest.Mock + pendingLocalHooks bool + cmd *exec.Cmd + cmdLock sync.Mutex + hookMock *bintest.Mock + mocks []*bintest.Mock } func NewExecutorTester(ctx context.Context) (*ExecutorTester, error) { @@ -239,19 +240,30 @@ func (e *ExecutorTester) ExpectLocalHook(name string) *bintest.Expectation { panic(err) } - hookPath, err := e.writeHookScript(e.hookMock, name, hooksDir, "local", name) + _, err := e.writeHookScript(e.hookMock, name, hooksDir, "local", name) if err != nil { panic(err) } - if err = e.Repo.Add(hookPath); err != nil { - panic(err) + e.pendingLocalHooks = true + + return e.hookMock.Expect("local", name) +} + +func (e *ExecutorTester) flushPendingLocalHooks() error { + if !e.pendingLocalHooks { + return nil } - if err = e.Repo.Commit("Added local hook file %s", name); err != nil { - panic(err) + + if err := e.Repo.Add(filepath.Join(".buildkite", "hooks")); err != nil { + return fmt.Errorf("adding local hooks: %w", err) + } + if err := e.Repo.Commit("Added local hook files"); err != nil { + return fmt.Errorf("committing local hooks: %w", err) } - return e.hookMock.Expect("local", name) + e.pendingLocalHooks = false + return nil } // ExpectGlobalHook creates a mock object and a script in the global buildkite hooks dir @@ -269,6 +281,10 @@ func (e *ExecutorTester) ExpectGlobalHook(name string) *bintest.Expectation { func (e *ExecutorTester) Run(t *testing.T, env ...string) error { t.Helper() + if err := e.flushPendingLocalHooks(); err != nil { + return err + } + // Mock out the meta-data calls to the agent after checkout if !e.HasMock("buildkite-agent") { agent := e.MockAgent(t) diff --git a/internal/job/integration/git.go b/internal/job/integration/git.go index 1e0c440ca2..489f35ae99 100644 --- a/internal/job/integration/git.go +++ b/internal/job/integration/git.go @@ -2,18 +2,46 @@ package integration import ( "fmt" + "io" "log" "os" "os/exec" "path/filepath" "strings" + "sync" ) type gitRepository struct { Path string } +var testGitRepositoryTemplate = sync.OnceValues(func() (string, error) { + repo, err := createSeedTestGitRepository() + if err != nil { + return "", err + } + return repo.Path, nil +}) + func createTestGitRespository() (*gitRepository, error) { + templatePath, err := testGitRepositoryTemplate() + if err != nil { + return nil, err + } + + repoPath, err := newGitRepositoryPath() + if err != nil { + return nil, err + } + if err := copyDirectory(templatePath, repoPath); err != nil { + _ = os.RemoveAll(repoPath) + return nil, fmt.Errorf("copying git repo template: %w", err) + } + + return &gitRepository{Path: repoPath}, nil +} + +func createSeedTestGitRepository() (*gitRepository, error) { repo, err := newGitRepository() if err != nil { return nil, err @@ -95,9 +123,25 @@ func createTestGitRespository() (*gitRepository, error) { } func newGitRepository() (*gitRepository, error) { + tempDir, err := newGitRepositoryPath() + if err != nil { + return nil, err + } + + gr := &gitRepository{Path: tempDir} + gitErr := gr.ExecuteAll([][]string{ + {"init"}, + {"config", "user.email", "you@example.com"}, + {"config", "user.name", "Your Name"}, + }) + + return gr, gitErr +} + +func newGitRepositoryPath() (string, error) { tempDirRaw, err := os.MkdirTemp("", "git-repo") if err != nil { - return nil, fmt.Errorf("Error creating temp dir: %w", err) + return "", fmt.Errorf("Error creating temp dir: %w", err) } // io.TempDir on Windows tilde-shortens (8.3 style?) long filenames in the path. @@ -118,17 +162,68 @@ func newGitRepository() (*gitRepository, error) { // EvalSymlinks seems best? Maybe there's a better way? tempDir, err := filepath.EvalSymlinks(tempDirRaw) if err != nil { - return nil, fmt.Errorf("EvalSymlinks for temp dir: %w", err) + return "", fmt.Errorf("EvalSymlinks for temp dir: %w", err) } + return tempDir, nil +} - gr := &gitRepository{Path: tempDir} - gitErr := gr.ExecuteAll([][]string{ - {"init"}, - {"config", "user.email", "you@example.com"}, - {"config", "user.name", "Your Name"}, +func copyDirectory(src, dst string) error { + return filepath.WalkDir(src, func(path string, d os.DirEntry, walkErr error) error { + if walkErr != nil { + return walkErr + } + + relPath, err := filepath.Rel(src, path) + if err != nil { + return err + } + if relPath == "." { + return nil + } + + targetPath := filepath.Join(dst, relPath) + info, err := os.Lstat(path) + if err != nil { + return err + } + + switch mode := info.Mode(); { + case mode.IsDir(): + return os.MkdirAll(targetPath, mode.Perm()) + case mode.IsRegular(): + return copyFile(path, targetPath, mode) + case mode&os.ModeSymlink != 0: + linkTarget, err := os.Readlink(path) + if err != nil { + return err + } + return os.Symlink(linkTarget, targetPath) + default: + return fmt.Errorf("unsupported file mode %s for %s", mode, path) + } }) +} - return gr, gitErr +func copyFile(src, dst string, mode os.FileMode) error { + reader, err := os.Open(src) + if err != nil { + return err + } + defer reader.Close() + + writer, err := os.OpenFile(dst, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, mode.Perm()) + if err != nil { + return err + } + + if _, err := io.Copy(writer, reader); err != nil { + writer.Close() + return err + } + if err := writer.Close(); err != nil { + return err + } + return os.Chmod(dst, mode.Perm()) } func (gr *gitRepository) Add(path string) error { diff --git a/internal/job/integration/perf_test.go b/internal/job/integration/perf_test.go new file mode 100644 index 0000000000..f86ebd5c2a --- /dev/null +++ b/internal/job/integration/perf_test.go @@ -0,0 +1,50 @@ +package integration + +import "testing" + +func BenchmarkNewExecutorTester(b *testing.B) { + b.ReportAllocs() + + for b.Loop() { + tester, err := NewExecutorTester(mainCtx) + if err != nil { + b.Fatalf("NewExecutorTester() error = %v", err) + } + if err := tester.Close(); err != nil { + b.Fatalf("tester.Close() error = %v", err) + } + } +} + +func BenchmarkFlushPendingLocalHooks(b *testing.B) { + b.ReportAllocs() + + hooks := []string{ + "environment", + "pre-checkout", + "post-checkout", + "pre-command", + "post-command", + "pre-artifact", + "post-artifact", + "pre-exit", + } + + for b.Loop() { + tester, err := NewExecutorTester(mainCtx) + if err != nil { + b.Fatalf("NewExecutorTester() error = %v", err) + } + + for _, hook := range hooks { + tester.ExpectLocalHook(hook).Once() + } + + if err := tester.flushPendingLocalHooks(); err != nil { + b.Fatalf("tester.flushPendingLocalHooks() error = %v", err) + } + if err := tester.Close(); err != nil { + b.Fatalf("tester.Close() error = %v", err) + } + } +}